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

[Fabio Fantoni <fabio.fantoni@m2r.biz>]

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; kevin@koconnor.net; Huangweidong (C); Hanweidong
>>>>> (Randy); mst@redhat.com; qemu-devel@nongnu.org; xen-
>>>>> devel@lists.xen.org; fabio.fantoni@m2r.biz;
>>>>> johannes.krampf@googlemail.com; 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?

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