Hi,
Resending as this has gone into void. Also there is a context dependency
with a change developed later possibly causing a merge conflict, so to
make it easier to queue the incoming patches I have folded the follow-up
change into this series, expanding it to 4 patches from the original 3 and
mechanically regenerating according to upstream changes. I have updated
the cover letter accordingly.
While working on the SiS85C497 PIRQ router I have noticed an odd
phenomenon with my venerable Tyan Tomcat IV S1564D board, where the PCI
INTD# line of the USB host controller included as function 3 of the PIIX3
southbridge cannot be routed in the `noapic' mode. As it turns out the
reason for this is the BIOS has two individual entries in its PIRQ table
for two of its three functions, and the wrong one is chosen for routing
said line.
Strictly speaking this violates the PCI BIOS specification, but it can be
easily worked around while preserving the semantics for compliant systems.
Therefore I have come up with this patch series, which addresses this
problem with 3/4, adds function reporting to the debug PIRQ table dump
with 2/4 and also prints a usable physical memory address of the PIRQ
table in a debug message with 1/4.
Then 4/4 follows, addressing the inability to use a PIRQ table to route
interrupts for devices placed behind PCI-to-PCI bridges on option cards,
and especially where the BIOS has failed to enumerate the whole bus tree
in the first place.
See individual change descriptions for further details.
Please apply.
Maciej
Contrary to the PCI BIOS specification[1] some systems include the PCI
function number for motherboard devices in their $PIR table, e.g. this
is what the Tyan Tomcat IV S1564D board reports:
00:14 slot=01
0:60/deb8
1:61/deb8
2:62/deb8
3:63/deb8
00:13 slot=02
0:61/deb8
1:62/deb8
2:63/deb8
3:60/deb8
00:12 slot=03
0:62/deb8
1:63/deb8
2:60/deb8
3:61/deb8
00:11 slot=04
0:63/deb8
1:60/deb8
2:61/deb8
3:62/deb8
00:07 slot=00
0:00/deb8
1:00/deb8
2:00/deb8
3:00/deb8
00:07 slot=00
0:00/deb8
1:00/deb8
2:00/deb8
3:63/deb8
Print the function number then in the debug $PIR table dump:
00:14.0 slot=01
0:60/deb8
1:61/deb8
2:62/deb8
3:63/deb8
00:13.0 slot=02
0:61/deb8
1:62/deb8
2:63/deb8
3:60/deb8
00:12.0 slot=03
0:62/deb8
1:63/deb8
2:60/deb8
3:61/deb8
00:11.0 slot=04
0:63/deb8
1:60/deb8
2:61/deb8
3:62/deb8
00:07.1 slot=00
0:00/deb8
1:00/deb8
2:00/deb8
3:00/deb8
00:07.2 slot=00
0:00/deb8
1:00/deb8
2:00/deb8
3:63/deb8
References:
[1] "PCI BIOS Specification", Revision 2.1, PCI Special Interest Group,
August 26, 1994, Table 4-1 "Layout of IRQ routing table entry.", p.
12
Signed-off-by: Maciej W. Rozycki <[email protected]>
---
No change from v1.
---
arch/x86/pci/irq.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
linux-x86-debug-pirq-fn.diff
Index: linux-macro/arch/x86/pci/irq.c
===================================================================
--- linux-macro.orig/arch/x86/pci/irq.c
+++ linux-macro/arch/x86/pci/irq.c
@@ -135,7 +135,8 @@ static void __init pirq_peer_trick(void)
#ifdef DEBUG
{
int j;
- DBG(KERN_DEBUG "%02x:%02x slot=%02x", e->bus, e->devfn/8, e->slot);
+ DBG(KERN_DEBUG "%02x:%02x.%x slot=%02x",
+ e->bus, e->devfn / 8, e->devfn % 8, e->slot);
for (j = 0; j < 4; j++)
DBG(" %d:%02x/%04x", j, e->irq[j].link, e->irq[j].bitmap);
DBG("\n");
It makes no sense to hide the address of the $PIR table in a debug dump:
PCI: Interrupt Routing Table found at 0x(ptrval)
let alone print its virtual address, given that this is a BIOS entity at
a fixed location in the system's memory map. Show the physical address
instead then, e.g.:
PCI: Interrupt Routing Table found at 0xfde10
Signed-off-by: Maciej W. Rozycki <[email protected]>
---
No change from v1.
---
arch/x86/pci/irq.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
linux-x86-debug-pirq-addr.diff
Index: linux-macro/arch/x86/pci/irq.c
===================================================================
--- linux-macro.orig/arch/x86/pci/irq.c
+++ linux-macro/arch/x86/pci/irq.c
@@ -84,8 +84,8 @@ static inline struct irq_routing_table *
for (i = 0; i < rt->size; i++)
sum += addr[i];
if (!sum) {
- DBG(KERN_DEBUG "PCI: Interrupt Routing Table found at 0x%p\n",
- rt);
+ DBG(KERN_DEBUG "PCI: Interrupt Routing Table found at 0x%lx\n",
+ __pa(rt));
return rt;
}
return NULL;
Contrary to the PCI BIOS specification[1] some systems include the PCI
function number for onboard devices in their $PIR table. Consequently
the wrong entry can be matched leading to interrupt routing failures.
For example the Tyan Tomcat IV S1564D board has:
00:07.1 slot=00
0:00/deb8
1:00/deb8
2:00/deb8
3:00/deb8
00:07.2 slot=00
0:00/deb8
1:00/deb8
2:00/deb8
3:63/deb8
for its IDE interface and USB controller functions of the 82371SB PIIX3
southbridge. Consequently the first entry matches causing the inability
to route the USB interrupt in the `noapic' mode, in which case we need
to rely on the interrupt line set by the BIOS:
uhci_hcd 0000:00:07.2: runtime IRQ mapping not provided by arch
uhci_hcd 0000:00:07.2: PCI INT D not routed
uhci_hcd 0000:00:07.2: enabling bus mastering
uhci_hcd 0000:00:07.2: UHCI Host Controller
uhci_hcd 0000:00:07.2: new USB bus registered, assigned bus number 1
uhci_hcd 0000:00:07.2: irq 11, io base 0x00006000
Try to match the PCI device and function combined then and if that fails
move on to PCI device matching only. Compliant systems will only have a
single $PIR table entry per PCI device, so this update does not change
the semantics with them, while systems that have several entries for
individual functions of a single PCI device each will match the correct
entry:
uhci_hcd 0000:00:07.2: runtime IRQ mapping not provided by arch
uhci_hcd 0000:00:07.2: PCI INT D -> PIRQ 63, mask deb8, excl 0c20
uhci_hcd 0000:00:07.2: PCI INT D -> newirq 11
uhci_hcd 0000:00:07.2: found PCI INT D -> IRQ 11
uhci_hcd 0000:00:07.2: sharing IRQ 11 with 0000:00:11.0
uhci_hcd 0000:00:07.2: enabling bus mastering
uhci_hcd 0000:00:07.2: UHCI Host Controller
uhci_hcd 0000:00:07.2: new USB bus registered, assigned bus number 1
uhci_hcd 0000:00:07.2: irq 11, io base 0x00006000
[1] "PCI BIOS Specification", Revision 2.1, PCI Special Interest Group,
August 26, 1994, Table 4-1 "Layout of IRQ routing table entry.", p.
12
Signed-off-by: Maciej W. Rozycki <[email protected]>
---
No change from v1.
---
arch/x86/pci/irq.c | 19 +++++++++++++++----
1 file changed, 15 insertions(+), 4 deletions(-)
linux-x86-pirq-fn.diff
Index: linux-macro/arch/x86/pci/irq.c
===================================================================
--- linux-macro.orig/arch/x86/pci/irq.c
+++ linux-macro/arch/x86/pci/irq.c
@@ -1132,18 +1132,29 @@ static void __init pirq_find_router(stru
/* The device remains referenced for the kernel lifetime */
}
+/*
+ * We're supposed to match on the PCI device only and not the function,
+ * but some BIOSes build their tables with the PCI function included
+ * for motherboard devices, so if a complete match is found, then give
+ * it precedence over a slot match.
+ */
static struct irq_info *pirq_get_info(struct pci_dev *dev)
{
struct irq_routing_table *rt = pirq_table;
int entries = (rt->size - sizeof(struct irq_routing_table)) /
sizeof(struct irq_info);
+ struct irq_info *slotinfo = NULL;
struct irq_info *info;
for (info = rt->slots; entries--; info++)
- if (info->bus == dev->bus->number &&
- PCI_SLOT(info->devfn) == PCI_SLOT(dev->devfn))
- return info;
- return NULL;
+ if (info->bus == dev->bus->number) {
+ if (info->devfn == dev->devfn)
+ return info;
+ if (!slotinfo &&
+ PCI_SLOT(info->devfn) == PCI_SLOT(dev->devfn))
+ slotinfo = info;
+ }
+ return slotinfo;
}
static int pcibios_lookup_irq(struct pci_dev *dev, int assign)
On Sun, Jan 02, 2022 at 11:24:19PM +0000, Maciej W. Rozycki wrote:
> Hi,
>
> Resending as this has gone into void. Also there is a context dependency
> with a change developed later possibly causing a merge conflict, so to
> make it easier to queue the incoming patches I have folded the follow-up
> change into this series, expanding it to 4 patches from the original 3 and
> mechanically regenerating according to upstream changes. I have updated
> the cover letter accordingly.
I'm assuming the x86/IRQ folks will handle this, too.
The following commit has been merged into the x86/irq branch of tip:
Commit-ID: 5c2830301a8784d0392aec617856f1b973bc5bea
Gitweb: https://git.kernel.org/tip/5c2830301a8784d0392aec617856f1b973bc5bea
Author: Maciej W. Rozycki <[email protected]>
AuthorDate: Sun, 02 Jan 2022 23:24:23
Committer: Thomas Gleixner <[email protected]>
CommitterDate: Wed, 02 Feb 2022 21:27:54 +01:00
x86/PCI: Show the physical address of the $PIR table
It makes no sense to hide the address of the $PIR table in a debug dump:
PCI: Interrupt Routing Table found at 0x(ptrval)
let alone print its virtual address, given that this is a BIOS entity at
a fixed location in the system's memory map. Show the physical address
instead then, e.g.:
PCI: Interrupt Routing Table found at 0xfde10
Signed-off-by: Maciej W. Rozycki <[email protected]>
Signed-off-by: Thomas Gleixner <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
arch/x86/pci/irq.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/x86/pci/irq.c b/arch/x86/pci/irq.c
index 97b63e3..a33fe9c 100644
--- a/arch/x86/pci/irq.c
+++ b/arch/x86/pci/irq.c
@@ -84,8 +84,8 @@ static inline struct irq_routing_table *pirq_check_routing_table(u8 *addr)
for (i = 0; i < rt->size; i++)
sum += addr[i];
if (!sum) {
- DBG(KERN_DEBUG "PCI: Interrupt Routing Table found at 0x%p\n",
- rt);
+ DBG(KERN_DEBUG "PCI: Interrupt Routing Table found at 0x%lx\n",
+ __pa(rt));
return rt;
}
return NULL;
The following commit has been merged into the x86/irq branch of tip:
Commit-ID: 9574931789945734fd8abcf99883b83a3b3b32ba
Gitweb: https://git.kernel.org/tip/9574931789945734fd8abcf99883b83a3b3b32ba
Author: Maciej W. Rozycki <[email protected]>
AuthorDate: Sun, 02 Jan 2022 23:24:29
Committer: Thomas Gleixner <[email protected]>
CommitterDate: Wed, 02 Feb 2022 21:27:54 +01:00
x86/PCI: Include function number in $PIR table dump
Contrary to the PCI BIOS specification[1] some systems include the PCI
function number for motherboard devices in their $PIR table, e.g. this
is what the Tyan Tomcat IV S1564D board reports:
00:14 slot=01
0:60/deb8
1:61/deb8
2:62/deb8
3:63/deb8
00:13 slot=02
0:61/deb8
1:62/deb8
2:63/deb8
3:60/deb8
00:12 slot=03
0:62/deb8
1:63/deb8
2:60/deb8
3:61/deb8
00:11 slot=04
0:63/deb8
1:60/deb8
2:61/deb8
3:62/deb8
00:07 slot=00
0:00/deb8
1:00/deb8
2:00/deb8
3:00/deb8
00:07 slot=00
0:00/deb8
1:00/deb8
2:00/deb8
3:63/deb8
Print the function number then in the debug $PIR table dump:
00:14.0 slot=01
0:60/deb8
1:61/deb8
2:62/deb8
3:63/deb8
00:13.0 slot=02
0:61/deb8
1:62/deb8
2:63/deb8
3:60/deb8
00:12.0 slot=03
0:62/deb8
1:63/deb8
2:60/deb8
3:61/deb8
00:11.0 slot=04
0:63/deb8
1:60/deb8
2:61/deb8
3:62/deb8
00:07.1 slot=00
0:00/deb8
1:00/deb8
2:00/deb8
3:00/deb8
00:07.2 slot=00
0:00/deb8
1:00/deb8
2:00/deb8
3:63/deb8
References:
[1] "PCI BIOS Specification", Revision 2.1, PCI Special Interest Group,
August 26, 1994, Table 4-1 "Layout of IRQ routing table entry.", p.
12
Signed-off-by: Maciej W. Rozycki <[email protected]>
Signed-off-by: Thomas Gleixner <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
arch/x86/pci/irq.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/x86/pci/irq.c b/arch/x86/pci/irq.c
index a33fe9c..b6b9853 100644
--- a/arch/x86/pci/irq.c
+++ b/arch/x86/pci/irq.c
@@ -135,7 +135,8 @@ static void __init pirq_peer_trick(void)
#ifdef DEBUG
{
int j;
- DBG(KERN_DEBUG "%02x:%02x slot=%02x", e->bus, e->devfn/8, e->slot);
+ DBG(KERN_DEBUG "%02x:%02x.%x slot=%02x",
+ e->bus, e->devfn / 8, e->devfn % 8, e->slot);
for (j = 0; j < 4; j++)
DBG(" %d:%02x/%04x", j, e->irq[j].link, e->irq[j].bitmap);
DBG("\n");
The following commit has been merged into the x86/irq branch of tip:
Commit-ID: 9598dca94cbf86c1bd62e752baf5edb8001f36dd
Gitweb: https://git.kernel.org/tip/9598dca94cbf86c1bd62e752baf5edb8001f36dd
Author: Maciej W. Rozycki <[email protected]>
AuthorDate: Sun, 02 Jan 2022 23:24:35
Committer: Thomas Gleixner <[email protected]>
CommitterDate: Wed, 02 Feb 2022 21:27:55 +01:00
x86/PCI: Also match function number in $PIR table
Contrary to the PCI BIOS specification[1] some systems include the PCI
function number for onboard devices in their $PIR table. Consequently
the wrong entry can be matched leading to interrupt routing failures.
For example the Tyan Tomcat IV S1564D board has:
00:07.1 slot=00
0:00/deb8
1:00/deb8
2:00/deb8
3:00/deb8
00:07.2 slot=00
0:00/deb8
1:00/deb8
2:00/deb8
3:63/deb8
for its IDE interface and USB controller functions of the 82371SB PIIX3
southbridge. Consequently the first entry matches causing the inability to
route the USB interrupt in the `noapic' mode, in which case the kernel must
rely on the interrupt line set by the BIOS:
uhci_hcd 0000:00:07.2: runtime IRQ mapping not provided by arch
uhci_hcd 0000:00:07.2: PCI INT D not routed
uhci_hcd 0000:00:07.2: enabling bus mastering
uhci_hcd 0000:00:07.2: UHCI Host Controller
uhci_hcd 0000:00:07.2: new USB bus registered, assigned bus number 1
uhci_hcd 0000:00:07.2: irq 11, io base 0x00006000
Try to match the PCI device and function combined then and if that fails
move on to PCI device matching only. Compliant systems will only have a
single $PIR table entry per PCI device, so this update does not change
the semantics with them, while systems that have several entries for
individual functions of a single PCI device each will match the correct
entry:
uhci_hcd 0000:00:07.2: runtime IRQ mapping not provided by arch
uhci_hcd 0000:00:07.2: PCI INT D -> PIRQ 63, mask deb8, excl 0c20
uhci_hcd 0000:00:07.2: PCI INT D -> newirq 11
uhci_hcd 0000:00:07.2: found PCI INT D -> IRQ 11
uhci_hcd 0000:00:07.2: sharing IRQ 11 with 0000:00:11.0
uhci_hcd 0000:00:07.2: enabling bus mastering
uhci_hcd 0000:00:07.2: UHCI Host Controller
uhci_hcd 0000:00:07.2: new USB bus registered, assigned bus number 1
uhci_hcd 0000:00:07.2: irq 11, io base 0x00006000
[1] "PCI BIOS Specification", Revision 2.1, PCI Special Interest Group,
August 26, 1994, Table 4-1 "Layout of IRQ routing table entry.", p.
12
Signed-off-by: Maciej W. Rozycki <[email protected]>
Signed-off-by: Thomas Gleixner <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
arch/x86/pci/irq.c | 19 +++++++++++++++----
1 file changed, 15 insertions(+), 4 deletions(-)
diff --git a/arch/x86/pci/irq.c b/arch/x86/pci/irq.c
index b6b9853..dcb9c21 100644
--- a/arch/x86/pci/irq.c
+++ b/arch/x86/pci/irq.c
@@ -1132,18 +1132,29 @@ static void __init pirq_find_router(struct irq_router *r)
/* The device remains referenced for the kernel lifetime */
}
+/*
+ * We're supposed to match on the PCI device only and not the function,
+ * but some BIOSes build their tables with the PCI function included
+ * for motherboard devices, so if a complete match is found, then give
+ * it precedence over a slot match.
+ */
static struct irq_info *pirq_get_info(struct pci_dev *dev)
{
struct irq_routing_table *rt = pirq_table;
int entries = (rt->size - sizeof(struct irq_routing_table)) /
sizeof(struct irq_info);
+ struct irq_info *slotinfo = NULL;
struct irq_info *info;
for (info = rt->slots; entries--; info++)
- if (info->bus == dev->bus->number &&
- PCI_SLOT(info->devfn) == PCI_SLOT(dev->devfn))
- return info;
- return NULL;
+ if (info->bus == dev->bus->number) {
+ if (info->devfn == dev->devfn)
+ return info;
+ if (!slotinfo &&
+ PCI_SLOT(info->devfn) == PCI_SLOT(dev->devfn))
+ slotinfo = info;
+ }
+ return slotinfo;
}
static int pcibios_lookup_irq(struct pci_dev *dev, int assign)
On Thu, 6 Jan 2022, Bjorn Helgaas wrote:
> On Sun, Jan 02, 2022 at 11:24:19PM +0000, Maciej W. Rozycki wrote:
> >
> > Resending as this has gone into void. Also there is a context dependency
> > with a change developed later possibly causing a merge conflict, so to
> > make it easier to queue the incoming patches I have folded the follow-up
> > change into this series, expanding it to 4 patches from the original 3 and
> > mechanically regenerating according to upstream changes. I have updated
> > the cover letter accordingly.
>
> I'm assuming the x86/IRQ folks will handle this, too.
Ping for:
<https://lore.kernel.org/lkml/[email protected]/>.
Series re-verified against 5.17-rc2. Thank you for your input, Bjorn!
Maciej