msi: remove the check that interrupt sources have been added

[royger]

Summary

When running as a specific type of Xen guest the hypervisor won't
provide any emulated IO-APICs or legacy PICs at all, thus hitting the
following assert in the MSI code:

panic: Assertion num_io_irqs > 0 failed at /usr/src/sys/x86/x86/msi.c:334
cpuid = 0
time = 1
KDB: stack backtrace:
db_trace_self_wrapper() at db_trace_self_wrapper+0x2b/frame 0xffffffff826ffa70
vpanic() at vpanic+0x1a3/frame 0xffffffff826ffad0
panic() at panic+0x43/frame 0xffffffff826ffb30
msi_init() at msi_init+0xed/frame 0xffffffff826ffb40
apic_setup_io() at apic_setup_io+0x72/frame 0xffffffff826ffb50
mi_startup() at mi_startup+0x118/frame 0xffffffff826ffb70
start_kernel() at start_kernel+0x10

Fix this by removing the assert in the MSI code, since it's possible
to get to the MSI initialization without having registered any other
interrupt sources.

Sponsored by: Citrix Systems R&D

[kib]

Would be useful to assert that there is no IO APICs or AT PICs configured when num_io_irqs == 0, but I am not sure that this is straightforward.

[royger]

In D17001#362583, @kib wrote:

Would be useful to assert that there is no IO APICs or AT PICs configured when num_io_irqs == 0, but I am not sure that this is straightforward.

I thought about this too, but I didn't find any straightforward way to check whether there are any PICs in the system. I will wait for John in case he has a suggestion.

[jhb]

In D17001#362597, @royger wrote:

In D17001#362583, @kib wrote:

Would be useful to assert that there is no IO APICs or AT PICs configured when num_io_irqs == 0, but I am not sure that this is straightforward.

I thought about this too, but I didn't find any straightforward way to check whether there are any PICs in the system. I will wait for John in case he has a suggestion.

There isn't a good way to assert that. The ATPIC code always assumes it can use the first 16 IRQs if 'device atpic' is enabled in the kernel which is a bit of a landmine. 'device atpic' isn't in GENERIC, but it might be nice to force it to be disabled for this particular Xen case? Or is there an emulated ATPIC with no useful interrupts? (In a kernel without 'device atpic' we use unconditional logic to init the 8259A's into a silent state.) If Xen is still providing a dummy set of 8259A's, then I think what I'd actually prefer is to patch the SYSINIT in local_apic.c that does 'apic_setup_io' to bump num_io_irqs up to 16 if it is still zero to avoid any possible conflicts with IRQs 0 - 15. Note that one idea I'm considering is removing MINIMUM_MSI_INT post-branch so that MSI IRQs would start at the end of the PIC range rather than at 256 in which case explicitly reserving 0-15 would still matter.

[royger]

In D17001#362843, @jhb wrote:

In D17001#362597, @royger wrote:

In D17001#362583, @kib wrote:

Would be useful to assert that there is no IO APICs or AT PICs configured when num_io_irqs == 0, but I am not sure that this is straightforward.

I thought about this too, but I didn't find any straightforward way to check whether there are any PICs in the system. I will wait for John in case he has a suggestion.

There isn't a good way to assert that. The ATPIC code always assumes it can use the first 16 IRQs if 'device atpic' is enabled in the kernel which is a bit of a landmine. 'device atpic' isn't in GENERIC, but it might be nice to force it to be disabled for this particular Xen case? Or is there an emulated ATPIC with no useful interrupts? (In a kernel without 'device atpic' we use unconditional logic to init the 8259A's into a silent state.) If Xen is still providing a dummy set of 8259A's, then I think what I'd actually prefer is to patch the SYSINIT in local_apic.c that does 'apic_setup_io' to bump num_io_irqs up to 16 if it is still zero to avoid any possible conflicts with IRQs 0 - 15. Note that one idea I'm considering is removing MINIMUM_MSI_INT post-branch so that MSI IRQs would start at the end of the PIC range rather than at 256 in which case explicitly reserving 0-15 would still matter.

In this case there's no IO-APIC or ATPIC at all, just a local APIC. When using pci-passthrough an emulated IO-APIC will be available to route the legacy IRQs.

Doing something like:

num_io_irqs = max(num_io_irqs, 16);

In apic_setup_io seems sensible, but it's not going to make much of a difference until MINIMUM_MSI_INT is removed and MSI IRQs start just after the IO-APIC (or ATPIC) ranges end.

Let me know whether you want me to add this chunk here, or you would rather do it when removing MINIMUM_MSI_INT.

[jhb]

The current patch is fine. We should perhaps neuter the atpic driver in sys/x86/isa/atpic.c for such systems by having it fail to add any PICs or interrupt sources at all.