tools/firmware: remove "_PS0/3" Method
Do not expose the ACPI power management "_PS0/3" Method to guest
firmware. According to section 3.4 of the APCI specification 4.0, PCI device
control the device power through its own specification but not through APCI.
Qemu pushes "_PS0/3" to guest will cause a mess between ACPI PM and
PCI PM as a result of incorrect ACPI table shipped with the guest BIOS, it may
cause a failure of PCI device PM state transition(from PCI_UNKNOWN to PCI_D0).
Signed-off-by: Xudong Hao <xudong.hao@intel.com>
Signed-off-by: Haitao Shan <haitao.shan@intel.com>
diff -r df7cec2c6c03 tools/firmware/hvmloader/acpi/mk_dsdt.c
--- a/tools/firmware/hvmloader/acpi/mk_dsdt.c Tue Nov 29 13:30:39 2011 -0500
+++ b/tools/firmware/hvmloader/acpi/mk_dsdt.c Wed Nov 30 15:08:20 2011 +0800
@@ -323,8 +323,6 @@
* the ACPI event:
* _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 @@
-344,13 +342,6 @@
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");
pop_block();
push_block("Method", "_EJ0, 1");
stmt("Store", "0x%02x, \\_GPE.DPT1", slot)
On Fri, 2011-12-02 at 08:38 +0000, Hao, Xudong wrote:> tools/firmware: remove "_PS0/3" Method > > Do not expose the ACPI power management "_PS0/3" Method to guest > firmware. According to section 3.4 of the APCI specification 4.0, PCI > device control the device power through its own specification but not > through APCI. > > Qemu pushes "_PS0/3" to guest will cause a mess between ACPI PM and > PCI PM as a result of incorrect ACPI table shipped with the guest > BIOS, it may cause a failure of PCI device PM state transition(from > PCI_UNKNOWN to PCI_D0).How have you tested this and with which guest OSes? Was there a specific failure which you observed? Is this a different fix for the issue fixed by Linux upstream changeset 47e9037ac166 "PCI hotplug: acpiphp: set current_state to D0 in register_slot"? Ian.> > Signed-off-by: Xudong Hao <xudong.hao@intel.com> > Signed-off-by: Haitao Shan <haitao.shan@intel.com> > > diff -r df7cec2c6c03 tools/firmware/hvmloader/acpi/mk_dsdt.c > --- a/tools/firmware/hvmloader/acpi/mk_dsdt.c Tue Nov 29 13:30:39 2011 -0500 > +++ b/tools/firmware/hvmloader/acpi/mk_dsdt.c Wed Nov 30 15:08:20 2011 +0800 > @@ -323,8 +323,6 @@ > * the ACPI event: > * _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 @@ -344,13 +342,6 @@ > 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"); > pop_block(); > push_block("Method", "_EJ0, 1"); > stmt("Store", "0x%02x, \\_GPE.DPT1", slot) > > > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xensource.com > http://lists.xensource.com/xen-devel
> -----Original Message----- > From: Ian Campbell [mailto:Ian.Campbell@citrix.com] > Sent: Friday, December 02, 2011 4:48 PM > To: Hao, Xudong > Cc: xen-devel@lists.xensource.com; Tian, Kevin; Keir Fraser; Shan, Haitao > Subject: Re: [Xen-devel] [PATCH] tools/firmware: remove "_PS0/3" Method > > On Fri, 2011-12-02 at 08:38 +0000, Hao, Xudong wrote: > > tools/firmware: remove "_PS0/3" Method > > > > Do not expose the ACPI power management "_PS0/3" Method to guest > > firmware. According to section 3.4 of the APCI specification 4.0, PCI > > device control the device power through its own specification but not > > through APCI. > > > > Qemu pushes "_PS0/3" to guest will cause a mess between ACPI PM and > > PCI PM as a result of incorrect ACPI table shipped with the guest > > BIOS, it may cause a failure of PCI device PM state transition(from > > PCI_UNKNOWN to PCI_D0). > > How have you tested this and with which guest OSes? Was there a specific > failure which you observed? >I tested it with SLES11 SP2 guest, where the assigned Kawela VF device can''t transit state to D0.> Is this a different fix for the issue fixed by Linux upstream changeset > 47e9037ac166 "PCI hotplug: acpiphp: set current_state to D0 in > register_slot"?I look at this patch, the acpiphp module set current_state to D0 as your patch. However, apciphp is used for PCI device hotplug. SELS11 SP2 build acpiphp as a module, my case is static assigning VF to guest when guest are creating, loading acpiphp module will workaround this failure, but the device static assignment should not be depend on hotplug mechanism.> > Ian. > > > > > Signed-off-by: Xudong Hao <xudong.hao@intel.com> > > Signed-off-by: Haitao Shan <haitao.shan@intel.com> > > > > diff -r df7cec2c6c03 tools/firmware/hvmloader/acpi/mk_dsdt.c > > --- a/tools/firmware/hvmloader/acpi/mk_dsdt.c Tue Nov 29 13:30:39 > 2011 -0500 > > +++ b/tools/firmware/hvmloader/acpi/mk_dsdt.c Wed Nov 30 15:08:20 > 2011 +0800 > > @@ -323,8 +323,6 @@ > > * the ACPI event: > > * _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 > @@ -344,13 +342,6 @@ > > 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"); > > pop_block(); > > push_block("Method", "_EJ0, 1"); > > stmt("Store", "0x%02x, \\_GPE.DPT1", slot) > > > > > > > > > > _______________________________________________ > > Xen-devel mailing list > > Xen-devel@lists.xensource.com > > http://lists.xensource.com/xen-devel >
On Fri, 2011-12-02 at 09:16 +0000, Hao, Xudong wrote:> > -----Original Message----- > > From: Ian Campbell [mailto:Ian.Campbell@citrix.com] > > Sent: Friday, December 02, 2011 4:48 PM > > To: Hao, Xudong > > Cc: xen-devel@lists.xensource.com; Tian, Kevin; Keir Fraser; Shan, Haitao > > Subject: Re: [Xen-devel] [PATCH] tools/firmware: remove "_PS0/3" Method > > > > On Fri, 2011-12-02 at 08:38 +0000, Hao, Xudong wrote: > > > tools/firmware: remove "_PS0/3" Method > > > > > > Do not expose the ACPI power management "_PS0/3" Method to guest > > > firmware. According to section 3.4 of the APCI specification 4.0, PCI > > > device control the device power through its own specification but not > > > through APCI. > > > > > > Qemu pushes "_PS0/3" to guest will cause a mess between ACPI PM and > > > PCI PM as a result of incorrect ACPI table shipped with the guest > > > BIOS, it may cause a failure of PCI device PM state transition(from > > > PCI_UNKNOWN to PCI_D0). > > > > How have you tested this and with which guest OSes? Was there a specific > > failure which you observed? > > > I tested it with SLES11 SP2 guest, where the assigned Kawela VF device can''t transit state to D0. > > > Is this a different fix for the issue fixed by Linux upstream changeset > > 47e9037ac166 "PCI hotplug: acpiphp: set current_state to D0 in > > register_slot"? > > I look at this patch, the acpiphp module set current_state to D0 as > your patch. However, apciphp is used for PCI device hotplug. > SELS11 SP2 build acpiphp as a module, my case is static assigning VF > to guest when guest are creating, loading acpiphp module will > workaround this failure, but the device static assignment should not > be depend on hotplug mechanism.Have you verified that hotplug still works after your change? Have you tested any other OSes? How does Windows for example respond to this change in the ACPI tables? Are there any devices which do not implement PCI PM and therefore rely on this ACPI mechanism to function? My understanding was that 47e9037ac166 was required in part due to the lack of PCI PM support on some VF devices. I think it was a different Intel SR-IOV NIC than the one you are testing, an 82559 if [0] is to be believed. Also there was a previous attempt to remove _PS0 in [1]. Allen Kay (CCd) tested and reported that removing these values causes Windows not to boot. It was suggested in that thread that both _PS0 and _PS3 need to be removed (which you do) but it was also suggested that doing so breaks Linux S3 support, have you tried this? Ian. [0] http://old-list-archives.xen.org/archives/html/xen-devel/2011-03/msg00434.html [1] http://old-list-archives.xen.org/archives/html/xen-devel/2011-02/threads.html> > > > Ian. > > > > > > > > Signed-off-by: Xudong Hao <xudong.hao@intel.com> > > > Signed-off-by: Haitao Shan <haitao.shan@intel.com> > > > > > > diff -r df7cec2c6c03 tools/firmware/hvmloader/acpi/mk_dsdt.c > > > --- a/tools/firmware/hvmloader/acpi/mk_dsdt.c Tue Nov 29 13:30:39 > > 2011 -0500 > > > +++ b/tools/firmware/hvmloader/acpi/mk_dsdt.c Wed Nov 30 15:08:20 > > 2011 +0800 > > > @@ -323,8 +323,6 @@ > > > * the ACPI event: > > > * _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 > > @@ -344,13 +342,6 @@ > > > 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"); > > > pop_block(); > > > push_block("Method", "_EJ0, 1"); > > > stmt("Store", "0x%02x, \\_GPE.DPT1", slot) > > > > > > > > > > > > > > > _______________________________________________ > > > Xen-devel mailing list > > > Xen-devel@lists.xensource.com > > > http://lists.xensource.com/xen-devel > > >
> Have you tested any other OSes? How does Windows for example respond to > this change in the ACPI tables? >Yes, I did some test with this patch, till now, all result shows patch works well with PCI device assign and hotplug, as well as HVM S3. Pass cases: RHEL6.1, SLES11 SP1, Win2008 VF device assign and hotplug. RHEL6.1, Winxp, Win7 e1000e NIC device assign and hotplug RHEL6.1, RHEL5.1 Guest S3> Are there any devices which do not implement PCI PM and therefore rely on > this ACPI mechanism to function? My understanding was that > 47e9037ac166 was required in part due to the lack of PCI PM support on some > VF devices. I think it was a different Intel SR-IOV NIC than the one you are > testing, an 82559 if [0] is to be believed. >VF is such device which do not have PCI PM capability, these device will be set PCI_D0 directly in function pci_platform_power_transition(). static int pci_platform_power_transition(struct pci_dev *dev, pci_power_t state) { int error; if (platform_pci_power_manageable(dev)) { error = platform_pci_set_power_state(dev, state); ... } else { error = -ENODEV; /* Fall back to PCI_D0 if native PM is not supported */ if (!dev->pm_cap) dev->current_state = PCI_D0;> Also there was a previous attempt to remove _PS0 in [1]. Allen Kay (CCd) tested > and reported that removing these values causes Windows not to boot. It was > suggested in that thread that both _PS0 and _PS3 need to be removed (which > you do) but it was also suggested that doing so breaks Linux S3 support, have > you tried this? >Windows does not to boot only happen when remove _PS0, however Windows guest can boot up with removing _PS0 and _PS3. According to the annotate of "_PS0/3", it''s for debug purpose. I do not know whether it''s required for other purpose, comments of others? Thanks, -Xudong> Ian. > > [0] > http://old-list-archives.xen.org/archives/html/xen-devel/2011-03/msg00434.ht > ml > [1] > http://old-list-archives.xen.org/archives/html/xen-devel/2011-02/threads.html > > > > > > > Ian. > > > > > > > > > > > Signed-off-by: Xudong Hao <xudong.hao@intel.com> > > > > Signed-off-by: Haitao Shan <haitao.shan@intel.com> > > > > > > > > diff -r df7cec2c6c03 tools/firmware/hvmloader/acpi/mk_dsdt.c > > > > --- a/tools/firmware/hvmloader/acpi/mk_dsdt.c Tue Nov 29 13:30:39 > > > 2011 -0500 > > > > +++ b/tools/firmware/hvmloader/acpi/mk_dsdt.c Wed Nov 30 > 15:08:20 > > > 2011 +0800 > > > > @@ -323,8 +323,6 @@ > > > > * the ACPI event: > > > > * _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 > > > @@ -344,13 +342,6 @@ > > > > 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"); > > > > pop_block(); > > > > push_block("Method", "_EJ0, 1"); > > > > stmt("Store", "0x%02x, \\_GPE.DPT1", slot) > > > > > > > > > > > > > > > > > > > > _______________________________________________ > > > > Xen-devel mailing list > > > > Xen-devel@lists.xensource.com > > > > http://lists.xensource.com/xen-devel > > > > > >
(Resend patch because the previous patch loss to remove one line code.)
tools/firmware: remove "_PS0/3" Method
Do not expose the ACPI power management "_PS0/3" Method to guest
firmware. According to section 3.4 of the APCI specification 4.0, PCI device
control the device power through its own specification but not through APCI.
Qemu pushes "_PS0/3" to guest will cause a mess between ACPI PM and
PCI PM as a result of incorrect ACPI table shipped with the guest BIOS, it may
cause a failure of PCI device PM state transition(from PCI_UNKNOWN to PCI_D0).
Signed-off-by: Xudong Hao <xudong.hao@intel.com>
Signed-off-by: Haitao Shan <haitao.shan@intel.com>
diff -r df7cec2c6c03 tools/firmware/hvmloader/acpi/mk_dsdt.c
--- a/tools/firmware/hvmloader/acpi/mk_dsdt.c Tue Nov 29 13:30:39 2011 -0500
+++ b/tools/firmware/hvmloader/acpi/mk_dsdt.c Fri Dec 02 21:56:14 2011 +0800
@@ -323,8 +323,6 @@
* the ACPI event:
* _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
@@ -344,14 +342,6 @@
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");
- pop_block();
push_block("Method", "_EJ0, 1");
stmt("Store", "0x%02x, \\_GPE.DPT1", slot);
stmt("Store", "0x88, \\_GPE.DPT2");
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Thanks,
-Xudong
> -----Original Message-----
> From: xen-devel-bounces@lists.xensource.com
> [mailto:xen-devel-bounces@lists.xensource.com] On Behalf Of Hao, Xudong
> Sent: Friday, December 02, 2011 4:39 PM
> To: xen-devel@lists.xensource.com
> Cc: Tian, Kevin; Keir Fraser; Shan, Haitao
> Subject: [Xen-devel] [PATCH] tools/firmware: remove "_PS0/3"
Method
>
> tools/firmware: remove "_PS0/3" Method
>
> Do not expose the ACPI power management "_PS0/3" Method to guest
> firmware. According to section 3.4 of the APCI specification 4.0, PCI
device
> control the device power through its own specification but not through
APCI.
>
> Qemu pushes "_PS0/3" to guest will cause a mess between ACPI PM
and PCI
> PM as a result of incorrect ACPI table shipped with the guest BIOS, it may
> cause a failure of PCI device PM state transition(from PCI_UNKNOWN to
> PCI_D0).
>
> Signed-off-by: Xudong Hao <xudong.hao@intel.com>
> Signed-off-by: Haitao Shan <haitao.shan@intel.com>
>
> diff -r df7cec2c6c03 tools/firmware/hvmloader/acpi/mk_dsdt.c
> --- a/tools/firmware/hvmloader/acpi/mk_dsdt.c Tue Nov 29 13:30:39 2011
> -0500
> +++ b/tools/firmware/hvmloader/acpi/mk_dsdt.c Wed Nov 30 15:08:20 2011
> +0800
> @@ -323,8 +323,6 @@
> * the ACPI event:
> * _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 @@
> -344,13 +342,6 @@
> 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");
> pop_block();
> push_block("Method", "_EJ0, 1");
> stmt("Store", "0x%02x, \\_GPE.DPT1", slot)
>
>
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
On Sun, 2011-12-04 at 04:38 +0000, Hao, Xudong wrote:> > Have you tested any other OSes? How does Windows for example respond to > > this change in the ACPI tables? > > > > Yes, I did some test with this patch, till now, all result shows patch > works well with PCI device assign and hotplug, as well as HVM S3. > > Pass cases: > RHEL6.1, SLES11 SP1, Win2008 VF device assign and hotplug. > RHEL6.1, Winxp, Win7 e1000e NIC device assign and hotplug > RHEL6.1, RHEL5.1 Guest S3 > > > Are there any devices which do not implement PCI PM and therefore rely on > > this ACPI mechanism to function? My understanding was that > > 47e9037ac166 was required in part due to the lack of PCI PM support on some > > VF devices. I think it was a different Intel SR-IOV NIC than the one you are > > testing, an 82559 if [0] is to be believed. > > > VF is such device which do not have PCI PM capability, these device > will be set PCI_D0 directly in function > pci_platform_power_transition(). > > static int pci_platform_power_transition(struct pci_dev *dev, pci_power_t state) > { > int error; > if (platform_pci_power_manageable(dev)) { > error = platform_pci_set_power_state(dev, state); > ... > } else { > error = -ENODEV; > /* Fall back to PCI_D0 if native PM is not supported */ > if (!dev->pm_cap) > dev->current_state = PCI_D0; > > > Also there was a previous attempt to remove _PS0 in [1]. Allen Kay (CCd) tested > > and reported that removing these values causes Windows not to boot. It was > > suggested in that thread that both _PS0 and _PS3 need to be removed (which > > you do) but it was also suggested that doing so breaks Linux S3 support, have > > you tried this? > > > Windows does not to boot only happen when remove _PS0, however Windows > guest can boot up with removing _PS0 and _PS3. > > According to the annotate of "_PS0/3", it''s for debug purpose. I do > not know whether it''s required for other purpose, comments of others?Your explanation / results are good enough for me. Thanks, Ian.
Possibly Parallel Threads
- Preventing hotplug devices in HVM guests
- Re: [Qemu-devel] Hvmloader: Modify ACPI to only supply _EJ0 methods for PCIslots that support hotplug by runtime patching
- [PATCH v3] Fix the mistake of exception execution
- [PATCH v2 0/4] XEN: fix vmx exception mistake
- [PATCH 0/3] XEN: fix vmx exception mistake