On Tue, Mar 15, 2022 at 4:33 PM Mario Limonciello <mario.limonciello@xxxxxxx> wrote: > > According to the Microsoft spec the _DSD `HotPlugSupportInD3` is > indicates the ability for a bridge to be able to wakeup from D3: > > This ACPI object [HotPlugSupportInD3] enables the operating system > to identify and power manage PCIe Root Ports that are capable of > handling hot plug events while in D3 state. > > 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 returns "0" indicating the inability to wake up the > device from D3 as explained in the ACPI specification: > > 7.3.20 _S0W (S0 Device Wake State) > > This object evaluates to an integer that conveys to OSPM the deepest > D-state supported by this device in the S0 system sleeping state > where the device can wake itself. > > This mismatch may lead to being unable to enumerate devices behind the > hotplug bridge when a device is plugged in. To remedy these situations > that `HotPlugSupportInD3` is specified by _S0W returns 0, explicitly > check that the ACPI companion has returned _S0W greater than or equal > to 3 and the device has a GPE allowing the device to generate wakeup > signals handled by the platform in `acpi_pci_bridge_d3`. > > Windows 10 and Windows 11 both will prevent the bridge from going in D3 > when the firmware is configured this way and this changes aligns the > handling of the situation to be the same. > > Link: https://uefi.org/htmlspecs/ACPI_Spec_6_4_html/07_Power_and_Performance_Mgmt/device-power-management-objects.html?highlight=s0w#s0w-s0-device-wake-state > Link: https://docs.microsoft.com/en-us/windows-hardware/drivers/pci/dsd-for-pcie-root-ports#identifying-pcie-root-ports-supporting-hot-plug-in-d3 > Fixes: 26ad34d510a87 ("PCI / ACPI: Whitelist D3 for more PCIe hotplug ports") > Signed-off-by: Mario Limonciello <mario.limonciello@xxxxxxx> No more comments from me: Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx> > --- > changes from v3->v4: > * rework comment > * only evaluate _S0W if necessary > * drop static function with only one caller > > drivers/pci/pci-acpi.c | 17 ++++++++++++++++- > 1 file changed, 16 insertions(+), 1 deletion(-) > > diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c > index a42dbf448860..e535dab2c888 100644 > --- a/drivers/pci/pci-acpi.c > +++ b/drivers/pci/pci-acpi.c > @@ -977,6 +977,7 @@ bool acpi_pci_bridge_d3(struct pci_dev *dev) > const union acpi_object *obj; > struct acpi_device *adev; > struct pci_dev *rpdev; > + unsigned long long ret; > > if (acpi_pci_disabled || !dev->is_hotplug_bridge) > return false; > @@ -1003,7 +1004,21 @@ bool acpi_pci_bridge_d3(struct pci_dev *dev) > ACPI_TYPE_INTEGER, &obj) < 0) > return false; > > - return obj->integer.value == 1; > + if (!obj->integer.value) > + return false; > + > + /* > + * If 'HotPlugSupportInD3' is set, but wakeup is not actually supported, > + * the former cannot be trusted anyway, so validate it by verifying the > + * latter. > + */ > + if (!adev->wakeup.flags.valid) > + return false; > + > + if (ACPI_FAILURE(acpi_evaluate_integer(adev->handle, "_S0W", NULL, &ret))) > + return false; > + > + return ret >= ACPI_STATE_D3_HOT; > } > > int acpi_pci_set_power_state(struct pci_dev *dev, pci_power_t state) > -- > 2.34.1 >
