From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38903) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WjrIg-0003hY-Gr for qemu-devel@nongnu.org; Mon, 12 May 2014 10:33:58 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WjrIb-0007CE-Ow for qemu-devel@nongnu.org; Mon, 12 May 2014 10:33:54 -0400 Received: from smtp02.citrix.com ([66.165.176.63]:20123) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WjrIb-0007Bq-Fl for qemu-devel@nongnu.org; Mon, 12 May 2014 10:33:49 -0400 Message-ID: <5370DB8F.4000807@citrix.com> Date: Mon, 12 May 2014 10:32:47 -0400 From: Ross Philipson MIME-Version: 1.0 References: <1399625235-8216-1-git-send-email-arei.gonglei@huawei.com> <536CBD840200007800010D2D@mail.emea.novell.com> <33183CC9F5247A488A2544077AF19020815E7487@SZXEMA503-MBS.china.huawei.com> <1399629430.9513.143.camel@kazak.uk.xensource.com> <33183CC9F5247A488A2544077AF19020815E74DA@SZXEMA503-MBS.china.huawei.com> <1399631195.9513.149.camel@kazak.uk.xensource.com> <20140509132553.GA3057@phenom.dumpdata.com> <1399642266.561.14.camel@kazak.uk.xensource.com> <536CE879.4050303@citrix.com> <20140509160039.GA5721@phenom.dumpdata.com> <1399651933.561.56.camel@kazak.uk.xensource.com> <9AAE0902D5BC7E449B7C8E4E778ABCD034E037@AMSPEX01CL01.citrite.net> <536D114A.3050409@citrix.com> <1399885519.561.89.camel@kazak.uk.xensource.com> In-Reply-To: <1399885519.561.89.camel@kazak.uk.xensource.com> Content-Type: multipart/mixed; boundary="------------080608070506080200020000" Subject: Re: [Qemu-devel] [Xen-devel] [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods for PCIslots that support hotplug by runtime patching List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Ian Campbell Cc: "Huangweidong (C)" , "Gonglei (Arei)" , Konrad Rzeszutek Wilk , "Hanweidong (Randy)" , "mst@redhat.com" , "qemu-devel@nongnu.org" , "xen-devel@lists.xen.org" , "fabio.fantoni@m2r.biz" , Anthony Perard , "kevin@koconnor.net" , Stefano Stabellini , "Gaowei (UVP)" , Jan Beulich , "johannes.krampf@googlemail.com" , Paul Durrant --------------080608070506080200020000 Content-Type: text/plain; charset="ISO-8859-1"; format=flowed Content-Transfer-Encoding: 7bit On 05/12/2014 05:05 AM, Ian Campbell wrote: > On Fri, 2014-05-09 at 13:32 -0400, Ross Philipson wrote: >> On 05/09/2014 12:34 PM, Paul Durrant wrote: >>>> -----Original Message----- >>>> From: Ian Campbell >>>> Sent: 09 May 2014 17:12 >>>> To: Konrad Rzeszutek Wilk >>>> Cc: Ross Philipson; kevin@koconnor.net; Huangweidong (C); Hanweidong >>>> (Randy); mst@redhat.com; qemu-devel@nongnu.org; xen- >>>> devel@lists.xen.org; fabio.fantoni@m2r.biz; >>>> johannes.krampf@googlemail.com; Gonglei (Arei); Stefano Stabellini; >>>> Gaowei (UVP); Jan Beulich; Anthony Perard; Paul Durrant >>>> Subject: Re: [Xen-devel] [PATCH v4] Hvmloader: Modify ACPI to only supply >>>> _EJ0 methods for PCIslots that support hotplug by runtime patching >>>> >>>> On Fri, 2014-05-09 at 12:00 -0400, Konrad Rzeszutek Wilk wrote: >>>> >>>>> So we could just then gat the _EJ0 functionality based on values that >>>>> are present (or not) in the SSDT ? >>>> >>>> AIUI the very presence of _EJ0 is what marks the device as being >>>> ejectable (e.g. in the Windows device manager). >>>> >>>> It would be possible to make _EJ0 conditionally turn itself into a NOP >>>> without resorting to an SSDT, but I don't think that solves the issue >>>> they are trying to solve, which is that the user can even try to eject >>>> an non-hotplug device. (grep for UAR1 in our dsdt.asl and >>>> acpi_info->com1_present in hvmloader/acpi/build.c for an example of this >>>> sort of conditional thing) >>>> >> >> Going back to the SSDT idea. A little poking around and what not and I >> came up with something like this that I build into an SSDT: >> >> DefinitionBlock ("SSDTX.aml", "SSDT", 2, "Xen", "HVM", 0) >> { >> /* S00 device is defined in DSDT, this allows me to >> * refrence it in this SSDT >> */ >> External (\_SB.PCI0.S00, DeviceObj) >> >> ... >> >> /* Extend the functionality of S00 */ >> Scope ( \_SB.PCI0.S00 ) { >> Method(_EJ0, 1, NotSerialized) >> { >> /* Do stuffs here */ >> } >> } >> } > > Thanks, this looks like the sort of thing I was naively imagining would > be possible. > >> So I did find some examples of this after all in my pile of ACPI >> firmware snapshots from all our supported platforms. > > Thanks (none of the machines I looked at had PCI hotplug apparently). I > was curious to know how Real Firmware Engineers(tm) dealt with this sort > of issue. > > I was worried how real life OSPMs might interpret this method being in > an SSDT instead of the DSDT. In theory it shouldn't matter, and the fact > that real firmware does this seem to suggest that at least Windows > treats it that way (which is a relief). I did actually find SSDTs that were specifically adding an _EJ0 to a device scope for a device defined externally. I attached an example from a Fujitsu system I have. The PRT1 device on SAT0 is external: External (\_SB_.PCI0.SAT0.PRT1, DeviceObj) And _EJ0 is added to the scope. > >> I think this would >> work allowing you to just add or not add _EJ0 methods to the PCI devices >> you want by either using different SSDTs or doing something to generate >> or munge the SSDT at runtime (which would be simpler than messing with >> the DSDT I think. > > Without filling out the body of _EJ0 (which I tried but failed to do) > your stub compiles to 60 bytes of AML, I suppose that even having filled > in _EJ0 in the result would be less than, say, 128 bytes. > > Given that there are 32 PCI slots we would be talking about a total of > 4k of space in hvmloader to provide a precompiled SSDT for each slot, > which can be inserted at runtime depending on each slots configuration. > > I wouldn't be especially surprised if the code to generate a suitable > SSDT dynamically was a reasonable proportion of that size, so unless > there is the possibility of needing other variants it seems like just > generating each of them would be the say to go. > >> I did not try it (actually I did but ran into other >> problems on our platform :). > > ;-) > > Ian. > -- Ross Philipson --------------080608070506080200020000 Content-Type: text/x-dsl; name="SSDT2.dsl" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="SSDT2.dsl" /* * Intel ACPI Component Architecture * AML Disassembler version 20100528 * * Disassembly of ./SSDT2.bin, Mon May 12 09:48:37 2014 * * * Original Table Header: * Signature "SSDT" * Length 0x000002C3 (707) * Revision 0x01 * Checksum 0x2C * OEM ID "FUJ " * OEM Table ID "FJNB210 " * OEM Revision 0x01200000 (18874368) * Compiler ID "FUJ " * Compiler Version 0x00000100 (256) */ DefinitionBlock ("./SSDT2.aml", "SSDT", 1, "FUJ ", "FJNB210 ", 0x01200000) { External (PIID, IntObj) External (SMIC) External (DID_) External (BCMD) External (SSTA) External (PRDE) External (PRDS) External (BAYF) External (DPCS, MethodObj) // 1 Arguments External (D1EN, IntObj) External (BY1O) External (BY1C) External (\_SB_.PCI0.SAT0.PMMN) External (\_SB_.PCI0.SAT0.PRT1, DeviceObj) Name (BYIS, 0xFF) Scope (\_SB.PCI0.SAT0.PRT1) { Method (_STA, 0, NotSerialized) { If (LAnd (LEqual (BY1C, Zero), BY1O)) { Return (0x0F) } Else { Return (0x00) } } Method (_EJ0, 1, NotSerialized) { BOFF () Return (Zero) } Method (BON, 0, NotSerialized) { If (D1EN) { If (LEqual (BY1C, Zero)) { Store (One, BY1O) Sleep (0x46) } } } Method (BOFF, 0, NotSerialized) { Store (Zero, BY1O) Sleep (0x46) } Method (EON, 0, NotSerialized) { \_SB.PCI0.SAT0.PRT1.BON () If (LAnd (LEqual (BY1C, Zero), BY1O)) { Notify (\_SB.PCI0.SAT0.PRT1, One) } } Method (EOFF, 0, NotSerialized) { \_SB.PCI0.SAT0.PRT1.BOFF () Notify (\_SB.PCI0.SAT0.PRT1, One) } Method (EPTS, 0, NotSerialized) { If (\_SB.PCI0.SAT0.PRT1._STA ()) { Store (Zero, BYIS) } Else { Store (0xFF, BYIS) } } Method (EWAK, 0, NotSerialized) { If (LEqual (BYIS, 0xFF)) { If (LEqual (BY1C, Zero)) { If (DPCS (\_SB.PCI0.SAT0.PRT1.BOFF ())) {} Else { If (LNot (\_SB.PCI0.SAT0.PRT1._STA ())) { Store (0x01, BAYF) Store (0x01, PRDS) Store (0x01, PRDE) } Else { Store (0x01, BAYF) Store (0x01, PRDS) Store (0x01, PRDE) } } } Else { Noop } } Else { If (LNot (\_SB.PCI0.SAT0.PRT1._STA ())) { Notify (\_SB.PCI0.SAT0.PRT1, One) } Else { If (LEqual (SSTA, 0x04)) { Store (0x87, BCMD) Store (0x01, DID) Store (Zero, SMIC) Name (BFMN, Buffer (0x28) {}) Store (PIID, BFMN) Store (Zero, Local0) Store (Zero, Local1) While (LAnd (LNot (Local0), LLess (Local1, 0x28))) { If (LEqual (DerefOf (Index (\_SB.PCI0.SAT0.PMMN, Local1)), DerefOf (Index (BFMN, Local1)))) { Increment (Local1) } Else { Store (0x01, Local0) } } If (Local0) { \_SB.PCI0.SAT0.PRT1.BOFF () Notify (\_SB.PCI0.SAT0.PRT1, One) } } } If (LAnd (LEqual (BY1C, Zero), LNot (\_SB.PCI0.SAT0.PRT1._STA ()))) { Store (0x01, BAYF) Store (0x01, PRDS) Store (0x01, PRDE) } } } } } --------------080608070506080200020000--