OpenBSD uses irq routing from mptable, but doesn't create it correctly for PCI bus. This patch adds PCI routing info into mptable.
Signed-off-by: Gleb Natapov gleb@redhat.com diff --git a/src/mptable.c b/src/mptable.c index 1920dfe..545a41b 100644 --- a/src/mptable.c +++ b/src/mptable.c @@ -9,6 +9,8 @@ #include "config.h" // CONFIG_* #include "mptable.h" // MPTABLE_SIGNATURE #include "paravirt.h" // qemu_cfg_irq0_override +#include "pci.h" +#include "pci_regs.h"
void mptable_init(void) @@ -21,9 +23,9 @@ mptable_init(void) // Allocate memory int length = (sizeof(struct mptable_config_s) + sizeof(struct mpt_cpu) * MaxCountCPUs - + sizeof(struct mpt_bus) + + sizeof(struct mpt_bus) * 2 + sizeof(struct mpt_ioapic) - + sizeof(struct mpt_intsrc) * 18); + + sizeof(struct mpt_intsrc) * 34); struct mptable_config_s *config = malloc_fseg(length); struct mptable_floating_s *floating = malloc_fseg(sizeof(*floating)); if (!config || !floating) { @@ -85,9 +87,17 @@ mptable_init(void) struct mpt_bus *bus = (void*)cpu; memset(bus, 0, sizeof(*bus)); bus->type = MPT_TYPE_BUS; + bus->busid = 1; memcpy(bus->bustype, "ISA ", sizeof(bus->bustype)); entrycount++;
+ bus++; + memset(bus, 0, sizeof(*bus)); + bus->type = MPT_TYPE_BUS; + bus->busid = 0; + memcpy(bus->bustype, "PCI ", sizeof(bus->bustype)); + entrycount++; + /* ioapic */ u8 ioapic_id = CountCPUs; struct mpt_ioapic *ioapic = (void*)&bus[1]; @@ -101,9 +111,33 @@ mptable_init(void)
/* irqs */ struct mpt_intsrc *intsrcs = (void*)&ioapic[1], *intsrc = intsrcs; + int bdf, max; + unsigned short mask = 0; + foreachpci(bdf, max) { + int pin = pci_config_readb(bdf, PCI_INTERRUPT_PIN); + int irq = pci_config_readb(bdf, PCI_INTERRUPT_LINE); + if (pin == 0) + continue; + mask |= (1 << irq); + memset(intsrc, 0, sizeof(*intsrc)); + intsrc->type = MPT_TYPE_INTSRC; + intsrc->irqtype = 0; /* INT */ + intsrc->irqflag = 1; /* active high */ + intsrc->srcbus = 0; /* PCI bus */ + intsrc->srcbusirq = (pci_bdf_to_dev(bdf) << 2) | (pin - 1); + intsrc->dstapic = ioapic_id; + intsrc->dstirq = irq; + intsrc++; + } + for (i = 0; i < 16; i++) { memset(intsrc, 0, sizeof(*intsrc)); + if (mask & (1 << i)) + continue; intsrc->type = MPT_TYPE_INTSRC; + intsrc->irqtype = 0; /* INT */ + intsrc->irqflag = 0; /* conform to bus spec */ + intsrc->srcbus = 1; /* ISA bus */ intsrc->srcbusirq = i; intsrc->dstapic = ioapic_id; intsrc->dstirq = i; @@ -123,7 +157,7 @@ mptable_init(void) intsrc->type = MPT_TYPE_LOCAL_INT; intsrc->irqtype = 3; /* ExtINT */ intsrc->irqflag = 0; /* PO, EL default */ - intsrc->srcbus = 0; + intsrc->srcbus = 1; /* ISA */ intsrc->srcbusirq = 0; intsrc->dstapic = 0; /* BSP == APIC #0 */ intsrc->dstirq = 0; /* LINTIN0 */ @@ -133,7 +167,7 @@ mptable_init(void) intsrc->type = MPT_TYPE_LOCAL_INT; intsrc->irqtype = 1; /* NMI */ intsrc->irqflag = 0; /* PO, EL default */ - intsrc->srcbus = 0; + intsrc->srcbus = 1; /* ISA */ intsrc->srcbusirq = 0; intsrc->dstapic = 0; /* BSP == APIC #0 */ intsrc->dstirq = 1; /* LINTIN1 */ -- Gleb.
Thanks Gleb.
On Wed, Dec 23, 2009 at 05:29:25PM +0200, Gleb Natapov wrote:
+ sizeof(struct mpt_intsrc) * 18);
+ sizeof(struct mpt_intsrc) * 34);
[...]
- foreachpci(bdf, max) {
int pin = pci_config_readb(bdf, PCI_INTERRUPT_PIN);int irq = pci_config_readb(bdf, PCI_INTERRUPT_LINE);if (pin == 0)continue;mask |= (1 << irq);memset(intsrc, 0, sizeof(*intsrc));intsrc->type = MPT_TYPE_INTSRC;intsrc->irqtype = 0; /* INT */intsrc->irqflag = 1; /* active high */intsrc->srcbus = 0; /* PCI bus */intsrc->srcbusirq = (pci_bdf_to_dev(bdf) << 2) | (pin - 1);intsrc->dstapic = ioapic_id;intsrc->dstirq = irq;intsrc++;- }
Why only increase the allocated storage for intsrc by 16? The loop above seems like it could add a large number of entries.
Also, the foreachpci() macro will iterate over every PCI function - there could be 8 functions to a pci device which could lead to duplicates in the table (two pci functions on the same device could use the same irq pin).
-Kevin
On Wed, Dec 23, 2009 at 01:11:43PM -0500, Kevin O'Connor wrote:
Thanks Gleb.
On Wed, Dec 23, 2009 at 05:29:25PM +0200, Gleb Natapov wrote:
+ sizeof(struct mpt_intsrc) * 18);
+ sizeof(struct mpt_intsrc) * 34);[...]
- foreachpci(bdf, max) {
int pin = pci_config_readb(bdf, PCI_INTERRUPT_PIN);int irq = pci_config_readb(bdf, PCI_INTERRUPT_LINE);if (pin == 0)continue;mask |= (1 << irq);memset(intsrc, 0, sizeof(*intsrc));intsrc->type = MPT_TYPE_INTSRC;intsrc->irqtype = 0; /* INT */intsrc->irqflag = 1; /* active high */intsrc->srcbus = 0; /* PCI bus */intsrc->srcbusirq = (pci_bdf_to_dev(bdf) << 2) | (pin - 1);intsrc->dstapic = ioapic_id;intsrc->dstirq = irq;intsrc++;- }
Why only increase the allocated storage for intsrc by 16? The loop above seems like it could add a large number of entries.
Unfortunately we have to allocate all memory in advance. The table can have max 4 entries per device (one entry for each pin), so 16 is enough for 4 devices.
Also, the foreachpci() macro will iterate over every PCI function - there could be 8 functions to a pci device which could lead to duplicates in the table (two pci functions on the same device could use the same irq pin).
Yes, correct. Need to rework this loop.
-- Gleb.
On Wed, Dec 23, 2009 at 08:45:07PM +0200, Gleb Natapov wrote:
On Wed, Dec 23, 2009 at 01:11:43PM -0500, Kevin O'Connor wrote:
Why only increase the allocated storage for intsrc by 16? The loop above seems like it could add a large number of entries.
Unfortunately we have to allocate all memory in advance. The table can have max 4 entries per device (one entry for each pin), so 16 is enough for 4 devices.
Yeah - I suppose the table could be built in temp space and then copied into the f-segment when the actual size is known.
Also, the foreachpci() macro will iterate over every PCI function - there could be 8 functions to a pci device which could lead to duplicates in the table (two pci functions on the same device could use the same irq pin).
Yes, correct. Need to rework this loop.
Sounds like it should find every function 0 pci entry and then loop over the four possible pins?
-Kevin
On Wed, Dec 23, 2009 at 02:48:24PM -0500, Kevin O'Connor wrote:
On Wed, Dec 23, 2009 at 08:45:07PM +0200, Gleb Natapov wrote:
On Wed, Dec 23, 2009 at 01:11:43PM -0500, Kevin O'Connor wrote:
Why only increase the allocated storage for intsrc by 16? The loop above seems like it could add a large number of entries.
Unfortunately we have to allocate all memory in advance. The table can have max 4 entries per device (one entry for each pin), so 16 is enough for 4 devices.
Yeah - I suppose the table could be built in temp space and then copied into the f-segment when the actual size is known.
Yes, and this is not the only table that needs it. Lets assume 16 entries for now for simplicity.
Also, the foreachpci() macro will iterate over every PCI function - there could be 8 functions to a pci device which could lead to duplicates in the table (two pci functions on the same device could use the same irq pin).
Yes, correct. Need to rework this loop.
Sounds like it should find every function 0 pci entry and then loop over the four possible pins?
Each function connected only to one pin. It needs to loop over all function on a device and record pins that it creates entry for. If the same pin is used by more then one function of a device the record will be checked and second entry will not be created.
-- Gleb.
On Wed, Dec 23, 2009 at 09:56:23PM +0200, Gleb Natapov wrote:
On Wed, Dec 23, 2009 at 02:48:24PM -0500, Kevin O'Connor wrote:
On Wed, Dec 23, 2009 at 08:45:07PM +0200, Gleb Natapov wrote:
On Wed, Dec 23, 2009 at 01:11:43PM -0500, Kevin O'Connor wrote:
Why only increase the allocated storage for intsrc by 16? The loop above seems like it could add a large number of entries.
Unfortunately we have to allocate all memory in advance. The table can have max 4 entries per device (one entry for each pin), so 16 is enough for 4 devices.
Yeah - I suppose the table could be built in temp space and then copied into the f-segment when the actual size is known.
Yes, and this is not the only table that needs it. Lets assume 16 entries for now for simplicity.
The mptable is the only variable length table stored in the f-segment. I can add the table copying though.
BTW, this is only intended for PCI bus zero entries, right? (In theory, one could map a card with a bridge into kvm..)
Also, the foreachpci() macro will iterate over every PCI function - there could be 8 functions to a pci device which could lead to duplicates in the table (two pci functions on the same device could use the same irq pin).
Yes, correct. Need to rework this loop.
Sounds like it should find every function 0 pci entry and then loop over the four possible pins?
Each function connected only to one pin. It needs to loop over all function on a device and record pins that it creates entry for. If the same pin is used by more then one function of a device the record will be checked and second entry will not be created.
I believe the intent of the irq routing in the mptable is to tell the OS what the pci pins are routed to so that it can change the irq to a different pin. If only one pin is described, the OS wont be able to switch to a different pin.
-Kevin
On Wed, Dec 23, 2009 at 03:26:55PM -0500, Kevin O'Connor wrote:
On Wed, Dec 23, 2009 at 09:56:23PM +0200, Gleb Natapov wrote:
On Wed, Dec 23, 2009 at 02:48:24PM -0500, Kevin O'Connor wrote:
On Wed, Dec 23, 2009 at 08:45:07PM +0200, Gleb Natapov wrote:
On Wed, Dec 23, 2009 at 01:11:43PM -0500, Kevin O'Connor wrote:
Why only increase the allocated storage for intsrc by 16? The loop above seems like it could add a large number of entries.
Unfortunately we have to allocate all memory in advance. The table can have max 4 entries per device (one entry for each pin), so 16 is enough for 4 devices.
Yeah - I suppose the table could be built in temp space and then copied into the f-segment when the actual size is known.
Yes, and this is not the only table that needs it. Lets assume 16 entries for now for simplicity.
The mptable is the only variable length table stored in the f-segment. I can add the table copying though.
Ah, OK.
BTW, this is only intended for PCI bus zero entries, right? (In theory, one could map a card with a bridge into kvm..)
Mptable should have description for each PCI bus. Devices behind PCI bridge are still on the same bus.
Also, the foreachpci() macro will iterate over every PCI function - there could be 8 functions to a pci device which could lead to duplicates in the table (two pci functions on the same device could use the same irq pin).
Yes, correct. Need to rework this loop.
Sounds like it should find every function 0 pci entry and then loop over the four possible pins?
Each function connected only to one pin. It needs to loop over all function on a device and record pins that it creates entry for. If the same pin is used by more then one function of a device the record will be checked and second entry will not be created.
I believe the intent of the irq routing in the mptable is to tell the OS what the pci pins are routed to so that it can change the irq to a different pin. If only one pin is described, the OS wont be able to switch to a different pin.
Each function hardwired to one pin only. It is impossible to change the pin. It is possible to route it to a different gsi though. Mptable maps pins of each device to gsis.
-- Gleb.
On Wed, Dec 23, 2009 at 10:35:55PM +0200, Gleb Natapov wrote:
On Wed, Dec 23, 2009 at 03:26:55PM -0500, Kevin O'Connor wrote:
BTW, this is only intended for PCI bus zero entries, right? (In theory, one could map a card with a bridge into kvm..)
Mptable should have description for each PCI bus. Devices behind PCI bridge are still on the same bus.
I believe mptable should describe each root pci bus. However, foreachpci() can iterate over both root pci buses and secondary buses - so I think the code should check for (pci_bdf_to_bus(bdf) > 0) and break from the loop.
Though, admittedly, the code in pciinit.c doesn't handle secondary buses anyway.
I believe the intent of the irq routing in the mptable is to tell the OS what the pci pins are routed to so that it can change the irq to a different pin. If only one pin is described, the OS wont be able to switch to a different pin.
Each function hardwired to one pin only. It is impossible to change the pin. It is possible to route it to a different gsi though. Mptable maps pins of each device to gsis.
Oops - you are correct.
-Kevin
-
Gleb Natapov -
Kevin O'Connor