| 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
| ||
|
Message-ID: <CAJZ5v0iQsYzudLGbbyhsBbBUwv9c5uQZpUCBCOcOTG=-vPiVgw@mail.gmail.com>
Date: Thu, 11 Sep 2025 15:28:11 +0200
From: "Rafael J. Wysocki" <rafael@...nel.org>
To: Lukas Wunner <lukas@...ner.de>
Cc: Bjorn Helgaas <helgaas@...nel.org>, "Rafael J. Wysocki" <rafael@...nel.org>,
Alan Stern <stern@...land.harvard.edu>, linux-pci@...r.kernel.org,
linux-pm@...r.kernel.org, Thomas Gleixner <tglx@...utronix.de>,
Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
Dave Hansen <dave.hansen@...ux.intel.com>, x86@...nel.org,
"H. Peter Anvin" <hpa@...or.com>, linux-kernel@...r.kernel.org,
Mario Limonciello <superm1@...nel.org>, Oleksij Rempel <o.rempel@...gutronix.de>,
Timo Jyrinki <timo.jyrinki@....fi>, Ernst Persson <ernstp@...il.com>, Steven Harms <sjharms@...il.com>,
James Ettle <james@...le.org.uk>, Nick Coghlan <ncoghlan@...il.com>, Weng Xuetian <wengxt@...il.com>,
Andrey Rahmatullin <wrar@...r.name>, Boris Barbour <boris.barbour@....fr>,
Vlastimil Zima <vlastimil.zima@...il.com>, David Banks <amoebae@...il.com>,
Michal Jaegermann <michal@...ddata.com>, Chris Moeller <kode54@...il.com>, Daniel Fraga <fragabr@...il.com>,
Javier Marcet <jmarcet@...il.com>, Pavel Pisa <pisa@....felk.cvut.cz>
Subject: Re: [PATCH] PCI/PM: Move ASUS EHCI workaround out of generic code
On Thu, Sep 11, 2025 at 3:12 PM Lukas Wunner <lukas@...ner.de> wrote:
>
> In 2012, commit dbf0e4c7257f ("PCI: EHCI: fix crash during suspend on ASUS
> computers") amended pci_pm_suspend_noirq() to work around a BIOS issue by
> clearing the Command register if the suspended device is a USB EHCI host
> controller.
>
> Commit 0b68c8e2c3af ("PCI: EHCI: Fix crash during hibernation on ASUS
> computers") subsequently amended pci_pm_poweroff_noirq() to do the same.
>
> Two years later, commit 7d2a01b87f16 ("PCI: Add pci_fixup_suspend_late
> quirk pass") introduced the ability to execute arbitrary quirks
> specifically in pci_pm_suspend_noirq() and pci_pm_poweroff_noirq().
>
> This allows moving the ASUS workaround out of generic code and into a
> proper quirk to improve maintainability and readability. Constrain to x86
> since the ASUS BIOS doesn't seem to have been used on other arches.
>
> lspci output of affected EHCI host controllers reveals that the only bits
> set in the Command register are Memory Space Enable and Bus Master Enable:
> https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=658778
>
> The latter is cleared by:
> hcd_pci_suspend()
> suspend_common()
> pci_disable_device()
>
> pci_disable_device() does not clear I/O and Memory Space Enable, although
> its name suggests otherwise. The kernel has never disabled these bits
> once they're enabled. Doing so would avoid the need for the quirk, but it
> is unclear what will break if this fundamental behavior is changed.
>
> Signed-off-by: Lukas Wunner <lukas@...ner.de>
Acked-by: Rafael J. Wysocki (Intel) <rafael@...nel.org>
> ---
> arch/x86/pci/fixup.c | 19 +++++++++++++++++++
> drivers/pci/pci-driver.c | 19 -------------------
> 2 files changed, 19 insertions(+), 19 deletions(-)
>
> diff --git a/arch/x86/pci/fixup.c b/arch/x86/pci/fixup.c
> index e7e71490bd25..c34ff72434f2 100644
> --- a/arch/x86/pci/fixup.c
> +++ b/arch/x86/pci/fixup.c
> @@ -1041,3 +1041,22 @@ static void quirk_tuxeo_rp_d3(struct pci_dev *pdev)
> }
> DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_AMD, 0x1502, quirk_tuxeo_rp_d3);
> #endif /* CONFIG_SUSPEND */
> +
> +#ifdef CONFIG_PM_SLEEP
> +/*
> + * Some BIOSes from ASUS have a bug: If a USB EHCI host controller's Command
> + * register is not 0 on suspend, the BIOS assumes that the controller has not
> + * been quiesced and tries to turn it off. If the controller is already in D3,
> + * this can hang or cause memory corruption.
> + *
> + * Since the value of the Command register does not matter once the device has
> + * been suspended, it can safely be set to 0.
> + */
> +static void quirk_clear_command_reg(struct pci_dev *pdev)
> +{
> + pci_write_config_word(pdev, PCI_COMMAND, 0);
> +}
> +DECLARE_PCI_FIXUP_CLASS_SUSPEND_LATE(PCI_ANY_ID, PCI_ANY_ID,
> + PCI_CLASS_SERIAL_USB_EHCI, 0,
> + quirk_clear_command_reg);
> +#endif /* CONFIG_PM_SLEEP */
> diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
> index 63665240ae87..e1089dfeb419 100644
> --- a/drivers/pci/pci-driver.c
> +++ b/drivers/pci/pci-driver.c
> @@ -914,18 +914,6 @@ static int pci_pm_suspend_noirq(struct device *dev)
>
> pci_pm_set_unknown_state(pci_dev);
>
> - /*
> - * Some BIOSes from ASUS have a bug: If a USB EHCI host controller's
> - * PCI COMMAND register isn't 0, the BIOS assumes that the controller
> - * hasn't been quiesced and tries to turn it off. If the controller
> - * is already in D3, this can hang or cause memory corruption.
> - *
> - * Since the value of the COMMAND register doesn't matter once the
> - * device has been suspended, we can safely set it to 0 here.
> - */
> - if (pci_dev->class == PCI_CLASS_SERIAL_USB_EHCI)
> - pci_write_config_word(pci_dev, PCI_COMMAND, 0);
> -
> Fixup:
> pci_fixup_device(pci_fixup_suspend_late, pci_dev);
>
> @@ -1205,13 +1193,6 @@ static int pci_pm_poweroff_noirq(struct device *dev)
> if (!pci_dev->state_saved && !pci_has_subordinate(pci_dev))
> pci_prepare_to_sleep(pci_dev);
>
> - /*
> - * The reason for doing this here is the same as for the analogous code
> - * in pci_pm_suspend_noirq().
> - */
> - if (pci_dev->class == PCI_CLASS_SERIAL_USB_EHCI)
> - pci_write_config_word(pci_dev, PCI_COMMAND, 0);
> -
> pci_fixup_device(pci_fixup_suspend_late, pci_dev);
>
> return 0;
> --
> 2.51.0
>
Powered by blists - more mailing lists