On Tue, Mar 15, 2022 at 5:35 PM Rafael J. Wysocki <rafael@xxxxxxxxxx> wrote: > > 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> Or please let me know if I should pick it up. > > --- > > 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 > >
