linux-kernel - Re: [PATCH] PCI/ACPI: Don't assume D3 support if a device is power manageable

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] [day] [month] [year] [list]

Message-ID: <CAJZ5v0hG0YtQ9msqAQ4y1GhEyWjVQmRYxSM6zUFit6tjx720xQ@mail.gmail.com>
Date:   Wed, 19 Oct 2022 20:13:09 +0200
From:   "Rafael J. Wysocki" <rafael@...nel.org>
To:     Mario Limonciello <mario.limonciello@....com>
Cc:     Mika Westerberg <mika.westerberg@...ux.intel.com>,
        "Rafael J. Wysocki" <rafael@...nel.org>,
        Len Brown <lenb@...nel.org>,
        Bjorn Helgaas <bhelgaas@...gle.com>,
        Mehta Sanju <Sanju.Mehta@....com>, linux-acpi@...r.kernel.org,
        linux-pci@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] PCI/ACPI: Don't assume D3 support if a device is power manageable

On Wed, Oct 19, 2022 at 7:59 PM Mario Limonciello
<mario.limonciello@....com> wrote:
>
> On some firmware configurations where D3 is not supported for
> "AMD Pink Sardine" the PCIe root port for tunneling will still be
> opted into runtime PM since `acpi_pci_bridge_d3` returns true.
>
> This later causes the device link between the USB4 router and the
> PCIe root port for tunneling to misbehave.  The USB4 router may
> enters D3 via runtime PM, but the PCIe root port for tunneling
> remains in D0.  The expectation is the USB4 router should also
> remain in D0 since the PCIe root port for tunneling remained in D0.
>
> `acpi_pci_bridge_d3` has a number of checks, but starts out with an
> assumption that if a device is power manageable introduced from
> commit c6e331312ebf ("PCI/ACPI: Whitelist hotplug ports for D3 if power
> managed by ACPI") that it will support D3.  This is not a valid
> assumption, as the PCIe root port for tunneling has power resources
> but does not support D3hot or D3cold.
>
> Instead of making this assertion from the power resources check
> immediately, move the check to later on, which will have validated
> that D3hot or D3cold can actually be reached.
>
> This fixes the USB4 router going into D3 when the firmware says that
> the PCIe root port for tunneling can't handle it.
>
> Fixes: dff6139015dc6 ("PCI/ACPI: Allow D3 only if Root Port can signal and wake from D3")
> Signed-off-by: Mario Limonciello <mario.limonciello@....com>
> ---
>  drivers/pci/pci-acpi.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c
> index a46fec776ad77..1e774a4645663 100644
> --- a/drivers/pci/pci-acpi.c
> +++ b/drivers/pci/pci-acpi.c
> @@ -984,10 +984,6 @@ bool acpi_pci_bridge_d3(struct pci_dev *dev)
>         if (acpi_pci_disabled || !dev->is_hotplug_bridge)
>                 return false;
>
> -       /* Assume D3 support if the bridge is power-manageable by ACPI. */
> -       if (acpi_pci_power_manageable(dev))
> -               return true;
> -
>         rpdev = pcie_find_root_port(dev);
>         if (!rpdev)
>                 return false;
> @@ -1023,6 +1019,10 @@ bool acpi_pci_bridge_d3(struct pci_dev *dev)
>             obj->integer.value == 1)
>                 return true;
>
> +       /* Assume D3 support if the bridge is power-manageable by ACPI. */
> +       if (acpi_pci_power_manageable(dev))
> +               return true;
> +
>         return false;

return acpi_pci_power_manageable(dev);

would be simpler.

Otherwise it looks OK to me.

>  }
>
> --
> 2.34.1
>