[PATCH] remove _PS0 from the DSDT

[PATCH] remove _PS0 from the DSDT

[Stefano Stabellini]

This patch removes all the _PS0 entries from the DSDT.
The reason for removing them is that if a passthrough device doesn't
have power management capabilities declared in its pci config space but
_PS0 is declared in the DSDT a Linux HVM guest gets confused and cannot
set the device to D0.  In order to reproduce this problem you can try to
assign a VF to a Linux HVM guest, the result is a guest kernel crash as
reported in this bug:
http://bugzilla.xensource.com/bugzilla/show_bug.cgi?id=1742.
I tested this patch with Linux and Windows guests and it looks like it
is not breaking anything (guest S3 in particular is working).
However I am afraid it could cause unwanted side effects, specifically
it could break gfx passthrough of Intel GPUs (I haven't tested this on xen
4.1 but it looks like it would break gfx passthrough on xen 3.4).
Could anyone from Intel confirm whether this patch is correct or might
cause any problems?

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

diff -r 13a7d1f7f62c tools/firmware/hvmloader/acpi/mk_dsdt.c
--- a/tools/firmware/hvmloader/acpi/mk_dsdt.c	Mon Feb 21 09:11:57 2011 +0000
+++ b/tools/firmware/hvmloader/acpi/mk_dsdt.c	Wed Feb 23 12:27:35 2011 +0000
@@ -252,7 +252,6 @@ int main(int argc, char **argv)
      *  _EJ0: eject a device
      *  _STA: return a device's status, e.g. enabled or removed
      * Other methods are optional: 
-     *  _PS0/3: put them here for debug purpose
      * 
      * Eject button would generate a general-purpose event, then the
      * control method for this event uses Notify() to inform OSPM which
@@ -271,10 +270,6 @@ int main(int argc, char **argv)
         stmt("Name", "_ADR, 0x%08x", ((slot & ~7) << 13) | (slot & 7));
         /* _SUN == dev */
         stmt("Name", "_SUN, 0x%08x", slot >> 3);
-        push_block("Method", "_PS0, 0");
-        stmt("Store", "0x%02x, \\_GPE.DPT1", slot);
-        stmt("Store", "0x80, \\_GPE.DPT2");
-        pop_block();
         push_block("Method", "_PS3, 0");
         stmt("Store", "0x%02x, \\_GPE.DPT1", slot);
         stmt("Store", "0x83, \\_GPE.DPT2");

RE: [PATCH] remove _PS0 from the DSDT

[Kay, Allen M]

Hi Stefano,

I just tried this patch booting Win7-64bit in following two configurations:

    1) Passthrough two NIC devices - onboard + PCIe E1000
    2) Passthrough SNB IGD + USB + audio + NIC

In both cases, Windows failed to boot complaining about BIOS is not ACPI compliant.

If I don't passthrough any devices, Windows can boot successfully.

Allen

-----Original Message-----
From: Stefano Stabellini [mailto:stefano.stabellini@eu.citrix.com] 
Sent: Wednesday, February 23, 2011 6:18 AM
To: xen-devel@lists.xensource.com
Cc: Kay, Allen M; Ian Campbell; Jean.Guyader@citrix.com
Subject: [PATCH] remove _PS0 from the DSDT

This patch removes all the _PS0 entries from the DSDT.
The reason for removing them is that if a passthrough device doesn't
have power management capabilities declared in its pci config space but
_PS0 is declared in the DSDT a Linux HVM guest gets confused and cannot
set the device to D0.  In order to reproduce this problem you can try to
assign a VF to a Linux HVM guest, the result is a guest kernel crash as
reported in this bug:
http://bugzilla.xensource.com/bugzilla/show_bug.cgi?id=1742.
I tested this patch with Linux and Windows guests and it looks like it
is not breaking anything (guest S3 in particular is working).
However I am afraid it could cause unwanted side effects, specifically
it could break gfx passthrough of Intel GPUs (I haven't tested this on xen
4.1 but it looks like it would break gfx passthrough on xen 3.4).
Could anyone from Intel confirm whether this patch is correct or might
cause any problems?

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

diff -r 13a7d1f7f62c tools/firmware/hvmloader/acpi/mk_dsdt.c
--- a/tools/firmware/hvmloader/acpi/mk_dsdt.c	Mon Feb 21 09:11:57 2011 +0000
+++ b/tools/firmware/hvmloader/acpi/mk_dsdt.c	Wed Feb 23 12:27:35 2011 +0000
@@ -252,7 +252,6 @@ int main(int argc, char **argv)
      *  _EJ0: eject a device
      *  _STA: return a device's status, e.g. enabled or removed
      * Other methods are optional: 
-     *  _PS0/3: put them here for debug purpose
      * 
      * Eject button would generate a general-purpose event, then the
      * control method for this event uses Notify() to inform OSPM which
@@ -271,10 +270,6 @@ int main(int argc, char **argv)
         stmt("Name", "_ADR, 0x%08x", ((slot & ~7) << 13) | (slot & 7));
         /* _SUN == dev */
         stmt("Name", "_SUN, 0x%08x", slot >> 3);
-        push_block("Method", "_PS0, 0");
-        stmt("Store", "0x%02x, \\_GPE.DPT1", slot);
-        stmt("Store", "0x80, \\_GPE.DPT2");
-        pop_block();
         push_block("Method", "_PS3, 0");
         stmt("Store", "0x%02x, \\_GPE.DPT1", slot);
         stmt("Store", "0x83, \\_GPE.DPT2");

RE: [PATCH] remove _PS0 from the DSDTo

[Stefano Stabellini]

On Sat, 26 Feb 2011, Kay, Allen M wrote:
> Hi Stefano,
> 
> I just tried this patch booting Win7-64bit in following two configurations:
> 
>     1) Passthrough two NIC devices - onboard + PCIe E1000
>     2) Passthrough SNB IGD + USB + audio + NIC
> 
> In both cases, Windows failed to boot complaining about BIOS is not ACPI compliant.
> 
> If I don't passthrough any devices, Windows can boot successfully.
> 

In that case we have a problem because I don't see any other way we
could fix the issue in a way that is acceptable for xen 4.1.
We could emulate PM capabilities in qemu even for devices that don't
support it or make the presence of _PS0 conditional on the presence of
PM capabilities on the devices. In both cases the fix will miss 4.1.

However there is a simple workaround for it: just disable acpi in the
config file of the VM.

Re: RE: [PATCH] remove _PS0 from the DSDTo

[Keir Fraser]

On 28/02/2011 15:33, "Stefano Stabellini" <stefano.stabellini@eu.citrix.com>
wrote:

> On Sat, 26 Feb 2011, Kay, Allen M wrote:
>> Hi Stefano,
>> 
>> I just tried this patch booting Win7-64bit in following two configurations:
>> 
>>     1) Passthrough two NIC devices - onboard + PCIe E1000
>>     2) Passthrough SNB IGD + USB + audio + NIC
>> 
>> In both cases, Windows failed to boot complaining about BIOS is not ACPI
>> compliant.
>> 
>> If I don't passthrough any devices, Windows can boot successfully.
>> 
> 
> In that case we have a problem because I don't see any other way we
> could fix the issue in a way that is acceptable for xen 4.1.
> We could emulate PM capabilities in qemu even for devices that don't
> support it or make the presence of _PS0 conditional on the presence of
> PM capabilities on the devices. In both cases the fix will miss 4.1.

Stefano: Your patch should have deleted the _PS3 method along with _PS0. It
is an ACPI requirement that if you define an object to turn a device off
(i.e., _PS3 in this case) then you must also supply a symmetric object to
turn on the device (i.e., _PS0). So you must remove both, and there's no
reason not to since they both only contain debug stuff.

I suggest try again and see if _PS3 removal solves Allen's Windows boot
issue.

 -- Keir

> However there is a simple workaround for it: just disable acpi in the
> config file of the VM.

> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

Re: RE: [PATCH] remove _PS0 from the DSDTo

[Stefano Stabellini]

On Mon, 28 Feb 2011, Keir Fraser wrote:
> On 28/02/2011 15:33, "Stefano Stabellini" <stefano.stabellini@eu.citrix.com>
> wrote:
> 
> > On Sat, 26 Feb 2011, Kay, Allen M wrote:
> >> Hi Stefano,
> >> 
> >> I just tried this patch booting Win7-64bit in following two configurations:
> >> 
> >>     1) Passthrough two NIC devices - onboard + PCIe E1000
> >>     2) Passthrough SNB IGD + USB + audio + NIC
> >> 
> >> In both cases, Windows failed to boot complaining about BIOS is not ACPI
> >> compliant.
> >> 
> >> If I don't passthrough any devices, Windows can boot successfully.
> >> 
> > 
> > In that case we have a problem because I don't see any other way we
> > could fix the issue in a way that is acceptable for xen 4.1.
> > We could emulate PM capabilities in qemu even for devices that don't
> > support it or make the presence of _PS0 conditional on the presence of
> > PM capabilities on the devices. In both cases the fix will miss 4.1.
> 
> Stefano: Your patch should have deleted the _PS3 method along with _PS0. It
> is an ACPI requirement that if you define an object to turn a device off
> (i.e., _PS3 in this case) then you must also supply a symmetric object to
> turn on the device (i.e., _PS0). So you must remove both, and there's no
> reason not to since they both only contain debug stuff.
> 
> I suggest try again and see if _PS3 removal solves Allen's Windows boot
> issue.

It might be worth testing that case, but I am afraid it will break guest
S3 (at least for linux).

Re: [PATCH] remove _PS0 from the DSDTo

[Paolo Bonzini]

On 02/28/2011 04:33 PM, Stefano Stabellini wrote:
> However there is a simple workaround for it: just disable acpi in the
> config file of the VM.

I think recent Windows versions (Vista and newer) do not boot at all 
without ACPI.

Paolo

Re: [PATCH] remove _PS0 from the DSDTo

[Stefano Stabellini]

On Mon, 28 Feb 2011, Paolo Bonzini wrote:
> On 02/28/2011 04:33 PM, Stefano Stabellini wrote:
> > However there is a simple workaround for it: just disable acpi in the
> > config file of the VM.
> 
> I think recent Windows versions (Vista and newer) do not boot at all 
> without ACPI.
 
I am not suggesting to disable ACPI altogether. I am just saying that
the only way to get a recent Linux HVM guest to drive a VF is to disable
ACPI, unfortunately.

Re: Re: [PATCH] remove _PS0 from the DSDTo

[Jason Kwon]

Stefano Stabellini wrote:
> On Mon, 28 Feb 2011, Paolo Bonzini wrote:
>> On 02/28/2011 04:33 PM, Stefano Stabellini wrote:
>>> However there is a simple workaround for it: just disable acpi in the
>>> config file of the VM.
>> I think recent Windows versions (Vista and newer) do not boot at all 
>> without ACPI.
>  
> I am not suggesting to disable ACPI altogether. I am just saying that
> the only way to get a recent Linux HVM guest to drive a VF is to disable
> ACPI, unfortunately.
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
> 

Hello,

Has this hvmloader patch been finalized?  I updated to Xen 4.1 rc7/Dom0 
2.6.32.32, and saw that the register_slot ACPI fixes were in the Dom0 
kernel, but not this hvmloader change.  I previously tested this patch 
on Xen 4.1 rc4 and had success with it when booting linux guests.

Thanks,

Jason

Re: Re: [PATCH] remove _PS0 from the DSDTo

[Stefano Stabellini]

On Thu, 17 Mar 2011, Jason Kwon wrote:
> Stefano Stabellini wrote:
> > On Mon, 28 Feb 2011, Paolo Bonzini wrote:
> >> On 02/28/2011 04:33 PM, Stefano Stabellini wrote:
> >>> However there is a simple workaround for it: just disable acpi in the
> >>> config file of the VM.
> >> I think recent Windows versions (Vista and newer) do not boot at all 
> >> without ACPI.
> >  
> > I am not suggesting to disable ACPI altogether. I am just saying that
> > the only way to get a recent Linux HVM guest to drive a VF is to disable
> > ACPI, unfortunately.
> > 
> > _______________________________________________
> > Xen-devel mailing list
> > Xen-devel@lists.xensource.com
> > http://lists.xensource.com/xen-devel
> > 
> 
> Hello,
> 
> Has this hvmloader patch been finalized?  I updated to Xen 4.1 rc7/Dom0 
> 2.6.32.32, and saw that the register_slot ACPI fixes were in the Dom0 
> kernel, but not this hvmloader change.  I previously tested this patch 
> on Xen 4.1 rc4 and had success with it when booting linux guests.

we are trying to fix this on the Linux kernel side, we have two patches
waiting to be applied:

https://lkml.org/lkml/2011/2/28/296
https://lkml.org/lkml/2011/3/8/212

Re: Re: [PATCH] remove _PS0 from the DSDTo

[Jason Kwon]

On 03/17/2011 11:40 AM, Stefano Stabellini wrote:
> On Thu, 17 Mar 2011, Jason Kwon wrote:
>> Stefano Stabellini wrote:
>>> On Mon, 28 Feb 2011, Paolo Bonzini wrote:
>>>> On 02/28/2011 04:33 PM, Stefano Stabellini wrote:
>>>>> However there is a simple workaround for it: just disable acpi in the
>>>>> config file of the VM.
>>>> I think recent Windows versions (Vista and newer) do not boot at all
>>>> without ACPI.
>>>
>>> I am not suggesting to disable ACPI altogether. I am just saying that
>>> the only way to get a recent Linux HVM guest to drive a VF is to disable
>>> ACPI, unfortunately.
>>>
>>> _______________________________________________
>>> Xen-devel mailing list
>>> Xen-devel@lists.xensource.com
>>> http://lists.xensource.com/xen-devel
>>>
>> Hello,
>>
>> Has this hvmloader patch been finalized?  I updated to Xen 4.1 rc7/Dom0
>> 2.6.32.32, and saw that the register_slot ACPI fixes were in the Dom0
>> kernel, but not this hvmloader change.  I previously tested this patch
>> on Xen 4.1 rc4 and had success with it when booting linux guests.
> we are trying to fix this on the Linux kernel side, we have two patches
> waiting to be applied:
>
> https://lkml.org/lkml/2011/2/28/296
> https://lkml.org/lkml/2011/3/8/212
>
Thanks Stefano, I missed the second kernel patch.  Just to clarify, 
should these patches be applied to Dom0 or the guest kernel (or both)?

Jason

Re: Re: [PATCH] remove _PS0 from the DSDTo

[Stefano Stabellini]

On Thu, 17 Mar 2011, Jason Kwon wrote:
> On 03/17/2011 11:40 AM, Stefano Stabellini wrote:
> > On Thu, 17 Mar 2011, Jason Kwon wrote:
> >> Stefano Stabellini wrote:
> >>> On Mon, 28 Feb 2011, Paolo Bonzini wrote:
> >>>> On 02/28/2011 04:33 PM, Stefano Stabellini wrote:
> >>>>> However there is a simple workaround for it: just disable acpi in the
> >>>>> config file of the VM.
> >>>> I think recent Windows versions (Vista and newer) do not boot at all
> >>>> without ACPI.
> >>>
> >>> I am not suggesting to disable ACPI altogether. I am just saying that
> >>> the only way to get a recent Linux HVM guest to drive a VF is to disable
> >>> ACPI, unfortunately.
> >>>
> >>> _______________________________________________
> >>> Xen-devel mailing list
> >>> Xen-devel@lists.xensource.com
> >>> http://lists.xensource.com/xen-devel
> >>>
> >> Hello,
> >>
> >> Has this hvmloader patch been finalized?  I updated to Xen 4.1 rc7/Dom0
> >> 2.6.32.32, and saw that the register_slot ACPI fixes were in the Dom0
> >> kernel, but not this hvmloader change.  I previously tested this patch
> >> on Xen 4.1 rc4 and had success with it when booting linux guests.
> > we are trying to fix this on the Linux kernel side, we have two patches
> > waiting to be applied:
> >
> > https://lkml.org/lkml/2011/2/28/296
> > https://lkml.org/lkml/2011/3/8/212
> >
> Thanks Stefano, I missed the second kernel patch.  Just to clarify, 
> should these patches be applied to Dom0 or the guest kernel (or both)?
 
the guest's kernel

Re: Re: [PATCH] remove _PS0 from the DSDTo

[Jason Kwon]

On 03/17/2011 12:15 PM, Stefano Stabellini wrote:
> On Thu, 17 Mar 2011, Jason Kwon wrote:
>> On 03/17/2011 11:40 AM, Stefano Stabellini wrote:
>>> On Thu, 17 Mar 2011, Jason Kwon wrote:
>>>> Stefano Stabellini wrote:
>>>>> On Mon, 28 Feb 2011, Paolo Bonzini wrote:
>>>>>> On 02/28/2011 04:33 PM, Stefano Stabellini wrote:
>>>>>>> However there is a simple workaround for it: just disable acpi in the
>>>>>>> config file of the VM.
>>>>>> I think recent Windows versions (Vista and newer) do not boot at all
>>>>>> without ACPI.
>>>>> I am not suggesting to disable ACPI altogether. I am just saying that
>>>>> the only way to get a recent Linux HVM guest to drive a VF is to disable
>>>>> ACPI, unfortunately.
>>>>>
>>>>> _______________________________________________
>>>>> Xen-devel mailing list
>>>>> Xen-devel@lists.xensource.com
>>>>> http://lists.xensource.com/xen-devel
>>>>>
>>>> Hello,
>>>>
>>>> Has this hvmloader patch been finalized?  I updated to Xen 4.1 rc7/Dom0
>>>> 2.6.32.32, and saw that the register_slot ACPI fixes were in the Dom0
>>>> kernel, but not this hvmloader change.  I previously tested this patch
>>>> on Xen 4.1 rc4 and had success with it when booting linux guests.
>>> we are trying to fix this on the Linux kernel side, we have two patches
>>> waiting to be applied:
>>>
>>> https://lkml.org/lkml/2011/2/28/296
>>> https://lkml.org/lkml/2011/3/8/212
>>>
>> Thanks Stefano, I missed the second kernel patch.  Just to clarify,
>> should these patches be applied to Dom0 or the guest kernel (or both)?
>
> the guest's kernel
Does this mean VF passthrough will not be supported for guests running 
kernels older than 2.6.39 (or whichever kernel incorporates these 
patches)?  I was happy with the hvmloader patch, in that it allowed me 
to use VF passthrough with older kernels.

Jason

Re: Re: [PATCH] remove _PS0 from the DSDTo

[Stefano Stabellini]

On Thu, 17 Mar 2011, Jason Kwon wrote:
> Does this mean VF passthrough will not be supported for guests running 
> kernels older than 2.6.39 (or whichever kernel incorporates these 
> patches)?  I was happy with the hvmloader patch, in that it allowed me 
> to use VF passthrough with older kernels.
 
Unfortunately the hvmloader patch caused troubles when assigning PCI
devices to Windows guests.
I hope that after the two patches are applied to the 2.6.39 tree they'll
be backported to the stable trees as critical bug fixes.