| 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: <74a63457-52fd-2f16-1b76-d3b3497b5a5d@redhat.com>
Date: Mon, 23 Jan 2023 14:55:36 +0100
From: Hans de Goede <hdegoede@...hat.com>
To: Mario Limonciello <mario.limonciello@....com>,
Shyam Sundar S K <Shyam-sundar.S-k@....com>
Cc: kai.heng.feng@...onical.com, xaver.hugl@...il.com,
Mark Gross <markgross@...nel.org>,
platform-driver-x86@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/2] platform/x86: amd: pmc: Disable IRQ1 wakeup for
RN/CZN
Hi,
On 1/20/23 20:15, Mario Limonciello wrote:
> By default when the system is configured for low power idle in the FADT
> the keyboard is set up as a wake source. This matches the behavior that
> Windows uses for Modern Standby as well.
>
> It has been reported that a variety of AMD based designs there are
> spurious wakeups are happening where two IRQ sources are active.
>
> For example:
> ```
> PM: Triggering wakeup from IRQ 9
> PM: Triggering wakeup from IRQ 1
> ```
>
> In these designs IRQ 9 is the ACPI SCI and IRQ 1 is the keyboard.
> One way to trigger this problem is to suspend the laptop and then unplug
> the AC adapter. The SOC will be in a hardware sleep state and plugging
> in the AC adapter returns control to the kernel's s2idle loop.
>
> Normally if just IRQ 9 was active the s2idle loop would advance any EC
> transactions and no other IRQ being active would cause the s2idle loop
> to put the SOC back into hardware sleep state.
>
> When this bug occurred IRQ 1 is also active even if no keyboard activity
> occurred. This causes the s2idle loop to break and the system to wake.
>
> This is a platform firmware bug triggering IRQ1 without keyboard activity.
> This occurs in Windows as well, but Windows will enter "SW DRIPS" and
> then with no activity enters back into "HW DRIPS" (hardware sleep state).
>
> This issue affects Renoir, Lucienne, Cezanne, and Barcelo platforms. It
> does not happen on newer systems such as Mendocino or Rembrandt.
>
> It's been fixed in newer platform firmware. To avoid triggering the bug
> on older systems check the SMU F/W version and adjust the policy at suspend
> time for s2idle wakeup from keyboard on these systems. A lot of thought
> and experimentation has been given around the timing of disabling IRQ1,
> and to make it work the "suspend" PM callback is restored.
>
> Reported-by: Kai-Heng Feng <kai.heng.feng@...onical.com>
> Reported-by: Xaver Hugl <xaver.hugl@...il.com>
> Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2115
> Link: https://gitlab.freedesktop.org/drm/amd/-/issues/1951
> Signed-off-by: Mario Limonciello <mario.limonciello@....com>
Thank you for your patch-series, I've applied the series to my
review-hans branch:
https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans
Note it will show up in my review-hans branch once I've pushed my
local branch there, which might take a while.
Once I've run some tests on this branch the patches there will be
added to the platform-drivers-x86/for-next branch and eventually
will be included in the pdx86 pull-request to Linus for the next
merge-window.
Regards,
Hans
> ---
> drivers/platform/x86/amd/pmc.c | 50 ++++++++++++++++++++++++++++++++++
> 1 file changed, 50 insertions(+)
>
> diff --git a/drivers/platform/x86/amd/pmc.c b/drivers/platform/x86/amd/pmc.c
> index 8d924986381be..be1b49824edbd 100644
> --- a/drivers/platform/x86/amd/pmc.c
> +++ b/drivers/platform/x86/amd/pmc.c
> @@ -22,6 +22,7 @@
> #include <linux/pci.h>
> #include <linux/platform_device.h>
> #include <linux/rtc.h>
> +#include <linux/serio.h>
> #include <linux/suspend.h>
> #include <linux/seq_file.h>
> #include <linux/uaccess.h>
> @@ -653,6 +654,33 @@ static int amd_pmc_get_os_hint(struct amd_pmc_dev *dev)
> return -EINVAL;
> }
>
> +static int amd_pmc_czn_wa_irq1(struct amd_pmc_dev *pdev)
> +{
> + struct device *d;
> + int rc;
> +
> + if (!pdev->major) {
> + rc = amd_pmc_get_smu_version(pdev);
> + if (rc)
> + return rc;
> + }
> +
> + if (pdev->major > 64 || (pdev->major == 64 && pdev->minor > 65))
> + return 0;
> +
> + d = bus_find_device_by_name(&serio_bus, NULL, "serio0");
> + if (!d)
> + return 0;
> + if (device_may_wakeup(d)) {
> + dev_info_once(d, "Disabling IRQ1 wakeup source to avoid platform firmware bug\n");
> + disable_irq_wake(1);
> + device_set_wakeup_enable(d, false);
> + }
> + put_device(d);
> +
> + return 0;
> +}
> +
> static int amd_pmc_verify_czn_rtc(struct amd_pmc_dev *pdev, u32 *arg)
> {
> struct rtc_device *rtc_device;
> @@ -782,6 +810,25 @@ static struct acpi_s2idle_dev_ops amd_pmc_s2idle_dev_ops = {
> .check = amd_pmc_s2idle_check,
> .restore = amd_pmc_s2idle_restore,
> };
> +
> +static int __maybe_unused amd_pmc_suspend_handler(struct device *dev)
> +{
> + struct amd_pmc_dev *pdev = dev_get_drvdata(dev);
> +
> + if (pdev->cpu_id == AMD_CPU_ID_CZN) {
> + int rc = amd_pmc_czn_wa_irq1(pdev);
> +
> + if (rc) {
> + dev_err(pdev->dev, "failed to adjust keyboard wakeup: %d\n", rc);
> + return rc;
> + }
> + }
> +
> + return 0;
> +}
> +
> +static SIMPLE_DEV_PM_OPS(amd_pmc_pm, amd_pmc_suspend_handler, NULL);
> +
> #endif
>
> static const struct pci_device_id pmc_pci_ids[] = {
> @@ -980,6 +1027,9 @@ static struct platform_driver amd_pmc_driver = {
> .name = "amd_pmc",
> .acpi_match_table = amd_pmc_acpi_ids,
> .dev_groups = pmc_groups,
> +#ifdef CONFIG_SUSPEND
> + .pm = &amd_pmc_pm,
> +#endif
> },
> .probe = amd_pmc_probe,
> .remove = amd_pmc_remove,
Powered by blists - more mailing lists