Message ID | alpine.DEB.2.21.2107061320570.1711@angie.orcam.me.uk |
---|---|
State | New |
Headers | show |
Series | [v2] x86/PCI: Handle PIRQ routing tables with no router device given | expand |
Hello Maciej, 06.07.2021 14:30, Maciej W. Rozycki: > Changes from v1: > > - preinitialise `dev' in `pirq_find_router' for `for_each_pci_dev', > > - avoid calling `pirq_try_router' with null `dev'. > --- > arch/x86/pci/irq.c | 64 ++++++++++++++++++++++++++++++++++++----------------- > 1 file changed, 44 insertions(+), 20 deletions(-) > > linux-x86-pirq-router-nodev.diff Success! Here is new log: https://pastebin.com/QXaUsCV4 and -- # 8259A.pl irq 0: 00, edge irq 1: 00, edge irq 2: 00, edge irq 3: 00, edge irq 4: 00, edge irq 5: 00, edge irq 6: 00, edge irq 7: 00, edge irq 8: 02, edge irq 9: 02, level irq 10: 02, edge irq 11: 02, edge irq 12: 02, edge irq 13: 02, edge irq 14: 02, edge irq 15: 02, edge Some notes: - please ignore the backtrace from 8139too, it is expected and will be worked on later; - the message line " -> edge" looks somewhat strange, my guess is it might be a small unintentional leftover of some bigger and more detailed message/debugging, anyway, I'd humbly suppose "Triggering mode adjusted" would look better. Thank you! Regards, Nikolai > Index: linux-macro-ide-tty/arch/x86/pci/irq.c > =================================================================== > --- linux-macro-ide-tty.orig/arch/x86/pci/irq.c > +++ linux-macro-ide-tty/arch/x86/pci/irq.c > @@ -908,10 +908,32 @@ static struct pci_dev *pirq_router_dev; > * chipset" ? > */ > > +static bool __init pirq_try_router(struct irq_router *r, > + struct irq_routing_table *rt, > + struct pci_dev *dev) > +{ > + struct irq_router_handler *h; > + > + DBG(KERN_DEBUG "PCI: Trying IRQ router for [%04x:%04x]\n", > + dev->vendor, dev->device); > + > + for (h = pirq_routers; h->vendor; h++) { > + /* First look for a router match */ > + if (rt->rtr_vendor == h->vendor&& > + h->probe(r, dev, rt->rtr_device)) > + return true; > + /* Fall back to a device match */ > + if (dev->vendor == h->vendor&& > + h->probe(r, dev, dev->device)) > + return true; > + } > + return false; > +} > + > static void __init pirq_find_router(struct irq_router *r) > { > struct irq_routing_table *rt = pirq_table; > - struct irq_router_handler *h; > + struct pci_dev *dev; > > #ifdef CONFIG_PCI_BIOS > if (!rt->signature) { > @@ -930,27 +952,29 @@ static void __init pirq_find_router(stru > DBG(KERN_DEBUG "PCI: Attempting to find IRQ router for [%04x:%04x]\n", > rt->rtr_vendor, rt->rtr_device); > > - pirq_router_dev = pci_get_domain_bus_and_slot(0, rt->rtr_bus, > - rt->rtr_devfn); > - if (!pirq_router_dev) { > - DBG(KERN_DEBUG "PCI: Interrupt router not found at " > - "%02x:%02x\n", rt->rtr_bus, rt->rtr_devfn); > - return; > + /* Use any vendor:device provided by the routing table or try all. */ > + if (rt->rtr_vendor) { > + dev = pci_get_domain_bus_and_slot(0, rt->rtr_bus, > + rt->rtr_devfn); > + if (dev&& pirq_try_router(r, rt, dev)) > + pirq_router_dev = dev; > + } else { > + dev = NULL; > + for_each_pci_dev(dev) { > + if (pirq_try_router(r, rt, dev)) { > + pirq_router_dev = dev; > + break; > + } > + } > } > > - for (h = pirq_routers; h->vendor; h++) { > - /* First look for a router match */ > - if (rt->rtr_vendor == h->vendor&& > - h->probe(r, pirq_router_dev, rt->rtr_device)) > - break; > - /* Fall back to a device match */ > - if (pirq_router_dev->vendor == h->vendor&& > - h->probe(r, pirq_router_dev, pirq_router_dev->device)) > - break; > - } > - dev_info(&pirq_router_dev->dev, "%s IRQ router [%04x:%04x]\n", > - pirq_router.name, > - pirq_router_dev->vendor, pirq_router_dev->device); > + if (pirq_router_dev) > + dev_info(&pirq_router_dev->dev, "%s IRQ router [%04x:%04x]\n", > + pirq_router.name, > + pirq_router_dev->vendor, pirq_router_dev->device); > + else > + DBG(KERN_DEBUG "PCI: Interrupt router not found at " > + "%02x:%02x\n", rt->rtr_bus, rt->rtr_devfn); > > /* The device remains referenced for the kernel lifetime */ > } >
Hi Nikolai, > > Changes from v1: > > > > - preinitialise `dev' in `pirq_find_router' for `for_each_pci_dev', > > > > - avoid calling `pirq_try_router' with null `dev'. > > --- > > arch/x86/pci/irq.c | 64 > > ++++++++++++++++++++++++++++++++++++----------------- > > 1 file changed, 44 insertions(+), 20 deletions(-) > > > > linux-x86-pirq-router-nodev.diff > > Success! > Here is new log: > > https://pastebin.com/QXaUsCV4 This is great news, thank you for doing this verification! I have since discovered and posted a fix for an issue with our PIRQ routing code that turned out incapable of routing interrupts behind PCI-to-PCI bridges, which were sometimes used even on classic PCI option cards (I think I have at least two such devices) either to bundle multiple devices or to meet PCI bus load limits. Such devices may work regardless if the BIOS has been developed enough to handle them and assign an IRQ number, but surely they make the likelihood of interrupt sharing rise considerably, so I think the more of them we can handle ourselves the better. Plus these days now that PCIe has come into picture you can get quite complex topologies with multiple logical PCI-to-PCI bridges via external expansion even with classic PCI systems. So I have also posted a PIRQ router implementation for the Intel SIO southbridge and I have also identified further two Intel devices, the IB ISA bridge and the PCEB/ESC EISA bridge combo, that have their PIRQ routers documented in my resources but not handled in Linux. I'll see if I can find some time the following days to get them implemented too just for the sake of people experimenting with odd hardware. Have you tried contacting Nvidia about your ALI chipset? Back in the day I tried to avoid undocumented stuff and Intel was reasonably open about most of their chipsets. Maciej
08.07.2021 23:45, Maciej W. Rozycki: > Have you tried contacting Nvidia about your ALI chipset? Back in the day > I tried to avoid undocumented stuff and Intel was reasonably open about > most of their chipsets. Well, being neither their customer nor a kernel developer, I'm not sure my request would be considered serious. Anyway, probably I'll give it a try a bit later when I have an opportunity to dismount this board for more comfortable testing. I was also going to try to modify its BIOS to remove some unwanted behaviour unrelated to IRQs, and it might happen that I also discover something about PCI handling along with (It is just 64k size, after all, and I have a 8086 debugger) Thank you, Regards, Nikolai > Maciej >
On Fri, 9 Jul 2021, Nikolai Zhubr wrote: > > Have you tried contacting Nvidia about your ALI chipset? Back in the day > > I tried to avoid undocumented stuff and Intel was reasonably open about > > most of their chipsets. > > Well, being neither their customer nor a kernel developer, I'm not sure my > request would be considered serious. Anyway, probably I'll give it a try a bit > later when I have an opportunity to dismount this board for more comfortable > testing. It never hurts asking. At worst you'll be ignored, and at the second worst they'll say nay. They may have lost it too. > I was also going to try to modify its BIOS to remove some unwanted > behaviour unrelated to IRQs, and it might happen that I also discover > something about PCI handling along with (It is just 64k size, after all, and I > have a 8086 debugger) Umm, the board may be old enough not to play any tricks with the BIOS, but mind that at reset x86 starts in the flat mode from 0xfffffff0 and the contents of ROM there may not be what you see at 0xf000:0xfff0 later on. I once worked on a project where I had an opportunity to access the BIOS at the reset vector (and poke at CPU registers, run, stop, single-step it, place hardware breakpoints, etc.) using GDB over JTAG with an Intel Atom board. It was an interesting experience, but sadly most x86 hardware does not have the capability let alone a JTAG connector (called XDP or eXtended Debug Port in Intel-speak) to attach a probe to. You may try disassembling the PCI BIOS 2.1 service however. Maciej
Index: linux-macro-ide-tty/arch/x86/pci/irq.c =================================================================== --- linux-macro-ide-tty.orig/arch/x86/pci/irq.c +++ linux-macro-ide-tty/arch/x86/pci/irq.c @@ -908,10 +908,32 @@ static struct pci_dev *pirq_router_dev; * chipset" ? */ +static bool __init pirq_try_router(struct irq_router *r, + struct irq_routing_table *rt, + struct pci_dev *dev) +{ + struct irq_router_handler *h; + + DBG(KERN_DEBUG "PCI: Trying IRQ router for [%04x:%04x]\n", + dev->vendor, dev->device); + + for (h = pirq_routers; h->vendor; h++) { + /* First look for a router match */ + if (rt->rtr_vendor == h->vendor && + h->probe(r, dev, rt->rtr_device)) + return true; + /* Fall back to a device match */ + if (dev->vendor == h->vendor && + h->probe(r, dev, dev->device)) + return true; + } + return false; +} + static void __init pirq_find_router(struct irq_router *r) { struct irq_routing_table *rt = pirq_table; - struct irq_router_handler *h; + struct pci_dev *dev; #ifdef CONFIG_PCI_BIOS if (!rt->signature) { @@ -930,27 +952,29 @@ static void __init pirq_find_router(stru DBG(KERN_DEBUG "PCI: Attempting to find IRQ router for [%04x:%04x]\n", rt->rtr_vendor, rt->rtr_device); - pirq_router_dev = pci_get_domain_bus_and_slot(0, rt->rtr_bus, - rt->rtr_devfn); - if (!pirq_router_dev) { - DBG(KERN_DEBUG "PCI: Interrupt router not found at " - "%02x:%02x\n", rt->rtr_bus, rt->rtr_devfn); - return; + /* Use any vendor:device provided by the routing table or try all. */ + if (rt->rtr_vendor) { + dev = pci_get_domain_bus_and_slot(0, rt->rtr_bus, + rt->rtr_devfn); + if (dev && pirq_try_router(r, rt, dev)) + pirq_router_dev = dev; + } else { + dev = NULL; + for_each_pci_dev(dev) { + if (pirq_try_router(r, rt, dev)) { + pirq_router_dev = dev; + break; + } + } } - for (h = pirq_routers; h->vendor; h++) { - /* First look for a router match */ - if (rt->rtr_vendor == h->vendor && - h->probe(r, pirq_router_dev, rt->rtr_device)) - break; - /* Fall back to a device match */ - if (pirq_router_dev->vendor == h->vendor && - h->probe(r, pirq_router_dev, pirq_router_dev->device)) - break; - } - dev_info(&pirq_router_dev->dev, "%s IRQ router [%04x:%04x]\n", - pirq_router.name, - pirq_router_dev->vendor, pirq_router_dev->device); + if (pirq_router_dev) + dev_info(&pirq_router_dev->dev, "%s IRQ router [%04x:%04x]\n", + pirq_router.name, + pirq_router_dev->vendor, pirq_router_dev->device); + else + DBG(KERN_DEBUG "PCI: Interrupt router not found at " + "%02x:%02x\n", rt->rtr_bus, rt->rtr_devfn); /* The device remains referenced for the kernel lifetime */ }
PIRQ routing tables provided by the PCI BIOS usually specify the PCI vendor:device ID as well as the bus address of the device implementing the PIRQ router, e.g.: PCI: Interrupt Routing Table found at 0xc00fde10 [...] PCI: Attempting to find IRQ router for [8086:7000] pci 0000:00:07.0: PIIX/ICH IRQ router [8086:7000] however in some cases they do not, in which case we fail to match the router handler, e.g.: PCI: Interrupt Routing Table found at 0xc00fdae0 [...] PCI: Attempting to find IRQ router for [0000:0000] PCI: Interrupt router not found at 00:00 This is because we always match the vendor:device ID and the bus address literally, even if they are all zeros. Handle this case then and iterate over all PCI devices until we find a matching router handler if the vendor ID given by the routing table is the invalid value of zero. Signed-off-by: Maciej W. Rozycki <macro@orcam.me.uk> --- Changes from v1: - preinitialise `dev' in `pirq_find_router' for `for_each_pci_dev', - avoid calling `pirq_try_router' with null `dev'. --- arch/x86/pci/irq.c | 64 ++++++++++++++++++++++++++++++++++++----------------- 1 file changed, 44 insertions(+), 20 deletions(-) linux-x86-pirq-router-nodev.diff