Re: [Xen-devel] [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods for PCIslots that support hotplug by runtime patching

[Ross Philipson <ross.philipson@xxxxxxxxxx>]

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@xxxxxxxxxxxx; Huangweidong (C); Hanweidong
> > > > (Randy); mst@xxxxxxxxxx; qemu-devel@xxxxxxxxxx; xen-
> > > > devel@xxxxxxxxxxxxx; fabio.fantoni@xxxxxxx;
> > > > johannes.krampf@xxxxxxxxxxxxxx; 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

Attachment: SSDT2.dsl
Description: Text Data

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel