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

["Rafael J. Wysocki" <rafael.j.wysocki@intel.com>]

On 3/10/2022 10:56 AM, Mika Westerberg wrote:
> +Rafael

Thanks!

+linux-pm (all PM-related stuff should go there)


>
> 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.

It is "wake up the device from D3".  The system is already in S0.


> 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?
>
>> To fix these situations explicitly check that the ACPI device claims the
>> system can be awoken in `acpi_pci_bridge_d3`.

This is not too precise.

wakeup.flags.valid is 1 if there is a GPE allowing the device to generate wakeup signals that will be handled by the platform.

Again, this is not only about waking up the system from sleep.

>> 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@amd.com>
> Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
>
>> ---
>>   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