Re: [Xen-devel] [PATCH] tools/firmware: remove "_PS0/3" Method

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Fri, 2011-12-02 at 09:16 +0000, Hao, Xudong wrote:
> > -----Original Message-----
> > From: Ian Campbell [mailto:Ian.Campbell@xxxxxxxxxx]
> > Sent: Friday, December 02, 2011 4:48 PM
> > To: Hao, Xudong
> > Cc: xen-devel@xxxxxxxxxxxxxxxxxxx; 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@xxxxxxxxx>
> > > Signed-off-by: Haitao Shan <haitao.shan@xxxxxxxxx>
> > >
> > > 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@xxxxxxxxxxxxxxxxxxx
> > > http://lists.xensource.com/xen-devel
> > 
> 



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