Patchwork [4/8] ide/qdev: add ide bus.

login
register
mail settings
Submitter Gerd Hoffmann
Date Sept. 11, 2009, 12:32 p.m.
Message ID <1252672351-12937-5-git-send-email-kraxel@redhat.com>
Download mbox | patch
Permalink /patch/33459/
State Superseded
Headers show

Comments

Gerd Hoffmann - Sept. 11, 2009, 12:32 p.m.
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/ide/internal.h |   24 ++++++++++-
 hw/ide/qdev.c     |  123 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 146 insertions(+), 1 deletions(-)
 create mode 100644 hw/ide/qdev.c
Gerd Hoffmann - Sept. 11, 2009, 2:10 p.m.
>>   struct IDEBus {
>>       BusState qbus;
>> +    IDEDevice *master;
>> +    IDEDevice *slave;
>
> Can this ones be embedded, not pointers, please.

No.

Also note that vmstate doesn't has to follow these pointers, so you have 
no reason in the first place ;)

cheers,
   Gerd
Markus Armbruster - Sept. 11, 2009, 2:54 p.m.
Gerd Hoffmann <kraxel@redhat.com> writes:

> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  hw/ide/internal.h |   24 ++++++++++-
>  hw/ide/qdev.c     |  123 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 146 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;

master corresponds to ifs[0], and slave to ifs[1].  Document?

> @@ -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..6026c28
> --- /dev/null
> +++ b/hw/ide/qdev.c
> @@ -0,0 +1,123 @@
> +/*
> + * 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);

Is always ide_drive_initfn() so far, but more callbacks come in later in
the series.

> +
> +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);
> +    qdev_init(dev);

Callback dev->info is ide_qdev_init(), which can fail, so qdev_init()
can fail, can't it?

> +    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);
Gerd Hoffmann - Sept. 11, 2009, 3:01 p.m.
>> +    qdev_init(dev);
>
> Callback dev->info is ide_qdev_init(), which can fail, so qdev_init()
> can fail, can't it?

Indeed, will fix.

cheers,
   Gerd

Patch

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..6026c28
--- /dev/null
+++ b/hw/ide/qdev.c
@@ -0,0 +1,123 @@ 
+/*
+ * 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);
+    qdev_init(dev);
+    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);