| Message ID | 20221113173442.5770-1-mat.jonczyk@o2.pl |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2] acpi,pci: warn about duplicate IRQ routing entries returned from _PRT | expand |
Hello, W dniu 15.11.2022 o 09:36, Jean Delvare pisze: > Hi Mateusz, > > On Sun, 13 Nov 2022 18:34:42 +0100, Mateusz Jończyk wrote: >> On some platforms, the ACPI _PRT function returns duplicate interrupt >> routing entries. Linux uses the first matching entry, but sometimes the >> second matching entry contains the correct interrupt vector. >> >> Print a warning to dmesg if duplicate interrupt routing entries are >> present, so that we could check how many models are affected. > Excellent idea. We want hardware manufacturers to fix such bugs in the > firmware, and the best way for this to happen is to report them > whenever they are encountered. > >> This happens on a Dell Latitude E6500 laptop with the i2c-i801 Intel >> SMBus controller. This controller was nonfunctional unless its interrupt >> usage was disabled (using the "disable_features=0x10" module parameter). >> >> After investigation, it turned out that the driver was using an >> incorrect interrupt vector: in lspci output for this device there was: >> Interrupt: pin B routed to IRQ 19 >> but after running i2cdetect (without using any i2c-i801 module >> parameters) the following was logged to dmesg: >> >> [...] >> i801_smbus 0000:00:1f.3: Timeout waiting for interrupt! >> i801_smbus 0000:00:1f.3: Transaction timeout >> i801_smbus 0000:00:1f.3: Timeout waiting for interrupt! >> i801_smbus 0000:00:1f.3: Transaction timeout >> irq 17: nobody cared (try booting with the "irqpoll" option) >> >> Existence of duplicate entries in a table returned by the _PRT method >> was confirmed by disassembling the ACPI DSDT table. > Excuse a probably stupid question, but what would happen if we would > plain ignore the IRQ routing information from ACPI in this case? Would > we fallback to some pure-PCI routing logic which may have a chance to > find the right IRQ routing (matching the second ACPI routing entry in > this case)?
diff --git a/drivers/acpi/pci_irq.c b/drivers/acpi/pci_irq.c index 08e15774fb9f..a4e41b7b71ed 100644 --- a/drivers/acpi/pci_irq.c +++ b/drivers/acpi/pci_irq.c @@ -203,6 +203,8 @@ static int acpi_pci_irq_find_prt_entry(struct pci_dev *dev, struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL }; struct acpi_pci_routing_table *entry; acpi_handle handle = NULL; + struct acpi_prt_entry *match = NULL; + const char *match_int_source = NULL; if (dev->bus->bridge) handle = ACPI_HANDLE(dev->bus->bridge); @@ -219,13 +221,30 @@ static int acpi_pci_irq_find_prt_entry(struct pci_dev *dev, entry = buffer.pointer; while (entry && (entry->length > 0)) { - if (!acpi_pci_irq_check_entry(handle, dev, pin, - entry, entry_ptr)) - break; + struct acpi_prt_entry *curr; + + if (!acpi_pci_irq_check_entry(handle, dev, pin, entry, &curr)) { + if (!match) { + match = curr; + match_int_source = entry->source; + } else { + pr_warn(FW_BUG + "ACPI _PRT returned duplicate IRQ routing entries for device " + "%04x:%02x:%02x[INT%c]: %s[%d] and %s[%d].\n", + curr->id.segment, curr->id.bus, curr->id.device, + pin_name(curr->pin), + match_int_source, match->index, + entry->source, curr->index); + // we use the first matching entry nonetheless + } + } + entry = (struct acpi_pci_routing_table *) ((unsigned long)entry + entry->length); } + *entry_ptr = match; + kfree(buffer.pointer); return 0; }
On some platforms, the ACPI _PRT function returns duplicate interrupt routing entries. Linux uses the first matching entry, but sometimes the second matching entry contains the correct interrupt vector. Print a warning to dmesg if duplicate interrupt routing entries are present, so that we could check how many models are affected. This happens on a Dell Latitude E6500 laptop with the i2c-i801 Intel SMBus controller. This controller was nonfunctional unless its interrupt usage was disabled (using the "disable_features=0x10" module parameter). After investigation, it turned out that the driver was using an incorrect interrupt vector: in lspci output for this device there was: Interrupt: pin B routed to IRQ 19 but after running i2cdetect (without using any i2c-i801 module parameters) the following was logged to dmesg: [...] i801_smbus 0000:00:1f.3: Timeout waiting for interrupt! i801_smbus 0000:00:1f.3: Transaction timeout i801_smbus 0000:00:1f.3: Timeout waiting for interrupt! i801_smbus 0000:00:1f.3: Transaction timeout irq 17: nobody cared (try booting with the "irqpoll" option) Existence of duplicate entries in a table returned by the _PRT method was confirmed by disassembling the ACPI DSDT table. Signed-off-by: Mateusz Jończyk <mat.jonczyk@o2.pl> Cc: Bjorn Helgaas <bhelgaas@google.com> Cc: "Rafael J. Wysocki" <rafael@kernel.org> Cc: Len Brown <lenb@kernel.org> Cc: Borislav Petkov <bp@suse.de> Cc: Jean Delvare <jdelvare@suse.com> -- v2: - add a newline at the end of the kernel log message, - replace: "if (match == NULL)" -> "if (!match)" - patch description tweaks. Tested on two computers, including the affected Dell Latitude E6500 laptop. drivers/acpi/pci_irq.c | 25 ++++++++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) base-commit: f0c4d9fc9cc9462659728d168387191387e903cc