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

[Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>]

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@citrix.com]
> >>>>>Sent: Friday, May 09, 2014 5:57 PM
> >>>>>To: Gonglei (Arei)
> >>>>>Cc: Jan Beulich; xen-devel@lists.xen.org; anthony.perard@citrix.com;
> >>>>>stefano.stabellini@eu.citrix.com; johannes.krampf@googlemail.com; Gaowei
> >>>>>(UVP); Hanweidong (Randy); Huangweidong (C); kevin@koconnor.net;
> >>>>>fabio.fantoni@m2r.biz; qemu-devel@nongnu.org; mst@redhat.com
> >>>>>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
	}

> 
> >
> >>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@lists.xen.org
> >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