[Qemu-devel] [PATCH 0/9] ide: convert to qdev.

[Qemu-devel] [PATCH 0/9] ide: convert to qdev.

[Gerd Hoffmann]

  Hi,

Here is a patch series which starts the conversion of ide to qdev.
It brings the core infrastructure and converts pci+isa ide adapters
to qdev.  Also some minor preparatory patches and bugfixes.

v2 changes:
 * check qdev_init for failure.
 * move whitespace changes to separate patch.
 * drop some needless code reorderings.

cheers,
  Gerd

[Qemu-devel] [PATCH 1/9] qdev/pci: add pci_create_noinit()

[Gerd Hoffmann]

Like pci_create_simple() but doesn't call qdev_init(), so one can
set properties before initializing the device.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/pci.c |   11 ++++++++---
 hw/pci.h |    1 +
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/hw/pci.c b/hw/pci.c
index c12b0be..64d70ed 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -923,15 +923,20 @@ void pci_qdev_register_many(PCIDeviceInfo *info)
     }
 }
 
-PCIDevice *pci_create_simple(PCIBus *bus, int devfn, const char *name)
+PCIDevice *pci_create_noinit(PCIBus *bus, int devfn, const char *name)
 {
     DeviceState *dev;
 
     dev = qdev_create(&bus->qbus, name);
     qdev_prop_set_uint32(dev, "addr", devfn);
-    qdev_init(dev);
+    return DO_UPCAST(PCIDevice, qdev, dev);
+}
 
-    return (PCIDevice *)dev;
+PCIDevice *pci_create_simple(PCIBus *bus, int devfn, const char *name)
+{
+    PCIDevice *dev = pci_create_noinit(bus, devfn, name);
+    qdev_init(&dev->qdev);
+    return dev;
 }
 
 static int pci_find_space(PCIDevice *pdev, uint8_t size)
diff --git a/hw/pci.h b/hw/pci.h
index 6196b6a..e7bf33a 100644
--- a/hw/pci.h
+++ b/hw/pci.h
@@ -328,6 +328,7 @@ void pci_qdev_register(PCIDeviceInfo *info);
 void pci_qdev_register_many(PCIDeviceInfo *info);
 
 PCIDevice *pci_create(const char *name, const char *devaddr);
+PCIDevice *pci_create_noinit(PCIBus *bus, int devfn, const char *name);
 PCIDevice *pci_create_simple(PCIBus *bus, int devfn, const char *name);
 
 /* lsi53c895a.c */
-- 
1.6.2.5

[Qemu-devel] [PATCH 2/9] support media=cdrom for if=none

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 vl.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/vl.c b/vl.c
index 0e61333..eb01da7 100644
--- a/vl.c
+++ b/vl.c
@@ -2176,6 +2176,7 @@ DriveInfo *drive_init(QemuOpts *opts, void *opaque,
     case IF_IDE:
     case IF_SCSI:
     case IF_XEN:
+    case IF_NONE:
         switch(media) {
 	case MEDIA_DISK:
             if (cyls != 0) {
@@ -2196,7 +2197,6 @@ DriveInfo *drive_init(QemuOpts *opts, void *opaque,
         break;
     case IF_PFLASH:
     case IF_MTD:
-    case IF_NONE:
         break;
     case IF_VIRTIO:
         /* add virtio block device */
-- 
1.6.2.5

[Qemu-devel] [PATCH 3/9] split away drive init from ide_init2()

[Gerd Hoffmann]

This allows the ide bus being initialized without drives attached
and the drives being attached and initialization later on as
separate step.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/ide/core.c     |   71 +++++++++++++++++++++++++++++------------------------
 hw/ide/internal.h |    1 +
 2 files changed, 40 insertions(+), 32 deletions(-)

diff --git a/hw/ide/core.c b/hw/ide/core.c
index 5f6e2cb..0b6de6c 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -2518,51 +2518,58 @@ void ide_reset(IDEState *s)
     s->media_changed = 0;
 }
 
+void ide_init_drive(IDEState *s, DriveInfo *dinfo)
+{
+    int cylinders, heads, secs;
+    uint64_t nb_sectors;
+
+    if (dinfo && dinfo->bdrv) {
+        s->bs = dinfo->bdrv;
+        bdrv_get_geometry(s->bs, &nb_sectors);
+        bdrv_guess_geometry(s->bs, &cylinders, &heads, &secs);
+        s->cylinders = cylinders;
+        s->heads = heads;
+        s->sectors = secs;
+        s->nb_sectors = nb_sectors;
+        /* The SMART values should be preserved across power cycles
+           but they aren't.  */
+        s->smart_enabled = 1;
+        s->smart_autosave = 1;
+        s->smart_errors = 0;
+        s->smart_selftest_count = 0;
+        if (bdrv_get_type_hint(s->bs) == BDRV_TYPE_CDROM) {
+            s->is_cdrom = 1;
+            bdrv_set_change_cb(s->bs, cdrom_change_cb, s);
+        }
+        strncpy(s->drive_serial_str, drive_get_serial(s->bs),
+                sizeof(s->drive_serial_str));
+    }
+    if (strlen(s->drive_serial_str) == 0)
+        snprintf(s->drive_serial_str, sizeof(s->drive_serial_str),
+                 "QM%05d", s->drive_serial);
+    ide_reset(s);
+}
+
 void ide_init2(IDEBus *bus, DriveInfo *hd0, DriveInfo *hd1,
                qemu_irq irq)
 {
     IDEState *s;
     static int drive_serial = 1;
-    int i, cylinders, heads, secs;
-    uint64_t nb_sectors;
+    int i;
 
     for(i = 0; i < 2; i++) {
         s = bus->ifs + i;
         s->bus = bus;
         s->unit = i;
-        if (i == 0 && hd0)
-            s->bs = hd0->bdrv;
-        if (i == 1 && hd1)
-            s->bs = hd1->bdrv;
-        s->io_buffer = qemu_blockalign(s->bs, IDE_DMA_BUF_SECTORS*512 + 4);
-        if (s->bs) {
-            bdrv_get_geometry(s->bs, &nb_sectors);
-            bdrv_guess_geometry(s->bs, &cylinders, &heads, &secs);
-            s->cylinders = cylinders;
-            s->heads = heads;
-            s->sectors = secs;
-            s->nb_sectors = nb_sectors;
-	    /* The SMART values should be preserved across power cycles
-	       but they aren't.  */
-	    s->smart_enabled = 1;
-	    s->smart_autosave = 1;
-	    s->smart_errors = 0;
-	    s->smart_selftest_count = 0;
-	    s->smart_selftest_data = qemu_blockalign(s->bs, 512);
-            if (bdrv_get_type_hint(s->bs) == BDRV_TYPE_CDROM) {
-                s->is_cdrom = 1;
-		bdrv_set_change_cb(s->bs, cdrom_change_cb, s);
-            }
-        }
         s->drive_serial = drive_serial++;
-        strncpy(s->drive_serial_str, drive_get_serial(s->bs),
-                sizeof(s->drive_serial_str));
-        if (strlen(s->drive_serial_str) == 0)
-            snprintf(s->drive_serial_str, sizeof(s->drive_serial_str),
-                    "QM%05d", s->drive_serial);
+        s->io_buffer = qemu_blockalign(s->bs, IDE_DMA_BUF_SECTORS*512 + 4);
+        s->smart_selftest_data = qemu_blockalign(s->bs, 512);
         s->sector_write_timer = qemu_new_timer(vm_clock,
                                                ide_sector_write_timer_cb, s);
-        ide_reset(s);
+        if (i == 0)
+            ide_init_drive(s, hd0);
+        if (i == 1)
+            ide_init_drive(s, hd1);
     }
     bus->irq = irq;
 }
diff --git a/hw/ide/internal.h b/hw/ide/internal.h
index b9a7c72..62aef1c 100644
--- a/hw/ide/internal.h
+++ b/hw/ide/internal.h
@@ -527,6 +527,7 @@ uint32_t ide_data_readw(void *opaque, uint32_t addr);
 void ide_data_writel(void *opaque, uint32_t addr, uint32_t val);
 uint32_t ide_data_readl(void *opaque, uint32_t addr);
 
+void ide_init_drive(IDEState *s, DriveInfo *dinfo);
 void ide_init2(IDEBus *bus, DriveInfo *hd0, DriveInfo *hd1,
                qemu_irq irq);
 void ide_init_ioport(IDEBus *bus, int iobase, int iobase2);
-- 
1.6.2.5

[Qemu-devel] [PATCH 4/9] ide/qdev: add ide bus.

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/ide/internal.h |   24 ++++++++++-
 hw/ide/qdev.c     |  124 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 147 insertions(+), 1 deletions(-)
 create mode 100644 hw/ide/qdev.c

diff --git a/hw/ide/internal.h b/hw/ide/internal.h
index 62aef1c..9df759d 100644
--- a/hw/ide/internal.h
+++ b/hw/ide/internal.h
@@ -15,6 +15,8 @@
 #define USE_DMA_CDROM
 
 typedef struct IDEBus IDEBus;
+typedef struct IDEDevice IDEDevice;
+typedef struct IDEDeviceInfo IDEDeviceInfo;
 typedef struct IDEState IDEState;
 typedef struct BMDMAState BMDMAState;
 
@@ -440,6 +442,8 @@ struct IDEState {
 
 struct IDEBus {
     BusState qbus;
+    IDEDevice *master;
+    IDEDevice *slave;
     BMDMAState *bmdma;
     IDEState ifs[2];
     uint8_t unit;
@@ -447,6 +451,20 @@ struct IDEBus {
     qemu_irq irq;
 };
 
+struct IDEDevice {
+    DeviceState qdev;
+    uint32_t unit;
+    DriveInfo *dinfo;
+};
+
+typedef int (*ide_qdev_initfn)(IDEDevice *dev);
+struct IDEDeviceInfo {
+    DeviceInfo qdev;
+    ide_qdev_initfn init;
+    uint32_t unit;
+    DriveInfo *drive;
+};
+
 #define BM_STATUS_DMAING 0x01
 #define BM_STATUS_ERROR  0x02
 #define BM_STATUS_INT    0x04
@@ -500,7 +518,7 @@ static inline void ide_set_irq(IDEBus *bus)
     }
 }
 
-/* ide.c */
+/* hw/ide/core.c */
 void ide_save(QEMUFile* f, IDEState *s);
 void ide_load(QEMUFile* f, IDEState *s, int version_id);
 void idebus_save(QEMUFile* f, IDEBus *bus);
@@ -532,4 +550,8 @@ void ide_init2(IDEBus *bus, DriveInfo *hd0, DriveInfo *hd1,
                qemu_irq irq);
 void ide_init_ioport(IDEBus *bus, int iobase, int iobase2);
 
+/* hw/ide/qdev.c */
+IDEBus *ide_bus_new(DeviceState *dev);
+IDEDevice *ide_create_drive(IDEBus *bus, int unit, DriveInfo *drive);
+
 #endif /* HW_IDE_INTERNAL_H */
diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
new file mode 100644
index 0000000..d467e3a
--- /dev/null
+++ b/hw/ide/qdev.c
@@ -0,0 +1,124 @@
+/*
+ * ide bus support for qdev.
+ *
+ * Copyright (c) 2009 Gerd Hoffmann <kraxel@redhat.com>
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ */
+#include <hw/hw.h>
+#include "sysemu.h"
+#include "dma.h"
+
+#include <hw/ide/internal.h>
+
+/* --------------------------------- */
+
+static struct BusInfo ide_bus_info = {
+    .name  = "IDE",
+    .size  = sizeof(IDEBus),
+};
+
+IDEBus *ide_bus_new(DeviceState *dev)
+{
+    IDEBus *idebus;
+
+    idebus = FROM_QBUS(IDEBus, qbus_create(&ide_bus_info, dev, NULL));
+    return idebus;
+}
+
+static int ide_qdev_init(DeviceState *qdev, DeviceInfo *base)
+{
+    IDEDevice *dev = DO_UPCAST(IDEDevice, qdev, qdev);
+    IDEDeviceInfo *info = DO_UPCAST(IDEDeviceInfo, qdev, base);
+    IDEBus *bus = DO_UPCAST(IDEBus, qbus, qdev->parent_bus);
+
+    if (!dev->dinfo) {
+        fprintf(stderr, "%s: no drive specified\n", qdev->info->name);
+        goto err;
+    }
+    if (dev->unit == -1) {
+        dev->unit = bus->master ? 1 : 0;
+    }
+    switch (dev->unit) {
+    case 0:
+        if (bus->master) {
+            fprintf(stderr, "ide: tried to assign master twice\n");
+            goto err;
+        }
+        bus->master = dev;
+        break;
+    case 1:
+        if (bus->slave) {
+            fprintf(stderr, "ide: tried to assign slave twice\n");
+            goto err;
+        }
+        bus->slave = dev;
+        break;
+    default:
+        goto err;
+    }
+    return info->init(dev);
+
+err:
+    return -1;
+}
+
+static void ide_qdev_register(IDEDeviceInfo *info)
+{
+    info->qdev.init = ide_qdev_init;
+    info->qdev.bus_info = &ide_bus_info;
+    qdev_register(&info->qdev);
+}
+
+IDEDevice *ide_create_drive(IDEBus *bus, int unit, DriveInfo *drive)
+{
+    DeviceState *dev;
+
+    dev = qdev_create(&bus->qbus, "ide-drive");
+    qdev_prop_set_uint32(dev, "unit", unit);
+    qdev_prop_set_drive(dev, "drive", drive);
+    if (qdev_init(dev) != 0)
+        return NULL;
+    return DO_UPCAST(IDEDevice, qdev, dev);
+}
+
+/* --------------------------------- */
+
+typedef struct IDEDrive {
+    IDEDevice dev;
+} IDEDrive;
+
+static int ide_drive_initfn(IDEDevice *dev)
+{
+    IDEBus *bus = DO_UPCAST(IDEBus, qbus, dev->qdev.parent_bus);
+    ide_init_drive(bus->ifs + dev->unit, dev->dinfo);
+    return 0;
+}
+
+static IDEDeviceInfo ide_drive_info = {
+    .qdev.name  = "ide-drive",
+    .qdev.size  = sizeof(IDEDrive),
+    .init       = ide_drive_initfn,
+    .qdev.props = (Property[]) {
+        DEFINE_PROP_UINT32("unit", IDEDrive, dev.unit, -1),
+        DEFINE_PROP_DRIVE("drive", IDEDrive, dev.dinfo),
+        DEFINE_PROP_END_OF_LIST(),
+    }
+};
+
+static void ide_drive_register(void)
+{
+    ide_qdev_register(&ide_drive_info);
+}
+device_init(ide_drive_register);
-- 
1.6.2.5

[Qemu-devel] [PATCH 5/9] ide/pci: fix indention

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/ide/pci.c |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/hw/ide/pci.c b/hw/ide/pci.c
index 607472b..e3b43a0 100644
--- a/hw/ide/pci.c
+++ b/hw/ide/pci.c
@@ -404,15 +404,15 @@ void pci_cmd646_ide_init(PCIBus *bus, DriveInfo **hd_table,
     }
 
     pci_register_bar((PCIDevice *)d, 0, 0x8,
-                           PCI_ADDRESS_SPACE_IO, ide_map);
+                     PCI_ADDRESS_SPACE_IO, ide_map);
     pci_register_bar((PCIDevice *)d, 1, 0x4,
-                           PCI_ADDRESS_SPACE_IO, ide_map);
+                     PCI_ADDRESS_SPACE_IO, ide_map);
     pci_register_bar((PCIDevice *)d, 2, 0x8,
-                           PCI_ADDRESS_SPACE_IO, ide_map);
+                     PCI_ADDRESS_SPACE_IO, ide_map);
     pci_register_bar((PCIDevice *)d, 3, 0x4,
-                           PCI_ADDRESS_SPACE_IO, ide_map);
+                     PCI_ADDRESS_SPACE_IO, ide_map);
     pci_register_bar((PCIDevice *)d, 4, 0x10,
-                           PCI_ADDRESS_SPACE_IO, bmdma_map);
+                     PCI_ADDRESS_SPACE_IO, bmdma_map);
 
     pci_conf[0x3d] = 0x01; // interrupt on pin 1
 
@@ -466,7 +466,7 @@ void pci_piix3_ide_init(PCIBus *bus, DriveInfo **hd_table, int devfn)
     piix3_reset(d);
 
     pci_register_bar((PCIDevice *)d, 4, 0x10,
-                           PCI_ADDRESS_SPACE_IO, bmdma_map);
+                     PCI_ADDRESS_SPACE_IO, bmdma_map);
 
     ide_init2(&d->bus[0], hd_table[0], hd_table[1], isa_reserve_irq(14));
     ide_init2(&d->bus[1], hd_table[2], hd_table[3], isa_reserve_irq(15));
-- 
1.6.2.5

[Qemu-devel] [PATCH 6/9] ide/pci: convert to qdev.

[Gerd Hoffmann]

With this patch applied ide drives (when attached to a pci adapter) can
be created via -device, like this:

  -drive if=none,id=mydisk,file=/path/to/disk.img
  -device ide-drive,drive=mydisk,bus=ide.0,unit=0

Note that creating a master on ide1 doesn't work that way.  That is a
side effect of qemu creating a cdrom automagically even if you don't
ask for it.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 Makefile.target |   10 ++-
 hw/ide/pci.c    |  186 ++++++++++++++++++++++++++++++++++--------------------
 2 files changed, 123 insertions(+), 73 deletions(-)

diff --git a/Makefile.target b/Makefile.target
index 6314e0e..7e841c1 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -194,7 +194,8 @@ obj-y += e1000.o
 obj-y += wdt_i6300esb.o
 
 # Hardware support
-obj-i386-y = ide/core.o ide/isa.o ide/pci.o pckbd.o $(sound-obj-y) dma.o isa-bus.o
+obj-i386-y = ide/core.o ide/qdev.o ide/isa.o ide/pci.o
+obj-i386-y += pckbd.o $(sound-obj-y) dma.o isa-bus.o
 obj-i386-y += vga.o vga-pci.o vga-isa.o
 obj-i386-y += fdc.o mc146818rtc.o serial.o i8259.o i8254.o pcspk.o pc.o
 obj-i386-y += cirrus_vga.o apic.o ioapic.o parallel.o acpi.o piix_pci.o
@@ -203,7 +204,7 @@ obj-i386-y += device-hotplug.o pci-hotplug.o smbios.o wdt_ib700.o
 obj-i386-y += ne2000-isa.o
 
 # shared objects
-obj-ppc-y = ppc.o ide/core.o ide/isa.o ide/pci.o ide/macio.o
+obj-ppc-y = ppc.o ide/core.o ide/qdev.o ide/isa.o ide/pci.o ide/macio.o
 obj-ppc-y += vga.o vga-pci.o $(sound-obj-y) dma.o isa-bus.o openpic.o
 # PREP target
 obj-ppc-y += pckbd.o serial.o i8259.o i8254.o fdc.o mc146818rtc.o
@@ -226,7 +227,7 @@ obj-mips-y = mips_r4k.o mips_jazz.o mips_malta.o mips_mipssim.o
 obj-mips-y += mips_timer.o mips_int.o dma.o vga.o serial.o i8254.o i8259.o rc4030.o
 obj-mips-y += vga-pci.o vga-isa.o vga-isa-mm.o
 obj-mips-y += g364fb.o jazz_led.o dp8393x.o
-obj-mips-y += ide/core.o ide/isa.o ide/pci.o
+obj-mips-y += ide/core.o ide/qdev.o ide/isa.o ide/pci.o
 obj-mips-y += gt64xxx.o pckbd.o fdc.o mc146818rtc.o usb-uhci.o acpi.o ds1225y.o
 obj-mips-y += piix4.o parallel.o cirrus_vga.o isa-bus.o pcspk.o $(sound-obj-y)
 obj-mips-y += mipsnet.o ne2000-isa.o
@@ -258,7 +259,8 @@ obj-cris-y += etraxfs_ser.o
 obj-cris-y += pflash_cfi02.o
 
 ifeq ($(TARGET_ARCH), sparc64)
-obj-sparc-y = sun4u.o ide/core.o ide/pci.o isa-bus.o pckbd.o apb_pci.o
+obj-sparc-y = sun4u.o isa-bus.o pckbd.o apb_pci.o
+obj-sparc-y += ide/core.o ide/qdev.o ide/pci.o
 obj-sparc-y += vga.o vga-pci.o
 obj-sparc-y += fdc.o mc146818rtc.o serial.o
 obj-sparc-y += cirrus_vga.o parallel.o
diff --git a/hw/ide/pci.c b/hw/ide/pci.c
index e3b43a0..89ecd44 100644
--- a/hw/ide/pci.c
+++ b/hw/ide/pci.c
@@ -25,6 +25,7 @@
 #include <hw/hw.h>
 #include <hw/pc.h>
 #include <hw/pci.h>
+#include <hw/isa.h>
 #include "block.h"
 #include "block_int.h"
 #include "sysemu.h"
@@ -50,9 +51,10 @@
 
 typedef struct PCIIDEState {
     PCIDevice dev;
-    IDEBus bus[2];
+    IDEBus *bus[2];
     BMDMAState bmdma[2];
     int type; /* see IDE_TYPE_xxx */
+    uint32_t secondary;
 } PCIIDEState;
 
 static void cmd646_update_irq(PCIIDEState *d);
@@ -64,7 +66,7 @@ static void ide_map(PCIDevice *pci_dev, int region_num,
     IDEBus *bus;
 
     if (region_num <= 3) {
-        bus = &d->bus[(region_num >> 1)];
+        bus = d->bus[(region_num >> 1)];
         if (region_num & 1) {
             register_ioport_read(addr + 2, 1, 1, ide_status_read, bus);
             register_ioport_write(addr + 2, 1, 1, ide_cmd_write, bus);
@@ -250,9 +252,9 @@ static void bmdma_map(PCIDevice *pci_dev, int region_num,
 
     for(i = 0;i < 2; i++) {
         BMDMAState *bm = &d->bmdma[i];
-        d->bus[i].bmdma = bm;
+        d->bus[i]->bmdma = bm;
         bm->pci_dev = DO_UPCAST(PCIIDEState, dev, pci_dev);
-        bm->bus = d->bus+i;
+        bm->bus = d->bus[i];
         qemu_add_vm_change_state_handler(ide_dma_restart_cb, bm);
 
         register_ioport_write(addr, 1, 1, bmdma_cmd_writeb, bm);
@@ -292,13 +294,13 @@ static void pci_ide_save(QEMUFile* f, void *opaque)
 
     /* per IDE interface data */
     for(i = 0; i < 2; i++) {
-        idebus_save(f, &d->bus[i]);
+        idebus_save(f, d->bus[i]);
     }
 
     /* per IDE drive data */
     for(i = 0; i < 2; i++) {
-        ide_save(f, &d->bus[i].ifs[0]);
-        ide_save(f, &d->bus[i].ifs[1]);
+        ide_save(f, &d->bus[i]->ifs[0]);
+        ide_save(f, &d->bus[i]->ifs[1]);
     }
 }
 
@@ -328,17 +330,31 @@ static int pci_ide_load(QEMUFile* f, void *opaque, int version_id)
 
     /* per IDE interface data */
     for(i = 0; i < 2; i++) {
-        idebus_load(f, &d->bus[i], version_id);
+        idebus_load(f, d->bus[i], version_id);
     }
 
     /* per IDE drive data */
     for(i = 0; i < 2; i++) {
-        ide_load(f, &d->bus[i].ifs[0], version_id);
-        ide_load(f, &d->bus[i].ifs[1], version_id);
+        ide_load(f, &d->bus[i]->ifs[0], version_id);
+        ide_load(f, &d->bus[i]->ifs[1], version_id);
     }
     return 0;
 }
 
+static void pci_ide_create_devs(PCIDevice *dev, DriveInfo **hd_table)
+{
+    PCIIDEState *d = DO_UPCAST(PCIIDEState, dev, dev);
+    static const int bus[4]  = { 0, 0, 1, 1 };
+    static const int unit[4] = { 0, 1, 0, 1 };
+    int i;
+
+    for (i = 0; i < 4; i++) {
+        if (hd_table[i] == NULL)
+            continue;
+        ide_create_drive(d->bus[bus[i]], unit[i], hd_table[i]);
+    }
+}
+
 /* XXX: call it also when the MRDMODE is changed from the PCI config
    registers */
 static void cmd646_update_irq(PCIIDEState *d)
@@ -375,19 +391,13 @@ static void cmd646_reset(void *opaque)
 }
 
 /* CMD646 PCI IDE controller */
-void pci_cmd646_ide_init(PCIBus *bus, DriveInfo **hd_table,
-                         int secondary_ide_enabled)
+static int pci_cmd646_ide_initfn(PCIDevice *dev)
 {
-    PCIIDEState *d;
-    uint8_t *pci_conf;
+    PCIIDEState *d = DO_UPCAST(PCIIDEState, dev, dev);
+    uint8_t *pci_conf = d->dev.config;
     qemu_irq *irq;
 
-    d = (PCIIDEState *)pci_register_device(bus, "CMD646 IDE",
-                                           sizeof(PCIIDEState),
-                                           -1,
-                                           NULL, NULL);
     d->type = IDE_TYPE_CMD646;
-    pci_conf = d->dev.config;
     pci_config_set_vendor_id(pci_conf, PCI_VENDOR_ID_CMD);
     pci_config_set_device_id(pci_conf, PCI_DEVICE_ID_CMD_646);
 
@@ -398,7 +408,7 @@ void pci_cmd646_ide_init(PCIBus *bus, DriveInfo **hd_table,
     pci_conf[PCI_HEADER_TYPE] = PCI_HEADER_TYPE_NORMAL; // header_type
 
     pci_conf[0x51] = 0x04; // enable IDE0
-    if (secondary_ide_enabled) {
+    if (d->secondary) {
         /* XXX: if not enabled, really disable the seconday IDE controller */
         pci_conf[0x51] |= 0x08; /* enable IDE1 */
     }
@@ -417,12 +427,27 @@ void pci_cmd646_ide_init(PCIBus *bus, DriveInfo **hd_table,
     pci_conf[0x3d] = 0x01; // interrupt on pin 1
 
     irq = qemu_allocate_irqs(cmd646_set_irq, d, 2);
-    ide_init2(&d->bus[0], hd_table[0], hd_table[1], irq[0]);
-    ide_init2(&d->bus[1], hd_table[2], hd_table[3], irq[1]);
+    d->bus[0] = ide_bus_new(&d->dev.qdev);
+    d->bus[1] = ide_bus_new(&d->dev.qdev);
+    ide_init2(d->bus[0], NULL, NULL, irq[0]);
+    ide_init2(d->bus[1], NULL, NULL, irq[1]);
 
     register_savevm("ide", 0, 3, pci_ide_save, pci_ide_load, d);
     qemu_register_reset(cmd646_reset, d);
     cmd646_reset(d);
+    return 0;
+}
+
+void pci_cmd646_ide_init(PCIBus *bus, DriveInfo **hd_table,
+                         int secondary_ide_enabled)
+{
+    PCIDevice *dev;
+
+    dev = pci_create_noinit(bus, -1, "CMD646 IDE");
+    qdev_prop_set_uint32(&dev->qdev, "secondary", secondary_ide_enabled);
+    qdev_init(&dev->qdev);
+
+    pci_ide_create_devs(dev, hd_table);
 }
 
 static void piix3_reset(void *opaque)
@@ -441,23 +466,10 @@ static void piix3_reset(void *opaque)
     pci_conf[0x20] = 0x01; /* BMIBA: 20-23h */
 }
 
-/* hd_table must contain 4 block drivers */
-/* NOTE: for the PIIX3, the IRQs and IOports are hardcoded */
-void pci_piix3_ide_init(PCIBus *bus, DriveInfo **hd_table, int devfn)
+static int pci_piix_ide_initfn(PCIIDEState *d)
 {
-    PCIIDEState *d;
-    uint8_t *pci_conf;
-
-    /* register a function 1 of PIIX3 */
-    d = (PCIIDEState *)pci_register_device(bus, "PIIX3 IDE",
-                                           sizeof(PCIIDEState),
-                                           devfn,
-                                           NULL, NULL);
-    d->type = IDE_TYPE_PIIX3;
+    uint8_t *pci_conf = d->dev.config;
 
-    pci_conf = d->dev.config;
-    pci_config_set_vendor_id(pci_conf, PCI_VENDOR_ID_INTEL);
-    pci_config_set_device_id(pci_conf, PCI_DEVICE_ID_INTEL_82371SB_1);
     pci_conf[0x09] = 0x80; // legacy ATA mode
     pci_config_set_class(pci_conf, PCI_CLASS_STORAGE_IDE);
     pci_conf[PCI_HEADER_TYPE] = PCI_HEADER_TYPE_NORMAL; // header_type
@@ -468,46 +480,82 @@ void pci_piix3_ide_init(PCIBus *bus, DriveInfo **hd_table, int devfn)
     pci_register_bar((PCIDevice *)d, 4, 0x10,
                      PCI_ADDRESS_SPACE_IO, bmdma_map);
 
-    ide_init2(&d->bus[0], hd_table[0], hd_table[1], isa_reserve_irq(14));
-    ide_init2(&d->bus[1], hd_table[2], hd_table[3], isa_reserve_irq(15));
-    ide_init_ioport(&d->bus[0], 0x1f0, 0x3f6);
-    ide_init_ioport(&d->bus[1], 0x170, 0x376);
-
     register_savevm("ide", 0, 3, pci_ide_save, pci_ide_load, d);
+
+    d->bus[0] = ide_bus_new(&d->dev.qdev);
+    d->bus[1] = ide_bus_new(&d->dev.qdev);
+    ide_init_ioport(d->bus[0], 0x1f0, 0x3f6);
+    ide_init_ioport(d->bus[1], 0x170, 0x376);
+
+    ide_init2(d->bus[0], NULL, NULL, isa_reserve_irq(14));
+    ide_init2(d->bus[1], NULL, NULL, isa_reserve_irq(15));
+    return 0;
 }
 
-/* hd_table must contain 4 block drivers */
-/* NOTE: for the PIIX4, the IRQs and IOports are hardcoded */
-void pci_piix4_ide_init(PCIBus *bus, DriveInfo **hd_table, int devfn)
+static int pci_piix3_ide_initfn(PCIDevice *dev)
 {
-    PCIIDEState *d;
-    uint8_t *pci_conf;
-
-    /* register a function 1 of PIIX4 */
-    d = (PCIIDEState *)pci_register_device(bus, "PIIX4 IDE",
-                                           sizeof(PCIIDEState),
-                                           devfn,
-                                           NULL, NULL);
-    d->type = IDE_TYPE_PIIX4;
+    PCIIDEState *d = DO_UPCAST(PCIIDEState, dev, dev);
 
-    pci_conf = d->dev.config;
-    pci_config_set_vendor_id(pci_conf, PCI_VENDOR_ID_INTEL);
-    pci_config_set_device_id(pci_conf, PCI_DEVICE_ID_INTEL_82371AB);
-    pci_conf[0x09] = 0x80; // legacy ATA mode
-    pci_config_set_class(pci_conf, PCI_CLASS_STORAGE_IDE);
-    pci_conf[PCI_HEADER_TYPE] = PCI_HEADER_TYPE_NORMAL; // header_type
+    d->type = IDE_TYPE_PIIX3;
+    pci_config_set_vendor_id(d->dev.config, PCI_VENDOR_ID_INTEL);
+    pci_config_set_device_id(d->dev.config, PCI_DEVICE_ID_INTEL_82371SB_1);
+    return pci_piix_ide_initfn(d);
+}
 
-    qemu_register_reset(piix3_reset, d);
-    piix3_reset(d);
+static int pci_piix4_ide_initfn(PCIDevice *dev)
+{
+    PCIIDEState *d = DO_UPCAST(PCIIDEState, dev, dev);
 
-    pci_register_bar((PCIDevice *)d, 4, 0x10,
-                           PCI_ADDRESS_SPACE_IO, bmdma_map);
+    d->type = IDE_TYPE_PIIX4;
+    pci_config_set_vendor_id(d->dev.config, PCI_VENDOR_ID_INTEL);
+    pci_config_set_device_id(d->dev.config, PCI_DEVICE_ID_INTEL_82371AB);
+    return pci_piix_ide_initfn(d);
+}
+
+/* hd_table must contain 4 block drivers */
+/* NOTE: for the PIIX3, the IRQs and IOports are hardcoded */
+void pci_piix3_ide_init(PCIBus *bus, DriveInfo **hd_table, int devfn)
+{
+    PCIDevice *dev;
 
-    ide_init2(&d->bus[0], hd_table[0], hd_table[1], isa_reserve_irq(14));
-    ide_init2(&d->bus[1], hd_table[2], hd_table[3], isa_reserve_irq(15));
-    ide_init_ioport(&d->bus[0], 0x1f0, 0x3f6);
-    ide_init_ioport(&d->bus[1], 0x170, 0x376);
+    dev = pci_create_simple(bus, devfn, "PIIX3 IDE");
+    pci_ide_create_devs(dev, hd_table);
+}
 
-    register_savevm("ide", 0, 3, pci_ide_save, pci_ide_load, d);
+/* hd_table must contain 4 block drivers */
+/* NOTE: for the PIIX4, the IRQs and IOports are hardcoded */
+void pci_piix4_ide_init(PCIBus *bus, DriveInfo **hd_table, int devfn)
+{
+    PCIDevice *dev;
+
+    dev = pci_create_simple(bus, devfn, "PIIX4 IDE");
+    pci_ide_create_devs(dev, hd_table);
 }
 
+static PCIDeviceInfo piix_ide_info[] = {
+    {
+        .qdev.name    = "PIIX3 IDE",
+        .qdev.size    = sizeof(PCIIDEState),
+        .init         = pci_piix3_ide_initfn,
+    },{
+        .qdev.name    = "PIIX4 IDE",
+        .qdev.size    = sizeof(PCIIDEState),
+        .init         = pci_piix4_ide_initfn,
+    },{
+        .qdev.name    = "CMD646 IDE",
+        .qdev.size    = sizeof(PCIIDEState),
+        .init         = pci_cmd646_ide_initfn,
+        .qdev.props   = (Property[]) {
+            DEFINE_PROP_UINT32("secondary", PCIIDEState, secondary, 0),
+            DEFINE_PROP_END_OF_LIST(),
+        },
+    },{
+        /* end of list */
+    }
+};
+
+static void piix_ide_register(void)
+{
+    pci_qdev_register_many(piix_ide_info);
+}
+device_init(piix_ide_register);
-- 
1.6.2.5

[Qemu-devel] [PATCH 7/9] ide/isa: convert to qdev.

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/ide.h      |    4 +-
 hw/ide/isa.c  |   61 +++++++++++++++++++++++++++++++++++++++++++++++++-------
 hw/mips_r4k.c |    2 +-
 hw/pc.c       |    3 +-
 hw/ppc_prep.c |    2 +-
 5 files changed, 58 insertions(+), 14 deletions(-)

diff --git a/hw/ide.h b/hw/ide.h
index e0a508b..0e7d540 100644
--- a/hw/ide.h
+++ b/hw/ide.h
@@ -4,8 +4,8 @@
 #include "qdev.h"
 
 /* ide-isa.c */
-void isa_ide_init(int iobase, int iobase2, qemu_irq irq,
-                  DriveInfo *hd0, DriveInfo *hd1);
+int isa_ide_init(int iobase, int iobase2, int isairq,
+                 DriveInfo *hd0, DriveInfo *hd1);
 
 /* ide-pci.c */
 void pci_cmd646_ide_init(PCIBus *bus, DriveInfo **hd_table,
diff --git a/hw/ide/isa.c b/hw/ide/isa.c
index ec2d6fb..d2fe0c0 100644
--- a/hw/ide/isa.c
+++ b/hw/ide/isa.c
@@ -24,6 +24,7 @@
  */
 #include <hw/hw.h>
 #include <hw/pc.h>
+#include <hw/isa.h>
 #include "block.h"
 #include "block_int.h"
 #include "sysemu.h"
@@ -35,7 +36,12 @@
 /* ISA IDE definitions */
 
 typedef struct ISAIDEState {
-    IDEBus *bus;
+    ISADevice dev;
+    IDEBus    *bus;
+    uint32_t  iobase;
+    uint32_t  iobase2;
+    uint32_t  isairq;
+    qemu_irq  irq;
 } ISAIDEState;
 
 static void isa_ide_save(QEMUFile* f, void *opaque)
@@ -57,15 +63,54 @@ static int isa_ide_load(QEMUFile* f, void *opaque, int version_id)
     return 0;
 }
 
-void isa_ide_init(int iobase, int iobase2, qemu_irq irq,
-                  DriveInfo *hd0, DriveInfo *hd1)
+static int isa_ide_initfn(ISADevice *dev)
 {
+    ISAIDEState *s = DO_UPCAST(ISAIDEState, dev, dev);
+
+    s->bus = ide_bus_new(&s->dev.qdev);
+    ide_init_ioport(s->bus, s->iobase, s->iobase2);
+    isa_init_irq(dev, &s->irq, s->isairq);
+    ide_init2(s->bus, NULL, NULL, s->irq);
+    register_savevm("isa-ide", 0, 3, isa_ide_save, isa_ide_load, s);
+    return 0;
+};
+
+int isa_ide_init(int iobase, int iobase2, int isairq,
+                 DriveInfo *hd0, DriveInfo *hd1)
+{
+    ISADevice *dev;
     ISAIDEState *s;
 
-    s = qemu_mallocz(sizeof(*s));
-    s->bus = qemu_mallocz(sizeof(IDEBus));
+    dev = isa_create("isa-ide");
+    qdev_prop_set_uint32(&dev->qdev, "iobase",  iobase);
+    qdev_prop_set_uint32(&dev->qdev, "iobase2", iobase2);
+    qdev_prop_set_uint32(&dev->qdev, "irq",     isairq);
+    if (qdev_init(&dev->qdev) != 0)
+        return -1;
 
-    ide_init2(s->bus, hd0, hd1, irq);
-    ide_init_ioport(s->bus, iobase, iobase2);
-    register_savevm("isa-ide", 0, 3, isa_ide_save, isa_ide_load, s);
+    s = DO_UPCAST(ISAIDEState, dev, dev);
+    if (hd0)
+        ide_create_drive(s->bus, 0, hd0);
+    if (hd1)
+        ide_create_drive(s->bus, 1, hd1);
+    return 0;
+}
+
+static ISADeviceInfo isa_ide_info = {
+    .qdev.name  = "isa-ide",
+    .qdev.size  = sizeof(ISAIDEState),
+    .init       = isa_ide_initfn,
+    .qdev.props = (Property[]) {
+        DEFINE_PROP_HEX32("iobase",  ISAIDEState, iobase,  0x1f0),
+        DEFINE_PROP_HEX32("iobase2", ISAIDEState, iobase2, 0x3f6),
+        DEFINE_PROP_UINT32("irq",    ISAIDEState, isairq,  14),
+        DEFINE_PROP_END_OF_LIST(),
+    },
+};
+
+static void isa_ide_register_devices(void)
+{
+    isa_qdev_register(&isa_ide_info);
 }
+
+device_init(isa_ide_register_devices)
diff --git a/hw/mips_r4k.c b/hw/mips_r4k.c
index d801417..fcc7fed 100644
--- a/hw/mips_r4k.c
+++ b/hw/mips_r4k.c
@@ -274,7 +274,7 @@ void mips_r4k_init (ram_addr_t ram_size,
     }
 
     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], ide_irq[i],
                      hd[MAX_IDE_DEVS * i],
 		     hd[MAX_IDE_DEVS * i + 1]);
 
diff --git a/hw/pc.c b/hw/pc.c
index bb78f0b..58de372 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -1366,8 +1366,7 @@ static void pc_init1(ram_addr_t ram_size,
         pci_piix3_ide_init(pci_bus, hd, piix3_devfn + 1);
     } else {
         for(i = 0; i < MAX_IDE_BUS; i++) {
-            isa_ide_init(ide_iobase[i], ide_iobase2[i],
-                         isa_reserve_irq(ide_irq[i]),
+            isa_ide_init(ide_iobase[i], ide_iobase2[i], ide_irq[i],
 	                 hd[MAX_IDE_DEVS * i], hd[MAX_IDE_DEVS * i + 1]);
         }
     }
diff --git a/hw/ppc_prep.c b/hw/ppc_prep.c
index 1930146..6001c86 100644
--- a/hw/ppc_prep.c
+++ b/hw/ppc_prep.c
@@ -707,7 +707,7 @@ static void ppc_prep_init (ram_addr_t ram_size,
     }
 
     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], ide_irq[i],
                      hd[2 * i],
 		     hd[2 * i + 1]);
     }
-- 
1.6.2.5

Re: [Qemu-devel] [PATCH 7/9] ide/isa: convert to qdev.

[Blue Swirl]

On Mon, Sep 14, 2009 at 6:49 PM, Gerd Hoffmann <kraxel@redhat.com> wrote:
>

Breaks PPC:
gdb ./qemu-system-ppc
GNU gdb 6.8-debian
Copyright (C) 2008 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
and "show warranty" for details.
This GDB was configured as "x86_64-linux-gnu"...
(gdb) r
[Thread debugging using libthread_db enabled]
[New Thread 0x7f63eee2e700 (LWP 8998)]

Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7f63eee2e700 (LWP 8998)]
0x000000000042cdc3 in ide_reset (s=0xec2048) at /src/qemu/hw/ide/core.c:2510
2510        bus->unit = s->unit;
(gdb) bt
#0  0x000000000042cdc3 in ide_reset (s=0xec2048)
    at /src/qemu/hw/ide/core.c:2510
#1  0x00000000004327d4 in pmac_ide_reset (opaque=<value optimized out>)
    at /src/qemu/hw/ide/macio.c:330
#2  0x0000000000432899 in pmac_ide_init (hd_table=<value optimized out>,
    irq=0xf5b268, dbdma=0xf597e0, channel=0x16, dma_irq=0xf5b160)
    at /src/qemu/hw/ide/macio.c:353
#3  0x000000000044ab09 in ppc_heathrow_init (ram_size=0x8000000,
    boot_device=<value optimized out>, kernel_filename=0x0,
    kernel_cmdline=0x5e9d0b "", initrd_filename=0x0,
    cpu_model=<value optimized out>) at /src/qemu/hw/ppc_oldworld.c:337
#4  0x000000000040ca15 in main (argc=0x1, argv=0x7ffff6f5b1f8,
    envp=<value optimized out>) at /src/qemu/vl.c:5742

Re: [Qemu-devel] [PATCH 7/9] ide/isa: convert to qdev.

[Blue Swirl]

On Mon, Sep 14, 2009 at 8:23 PM, Blue Swirl <blauwirbel@gmail.com> wrote:
> On Mon, Sep 14, 2009 at 6:49 PM, Gerd Hoffmann <kraxel@redhat.com> wrote:
>>

Stupid me, it was 4/8 which broke it! Sorry.

Re: [Qemu-devel] [PATCH 7/9] ide/isa: convert to qdev.

[Gerd Hoffmann]

On 09/14/09 19:46, Blue Swirl wrote:
> On Mon, Sep 14, 2009 at 8:23 PM, Blue Swirl<blauwirbel@gmail.com>  wrote:
>> On Mon, Sep 14, 2009 at 6:49 PM, Gerd Hoffmann<kraxel@redhat.com>  wrote:
>>>
>
> Stupid me, it was 4/8 which broke it! Sorry.

Not reproducable.  Can you double-check?

thanks,
   Gerd

Re: [Qemu-devel] [PATCH 7/9] ide/isa: convert to qdev.

[Blue Swirl]

On Tue, Sep 15, 2009 at 12:33 PM, Gerd Hoffmann <kraxel@redhat.com> wrote:
> On 09/14/09 19:46, Blue Swirl wrote:
>>
>> On Mon, Sep 14, 2009 at 8:23 PM, Blue Swirl<blauwirbel@gmail.com>  wrote:
>>>
>>> On Mon, Sep 14, 2009 at 6:49 PM, Gerd Hoffmann<kraxel@redhat.com>  wrote:
>>>>
>>
>> Stupid me, it was 4/8 which broke it! Sorry.
>
> Not reproducable.  Can you double-check?

Strange, I can't reproduce it anymore.

[Qemu-devel] [PATCH 8/9] isa: refine irq reservations

[Gerd Hoffmann]

There are a few cases where IRQ sharing on the ISA bus is used and
possible.  In general only devices of the same kind can do that.
A few use cases:

  * serial lines 1+3 share irq 4
  * serial lines 2+4 share irq 3
  * parallel ports share irq 7
  * ppc/prep: ide ports share irq 13

This patch refines the irq reservation mechanism for the isa bus to
handle those cases.  It keeps track of the driver which owns the IRQ in
question and allows irq sharing for devices handled by the same driver.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/isa-bus.c |   16 +++++++++++++---
 1 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/hw/isa-bus.c b/hw/isa-bus.c
index 4ecc0f8..1d2ca90 100644
--- a/hw/isa-bus.c
+++ b/hw/isa-bus.c
@@ -26,6 +26,7 @@ struct ISABus {
     BusState qbus;
     qemu_irq *irqs;
     uint32_t assigned;
+    DeviceInfo *irq_owner[16];
 };
 static ISABus *isabus;
 
@@ -71,7 +72,9 @@ qemu_irq isa_reserve_irq(int isairq)
         exit(1);
     }
     if (isabus->assigned & (1 << isairq)) {
-        fprintf(stderr, "isa irq %d already assigned\n", isairq);
+        DeviceInfo *owner = isabus->irq_owner[isairq];
+        fprintf(stderr, "isa irq %d already assigned (%s)\n",
+                isairq, owner ? owner->name : "unknown");
         exit(1);
     }
     isabus->assigned |= (1 << isairq);
@@ -82,10 +85,17 @@ void isa_init_irq(ISADevice *dev, qemu_irq *p, int isairq)
 {
     assert(dev->nirqs < ARRAY_SIZE(dev->isairq));
     if (isabus->assigned & (1 << isairq)) {
-        fprintf(stderr, "isa irq %d already assigned\n", isairq);
-        exit(1);
+        DeviceInfo *owner = isabus->irq_owner[isairq];
+        if (owner == dev->qdev.info) {
+            /* irq sharing is ok in case the same driver handles both */;
+        } else {
+            fprintf(stderr, "isa irq %d already assigned (%s)\n",
+                    isairq, owner ? owner->name : "unknown");
+            exit(1);
+        }
     }
     isabus->assigned |= (1 << isairq);
+    isabus->irq_owner[isairq] = dev->qdev.info;
     dev->isairq[dev->nirqs] = isairq;
     *p = isabus->irqs[isairq];
     dev->nirqs++;
-- 
1.6.2.5

Re: [Qemu-devel] [PATCH 8/9] isa: refine irq reservations

[Blue Swirl]

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

On Mon, Sep 14, 2009 at 6:49 PM, Gerd Hoffmann <kraxel@redhat.com> wrote:
> There are a few cases where IRQ sharing on the ISA bus is used and
> possible.  In general only devices of the same kind can do that.
> A few use cases:
>
>  * serial lines 1+3 share irq 4
>  * serial lines 2+4 share irq 3
>  * parallel ports share irq 7
>  * ppc/prep: ide ports share irq 13

There is another ppc/prep case where RTC and m48t59 share irq 8. The
attached patch converts m48t59 to ISA and makes this more visible.

Does sharing of IRQs really have to be fatal? Wasn't ISA bus edge
triggered and so IRQ sharing was a bit unreliable (but not completely
broken) also on real HW?

[-- Attachment #2: 0001-PPC-use-ISA-version-of-m48t59-for-PREP-machine.patch --]
[-- Type: application/mbox, Size: 779 bytes --]

Re: [Qemu-devel] [PATCH 8/9] isa: refine irq reservations

[Markus Armbruster]

Blue Swirl <blauwirbel@gmail.com> writes:

> On Mon, Sep 14, 2009 at 6:49 PM, Gerd Hoffmann <kraxel@redhat.com> wrote:
>> There are a few cases where IRQ sharing on the ISA bus is used and
>> possible.  In general only devices of the same kind can do that.
>> A few use cases:
>>
>>  * serial lines 1+3 share irq 4
>>  * serial lines 2+4 share irq 3
>>  * parallel ports share irq 7
>>  * ppc/prep: ide ports share irq 13
>
> There is another ppc/prep case where RTC and m48t59 share irq 8. The
> attached patch converts m48t59 to ISA and makes this more visible.
>
> Does sharing of IRQs really have to be fatal? Wasn't ISA bus edge
> triggered and so IRQ sharing was a bit unreliable (but not completely
> broken) also on real HW?

If I remember correctly (and that's a considerable if), sufficiently
well-behaved hardware with sufficiently smart drivers could share IRQs.

Re: [Qemu-devel] [PATCH 8/9] isa: refine irq reservations

[Gerd Hoffmann]

On 09/14/09 18:55, Blue Swirl wrote:
> On Mon, Sep 14, 2009 at 6:49 PM, Gerd Hoffmann<kraxel@redhat.com>  wrote:
>> There are a few cases where IRQ sharing on the ISA bus is used and
>> possible.  In general only devices of the same kind can do that.
>> A few use cases:
>>
>>   * serial lines 1+3 share irq 4
>>   * serial lines 2+4 share irq 3
>>   * parallel ports share irq 7
>>   * ppc/prep: ide ports share irq 13
>
> There is another ppc/prep case where RTC and m48t59 share irq 8. The
> attached patch converts m48t59 to ISA and makes this more visible.

Hmm, ok.  So scratch this and maybe use a 'sharing the irq is is fine 
with me' flag instead?

> Does sharing of IRQs really have to be fatal? Wasn't ISA bus edge
> triggered and so IRQ sharing was a bit unreliable (but not completely
> broken) also on real HW?

Well, the fundamental problem is that (1) the drivers must be prepared 
to handle that and (b) the hardware must be designed sanely.  There are 
cases where it works fine (see the list above).  It isn't true in 
general though.  You can't configure two random devices (say sb16 + 
ne2k) to share the same irq and expect everything to work fine.

cheers,
   Gerd

Re: [Qemu-devel] [PATCH 8/9] isa: refine irq reservations

[Blue Swirl]

On Tue, Sep 15, 2009 at 10:23 AM, Gerd Hoffmann <kraxel@redhat.com> wrote:
> On 09/14/09 18:55, Blue Swirl wrote:
>>
>> On Mon, Sep 14, 2009 at 6:49 PM, Gerd Hoffmann<kraxel@redhat.com>  wrote:
>>>
>>> There are a few cases where IRQ sharing on the ISA bus is used and
>>> possible.  In general only devices of the same kind can do that.
>>> A few use cases:
>>>
>>>  * serial lines 1+3 share irq 4
>>>  * serial lines 2+4 share irq 3
>>>  * parallel ports share irq 7
>>>  * ppc/prep: ide ports share irq 13
>>
>> There is another ppc/prep case where RTC and m48t59 share irq 8. The
>> attached patch converts m48t59 to ISA and makes this more visible.
>
> Hmm, ok.  So scratch this and maybe use a 'sharing the irq is is fine with
> me' flag instead?
>
>> Does sharing of IRQs really have to be fatal? Wasn't ISA bus edge
>> triggered and so IRQ sharing was a bit unreliable (but not completely
>> broken) also on real HW?
>
> Well, the fundamental problem is that (1) the drivers must be prepared to
> handle that and (b) the hardware must be designed sanely.  There are cases
> where it works fine (see the list above).  It isn't true in general though.
>  You can't configure two random devices (say sb16 + ne2k) to share the same
> irq and expect everything to work fine.

I wonder if the PREP machine is emulated correctly. I couldn't find
docs about PREP NVRAM or RTC.

[Qemu-devel] [PATCH 9/9] unbreak ppc/prep

[Gerd Hoffmann]

Changes:
  * added isa bus, hooked up to the system bus. Not sure this is correct,
    but 'info pci' lists lists no pci-isa bridge in the machine ...).
  * switches the default cpu to one which actually works.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/ppc_prep.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/hw/ppc_prep.c b/hw/ppc_prep.c
index 6001c86..5392072 100644
--- a/hw/ppc_prep.c
+++ b/hw/ppc_prep.c
@@ -572,7 +572,7 @@ static void ppc_prep_init (ram_addr_t ram_size,
 
     /* init CPUs */
     if (cpu_model == NULL)
-        cpu_model = "default";
+        cpu_model = "602";
     for (i = 0; i < smp_cpus; i++) {
         env = cpu_init(cpu_model);
         if (!env) {
@@ -670,6 +670,9 @@ static void ppc_prep_init (ram_addr_t ram_size,
     }
     i8259 = i8259_init(first_cpu->irq_inputs[PPC6xx_INPUT_INT]);
     pci_bus = pci_prep_init(i8259);
+    /* Hmm, prep has no pci-isa bridge ??? */
+    isa_bus_new(NULL);
+    isa_bus_irqs(i8259);
     //    pci_bus = i440fx_init();
     /* Register 8 MB of ISA IO space (needed for non-contiguous map) */
     PPC_io_memory = cpu_register_io_memory(PPC_prep_io_read,
-- 
1.6.2.5

Re: [Qemu-devel] [PATCH 0/9] ide: convert to qdev.

[Markus Armbruster]

Gerd Hoffmann <kraxel@redhat.com> writes:

>   Hi,
>
> Here is a patch series which starts the conversion of ide to qdev.
> It brings the core infrastructure and converts pci+isa ide adapters
> to qdev.  Also some minor preparatory patches and bugfixes.
>
> v2 changes:
>  * check qdev_init for failure.
>  * move whitespace changes to separate patch.
>  * drop some needless code reorderings.
>
> cheers,
>   Gerd

Looks good to me.  I'd still like to see the relationship between struct
IDEBus members master, slave and ifs[] documented, but that's detail.
ISA interrupt sharing may need further refinement later on.