[Qemu-devel] [RFC PATCH 0/3] Add realize unrealize interfaces for BusState

[Qemu-devel] [RFC PATCH 0/3] Add realize unrealize interfaces for BusState

[Bandan Das]

These patches add realize and unrealize interfaces to BusState just
like we have for DeviceState. 1/3 and 2/3 implement the interface and 
attempt to integrate it to the existing DeviceState call path. 
3/3 is an example user of the interface, pci in this case. Please
see individual patches for more details.

Bandan Das (3):
  qdev: add realize/unrealize interfaces for BusState
  qdev: Integrate the bus realized property to get 
    called when device realized property changes
  pci: move vmstate_pcibus registration/unregistration to realize
    and unrealize interfaces

 hw/core/qdev.c         | 130 ++++++++++++++++++++++++++++++++++++++++++++-----
 hw/pci/pci.c           |  49 +++++++++++--------
 include/hw/qdev-core.h |   8 +++
 3 files changed, 153 insertions(+), 34 deletions(-)

-- 
1.8.3.1

[Qemu-devel] [RFC PATCH 1/3] qdev: add realize/unrealize interfaces for BusState

[Bandan Das]

Add simple set/get functions for bus. The setter also checks if it has 
children devices and sets "realize" accordingly.

Signed-off-by: Bandan Das <bsd@redhat.com>
---
 hw/core/qdev.c         | 67 ++++++++++++++++++++++++++++++++++++++++++++++++++
 include/hw/qdev-core.h |  7 ++++++
 2 files changed, 74 insertions(+)

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index e374a93..b503cc8 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -464,6 +464,70 @@ static void bus_unparent(Object *obj)
     }
 }
 
+static bool bus_get_realized(Object *obj, Error **err)
+{
+    BusState *bus = BUS(obj);
+    return bus->realized;
+}
+
+static void bus_set_realized(Object *obj, bool value, Error **err)
+{
+    BusState *bus = BUS(obj);
+    BusClass *bc = BUS_GET_CLASS(bus);
+    Error *local_err = NULL;
+    BusChild *kid;
+
+    if (value && !bus->realized) {
+
+        if (bc->realize) {
+            bc->realize(bus, &local_err);
+
+            if (local_err != NULL) {
+                goto error;
+            }
+
+        }
+
+        QTAILQ_FOREACH(kid, &bus->children, sibling) {
+
+            DeviceState *dev = kid->child;
+            object_property_set_bool(OBJECT(dev), true,
+                                     "realized", &local_err);
+
+            if (local_err != NULL) {
+                goto error;
+            }
+        }
+
+    } else if (!value && bus->realized) {
+
+        QTAILQ_FOREACH(kid, &bus->children, sibling) {
+
+            DeviceState *dev = kid->child;
+            object_property_set_bool(OBJECT(dev), false,
+                                     "realized", &local_err);
+
+            if (local_err != NULL) {
+                goto error;
+            }
+        }
+
+        if (bc->unrealize) {
+            bc->unrealize(bus, &local_err);
+
+            if (local_err != NULL) {
+                goto error;
+            }
+        }
+    }
+
+    bus->realized = value;
+    return;
+
+error:
+    error_propagate(err, local_err);
+}
+
 void qbus_create_inplace(void *bus, size_t size, const char *typename,
                          DeviceState *parent, const char *name)
 {
@@ -868,6 +932,9 @@ static void qbus_initfn(Object *obj)
     BusState *bus = BUS(obj);
 
     QTAILQ_INIT(&bus->children);
+    object_property_add_bool(obj, "realized",
+                             bus_get_realized, bus_set_realized, NULL);
+
 }
 
 static char *default_bus_get_fw_dev_path(DeviceState *dev)
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index f2043a6..405b029 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -35,6 +35,9 @@ typedef int (*qdev_event)(DeviceState *dev);
 typedef void (*qdev_resetfn)(DeviceState *dev);
 typedef void (*DeviceRealize)(DeviceState *dev, Error **errp);
 typedef void (*DeviceUnrealize)(DeviceState *dev, Error **errp);
+typedef void (*BusRealize)(BusState *dev, Error **errp);
+typedef void (*BusUnrealize)(BusState *dev, Error **errp);
+
 
 struct VMStateDescription;
 
@@ -159,6 +162,9 @@ struct BusClass {
      */
     char *(*get_fw_dev_path)(DeviceState *dev);
     int (*reset)(BusState *bus);
+    BusRealize realize;
+    BusUnrealize unrealize;
+
     /* maximum devices allowed on the bus, 0: no limit. */
     int max_dev;
 };
@@ -178,6 +184,7 @@ struct BusState {
     const char *name;
     int allow_hotplug;
     int max_index;
+    bool realized;
     QTAILQ_HEAD(ChildrenHead, BusChild) children;
     QLIST_ENTRY(BusState) sibling;
 };
-- 
1.8.3.1

[Qemu-devel] [RFC PATCH 2/3] qdev: Integrate the bus realized property to get

[Bandan Das]

This introduces three changes - first, it sets the realized property for
BusState in the setter functions for DeviceState so that a call to the 
(device) setter will also set the value for any children buses 
appropriately. Second, it introduces a bool called realized_cache that 
tracks the most recent value of the "realized" property so that an event 
can be sent when the device is being removed. Third, it reorders code
in device_unparent so that unrealize can be recursively called for 
its children

Signed-off-by: Bandan Das <bsd@redhat.com>
---
 hw/core/qdev.c         | 57 +++++++++++++++++++++++++++++++++++++++++---------
 include/hw/qdev-core.h |  1 +
 2 files changed, 48 insertions(+), 10 deletions(-)

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index b503cc8..a51238c 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -740,7 +740,9 @@ static void device_set_realized(Object *obj, bool value, Error **err)
 {
     DeviceState *dev = DEVICE(obj);
     DeviceClass *dc = DEVICE_GET_CLASS(dev);
+    BusState *bus;
     Error *local_err = NULL;
+    bool rcache = dev->realized_cache;
 
     if (value && !dev->realized) {
         if (!obj->parent && local_err == NULL) {
@@ -753,8 +755,13 @@ static void device_set_realized(Object *obj, bool value, Error **err)
             g_free(name);
         }
 
+        dev->realized_cache = true;
+
         if (dc->realize) {
             dc->realize(dev, &local_err);
+            if (local_err != NULL) {
+                goto error;
+            }
         }
 
         if (qdev_get_vmsd(dev) && local_err == NULL) {
@@ -762,24 +769,50 @@ static void device_set_realized(Object *obj, bool value, Error **err)
                                            dev->instance_id_alias,
                                            dev->alias_required_for_version);
         }
+
+       QLIST_FOREACH(bus, &dev->child_bus, sibling) {
+            object_property_set_bool(OBJECT(bus), true,
+                                     "realized", &local_err);
+
+            if (local_err != NULL) {
+                goto error;
+            }
+       }
+
         if (dev->hotplugged && local_err == NULL) {
             device_reset(dev);
         }
+
     } else if (!value && dev->realized) {
+
+        QLIST_FOREACH(bus, &dev->child_bus, sibling) {
+                 object_property_set_bool(OBJECT(bus), false,
+                                          "realized", &local_err);
+
+                if (local_err != NULL) {
+                    goto error;
+                  }
+           }
+
         if (qdev_get_vmsd(dev)) {
             vmstate_unregister(dev, qdev_get_vmsd(dev), dev);
         }
+
         if (dc->unrealize) {
             dc->unrealize(dev, &local_err);
-        }
-    }
 
-    if (local_err != NULL) {
-        error_propagate(err, local_err);
-        return;
+            if (local_err != NULL) {
+                goto error;
+            }
+       }
     }
 
     dev->realized = value;
+    return;
+
+error:
+    dev->realized_cache = rcache;
+    error_propagate(err, local_err);
 }
 
 static void device_initfn(Object *obj)
@@ -796,6 +829,7 @@ static void device_initfn(Object *obj)
 
     dev->instance_id_alias = -1;
     dev->realized = false;
+    dev->realized_cache = false;
 
     object_property_add_bool(obj, "realized",
                              device_get_realized, device_set_realized, NULL);
@@ -854,15 +888,17 @@ static void device_unparent(Object *obj)
     DeviceState *dev = DEVICE(obj);
     BusState *bus;
     QObject *event_data;
-    bool have_realized = dev->realized;
 
+    /* Call this first so that the change can propagate */
+
+    if (dev->realized) {
+        object_property_set_bool(obj, false, "realized", NULL);
+    }
     while (dev->num_child_bus) {
         bus = QLIST_FIRST(&dev->child_bus);
         qbus_free(bus);
     }
-    if (dev->realized) {
-        object_property_set_bool(obj, false, "realized", NULL);
-    }
+
     if (dev->parent_bus) {
         bus_remove_child(dev->parent_bus, dev);
         object_unref(OBJECT(dev->parent_bus));
@@ -870,9 +906,10 @@ static void device_unparent(Object *obj)
     }
 
     /* Only send event if the device had been completely realized */
-    if (have_realized) {
+    if (!dev->realized && dev->realized_cache) {
         gchar *path = object_get_canonical_path(OBJECT(dev));
 
+        dev->realized_cache = false;
         if (dev->id) {
             event_data = qobject_from_jsonf("{ 'device': %s, 'path': %s }",
                                             dev->id, path);
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index 405b029..82f86e7 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -142,6 +142,7 @@ struct DeviceState {
     int num_child_bus;
     int instance_id_alias;
     int alias_required_for_version;
+    bool realized_cache;
 };
 
 #define TYPE_BUS "bus"
-- 
1.8.3.1

[Qemu-devel] [RFC PATCH 3/3] pci: move vmstate_pcibus registration/unregistration to realize and unrealize interfaces

[Bandan Das]

Relocate some stuff to avoid forward declarations and use the 
realize and unrealize hooks to call into register and unregister vmstate_pcibus
respectively

Signed-off-by: Bandan Das <bsd@redhat.com>
---
 hw/pci/pci.c | 49 ++++++++++++++++++++++++++++---------------------
 1 file changed, 28 insertions(+), 21 deletions(-)

diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 49eca95..a43f84f 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -47,7 +47,6 @@ static void pcibus_dev_print(Monitor *mon, DeviceState *dev, int indent);
 static char *pcibus_get_dev_path(DeviceState *dev);
 static char *pcibus_get_fw_dev_path(DeviceState *dev);
 static int pcibus_reset(BusState *qbus);
-static void pci_bus_finalize(Object *obj);
 
 static Property pci_props[] = {
     DEFINE_PROP_PCI_DEVFN("addr", PCIDevice, devfn, -1),
@@ -60,6 +59,32 @@ static Property pci_props[] = {
     DEFINE_PROP_END_OF_LIST()
 };
 
+static const VMStateDescription vmstate_pcibus = {
+    .name = "PCIBUS",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .minimum_version_id_old = 1,
+    .fields      = (VMStateField[]) {
+        VMSTATE_INT32_EQUAL(nirq, PCIBus),
+        VMSTATE_VARRAY_INT32(irq_count, PCIBus,
+                             nirq, 0, vmstate_info_int32,
+                             int32_t),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static void pci_bus_realize(BusState *qbus, Error **errp)
+{
+    PCIBus *bus = DO_UPCAST(PCIBus, qbus, qbus);
+    vmstate_register(NULL, -1, &vmstate_pcibus, bus);
+}
+
+static void pci_bus_unrealize(BusState *qbus, Error **errp)
+{
+    PCIBus *bus = DO_UPCAST(PCIBus, qbus, qbus);
+    vmstate_unregister(NULL, &vmstate_pcibus, bus);
+}
+
 static void pci_bus_class_init(ObjectClass *klass, void *data)
 {
     BusClass *k = BUS_CLASS(klass);
@@ -67,6 +92,8 @@ static void pci_bus_class_init(ObjectClass *klass, void *data)
     k->print_dev = pcibus_dev_print;
     k->get_dev_path = pcibus_get_dev_path;
     k->get_fw_dev_path = pcibus_get_fw_dev_path;
+    k->realize = pci_bus_realize;
+    k->unrealize = pci_bus_unrealize;
     k->reset = pcibus_reset;
 }
 
@@ -74,7 +101,6 @@ static const TypeInfo pci_bus_info = {
     .name = TYPE_PCI_BUS,
     .parent = TYPE_BUS,
     .instance_size = sizeof(PCIBus),
-    .instance_finalize = pci_bus_finalize,
     .class_init = pci_bus_class_init,
 };
 
@@ -94,17 +120,6 @@ static uint16_t pci_default_sub_device_id = PCI_SUBDEVICE_ID_QEMU;
 
 static QLIST_HEAD(, PCIHostState) pci_host_bridges;
 
-static const VMStateDescription vmstate_pcibus = {
-    .name = "PCIBUS",
-    .version_id = 1,
-    .minimum_version_id = 1,
-    .minimum_version_id_old = 1,
-    .fields      = (VMStateField []) {
-        VMSTATE_INT32_EQUAL(nirq, PCIBus),
-        VMSTATE_VARRAY_INT32(irq_count, PCIBus, nirq, 0, vmstate_info_int32, int32_t),
-        VMSTATE_END_OF_LIST()
-    }
-};
 static int pci_bar(PCIDevice *d, int reg)
 {
     uint8_t type;
@@ -300,8 +315,6 @@ static void pci_bus_init(PCIBus *bus, DeviceState *parent,
     QLIST_INIT(&bus->child);
 
     pci_host_bus_register(bus, parent);
-
-    vmstate_register(NULL, -1, &vmstate_pcibus, bus);
 }
 
 bool pci_bus_is_express(PCIBus *bus)
@@ -377,12 +390,6 @@ int pci_bus_num(PCIBus *s)
     return s->parent_dev->config[PCI_SECONDARY_BUS];
 }
 
-static void pci_bus_finalize(Object *obj)
-{
-    PCIBus *bus = PCI_BUS(obj);
-    vmstate_unregister(NULL, &vmstate_pcibus, bus);
-}
-
 static int get_pci_config_device(QEMUFile *f, void *pv, size_t size)
 {
     PCIDevice *s = container_of(pv, PCIDevice, config);
-- 
1.8.3.1

Re: [Qemu-devel] [RFC PATCH 3/3] pci: move vmstate_pcibus registration/unregistration to realize and unrealize interfaces

[Michael S. Tsirkin]

On Mon, Nov 25, 2013 at 05:48:42PM -0500, Bandan Das wrote:
> Relocate some stuff to avoid forward declarations and use the 
> realize and unrealize hooks to call into register and unregister vmstate_pcibus
> respectively
> 
> Signed-off-by: Bandan Das <bsd@redhat.com>

Acked-by: Michael S. Tsirkin <mst@redhat.com>

> ---
>  hw/pci/pci.c | 49 ++++++++++++++++++++++++++++---------------------
>  1 file changed, 28 insertions(+), 21 deletions(-)
> 
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index 49eca95..a43f84f 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -47,7 +47,6 @@ static void pcibus_dev_print(Monitor *mon, DeviceState *dev, int indent);
>  static char *pcibus_get_dev_path(DeviceState *dev);
>  static char *pcibus_get_fw_dev_path(DeviceState *dev);
>  static int pcibus_reset(BusState *qbus);
> -static void pci_bus_finalize(Object *obj);
>  
>  static Property pci_props[] = {
>      DEFINE_PROP_PCI_DEVFN("addr", PCIDevice, devfn, -1),
> @@ -60,6 +59,32 @@ static Property pci_props[] = {
>      DEFINE_PROP_END_OF_LIST()
>  };
>  
> +static const VMStateDescription vmstate_pcibus = {
> +    .name = "PCIBUS",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .minimum_version_id_old = 1,
> +    .fields      = (VMStateField[]) {
> +        VMSTATE_INT32_EQUAL(nirq, PCIBus),
> +        VMSTATE_VARRAY_INT32(irq_count, PCIBus,
> +                             nirq, 0, vmstate_info_int32,
> +                             int32_t),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
> +static void pci_bus_realize(BusState *qbus, Error **errp)
> +{
> +    PCIBus *bus = DO_UPCAST(PCIBus, qbus, qbus);
> +    vmstate_register(NULL, -1, &vmstate_pcibus, bus);
> +}
> +
> +static void pci_bus_unrealize(BusState *qbus, Error **errp)
> +{
> +    PCIBus *bus = DO_UPCAST(PCIBus, qbus, qbus);
> +    vmstate_unregister(NULL, &vmstate_pcibus, bus);
> +}
> +
>  static void pci_bus_class_init(ObjectClass *klass, void *data)
>  {
>      BusClass *k = BUS_CLASS(klass);
> @@ -67,6 +92,8 @@ static void pci_bus_class_init(ObjectClass *klass, void *data)
>      k->print_dev = pcibus_dev_print;
>      k->get_dev_path = pcibus_get_dev_path;
>      k->get_fw_dev_path = pcibus_get_fw_dev_path;
> +    k->realize = pci_bus_realize;
> +    k->unrealize = pci_bus_unrealize;
>      k->reset = pcibus_reset;
>  }
>  
> @@ -74,7 +101,6 @@ static const TypeInfo pci_bus_info = {
>      .name = TYPE_PCI_BUS,
>      .parent = TYPE_BUS,
>      .instance_size = sizeof(PCIBus),
> -    .instance_finalize = pci_bus_finalize,
>      .class_init = pci_bus_class_init,
>  };
>  
> @@ -94,17 +120,6 @@ static uint16_t pci_default_sub_device_id = PCI_SUBDEVICE_ID_QEMU;
>  
>  static QLIST_HEAD(, PCIHostState) pci_host_bridges;
>  
> -static const VMStateDescription vmstate_pcibus = {
> -    .name = "PCIBUS",
> -    .version_id = 1,
> -    .minimum_version_id = 1,
> -    .minimum_version_id_old = 1,
> -    .fields      = (VMStateField []) {
> -        VMSTATE_INT32_EQUAL(nirq, PCIBus),
> -        VMSTATE_VARRAY_INT32(irq_count, PCIBus, nirq, 0, vmstate_info_int32, int32_t),
> -        VMSTATE_END_OF_LIST()
> -    }
> -};
>  static int pci_bar(PCIDevice *d, int reg)
>  {
>      uint8_t type;
> @@ -300,8 +315,6 @@ static void pci_bus_init(PCIBus *bus, DeviceState *parent,
>      QLIST_INIT(&bus->child);
>  
>      pci_host_bus_register(bus, parent);
> -
> -    vmstate_register(NULL, -1, &vmstate_pcibus, bus);
>  }
>  
>  bool pci_bus_is_express(PCIBus *bus)
> @@ -377,12 +390,6 @@ int pci_bus_num(PCIBus *s)
>      return s->parent_dev->config[PCI_SECONDARY_BUS];
>  }
>  
> -static void pci_bus_finalize(Object *obj)
> -{
> -    PCIBus *bus = PCI_BUS(obj);
> -    vmstate_unregister(NULL, &vmstate_pcibus, bus);
> -}
> -
>  static int get_pci_config_device(QEMUFile *f, void *pv, size_t size)
>  {
>      PCIDevice *s = container_of(pv, PCIDevice, config);
> -- 
> 1.8.3.1
> 

Re: [Qemu-devel] [RFC PATCH 0/3] Add realize unrealize interfaces for BusState

[Bandan Das]

Bandan Das <bsd@redhat.com> writes:

> These patches add realize and unrealize interfaces to BusState just
> like we have for DeviceState. 1/3 and 2/3 implement the interface and 
> attempt to integrate it to the existing DeviceState call path. 
> 3/3 is an example user of the interface, pci in this case. Please
> see individual patches for more details.
>
> Bandan Das (3):
>   qdev: add realize/unrealize interfaces for BusState
>   qdev: Integrate the bus realized property to get 
>     called when device realized property changes
>   pci: move vmstate_pcibus registration/unregistration to realize
>     and unrealize interfaces
>
>  hw/core/qdev.c         | 130 ++++++++++++++++++++++++++++++++++++++++++++-----
>  hw/pci/pci.c           |  49 +++++++++++--------
>  include/hw/qdev-core.h |   8 +++
>  3 files changed, 153 insertions(+), 34 deletions(-)

Ping ? Any comments ?

Re: [Qemu-devel] [RFC PATCH 0/3] Add realize unrealize interfaces for BusState

[Andreas Färber]

Am 11.12.2013 16:27, schrieb Bandan Das:
> Bandan Das <bsd@redhat.com> writes:
> 
>> These patches add realize and unrealize interfaces to BusState just
>> like we have for DeviceState. 1/3 and 2/3 implement the interface and 
>> attempt to integrate it to the existing DeviceState call path. 
>> 3/3 is an example user of the interface, pci in this case. Please
>> see individual patches for more details.
>>
>> Bandan Das (3):
>>   qdev: add realize/unrealize interfaces for BusState
>>   qdev: Integrate the bus realized property to get 
>>     called when device realized property changes
>>   pci: move vmstate_pcibus registration/unregistration to realize
>>     and unrealize interfaces
>>
>>  hw/core/qdev.c         | 130 ++++++++++++++++++++++++++++++++++++++++++++-----
>>  hw/pci/pci.c           |  49 +++++++++++--------
>>  include/hw/qdev-core.h |   8 +++
>>  3 files changed, 153 insertions(+), 34 deletions(-)
> 
> Ping ? Any comments ?

I've stripped down patches 1 and 2 and queued the series:
https://github.com/afaerber/qemu-cpu/commits/qom-next

Still doing final testing.

Thanks,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg