RE: [PATCH] PCI: ACPI: Don't blindly trust `HotPlugSupportInD3`

["Limonciello, Mario" <Mario.Limonciello@xxxxxxx>]

[Public]



> -----Original Message-----
> From: Mika Westerberg <mika.westerberg@xxxxxxxxxxxxxxx>
> Sent: Thursday, March 10, 2022 03:56
> To: Limonciello, Mario <Mario.Limonciello@xxxxxxx>
> Cc: Bjorn Helgaas <bhelgaas@xxxxxxxxxx>; open list:PCI SUBSYSTEM <linux-
> pci@xxxxxxxxxxxxxxx>; Mehta, Sanju <Sanju.Mehta@xxxxxxx>; Rafael J.
> Wysocki <rafael.j.wysocki@xxxxxxxxx>
> Subject: Re: [PATCH] PCI: ACPI: Don't blindly trust `HotPlugSupportInD3`
> 
> +Rafael
> 
> On Wed, Mar 09, 2022 at 04:43:02PM -0600, Mario Limonciello wrote:
> > The `_DSD` `HotPlugSupportInD3` is supposed to indicate the ability for a
> > bridge to be able to wakeup from D3.
> >
> > This however is static information in the ACPI table at BIOS compilation
> > time and on some platforms it's possible to configure the firmware at boot
> > up such that `_S0W` will not return "0" indicating the inability to wake
> > up the system from D3.
> 
> Ideally the BIOS should not allow this to happen in the first place but
> yeah we've seen all kinds of weird behaviour in the past so just need
> to deal with it :/
> 
> I wonder if it makes sense to log this situation?
> 

We double checked how Windows handles these circumstances and this
change makes it match how Windows handles it too.

> > To fix these situations explicitly check that the ACPI device claims the
> > system can be awoken in `acpi_pci_bridge_d3`.
> >
> > Link:
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fuefi.
> org%2Fhtmlspecs%2FACPI_Spec_6_4_html%2F07_Power_and_Performance
> _Mgmt%2Fdevice-power-management-
> objects.html%3Fhighlight%3Ds0w%23s0w-s0-device-wake-
> state&amp;data=04%7C01%7Cmario.limonciello%40amd.com%7Cde80f5d9ce
> 484b608b0c08da027c84d3%7C3dd8961fe4884e608e11a82d994e183d%7C0%7
> C0%7C637825031070157114%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4w
> LjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&am
> p;sdata=mD6EJTcL5V0syIkscYe5kmmbyg3zDqEQBwP8zCAO5h4%3D&amp;res
> erved=0
> > Link:
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdocs
> .microsoft.com%2Fen-us%2Fwindows-hardware%2Fdrivers%2Fpci%2Fdsd-
> for-pcie-root-ports%23identifying-pcie-root-ports-supporting-hot-plug-in-
> d3&amp;data=04%7C01%7Cmario.limonciello%40amd.com%7Cde80f5d9ce48
> 4b608b0c08da027c84d3%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0
> %7C637825031070157114%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjA
> wMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;s
> data=S2JtOwrUHOb5VXNV3BepBS2VxlF0s9FW4DTMHC54ULU%3D&amp;res
> erved=0
> > Fixes: 26ad34d510a87 ("PCI / ACPI: Whitelist D3 for more PCIe hotplug
> ports")
> > Signed-off-by: Mario Limonciello <mario.limonciello@xxxxxxx>
> 
> Reviewed-by: Mika Westerberg <mika.westerberg@xxxxxxxxxxxxxxx>
> 

Thanks!

> > ---
> >  drivers/pci/pci-acpi.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c
> > index a42dbf448860..9f8f55ed09d9 100644
> > --- a/drivers/pci/pci-acpi.c
> > +++ b/drivers/pci/pci-acpi.c
> > @@ -999,6 +999,9 @@ bool acpi_pci_bridge_d3(struct pci_dev *dev)
> >  	if (!adev)
> >  		return false;
> >
> > +	if (!adev->wakeup.flags.valid)
> > +		return false;
> > +
> >  	if (acpi_dev_get_property(adev, "HotPlugSupportInD3",
> >  				   ACPI_TYPE_INTEGER, &obj) < 0)
> >  		return false;
> > --
> > 2.34.1