| 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: <d4522480-8095-3162-2842-25d5dc0ccfff@gmail.com>
Date: Tue, 23 May 2023 17:20:57 -0400
From: Adam <adam.niederer@...il.com>
To: Mario Limonciello <mario.limonciello@....com>, rafael@...nel.org
Cc: gch981213@...il.com, linux-acpi@...r.kernel.org,
linux-kernel@...r.kernel.org, regressions@...mhuis.info,
ofenfisch@...glemail.com, wse@...edocomputers.com,
adrian@...und.io, jirislaby@...nel.org,
Renjith.Pananchikkal@....com, anson.tsao@....com,
Richard.Gong@....com, evilsnoo@...ton.me, ruinairas1992@...il.com
Subject: Re: [PATCH v2] ACPI: resource: Remove "Zen" specific match and quirks
Hey, I have tested 6.4-rc3 and this breaks the integrated trackpad on my
MAINGEAR Vector Pro 2 15" 6900H model (MG-VCP2-15A3070T), but the
keyboard still works (the trackpad worked and keyboard did not before I
submitted my ACPI override). I'll send over an acpidump directly as well.
On 5/18/23 14:39, Mario Limonciello wrote:
> commit 9946e39fe8d0 ("ACPI: resource: skip IRQ override on
> AMD Zen platforms") attempted to overhaul the override logic so it
> didn't apply on X86 AMD Zen systems. This was intentional so that
> systems would prefer DSDT values instead of default MADT value for
> IRQ 1 on Ryzen 6000 systems which use ActiveLow for IRQ1.
>
> This turned out to be a bad assumption because several vendors seem
> to add Interrupt Source Override but don't fix the DSDT. A pile of
> quirks was collecting that proved this wasn't sustaintable.
>
> Adjust the logic so that only IRQ1 is overridden in Ryzen 6000 case.
>
> This effectively reverts the following commits:
> commit 17bb7046e7ce ("ACPI: resource: Do IRQ override on all TongFang
> GMxRGxx")
> commit f3cb9b740869 ("ACPI: resource: do IRQ override on Lenovo 14ALC7")
> commit bfcdf58380b1 ("ACPI: resource: do IRQ override on LENOVO IdeaPad")
> commit 7592b79ba4a9 ("ACPI: resource: do IRQ override on XMG Core 15")
>
> Cc: ofenfisch@...glemail.com
> Cc: wse@...edocomputers.com
> Cc: adam.niederer@...il.com
> Cc: adrian@...und.io
> Cc: jirislaby@...nel.org
> Tested-by: Renjith.Pananchikkal@....com
> Tested-by: anson.tsao@....com
> Tested-by: Richard.Gong@....com
> Tested-by: Chuanhong Guo <gch981213@...il.com>
> Reported-by: evilsnoo@...ton.me
> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217394
> Reported-by: ruinairas1992@...il.com
> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217406
> Fixes: 9946e39fe8d0 ("ACPI: resource: skip IRQ override on AMD Zen platforms")
> Signed-off-by: Mario Limonciello <mario.limonciello@....com>
> ---
> v1->v2:
> * Rebase on 71a485624c4c ("ACPI: resource: Add IRQ override quirk for LG UltraPC 17U70P")
> * Pick up tag
> ---
> drivers/acpi/resource.c | 154 +++++++++++++++++-----------------------
> 1 file changed, 65 insertions(+), 89 deletions(-)
>
> diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c
> index 0800a9d77558..c6ac87e01e1c 100644
> --- a/drivers/acpi/resource.c
> +++ b/drivers/acpi/resource.c
> @@ -470,52 +470,6 @@ static const struct dmi_system_id asus_laptop[] = {
> { }
> };
>
> -static const struct dmi_system_id lenovo_laptop[] = {
> - {
> - .ident = "LENOVO IdeaPad Flex 5 14ALC7",
> - .matches = {
> - DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
> - DMI_MATCH(DMI_PRODUCT_NAME, "82R9"),
> - },
> - },
> - {
> - .ident = "LENOVO IdeaPad Flex 5 16ALC7",
> - .matches = {
> - DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
> - DMI_MATCH(DMI_PRODUCT_NAME, "82RA"),
> - },
> - },
> - { }
> -};
> -
> -static const struct dmi_system_id tongfang_gm_rg[] = {
> - {
> - .ident = "TongFang GMxRGxx/XMG CORE 15 (M22)/TUXEDO Stellaris 15 Gen4 AMD",
> - .matches = {
> - DMI_MATCH(DMI_BOARD_NAME, "GMxRGxx"),
> - },
> - },
> - { }
> -};
> -
> -static const struct dmi_system_id maingear_laptop[] = {
> - {
> - .ident = "MAINGEAR Vector Pro 2 15",
> - .matches = {
> - DMI_MATCH(DMI_SYS_VENDOR, "Micro Electronics Inc"),
> - DMI_MATCH(DMI_PRODUCT_NAME, "MG-VCP2-15A3070T"),
> - }
> - },
> - {
> - .ident = "MAINGEAR Vector Pro 2 17",
> - .matches = {
> - DMI_MATCH(DMI_SYS_VENDOR, "Micro Electronics Inc"),
> - DMI_MATCH(DMI_PRODUCT_NAME, "MG-VCP2-17A3070T"),
> - },
> - },
> - { }
> -};
> -
> static const struct dmi_system_id lg_laptop[] = {
> {
> .ident = "LG Electronics 17U70P",
> @@ -527,7 +481,7 @@ static const struct dmi_system_id lg_laptop[] = {
> { }
> };
>
> -struct irq_override_cmp {
> +struct irq_override_dmi_cmp {
> const struct dmi_system_id *system;
> unsigned char irq;
> unsigned char triggering;
> @@ -536,50 +490,86 @@ struct irq_override_cmp {
> bool override;
> };
>
> -static const struct irq_override_cmp override_table[] = {
> +struct irq_override_acpi_cmp {
> + const char *id;
> + unsigned char irq;
> + unsigned char triggering;
> + unsigned char polarity;
> +};
> +
> +static const struct irq_override_dmi_cmp dmi_override_table[] = {
> { medion_laptop, 1, ACPI_LEVEL_SENSITIVE, ACPI_ACTIVE_LOW, 0, false },
> { asus_laptop, 1, ACPI_LEVEL_SENSITIVE, ACPI_ACTIVE_LOW, 0, false },
> - { lenovo_laptop, 6, ACPI_LEVEL_SENSITIVE, ACPI_ACTIVE_LOW, 0, true },
> - { lenovo_laptop, 10, ACPI_LEVEL_SENSITIVE, ACPI_ACTIVE_LOW, 0, true },
> - { tongfang_gm_rg, 1, ACPI_EDGE_SENSITIVE, ACPI_ACTIVE_LOW, 1, true },
> - { maingear_laptop, 1, ACPI_EDGE_SENSITIVE, ACPI_ACTIVE_LOW, 1, true },
> { lg_laptop, 1, ACPI_LEVEL_SENSITIVE, ACPI_ACTIVE_LOW, 0, false },
> };
>
> -static bool acpi_dev_irq_override(u32 gsi, u8 triggering, u8 polarity,
> - u8 shareable)
> +/*
> + * Ryzen 6000 requires ActiveLow for keyboard, but a number of machines
> + * seem to get it wrong in DSDT or don't have an Interrupt Source
> + * Override.
> + */
> +static const struct irq_override_acpi_cmp acpi_override_table[] = {
> + { "AMDI0007", 1, ACPI_EDGE_SENSITIVE, ACPI_ACTIVE_LOW },
> +};
> +
> +static void acpi_dev_irq_override(u32 gsi, u8 *triggering, u8 *polarity,
> + u8 *shareable)
> {
> - int i;
> + int i, p, t;
> + int check_override = true;
>
> - for (i = 0; i < ARRAY_SIZE(override_table); i++) {
> - const struct irq_override_cmp *entry = &override_table[i];
> + for (i = 0; i < ARRAY_SIZE(dmi_override_table); i++) {
> + const struct irq_override_dmi_cmp *entry = &dmi_override_table[i];
>
> if (dmi_check_system(entry->system) &&
> entry->irq == gsi &&
> - entry->triggering == triggering &&
> - entry->polarity == polarity &&
> - entry->shareable == shareable)
> - return entry->override;
> + entry->triggering == *triggering &&
> + entry->polarity == *polarity &&
> + entry->shareable == *shareable)
> + check_override = entry->override;
> }
>
> -#ifdef CONFIG_X86
> - /*
> - * IRQ override isn't needed on modern AMD Zen systems and
> - * this override breaks active low IRQs on AMD Ryzen 6000 and
> - * newer systems. Skip it.
> - */
> - if (boot_cpu_has(X86_FEATURE_ZEN))
> - return false;
> -#endif
> + if (!check_override)
> + return;
>
> - return true;
> + if (!acpi_get_override_irq(gsi, &t, &p)) {
> + u8 trig = t ? ACPI_LEVEL_SENSITIVE : ACPI_EDGE_SENSITIVE;
> + u8 pol = p ? ACPI_ACTIVE_LOW : ACPI_ACTIVE_HIGH;
> +
> + if (*triggering != trig || *polarity != pol) {
> + pr_warn("ACPI: IRQ %d override to %s%s, %s%s\n", gsi,
> + t ? "level" : "edge",
> + trig == *triggering ? "" : "(!)",
> + p ? "low" : "high",
> + pol == *polarity ? "" : "(!)");
> + *triggering = trig;
> + *polarity = pol;
> + }
> + }
> +
> + for (i = 0; i < ARRAY_SIZE(acpi_override_table); i++) {
> + const struct irq_override_acpi_cmp *entry = &acpi_override_table[i];
> +
> + if (acpi_dev_found(entry->id) && gsi == entry->irq &&
> + (*polarity != entry->polarity || *triggering != entry->triggering)) {
> + pr_warn("ACPI: IRQ %d override to %s%s, %s%s due to %s\n",
> + gsi,
> + entry->triggering ? "level" : "edge",
> + entry->triggering == *triggering ? "" : "(!)",
> + entry->polarity ? "low" : "high",
> + entry->polarity == *polarity ? "" : "(!)",
> + entry->id);
> + *polarity = entry->polarity;
> + *triggering = entry->triggering;
> + }
> + }
> }
>
> static void acpi_dev_get_irqresource(struct resource *res, u32 gsi,
> u8 triggering, u8 polarity, u8 shareable,
> u8 wake_capable, bool check_override)
> {
> - int irq, p, t;
> + int irq;
>
> if (!valid_IRQ(gsi)) {
> irqresource_disabled(res, gsi);
> @@ -592,26 +582,12 @@ static void acpi_dev_get_irqresource(struct resource *res, u32 gsi,
> * 2. BIOS uses IO-APIC mode Interrupt Source Override
> *
> * We do this only if we are dealing with IRQ() or IRQNoFlags()
> - * resource (the legacy ISA resources). With modern ACPI 5 devices
> + * resource (the legacy ISA resources). With ACPI devices
> * using extended IRQ descriptors we take the IRQ configuration
> * from _CRS directly.
> */
> - if (check_override &&
> - acpi_dev_irq_override(gsi, triggering, polarity, shareable) &&
> - !acpi_get_override_irq(gsi, &t, &p)) {
> - u8 trig = t ? ACPI_LEVEL_SENSITIVE : ACPI_EDGE_SENSITIVE;
> - u8 pol = p ? ACPI_ACTIVE_LOW : ACPI_ACTIVE_HIGH;
> -
> - if (triggering != trig || polarity != pol) {
> - pr_warn("ACPI: IRQ %d override to %s%s, %s%s\n", gsi,
> - t ? "level" : "edge",
> - trig == triggering ? "" : "(!)",
> - p ? "low" : "high",
> - pol == polarity ? "" : "(!)");
> - triggering = trig;
> - polarity = pol;
> - }
> - }
> + if (check_override)
> + acpi_dev_irq_override(gsi, &triggering, &polarity, &shareable);
>
> res->flags = acpi_dev_irq_flags(triggering, polarity, shareable, wake_capable);
> irq = acpi_register_gsi(NULL, gsi, triggering, polarity);
>
> base-commit: c554eee18c9a440bd2dd5a363b0f79325717f0bf
Powered by blists - more mailing lists