| Message ID | 20230809085526.84913-4-hdegoede@redhat.com |
|---|---|
| State | Accepted |
| Commit | c6a1fd910d1bf8a0e3db7aebb229e3c81bc305c4 |
| Headers | show |
| Series | ACPI: resource: Fix regressions from "Remove "Zen" specific match and quirks" | expand |
On 09. 08. 23, 10:55, Hans de Goede wrote: > On AMD Zen acpi_dev_irq_override() by default prefers the DSDT IRQ 1 > settings over the MADT settings. > > This causes the keyboard to malfunction on some laptop models > (see Links), all models from the Links have an INT_SRC_OVR MADT entry > for IRQ 1. ... > Signed-off-by: Hans de Goede <hdegoede@redhat.com> ... > diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c > index 21b542a6866c..b88e5e0135ab 100644 > --- a/arch/x86/kernel/acpi/boot.c > +++ b/arch/x86/kernel/acpi/boot.c > @@ -52,6 +52,7 @@ int acpi_lapic; > int acpi_ioapic; > int acpi_strict; > int acpi_disable_cmcff; > +int acpi_int_src_ovr[NR_IRQS_LEGACY]; So why not to use bool to make it clear this is not an irq number, but a state? > > /* ACPI SCI override configuration */ > u8 acpi_sci_flags __initdata; > @@ -588,6 +589,9 @@ acpi_parse_int_src_ovr(union acpi_subtable_headers * header, > > acpi_table_print_madt_entry(&header->common); > > + if (intsrc->source_irq < NR_IRQS_LEGACY) > + acpi_int_src_ovr[intsrc->source_irq] = 1; And "true" here. thanks,
Hi, On 8/9/23 16:57, Rafael J. Wysocki wrote: > On Wed, Aug 9, 2023 at 4:40 PM Hans de Goede <hdegoede@redhat.com> wrote: >> >> Hi, >> >> On 8/9/23 11:20, Jiri Slaby wrote: >>> On 09. 08. 23, 10:55, Hans de Goede wrote: >>>> On AMD Zen acpi_dev_irq_override() by default prefers the DSDT IRQ 1 >>>> settings over the MADT settings. >>>> >>>> This causes the keyboard to malfunction on some laptop models >>>> (see Links), all models from the Links have an INT_SRC_OVR MADT entry >>>> for IRQ 1. >>> ... >>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com> >>> ... >>>> diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c >>>> index 21b542a6866c..b88e5e0135ab 100644 >>>> --- a/arch/x86/kernel/acpi/boot.c >>>> +++ b/arch/x86/kernel/acpi/boot.c >>>> @@ -52,6 +52,7 @@ int acpi_lapic; >>>> int acpi_ioapic; >>>> int acpi_strict; >>>> int acpi_disable_cmcff; >>>> +int acpi_int_src_ovr[NR_IRQS_LEGACY]; >>> >>> So why not to use bool to make it clear this is not an irq number, but a state? >>> >>>> /* ACPI SCI override configuration */ >>>> u8 acpi_sci_flags __initdata; >>>> @@ -588,6 +589,9 @@ acpi_parse_int_src_ovr(union acpi_subtable_headers * header, >>>> acpi_table_print_madt_entry(&header->common); >>>> + if (intsrc->source_irq < NR_IRQS_LEGACY) >>>> + acpi_int_src_ovr[intsrc->source_irq] = 1; >>> >>> And "true" here. >> >> Ack that would indeed be better. >> >> Rafael, can you fix this up while merging or do you want a v4 series ? > > I think I can do that. Great, thank you. Do you have any comments on this series, or is this ready for merging now? Regards, Hans
Hi, On 8/9/23 17:58, August Wikerfors wrote: > On 2023-08-09 10:55, Hans de Goede wrote: >> On AMD Zen acpi_dev_irq_override() by default prefers the DSDT IRQ 1 >> settings over the MADT settings. >> >> This causes the keyboard to malfunction on some laptop models >> (see Links), all models from the Links have an INT_SRC_OVR MADT entry >> for IRQ 1. >> >> Fixes: a9c4a912b7dc ("ACPI: resource: Remove "Zen" specific match and quirks") >> Link: https://bugzilla.kernel.org/show_bug.cgi?id=217336 >> Link: https://bugzilla.kernel.org/show_bug.cgi?id=217394 >> Link: https://bugzilla.kernel.org/show_bug.cgi?id=217406 >> Cc: Mario Limonciello <mario.limonciello@amd.com> >> Cc: Linux regressions mailing list <regressions@lists.linux.dev> >> Cc: stable@vger.kernel.org >> Signed-off-by: Hans de Goede <hdegoede@redhat.com> > > One of the laptops fixed by a9c4a912b7dc, PCSpecialist Elimina Pro 16 M [1], seems to have no INT_SRC_OVR entry for IRQ 1 [2]: > >> [ 0.084265] ACPI: INT_SRC_OVR (bus 0 bus_irq 0 global_irq 2 dfl dfl) >> [ 0.084266] ACPI: INT_SRC_OVR (bus 0 bus_irq 9 global_irq 9 low level) > > I'm not sure if it was IRQ 1 that needed to be overridden for that model though, so it may work anyway with patch 2 of this series. > > [1] https://bugzilla.kernel.org/show_bug.cgi?id=217394#c18 > [2] https://bugzilla.kernel.org/attachment.cgi?id=304338 Good catch, thanks. So it looks like this one needs a DMI quirk (until we have a better generic solution. I'll reach out to the reporter and ask for dmidecode output and prepare a follow-up patch. Still I think that we should move forward with this series to fix the 6 bugs which are linked to from PAtch 1's commitmsg and those are likely just the top of the iceberg. Regards, Hans
On Wed, Aug 9, 2023 at 9:20 PM Hans de Goede <hdegoede@redhat.com> wrote: > > Hi, > > On 8/9/23 17:58, August Wikerfors wrote: > > On 2023-08-09 10:55, Hans de Goede wrote: > >> On AMD Zen acpi_dev_irq_override() by default prefers the DSDT IRQ 1 > >> settings over the MADT settings. > >> > >> This causes the keyboard to malfunction on some laptop models > >> (see Links), all models from the Links have an INT_SRC_OVR MADT entry > >> for IRQ 1. > >> > >> Fixes: a9c4a912b7dc ("ACPI: resource: Remove "Zen" specific match and quirks") > >> Link: https://bugzilla.kernel.org/show_bug.cgi?id=217336 > >> Link: https://bugzilla.kernel.org/show_bug.cgi?id=217394 > >> Link: https://bugzilla.kernel.org/show_bug.cgi?id=217406 > >> Cc: Mario Limonciello <mario.limonciello@amd.com> > >> Cc: Linux regressions mailing list <regressions@lists.linux.dev> > >> Cc: stable@vger.kernel.org > >> Signed-off-by: Hans de Goede <hdegoede@redhat.com> > > > > One of the laptops fixed by a9c4a912b7dc, PCSpecialist Elimina Pro 16 M [1], seems to have no INT_SRC_OVR entry for IRQ 1 [2]: > > > >> [ 0.084265] ACPI: INT_SRC_OVR (bus 0 bus_irq 0 global_irq 2 dfl dfl) > >> [ 0.084266] ACPI: INT_SRC_OVR (bus 0 bus_irq 9 global_irq 9 low level) > > > > I'm not sure if it was IRQ 1 that needed to be overridden for that model though, so it may work anyway with patch 2 of this series. > > > > [1] https://bugzilla.kernel.org/show_bug.cgi?id=217394#c18 > > [2] https://bugzilla.kernel.org/attachment.cgi?id=304338 > > Good catch, thanks. So it looks like this one needs a DMI quirk (until we have a better generic solution. > > I'll reach out to the reporter and ask for dmidecode output and prepare a follow-up patch. Still I think > that we should move forward with this series to fix the 6 bugs which are linked to from PAtch 1's > commitmsg and those are likely just the top of the iceberg. You are probably right, but it would be good to get a fix for this ASAP, as I would prefer it to go in along with the series, if possible.
Hi, On 8/9/23 21:41, Hans de Goede wrote: > Hi, > > On 8/9/23 21:25, Rafael J. Wysocki wrote: >> On Wed, Aug 9, 2023 at 9:20 PM Hans de Goede <hdegoede@redhat.com> wrote: >>> >>> Hi, >>> >>> On 8/9/23 17:58, August Wikerfors wrote: >>>> On 2023-08-09 10:55, Hans de Goede wrote: >>>>> On AMD Zen acpi_dev_irq_override() by default prefers the DSDT IRQ 1 >>>>> settings over the MADT settings. >>>>> >>>>> This causes the keyboard to malfunction on some laptop models >>>>> (see Links), all models from the Links have an INT_SRC_OVR MADT entry >>>>> for IRQ 1. >>>>> >>>>> Fixes: a9c4a912b7dc ("ACPI: resource: Remove "Zen" specific match and quirks") >>>>> Link: https://bugzilla.kernel.org/show_bug.cgi?id=217336 >>>>> Link: https://bugzilla.kernel.org/show_bug.cgi?id=217394 >>>>> Link: https://bugzilla.kernel.org/show_bug.cgi?id=217406 >>>>> Cc: Mario Limonciello <mario.limonciello@amd.com> >>>>> Cc: Linux regressions mailing list <regressions@lists.linux.dev> >>>>> Cc: stable@vger.kernel.org >>>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com> >>>> >>>> One of the laptops fixed by a9c4a912b7dc, PCSpecialist Elimina Pro 16 M [1], seems to have no INT_SRC_OVR entry for IRQ 1 [2]: >>>> >>>>> [ 0.084265] ACPI: INT_SRC_OVR (bus 0 bus_irq 0 global_irq 2 dfl dfl) >>>>> [ 0.084266] ACPI: INT_SRC_OVR (bus 0 bus_irq 9 global_irq 9 low level) >>>> >>>> I'm not sure if it was IRQ 1 that needed to be overridden for that model though, so it may work anyway with patch 2 of this series. >>>> >>>> [1] https://bugzilla.kernel.org/show_bug.cgi?id=217394#c18 >>>> [2] https://bugzilla.kernel.org/attachment.cgi?id=304338 >>> >>> Good catch, thanks. So it looks like this one needs a DMI quirk (until we have a better generic solution. >>> >>> I'll reach out to the reporter and ask for dmidecode output and prepare a follow-up patch. Still I think >>> that we should move forward with this series to fix the 6 bugs which are linked to from PAtch 1's >>> commitmsg and those are likely just the top of the iceberg. >> >> You are probably right, but it would be good to get a fix for this >> ASAP, as I would prefer it to go in along with the series, if >> possible. > > Agreed I've asked in the bugzilla for dmidecode output for the laptop model in question (I checked https://linux-hardware.org/ and it does not have this model). > > As soon as I've dmidecode info I'll prepare the follow-up patch as well as a Fedora kernel with the entire series + qurik patch for the reporter to test. I have just send out the follow up patch with the DMI quirk: https://lore.kernel.org/linux-acpi/20230810090011.104770-1-hdegoede@redhat.com/ And I have started a Fedora kernel test build with this series + the quirk for the reporter to test: https://bugzilla.kernel.org/show_bug.cgi?id=217394#c33 Regards, Hans
diff --git a/arch/x86/include/asm/acpi.h b/arch/x86/include/asm/acpi.h index 8eb74cf386db..6bc00fddf8dd 100644 --- a/arch/x86/include/asm/acpi.h +++ b/arch/x86/include/asm/acpi.h @@ -15,6 +15,7 @@ #include <asm/mpspec.h> #include <asm/x86_init.h> #include <asm/cpufeature.h> +#include <asm/irq_vectors.h> #ifdef CONFIG_ACPI_APEI # include <asm/pgtable_types.h> @@ -31,6 +32,7 @@ extern int acpi_skip_timer_override; extern int acpi_use_timer_override; extern int acpi_fix_pin2_polarity; extern int acpi_disable_cmcff; +extern int acpi_int_src_ovr[NR_IRQS_LEGACY]; extern u8 acpi_sci_flags; extern u32 acpi_sci_override_gsi; diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c index 21b542a6866c..b88e5e0135ab 100644 --- a/arch/x86/kernel/acpi/boot.c +++ b/arch/x86/kernel/acpi/boot.c @@ -52,6 +52,7 @@ int acpi_lapic; int acpi_ioapic; int acpi_strict; int acpi_disable_cmcff; +int acpi_int_src_ovr[NR_IRQS_LEGACY]; /* ACPI SCI override configuration */ u8 acpi_sci_flags __initdata; @@ -588,6 +589,9 @@ acpi_parse_int_src_ovr(union acpi_subtable_headers * header, acpi_table_print_madt_entry(&header->common); + if (intsrc->source_irq < NR_IRQS_LEGACY) + acpi_int_src_ovr[intsrc->source_irq] = 1; + if (intsrc->source_irq == acpi_gbl_FADT.sci_interrupt) { acpi_sci_ioapic_setup(intsrc->source_irq, intsrc->inti_flags & ACPI_MADT_POLARITY_MASK, diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c index 380cda1e86f4..a0bb53868e18 100644 --- a/drivers/acpi/resource.c +++ b/drivers/acpi/resource.c @@ -551,6 +551,10 @@ static bool acpi_dev_irq_override(u32 gsi, u8 triggering, u8 polarity, if (gsi != 1 && gsi != 12) return true; + /* If the override comes from an INT_SRC_OVR MADT entry honor it */ + if (acpi_int_src_ovr[gsi]) + return true; + /* * IRQ override isn't needed on modern AMD Zen systems and * this override breaks active low IRQs on AMD Ryzen 6000 and
On AMD Zen acpi_dev_irq_override() by default prefers the DSDT IRQ 1 settings over the MADT settings. This causes the keyboard to malfunction on some laptop models (see Links), all models from the Links have an INT_SRC_OVR MADT entry for IRQ 1. Fixes: a9c4a912b7dc ("ACPI: resource: Remove "Zen" specific match and quirks") Link: https://bugzilla.kernel.org/show_bug.cgi?id=217336 Link: https://bugzilla.kernel.org/show_bug.cgi?id=217394 Link: https://bugzilla.kernel.org/show_bug.cgi?id=217406 Cc: Mario Limonciello <mario.limonciello@amd.com> Cc: Linux regressions mailing list <regressions@lists.linux.dev> Cc: stable@vger.kernel.org Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- Changes in v3: - Make acpi_int_src_ovr an array which keep track of the status of all legacy IRQs and not just IRQ 1 --- arch/x86/include/asm/acpi.h | 2 ++ arch/x86/kernel/acpi/boot.c | 4 ++++ drivers/acpi/resource.c | 4 ++++ 3 files changed, 10 insertions(+)