[Qemu-devel] New device API

[Qemu-devel] New device API

[Paul Brook]

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

I've just pushed out changes implementing a new device API.

This is based on my previous RFC[1], but with a few changes. The main one 
being increased used of proxies/wrappers to implement common bus 
functionality. For example there are frameworks for PCI, "ISA"[1] and I2C 
devices all built on top of a common core.

There's a fair amount still needs to be converted, but I'm reasonably happy 
that it is headed in the right direction. Hopefully most of it should be 
fairly self-explanatory.

Next up is actual config driven machine creation.

Paul

[1] http://lists.gnu.org/archive/html/qemu-devel/2009-05/msg00210.html
[2] In this context ISA == any device connected to the main cpu/system bus.


[-- Attachment #2: Type: text/html, Size: 2078 bytes --]

Re: [Qemu-devel] New device API

[Gerd Hoffmann]

On 05/14/09 23:39, Paul Brook wrote:
> I've just pushed out changes implementing a new device API.
>
> This is based on my previous RFC[1], but with a few changes. The main one
> being increased used of proxies/wrappers to implement common bus
> functionality. For example there are frameworks for PCI, "ISA"[1] and I2C
> devices all built on top of a common core.

I think we should also have a generic BusState with a name and a list of 
devices attached (maybe more).  Then have "BusState *bus" instead of 
"void *bus".  Then you can actually walk the device tree in common code, 
for example have a "info devtree" monitor command printing all devices 
of the machine.

cheers,
   Gerd

Re: [Qemu-devel] New device API

[Gerd Hoffmann]

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

On 05/15/09 11:40, Gerd Hoffmann wrote:
> I think we should also have a generic BusState with a name and a list of
> devices attached (maybe more). Then have "BusState *bus" instead of
> "void *bus".

i.e. something like the attached patch.  It is just a quick outline, far 
from being complete, and with some FIXMEs.  Compiles and works though.

I also think we should switch to sys-queue.h list functions in 
qdev.[ch].  open-coded lists are easy as long as you only add elements, 
but some day we'll have to deal with hot-unplug.

cheers,
   Gerd

[-- Attachment #2: qemu-bus-state.diff --]
[-- Type: text/plain, Size: 5487 bytes --]

diff --git a/hw/i2c.c b/hw/i2c.c
index ce9de29..726f6fc 100644
--- a/hw/i2c.c
+++ b/hw/i2c.c
@@ -11,6 +11,7 @@
 
 struct i2c_bus
 {
+    BusState qbus;
     i2c_slave *current_dev;
     i2c_slave *dev;
     int saved_address;
@@ -161,7 +162,7 @@ DeviceState *i2c_create_slave(i2c_bus *bus, const char *name, int addr)
 {
     DeviceState *dev;
 
-    dev = qdev_create(bus, name);
+    dev = qdev_create(&bus->qbus, name);
     qdev_set_prop_int(dev, "address", addr);
     qdev_init(dev);
     return dev;
diff --git a/hw/pc.c b/hw/pc.c
index ed99488..891511b 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -1115,7 +1115,7 @@ static void pc_init1(ram_addr_t ram_size,
         smbus = piix4_pm_init(pci_bus, piix3_devfn + 3, 0xb100, i8259[9]);
         for (i = 0; i < 8; i++) {
             DeviceState *eeprom;
-            eeprom = qdev_create(smbus, "smbus-eeprom");
+            eeprom = qdev_create((BusState*)smbus /* FIXME: kill cast */, "smbus-eeprom");
             qdev_set_prop_int(eeprom, "address", 0x50 + i);
             qdev_set_prop_ptr(eeprom, "data", eeprom_buf + (i * 256));
             qdev_init(eeprom);
diff --git a/hw/pci.c b/hw/pci.c
index 2b1d396..a4bb173 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -31,6 +31,7 @@
 //#define DEBUG_PCI
 
 struct PCIBus {
+    BusState qbus;
     int bus_num;
     int devfn_min;
     pci_set_irq_fn set_irq;
@@ -822,7 +823,7 @@ PCIDevice *pci_nic_init(PCIBus *bus, NICInfo *nd, int devfn,
 
     for (i = 0; pci_nic_models[i]; i++) {
         if (strcmp(nd->model, pci_nic_models[i]) == 0) {
-            dev = qdev_create(bus, pci_nic_names[i]);
+            dev = qdev_create(&bus->qbus, pci_nic_names[i]);
             qdev_set_prop_int(dev, "devfn", devfn);
             qdev_set_netdev(dev, nd);
             qdev_init(dev);
@@ -927,7 +928,7 @@ PCIDevice *pci_create_simple(PCIBus *bus, int devfn, const char *name)
 {
     DeviceState *dev;
 
-    dev = qdev_create(bus, name);
+    dev = qdev_create(&bus->qbus, name);
     qdev_set_prop_int(dev, "devfn", devfn);
     qdev_init(dev);
 
diff --git a/hw/qdev.c b/hw/qdev.c
index a8de278..9d13672 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -77,7 +77,7 @@ DeviceType *qdev_register(const char *name, int size, qdev_initfn init,
 /* Create a new device.  This only initializes the device state structure
    and allows properties to be set.  qdev_init should be called to
    initialize the actual device emulation.  */
-DeviceState *qdev_create(void *bus, const char *name)
+DeviceState *qdev_create(BusState *bus, const char *name)
 {
     DeviceType *t;
     DeviceState *dev;
@@ -96,6 +96,13 @@ DeviceState *qdev_create(void *bus, const char *name)
     dev->name = name;
     dev->type = t;
     dev->bus = bus;
+
+#if 1
+    /* FIXME: should go to some BusState init function */
+    if (!bus->qdevs.tqh_first)
+        TAILQ_INIT(&bus->qdevs);
+#endif
+    TAILQ_INSERT_TAIL(&bus->qdevs, dev, qbuslist);
     return dev;
 }
 
@@ -279,7 +286,7 @@ void *qdev_get_child_bus(DeviceState *dev, const char *name)
     return NULL;
 }
 
-void qdev_attach_child_bus(DeviceState *dev, const char *name, void *bus)
+void qdev_attach_child_bus(DeviceState *dev, const char *name, BusState *bus)
 {
     ChildBusList *p;
 
diff --git a/hw/qdev.h b/hw/qdev.h
index 210062a..8d8184f 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -2,6 +2,7 @@
 #define QDEV_H
 
 #include "hw.h"
+#include "sys-queue.h"
 
 typedef struct DeviceType DeviceType;
 
@@ -9,13 +10,15 @@ typedef struct DeviceProperty DeviceProperty;
 
 typedef struct ChildBusList ChildBusList;
 
+typedef struct BusState BusState;
+
 /* This structure should not be accessed directly.  We declare it here
    so that it can be embedded in individual device state structures.  */
 struct DeviceState
 {
     const char *name;
     DeviceType *type;
-    void *bus;
+    BusState *bus;
     DeviceProperty *props;
     int num_irq_sink;
     qemu_irq *irq_sink;
@@ -25,11 +28,19 @@ struct DeviceState
     qemu_irq *gpio_in;
     ChildBusList *child_bus;
     NICInfo *nd;
+    TAILQ_ENTRY(DeviceState) qbuslist;
+};
+
+struct BusState
+{
+    char *name;
+    /* BusType ??? */
+    TAILQ_HEAD(qdevs, DeviceState) qdevs;
 };
 
 /*** Board API.  This should go away once we have a machine config file.  ***/
 
-DeviceState *qdev_create(void *bus, const char *name);
+DeviceState *qdev_create(BusState *bus, const char *name);
 void qdev_init(DeviceState *dev);
 
 /* Set properties between creation and init.  */
@@ -56,7 +67,7 @@ DeviceType *qdev_register(const char *name, int size, qdev_initfn init,
 void qdev_init_irq_sink(DeviceState *dev, qemu_irq_handler handler, int nirq);
 void qdev_init_gpio_in(DeviceState *dev, qemu_irq_handler handler, int n);
 void qdev_init_gpio_out(DeviceState *dev, qemu_irq *pins, int n);
-void qdev_attach_child_bus(DeviceState *dev, const char *name, void *bus);
+void qdev_attach_child_bus(DeviceState *dev, const char *name, BusState *bus);
 
 void scsi_bus_new(DeviceState *host, SCSIAttachFn attach);
 
diff --git a/hw/ssi.c b/hw/ssi.c
index 9dfdbe9..70a3862 100644
--- a/hw/ssi.c
+++ b/hw/ssi.c
@@ -10,6 +10,7 @@
 #include "ssi.h"
 
 struct SSIBus {
+    BusState qbus;
     SSISlave *slave;
 };
 
@@ -33,7 +34,7 @@ void ssi_register_slave(const char *name, int size, SSISlaveInfo *info)
 DeviceState *ssi_create_slave(SSIBus *bus, const char *name)
 {
     DeviceState *dev;
-    dev = qdev_create(bus, name);
+    dev = qdev_create(&bus->qbus, name);
     qdev_init(dev);
     return dev;
 }

Re: [Qemu-devel] New device API

[Paul Brook]

On Friday 15 May 2009, Gerd Hoffmann wrote:
> On 05/15/09 11:40, Gerd Hoffmann wrote:
> > I think we should also have a generic BusState with a name and a list of
> > devices attached (maybe more). Then have "BusState *bus" instead of
> > "void *bus".
>
> i.e. something like the attached patch.  It is just a quick outline, far
> from being complete, and with some FIXMEs.  Compiles and works though.

I implemented something similar, plus several resulting cleanups and 
simplifications.

Paul