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

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

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 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.


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