On 3/15/22 11:36, Rafael J. Wysocki wrote:
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://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&data=04%7C01%7Cmario.limonciello%40amd.com%7Cae5d5d8f4ac1452dac0c08da06a200b2%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637829590116901616%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=3cTSyBgSEOBo5mPeUcNDrc9qhdMJeZ4cVtLqroDRZqY%3D&reserved=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&data=04%7C01%7Cmario.limonciello%40amd.com%7Cae5d5d8f4ac1452dac0c08da06a200b2%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637829590116901616%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=rZHk%2FgII0qOvr9cmmB8N1auNc1nLSFViVTG9f%2FwEREY%3D&reserved=0
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.
Bjorn,
Friendly reminder on this one.
---
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
