Patchwork [1/2] Route PC irqs to ISA bus instead of i8259 directly

login
register
mail settings
Submitter Avi Kivity
Date Aug. 9, 2009, 4:44 p.m.
Message ID <1249836296-13288-2-git-send-email-avi@redhat.com>
Download mbox | patch
Permalink /patch/31027/
State Superseded
Headers show

Comments

Avi Kivity - Aug. 9, 2009, 4:44 p.m.
A PC has its motherboard IRQ lines connected to both the PIC and IOAPIC.
Currently, qemu routes IRQs to the PIC which then calls the IOAPIC, an
incestuous arrangement.  In order to clean this up, create a new ISA IRQ
abstraction, and have devices raise ISA IRQs (which in turn raise the i8259
IRQs as usual).

Signed-off-by: Avi Kivity <avi@redhat.com>
---
 hw/pc.c |   44 ++++++++++++++++++++++++++++++--------------
 1 files changed, 30 insertions(+), 14 deletions(-)
Gerd Hoffmann - Aug. 10, 2009, 8:34 a.m.
On 08/09/09 18:44, Avi Kivity wrote:
> A PC has its motherboard IRQ lines connected to both the PIC and IOAPIC.
> Currently, qemu routes IRQs to the PIC which then calls the IOAPIC, an
> incestuous arrangement.  In order to clean this up, create a new ISA IRQ
> abstraction, and have devices raise ISA IRQs (which in turn raise the i8259
> IRQs as usual).

There are isa-bus qdev patches in anthonys patch queue.  I think it 
makes sense to do this cleanup on top of the isa-bus patches.  Maybe it 
makes sense to integrate it more, i.e. have the irqs as element in 
IsaBus ...

cheers,
   Gerd
Avi Kivity - Aug. 10, 2009, 8:46 a.m.
On 08/10/2009 11:34 AM, Gerd Hoffmann wrote:
> On 08/09/09 18:44, Avi Kivity wrote:
>> A PC has its motherboard IRQ lines connected to both the PIC and IOAPIC.
>> Currently, qemu routes IRQs to the PIC which then calls the IOAPIC, an
>> incestuous arrangement.  In order to clean this up, create a new ISA IRQ
>> abstraction, and have devices raise ISA IRQs (which in turn raise the 
>> i8259
>> IRQs as usual).
>
> There are isa-bus qdev patches in anthonys patch queue.  I think it 
> makes sense to do this cleanup on top of the isa-bus patches.  Maybe 
> it makes sense to integrate it more, i.e. have the irqs as element in 
> IsaBus ...
>

One thing at a time.
Alexander Graf - Aug. 10, 2009, 9:04 a.m.
Am 09.08.2009 um 18:44 schrieb Avi Kivity <avi@redhat.com>:

> A PC has its motherboard IRQ lines connected to both the PIC and  
> IOAPIC.
> Currently, qemu routes IRQs to the PIC which then calls the IOAPIC, an
> incestuous arrangement.  In order to clean this up, create a new ISA  
> IRQ
> abstraction, and have devices raise ISA IRQs (which in turn raise  
> the i8259
> IRQs as usual).

Is this really true? From my understanding the PIC in modern systems  
is emulated through the IOAPIC, which is the reason we have legacy  
interrupts.

Copying the boot interrupt experts...

Alex
Avi Kivity - Aug. 10, 2009, 9:43 a.m.
On 08/10/2009 12:04 PM, Alexander Graf wrote:
>
> Am 09.08.2009 um 18:44 schrieb Avi Kivity <avi@redhat.com>:
>
>> A PC has its motherboard IRQ lines connected to both the PIC and IOAPIC.
>> Currently, qemu routes IRQs to the PIC which then calls the IOAPIC, an
>> incestuous arrangement.  In order to clean this up, create a new ISA IRQ
>> abstraction, and have devices raise ISA IRQs (which in turn raise the 
>> i8259
>> IRQs as usual).
>
> Is this really true? From my understanding the PIC in modern systems 
> is emulated through the IOAPIC, which is the reason we have legacy 
> interrupts.
>

For the PC emulated by qemu, it is certainly true.  I don't know for 
sure about modern PCs, but I bet they do respond to the PIC io ports.
Stefan Assmann - Aug. 10, 2009, 9:45 a.m.
On 10.08.2009 11:04, Alexander Graf wrote:
>
> Am 09.08.2009 um 18:44 schrieb Avi Kivity <avi@redhat.com>:
>
>> A PC has its motherboard IRQ lines connected to both the PIC and IOAPIC.
>> Currently, qemu routes IRQs to the PIC which then calls the IOAPIC, an
>> incestuous arrangement. In order to clean this up, create a new ISA IRQ
>> abstraction, and have devices raise ISA IRQs (which in turn raise the
>> i8259
>> IRQs as usual).
>
> Is this really true? From my understanding the PIC in modern systems is
> emulated through the IOAPIC, which is the reason we have legacy interrupts.

While not sure how the hardware implementation is done in detail I can
confirm that the IRQs indeed end up at both PIC and IO-APIC0 if the
device is connected to the southbridge directly. If that's not the case
for example a PCI bus connected via PCIe that sports it's own IO-APIC
then IRQs are forwarded (over PCIe) from the IO-APIC to the southbridge
(PIC).

In any case, to come closer to the real hardware having an abstraction
that receives IRQs from devices and delivers them to the appropriate
interrupt controller(s) seems to be a valid step IMHO.

Does qemu support multiple IO-APICs? I guess not so no need for boot
interrupts. (If yes then there would be the question how close you
really want to be to existing hardware.)

   Stefan
--
Stefan Assmann         | Red Hat GmbH
Software Engineer      | Otto-Hahn-Strasse 20, 85609 Dornach
                        | HR: Amtsgericht Muenchen HRB 153243
                        | GF: Brendan Lane, Charlie Peters,
sassmann at redhat.com |     Michael Cunningham, Charles Cachera
Avi Kivity - Aug. 10, 2009, 9:52 a.m.
On 08/10/2009 12:45 PM, Stefan Assmann wrote:
> Does qemu support multiple IO-APICs? I guess not so no need for boot
> interrupts. (If yes then there would be the question how close you
> really want to be to existing hardware.)


Not at present.  We've considered adding IOAPICs to reduce interrupt 
sharing, but MSI solves the problem much more neatly.
Olaf Dabrunz - Aug. 10, 2009, 1:14 p.m.
On 10-Aug-09, Stefan Assmann wrote:
> On 10.08.2009 11:04, Alexander Graf wrote:
>>
>> Am 09.08.2009 um 18:44 schrieb Avi Kivity <avi@redhat.com>:
>>
>>> A PC has its motherboard IRQ lines connected to both the PIC and IOAPIC.
>>> Currently, qemu routes IRQs to the PIC which then calls the IOAPIC, an
>>> incestuous arrangement. In order to clean this up, create a new ISA IRQ
>>> abstraction, and have devices raise ISA IRQs (which in turn raise the
>>> i8259
>>> IRQs as usual).
>>
>> Is this really true? From my understanding the PIC in modern systems is
>> emulated through the IOAPIC, which is the reason we have legacy interrupts.
>
> While not sure how the hardware implementation is done in detail I can
> confirm that the IRQs indeed end up at both PIC and IO-APIC0 if the
> device is connected to the southbridge directly. If that's not the case
> for example a PCI bus connected via PCIe that sports it's own IO-APIC
> then IRQs are forwarded (over PCIe) from the IO-APIC to the southbridge
> (PIC).

To be exact, IRQs on modern PCs are routed to both an IO-APIC and the
PIC via wires and some routing mechanism. (Support for the PIC is
required by ACPI.) The routing mechanism to the PIC does _not_ involve
an IO-APIC. It is implementation-specific to the chipset and the board,
and operating systems get routing info from the ACPI tables (delivered
by the BIOS). The routing to the PIC is set up by the BIOS, which knows
how to program the chipset-specific routing registers.

The IO-APICs are standard components that are programmed by the
operating system. They do not have any "forwarding to the PIC"
component. The intention is that there are no implementation-specific
differences between them (except for number of lines, the generation of
the IO-APIC spec and slight differences in the hardware implementation).
They are supposed to be programmable by the OS in a standard way. That
is why they cannot be part of a scheme that routes IRQs to the PIC.

Now that being said, there are mechanisms like boot interrupts that on
some chipsets activate forwarding of interrupts to the PIC, when the
corresponding "line" in a (secondary) IO-APIC is disabled. This means
that the IO-APIC state is "sampled" by some logic to select forwarding
to the PIC. The IO-APIC itself is not part of this mechanism, it is just
"being watched".

> In any case, to come closer to the real hardware having an abstraction
> that receives IRQs from devices and delivers them to the appropriate
> interrupt controller(s) seems to be a valid step IMHO.
>
> Does qemu support multiple IO-APICs? I guess not so no need for boot
> interrupts. (If yes then there would be the question how close you
> really want to be to existing hardware.)
>
>   Stefan
Olaf Dabrunz - Aug. 10, 2009, 1:20 p.m.
On 10-Aug-09, Avi Kivity wrote:
> On 08/10/2009 12:45 PM, Stefan Assmann wrote:
>> Does qemu support multiple IO-APICs? I guess not so no need for boot
>> interrupts. (If yes then there would be the question how close you
>> really want to be to existing hardware.)
>
>
> Not at present.  We've considered adding IOAPICs to reduce interrupt 
> sharing, but MSI solves the problem much more neatly.

Yes, all modern operating systems that come to my mind support MSIs, so
as long as you do not want to run some old system, this is ok. I also
guess that you do not try to emulate one of the chipsets that has a
broken MSI implementation.

Patch

diff --git a/hw/pc.c b/hw/pc.c
index bc9e646..a6be4a8 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -87,6 +87,17 @@  static void option_rom_setup_reset(target_phys_addr_t addr, unsigned size)
     qemu_register_reset(option_rom_reset, rrd);
 }
 
+typedef struct isa_irq_state {
+    qemu_irq *i8259;
+} IsaIrqState;
+
+static void isa_irq_handler(void *opaque, int n, int level)
+{
+    IsaIrqState *isa = (IsaIrqState *)opaque;
+
+    qemu_set_irq(isa->i8259[n], level);
+}
+
 static void ioport80_write(void *opaque, uint32_t addr, uint32_t data)
 {
 }
@@ -1119,7 +1130,9 @@  static void pc_init1(ram_addr_t ram_size,
     int piix3_devfn = -1;
     CPUState *env;
     qemu_irq *cpu_irq;
+    qemu_irq *isa_irq;
     qemu_irq *i8259;
+    IsaIrqState *isa_irq_state;
     DriveInfo *dinfo;
     BlockDriverState *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
     BlockDriverState *fd[MAX_FD];
@@ -1264,10 +1277,13 @@  static void pc_init1(ram_addr_t ram_size,
 
     cpu_irq = qemu_allocate_irqs(pic_irq_request, NULL, 1);
     i8259 = i8259_init(cpu_irq[0]);
-    ferr_irq = i8259[13];
+    isa_irq_state = qemu_mallocz(sizeof(*isa_irq_state));
+    isa_irq_state->i8259 = i8259;
+    isa_irq = qemu_allocate_irqs(isa_irq_handler, isa_irq_state, 16);
+    ferr_irq = isa_irq[13];
 
     if (pci_enabled) {
-        pci_bus = i440fx_init(&i440fx_state, i8259);
+        pci_bus = i440fx_init(&i440fx_state, isa_irq);
         piix3_devfn = piix3_init(pci_bus, -1);
     } else {
         pci_bus = NULL;
@@ -1297,7 +1313,7 @@  static void pc_init1(ram_addr_t ram_size,
         }
     }
 
-    rtc_state = rtc_init(0x70, i8259[8], 2000);
+    rtc_state = rtc_init(0x70, isa_irq[8], 2000);
 
     qemu_register_boot_set(pc_boot_set, rtc_state);
 
@@ -1307,10 +1323,10 @@  static void pc_init1(ram_addr_t ram_size,
     if (pci_enabled) {
         ioapic = ioapic_init();
     }
-    pit = pit_init(0x40, i8259[0]);
+    pit = pit_init(0x40, isa_irq[0]);
     pcspk_init(pit);
     if (!no_hpet) {
-        hpet_init(i8259);
+        hpet_init(isa_irq);
     }
     if (pci_enabled) {
         pic_set_alt_irq_func(isa_pic, ioapic_set_irq, ioapic);
@@ -1318,14 +1334,14 @@  static void pc_init1(ram_addr_t ram_size,
 
     for(i = 0; i < MAX_SERIAL_PORTS; i++) {
         if (serial_hds[i]) {
-            serial_init(serial_io[i], i8259[serial_irq[i]], 115200,
+            serial_init(serial_io[i], isa_irq[serial_irq[i]], 115200,
                         serial_hds[i]);
         }
     }
 
     for(i = 0; i < MAX_PARALLEL_PORTS; i++) {
         if (parallel_hds[i]) {
-            parallel_init(parallel_io[i], i8259[parallel_irq[i]],
+            parallel_init(parallel_io[i], isa_irq[parallel_irq[i]],
                           parallel_hds[i]);
         }
     }
@@ -1336,7 +1352,7 @@  static void pc_init1(ram_addr_t ram_size,
         NICInfo *nd = &nd_table[i];
 
         if (!pci_enabled || (nd->model && strcmp(nd->model, "ne2k_isa") == 0))
-            pc_init_ne2k_isa(nd, i8259);
+            pc_init_ne2k_isa(nd, isa_irq);
         else
             pci_nic_init(nd, "ne2k_pci", NULL);
     }
@@ -1354,25 +1370,25 @@  static void pc_init1(ram_addr_t ram_size,
     }
 
     if (pci_enabled) {
-        pci_piix3_ide_init(pci_bus, hd, piix3_devfn + 1, i8259);
+        pci_piix3_ide_init(pci_bus, hd, piix3_devfn + 1, isa_irq);
     } else {
         for(i = 0; i < MAX_IDE_BUS; i++) {
-            isa_ide_init(ide_iobase[i], ide_iobase2[i], i8259[ide_irq[i]],
+            isa_ide_init(ide_iobase[i], ide_iobase2[i], isa_irq[ide_irq[i]],
 	                 hd[MAX_IDE_DEVS * i], hd[MAX_IDE_DEVS * i + 1]);
         }
     }
 
-    i8042_init(i8259[1], i8259[12], 0x60);
+    i8042_init(isa_irq[1], isa_irq[12], 0x60);
     DMA_init(0);
 #ifdef HAS_AUDIO
-    audio_init(pci_enabled ? pci_bus : NULL, i8259);
+    audio_init(pci_enabled ? pci_bus : NULL, isa_irq);
 #endif
 
     for(i = 0; i < MAX_FD; i++) {
         dinfo = drive_get(IF_FLOPPY, 0, i);
         fd[i] = dinfo ? dinfo->bdrv : NULL;
     }
-    floppy_controller = fdctrl_init(i8259[6], 2, 0, 0x3f0, fd);
+    floppy_controller = fdctrl_init(isa_irq[6], 2, 0, 0x3f0, fd);
 
     cmos_init(below_4g_mem_size, above_4g_mem_size, boot_device, hd);
 
@@ -1385,7 +1401,7 @@  static void pc_init1(ram_addr_t ram_size,
         i2c_bus *smbus;
 
         /* TODO: Populate SPD eeprom data.  */
-        smbus = piix4_pm_init(pci_bus, piix3_devfn + 3, 0xb100, i8259[9]);
+        smbus = piix4_pm_init(pci_bus, piix3_devfn + 3, 0xb100, isa_irq[9]);
         for (i = 0; i < 8; i++) {
             DeviceState *eeprom;
             eeprom = qdev_create((BusState *)smbus, "smbus-eeprom");