Page MenuHomeFreeBSD

Use bus multipass for interrupt controllers on PowerPC
Needs ReviewPublic

Authored by nwhitehorn on Aug 19 2016, 8:19 PM.

Details

Reviewers
jhibbits
jhb
Summary

This adjusts all PPC interrupt controllers to attach at BUS_PASS_INTERRUPT and all potential parent buses to attach at BUS_PASS_BUS. Where parent devices and interrupt controllers need to set up interrupts on interrupt controllers, delay interrupt setup until after BUS_PASS_INTERRUPT has finished using a bus_new_pass() hook. This will eventually allow us to turn off the hope-for-the-best interrupt preallocation code in powerpc/intr_machdep.c.

Test Plan

Tested only on a G5 powermac so far.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

nwhitehorn retitled this revision from to Use bus multipass for interrupt controllers on PowerPC.
nwhitehorn updated this object.
nwhitehorn edited the test plan for this revision. (Show Details)
nwhitehorn added reviewers: jhibbits, jhb.
nwhitehorn set the repository for this revision to rS FreeBSD src repository - subversion.

Note: This partially implements the changes in D918.

Fix unused variable and make patch relative to src/ rather than src/sys.

nwhitehorn edited edge metadata.

Fix missing file.

nwhitehorn edited edge metadata.

Actually update the diff, since I still forgot that file last time.

In general I think this looks fine. I do think it's probably simpler to just defer all of the interrupt-related bits (including bus_alloc_resource()). You could imagine systems that create IRQ resources in a BUS_PASS_INTERRUPT driver that won't be available to allocate prior to that. I also think it's a bit cleaner to have a separate routine when deferring the IRQ allocation/setup that is called from bus_new_pass. Eventually we may also do something similar with SYS_RES_IOPORT/SYS_RES_MEMORY resources being deferred until after BUS_PASS_RESOURCE. Having bus_new_pass methods that just call various helper routines based on pass number will be clearer in that case I think than moving all the logic into the bus_new_pass callback.

I have a mild preference for committing the MI drivers as a separate change first, but it's also fine as one change since people on non-powerpc can still bisect the entire revision effectively.

sys/dev/pci/pci_pci.c
1350

I would perhaps call this 'pcib_pcie_alloc_irq()' and have pcib_pcie_new_pass() be something like:

static void
pcib_pcie_new_pass(device_t dev)
{

    if (bus_current_pass > BUS_PASS_INTERRUPT + BUS_PASS_ORDER_LAST)
        pcib_pcie_alloc_irq(dev);
    bus_generic_new_pass(dev);
}
1386

Ah, I would actually defer all of pcib_pcie_alloc_irq() (including all the MSI song and dance) in which case you won't need this change or the explicit bus_activate_resource(). (Missed that part of the change in the comment above). That would then let you still have a "simple" bus_new_pass method that just calls pcib_pcie_alloc_irq() if need be.