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

[Gerd Hoffmann <kraxel@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1277 bytes --]

On 08/11/09 16:45, Jes Sorensen wrote:
> On 08/11/2009 04:08 PM, Gerd Hoffmann wrote:
>> On 08/11/09 13:32, Jes Sorensen wrote:
>>> 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 :(

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

[-- Attachment #2: 0001-isabus-fixes.patch --]
[-- Type: text/plain, Size: 10379 bytes --]

>From d5bbd89ee65d5a29816d5f126425d1f26cfeaafe Mon Sep 17 00:00:00 2001
From: Gerd Hoffmann <kraxel@redhat.com>
Date: Tue, 11 Aug 2009 22:27:38 +0200
Subject: [PATCH] isabus fixes


Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 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(-)

diff --git a/hw/fdc.c b/hw/fdc.c
index c55560f..4011cc3 100644
--- a/hw/fdc.c
+++ b/hw/fdc.c
@@ -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;
diff --git a/hw/fdc.h b/hw/fdc.h
index 7b6a9de..04d64ea 100644
--- a/hw/fdc.h
+++ b/hw/fdc.h
@@ -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);
diff --git a/hw/isa-bus.c b/hw/isa-bus.c
index 8c14b1d..fdc42c4 100644
--- a/hw/isa-bus.c
+++ b/hw/isa-bus.c
@@ -18,17 +18,24 @@
  */
 #include "hw.h"
 #include "sysemu.h"
+#include "monitor.h"
+#include "sysbus.h"
 #include "isa.h"
 
 struct ISABus {
     BusState qbus;
+    qemu_irq *irqs;
+    uint32_t assigned;
 };
 static ISABus *isabus;
 
+static void isabus_dev_print(Monitor *mon, DeviceState *dev, int indent);
+
 static struct BusInfo isa_bus_info = {
-    .name  = "ISA",
-    .size  = sizeof(ISABus),
-    .props = (Property[]) {
+    .name      = "ISA",
+    .size      = sizeof(ISABus),
+    .print_dev = isabus_dev_print,
+    .props     = (Property[]) {
         {
             .name   = "iobase",
             .info   = &qdev_prop_hex32,
@@ -50,16 +57,32 @@ ISABus *isa_bus_new(DeviceState *dev)
         fprintf(stderr, "Can't create a second ISA bus\n");
         return NULL;
     }
+    if (NULL == dev) {
+        dev = qdev_create(NULL, "isabus-bridge");
+        qdev_init(dev);
+    }
 
     isabus = FROM_QBUS(ISABus, qbus_create(&isa_bus_info, dev, NULL));
     return isabus;
 }
 
-void isa_connect_irq(ISADevice *dev, int n, qemu_irq irq)
+void isa_bus_irqs(qemu_irq *irqs)
 {
-    assert(n >= 0 && n < dev->nirqs);
-    if (dev->irqs[n])
-        *dev->irqs[n] = irq;
+    isabus->irqs = irqs;
+}
+
+void isa_connect_irq(ISADevice *dev, int devnr, int isairq)
+{
+    assert(devnr >= 0 && devnr < dev->nirqs);
+    if (isabus->assigned & (1 << isairq)) {
+        fprintf(stderr, "isa irq %d already assigned\n", isairq);
+        exit(1);
+    }
+    if (dev->irqs[devnr]) {
+        isabus->assigned |= (1 << isairq);
+        dev->isairq[devnr] = isairq;
+        *dev->irqs[devnr] = isabus->irqs[isairq];
+    }
 }
 
 void isa_init_irq(ISADevice *dev, qemu_irq *p)
@@ -74,6 +97,8 @@ static void isa_qdev_init(DeviceState *qdev, DeviceInfo *base)
     ISADevice *dev = DO_UPCAST(ISADevice, qdev, qdev);
     ISADeviceInfo *info = DO_UPCAST(ISADeviceInfo, qdev, base);
 
+    dev->isairq[0] = -1;
+    dev->isairq[1] = -1;
     info->init(dev);
 }
 
@@ -100,3 +125,34 @@ ISADevice *isa_create_simple(const char *name, uint32_t iobase, uint32_t iobase2
     qdev_init(dev);
     return isa;
 }
+
+static void isabus_dev_print(Monitor *mon, DeviceState *dev, int indent)
+{
+    ISADevice *d = DO_UPCAST(ISADevice, qdev, dev);
+
+    if (d->isairq[1] != -1) {
+        monitor_printf(mon, "%*sisa irqs %d,%d\n", indent, "",
+                       d->isairq[0], d->isairq[1]);
+    } else if (d->isairq[0] != -1) {
+        monitor_printf(mon, "%*sisa irq %d\n", indent, "",
+                       d->isairq[0]);
+    }
+}
+
+static void isabus_bridge_init(SysBusDevice *dev)
+{
+    /* nothing */
+}
+
+static SysBusDeviceInfo isabus_bridge_info = {
+    .init = isabus_bridge_init,
+    .qdev.name  = "isabus-bridge",
+    .qdev.size  = sizeof(SysBusDevice),
+};
+
+static void isabus_register_devices(void)
+{
+    sysbus_register_withprop(&isabus_bridge_info);
+}
+
+device_init(isabus_register_devices)
diff --git a/hw/isa.h b/hw/isa.h
index 49c58f8..1a3bb5b 100644
--- a/hw/isa.h
+++ b/hw/isa.h
@@ -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);
diff --git a/hw/mips_jazz.c b/hw/mips_jazz.c
index e09a6b2..e283b7a 100644
--- a/hw/mips_jazz.c
+++ b/hw/mips_jazz.c
@@ -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);
diff --git a/hw/mips_malta.c b/hw/mips_malta.c
index 4e51c8d..1ac4c41 100644
--- a/hw/mips_malta.c
+++ b/hw/mips_malta.c
@@ -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
diff --git a/hw/pc.c b/hw/pc.c
index cc6e7e8..88a9ef8 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -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);
 
diff --git a/hw/ppc_prep.c b/hw/ppc_prep.c
index 97190a2..51a3df9 100644
--- a/hw/ppc_prep.c
+++ b/hw/ppc_prep.c
@@ -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);
diff --git a/hw/sun4u.c b/hw/sun4u.c
index 77164ca..6a3a1f9 100644
--- a/hw/sun4u.c
+++ b/hw/sun4u.c
@@ -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;
-- 
1.6.2.5