Skip to contentSkip to conversation

qemu_reserve_isa_irq()

PATCH In review Jes Sorensen 5 participants 19 messages Aug 6, 2009 – Aug 14, 2009

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 qemu_reserve_isa_irq()

Hi

As part of trying to provide the BIOS with information about PCI interrupt availability, I have added qemu_reserve_isa_irq() to try and keep track of which IRQs are claimed for ISA.

If we can agree on this approach, then I'll be using it to pass info on to the guest BIOS in a follow on patch.

Thanks,
Jes

Implement qemu_reserve_isa_irq() and fix the code to call it whenever an ISA IRQ is claimed.

The old QEMU code simply claimed ISA IRQs without keeping track of it in any way. This patch tries to catch the culprits and make them call qemu_reserve_isa_irq() where appropriate.

This allows us to keep track of which IRQs are available for PCI and later pass that on to the BIOS.

Note that on non PC architectures, this should have zero effect.

---
 hw/cs4231a.c |    3 +++
 hw/gus.c     |    3 +++
 hw/ide.c     |   24 ++++++++++++++++--------
 hw/irq.c     |   12 ++++++++++++
 hw/irq.h     |    2 ++
 hw/pc.c      |   38 ++++++++++++++++++++++++++++++--------
 hw/sb16.c    |    3 +++
 7 files changed, 69 insertions(+), 16 deletions(-)
hw/cs4231a.c +3 -0
Index: qemu/hw/cs4231a.c
===================================================================
@@ -641,6 +641,9 @@ int cs4231a_init (qemu_irq *pic)
     int i;
     CSState *s;
 
+    if (qemu_reserve_isa_irq(conf.irq))
+        return -1;
+
     s = qemu_mallocz (sizeof (*s));
 
     s->pic = pic;
hw/gus.c +3 -0
Index: qemu/hw/gus.c
===================================================================
@@ -255,6 +255,9 @@ int GUS_init (qemu_irq *pic)
     GUSState *s;
     struct audsettings as;
 
+    if (qemu_reserve_isa_irq(conf.irq))
+        return -1;
+
     s = qemu_mallocz (sizeof (*s));
 
     AUD_register_card ("gus", &s->card);
hw/ide.c +16 -8
Index: qemu/hw/ide.c
===================================================================
@@ -3405,10 +3405,14 @@ void pci_piix3_ide_init(PCIBus *bus, Blo
     pci_register_bar((PCIDevice *)d, 4, 0x10,
                            PCI_ADDRESS_SPACE_IO, bmdma_map);
 
-    ide_init2(&d->ide_if[0], hd_table[0], hd_table[1], pic[14]);
-    ide_init2(&d->ide_if[2], hd_table[2], hd_table[3], pic[15]);
-    ide_init_ioport(&d->ide_if[0], 0x1f0, 0x3f6);
-    ide_init_ioport(&d->ide_if[2], 0x170, 0x376);
+    if (!qemu_reserve_isa_irq(14)) {
+        ide_init2(&d->ide_if[0], hd_table[0], hd_table[1], pic[14]);
+        ide_init_ioport(&d->ide_if[0], 0x1f0, 0x3f6);
+    }
+    if (!qemu_reserve_isa_irq(15)) {
+        ide_init2(&d->ide_if[2], hd_table[2], hd_table[3], pic[15]);
+        ide_init_ioport(&d->ide_if[2], 0x170, 0x376);
+    }
 
     for (i = 0; i < 4; i++)
         if (hd_table[i])
@@ -3445,10 +3449,14 @@ void pci_piix4_ide_init(PCIBus *bus, Blo
     pci_register_bar((PCIDevice *)d, 4, 0x10,
                            PCI_ADDRESS_SPACE_IO, bmdma_map);
 
-    ide_init2(&d->ide_if[0], hd_table[0], hd_table[1], pic[14]);
-    ide_init2(&d->ide_if[2], hd_table[2], hd_table[3], pic[15]);
-    ide_init_ioport(&d->ide_if[0], 0x1f0, 0x3f6);
-    ide_init_ioport(&d->ide_if[2], 0x170, 0x376);
+    if (!qemu_reserve_isa_irq(14)) {
+        ide_init2(&d->ide_if[0], hd_table[0], hd_table[1], pic[14]);
+        ide_init_ioport(&d->ide_if[0], 0x1f0, 0x3f6);
+    }
+    if (!qemu_reserve_isa_irq(15)) {
+        ide_init2(&d->ide_if[2], hd_table[2], hd_table[3], pic[15]);
+        ide_init_ioport(&d->ide_if[2], 0x170, 0x376);
+    }
 
     register_savevm("ide", 0, 2, pci_ide_save, pci_ide_load, d);
 }
hw/irq.c +12 -0
Index: qemu/hw/irq.c
===================================================================
@@ -30,6 +30,18 @@ struct IRQState {
     int n;
 };
 
+uint16_t isa_reserved_irqs = 0;
+
+int qemu_reserve_isa_irq(int irq)
+{
+    if (isa_reserved_irqs & (1 << irq)) {
+        fprintf(stderr, "qemu: failed to reserve isa irq %i\n", irq);
+        return -1;
+    }
+    isa_reserved_irqs |= (1 << irq);
+    return 0;
+}
+
 void qemu_set_irq(qemu_irq irq, int level)
 {
     if (!irq)
hw/irq.h +2 -0
Index: qemu/hw/irq.h
===================================================================
@@ -7,6 +7,8 @@
 typedef void (*qemu_irq_handler)(void *opaque, int n, int level);
 typedef void SetIRQFunc(void *opaque, int irq_num, int level);
 
+extern uint16_t isa_reserved_irqs;
+int qemu_reserve_isa_irq(int irq);
 void qemu_set_irq(qemu_irq irq, int level);
 
 static inline void qemu_irq_raise(qemu_irq irq)
hw/pc.c +30 -8
Index: qemu/hw/pc.c
===================================================================
@@ -1047,10 +1047,14 @@ static void audio_init (PCIBus *pci_bus,
 static void pc_init_ne2k_isa(NICInfo *nd, qemu_irq *pic)
 {
     static int nb_ne2k = 0;
+    int irq = ne2000_irq[nb_ne2k];
 
     if (nb_ne2k == NE2000_NB_MAX)
         return;
-    isa_ne2000_init(ne2000_io[nb_ne2k], pic[ne2000_irq[nb_ne2k]], nd);
+    if (qemu_reserve_isa_irq(irq)) {
+        return;
+    }
+    isa_ne2000_init(ne2000_io[nb_ne2k], pic[irq], nd);
     nb_ne2k++;
 }
 
@@ -1271,6 +1275,9 @@ 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]);
+    if (qemu_reserve_isa_irq(13)) {
+        exit(1);
+    }
     ferr_irq = i8259[13];
 
     if (pci_enabled) {
@@ -1304,6 +1311,9 @@ static void pc_init1(ram_addr_t ram_size
         }
     }
 
+    if (qemu_reserve_isa_irq(8)) {
+        exit(1);
+    }
     rtc_state = rtc_init(0x70, i8259[8], 2000);
 
     qemu_register_boot_set(pc_boot_set, rtc_state);
@@ -1314,6 +1324,9 @@ static void pc_init1(ram_addr_t ram_size
     if (pci_enabled) {
         ioapic = ioapic_init();
     }
+    if (qemu_reserve_isa_irq(0)) {
+        exit(1);
+    }
     pit = pit_init(0x40, i8259[0]);
     pcspk_init(pit);
     if (!no_hpet) {
@@ -1325,15 +1338,19 @@ 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_hds[i]);
+            int irq = serial_irq[i];
+            if (qemu_reserve_isa_irq(irq))
+                break;
+            serial_init(serial_io[i], i8259[irq], 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_hds[i]);
+            int irq = parallel_irq[i];
+            if (qemu_reserve_isa_irq(irq))
+                break;
+            parallel_init(parallel_io[i], i8259[irq], parallel_hds[i]);
         }
     }
 
@@ -1364,11 +1381,15 @@ static void pc_init1(ram_addr_t ram_size
         pci_piix3_ide_init(pci_bus, hd, piix3_devfn + 1, i8259);
     } else {
         for(i = 0; i < MAX_IDE_BUS; i++) {
-            isa_ide_init(ide_iobase[i], ide_iobase2[i], i8259[ide_irq[i]],
-	                 hd[MAX_IDE_DEVS * i], hd[MAX_IDE_DEVS * i + 1]);
+            int irq = ide_irq[i];
+            if (!qemu_reserve_isa_irq(irq))
+                isa_ide_init(ide_iobase[i], ide_iobase2[i], i8259[irq],
+                             hd[MAX_IDE_DEVS * i], hd[MAX_IDE_DEVS * i + 1]);
         }
     }
 
+    if (qemu_reserve_isa_irq(1) || qemu_reserve_isa_irq(12))
+        exit(1);
     i8042_init(i8259[1], i8259[12], 0x60);
     DMA_init(0);
 #ifdef HAS_AUDIO
@@ -1379,7 +1400,8 @@ static void pc_init1(ram_addr_t ram_size
         dinfo = drive_get(IF_FLOPPY, 0, i);
         fd[i] = dinfo ? dinfo->bdrv : NULL;
     }
-    floppy_controller = fdctrl_init(i8259[6], 2, 0, 0x3f0, fd);
+    if (!qemu_reserve_isa_irq(6))
+        floppy_controller = fdctrl_init(i8259[6], 2, 0, 0x3f0, fd);
 
     cmos_init(below_4g_mem_size, above_4g_mem_size, boot_device, hd);
 
hw/sb16.c +3 -0
Index: qemu/hw/sb16.c
===================================================================
@@ -1405,6 +1405,9 @@ int SB16_init (qemu_irq *pic)
     static const uint8_t dsp_write_ports[] = {0x6, 0xc};
     static const uint8_t dsp_read_ports[] = {0x6, 0xa, 0xc, 0xd, 0xe, 0xf};
 
+    if (qemu_reserve_isa_irq(conf.irq))
+        return -1;
+
     s = qemu_mallocz (sizeof (*s));
 
     s->cmd = -1;
#1 lore raw
Quoted 9 lines · Jes Sorensen

Hi

As part of trying to provide the BIOS with information about PCI interrupt availability, I have added qemu_reserve_isa_irq() to try and keep track of which IRQs are claimed for ISA.

If we can agree on this approach, then I'll be using it to pass info on to the guest BIOS in a follow on patch.

Looks good to me. I don't know qdev enough to tell if the same can be done as part of qdev infrastructure.

--
Gleb.

#2 lore raw
In reply to Jes Sorensen
Quoted 8 lines · Jes Sorensen

Hi

As part of trying to provide the BIOS with information about PCI interrupt availability, I have added qemu_reserve_isa_irq() to try and keep track of which IRQs are claimed for ISA.

If we can agree on this approach, then I'll be using it to pass info on to the guest BIOS in a follow on patch.

Given the new isa qdev support, maybe it would be better to hook into isa_connect_irq?

Quoted 3 lines

Thanks,
Jes

Regards,

Anthony Liguori

#3 lore raw
Quoted 10 lines · Anthony Liguori
Quoted 6 lines · Jes Sorensen

As part of trying to provide the BIOS with information about PCI interrupt availability, I have added qemu_reserve_isa_irq() to try and keep track of which IRQs are claimed for ISA.

If we can agree on this approach, then I'll be using it to pass info on to the guest BIOS in a follow on patch.

Given the new isa qdev support, maybe it would be better to hook into isa_connect_irq?

Hi Anthony,

In principle that would be good, the problem is just that the most of the code still brute force messages with the i8259 array directly, including the new ISA code. It really needs to be fixed to reference the ISA IRQ number and not the i8259 array directly :-(

Jes

#4 lore raw
Quoted 10 lines · Jes Sorensen
Quoted 2 lines · Anthony Liguori

Given the new isa qdev support, maybe it would be better to hook into isa_connect_irq?

Hi Anthony,

In principle that would be good, the problem is just that the most of the code still brute force messages with the i8259 array directly, including the new ISA code. It really needs to be fixed to reference the ISA IRQ number and not the i8259 array directly :-(

How about making isa-bus.c own the i8259 array then? We could pass it to isa_bus_new. Then switch over to reference isa irqs by number. That allows isa-bus to keep track of the allocations. Maybe it makes sense to kill the sysbus-style isa_{init,connect}_irq split and have a irq bus property then.

cheers,
Gerd

#5 lore raw
Quoted 11 lines · Gerd Hoffmann
Quoted 4 lines · Jes Sorensen

In principle that would be good, the problem is just that the most of the code still brute force messages with the i8259 array directly, including the new ISA code. It really needs to be fixed to reference the ISA IRQ number and not the i8259 array directly :-(

How about making isa-bus.c own the i8259 array then? We could pass it to isa_bus_new. Then switch over to reference isa irqs by number. That allows isa-bus to keep track of the allocations. Maybe it makes sense to kill the sysbus-style isa_{init,connect}_irq split and have a irq bus property then.

Hi Gerd,

I would like to see that. I was looking into how much it would be, but I got lost in the qdev dependencies :(

Cheers,
Jes

#6 lore raw
Quoted 2 lines · Jes Sorensen

I would like to see that. I was looking into how much it would be, but I got lost in the qdev dependencies :(

I'll try to have a look tomorrow.

cheers,
Gerd

#7 lore raw
In reply to Jes Sorensen
Quoted 13 lines · Jes Sorensen
Quoted 11 lines · Gerd Hoffmann
Quoted 4 lines · Jes Sorensen

In principle that would be good, the problem is just that the most of the code still brute force messages with the i8259 array directly, including the new ISA code. It really needs to be fixed to reference the ISA IRQ number and not the i8259 array directly :-(

How about making isa-bus.c own the i8259 array then? We could pass it to isa_bus_new. Then switch over to reference isa irqs by number. That allows isa-bus to keep track of the allocations. Maybe it makes sense to kill the sysbus-style isa_{init,connect}_irq split and have a irq bus property then.

Quoted 4 lines

Hi Gerd,

I would like to see that. I was looking into how much it would be, but I got lost in the qdev dependencies :(

Attached a patch. It will:

(1) make isa-bus maintain isa irqs, complain when allocating already taken irqs.
(2) note that (1) works only for isa devices converted to qdev already (floppy and ps2/kbd/mouse right now), so more work is needed to make this really useful.
(3) split floppy init into isa and sysbus versions.
(4) add sysbus->isa bridge & fix -M isapc breakage.

cheers,
Gerd

Quoted 1 line

From d5bbd89ee65d5a29816d5f126425d1f26cfeaafe Mon Sep 17 00:00:00 2001

From: Gerd Hoffmann <[email protected]>
Date: Tue, 11 Aug 2009 22:27:38 +0200
Subject: [PATCH] isabus fixes

---
 hw/fdc.c        |   47 ++++++++++++++++++++++---------------
 hw/fdc.h        |    9 ++++--
 hw/isa-bus.c    |   70 +++++++++++++++++++++++++++++++++++++++++++++++++-----
 hw/isa.h        |    4 ++-
 hw/mips_jazz.c  |    2 +-
 hw/mips_malta.c |    2 +-
 hw/pc.c         |    8 ++++--
 hw/ppc_prep.c   |    2 +-
 hw/sun4u.c      |    2 +-
 9 files changed, 109 insertions(+), 37 deletions(-)
hw/fdc.c +28 -19
index c55560f..4011cc3 100644
@@ -1871,33 +1871,42 @@ static void fdctrl_connect_drives(fdctrl_t *fdctrl, BlockDriverState **fds)
     }
 }
 
-fdctrl_t *fdctrl_init (qemu_irq irq, int dma_chann, int mem_mapped,
-                       target_phys_addr_t io_base,
-                       BlockDriverState **fds)
+fdctrl_t *fdctrl_init_isa(int isairq, int dma_chann,
+                          uint32_t io_base,
+                          BlockDriverState **fds)
 {
     fdctrl_t *fdctrl;
+    ISADevice *dev;
 
-    if (mem_mapped) {
-        DeviceState *dev;
-        fdctrl_sysbus_t *sys;
+    dev = isa_create_simple("isa-fdc", io_base, 0);
+    fdctrl = &(DO_UPCAST(fdctrl_isabus_t, busdev, dev)->state);
+    isa_connect_irq(dev, 0, isairq);
 
-        dev = qdev_create(NULL, "sysbus-fdc");
-        qdev_init(dev);
-        sys = DO_UPCAST(fdctrl_sysbus_t, busdev.qdev, dev);
-        fdctrl = &sys->state;
-        sysbus_connect_irq(&sys->busdev, 0, irq);
-        sysbus_mmio_map(&sys->busdev, 0, io_base);
-    } else {
-        ISADevice *dev;
+    fdctrl->dma_chann = dma_chann;
+    DMA_register_channel(dma_chann, &fdctrl_transfer_handler, fdctrl);
 
-        dev = isa_create_simple("isa-fdc", io_base, 0);
-        fdctrl = &(DO_UPCAST(fdctrl_isabus_t, busdev, dev)->state);
-        isa_connect_irq(dev, 0, irq);
-    }
+    fdctrl_connect_drives(fdctrl, fds);
+
+    return fdctrl;
+}
+
+fdctrl_t *fdctrl_init_sysbus(qemu_irq irq, int dma_chann,
+                             target_phys_addr_t mmio_base,
+                             BlockDriverState **fds)
+{
+    fdctrl_t *fdctrl;
+    DeviceState *dev;
+    fdctrl_sysbus_t *sys;
+
+    dev = qdev_create(NULL, "sysbus-fdc");
+    qdev_init(dev);
+    sys = DO_UPCAST(fdctrl_sysbus_t, busdev.qdev, dev);
+    fdctrl = &sys->state;
+    sysbus_connect_irq(&sys->busdev, 0, irq);
+    sysbus_mmio_map(&sys->busdev, 0, mmio_base);
 
     fdctrl->dma_chann = dma_chann;
     DMA_register_channel(dma_chann, &fdctrl_transfer_handler, fdctrl);
-
     fdctrl_connect_drives(fdctrl, fds);
 
     return fdctrl;
hw/fdc.h +6 -3
index 7b6a9de..04d64ea 100644
@@ -3,9 +3,12 @@
 
 typedef struct fdctrl_t fdctrl_t;
 
-fdctrl_t *fdctrl_init (qemu_irq irq, int dma_chann, int mem_mapped,
-                       target_phys_addr_t io_base,
-                       BlockDriverState **fds);
+fdctrl_t *fdctrl_init_isa(int isairq, int dma_chann,
+                          uint32_t io_base,
+                          BlockDriverState **fds);
+fdctrl_t *fdctrl_init_sysbus(qemu_irq irq, int dma_chann,
+                             target_phys_addr_t mmio_base,
+                             BlockDriverState **fds);
 fdctrl_t *sun4m_fdctrl_init (qemu_irq irq, target_phys_addr_t io_base,
                              BlockDriverState **fds, qemu_irq *fdc_tc);
 int fdctrl_get_drive_type(fdctrl_t *fdctrl, int drive_num);
hw/isa-bus.c +63 -7

105 lines not on this page · raw message

hw/isa.h +3 -1
index 49c58f8..1a3bb5b 100644
@@ -13,6 +13,7 @@ typedef struct ISADeviceInfo ISADeviceInfo;
 struct ISADevice {
     DeviceState qdev;
     uint32_t iobase[2];
+    uint32_t isairq[2];
     qemu_irq *irqs[2];
     int nirqs;
 };
@@ -24,7 +25,8 @@ struct ISADeviceInfo {
 };
 
 ISABus *isa_bus_new(DeviceState *dev);
-void isa_connect_irq(ISADevice *dev, int n, qemu_irq irq);
+void isa_bus_irqs(qemu_irq *irqs);
+void isa_connect_irq(ISADevice *dev, int devirq, int isairq);
 void isa_init_irq(ISADevice *dev, qemu_irq *p);
 void isa_qdev_register(ISADeviceInfo *info);
 ISADevice *isa_create_simple(const char *name, uint32_t iobase, uint32_t iobase2);
hw/mips_jazz.c +1 -1
index e09a6b2..e283b7a 100644
@@ -238,7 +238,7 @@ void mips_jazz_init (ram_addr_t ram_size,
         DriveInfo *dinfo = drive_get(IF_FLOPPY, 0, n);
         fds[n] = dinfo ? dinfo->bdrv : NULL;
     }
-    fdctrl_init(rc4030[1], 0, 1, 0x80003000, fds);
+    fdctrl_init_sysbus(rc4030[1], 0, 0x80003000, fds);
 
     /* Real time clock */
     rtc_init(0x70, i8259[8], 1980);
hw/mips_malta.c +1 -1
index 4e51c8d..1ac4c41 100644
@@ -929,7 +929,7 @@ void mips_malta_init (ram_addr_t ram_size,
         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(6, 2, 0x3f0, fd);
 
     /* Sound card */
 #ifdef HAS_AUDIO
hw/pc.c +5 -3
index cc6e7e8..88a9ef8 100644
@@ -1272,7 +1272,9 @@ static void pc_init1(ram_addr_t ram_size,
         piix3_devfn = piix3_init(pci_bus, -1);
     } else {
         pci_bus = NULL;
+        isa_bus_new(NULL);
     }
+    isa_bus_irqs(i8259);
 
     /* init basic PC hardware */
     register_ioport_write(0x80, 1, 1, ioport80_write, NULL);
@@ -1364,8 +1366,8 @@ static void pc_init1(ram_addr_t ram_size,
     }
 
     isa_dev = isa_create_simple("i8042", 0x60, 0x64);
-    isa_connect_irq(isa_dev, 0, i8259[1]);
-    isa_connect_irq(isa_dev, 1, i8259[12]);
+    isa_connect_irq(isa_dev, 0, 1);
+    isa_connect_irq(isa_dev, 1, 12);
     DMA_init(0);
 #ifdef HAS_AUDIO
     audio_init(pci_enabled ? pci_bus : NULL, i8259);
@@ -1375,7 +1377,7 @@ static void pc_init1(ram_addr_t ram_size,
         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(6, 2, 0x3f0, fd);
 
     cmos_init(below_4g_mem_size, above_4g_mem_size, boot_device, hd);
 
hw/ppc_prep.c +1 -1
index 97190a2..51a3df9 100644
@@ -708,7 +708,7 @@ static void ppc_prep_init (ram_addr_t ram_size,
         dinfo = drive_get(IF_FLOPPY, 0, i);
         fd[i] = dinfo ? dinfo->bdrv : NULL;
     }
-    fdctrl_init(i8259[6], 2, 0, 0x3f0, fd);
+    fdctrl_init_isa(6, 2, 0x3f0, fd);
 
     /* Register speaker port */
     register_ioport_read(0x61, 1, 1, speaker_ioport_read, NULL);
hw/sun4u.c +1 -1
index 77164ca..6a3a1f9 100644
@@ -616,7 +616,7 @@ static void sun4uv_init(ram_addr_t RAM_size,
         dinfo = drive_get(IF_FLOPPY, 0, i);
         fd[i] = dinfo ? dinfo->bdrv : NULL;
     }
-    floppy_controller = fdctrl_init(NULL/*6*/, 2, 0, 0x3f0, fd);
+    floppy_controller = fdctrl_init_isa(6, 2, 0x3f0, fd);
     nvram = m48t59_init(NULL/*8*/, 0, 0x0074, NVRAM_SIZE, 59);
 
     initrd_size = 0;
Signature · 1 line
1.6.2.5
#8 lore raw

PATCH 1/1 isa_reserve_irq()

Quoted 9 lines · Gerd Hoffmann

Attached a patch. It will:

(1) make isa-bus maintain isa irqs, complain when allocating already taken irqs.
(2) note that (1) works only for isa devices converted to qdev already (floppy and ps2/kbd/mouse right now), so more work is needed to make this really useful.
(3) split floppy init into isa and sysbus versions.
(4) add sysbus->isa bridge & fix -M isapc breakage.

Hi Gerd,

This looks great! I have implemented isa_reserve_irq() on top of this. It allows one to reserve ISA irqs without converting them to qdev, which for certain interrupts makes sense, and I have also made the other users call it until they are converted to qdev.

Let me know what you think. With this I get what I am after.

Cheers,
Jes

Introduce isa_reserve_irq() which marks an irq reserved and returns the appropriate qemu_irq entry from the i8259 table.

isa_reserve_irq() is to be used to allocate ISA IRQs for devices which have not yet, or are not meant to be converted to qdev. Like the ferr irq.

This patch goes on top of Gerd Hoffmann's which makes isa-bus.c own the ISA irq table.

---
 hw/cs4231a.c |   12 +++++-------
 hw/hpet.c    |    5 +++++
 hw/ide.c     |    7 +++++--
 hw/isa-bus.c |   14 ++++++++++++++
 hw/isa.h     |    1 +
 hw/pc.c      |   23 +++++++++++++----------
 hw/sb16.c    |   25 ++++++++++++-------------
 7 files changed, 55 insertions(+), 32 deletions(-)
hw/cs4231a.c +5 -7
Index: qemu/hw/cs4231a.c
===================================================================
@@ -60,10 +60,9 @@ static struct {
 
 typedef struct CSState {
     QEMUSoundCard card;
-    qemu_irq *pic;
+    qemu_irq pic;
     uint32_t regs[CS_REGS];
     uint8_t dregs[CS_DREGS];
-    int irq;
     int dma;
     int port;
     int shift;
@@ -483,7 +482,7 @@ IO_WRITE_PROTO (cs_write)
         case Alternate_Feature_Status:
             if ((s->dregs[iaddr] & PI) && !(val & PI)) {
                 /* XXX: TI CI */
-                qemu_irq_lower (s->pic[s->irq]);
+                qemu_irq_lower (s->pic);
                 s->regs[Status] &= ~INT;
             }
             s->dregs[iaddr] = val;
@@ -503,7 +502,7 @@ IO_WRITE_PROTO (cs_write)
 
     case Status:
         if (s->regs[Status] & INT) {
-            qemu_irq_lower (s->pic[s->irq]);
+            qemu_irq_lower (s->pic);
         }
         s->regs[Status] &= ~INT;
         s->dregs[Alternate_Feature_Status] &= ~(PI | CI | TI);
@@ -588,7 +587,7 @@ static int cs_dma_read (void *opaque, in
         s->regs[Status] |= INT;
         s->dregs[Alternate_Feature_Status] |= PI;
         s->transferred = 0;
-        qemu_irq_raise (s->pic[s->irq]);
+        qemu_irq_raise (s->pic);
     }
     else {
         s->transferred += written;
@@ -643,8 +642,7 @@ int cs4231a_init (qemu_irq *pic)
 
     s = qemu_mallocz (sizeof (*s));
 
-    s->pic = pic;
-    s->irq = conf.irq;
+    s->pic = isa_reserve_irq(conf.irq);
     s->dma = conf.dma;
     s->port = conf.port;
 
hw/hpet.c +5 -0
Index: qemu/hw/hpet.c
===================================================================
@@ -29,6 +29,7 @@
 #include "console.h"
 #include "qemu-timer.h"
 #include "hpet_emul.h"
+#include "isa.h"
 
 //#define HPET_DEBUG
 #ifdef HPET_DEBUG
@@ -574,6 +575,10 @@ void hpet_init(qemu_irq *irq) {
     s = qemu_mallocz(sizeof(HPETState));
     hpet_statep = s;
     s->irqs = irq;
+    /*
+     * Rely on irq2 being used by HPET
+     */
+    isa_reserve_irq(2);
     for (i=0; i<HPET_NUM_TIMERS; i++) {
         HPETTimer *timer = &s->timer[i];
         timer->qemu_timer = qemu_new_timer(vm_clock, hpet_timer, timer);
hw/ide.c +5 -2
Index: qemu/hw/ide.c
===================================================================
@@ -3414,8 +3414,8 @@ void pci_piix3_ide_init(PCIBus *bus, Blo
     pci_register_bar((PCIDevice *)d, 4, 0x10,
                            PCI_ADDRESS_SPACE_IO, bmdma_map);
 
-    ide_init2(&d->ide_if[0], hd_table[0], hd_table[1], pic[14]);
-    ide_init2(&d->ide_if[2], hd_table[2], hd_table[3], pic[15]);
+    ide_init2(&d->ide_if[0], hd_table[0], hd_table[1], isa_reserve_irq(14));
+    ide_init2(&d->ide_if[2], hd_table[2], hd_table[3], isa_reserve_irq(15));
     ide_init_ioport(&d->ide_if[0], 0x1f0, 0x3f6);
     ide_init_ioport(&d->ide_if[2], 0x170, 0x376);
 
@@ -3454,6 +3454,9 @@ void pci_piix4_ide_init(PCIBus *bus, Blo
     pci_register_bar((PCIDevice *)d, 4, 0x10,
                            PCI_ADDRESS_SPACE_IO, bmdma_map);
 
+    /*
+     * These should call isa_reserve_irq() instead when MIPS supports it
+     */
     ide_init2(&d->ide_if[0], hd_table[0], hd_table[1], pic[14]);
     ide_init2(&d->ide_if[2], hd_table[2], hd_table[3], pic[15]);
     ide_init_ioport(&d->ide_if[0], 0x1f0, 0x3f6);
hw/isa-bus.c +14 -0
Index: qemu/hw/isa-bus.c
===================================================================
@@ -85,6 +85,20 @@ void isa_connect_irq(ISADevice *dev, int
     }
 }
 
+qemu_irq isa_reserve_irq(int isairq)
+{
+    if (isairq < 0 || isairq > 15) {
+        fprintf(stderr, "isa irq %d invalid\n", isairq);
+        exit(1);
+    }
+    if (isabus->assigned & (1 << isairq)) {
+        fprintf(stderr, "isa irq %d already assigned\n", isairq);
+        exit(1);
+    }
+    isabus->assigned |= (1 << isairq);
+    return isabus->irqs[isairq];
+}
+
 void isa_init_irq(ISADevice *dev, qemu_irq *p)
 {
     assert(dev->nirqs < ARRAY_SIZE(dev->irqs));
hw/isa.h +1 -0
Index: qemu/hw/isa.h
===================================================================
@@ -27,6 +27,7 @@ struct ISADeviceInfo {
 ISABus *isa_bus_new(DeviceState *dev);
 void isa_bus_irqs(qemu_irq *irqs);
 void isa_connect_irq(ISADevice *dev, int devirq, int isairq);
+qemu_irq isa_reserve_irq(int isairq);
 void isa_init_irq(ISADevice *dev, qemu_irq *p);
 void isa_qdev_register(ISADeviceInfo *info);
 ISADevice *isa_create_simple(const char *name, uint32_t iobase, uint32_t iobase2);
hw/pc.c +13 -10
Index: qemu/hw/pc.c
===================================================================
@@ -1037,13 +1037,14 @@ static void audio_init (PCIBus *pci_bus,
 }
 #endif
 
-static void pc_init_ne2k_isa(NICInfo *nd, qemu_irq *pic)
+static void pc_init_ne2k_isa(NICInfo *nd)
 {
     static int nb_ne2k = 0;
 
     if (nb_ne2k == NE2000_NB_MAX)
         return;
-    isa_ne2000_init(ne2000_io[nb_ne2k], pic[ne2000_irq[nb_ne2k]], nd);
+    isa_ne2000_init(ne2000_io[nb_ne2k],
+                    isa_reserve_irq(ne2000_irq[nb_ne2k]), nd);
     nb_ne2k++;
 }
 
@@ -1265,7 +1266,6 @@ 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];
 
     if (pci_enabled) {
         pci_bus = i440fx_init(&i440fx_state, i8259);
@@ -1275,6 +1275,7 @@ static void pc_init1(ram_addr_t ram_size
         isa_bus_new(NULL);
     }
     isa_bus_irqs(i8259);
+    ferr_irq = isa_reserve_irq(13);
 
     /* init basic PC hardware */
     register_ioport_write(0x80, 1, 1, ioport80_write, NULL);
@@ -1300,7 +1301,7 @@ static void pc_init1(ram_addr_t ram_size
         }
     }
 
-    rtc_state = rtc_init(0x70, i8259[8], 2000);
+    rtc_state = rtc_init(0x70, isa_reserve_irq(8), 2000);
 
     qemu_register_boot_set(pc_boot_set, rtc_state);
 
@@ -1310,7 +1311,7 @@ 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_reserve_irq(0));
     pcspk_init(pit);
     if (!no_hpet) {
         hpet_init(i8259);
@@ -1321,14 +1322,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_reserve_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_reserve_irq(parallel_irq[i]),
                           parallel_hds[i]);
         }
     }
@@ -1339,7 +1340,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);
         else
             pci_nic_init(nd, "ne2k_pci", NULL);
     }
@@ -1360,7 +1361,8 @@ static void pc_init1(ram_addr_t ram_size
         pci_piix3_ide_init(pci_bus, hd, piix3_devfn + 1, i8259);
     } 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_reserve_irq(ide_irq[i]),
 	                 hd[MAX_IDE_DEVS * i], hd[MAX_IDE_DEVS * i + 1]);
         }
     }
@@ -1390,7 +1392,8 @@ 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_reserve_irq(9));
         for (i = 0; i < 8; i++) {
             DeviceState *eeprom;
             eeprom = qdev_create((BusState *)smbus, "smbus-eeprom");
hw/sb16.c +12 -13
Index: qemu/hw/sb16.c
===================================================================
@@ -56,7 +56,7 @@ static struct {
 
 typedef struct SB16State {
     QEMUSoundCard card;
-    qemu_irq *pic;
+    qemu_irq pic;
     int irq;
     int dma;
     int hdma;
@@ -190,7 +190,7 @@ static void aux_timer (void *opaque)
 {
     SB16State *s = opaque;
     s->can_write = 1;
-    qemu_irq_raise (s->pic[s->irq]);
+    qemu_irq_raise (s->pic);
 }
 
 #define DMA8_AUTO 1
@@ -598,7 +598,7 @@ static void command (SB16State *s, uint8
         case 0xf3:
             dsp_out_data (s, 0xaa);
             s->mixer_regs[0x82] |= (cmd == 0xf2) ? 1 : 2;
-            qemu_irq_raise (s->pic[s->irq]);
+            qemu_irq_raise (s->pic);
             break;
 
         case 0xf9:
@@ -766,7 +766,7 @@ static void complete (SB16State *s)
                 bytes = samples << s->fmt_stereo << (s->fmt_bits == 16);
                 ticks = (bytes * ticks_per_sec) / freq;
                 if (ticks < ticks_per_sec / 1024) {
-                    qemu_irq_raise (s->pic[s->irq]);
+                    qemu_irq_raise (s->pic);
                 }
                 else {
                     if (s->aux_ts) {
@@ -858,10 +858,10 @@ static void legacy_reset (SB16State *s)
 
 static void reset (SB16State *s)
 {
-    qemu_irq_lower (s->pic[s->irq]);
+    qemu_irq_lower (s->pic);
     if (s->dma_auto) {
-        qemu_irq_raise (s->pic[s->irq]);
-        qemu_irq_lower (s->pic[s->irq]);
+        qemu_irq_raise (s->pic);
+        qemu_irq_lower (s->pic);
     }
 
     s->mixer_regs[0x82] = 0;
@@ -897,7 +897,7 @@ static IO_WRITE_PROTO (dsp_write)
             if (s->v2x6 == 1) {
                 if (0 && s->highspeed) {
                     s->highspeed = 0;
-                    qemu_irq_lower (s->pic[s->irq]);
+                    qemu_irq_lower (s->pic);
                     control (s, 0);
                 }
                 else {
@@ -1008,7 +1008,7 @@ static IO_READ_PROTO (dsp_read)
         if (s->mixer_regs[0x82] & 1) {
             ack = 1;
             s->mixer_regs[0x82] &= 1;
-            qemu_irq_lower (s->pic[s->irq]);
+            qemu_irq_lower (s->pic);
         }
         break;
 
@@ -1017,7 +1017,7 @@ static IO_READ_PROTO (dsp_read)
         if (s->mixer_regs[0x82] & 2) {
             ack = 1;
             s->mixer_regs[0x82] &= 2;
-            qemu_irq_lower (s->pic[s->irq]);
+            qemu_irq_lower (s->pic);
         }
         break;
 
@@ -1231,7 +1231,7 @@ static int SB_read_DMA (void *opaque, in
 
     if (s->left_till_irq <= 0) {
         s->mixer_regs[0x82] |= (nchan & 4) ? 2 : 1;
-        qemu_irq_raise (s->pic[s->irq]);
+        qemu_irq_raise (s->pic);
         if (0 == s->dma_auto) {
             control (s, 0);
             speaker (s, 0);
@@ -1408,8 +1408,7 @@ int SB16_init (qemu_irq *pic)
     s = qemu_mallocz (sizeof (*s));
 
     s->cmd = -1;
-    s->pic = pic;
-    s->irq = conf.irq;
+    s->pic = isa_reserve_irq(conf.irq);
     s->dma = conf.dma;
     s->hdma = conf.hdma;
     s->port = conf.port;
#9 lore raw

[Qemu-devel] Re: [PATCH] isa_reserve_irq()

Quoted 5 lines · Jes Sorensen

Hi Gerd,

This looks great! I have implemented isa_reserve_irq() on top of this. It allows one to reserve ISA irqs without converting them to qdev, which for certain interrupts makes sense,

which ones?

Quoted 2 lines

and I have also made the other users
call it until they are converted to qdev.

I think that deserves a comment clearly stating that this interface is a (temporary?) thing for not-yet converted devices.

Otherwise it looks fine to me.

cheers,
Gerd

#10 lore raw

[Qemu-devel] Re: [PATCH] isa_reserve_irq()

Quoted 8 lines · Gerd Hoffmann
Quoted 5 lines · Jes Sorensen

Hi Gerd,

This looks great! I have implemented isa_reserve_irq() on top of this. It allows one to reserve ISA irqs without converting them to qdev, which for certain interrupts makes sense,

which ones?

ferr

Quoted 7 lines
Quoted 2 lines

and I have also made the other users
call it until they are converted to qdev.

I think that deserves a comment clearly stating that this interface is a (temporary?) thing for not-yet converted devices.

Otherwise it looks fine to me.

If you think it makes sense to convert ferr as well, then I'll be happy to add the comment.

Anthony also pointed out that hpet shouldn't be on the list - it makes sense. The BIOS can filter out irq2 based on the irq0override flag, so I will take out the hpet bits too.

Cheers,
Jes

#11 lore raw

[Qemu-devel] Re: [PATCH] isa_reserve_irq()

Quoted 11 lines · Jes Sorensen
Quoted 8 lines · Gerd Hoffmann
Quoted 5 lines · Jes Sorensen

Hi Gerd,

This looks great! I have implemented isa_reserve_irq() on top of this. It allows one to reserve ISA irqs without converting them to qdev, which for certain interrupts makes sense,

which ones?

ferr

Ah, that one.

Quoted 7 lines
Quoted 4 lines

I think that deserves a comment clearly stating that this interface is a (temporary?) thing for not-yet converted devices.

Otherwise it looks fine to me.

If you think it makes sense to convert ferr as well, then I'll be happy to add the comment.

Hmm, I don't think it makes sense to pull a dummy device out of thin air, isa_reserve_irq() will do fine. But it should be be clearly documented that this is for this special case and not-yet converted devices. qdev-ified devices should use the other isa_*_irq functions instead.

cheers,
Gerd

#12 lore raw

[Qemu-devel] Re: [PATCH] isa_reserve_irq()

Quoted 5 lines · Gerd Hoffmann

Hmm, I don't think it makes sense to pull a dummy device out of thin air, isa_reserve_irq() will do fine. But it should be be clearly documented that this is for this special case and not-yet converted devices. qdev-ified devices should use the other isa_*_irq functions instead.

Ok - do you want me to update the documentation, or is it fine as is then?

Cheers,
Jes

#13 lore raw

[Qemu-devel] Re: [PATCH] isa_reserve_irq()

Quoted 9 lines · Jes Sorensen
Quoted 5 lines · Gerd Hoffmann

Hmm, I don't think it makes sense to pull a dummy device out of thin air, isa_reserve_irq() will do fine. But it should be be clearly documented that this is for this special case and not-yet converted devices. qdev-ified devices should use the other isa_*_irq functions instead.

Ok - do you want me to update the documentation, or is it fine as is then?

Would be great to update the commit message and also place a comment into the source code.

thanks,
Gerd

#14 lore raw

PATCH 1/1 isa_reserve_irq() v4

In reply to Gerd Hoffmann

Hi,

Guess it's v4 by now - I have added comments to the code making it clear that the interface is for special cases such as 'ferr' and otherwise for temporary use until qdev-ification.

Hope this covers what you were after.

Cheers,
Jes

Introduce isa_reserve_irq() which marks an irq reserved and returns the appropriate qemu_irq entry from the i8259 table.

isa_reserve_irq() is a temporary interface to be used to allocate ISA IRQs for devices which have not yet been converted to qdev, and for special cases which are not suited for qdev conversions, such as the 'ferr'.

This patch goes on top of Gerd Hoffmann's which makes isa-bus.c own the ISA irq table.

---
 hw/cs4231a.c |   12 +++++-------
 hw/ide.c     |    7 +++++--
 hw/isa-bus.c |   21 +++++++++++++++++++++
 hw/isa.h     |    1 +
 hw/pc.c      |   23 +++++++++++++----------
 hw/sb16.c    |   25 ++++++++++++-------------
 6 files changed, 57 insertions(+), 32 deletions(-)
hw/cs4231a.c +5 -7
Index: qemu/hw/cs4231a.c
===================================================================
@@ -60,10 +60,9 @@ static struct {
 
 typedef struct CSState {
     QEMUSoundCard card;
-    qemu_irq *pic;
+    qemu_irq pic;
     uint32_t regs[CS_REGS];
     uint8_t dregs[CS_DREGS];
-    int irq;
     int dma;
     int port;
     int shift;
@@ -483,7 +482,7 @@ IO_WRITE_PROTO (cs_write)
         case Alternate_Feature_Status:
             if ((s->dregs[iaddr] & PI) && !(val & PI)) {
                 /* XXX: TI CI */
-                qemu_irq_lower (s->pic[s->irq]);
+                qemu_irq_lower (s->pic);
                 s->regs[Status] &= ~INT;
             }
             s->dregs[iaddr] = val;
@@ -503,7 +502,7 @@ IO_WRITE_PROTO (cs_write)
 
     case Status:
         if (s->regs[Status] & INT) {
-            qemu_irq_lower (s->pic[s->irq]);
+            qemu_irq_lower (s->pic);
         }
         s->regs[Status] &= ~INT;
         s->dregs[Alternate_Feature_Status] &= ~(PI | CI | TI);
@@ -588,7 +587,7 @@ static int cs_dma_read (void *opaque, in
         s->regs[Status] |= INT;
         s->dregs[Alternate_Feature_Status] |= PI;
         s->transferred = 0;
-        qemu_irq_raise (s->pic[s->irq]);
+        qemu_irq_raise (s->pic);
     }
     else {
         s->transferred += written;
@@ -643,8 +642,7 @@ int cs4231a_init (qemu_irq *pic)
 
     s = qemu_mallocz (sizeof (*s));
 
-    s->pic = pic;
-    s->irq = conf.irq;
+    s->pic = isa_reserve_irq(conf.irq);
     s->dma = conf.dma;
     s->port = conf.port;
 
hw/ide.c +5 -2
Index: qemu/hw/ide.c
===================================================================
@@ -3414,8 +3414,8 @@ void pci_piix3_ide_init(PCIBus *bus, Blo
     pci_register_bar((PCIDevice *)d, 4, 0x10,
                            PCI_ADDRESS_SPACE_IO, bmdma_map);
 
-    ide_init2(&d->ide_if[0], hd_table[0], hd_table[1], pic[14]);
-    ide_init2(&d->ide_if[2], hd_table[2], hd_table[3], pic[15]);
+    ide_init2(&d->ide_if[0], hd_table[0], hd_table[1], isa_reserve_irq(14));
+    ide_init2(&d->ide_if[2], hd_table[2], hd_table[3], isa_reserve_irq(15));
     ide_init_ioport(&d->ide_if[0], 0x1f0, 0x3f6);
     ide_init_ioport(&d->ide_if[2], 0x170, 0x376);
 
@@ -3454,6 +3454,9 @@ void pci_piix4_ide_init(PCIBus *bus, Blo
     pci_register_bar((PCIDevice *)d, 4, 0x10,
                            PCI_ADDRESS_SPACE_IO, bmdma_map);
 
+    /*
+     * These should call isa_reserve_irq() instead when MIPS supports it
+     */
     ide_init2(&d->ide_if[0], hd_table[0], hd_table[1], pic[14]);
     ide_init2(&d->ide_if[2], hd_table[2], hd_table[3], pic[15]);
     ide_init_ioport(&d->ide_if[0], 0x1f0, 0x3f6);
hw/isa-bus.c +21 -0
Index: qemu/hw/isa-bus.c
===================================================================
@@ -85,6 +85,27 @@ void isa_connect_irq(ISADevice *dev, int
     }
 }
 
+/*
+ * isa_reserve_irq() reserves the ISA irq and returns the corresponding
+ * qemu_irq entry for the i8259.
+ *
+ * This function is only for special cases such as the 'ferr', and
+ * temporary use for normal devices until they are converted to qdev.
+ */
+qemu_irq isa_reserve_irq(int isairq)
+{
+    if (isairq < 0 || isairq > 15) {
+        fprintf(stderr, "isa irq %d invalid\n", isairq);
+        exit(1);
+    }
+    if (isabus->assigned & (1 << isairq)) {
+        fprintf(stderr, "isa irq %d already assigned\n", isairq);
+        exit(1);
+    }
+    isabus->assigned |= (1 << isairq);
+    return isabus->irqs[isairq];
+}
+
 void isa_init_irq(ISADevice *dev, qemu_irq *p)
 {
     assert(dev->nirqs < ARRAY_SIZE(dev->irqs));
hw/isa.h +1 -0
Index: qemu/hw/isa.h
===================================================================
@@ -27,6 +27,7 @@ struct ISADeviceInfo {
 ISABus *isa_bus_new(DeviceState *dev);
 void isa_bus_irqs(qemu_irq *irqs);
 void isa_connect_irq(ISADevice *dev, int devirq, int isairq);
+qemu_irq isa_reserve_irq(int isairq);
 void isa_init_irq(ISADevice *dev, qemu_irq *p);
 void isa_qdev_register(ISADeviceInfo *info);
 ISADevice *isa_create_simple(const char *name, uint32_t iobase, uint32_t iobase2);
hw/pc.c +13 -10
Index: qemu/hw/pc.c
===================================================================
@@ -1037,13 +1037,14 @@ static void audio_init (PCIBus *pci_bus,
 }
 #endif
 
-static void pc_init_ne2k_isa(NICInfo *nd, qemu_irq *pic)
+static void pc_init_ne2k_isa(NICInfo *nd)
 {
     static int nb_ne2k = 0;
 
     if (nb_ne2k == NE2000_NB_MAX)
         return;
-    isa_ne2000_init(ne2000_io[nb_ne2k], pic[ne2000_irq[nb_ne2k]], nd);
+    isa_ne2000_init(ne2000_io[nb_ne2k],
+                    isa_reserve_irq(ne2000_irq[nb_ne2k]), nd);
     nb_ne2k++;
 }
 
@@ -1265,7 +1266,6 @@ 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];
 
     if (pci_enabled) {
         pci_bus = i440fx_init(&i440fx_state, i8259);
@@ -1275,6 +1275,7 @@ static void pc_init1(ram_addr_t ram_size
         isa_bus_new(NULL);
     }
     isa_bus_irqs(i8259);
+    ferr_irq = isa_reserve_irq(13);
 
     /* init basic PC hardware */
     register_ioport_write(0x80, 1, 1, ioport80_write, NULL);
@@ -1300,7 +1301,7 @@ static void pc_init1(ram_addr_t ram_size
         }
     }
 
-    rtc_state = rtc_init(0x70, i8259[8], 2000);
+    rtc_state = rtc_init(0x70, isa_reserve_irq(8), 2000);
 
     qemu_register_boot_set(pc_boot_set, rtc_state);
 
@@ -1310,7 +1311,7 @@ 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_reserve_irq(0));
     pcspk_init(pit);
     if (!no_hpet) {
         hpet_init(i8259);
@@ -1321,14 +1322,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_reserve_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_reserve_irq(parallel_irq[i]),
                           parallel_hds[i]);
         }
     }
@@ -1339,7 +1340,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);
         else
             pci_nic_init(nd, "ne2k_pci", NULL);
     }
@@ -1360,7 +1361,8 @@ static void pc_init1(ram_addr_t ram_size
         pci_piix3_ide_init(pci_bus, hd, piix3_devfn + 1, i8259);
     } 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_reserve_irq(ide_irq[i]),
 	                 hd[MAX_IDE_DEVS * i], hd[MAX_IDE_DEVS * i + 1]);
         }
     }
@@ -1390,7 +1392,8 @@ 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_reserve_irq(9));
         for (i = 0; i < 8; i++) {
             DeviceState *eeprom;
             eeprom = qdev_create((BusState *)smbus, "smbus-eeprom");
hw/sb16.c +12 -13
Index: qemu/hw/sb16.c
===================================================================
@@ -56,7 +56,7 @@ static struct {
 
 typedef struct SB16State {
     QEMUSoundCard card;
-    qemu_irq *pic;
+    qemu_irq pic;
     int irq;
     int dma;
     int hdma;
@@ -190,7 +190,7 @@ static void aux_timer (void *opaque)
 {
     SB16State *s = opaque;
     s->can_write = 1;
-    qemu_irq_raise (s->pic[s->irq]);
+    qemu_irq_raise (s->pic);
 }
 
 #define DMA8_AUTO 1
@@ -598,7 +598,7 @@ static void command (SB16State *s, uint8
         case 0xf3:
             dsp_out_data (s, 0xaa);
             s->mixer_regs[0x82] |= (cmd == 0xf2) ? 1 : 2;
-            qemu_irq_raise (s->pic[s->irq]);
+            qemu_irq_raise (s->pic);
             break;
 
         case 0xf9:
@@ -766,7 +766,7 @@ static void complete (SB16State *s)
                 bytes = samples << s->fmt_stereo << (s->fmt_bits == 16);
                 ticks = (bytes * ticks_per_sec) / freq;
                 if (ticks < ticks_per_sec / 1024) {
-                    qemu_irq_raise (s->pic[s->irq]);
+                    qemu_irq_raise (s->pic);
                 }
                 else {
                     if (s->aux_ts) {
@@ -858,10 +858,10 @@ static void legacy_reset (SB16State *s)
 
 static void reset (SB16State *s)
 {
-    qemu_irq_lower (s->pic[s->irq]);
+    qemu_irq_lower (s->pic);
     if (s->dma_auto) {
-        qemu_irq_raise (s->pic[s->irq]);
-        qemu_irq_lower (s->pic[s->irq]);
+        qemu_irq_raise (s->pic);
+        qemu_irq_lower (s->pic);
     }
 
     s->mixer_regs[0x82] = 0;
@@ -897,7 +897,7 @@ static IO_WRITE_PROTO (dsp_write)
             if (s->v2x6 == 1) {
                 if (0 && s->highspeed) {
                     s->highspeed = 0;
-                    qemu_irq_lower (s->pic[s->irq]);
+                    qemu_irq_lower (s->pic);
                     control (s, 0);
                 }
                 else {
@@ -1008,7 +1008,7 @@ static IO_READ_PROTO (dsp_read)
         if (s->mixer_regs[0x82] & 1) {
             ack = 1;
             s->mixer_regs[0x82] &= 1;
-            qemu_irq_lower (s->pic[s->irq]);
+            qemu_irq_lower (s->pic);
         }
         break;
 
@@ -1017,7 +1017,7 @@ static IO_READ_PROTO (dsp_read)
         if (s->mixer_regs[0x82] & 2) {
             ack = 1;
             s->mixer_regs[0x82] &= 2;
-            qemu_irq_lower (s->pic[s->irq]);
+            qemu_irq_lower (s->pic);
         }
         break;
 
@@ -1231,7 +1231,7 @@ static int SB_read_DMA (void *opaque, in
 
     if (s->left_till_irq <= 0) {
         s->mixer_regs[0x82] |= (nchan & 4) ? 2 : 1;
-        qemu_irq_raise (s->pic[s->irq]);
+        qemu_irq_raise (s->pic);
         if (0 == s->dma_auto) {
             control (s, 0);
             speaker (s, 0);
@@ -1408,8 +1408,7 @@ int SB16_init (qemu_irq *pic)
     s = qemu_mallocz (sizeof (*s));
 
     s->cmd = -1;
-    s->pic = pic;
-    s->irq = conf.irq;
+    s->pic = isa_reserve_irq(conf.irq);
     s->dma = conf.dma;
     s->hdma = conf.hdma;
     s->port = conf.port;
Acked-by on v1 1/1
#15 lore raw

[Qemu-devel] Re: [PATCH] isa_reserve_irq() v4

Quoted 7 lines · Jes Sorensen

Hi,

Guess it's v4 by now - I have added comments to the code making it clear that the interface is for special cases such as 'ferr' and otherwise for temporary use until qdev-ification.

Hope this covers what you were after.

Looks good, thanks.

#16 lore raw

[Qemu-devel] Re: [PATCH] isa_reserve_irq() v4

In reply to Jes Sorensen
Quoted 5 lines · Jes Sorensen

Hi,

Guess it's v4 by now - I have added comments to the code making it clear that the interface is for special cases such as 'ferr' and otherwise for temporary use until qdev-ification.

Funny build errors, part 42:

ide.o: In function `pci_piix3_ide_init':
/home/kraxel/projects/qemu/hw/ide.c:3418: undefined reference to isa_reserve_irq' /home/kraxel/projects/qemu/hw/ide.c:3419: undefined reference to isa_reserve_irq'
collect2: ld returned 1 exit status
make[1]: *** [qemu-system-arm] Error 1

didn't know arm has piix3 ;)

I'll go fix it up and re-post all pending isa-bus bits as patch series.

cheers,
Gerd

#17 lore raw

[Qemu-devel] Re: [PATCH] isa_reserve_irq()

In reply to Gerd Hoffmann
Quoted 6 lines · Gerd Hoffmann
Quoted 2 lines · Jes Sorensen

and I have also made the other users
call it until they are converted to qdev.

I think that deserves a comment clearly stating that this interface is a (temporary?) thing for not-yet converted devices.

Hi Gerd,

Thought about it a bit more, I think the only real one is ferr, and I guess we can implement that as a fake device in qdev if needed.

Updated patch without hpet and with the temporary comment included.

Cheers,
Jes

Introduce isa_reserve_irq() which marks an irq reserved and returns the appropriate qemu_irq entry from the i8259 table.

isa_reserve_irq() is a temporary interface to be used to allocate ISA IRQs for devices which have not yet been converted to qdev.

This patch goes on top of Gerd Hoffmann's which makes isa-bus.c own the ISA irq table.

---
 hw/cs4231a.c |   12 +++++-------
 hw/ide.c     |    7 +++++--
 hw/isa-bus.c |   14 ++++++++++++++
 hw/isa.h     |    1 +
 hw/pc.c      |   23 +++++++++++++----------
 hw/sb16.c    |   25 ++++++++++++-------------
 6 files changed, 50 insertions(+), 32 deletions(-)
hw/cs4231a.c +5 -7
Index: qemu/hw/cs4231a.c
===================================================================
@@ -60,10 +60,9 @@ static struct {
 
 typedef struct CSState {
     QEMUSoundCard card;
-    qemu_irq *pic;
+    qemu_irq pic;
     uint32_t regs[CS_REGS];
     uint8_t dregs[CS_DREGS];
-    int irq;
     int dma;
     int port;
     int shift;
@@ -483,7 +482,7 @@ IO_WRITE_PROTO (cs_write)
         case Alternate_Feature_Status:
             if ((s->dregs[iaddr] & PI) && !(val & PI)) {
                 /* XXX: TI CI */
-                qemu_irq_lower (s->pic[s->irq]);
+                qemu_irq_lower (s->pic);
                 s->regs[Status] &= ~INT;
             }
             s->dregs[iaddr] = val;
@@ -503,7 +502,7 @@ IO_WRITE_PROTO (cs_write)
 
     case Status:
         if (s->regs[Status] & INT) {
-            qemu_irq_lower (s->pic[s->irq]);
+            qemu_irq_lower (s->pic);
         }
         s->regs[Status] &= ~INT;
         s->dregs[Alternate_Feature_Status] &= ~(PI | CI | TI);
@@ -588,7 +587,7 @@ static int cs_dma_read (void *opaque, in
         s->regs[Status] |= INT;
         s->dregs[Alternate_Feature_Status] |= PI;
         s->transferred = 0;
-        qemu_irq_raise (s->pic[s->irq]);
+        qemu_irq_raise (s->pic);
     }
     else {
         s->transferred += written;
@@ -643,8 +642,7 @@ int cs4231a_init (qemu_irq *pic)
 
     s = qemu_mallocz (sizeof (*s));
 
-    s->pic = pic;
-    s->irq = conf.irq;
+    s->pic = isa_reserve_irq(conf.irq);
     s->dma = conf.dma;
     s->port = conf.port;
 
hw/ide.c +5 -2
Index: qemu/hw/ide.c
===================================================================
@@ -3414,8 +3414,8 @@ void pci_piix3_ide_init(PCIBus *bus, Blo
     pci_register_bar((PCIDevice *)d, 4, 0x10,
                            PCI_ADDRESS_SPACE_IO, bmdma_map);
 
-    ide_init2(&d->ide_if[0], hd_table[0], hd_table[1], pic[14]);
-    ide_init2(&d->ide_if[2], hd_table[2], hd_table[3], pic[15]);
+    ide_init2(&d->ide_if[0], hd_table[0], hd_table[1], isa_reserve_irq(14));
+    ide_init2(&d->ide_if[2], hd_table[2], hd_table[3], isa_reserve_irq(15));
     ide_init_ioport(&d->ide_if[0], 0x1f0, 0x3f6);
     ide_init_ioport(&d->ide_if[2], 0x170, 0x376);
 
@@ -3454,6 +3454,9 @@ void pci_piix4_ide_init(PCIBus *bus, Blo
     pci_register_bar((PCIDevice *)d, 4, 0x10,
                            PCI_ADDRESS_SPACE_IO, bmdma_map);
 
+    /*
+     * These should call isa_reserve_irq() instead when MIPS supports it
+     */
     ide_init2(&d->ide_if[0], hd_table[0], hd_table[1], pic[14]);
     ide_init2(&d->ide_if[2], hd_table[2], hd_table[3], pic[15]);
     ide_init_ioport(&d->ide_if[0], 0x1f0, 0x3f6);
hw/isa-bus.c +14 -0
Index: qemu/hw/isa-bus.c
===================================================================
@@ -85,6 +85,20 @@ void isa_connect_irq(ISADevice *dev, int
     }
 }
 
+qemu_irq isa_reserve_irq(int isairq)
+{
+    if (isairq < 0 || isairq > 15) {
+        fprintf(stderr, "isa irq %d invalid\n", isairq);
+        exit(1);
+    }
+    if (isabus->assigned & (1 << isairq)) {
+        fprintf(stderr, "isa irq %d already assigned\n", isairq);
+        exit(1);
+    }
+    isabus->assigned |= (1 << isairq);
+    return isabus->irqs[isairq];
+}
+
 void isa_init_irq(ISADevice *dev, qemu_irq *p)
 {
     assert(dev->nirqs < ARRAY_SIZE(dev->irqs));
hw/isa.h +1 -0
Index: qemu/hw/isa.h
===================================================================
@@ -27,6 +27,7 @@ struct ISADeviceInfo {
 ISABus *isa_bus_new(DeviceState *dev);
 void isa_bus_irqs(qemu_irq *irqs);
 void isa_connect_irq(ISADevice *dev, int devirq, int isairq);
+qemu_irq isa_reserve_irq(int isairq);
 void isa_init_irq(ISADevice *dev, qemu_irq *p);
 void isa_qdev_register(ISADeviceInfo *info);
 ISADevice *isa_create_simple(const char *name, uint32_t iobase, uint32_t iobase2);
hw/pc.c +13 -10
Index: qemu/hw/pc.c
===================================================================
@@ -1037,13 +1037,14 @@ static void audio_init (PCIBus *pci_bus,
 }
 #endif
 
-static void pc_init_ne2k_isa(NICInfo *nd, qemu_irq *pic)
+static void pc_init_ne2k_isa(NICInfo *nd)
 {
     static int nb_ne2k = 0;
 
     if (nb_ne2k == NE2000_NB_MAX)
         return;
-    isa_ne2000_init(ne2000_io[nb_ne2k], pic[ne2000_irq[nb_ne2k]], nd);
+    isa_ne2000_init(ne2000_io[nb_ne2k],
+                    isa_reserve_irq(ne2000_irq[nb_ne2k]), nd);
     nb_ne2k++;
 }
 
@@ -1265,7 +1266,6 @@ 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];
 
     if (pci_enabled) {
         pci_bus = i440fx_init(&i440fx_state, i8259);
@@ -1275,6 +1275,7 @@ static void pc_init1(ram_addr_t ram_size
         isa_bus_new(NULL);
     }
     isa_bus_irqs(i8259);
+    ferr_irq = isa_reserve_irq(13);
 
     /* init basic PC hardware */
     register_ioport_write(0x80, 1, 1, ioport80_write, NULL);
@@ -1300,7 +1301,7 @@ static void pc_init1(ram_addr_t ram_size
         }
     }
 
-    rtc_state = rtc_init(0x70, i8259[8], 2000);
+    rtc_state = rtc_init(0x70, isa_reserve_irq(8), 2000);
 
     qemu_register_boot_set(pc_boot_set, rtc_state);
 
@@ -1310,7 +1311,7 @@ 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_reserve_irq(0));
     pcspk_init(pit);
     if (!no_hpet) {
         hpet_init(i8259);
@@ -1321,14 +1322,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_reserve_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_reserve_irq(parallel_irq[i]),
                           parallel_hds[i]);
         }
     }
@@ -1339,7 +1340,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);
         else
             pci_nic_init(nd, "ne2k_pci", NULL);
     }
@@ -1360,7 +1361,8 @@ static void pc_init1(ram_addr_t ram_size
         pci_piix3_ide_init(pci_bus, hd, piix3_devfn + 1, i8259);
     } 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_reserve_irq(ide_irq[i]),
 	                 hd[MAX_IDE_DEVS * i], hd[MAX_IDE_DEVS * i + 1]);
         }
     }
@@ -1390,7 +1392,8 @@ 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_reserve_irq(9));
         for (i = 0; i < 8; i++) {
             DeviceState *eeprom;
             eeprom = qdev_create((BusState *)smbus, "smbus-eeprom");
hw/sb16.c +12 -13
Index: qemu/hw/sb16.c
===================================================================
@@ -56,7 +56,7 @@ static struct {
 
 typedef struct SB16State {
     QEMUSoundCard card;
-    qemu_irq *pic;
+    qemu_irq pic;
     int irq;
     int dma;
     int hdma;
@@ -190,7 +190,7 @@ static void aux_timer (void *opaque)
 {
     SB16State *s = opaque;
     s->can_write = 1;
-    qemu_irq_raise (s->pic[s->irq]);
+    qemu_irq_raise (s->pic);
 }
 
 #define DMA8_AUTO 1
@@ -598,7 +598,7 @@ static void command (SB16State *s, uint8
         case 0xf3:
             dsp_out_data (s, 0xaa);
             s->mixer_regs[0x82] |= (cmd == 0xf2) ? 1 : 2;
-            qemu_irq_raise (s->pic[s->irq]);
+            qemu_irq_raise (s->pic);
             break;
 
         case 0xf9:
@@ -766,7 +766,7 @@ static void complete (SB16State *s)
                 bytes = samples << s->fmt_stereo << (s->fmt_bits == 16);
                 ticks = (bytes * ticks_per_sec) / freq;
                 if (ticks < ticks_per_sec / 1024) {
-                    qemu_irq_raise (s->pic[s->irq]);
+                    qemu_irq_raise (s->pic);
                 }
                 else {
                     if (s->aux_ts) {
@@ -858,10 +858,10 @@ static void legacy_reset (SB16State *s)
 
 static void reset (SB16State *s)
 {
-    qemu_irq_lower (s->pic[s->irq]);
+    qemu_irq_lower (s->pic);
     if (s->dma_auto) {
-        qemu_irq_raise (s->pic[s->irq]);
-        qemu_irq_lower (s->pic[s->irq]);
+        qemu_irq_raise (s->pic);
+        qemu_irq_lower (s->pic);
     }
 
     s->mixer_regs[0x82] = 0;
@@ -897,7 +897,7 @@ static IO_WRITE_PROTO (dsp_write)
             if (s->v2x6 == 1) {
                 if (0 && s->highspeed) {
                     s->highspeed = 0;
-                    qemu_irq_lower (s->pic[s->irq]);
+                    qemu_irq_lower (s->pic);
                     control (s, 0);
                 }
                 else {
@@ -1008,7 +1008,7 @@ static IO_READ_PROTO (dsp_read)
         if (s->mixer_regs[0x82] & 1) {
             ack = 1;
             s->mixer_regs[0x82] &= 1;
-            qemu_irq_lower (s->pic[s->irq]);
+            qemu_irq_lower (s->pic);
         }
         break;
 
@@ -1017,7 +1017,7 @@ static IO_READ_PROTO (dsp_read)
         if (s->mixer_regs[0x82] & 2) {
             ack = 1;
             s->mixer_regs[0x82] &= 2;
-            qemu_irq_lower (s->pic[s->irq]);
+            qemu_irq_lower (s->pic);
         }
         break;
 
@@ -1231,7 +1231,7 @@ static int SB_read_DMA (void *opaque, in
 
     if (s->left_till_irq <= 0) {
         s->mixer_regs[0x82] |= (nchan & 4) ? 2 : 1;
-        qemu_irq_raise (s->pic[s->irq]);
+        qemu_irq_raise (s->pic);
         if (0 == s->dma_auto) {
             control (s, 0);
             speaker (s, 0);
@@ -1408,8 +1408,7 @@ int SB16_init (qemu_irq *pic)
     s = qemu_mallocz (sizeof (*s));
 
     s->cmd = -1;
-    s->pic = pic;
-    s->irq = conf.irq;
+    s->pic = isa_reserve_irq(conf.irq);
     s->dma = conf.dma;
     s->hdma = conf.hdma;
     s->port = conf.port;
#18 lore raw
In reply to Gerd Hoffmann
Quoted 9 lines · Gerd Hoffmann

Attached a patch. It will:

(1) make isa-bus maintain isa irqs, complain when allocating already taken irqs.
(2) note that (1) works only for isa devices converted to qdev already (floppy and ps2/kbd/mouse right now), so more work is needed to make this really useful.
(3) split floppy init into isa and sysbus versions.
(4) add sysbus->isa bridge & fix -M isapc breakage.

It fixes this:

$ qemu tmp.qcow2 -S -M isapc
Tried to create isa device i8042 with no isa bus present.
Segmentation fault (core dumped)