Problems since commit Recognize that Interrupt Line 255 means "not connected"

DISCUSSION Heiner Kallweit 5 participants 8 messages Feb 5, 2016 – Feb 15, 2016

Subsystems (3)

ACPI PCI IRQ

lore

Share & tools

lore

Heiner Kallweit

author

2016-02-05 23:00 UTC #0 lore raw

Since commit a44c386a361e "x86/PCI/ACPI: Recognize that Interrupt Line 255 means "not connected"" I get several "PCI INT not connected" warnings on a Zotac CI321 and EHCI failes to load:

ehci-pci 0000:00:1d.0: PCI INT not connected
ehci-pci 0000:00:1d.0: EHCI Host Controller
ehci-pci 0000:00:1d.0: new USB bus registered, assigned bus number 3 ehci-pci 0000:00:1d.0: debug port 2
ehci-pci 0000:00:1d.0: cache line size of 64 is not supported ehci-pci 0000:00:1d.0: request interrupt -2147483648 failed ehci-pci 0000:00:1d.0: USB bus 3 deregistered
ehci-pci 0000:00:1d.0: init 0000:00:1d.0 fail, -107
ehci-pci: probe of 0000:00:1d.0 failed with error -107

XHCI brings the "PCI INT not connected" warning too but loads properly (most likely because it uses a MSI interrupt).

Not sure whether this commit is to blame or a broken BIOS. Just let me know in case you need additional information.

Regards, Heiner

Rafael J. Wysocki

2016-02-05 23:37 UTC #1 lore raw

Quoted 3 lines · Heiner Kallweit

Since commit a44c386a361e "x86/PCI/ACPI: Recognize that Interrupt Line 255 means "not connected"" I get several "PCI INT not connected" warnings on a Zotac CI321 and EHCI failes to load:

That doesn't follow clearly from your report, but I'm assuming that it works correctly without that commit, right?

Quoted 10 lines

ehci-pci 0000:00:1d.0: PCI INT not connected
ehci-pci 0000:00:1d.0: EHCI Host Controller
ehci-pci 0000:00:1d.0: new USB bus registered, assigned bus number 3 ehci-pci 0000:00:1d.0: debug port 2
ehci-pci 0000:00:1d.0: cache line size of 64 is not supported ehci-pci 0000:00:1d.0: request interrupt -2147483648 failed ehci-pci 0000:00:1d.0: USB bus 3 deregistered
ehci-pci 0000:00:1d.0: init 0000:00:1d.0 fail, -107
ehci-pci: probe of 0000:00:1d.0 failed with error -107

Quoted 5 lines

XHCI brings the "PCI INT not connected" warning too but loads properly (most likely because it uses a MSI interrupt).
Not sure whether this commit is to blame or a broken BIOS. Just let me know in case you need additional information.

Well, even if the BIOS is "broken" and the kernel without the above commit is able to work with it, we cannot break that.

Thanks,
Rafael

Heiner Kallweit

author

2016-02-06 08:09 UTC #2 lore raw

Am 06.02.2016 um 00:37 schrieb Rafael J. Wysocki:

Quoted 8 lines · Heiner Kallweit

Quoted 3 lines
Since commit a44c386a361e "x86/PCI/ACPI: Recognize that Interrupt Line 255 means "not connected"" I get several "PCI INT not connected" warnings on a Zotac CI321 and EHCI failes to load:
That doesn't follow clearly from your report, but I'm assuming that it works correctly without that commit, right?

Right, w/o this commit it looks like this:

dmesg
ehci-pci 0000:00:1d.0: irq 23, io mem 0xf7d1b000

/proc/interrupts
IO-APIC 23-fasteoi ehci_hcd:usb3

Quoted 23 lines

Quoted 10 lines
ehci-pci 0000:00:1d.0: PCI INT not connected
ehci-pci 0000:00:1d.0: EHCI Host Controller
ehci-pci 0000:00:1d.0: new USB bus registered, assigned bus number 3 ehci-pci 0000:00:1d.0: debug port 2
ehci-pci 0000:00:1d.0: cache line size of 64 is not supported ehci-pci 0000:00:1d.0: request interrupt -2147483648 failed ehci-pci 0000:00:1d.0: USB bus 3 deregistered
ehci-pci 0000:00:1d.0: init 0000:00:1d.0 fail, -107
ehci-pci: probe of 0000:00:1d.0 failed with error -107
Quoted 5 lines
XHCI brings the "PCI INT not connected" warning too but loads properly (most likely because it uses a MSI interrupt).
Not sure whether this commit is to blame or a broken BIOS. Just let me know in case you need additional information.
Well, even if the BIOS is "broken" and the kernel without the above commit is able to work with it, we cannot break that.
Thanks,
Rafael

Bjorn Helgaas

2016-02-06 15:19 UTC #3 lore raw

Hi Heiner,

Quoted 9 lines · Heiner Kallweit

Am 06.02.2016 um 00:37 schrieb Rafael J. Wysocki:
Quoted 8 lines · Heiner Kallweit
Quoted 3 lines
Since commit a44c386a361e "x86/PCI/ACPI: Recognize that Interrupt Line 255 means "not connected"" I get several "PCI INT not connected" warnings on a Zotac CI321 and EHCI failes to load:
That doesn't follow clearly from your report, but I'm assuming that it works correctly without that commit, right?

Quoted 7 lines

Right, w/o this commit it looks like this:
dmesg
ehci-pci 0000:00:1d.0: irq 23, io mem 0xf7d1b000
/proc/interrupts
IO-APIC 23-fasteoi ehci_hcd:usb3

Thanks a lot for your report! This is a bit of a minefield, and I was worried that we'd trip over something with a44c386a361e.

Oops, I think I see a problem with a44c386a361e. We're checking for Interrupt Line == 255 even before we try to look it up in the _PRT. I think we should only check Interrupt Line *after* everything else has failed. Can you try the patch below instead of a44c386a361e?

If the patch below doesn't work, would you mind collecting the complete output of "lspci -vvv" and the complete dmesg logs from kernels with and without a44c386a361e, and putting them somewhere (maybe a bugzilla.kernel.org report)?

Bjorn

drivers/acpi/pci_irq.c +25 -4

index d30184c..807a0a8 100644
@@ -33,6 +33,7 @@
 #include <linux/pci.h>
 #include <linux/acpi.h>
 #include <linux/slab.h>
+#include <linux/interrupt.h>
 
 #define PREFIX "ACPI: "

@@ -387,6 +388,23 @@ static inline int acpi_isa_register_gsi(struct pci_dev *dev)
 }
 #endif
 
+static inline bool acpi_pci_irq_valid(struct pci_dev *dev, u8 pin)
+{
+#ifdef CONFIG_X86
+	/*
+	 * On x86, Interrupt Line 0xff means "unknown" or "no connection"
+	 * (PCI 3.0, Section 6.2.4, footnote on page 223).
+	 */
+	if (dev->irq == 0xff) {
+		dev->irq = IRQ_NOTCONNECTED;
+		dev_warn(&dev->dev, "PCI INT %c: not connected\n",
+			 pin_name(pin));
+		return false;
+	}
+#endif
+	return true;
+}
+
 int acpi_pci_irq_enable(struct pci_dev *dev)
 {
 	struct acpi_prt_entry *entry;

@@ -431,11 +449,14 @@ int acpi_pci_irq_enable(struct pci_dev *dev)
 	} else
 		gsi = -1;
 
-	/*
-	 * No IRQ known to the ACPI subsystem - maybe the BIOS / 
-	 * driver reported one, then use it. Exit in any case.
-	 */
 	if (gsi < 0) {
+		/*
+		 * No IRQ known to the ACPI subsystem - maybe the BIOS /
+		 * driver reported one, then use it. Exit in any case.
+		 */
+		if (!acpi_pci_irq_valid(dev, pin))
+			return 0;
+
 		if (acpi_isa_register_gsi(dev))
 			dev_warn(&dev->dev, "PCI INT %c: no GSI\n",
 				 pin_name(pin));

include/linux/interrupt.h +10 -0

index 0e95fcc..358076e 100644
@@ -125,6 +125,16 @@ struct irqaction {
 
 extern irqreturn_t no_action(int cpl, void *dev_id);
 
+/*
+ * If a (PCI) device interrupt is not connected we set dev->irq to
+ * IRQ_NOTCONNECTED. This causes request_irq() to fail with -ENOTCONN, so we
+ * can distingiush that case from other error returns.
+ *
+ * 0x80000000 is guaranteed to be outside the available range of interrupts
+ * and easy to distinguish from other possible incorrect values.
+ */
+#define IRQ_NOTCONNECTED	(1U << 31)
+
 extern int __must_check
 request_threaded_irq(unsigned int irq, irq_handler_t handler,
 		     irq_handler_t thread_fn,

kernel/irq/manage.c +8 -1

index 8411872..e79e60f 100644
@@ -1609,6 +1609,9 @@ int request_threaded_irq(unsigned int irq, irq_handler_t handler,
 	struct irq_desc *desc;
 	int retval;
 
+	if (irq == IRQ_NOTCONNECTED)
+		return -ENOTCONN;
+
 	/*
 	 * Sanity-check: shared interrupts must pass in a real dev-ID,
 	 * otherwise we'll have trouble later trying to figure out

@@ -1699,9 +1702,13 @@ EXPORT_SYMBOL(request_threaded_irq);
 int request_any_context_irq(unsigned int irq, irq_handler_t handler,
 			    unsigned long flags, const char *name, void *dev_id)
 {
-	struct irq_desc *desc = irq_to_desc(irq);
+	struct irq_desc *desc;
 	int ret;
 
+	if (irq == IRQ_NOTCONNECTED)
+		return -ENOTCONN;
+
+	desc = irq_to_desc(irq);
 	if (!desc)
 		return -EINVAL;

Heiner Kallweit

author

2016-02-06 15:43 UTC #4 lore raw

Am 06.02.2016 um 16:19 schrieb Bjorn Helgaas:

Quoted 20 lines

Hi Heiner,
Quoted 9 lines · Heiner Kallweit
Am 06.02.2016 um 00:37 schrieb Rafael J. Wysocki:
Quoted 8 lines · Heiner Kallweit
Quoted 3 lines
Since commit a44c386a361e "x86/PCI/ACPI: Recognize that Interrupt Line 255 means "not connected"" I get several "PCI INT not connected" warnings on a Zotac CI321 and EHCI failes to load:
That doesn't follow clearly from your report, but I'm assuming that it works correctly without that commit, right?
Quoted 7 lines
Right, w/o this commit it looks like this:
dmesg
ehci-pci 0000:00:1d.0: irq 23, io mem 0xf7d1b000
/proc/interrupts
IO-APIC 23-fasteoi ehci_hcd:usb3

Quoted 8 lines

Thanks a lot for your report! This is a bit of a minefield, and I was worried that we'd trip over something with a44c386a361e.
Oops, I think I see a problem with a44c386a361e. We're checking for Interrupt Line == 255 even before we try to look it up in the _PRT. I think we should only check Interrupt Line *after* everything else has failed. Can you try the patch below instead of a44c386a361e?

With the attached patch everything is fine. No warnings and EHCI loads properly.

Regards, Heiner

Quoted 114 lines

Bjorn

drivers/acpi/pci_irq.c +25 -4

index d30184c..807a0a8 100644
@@ -33,6 +33,7 @@
 #include <linux/pci.h>
 #include <linux/acpi.h>
 #include <linux/slab.h>
+#include <linux/interrupt.h>
 
 #define PREFIX "ACPI: "

@@ -387,6 +388,23 @@ static inline int acpi_isa_register_gsi(struct pci_dev *dev)
 }
 #endif
 
+static inline bool acpi_pci_irq_valid(struct pci_dev *dev, u8 pin)
+{
+#ifdef CONFIG_X86
+	/*
+	 * On x86, Interrupt Line 0xff means "unknown" or "no connection"
+	 * (PCI 3.0, Section 6.2.4, footnote on page 223).
+	 */
+	if (dev->irq == 0xff) {
+		dev->irq = IRQ_NOTCONNECTED;
+		dev_warn(&dev->dev, "PCI INT %c: not connected\n",
+			 pin_name(pin));
+		return false;
+	}
+#endif
+	return true;
+}
+
 int acpi_pci_irq_enable(struct pci_dev *dev)
 {
 	struct acpi_prt_entry *entry;

@@ -431,11 +449,14 @@ int acpi_pci_irq_enable(struct pci_dev *dev)
 	} else
 		gsi = -1;
 
-	/*
-	 * No IRQ known to the ACPI subsystem - maybe the BIOS / 
-	 * driver reported one, then use it. Exit in any case.
-	 */
 	if (gsi < 0) {
+		/*
+		 * No IRQ known to the ACPI subsystem - maybe the BIOS /
+		 * driver reported one, then use it. Exit in any case.
+		 */
+		if (!acpi_pci_irq_valid(dev, pin))
+			return 0;
+
 		if (acpi_isa_register_gsi(dev))
 			dev_warn(&dev->dev, "PCI INT %c: no GSI\n",
 				 pin_name(pin));

include/linux/interrupt.h +10 -0

index 0e95fcc..358076e 100644
@@ -125,6 +125,16 @@ struct irqaction {
 
 extern irqreturn_t no_action(int cpl, void *dev_id);
 
+/*
+ * If a (PCI) device interrupt is not connected we set dev->irq to
+ * IRQ_NOTCONNECTED. This causes request_irq() to fail with -ENOTCONN, so we
+ * can distingiush that case from other error returns.
+ *
+ * 0x80000000 is guaranteed to be outside the available range of interrupts
+ * and easy to distinguish from other possible incorrect values.
+ */
+#define IRQ_NOTCONNECTED	(1U << 31)
+
 extern int __must_check
 request_threaded_irq(unsigned int irq, irq_handler_t handler,
 		     irq_handler_t thread_fn,

kernel/irq/manage.c +8 -1

index 8411872..e79e60f 100644
@@ -1609,6 +1609,9 @@ int request_threaded_irq(unsigned int irq, irq_handler_t handler,
 	struct irq_desc *desc;
 	int retval;
 
+	if (irq == IRQ_NOTCONNECTED)
+		return -ENOTCONN;
+
 	/*
 	 * Sanity-check: shared interrupts must pass in a real dev-ID,
 	 * otherwise we'll have trouble later trying to figure out

@@ -1699,9 +1702,13 @@ EXPORT_SYMBOL(request_threaded_irq);
 int request_any_context_irq(unsigned int irq, irq_handler_t handler,
 			    unsigned long flags, const char *name, void *dev_id)
 {
-	struct irq_desc *desc = irq_to_desc(irq);
+	struct irq_desc *desc;
 	int ret;
 
+	if (irq == IRQ_NOTCONNECTED)
+		return -ENOTCONN;
+
+	desc = irq_to_desc(irq);
 	if (!desc)
 		return -EINVAL;

Bjorn Helgaas

2016-02-09 16:44 UTC #5 lore raw

Quoted 22 lines · Heiner Kallweit

Am 06.02.2016 um 16:19 schrieb Bjorn Helgaas:
Quoted 21 lines · Heiner Kallweit
Quoted 9 lines
Am 06.02.2016 um 00:37 schrieb Rafael J. Wysocki:
Quoted 8 lines · Heiner Kallweit
Quoted 3 lines
Since commit a44c386a361e "x86/PCI/ACPI: Recognize that Interrupt Line 255 means "not connected"" I get several "PCI INT not connected" warnings on a Zotac CI321 and EHCI failes to load:
That doesn't follow clearly from your report, but I'm assuming that it works correctly without that commit, right?
Quoted 7 lines
Right, w/o this commit it looks like this:
dmesg
ehci-pci 0000:00:1d.0: irq 23, io mem 0xf7d1b000
/proc/interrupts
IO-APIC 23-fasteoi ehci_hcd:usb3
Thanks a lot for your report! This is a bit of a minefield, and I was worried that we'd trip over something with a44c386a361e.

Quoted 6 lines

Quoted 5 lines
Oops, I think I see a problem with a44c386a361e. We're checking for Interrupt Line == 255 even before we try to look it up in the _PRT. I think we should only check Interrupt Line *after* everything else has failed. Can you try the patch below instead of a44c386a361e?
With the attached patch everything is fine. No warnings and EHCI loads properly.

Great, thanks a lot for testing this!

Chen, will you update and repost your patch? I know Rafael already had it on a branch, but I think he dropped it, so we need to get the update merged somehow.

Bjorn

Quoted 119 lines

Quoted 104 lines

Bjorn

drivers/acpi/pci_irq.c +25 -4

index d30184c..807a0a8 100644
@@ -33,6 +33,7 @@
 #include <linux/pci.h>
 #include <linux/acpi.h>
 #include <linux/slab.h>
+#include <linux/interrupt.h>
 
 #define PREFIX "ACPI: "

@@ -387,6 +388,23 @@ static inline int acpi_isa_register_gsi(struct pci_dev *dev)
 }
 #endif
 
+static inline bool acpi_pci_irq_valid(struct pci_dev *dev, u8 pin)
+{
+#ifdef CONFIG_X86
+	/*
+	 * On x86, Interrupt Line 0xff means "unknown" or "no connection"
+	 * (PCI 3.0, Section 6.2.4, footnote on page 223).
+	 */
+	if (dev->irq == 0xff) {
+		dev->irq = IRQ_NOTCONNECTED;
+		dev_warn(&dev->dev, "PCI INT %c: not connected\n",
+			 pin_name(pin));
+		return false;
+	}
+#endif
+	return true;
+}
+
 int acpi_pci_irq_enable(struct pci_dev *dev)
 {
 	struct acpi_prt_entry *entry;

@@ -431,11 +449,14 @@ int acpi_pci_irq_enable(struct pci_dev *dev)
 	} else
 		gsi = -1;
 
-	/*
-	 * No IRQ known to the ACPI subsystem - maybe the BIOS / 
-	 * driver reported one, then use it. Exit in any case.
-	 */
 	if (gsi < 0) {
+		/*
+		 * No IRQ known to the ACPI subsystem - maybe the BIOS /
+		 * driver reported one, then use it. Exit in any case.
+		 */
+		if (!acpi_pci_irq_valid(dev, pin))
+			return 0;
+
 		if (acpi_isa_register_gsi(dev))
 			dev_warn(&dev->dev, "PCI INT %c: no GSI\n",
 				 pin_name(pin));

include/linux/interrupt.h +10 -0

index 0e95fcc..358076e 100644
@@ -125,6 +125,16 @@ struct irqaction {
 
 extern irqreturn_t no_action(int cpl, void *dev_id);
 
+/*
+ * If a (PCI) device interrupt is not connected we set dev->irq to
+ * IRQ_NOTCONNECTED. This causes request_irq() to fail with -ENOTCONN, so we
+ * can distingiush that case from other error returns.
+ *
+ * 0x80000000 is guaranteed to be outside the available range of interrupts
+ * and easy to distinguish from other possible incorrect values.
+ */
+#define IRQ_NOTCONNECTED	(1U << 31)
+
 extern int __must_check
 request_threaded_irq(unsigned int irq, irq_handler_t handler,
 		     irq_handler_t thread_fn,

kernel/irq/manage.c +3 -1

index 8411872..e79e60f 100644
@@ -1609,6 +1609,9 @@ int request_threaded_irq(unsigned int irq, irq_handler_t handler,
 	struct irq_desc *desc;
 	int retval;
 
+	if (irq == IRQ_NOTCONNECTED)
+		return -ENOTCONN;
+
 	/*
 	 * Sanity-check: shared interrupts must pass in a real dev-ID,
 	 * otherwise we'll have trouble later trying to figure out

@@ -1699,9 +1702,13 @@ EXPORT_SYMBOL(request_threaded_irq);
 int request_any_context_irq(unsigned int irq, irq_handler_t handler,
 			    unsigned long flags, const char *name, void *dev_id)
 {
-	struct irq_desc *desc = irq_to_desc(irq);

kernel/irq/manage.c EXPORT_SYMBOL(request_threaded_irq); · line 1714

Quoted 10 lines

kernel/irq/manage.c +5 -0

+	struct irq_desc *desc;
 	int ret;
 
+	if (irq == IRQ_NOTCONNECTED)
+		return -ENOTCONN;
+
+	desc = irq_to_desc(irq);
 	if (!desc)
 		return -EINVAL;

--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Rafael J. Wysocki

maintainer

2016-02-09 19:24 UTC #6 lore raw

Quoted 28 lines · Bjorn Helgaas

Quoted 27 lines · Heiner Kallweit
Am 06.02.2016 um 16:19 schrieb Bjorn Helgaas:
Quoted 26 lines · Heiner Kallweit
Quoted 9 lines
Am 06.02.2016 um 00:37 schrieb Rafael J. Wysocki:
Quoted 8 lines · Heiner Kallweit
Quoted 3 lines
Since commit a44c386a361e "x86/PCI/ACPI: Recognize that Interrupt Line 255 means "not connected"" I get several "PCI INT not connected" warnings on a Zotac CI321 and EHCI failes to load:
That doesn't follow clearly from your report, but I'm assuming that it works correctly without that commit, right?
Quoted 7 lines
Right, w/o this commit it looks like this:
dmesg
ehci-pci 0000:00:1d.0: irq 23, io mem 0xf7d1b000
/proc/interrupts
IO-APIC 23-fasteoi ehci_hcd:usb3
Thanks a lot for your report! This is a bit of a minefield, and I was worried that we'd trip over something with a44c386a361e.
Oops, I think I see a problem with a44c386a361e. We're checking for Interrupt Line == 255 even before we try to look it up in the _PRT. I think we should only check Interrupt Line *after* everything else has failed. Can you try the patch below instead of a44c386a361e?

Quoted 7 lines

Quoted 1 line
With the attached patch everything is fine. No warnings and EHCI loads properly.
Great, thanks a lot for testing this!
Chen, will you update and repost your patch? I know Rafael already had it on a branch, but I think he dropped it, so we need to get the update merged somehow.

I'd prefer a new updated patch if that's possible.

Thanks,
Rafael

Chen Fan

2016-02-15 03:02 UTC #7 lore raw

Quoted 27 lines · Rafael J. Wysocki

Quoted 26 lines · Bjorn Helgaas
Quoted 25 lines · Heiner Kallweit
Am 06.02.2016 um 16:19 schrieb Bjorn Helgaas:
Quoted 24 lines · Heiner Kallweit
Quoted 8 lines
Am 06.02.2016 um 00:37 schrieb Rafael J. Wysocki:
Quoted 7 lines · Heiner Kallweit
Quoted 3 lines
Since commit a44c386a361e "x86/PCI/ACPI: Recognize that Interrupt Line 255 means "not connected"" I get several "PCI INT not connected" warnings on a Zotac CI321 and EHCI failes to load:
That doesn't follow clearly from your report, but I'm assuming that it works correctly without that commit, right?
Quoted 7 lines
Right, w/o this commit it looks like this:
dmesg
ehci-pci 0000:00:1d.0: irq 23, io mem 0xf7d1b000
/proc/interrupts
IO-APIC 23-fasteoi ehci_hcd:usb3
Thanks a lot for your report! This is a bit of a minefield, and I was worried that we'd trip over something with a44c386a361e.
Oops, I think I see a problem with a44c386a361e. We're checking for Interrupt Line == 255 even before we try to look it up in the _PRT. I think we should only check Interrupt Line *after* everything else has failed. Can you try the patch below instead of a44c386a361e?

Quoted 7 lines

Quoted 6 lines
Quoted 1 line
With the attached patch everything is fine. No warnings and EHCI loads properly.
Great, thanks a lot for testing this!
Chen, will you update and repost your patch? I know Rafael already had it on a branch, but I think he dropped it, so we need to get the update merged somehow.
I'd prefer a new updated patch if that's possible.

patch is coming soon, thank all of your help.

Chen

Quoted 6 lines

Thanks,
Rafael