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/09/2014 12:00 PM, Konrad Rzeszutek Wilk wrote:
> On Fri, May 09, 2014 at 10:38:49AM -0400, Ross Philipson wrote:
> > On 05/09/2014 09:31 AM, Ian Campbell wrote:
> > > On Fri, 2014-05-09 at 09:25 -0400, Konrad Rzeszutek Wilk wrote:
> > > > On Fri, May 09, 2014 at 11:26:35AM +0100, Ian Campbell wrote:
> > > > > On Fri, 2014-05-09 at 10:15 +0000, Gonglei (Arei) wrote:
> > > > > > Hi,
> > > > > >
> > > > > > First, please forgive me for my bad English.
> > > > > > It's so sad.
> > > > > >
> > > > > > > -----Original Message-----
> > > > > > > From: Ian Campbell [mailto:Ian.Campbell@xxxxxxxxxx]
> > > > > > > Sent: Friday, May 09, 2014 5:57 PM
> > > > > > > To: Gonglei (Arei)
> > > > > > > Cc: Jan Beulich; xen-devel@xxxxxxxxxxxxx; anthony.perard@xxxxxxxxxx;
> > > > > > > stefano.stabellini@xxxxxxxxxxxxx; johannes.krampf@xxxxxxxxxxxxxx; Gaowei
> > > > > > > (UVP); Hanweidong (Randy); Huangweidong (C); kevin@xxxxxxxxxxxx;
> > > > > > > fabio.fantoni@xxxxxxx; qemu-devel@xxxxxxxxxx; mst@xxxxxxxxxx
> > > > > > > Subject: Re: [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods
> > > > > > > for PCIslots that support hotplug by runtime patching
> > > > > > >
> > > > > > > On Fri, 2014-05-09 at 09:45 +0000, Gonglei (Arei) wrote:
> > > > > > > > > And it also seem pretty pointless to send a v4 without addressing
> > > > > > > > > all comments you got on v3.
> > > > > > > > I don't think so. I have absorbed Ian's all suggestion on v3. And for other
> > > > > > > > questions have been answered too, in despite of is me or not.
> > > > > > > Actually you haven't answered "Why is runtime patching the only
> > > > > > > option here?" which was originally phrased as:
> > > > > > > > > Which appears to involve an awful lot of jumping through hoops... Please
> > > > > > > > > can you explain why it is necessary, as opposed to e.g. using a dynamic
> > > > > > > > > set of SSDTs?
> > > > > > Ian, I understand your mean now, which consider our method to address
> > > > > > this issue is maybe unnecessary, right? And you suggest us to use a dynamic
> > > > > > set of SSDTs.
> > > > > Really what I'm asking is what set of constraints and requirements led
> > > > > you to this particular solution.
> > > > >
> > > > > I think the method seems complicated, and I'd therefore like to know why
> > > > > it was preferred over other alternatives, or perhaps why it is the only
> > > > > option.
> > > > >
> > > > > > TBH I don't know more about the dynamic SSDTs, if you have any details,
> > > > > > tell me please, thanks in advance!
> > > > > I'm not an ACPI expert, but AIUI an SSDT is essentially a little piece
> > > > > of DSDT which is grafted onto the main DSDT at runtime by the OSPM. They
> > > > > make it somewhat easier for BIOS (or ACPI table) authors to include or
> > > > > exclude functionality at runtime, perhaps on a physical system in
> > > > > response to a user changing something in the BIOS setup screens. In Xen
> > > > > we appear to use SSDTs for HPET, TPM and S3/S4 functionality, depending
> > > > > on the guest configuration
> > > > > (hvmloader/acpi/build.c:construct_secondary_tables()).
> > > > Can it be used to patch the DSDT? Or were you (Ian) thinking that the bulk
> > > > of the ACPI PCI stuff can be moved there ?
> > > I think it can "shadow" or extend existing DSDT stuff, I don't think it
> > > can patch as sych. But here we want to dynamically add an entire method
> > > I think? (or hide, but I don't think that is possible).
> > Yea the SSDTs extend the DSDT. The DSDT is loaded to create the name space
> > and then SSDTs are loaded and added to the name space. If you need to make
> > runtime modifications like this, it is much easier to do it in an SSDT as
> > you suggest. What I don't know is whether you could extend say a device, as
> > in this case, with with a single method in a separate SSDT. I have never
> > really seen something like that before.
> >
> > WRT to hide vs. remove, I believe the intent is to effectively remove the
> > eject method from a given device by renaming it. It could simply be removed
> > making the device not eject-able but I think they are trying to avoid having
> > to recalculate and update the checksum on the DSDT.
> >
> > As to whether this has to be done at runtime, I don't know. If it does, I
> > wrote a lib that can generate basically any (rev2) AML on the fly. We used
> > it e.g. to generate WMI functionality in an SSDT at runtime. I would be
> > happy to share if it is useful.
> So we could just then gat the _EJ0 functionality based on values that
> are present (or not) in the SSDT ?
>
> Something like this in SSDT:
>
> Name (EJA, Package(), {0,1,1,1,..})
>
> (to be generated by hvmloader)
>
> And in the DSDT do a similar mechanims that the CPU hotplug does.
> Something like this for EJ0 for function SLOT 0->7:
>         Store (ToBuffer (EJA), Local0)
>         Store (DerefOf (Index(Local0, Zero)), Local1)
>         /* First eight bits in local1 */
>         And (Local1, One, Local2)
>         /* EJA[0:7] & 1 is stored in Local2 */
>         Store(Local2, "\\_GPE.DPT1")
>         /* In the debug port it goes. */
>         If (LNotEqual(Local2, 1) {
>                 // Don't do the EJ0
>         }
Well something like this might work as long as the OSPM checked to see 
if the device did in fact eject after calling this. If it just assumes 
the eject succeeds, things could go badly.
If something like this works, why not go a step farther and just invent 
an OpRegion, maybe in IO space, and have each _EJ0 method ask at runtime?
> > > > How would this work with the 'secondary emulator' patches that
> > > > do all of this PCI hotplug in the hypervisor?
> > > It should just mean that the magic I/O port protocol becomes backed by
> > > Xen, although now you mentioned it I suppose it will be necessary for
> > > Xen to know the answer now ;-)
> > >
> > > > (CC-ing the author
> > > > of said patches).
> > >   I thought I did earlier, perhaps I forgot or changed my mind.
> > >
> > > Ian.
> > >
> > >
> > > _______________________________________________
> > > Xen-devel mailing list
> > > Xen-devel@xxxxxxxxxxxxx
> > > http://lists.xen.org/xen-devel
> > >
> > > -----
> > > No virus found in this message.
> > > Checked by AVG - www.avg.com
> > > Version: 2014.0.4570 / Virus Database: 3931/7443 - Release Date: 05/05/14
> >
> > --
> > Ross Philipson
> -----
> No virus found in this message.
> Checked by AVG - www.avg.com
> Version: 2014.0.4570 / Virus Database: 3931/7443 - Release Date: 05/05/14

--
Ross Philipson

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