linux-kernel - Re: [PATCH v2] PCI: Don't assume root ports from

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <da80fb19-0b98-bf19-eea3-bc2cca2024f5@amd.com>
Date:   Mon, 22 May 2023 06:28:25 -0500
From:   Mario Limonciello <mario.limonciello@....com>
To:     Mika Westerberg <mika.westerberg@...ux.intel.com>
Cc:     Bjorn Helgaas <bhelgaas@...gle.com>, linux-pci@...r.kernel.org,
        linux-kernel@...r.kernel.org,
        S-k Shyam-sundar <Shyam-sundar.S-k@....com>,
        Natikar Basavaraj <Basavaraj.Natikar@....com>,
        Deucher Alexander <Alexander.Deucher@....com>,
        Iain Lane <iain@...ngesquash.org.uk>,
        "Rafael J. Wysocki" <rafael.j.wysocki@...el.com>
Subject: Re: [PATCH v2] PCI: Don't assume root ports from > 2015 are power
 manageable

On 5/22/23 00:55, Mika Westerberg wrote:
> Hi Mario,
> 
> On Wed, May 17, 2023 at 10:08:27AM -0500, Mario Limonciello wrote:
>> Using an XHCI device to wakeup the system from s2idle fails when
>> that XHCI device is connected to a USB-C port for an AMD USB4
>> router.
>>
>> Due to commit 9d26d3a8f1b0 ("PCI: Put PCIe ports into D3 during
>> suspend") all root port go into D3 during s2idle.
>> When the root ports are in D3 over s2idle it's not possible for the
>> platform firmware to properly identify the wakeup source.
>>
>> Comparing registers between Linux and Windows 11 this behavior to put root
>> ports into D3 at suspend is unique to Linux.  On an affected system
>> Windows does not put the root ports into D3 over Modern Standby.
>>
>> Windows doesn't put the root ports into D3 because root ports are not
>> power manageable; they're missing _PRW and _S0W.
>>
>> Linux shouldn't be assuming they support D3 just because they're newer
>> than 2015, the ports should also be deemed power manageable.
>> Add an extra check for this to ensure D3 isn't selected for such machines.
>>
>> Fixes: 9d26d3a8f1b0 ("PCI: Put PCIe ports into D3 during suspend")
>> Reported-by: Iain Lane <iain@...ngesquash.org.uk>
>> Closes: https://forums.lenovo.com/t5/Ubuntu/Z13-can-t-resume-from-suspend-with-external-USB-keyboard/m-p/5217121
>> Signed-off-by: Mario Limonciello <mario.limonciello@....com>
>> ---
>>   drivers/pci/pci.c | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
>> index 5ede93222bc1..3fe27aef09e6 100644
>> --- a/drivers/pci/pci.c
>> +++ b/drivers/pci/pci.c
>> @@ -3010,6 +3010,9 @@ bool pci_bridge_d3_possible(struct pci_dev *bridge)
>>   		if (dmi_check_system(bridge_d3_blacklist))
>>   			return false;
>>   
>> +		if (!platform_pci_power_manageable(bridge))
>> +			return false;
>> +
> 
> We already call platform_pci_bridge_d3() few lines up. That function
> should know whether "platform" supports D3 for the bridges, and I think
> it actually calls acpi_device_power_manageable() that platform_pci_power_manageable()
> ends up checking too.

It does, but it doesn't end up returning false if it doesn't support it. 
  It only returns true if it does.

I've tested this patch on two different failing platforms and it works 
now on both.