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

[Pasi Kärkkäinen]

Hello,

On Fri, Aug 22, 2014 at 08:45:03AM +0000, Gonglei (Arei) wrote:
> > From: Konrad Rzeszutek Wilk [mailto:address@hidden
> > Sent: Thursday, August 21, 2014 6:31 AM
> > To: Fabio Fantoni
> > Cc: Ross Philipson; Ian Campbell; Paul Durrant; address@hidden;
> > Huangweidong (C); Hanweidong (Randy); address@hidden;
> > address@hidden; address@hidden;
> > address@hidden; Gonglei (Arei); Stefano Stabellini; Gaowei
> > (UVP); Jan Beulich; Anthony Perard
> > Subject: Re: [Xen-devel] [PATCH v4] Hvmloader: Modify ACPI to only supply 
> > _EJ0
> > methods for PCIslots that support hotplug by runtime patching
> > 
> > On Wed, Aug 20, 2014 at 02:11:48PM +0200, Fabio Fantoni wrote:
> > > Il 12/05/2014 16:32, Ross Philipson ha scritto:
> > > >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; address@hidden; Huangweidong (C);
> > Hanweidong
> > > >>>>>(Randy); address@hidden; address@hidden; xen-
> > > >>>>>address@hidden; address@hidden;
> > > >>>>>address@hidden; 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
> > >
> > > Ping...
> > > Are there any news about this patch?
> > 
> > I think we are waiting on the patch submitter to do some homework
> > and reimplement the patch based on our feedback.
> > >
> 
> I' m so sorry. It's so long time.
> 
> And this work is not a top job for me right now.
>

Now that Xen 4.6 development has been opened, it would be good to get this ACPI 
hotplug issue fixed aswell.

Gonglei: Do you think you'll have time to look at this, or should someone else 
take a look at it? 


Thanks,

-- Pasi

 
> Best regards,
> -Gonglei
> 
> > > Thanks for any reply.
> > >
> > > >>>>>
> > > >>>>>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.
> > > >>
> > > >
> > > >
> > >
> 
> _______________________________________________
> Xen-devel mailing list
> address@hidden
> http://lists.xen.org/xen-devel