I was trying to revive the patch to add Qemu INTx routing support for Q35 and stumbled on some rather broken interrupt routing problems. Sadly a seabios release claiming Q35 support snuck out already, but I'd like to at least discuss these before Qemu 1.4 even though they may not get fixed for that release either.
ICH9 adds 4 more PCI interrupt routing registers, so in addition to the previous PIRQ[A:D], we now also have PIRQ[E:H]. AIUI, each pin can operate in either PIC or APIC mode based on whether the IRQEN bit of each register is clear or set respectively. In PIC mode bits 3:0 of the PIRQn register identifiy the ISA compatible interrupt to trigger. In APIC mode each PIRQn is statically mapped to an APIC pin, where PIRQA->16, PIRQB->17, ..., PIRQH->23.
The first problem we encounter is that the system boots into PIC mode but we're programming the PIRQn registers into APIC mode. After fixing that, we quickly notice that seabios has a single function for programming PCI interrupt line registers based on the PIIX mapping of PIRQn registers. A comment in the Qemu source indicates the slot:pin to PIRQn mapping is arbitrary so long as Qemu and ACPI match, disregarding that boot ROMs may also use interrupts and have no ACPI support. Solutions to this are either to create an ICH9 specific mapping function or to adjust slot:pin mappings to match PIIX. I've done the latter here, but it's incomplete as slots 25-31 have directly programmable mappings.
I'm looking for comments on how to proceed. Do we want to try to meld ICH9 to be more PIIX compatible or do we want to do our own thing. I suspect there are still a number of holes in ICH9 irq programming no matter which path we take and these in particular are likely to be challenging for migration compatibility. Thanks,
Alex
---
Alex Williamson (2): q35: Fix PIC-mode interrupt setup q35: Fix ACPI _PRT routing to match PIIX
src/pciinit.c | 6 +-- src/q35-acpi-dsdt.dsl | 100 +++++++++++++++++++++++++------------------------ 2 files changed, 52 insertions(+), 54 deletions(-)
We're initializing the ICH9 PIRQn registers with the IRQEN bit set, which actuall makes them operate in APIC mode rather than PIC mode (see 13.1.17 & 13.1.9). AFAICT, the system boots up in PIC mode and trying to make use of APIC IRQs in boot ROMs does not work. Fix this to use the ISA compatible IRQs.
Signed-off-by: Alex Williamson alex.williamson@redhat.com --- src/pciinit.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/src/pciinit.c b/src/pciinit.c index a406bbd..857e8af 100644 --- a/src/pciinit.c +++ b/src/pciinit.c @@ -143,11 +143,9 @@ void mch_isa_bridge_init(struct pci_device *dev, void *arg) /* activate irq remapping in LPC */
/* PIRQ[A-D] routing */ - pci_config_writeb(bdf, ICH9_LPC_PIRQA_ROUT + i, - irq | ICH9_LPC_PIRQ_ROUT_IRQEN); + pci_config_writeb(bdf, ICH9_LPC_PIRQA_ROUT + i, irq); /* PIRQ[E-H] routing */ - pci_config_writeb(bdf, ICH9_LPC_PIRQE_ROUT + i, - irq | ICH9_LPC_PIRQ_ROUT_IRQEN); + pci_config_writeb(bdf, ICH9_LPC_PIRQE_ROUT + i, irq); } outb(elcr[0], ICH9_LPC_PORT_ELCR1); outb(elcr[1], ICH9_LPC_PORT_ELCR2);
When seabios programs PCI interrupt line registers it expects a specific slot:pin to PIRQ mapping (see pci_slot_get_irq). If we want to re-use this function between both PIIX and ICH9, then the _PRT exposed to the guest needs to change. NB. slots 25-31 on ICH9 don't follow this routing pattern and can be changed independently, so even with this boot ROMs behind express ports may not work correctly with interrupts. We can possibly make them follow this pattern or come up with a new mch_pci_slot_get_irq() function, in which case we don't need to match PIIX.
PIIX slot:pin array - D/A/B/C, A/B/C/D, B/C/D/A, C/D/A/B Current ICH9 - E/F/G/H, F/G/H/E, G/H/E/F, H/E/F/G Revised ICH9 - H/E/F/G, E/F/G/H, F/G/H/E, G/H/E/F
Signed-off-by: Alex Williamson alex.williamson@redhat.com --- src/q35-acpi-dsdt.dsl | 100 +++++++++++++++++++++++++------------------------ 1 file changed, 50 insertions(+), 50 deletions(-)
diff --git a/src/q35-acpi-dsdt.dsl b/src/q35-acpi-dsdt.dsl index c031d83..ebb9349 100644 --- a/src/q35-acpi-dsdt.dsl +++ b/src/q35-acpi-dsdt.dsl @@ -201,31 +201,31 @@ DefinitionBlock ( #define prt_slot_lnkH(nr) prt_slot_lnk(nr, LNKH, LNKE, LNKF, LNKG)
Name(PRTP, package() { - prt_slot_lnkE(0x0000), - prt_slot_lnkF(0x0001), - prt_slot_lnkG(0x0002), - prt_slot_lnkH(0x0003), - prt_slot_lnkE(0x0004), - prt_slot_lnkF(0x0005), - prt_slot_lnkG(0x0006), - prt_slot_lnkH(0x0007), - prt_slot_lnkE(0x0008), - prt_slot_lnkF(0x0009), - prt_slot_lnkG(0x000a), - prt_slot_lnkH(0x000b), - prt_slot_lnkE(0x000c), - prt_slot_lnkF(0x000d), - prt_slot_lnkG(0x000e), - prt_slot_lnkH(0x000f), - prt_slot_lnkE(0x0010), - prt_slot_lnkF(0x0011), - prt_slot_lnkG(0x0012), - prt_slot_lnkH(0x0013), - prt_slot_lnkE(0x0014), - prt_slot_lnkF(0x0015), - prt_slot_lnkG(0x0016), - prt_slot_lnkH(0x0017), - prt_slot_lnkE(0x0018), + prt_slot_lnkH(0x0000), + prt_slot_lnkE(0x0001), + prt_slot_lnkF(0x0002), + prt_slot_lnkG(0x0003), + prt_slot_lnkH(0x0004), + prt_slot_lnkE(0x0005), + prt_slot_lnkF(0x0006), + prt_slot_lnkG(0x0007), + prt_slot_lnkH(0x0008), + prt_slot_lnkE(0x0009), + prt_slot_lnkF(0x000a), + prt_slot_lnkG(0x000b), + prt_slot_lnkH(0x000c), + prt_slot_lnkE(0x000d), + prt_slot_lnkF(0x000e), + prt_slot_lnkG(0x000f), + prt_slot_lnkH(0x0010), + prt_slot_lnkE(0x0011), + prt_slot_lnkF(0x0012), + prt_slot_lnkG(0x0013), + prt_slot_lnkH(0x0014), + prt_slot_lnkE(0x0015), + prt_slot_lnkF(0x0016), + prt_slot_lnkG(0x0017), + prt_slot_lnkH(0x0018),
/* INTA -> PIRQA for slot 25 - 31 see the default value of D<N>IR */ @@ -258,31 +258,31 @@ DefinitionBlock ( #define prt_slot_gsiH(nr) prt_slot_gsi(nr, GSIH, GSIE, GSIF, GSIG)
Name(PRTA, package() { - prt_slot_gsiE(0x0000), - prt_slot_gsiF(0x0001), - prt_slot_gsiG(0x0002), - prt_slot_gsiH(0x0003), - prt_slot_gsiE(0x0004), - prt_slot_gsiF(0x0005), - prt_slot_gsiG(0x0006), - prt_slot_gsiH(0x0007), - prt_slot_gsiE(0x0008), - prt_slot_gsiF(0x0009), - prt_slot_gsiG(0x000a), - prt_slot_gsiH(0x000b), - prt_slot_gsiE(0x000c), - prt_slot_gsiF(0x000d), - prt_slot_gsiG(0x000e), - prt_slot_gsiH(0x000f), - prt_slot_gsiE(0x0010), - prt_slot_gsiF(0x0011), - prt_slot_gsiG(0x0012), - prt_slot_gsiH(0x0013), - prt_slot_gsiE(0x0014), - prt_slot_gsiF(0x0015), - prt_slot_gsiG(0x0016), - prt_slot_gsiH(0x0017), - prt_slot_gsiE(0x0018), + prt_slot_gsiH(0x0000), + prt_slot_gsiE(0x0001), + prt_slot_gsiF(0x0002), + prt_slot_gsiG(0x0003), + prt_slot_gsiH(0x0004), + prt_slot_gsiE(0x0005), + prt_slot_gsiF(0x0006), + prt_slot_gsiG(0x0007), + prt_slot_gsiH(0x0008), + prt_slot_gsiE(0x0009), + prt_slot_gsiF(0x000a), + prt_slot_gsiG(0x000b), + prt_slot_gsiH(0x000c), + prt_slot_gsiE(0x000d), + prt_slot_gsiF(0x000e), + prt_slot_gsiG(0x000f), + prt_slot_gsiH(0x0010), + prt_slot_gsiE(0x0011), + prt_slot_gsiF(0x0012), + prt_slot_gsiG(0x0013), + prt_slot_gsiH(0x0014), + prt_slot_gsiE(0x0015), + prt_slot_gsiF(0x0016), + prt_slot_gsiG(0x0017), + prt_slot_gsiH(0x0018),
/* INTA -> PIRQA for slot 25 - 31, but 30 see the default value of D<N>IR */
It's really not that arbitrary how we translate slot:pin to pirq register. We not only need to match the ACPI table we provide to the guest, but the BIOS has dependencies for programming PCI interrupt line registers for boot ROMs that use interrupts. If we don't match the slot:pin to pirq of PIIX then we need to come up with our own mapping function. NB. We might need our own map function anyway because slots 25-31 do their own thing so BIOS interrupt line mapping will still fail (no interrupt mode boot ROMs below express ports).
Signed-off-by: Alex Williamson alex.williamson@redhat.com --- hw/lpc_ich9.c | 9 ++++----- pc-bios/q35-acpi-dsdt.aml | Bin 2 files changed, 4 insertions(+), 5 deletions(-)
Binary not included. This includes the required Qemu changes to match the slot:pin updates in seabios.
diff --git a/hw/lpc_ich9.c b/hw/lpc_ich9.c index 16843d7..7b9e348 100644 --- a/hw/lpc_ich9.c +++ b/hw/lpc_ich9.c @@ -109,17 +109,16 @@ static void ich9_cc_init(ICH9LPCState *lpc) int intx;
/* the default irq routing is arbitrary as long as it matches with - * acpi irq routing table. - * The one that is incompatible with piix_pci(= bochs) one is - * intentionally chosen to let the users know that the different - * board is used. + * acpi irq routing table and BIOS. * * int[A-D] -> pirq[E-F] * avoid pirq A-D because they are used for pci express port + * Keep the same slot rotation as piix or the bios won't know + * how to program PCI interrupt line registers for boot ROMs. */ for (slot = 0; slot < PCI_SLOT_MAX; slot++) { for (intx = 0; intx < PCI_NUM_PINS; intx++) { - lpc->irr[slot][intx] = (slot + intx) % 4 + 4; + lpc->irr[slot][intx] = ((slot + intx - 1) & 3) + 4; } } ich9_cc_update(lpc);
-
Alex Williamson