[Qemu-devel] [RFC v1 00/25] Memory and GPIO QOMification

[Qemu-devel] [RFC v1 00/25] Memory and GPIO QOMification

[Peter Crosthwaite]

Hi All,

This series is my Memory and GPIO qomification as promised. This was
discussed at length on the KVM call 2014-05-13.

This is the preview RFC designed to show the capability of the full
solution. There are many parts to this that stand in their own right and
the merge would be done in smaller chunks (E.G. Memory and GPIO works
can go separately).

Theres some Microblaze stuff at the back of the series, leading to an
example of sysbus hotplug.

Some patches are already on list. I put those at the front of the
series.

Only build tested for microblaze. There's some periphery build issues
for fully configured builds that i'll figure should we get some
conceptual level acceptance of this whole idea.

configure --target-list=microblaze-softmmu

to play with it.

Contents:

QOMify Memory regions. So they are added as child objects to devices.
Devices can do this explicitly in instance_init, or sysbus can handle
it - sysbus_init_mmio parents the memory region to the device to
transparently convert all existing devs to compliance.

This prepares support for the Memory heirachy work. Machines can
create memory regions matching the bus architectures. Masters and
slaves can gen hand these to devices as proper Links.

The address and container (the memory region containing this one as a
subregion) of memory regions are QOM properties, of type integer and
link resp. Setting the properties does the
memory_region_add_subregion().

The root Memory Region is parented the machine in exec.c. This give
the Memory Region a canonical path.

Sysbus IRQ are abandoned completely and re-implemented as named GPIOs.
The sysbus irq API remains and makes this transition
behind-the-scenes.

GPIOs are QOMified. qemu_allocate_irqs does the object_new() etc. IRQ
inputs are then added as child objects of the instantiating device.
Handled by qemu_init_gpio_in_named(). gpio_outs are setup as links.
qdev_connect_gpio_out does the linkage.

QOM is patched to allow setting of a device's child's properties from
a ref to the parent. Best illustrated by example (see below).

Anyways without a single patch to the command line interface, HMP,
QMP, or implementing any machine specific hotplug or adding any new
special busses, this works:

-device xlnx.xps-timer,\
sysbus-mr-0/container=/machine/sysmem,\
sysbus-mr-0/addr=0x83c00000,\
sysbus-irq-0=/machine/intc/unnamed-gpio-0

All the other ways to create devices should just work, this is not a
command line specific feature.

Note, I edited my machine model to sanitize the canonical path of the
interrupt controller.

--- a/hw/microblaze/petalogix_s3adsp1800_mmu.c
+++ b/hw/microblaze/petalogix_s3adsp1800_mmu.c
@@ -97,6 +97,8 @@ petalogix_s3adsp1800_init(QEMUMachineInitArgs *args)
                           1, 0x89, 0x18, 0x0000, 0x0, 1);

     dev = qdev_create(NULL, "xlnx.xps-intc");
+    object_property_add_child(qdev_get_machine(), "intc", OBJECT(dev),
+                              &error_abort);

But you could have done the whole /machine/unattached/... ugliness
too. If you name the irq inputs in the intc with my named GPIO series
stuff the unnamed-gpio ugliness goes away too. If you throw away
sysbus and do the memory-regions and IRQs naming the child objects
properly the ugly sysbus names can de dispensed with too. But thats a
big tree-wide to name everything properly.


Andreas Färber (1):
  irq: Slim conversion of qemu_irq to QOM [WIP]

Peter Crosthwaite (24):
  qdev: Implement named GPIOs
  memory: Simplify mr_add_subregion() if-else
  memory: Coreify subregion add functionality
  memory: MemoryRegion: QOMify
  memory: MemoryRegion: Add contained flag
  memory: MemoryRegion: Add container and addr props
  memory: MemoryRegion: factor out memory region re-adder
  memory: MemoryRegion: Add may-overlap and priority props
  memory: MemoryRegion: Add size property
  exec: Parent root MRs to the machine
  qdev: gpio: Don't allow name share between I and O
  qdev: gpio: Register GPIO inputs as child objects
  qdev: gpio: Register GPIO outputs as QOM links
  qdev: gpio: Re-impement qdev_connect_gpio QOM style
  qom: object_property_set/get: Add child recursion
  sysbus: Use TYPE_DEVICE GPIO functionality
  sysbus: Rework sysbus_mmio_map to use mr QOMification
  sysbus: Setup memory regions as dynamic props
  sysbus: Enable hotplug.
  microblaze: s3adsp: Expand UART creator
  microblaze: s3adsp: Parent devices with sane names
  timer: xilinx_timer: Convert to realize()
  timer: xilinx_timer: init MMIO ASAP
  TEST: microblaze: s3adsp: Remove timer

 exec.c                                   |   4 +
 hw/core/irq.c                            |  44 +++++-
 hw/core/qdev.c                           | 107 +++++++++++--
 hw/core/sysbus.c                         |  85 ++++------
 hw/microblaze/petalogix_s3adsp1800_mmu.c |  16 +-
 hw/timer/xilinx_timer.c                  |  26 ++--
 include/exec/memory.h                    |   9 +-
 include/hw/irq.h                         |   2 +
 include/hw/qdev-core.h                   |  24 ++-
 include/hw/sysbus.h                      |  12 +-
 memory.c                                 | 258 +++++++++++++++++++++++++------
 qdev-monitor.c                           |  16 +-
 qom/object.c                             |  27 +++-
 qtest.c                                  |  19 ++-
 14 files changed, 500 insertions(+), 149 deletions(-)

-- 
1.9.3.1.ga73a6ad

[Qemu-devel] [RFC v1 01/25] qdev: Implement named GPIOs

[Peter Crosthwaite]

Implement named GPIOs on the Device layer. Listifies the existing GPIOs
stuff using string keys. Legacy un-named GPIOs are preserved by using
a NULL name string - they are just a single matchable element in the
name list.

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---
Changed since v3 (PMM review):
Added finalize() cleanup of all allocated mem
Remove %s printf of NULL in info qtree
Restricted Qtest to unnamed GPIOs only
Changed since v2:
Reordered fn args to be name-before-number (PMM review)
changed since v1:
Use QLIST instead of RYO list implementation (AF review)
Reorder struct fields for consistency

 hw/core/qdev.c         | 82 ++++++++++++++++++++++++++++++++++++++++++++------
 include/hw/qdev-core.h | 24 ++++++++++++---
 qdev-monitor.c         | 16 +++++++---
 qtest.c                | 19 +++++++++---
 4 files changed, 117 insertions(+), 24 deletions(-)

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 936eae6..8efeb04 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -312,30 +312,82 @@ BusState *qdev_get_parent_bus(DeviceState *dev)
     return dev->parent_bus;
 }
 
+static NamedGPIOList *qdev_get_named_gpio_list(DeviceState *dev,
+                                               const char *name)
+{
+    NamedGPIOList *ngl;
+
+    QLIST_FOREACH(ngl, &dev->gpios, node) {
+        /* NULL is a valid and matchable name, otherwise do a normal
+         * strcmp match.
+         */
+        if ((!ngl->name && !name) ||
+                (name && ngl->name && !strcmp(name, ngl->name))) {
+            return ngl;
+        }
+    }
+
+    ngl = g_malloc0(sizeof(*ngl));
+    ngl->name = g_strdup(name);
+    QLIST_INSERT_HEAD(&dev->gpios, ngl, node);
+    return ngl;
+}
+
+void qdev_init_gpio_in_named(DeviceState *dev, qemu_irq_handler handler,
+                             const char *name, int n)
+{
+    NamedGPIOList *gpio_list = qdev_get_named_gpio_list(dev, name);
+
+    gpio_list->in = qemu_extend_irqs(gpio_list->in, gpio_list->num_in, handler,
+                                     dev, n);
+    gpio_list->num_in += n;
+}
+
 void qdev_init_gpio_in(DeviceState *dev, qemu_irq_handler handler, int n)
 {
-    dev->gpio_in = qemu_extend_irqs(dev->gpio_in, dev->num_gpio_in, handler,
-                                        dev, n);
-    dev->num_gpio_in += n;
+    qdev_init_gpio_in_named(dev, handler, NULL, n);
+}
+
+void qdev_init_gpio_out_named(DeviceState *dev, qemu_irq *pins,
+                              const char *name, int n)
+{
+    NamedGPIOList *gpio_list = qdev_get_named_gpio_list(dev, name);
+
+    assert(gpio_list->num_out == 0);
+    gpio_list->num_out = n;
+    gpio_list->out = pins;
 }
 
 void qdev_init_gpio_out(DeviceState *dev, qemu_irq *pins, int n)
 {
-    assert(dev->num_gpio_out == 0);
-    dev->num_gpio_out = n;
-    dev->gpio_out = pins;
+    qdev_init_gpio_out_named(dev, pins, NULL, n);
+}
+
+qemu_irq qdev_get_gpio_in_named(DeviceState *dev, const char *name, int n)
+{
+    NamedGPIOList *gpio_list = qdev_get_named_gpio_list(dev, name);
+
+    assert(n >= 0 && n < gpio_list->num_in);
+    return gpio_list->in[n];
 }
 
 qemu_irq qdev_get_gpio_in(DeviceState *dev, int n)
 {
-    assert(n >= 0 && n < dev->num_gpio_in);
-    return dev->gpio_in[n];
+    return qdev_get_gpio_in_named(dev, NULL, n);
+}
+
+void qdev_connect_gpio_out_named(DeviceState *dev, const char *name, int n,
+                                 qemu_irq pin)
+{
+    NamedGPIOList *gpio_list = qdev_get_named_gpio_list(dev, name);
+
+    assert(n >= 0 && n < gpio_list->num_out);
+    gpio_list->out[n] = pin;
 }
 
 void qdev_connect_gpio_out(DeviceState * dev, int n, qemu_irq pin)
 {
-    assert(n >= 0 && n < dev->num_gpio_out);
-    dev->gpio_out[n] = pin;
+    qdev_connect_gpio_out_named(dev, NULL, n, pin);
 }
 
 BusState *qdev_get_child_bus(DeviceState *dev, const char *name)
@@ -844,6 +896,7 @@ static void device_initfn(Object *obj)
     object_property_add_link(OBJECT(dev), "parent_bus", TYPE_BUS,
                              (Object **)&dev->parent_bus, NULL, 0,
                              &error_abort);
+    QLIST_INIT(&dev->gpios);
 }
 
 static void device_post_init(Object *obj)
@@ -854,10 +907,19 @@ static void device_post_init(Object *obj)
 /* Unlink device from bus and free the structure.  */
 static void device_finalize(Object *obj)
 {
+    NamedGPIOList *ngl, *next;
+
     DeviceState *dev = DEVICE(obj);
     if (dev->opts) {
         qemu_opts_del(dev->opts);
     }
+
+    QLIST_FOREACH_SAFE(ngl, &dev->gpios, node, next) {
+        QLIST_REMOVE(ngl, node);
+        qemu_free_irqs(ngl->in);
+        g_free(ngl->name);
+        g_free(ngl);
+    }
 }
 
 static void device_class_base_init(ObjectClass *class, void *data)
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index dbe473c..ae31575 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -131,6 +131,17 @@ typedef struct DeviceClass {
     const char *bus_type;
 } DeviceClass;
 
+typedef struct NamedGPIOList NamedGPIOList;
+
+struct NamedGPIOList {
+    char *name;
+    qemu_irq *in;
+    int num_in;
+    qemu_irq *out;
+    int num_out;
+    QLIST_ENTRY(NamedGPIOList) node;
+};
+
 /**
  * DeviceState:
  * @realized: Indicates whether the device has been fully constructed.
@@ -148,10 +159,7 @@ struct DeviceState {
     QemuOpts *opts;
     int hotplugged;
     BusState *parent_bus;
-    int num_gpio_out;
-    qemu_irq *gpio_out;
-    int num_gpio_in;
-    qemu_irq *gpio_in;
+    QLIST_HEAD(, NamedGPIOList) gpios;
     QLIST_HEAD(, BusState) child_bus;
     int num_child_bus;
     int instance_id_alias;
@@ -252,7 +260,11 @@ void qdev_machine_creation_done(void);
 bool qdev_machine_modified(void);
 
 qemu_irq qdev_get_gpio_in(DeviceState *dev, int n);
+qemu_irq qdev_get_gpio_in_named(DeviceState *dev, const char *name, int n);
+
 void qdev_connect_gpio_out(DeviceState *dev, int n, qemu_irq pin);
+void qdev_connect_gpio_out_named(DeviceState *dev, const char *name, int n,
+                                 qemu_irq pin);
 
 BusState *qdev_get_child_bus(DeviceState *dev, const char *name);
 
@@ -262,6 +274,10 @@ BusState *qdev_get_child_bus(DeviceState *dev, const char *name);
 /* GPIO inputs also double as IRQ sinks.  */
 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_init_gpio_in_named(DeviceState *dev, qemu_irq_handler handler,
+                             const char *name, int n);
+void qdev_init_gpio_out_named(DeviceState *dev, qemu_irq *pins,
+                              const char *name, int n);
 
 BusState *qdev_get_parent_bus(DeviceState *dev);
 
diff --git a/qdev-monitor.c b/qdev-monitor.c
index 02cbe43..f87f3d8 100644
--- a/qdev-monitor.c
+++ b/qdev-monitor.c
@@ -613,14 +613,20 @@ static void qdev_print(Monitor *mon, DeviceState *dev, int indent)
 {
     ObjectClass *class;
     BusState *child;
+    NamedGPIOList *ngl;
+
     qdev_printf("dev: %s, id \"%s\"\n", object_get_typename(OBJECT(dev)),
                 dev->id ? dev->id : "");
     indent += 2;
-    if (dev->num_gpio_in) {
-        qdev_printf("gpio-in %d\n", dev->num_gpio_in);
-    }
-    if (dev->num_gpio_out) {
-        qdev_printf("gpio-out %d\n", dev->num_gpio_out);
+    QLIST_FOREACH(ngl, &dev->gpios, node) {
+        if (ngl->num_in) {
+            qdev_printf("gpio-in \"%s\" %d\n", ngl->name ? ngl->name : "",
+                        ngl->num_in);
+        }
+        if (ngl->num_out) {
+            qdev_printf("gpio-out \"%s\" %d\n", ngl->name ? ngl->name : "",
+                        ngl->num_out);
+        }
     }
     class = object_get_class(OBJECT(dev));
     do {
diff --git a/qtest.c b/qtest.c
index 2aba20d..b6b60d5 100644
--- a/qtest.c
+++ b/qtest.c
@@ -233,7 +233,8 @@ static void qtest_process_command(CharDriverState *chr, gchar **words)
     g_assert(command);
     if (strcmp(words[0], "irq_intercept_out") == 0
         || strcmp(words[0], "irq_intercept_in") == 0) {
-	DeviceState *dev;
+        DeviceState *dev;
+        NamedGPIOList *ngl;
 
         g_assert(words[1]);
         dev = DEVICE(object_resolve_path(words[1], NULL));
@@ -253,10 +254,18 @@ static void qtest_process_command(CharDriverState *chr, gchar **words)
 	    return;
         }
 
-        if (words[0][14] == 'o') {
-            qemu_irq_intercept_out(&dev->gpio_out, qtest_irq_handler, dev->num_gpio_out);
-        } else {
-            qemu_irq_intercept_in(dev->gpio_in, qtest_irq_handler, dev->num_gpio_in);
+        QLIST_FOREACH(ngl, &dev->gpios, node) {
+            /* We dont support intercept of named GPIOs yet */
+            if (ngl->name) {
+                continue;
+            }
+            if (words[0][14] == 'o') {
+                qemu_irq_intercept_out(&ngl->out, qtest_irq_handler,
+                                       ngl->num_out);
+            } else {
+                qemu_irq_intercept_in(ngl->in, qtest_irq_handler,
+                                      ngl->num_in);
+            }
         }
         irq_intercept_dev = dev;
         qtest_send_prefix(chr);
-- 
1.9.3.1.ga73a6ad

[Qemu-devel] [RFC v1 02/25] memory: Simplify mr_add_subregion() if-else

[Peter Crosthwaite]

This if else is not needed. The previous call to memory_region_add
(whether _overlap or not) will always set priority and may_overlap
to desired values. And its not possible to get here without having
called memory_region_add_subregion due to the null guard on parent.
So we can just directly call memory_region_add_subregion_common.

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---

 memory.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/memory.c b/memory.c
index 3f1df23..1352881 100644
--- a/memory.c
+++ b/memory.c
@@ -1501,8 +1501,6 @@ void memory_region_set_enabled(MemoryRegion *mr, bool enabled)
 void memory_region_set_address(MemoryRegion *mr, hwaddr addr)
 {
     MemoryRegion *parent = mr->parent;
-    int priority = mr->priority;
-    bool may_overlap = mr->may_overlap;
 
     if (addr == mr->addr || !parent) {
         mr->addr = addr;
@@ -1512,11 +1510,7 @@ void memory_region_set_address(MemoryRegion *mr, hwaddr addr)
     memory_region_transaction_begin();
     memory_region_ref(mr);
     memory_region_del_subregion(parent, mr);
-    if (may_overlap) {
-        memory_region_add_subregion_overlap(parent, addr, mr, priority);
-    } else {
-        memory_region_add_subregion(parent, addr, mr);
-    }
+    memory_region_add_subregion_common(parent, addr, mr);
     memory_region_unref(mr);
     memory_region_transaction_commit();
 }
-- 
1.9.3.1.ga73a6ad

[Qemu-devel] [RFC v1 03/25] memory: Coreify subregion add functionality

[Peter Crosthwaite]

Split off the core looping code that actually adds subregions into
it's own fn. This prepares support for Memory Region qomification
where setting the MR address or parent via QOM will back onto this more
minimal function.

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---

 memory.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/memory.c b/memory.c
index 1352881..dd0a576 100644
--- a/memory.c
+++ b/memory.c
@@ -1410,18 +1410,15 @@ void memory_region_del_eventfd(MemoryRegion *mr,
     memory_region_transaction_commit();
 }
 
-static void memory_region_add_subregion_common(MemoryRegion *mr,
-                                               hwaddr offset,
-                                               MemoryRegion *subregion)
+static void do_memory_region_add_subregion_common(MemoryRegion *subregion)
 {
+    hwaddr offset = subregion->addr;
+    MemoryRegion *mr = subregion->parent;
     MemoryRegion *other;
 
     memory_region_transaction_begin();
 
-    assert(!subregion->parent);
     memory_region_ref(subregion);
-    subregion->parent = mr;
-    subregion->addr = offset;
     QTAILQ_FOREACH(other, &mr->subregions, subregions_link) {
         if (subregion->may_overlap || other->may_overlap) {
             continue;
@@ -1455,6 +1452,15 @@ done:
     memory_region_transaction_commit();
 }
 
+static void memory_region_add_subregion_common(MemoryRegion *mr,
+                                               hwaddr offset,
+                                               MemoryRegion *subregion)
+{
+    assert(!subregion->parent);
+    subregion->parent = mr;
+    subregion->addr = offset;
+    do_memory_region_add_subregion_common(subregion);
+}
 
 void memory_region_add_subregion(MemoryRegion *mr,
                                  hwaddr offset,
-- 
1.9.3.1.ga73a6ad

[Qemu-devel] [RFC v1 04/25] memory: MemoryRegion: QOMify

[Peter Crosthwaite]

QOMify memory regions as an Object. The former init() and destroy()
routines become instance_init() and instance_finalize() resp.

memory_region_init() is re-implemented to be:
object_initialize() + set fields

memory_region_destroy() is re-implemented to call finalize().

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---

 include/exec/memory.h |  6 ++++++
 memory.c              | 55 +++++++++++++++++++++++++++++++++------------------
 2 files changed, 42 insertions(+), 19 deletions(-)

diff --git a/include/exec/memory.h b/include/exec/memory.h
index 1d55ad9..371c066 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -31,10 +31,15 @@
 #include "qemu/queue.h"
 #include "qemu/int128.h"
 #include "qemu/notify.h"
+#include "qom/object.h"
 
 #define MAX_PHYS_ADDR_SPACE_BITS 62
 #define MAX_PHYS_ADDR            (((hwaddr)1 << MAX_PHYS_ADDR_SPACE_BITS) - 1)
 
+#define TYPE_MEMORY_REGION "qemu:memory-region"
+#define MEMORY_REGION(obj) \
+        OBJECT_CHECK(MemoryRegion, (obj), TYPE_MEMORY_REGION)
+
 typedef struct MemoryRegionOps MemoryRegionOps;
 typedef struct MemoryRegionMmio MemoryRegionMmio;
 
@@ -130,6 +135,7 @@ typedef struct CoalescedMemoryRange CoalescedMemoryRange;
 typedef struct MemoryRegionIoeventfd MemoryRegionIoeventfd;
 
 struct MemoryRegion {
+    Object parent_obj;
     /* All fields are private - violators will be prosecuted */
     const MemoryRegionOps *ops;
     const MemoryRegionIOMMUOps *iommu_ops;
diff --git a/memory.c b/memory.c
index dd0a576..3c32d5a 100644
--- a/memory.c
+++ b/memory.c
@@ -833,35 +833,28 @@ void memory_region_init(MemoryRegion *mr,
                         const char *name,
                         uint64_t size)
 {
-    mr->ops = &unassigned_mem_ops;
-    mr->opaque = NULL;
+    object_initialize(mr, sizeof(*mr), TYPE_MEMORY_REGION);
+
+    /* FIXME: convert all to Properties */
     mr->owner = owner;
-    mr->iommu_ops = NULL;
-    mr->parent = NULL;
     mr->size = int128_make64(size);
     if (size == UINT64_MAX) {
         mr->size = int128_2_64();
     }
-    mr->addr = 0;
-    mr->subpage = false;
+    mr->name = g_strdup(name);
+}
+
+static void memory_region_initfn(Object *obj)
+{
+    MemoryRegion *mr = MEMORY_REGION(obj);
+
+    mr->ops = &unassigned_mem_ops;
     mr->enabled = true;
-    mr->terminates = false;
-    mr->ram = false;
     mr->romd_mode = true;
-    mr->readonly = false;
-    mr->rom_device = false;
     mr->destructor = memory_region_destructor_none;
-    mr->priority = 0;
-    mr->may_overlap = false;
-    mr->alias = NULL;
     QTAILQ_INIT(&mr->subregions);
     memset(&mr->subregions_link, 0, sizeof mr->subregions_link);
     QTAILQ_INIT(&mr->coalesced);
-    mr->name = g_strdup(name);
-    mr->dirty_log_mask = 0;
-    mr->ioeventfd_nb = 0;
-    mr->ioeventfds = NULL;
-    mr->flush_coalesced_mmio = false;
 }
 
 static uint64_t unassigned_mem_read(void *opaque, hwaddr addr,
@@ -1082,8 +1075,10 @@ void memory_region_init_reservation(MemoryRegion *mr,
     memory_region_init_io(mr, owner, &unassigned_mem_ops, mr, name, size);
 }
 
-void memory_region_destroy(MemoryRegion *mr)
+static void memory_region_finalize(Object *obj)
 {
+    MemoryRegion *mr = MEMORY_REGION(obj);
+
     assert(QTAILQ_EMPTY(&mr->subregions));
     assert(memory_region_transaction_depth == 0);
     mr->destructor(mr);
@@ -1092,6 +1087,13 @@ void memory_region_destroy(MemoryRegion *mr)
     g_free(mr->ioeventfds);
 }
 
+void memory_region_destroy(MemoryRegion *mr)
+{
+    /*FIXME: whatever the opposite of object initialize is, do it here */
+    memory_region_finalize(OBJECT(mr));
+}
+
+
 Object *memory_region_owner(MemoryRegion *mr)
 {
     return mr->owner;
@@ -1878,3 +1880,18 @@ void mtree_info(fprintf_function mon_printf, void *f)
         g_free(ml);
     }
 }
+
+static const TypeInfo memory_region_info = {
+    .parent             = TYPE_OBJECT,
+    .name               = TYPE_MEMORY_REGION,
+    .instance_size      = sizeof(MemoryRegion),
+    .instance_init      = memory_region_initfn,
+    .instance_finalize  = memory_region_finalize,
+};
+
+static void memory_register_types(void)
+{
+    type_register_static(&memory_region_info);
+}
+
+type_init(memory_register_types)
-- 
1.9.3.1.ga73a6ad

[Qemu-devel] [RFC v1 05/25] memory: MemoryRegion: Add contained flag

[Peter Crosthwaite]

Rather than use the .parent == NULL check to determine if a memory
region is contained, add a purpose specific boolean flag. This allows
for .parent to be easily converted to a link property while preserving
all the semantics of the legacy API.

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---

 include/exec/memory.h | 1 +
 memory.c              | 9 +++++----
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/include/exec/memory.h b/include/exec/memory.h
index 371c066..2bb074f 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -142,6 +142,7 @@ struct MemoryRegion {
     void *opaque;
     struct Object *owner;
     MemoryRegion *parent;
+    bool contained;
     Int128 size;
     hwaddr addr;
     void (*destructor)(MemoryRegion *mr);
diff --git a/memory.c b/memory.c
index 3c32d5a..a37fdd2 100644
--- a/memory.c
+++ b/memory.c
@@ -1451,6 +1451,7 @@ static void do_memory_region_add_subregion_common(MemoryRegion *subregion)
     QTAILQ_INSERT_TAIL(&mr->subregions, subregion, subregions_link);
 done:
     memory_region_update_pending |= mr->enabled && subregion->enabled;
+    subregion->contained = true;
     memory_region_transaction_commit();
 }
 
@@ -1487,8 +1488,8 @@ void memory_region_del_subregion(MemoryRegion *mr,
                                  MemoryRegion *subregion)
 {
     memory_region_transaction_begin();
-    assert(subregion->parent == mr);
-    subregion->parent = NULL;
+    assert(subregion->parent == mr && subregion->contained);
+    subregion->contained = false;
     QTAILQ_REMOVE(&mr->subregions, subregion, subregions_link);
     memory_region_unref(subregion);
     memory_region_update_pending |= mr->enabled && subregion->enabled;
@@ -1510,7 +1511,7 @@ void memory_region_set_address(MemoryRegion *mr, hwaddr addr)
 {
     MemoryRegion *parent = mr->parent;
 
-    if (addr == mr->addr || !parent) {
+    if (addr == mr->addr || !mr->contained) {
         mr->addr = addr;
         return;
     }
@@ -1518,7 +1519,7 @@ void memory_region_set_address(MemoryRegion *mr, hwaddr addr)
     memory_region_transaction_begin();
     memory_region_ref(mr);
     memory_region_del_subregion(parent, mr);
-    memory_region_add_subregion_common(parent, addr, mr);
+    do_memory_region_add_subregion_common(mr);
     memory_region_unref(mr);
     memory_region_transaction_commit();
 }
-- 
1.9.3.1.ga73a6ad

[Qemu-devel] [RFC v1 06/25] memory: MemoryRegion: Add container and addr props

[Peter Crosthwaite]

Expose the already existing .parent and .addr fields as QOM properties.
Setting the address will cause the memory subregion adding to happen if
it has not already. If the memory region is already contained, then
change it's address as per the established memory_region_set_address()
semantics.

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---

 memory.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 54 insertions(+)

diff --git a/memory.c b/memory.c
index a37fdd2..4a70920 100644
--- a/memory.c
+++ b/memory.c
@@ -16,6 +16,7 @@
 #include "exec/memory.h"
 #include "exec/address-spaces.h"
 #include "exec/ioport.h"
+#include "qapi/visitor.h"
 #include "qemu/bitops.h"
 #include "qom/object.h"
 #include "trace.h"
@@ -844,6 +845,49 @@ void memory_region_init(MemoryRegion *mr,
     mr->name = g_strdup(name);
 }
 
+static void do_memory_region_add_subregion_common(MemoryRegion *subregion);
+
+static void memory_region_get_addr(Object *obj, Visitor *v, void *opaque,
+                                   const char *name, Error **errp)
+{
+    MemoryRegion *mr = MEMORY_REGION(obj);
+    Error *local_err = NULL;
+    uint64_t value = mr->addr;
+
+    visit_type_uint64(v, &value, name, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+    }
+}
+
+static void memory_region_set_addr(Object *obj, Visitor *v, void *opaque,
+                                   const char *name, Error **errp)
+{
+    MemoryRegion *mr = MEMORY_REGION(obj);
+    Error *local_err = NULL;
+    uint64_t value;
+
+    visit_type_uint64(v, &value, name, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        return;
+    }
+
+    /* In an ideal world we wait until both the parent and address is set in
+     * either order, then call do_memory_region_add_subregion_common()
+     * from the latter property setter. However, as parent is just a dumb link
+     * we only support setting from the address setter. We therefore assert
+     * here that the two props are set in correct order.
+     */
+    assert(mr->parent);
+    if (!mr->contained) {
+        mr->addr = value;
+        do_memory_region_add_subregion_common(mr);
+    } else {
+        memory_region_set_address(mr, value);
+    }
+}
+
 static void memory_region_initfn(Object *obj)
 {
     MemoryRegion *mr = MEMORY_REGION(obj);
@@ -855,6 +899,16 @@ static void memory_region_initfn(Object *obj)
     QTAILQ_INIT(&mr->subregions);
     memset(&mr->subregions_link, 0, sizeof mr->subregions_link);
     QTAILQ_INIT(&mr->coalesced);
+
+    object_property_add_link(OBJECT(mr), "container", TYPE_MEMORY_REGION,
+                             (Object **)&mr->parent,
+                             object_property_allow_set_link,
+                             OBJ_PROP_LINK_UNREF_ON_RELEASE,
+                             &error_abort);
+    object_property_add(OBJECT(mr), "addr", "uint64",
+                        memory_region_get_addr,
+                        memory_region_set_addr,
+                        NULL, NULL, &error_abort);
 }
 
 static uint64_t unassigned_mem_read(void *opaque, hwaddr addr,
-- 
1.9.3.1.ga73a6ad

[Qemu-devel] [RFC v1 07/25] memory: MemoryRegion: factor out memory region re-adder

[Peter Crosthwaite]

memory_region_set_address is mostly just a function that deletes and
re-adds a memory region. Factor this generic functionality out into a
re-usable function. This prepares support for further QOMification
of MemoryRegion.

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---

 memory.c | 28 ++++++++++++++++------------
 1 file changed, 16 insertions(+), 12 deletions(-)

diff --git a/memory.c b/memory.c
index 4a70920..9b9cfad 100644
--- a/memory.c
+++ b/memory.c
@@ -829,6 +829,20 @@ static void memory_region_destructor_rom_device(MemoryRegion *mr)
     qemu_ram_free(mr->ram_addr & TARGET_PAGE_MASK);
 }
 
+static void do_memory_region_add_subregion_common(MemoryRegion *subregion);
+
+static void memory_region_readd_subregion(MemoryRegion *mr)
+{
+    if (mr->contained) {
+        memory_region_transaction_begin();
+        memory_region_ref(mr);
+        memory_region_del_subregion(mr->parent, mr);
+        do_memory_region_add_subregion_common(mr);
+        memory_region_unref(mr);
+        memory_region_transaction_commit();
+        }
+}
+
 void memory_region_init(MemoryRegion *mr,
                         Object *owner,
                         const char *name,
@@ -845,7 +859,6 @@ void memory_region_init(MemoryRegion *mr,
     mr->name = g_strdup(name);
 }
 
-static void do_memory_region_add_subregion_common(MemoryRegion *subregion);
 
 static void memory_region_get_addr(Object *obj, Visitor *v, void *opaque,
                                    const char *name, Error **errp)
@@ -1563,19 +1576,10 @@ void memory_region_set_enabled(MemoryRegion *mr, bool enabled)
 
 void memory_region_set_address(MemoryRegion *mr, hwaddr addr)
 {
-    MemoryRegion *parent = mr->parent;
-
-    if (addr == mr->addr || !mr->contained) {
+    if (addr != mr->addr) {
         mr->addr = addr;
-        return;
+        memory_region_readd_subregion(mr);
     }
-
-    memory_region_transaction_begin();
-    memory_region_ref(mr);
-    memory_region_del_subregion(parent, mr);
-    do_memory_region_add_subregion_common(mr);
-    memory_region_unref(mr);
-    memory_region_transaction_commit();
 }
 
 void memory_region_set_alias_offset(MemoryRegion *mr, hwaddr offset)
-- 
1.9.3.1.ga73a6ad

[Qemu-devel] [RFC v1 08/25] memory: MemoryRegion: Add may-overlap and priority props

[Peter Crosthwaite]

QOM propertyify the .may-overlap and .priority fields. The setters
will re-add the memory as a subregion if needed (i.e. the values change
when the memory region is already contained).

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---

 include/exec/memory.h |  2 +-
 memory.c              | 58 ++++++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 58 insertions(+), 2 deletions(-)

diff --git a/include/exec/memory.h b/include/exec/memory.h
index 2bb074f..1f12c15 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -158,7 +158,7 @@ struct MemoryRegion {
     bool flush_coalesced_mmio;
     MemoryRegion *alias;
     hwaddr alias_offset;
-    int priority;
+    uint32_t priority;
     bool may_overlap;
     QTAILQ_HEAD(subregions, MemoryRegion) subregions;
     QTAILQ_ENTRY(MemoryRegion) subregions_link;
diff --git a/memory.c b/memory.c
index 9b9cfad..e1236c1 100644
--- a/memory.c
+++ b/memory.c
@@ -859,7 +859,6 @@ void memory_region_init(MemoryRegion *mr,
     mr->name = g_strdup(name);
 }
 
-
 static void memory_region_get_addr(Object *obj, Visitor *v, void *opaque,
                                    const char *name, Error **errp)
 {
@@ -901,6 +900,55 @@ static void memory_region_set_addr(Object *obj, Visitor *v, void *opaque,
     }
 }
 
+static void memory_region_get_priority(Object *obj, Visitor *v, void *opaque,
+                                       const char *name, Error **errp)
+{
+    MemoryRegion *mr = MEMORY_REGION(obj);
+    Error *local_err = NULL;
+    uint32_t value = mr->addr;
+
+    visit_type_uint32(v, &value, name, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+    }
+}
+
+static void memory_region_set_priority(Object *obj, Visitor *v, void *opaque,
+                                       const char *name, Error **errp)
+{
+    MemoryRegion *mr = MEMORY_REGION(obj);
+    Error *local_err = NULL;
+    uint32_t value;
+
+    visit_type_uint32(v, &value, name, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        return;
+    }
+
+    if (mr->priority != value) {
+        mr->priority = value;
+        memory_region_readd_subregion(mr);
+    }
+}
+
+static bool memory_region_get_may_overlap(Object *obj, Error **errp)
+{
+    MemoryRegion *mr = MEMORY_REGION(obj);
+
+    return mr->may_overlap;
+}
+
+static void memory_region_set_may_overlap(Object *obj, bool value, Error **errp)
+{
+    MemoryRegion *mr = MEMORY_REGION(obj);
+
+    if (mr->may_overlap != value) {
+        mr->may_overlap = value;
+        memory_region_readd_subregion(mr);
+    }
+}
+
 static void memory_region_initfn(Object *obj)
 {
     MemoryRegion *mr = MEMORY_REGION(obj);
@@ -922,6 +970,14 @@ static void memory_region_initfn(Object *obj)
                         memory_region_get_addr,
                         memory_region_set_addr,
                         NULL, NULL, &error_abort);
+    object_property_add(OBJECT(mr), "priority", "uint32",
+                        memory_region_get_priority,
+                        memory_region_set_priority,
+                        NULL, NULL, &error_abort);
+    object_property_add_bool(OBJECT(mr), "may-overlap",
+                             memory_region_get_may_overlap,
+                             memory_region_set_may_overlap,
+                             &error_abort);
 }
 
 static uint64_t unassigned_mem_read(void *opaque, hwaddr addr,
-- 
1.9.3.1.ga73a6ad

[Qemu-devel] [RFC v1 09/25] memory: MemoryRegion: Add size property

[Peter Crosthwaite]

To allow devices to dynamically resize the device. The motivation is
to allow devices with variable size to init their memory_region
without size early and then correctly populate size at realize() time.

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---

 memory.c | 38 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/memory.c b/memory.c
index e1236c1..2ff9ab7 100644
--- a/memory.c
+++ b/memory.c
@@ -949,6 +949,40 @@ static void memory_region_set_may_overlap(Object *obj, bool value, Error **errp)
     }
 }
 
+static void memory_region_get_size(Object *obj, Visitor *v, void *opaque,
+                                   const char *name, Error **errp)
+{
+    MemoryRegion *mr = MEMORY_REGION(obj);
+    Error *local_err = NULL;
+    uint64_t value = int128_get64(mr->size);
+
+    visit_type_uint64(v, &value, name, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+    }
+}
+
+static void memory_region_set_size(Object *obj, Visitor *v, void *opaque,
+                                   const char *name, Error **errp)
+{
+    MemoryRegion *mr = MEMORY_REGION(obj);
+    Error *local_err = NULL;
+    uint64_t value;
+    Int128 v128;
+
+    visit_type_uint64(v, &value, name, &local_err);
+    v128 = int128_make64(value);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        return;
+    }
+
+    if (!int128_eq(v128, mr->size)) {
+        mr->size = v128;
+        memory_region_readd_subregion(mr);
+    }
+}
+
 static void memory_region_initfn(Object *obj)
 {
     MemoryRegion *mr = MEMORY_REGION(obj);
@@ -978,6 +1012,10 @@ static void memory_region_initfn(Object *obj)
                              memory_region_get_may_overlap,
                              memory_region_set_may_overlap,
                              &error_abort);
+    object_property_add(OBJECT(mr), "size", "uint64",
+                        memory_region_get_size,
+                        memory_region_set_size,
+                        NULL, NULL, &error_abort);
 }
 
 static uint64_t unassigned_mem_read(void *opaque, hwaddr addr,
-- 
1.9.3.1.ga73a6ad

[Qemu-devel] [RFC v1 10/25] exec: Parent root MRs to the machine

[Peter Crosthwaite]

Parent the root MemoryRegions for Memory and IO to the machine. This
gives them a QOM path.

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---

 exec.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/exec.c b/exec.c
index cf12049..7ffca81 100644
--- a/exec.c
+++ b/exec.c
@@ -1886,11 +1886,15 @@ static void memory_map_init(void)
     system_memory = g_malloc(sizeof(*system_memory));
 
     memory_region_init(system_memory, NULL, "system", UINT64_MAX);
+    object_property_add_child(qdev_get_machine(), "sysmem",
+                              OBJECT(system_memory), &error_abort);
     address_space_init(&address_space_memory, system_memory, "memory");
 
     system_io = g_malloc(sizeof(*system_io));
     memory_region_init_io(system_io, NULL, &unassigned_io_ops, NULL, "io",
                           65536);
+    object_property_add_child(qdev_get_machine(), "sysio",
+                              OBJECT(system_io), &error_abort);
     address_space_init(&address_space_io, system_io, "I/O");
 
     memory_listener_register(&core_memory_listener, &address_space_memory);
-- 
1.9.3.1.ga73a6ad

[Qemu-devel] [RFC v1 11/25] irq: Slim conversion of qemu_irq to QOM [WIP]

[Peter Crosthwaite]

From: Andreas Färber <afaerber@suse.de>

As a prequel to any big Pin refactoring plans, do an in-place conversion
of qemu_irq to an Object, so that we can reference it in link<> properties.

Signed-off-by: Andreas Färber <afaerber@suse.de>
---

 hw/core/irq.c    | 44 +++++++++++++++++++++++++++++++++++++++++---
 include/hw/irq.h |  2 ++
 2 files changed, 43 insertions(+), 3 deletions(-)

diff --git a/hw/core/irq.c b/hw/core/irq.c
index 03c8cb3..0bcd27b 100644
--- a/hw/core/irq.c
+++ b/hw/core/irq.c
@@ -23,8 +23,13 @@
  */
 #include "qemu-common.h"
 #include "hw/irq.h"
+#include "qom/object.h"
+
+#define IRQ(obj) OBJECT_CHECK(struct IRQState, (obj), TYPE_IRQ)
 
 struct IRQState {
+    Object parent_obj;
+
     qemu_irq_handler handler;
     void *opaque;
     int n;
@@ -38,6 +43,14 @@ void qemu_set_irq(qemu_irq irq, int level)
     irq->handler(irq->opaque, irq->n, level);
 }
 
+static void irq_nonfirst_free(void *obj)
+{
+    struct IRQState *s = obj;
+
+    /* Unreference the first IRQ in this array */
+    object_unref(OBJECT(s - s->n));
+}
+
 qemu_irq *qemu_extend_irqs(qemu_irq *old, int n_old, qemu_irq_handler handler,
                            void *opaque, int n)
 {
@@ -51,11 +64,23 @@ qemu_irq *qemu_extend_irqs(qemu_irq *old, int n_old, qemu_irq_handler handler,
     s = old ? g_renew(qemu_irq, old, n + n_old) : g_new(qemu_irq, n);
     p = old ? g_renew(struct IRQState, s[0], n + n_old) :
                 g_new(struct IRQState, n);
+    memset(p + n_old, 0, n * sizeof(*p));
     for (i = 0; i < n + n_old; i++) {
         if (i >= n_old) {
+            Object *obj;
+
+            object_initialize(p, sizeof(*p), TYPE_IRQ);
             p->handler = handler;
             p->opaque = opaque;
             p->n = i;
+            obj = OBJECT(p);
+            /* Let the first IRQ clean them all up */
+            if (unlikely(i == 0)) {
+                obj->free = g_free;
+            } else {
+                object_ref(OBJECT(s[0]));
+                obj->free = irq_nonfirst_free;
+            }
         }
         s[i] = p;
         p++;
@@ -72,7 +97,7 @@ qemu_irq qemu_allocate_irq(qemu_irq_handler handler, void *opaque, int n)
 {
     struct IRQState *irq;
 
-    irq = g_new(struct IRQState, 1);
+    irq = IRQ(object_new(TYPE_IRQ));
     irq->handler = handler;
     irq->opaque = opaque;
     irq->n = n;
@@ -82,13 +107,13 @@ qemu_irq qemu_allocate_irq(qemu_irq_handler handler, void *opaque, int n)
 
 void qemu_free_irqs(qemu_irq *s)
 {
-    g_free(s[0]);
+    object_unref(OBJECT(s[0]));
     g_free(s);
 }
 
 void qemu_free_irq(qemu_irq irq)
 {
-    g_free(irq);
+    object_unref(OBJECT(irq));
 }
 
 static void qemu_notirq(void *opaque, int line, int level)
@@ -150,3 +175,16 @@ void qemu_irq_intercept_out(qemu_irq **gpio_out, qemu_irq_handler handler, int n
     qemu_irq *old_irqs = *gpio_out;
     *gpio_out = qemu_allocate_irqs(handler, old_irqs, n);
 }
+
+static const TypeInfo irq_type_info = {
+   .name = TYPE_IRQ,
+   .parent = TYPE_OBJECT,
+   .instance_size = sizeof(struct IRQState),
+};
+
+static void irq_register_types(void)
+{
+    type_register_static(&irq_type_info);
+}
+
+type_init(irq_register_types)
diff --git a/include/hw/irq.h b/include/hw/irq.h
index d08bc02..ac76ea7 100644
--- a/include/hw/irq.h
+++ b/include/hw/irq.h
@@ -3,6 +3,8 @@
 
 /* Generic IRQ/GPIO pin infrastructure.  */
 
+#define TYPE_IRQ "irq"
+
 typedef struct IRQState *qemu_irq;
 
 typedef void (*qemu_irq_handler)(void *opaque, int n, int level);
-- 
1.9.3.1.ga73a6ad

[Qemu-devel] [RFC v1 12/25] qdev: gpio: Don't allow name share between I and O

[Peter Crosthwaite]

Only allow a GPIO name to be one or the other. Inputs and outputs are
functionally different and should be in different namespaces. Prepares
support for the QOMification of IRQs as Links.

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---

 hw/core/qdev.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 8efeb04..3330838 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -338,6 +338,7 @@ void qdev_init_gpio_in_named(DeviceState *dev, qemu_irq_handler handler,
 {
     NamedGPIOList *gpio_list = qdev_get_named_gpio_list(dev, name);
 
+    assert(gpio_list->num_out == 0 || !name);
     gpio_list->in = qemu_extend_irqs(gpio_list->in, gpio_list->num_in, handler,
                                      dev, n);
     gpio_list->num_in += n;
@@ -353,6 +354,7 @@ void qdev_init_gpio_out_named(DeviceState *dev, qemu_irq *pins,
 {
     NamedGPIOList *gpio_list = qdev_get_named_gpio_list(dev, name);
 
+    assert(gpio_list->num_in == 0 || !name);
     assert(gpio_list->num_out == 0);
     gpio_list->num_out = n;
     gpio_list->out = pins;
-- 
1.9.3.1.ga73a6ad

[Qemu-devel] [RFC v1 13/25] qdev: gpio: Register GPIO inputs as child objects

[Peter Crosthwaite]

To the device that contains them. This will allow for referencing
a GPIO input from its canonical path (exciting for dynamic machine
generation!)

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---

 hw/core/qdev.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 3330838..cd273e8 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -336,11 +336,21 @@ static NamedGPIOList *qdev_get_named_gpio_list(DeviceState *dev,
 void qdev_init_gpio_in_named(DeviceState *dev, qemu_irq_handler handler,
                              const char *name, int n)
 {
+    int i;
     NamedGPIOList *gpio_list = qdev_get_named_gpio_list(dev, name);
 
     assert(gpio_list->num_out == 0 || !name);
     gpio_list->in = qemu_extend_irqs(gpio_list->in, gpio_list->num_in, handler,
                                      dev, n);
+
+    for (i = gpio_list->num_in; i < gpio_list->num_in + n; i++) {
+        char *propname = g_strdup_printf("%s-%d",
+                                         name ? name : "unnamed-gpio-in", i);
+        object_property_add_child(OBJECT(dev), propname,
+                                  OBJECT(gpio_list->in[i]), &error_abort);
+        g_free(propname);
+    }
+
     gpio_list->num_in += n;
 }
 
-- 
1.9.3.1.ga73a6ad

[Qemu-devel] [RFC v1 14/25] qdev: gpio: Register GPIO outputs as QOM links

[Peter Crosthwaite]

Within the object that contains the GPIO output. This allows for
connecting GPIO outputs via setting of a Link property.

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---

 hw/core/qdev.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index cd273e8..7b1430b 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -362,12 +362,24 @@ void qdev_init_gpio_in(DeviceState *dev, qemu_irq_handler handler, int n)
 void qdev_init_gpio_out_named(DeviceState *dev, qemu_irq *pins,
                               const char *name, int n)
 {
+    int i;
     NamedGPIOList *gpio_list = qdev_get_named_gpio_list(dev, name);
 
     assert(gpio_list->num_in == 0 || !name);
     assert(gpio_list->num_out == 0);
     gpio_list->num_out = n;
     gpio_list->out = pins;
+
+    for (i = 0; i < n; ++i) {
+        char *propname = g_strdup_printf("%s-%d",
+                                         name ? name : "unnamed-gpio-out", i);
+        object_property_add_link(OBJECT(dev), propname, TYPE_IRQ,
+                                 (Object **)&pins[i],
+                                 object_property_allow_set_link,
+                                 OBJ_PROP_LINK_UNREF_ON_RELEASE,
+                                 &error_abort);
+        g_free(propname);
+    }
 }
 
 void qdev_init_gpio_out(DeviceState *dev, qemu_irq *pins, int n)
-- 
1.9.3.1.ga73a6ad

[Qemu-devel] [RFC v1 15/25] qdev: gpio: Re-impement qdev_connect_gpio QOM style

[Peter Crosthwaite]

Re-implement as a link setter. This should allow the QOM framework to
keep track of ref counts properly etc.

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---

 hw/core/qdev.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 7b1430b..94b4e0e 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -403,10 +403,11 @@ qemu_irq qdev_get_gpio_in(DeviceState *dev, int n)
 void qdev_connect_gpio_out_named(DeviceState *dev, const char *name, int n,
                                  qemu_irq pin)
 {
-    NamedGPIOList *gpio_list = qdev_get_named_gpio_list(dev, name);
+    char *propname = g_strdup_printf("%s-%d",
+                                     name ? name : "unnamed-gpio-out", n);
 
-    assert(n >= 0 && n < gpio_list->num_out);
-    gpio_list->out[n] = pin;
+    object_property_set_link(OBJECT(dev), OBJECT(pin), propname, &error_abort);
+    g_free(propname);
 }
 
 void qdev_connect_gpio_out(DeviceState * dev, int n, qemu_irq pin)
-- 
1.9.3.1.ga73a6ad

[Qemu-devel] [RFC v1 16/25] qom: object_property_set/get: Add child recursion

[Peter Crosthwaite]

Allow giving a "/" delimiter to indicate that a property set/get is
accessing a property within a child. E.G. "foo/bar" will set the
"bar" property on child "foo". Multiple "/"s will traverse multiple
child levels.

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---

 qom/object.c | 27 +++++++++++++++++++++++++--
 1 file changed, 25 insertions(+), 2 deletions(-)

diff --git a/qom/object.c b/qom/object.c
index e42b254..a23ccb1 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -772,6 +772,29 @@ ObjectProperty *object_property_find(Object *obj, const char *name,
     return NULL;
 }
 
+static ObjectProperty *object_property_find_traverse(Object **obj,
+                                                     const char **name,
+                                                     Error **errp)
+{
+    const char *slash = strchr(*name, '/');
+
+    if (slash) {
+        Error *err = NULL;
+        char *child_name = g_strndup(*name, slash - *name);
+        *obj = object_property_get_link(*obj, child_name, &err);
+        g_free(child_name);
+        if (err) {
+            error_propagate(errp, err);
+        } else {
+            *name = slash + 1;
+            return object_property_find_traverse(obj, name, errp);
+        }
+        return NULL;
+    }
+
+    return object_property_find(*obj, *name, errp);
+}
+
 void object_property_del(Object *obj, const char *name, Error **errp)
 {
     ObjectProperty *prop = object_property_find(obj, name, errp);
@@ -793,7 +816,7 @@ void object_property_del(Object *obj, const char *name, Error **errp)
 void object_property_get(Object *obj, Visitor *v, const char *name,
                          Error **errp)
 {
-    ObjectProperty *prop = object_property_find(obj, name, errp);
+    ObjectProperty *prop = object_property_find_traverse(&obj, &name, errp);
     if (prop == NULL) {
         return;
     }
@@ -808,7 +831,7 @@ void object_property_get(Object *obj, Visitor *v, const char *name,
 void object_property_set(Object *obj, Visitor *v, const char *name,
                          Error **errp)
 {
-    ObjectProperty *prop = object_property_find(obj, name, errp);
+    ObjectProperty *prop = object_property_find_traverse(&obj, &name, errp);
     if (prop == NULL) {
         return;
     }
-- 
1.9.3.1.ga73a6ad

[Qemu-devel] [RFC v1 17/25] sysbus: Use TYPE_DEVICE GPIO functionality

[Peter Crosthwaite]

Re-implement the Sysbus GPIOs to use the existing TYPE_DEVICE
GPIO named framework. A constant string name is chosen to avoid
conflicts with existing unnamed GPIOs.

This unifies GPIOs are IRQs for sysbus devices and allows removal
of all Sysbus state for GPIOs.

Any existing and future-added functionality for GPIOs is now
also available for sysbus IRQs.

For the anti-sysbus campaigners, this patch brings us one step
closer to deleting the abstraction completely.

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---

 hw/core/sysbus.c    | 20 +++-----------------
 include/hw/sysbus.h |  7 +++----
 2 files changed, 6 insertions(+), 21 deletions(-)

diff --git a/hw/core/sysbus.c b/hw/core/sysbus.c
index f4e760d..2fae2bd 100644
--- a/hw/core/sysbus.c
+++ b/hw/core/sysbus.c
@@ -41,11 +41,7 @@ static const TypeInfo system_bus_info = {
 
 void sysbus_connect_irq(SysBusDevice *dev, int n, qemu_irq irq)
 {
-    assert(n >= 0 && n < dev->num_irq);
-    dev->irqs[n] = NULL;
-    if (dev->irqp[n]) {
-        *dev->irqp[n] = irq;
-    }
+    qdev_connect_gpio_out_named(DEVICE(dev), SYSBUS_DEVICE_GPIO_IRQ, n, irq);
 }
 
 static void sysbus_mmio_map_common(SysBusDevice *dev, int n, hwaddr addr,
@@ -89,22 +85,13 @@ void sysbus_mmio_map_overlap(SysBusDevice *dev, int n, hwaddr addr,
 /* Request an IRQ source.  The actual IRQ object may be populated later.  */
 void sysbus_init_irq(SysBusDevice *dev, qemu_irq *p)
 {
-    int n;
-
-    assert(dev->num_irq < QDEV_MAX_IRQ);
-    n = dev->num_irq++;
-    dev->irqp[n] = p;
+    qdev_init_gpio_out_named(DEVICE(dev), p, SYSBUS_DEVICE_GPIO_IRQ, 1);
 }
 
 /* Pass IRQs from a target device.  */
 void sysbus_pass_irq(SysBusDevice *dev, SysBusDevice *target)
 {
-    int i;
-    assert(dev->num_irq == 0);
-    dev->num_irq = target->num_irq;
-    for (i = 0; i < dev->num_irq; i++) {
-        dev->irqp[i] = target->irqp[i];
-    }
+    /* FIXME */
 }
 
 void sysbus_init_mmio(SysBusDevice *dev, MemoryRegion *memory)
@@ -210,7 +197,6 @@ static void sysbus_dev_print(Monitor *mon, DeviceState *dev, int indent)
     hwaddr size;
     int i;
 
-    monitor_printf(mon, "%*sirq %d\n", indent, "", s->num_irq);
     for (i = 0; i < s->num_mmio; i++) {
         size = memory_region_size(s->mmio[i].memory);
         monitor_printf(mon, "%*smmio " TARGET_FMT_plx "/" TARGET_FMT_plx "\n",
diff --git a/include/hw/sysbus.h b/include/hw/sysbus.h
index f5aaa05..7ee4a04 100644
--- a/include/hw/sysbus.h
+++ b/include/hw/sysbus.h
@@ -8,7 +8,6 @@
 
 #define QDEV_MAX_MMIO 32
 #define QDEV_MAX_PIO 32
-#define QDEV_MAX_IRQ 512
 
 #define TYPE_SYSTEM_BUS "System"
 #define SYSTEM_BUS(obj) OBJECT_CHECK(IDEBus, (obj), TYPE_IDE_BUS)
@@ -33,6 +32,9 @@ typedef struct SysBusDevice SysBusDevice;
  * SysBusDeviceClass is not overriding #DeviceClass.realize, so derived
  * classes overriding it are not required to invoke its implementation.
  */
+
+#define SYSBUS_DEVICE_GPIO_IRQ "sysbus-irq"
+
 typedef struct SysBusDeviceClass {
     /*< private >*/
     DeviceClass parent_class;
@@ -46,9 +48,6 @@ struct SysBusDevice {
     DeviceState parent_obj;
     /*< public >*/
 
-    int num_irq;
-    qemu_irq irqs[QDEV_MAX_IRQ];
-    qemu_irq *irqp[QDEV_MAX_IRQ];
     int num_mmio;
     struct {
         hwaddr addr;
-- 
1.9.3.1.ga73a6ad

[Qemu-devel] [RFC v1 18/25] sysbus: Rework sysbus_mmio_map to use mr QOMification

[Peter Crosthwaite]

The QOMified MemoryRegion properties "parent", "may-overlap",
"priority" and "addr" now handle all these semantics. Just set them
unconditionally. The Memory API also preserves the fast path API
behaviour implemented where if nothing changes don't bounce the
subregion. So all conditional logic can be removed. This also
allow for removal of the mmio.addr field within SysBus.
Another annoying and redundant piece of SysBus state bites the dust!

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---

 hw/core/sysbus.c    | 39 +++++++++++++++------------------------
 include/hw/sysbus.h |  1 -
 2 files changed, 15 insertions(+), 25 deletions(-)

diff --git a/hw/core/sysbus.c b/hw/core/sysbus.c
index 2fae2bd..7cdc428 100644
--- a/hw/core/sysbus.c
+++ b/hw/core/sysbus.c
@@ -47,28 +47,17 @@ void sysbus_connect_irq(SysBusDevice *dev, int n, qemu_irq irq)
 static void sysbus_mmio_map_common(SysBusDevice *dev, int n, hwaddr addr,
                                    bool may_overlap, int priority)
 {
+    MemoryRegion *mr;
     assert(n >= 0 && n < dev->num_mmio);
 
-    if (dev->mmio[n].addr == addr) {
-        /* ??? region already mapped here.  */
-        return;
-    }
-    if (dev->mmio[n].addr != (hwaddr)-1) {
-        /* Unregister previous mapping.  */
-        memory_region_del_subregion(get_system_memory(), dev->mmio[n].memory);
-    }
-    dev->mmio[n].addr = addr;
-    if (may_overlap) {
-        memory_region_add_subregion_overlap(get_system_memory(),
-                                            addr,
-                                            dev->mmio[n].memory,
-                                            priority);
-    }
-    else {
-        memory_region_add_subregion(get_system_memory(),
-                                    addr,
-                                    dev->mmio[n].memory);
-    }
+    mr = dev->mmio[n].memory;
+
+    object_property_set_link(OBJECT(mr), OBJECT(get_system_memory()),
+                             "container", &error_abort);
+    object_property_set_bool(OBJECT(mr), may_overlap, "may-overlap",
+                             &error_abort);
+    object_property_set_int(OBJECT(mr), priority, "priority", &error_abort);
+    object_property_set_int(OBJECT(mr), addr, "addr", &error_abort);
 }
 
 void sysbus_mmio_map(SysBusDevice *dev, int n, hwaddr addr)
@@ -100,7 +89,6 @@ void sysbus_init_mmio(SysBusDevice *dev, MemoryRegion *memory)
 
     assert(dev->num_mmio < QDEV_MAX_MMIO);
     n = dev->num_mmio++;
-    dev->mmio[n].addr = -1;
     dev->mmio[n].memory = memory;
 }
 
@@ -198,9 +186,11 @@ static void sysbus_dev_print(Monitor *mon, DeviceState *dev, int indent)
     int i;
 
     for (i = 0; i < s->num_mmio; i++) {
+        hwaddr addr = object_property_get_int(OBJECT(s->mmio[i].memory),
+                                              "addr", &error_abort);
         size = memory_region_size(s->mmio[i].memory);
         monitor_printf(mon, "%*smmio " TARGET_FMT_plx "/" TARGET_FMT_plx "\n",
-                       indent, "", s->mmio[i].addr, size);
+                       indent, "", addr, size);
     }
 }
 
@@ -213,8 +203,9 @@ static char *sysbus_get_fw_dev_path(DeviceState *dev)
     off = snprintf(path, sizeof(path), "%s", qdev_fw_name(dev));
 
     if (s->num_mmio) {
-        snprintf(path + off, sizeof(path) - off, "@"TARGET_FMT_plx,
-                 s->mmio[0].addr);
+        hwaddr addr = object_property_get_int(OBJECT(s->mmio[0].memory),
+                                              "addr", &error_abort);
+        snprintf(path + off, sizeof(path) - off, "@"TARGET_FMT_plx, addr);
     } else if (s->num_pio) {
         snprintf(path + off, sizeof(path) - off, "@i%04x", s->pio[0]);
     }
diff --git a/include/hw/sysbus.h b/include/hw/sysbus.h
index 7ee4a04..4499020 100644
--- a/include/hw/sysbus.h
+++ b/include/hw/sysbus.h
@@ -50,7 +50,6 @@ struct SysBusDevice {
 
     int num_mmio;
     struct {
-        hwaddr addr;
         MemoryRegion *memory;
     } mmio[QDEV_MAX_MMIO];
     int num_pio;
-- 
1.9.3.1.ga73a6ad

[Qemu-devel] [RFC v1 19/25] sysbus: Setup memory regions as dynamic props

[Peter Crosthwaite]

Dynamically allocate Memory Region pointers and set them up as QOM
links. Plug these dynamic links to the sysbus_init_mmio() and
sysbus_mmio_get_region APIs.

This allows for removal of the Sysbus Memory Regions as state. All
that's needed now is the counter for total number of regions. Another
piece of SysBus state bites the dust!

This also removes the artificial limit of 32 memory regions per device.
The number of memory regions is now practically unbounded (unless you
cause a wrap on the total counter).

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---

 hw/core/sysbus.c    | 29 ++++++++++++++++++-----------
 include/hw/sysbus.h |  4 ----
 2 files changed, 18 insertions(+), 15 deletions(-)

diff --git a/hw/core/sysbus.c b/hw/core/sysbus.c
index 7cdc428..6858336 100644
--- a/hw/core/sysbus.c
+++ b/hw/core/sysbus.c
@@ -50,7 +50,7 @@ static void sysbus_mmio_map_common(SysBusDevice *dev, int n, hwaddr addr,
     MemoryRegion *mr;
     assert(n >= 0 && n < dev->num_mmio);
 
-    mr = dev->mmio[n].memory;
+    mr = sysbus_mmio_get_region(dev, n);
 
     object_property_set_link(OBJECT(mr), OBJECT(get_system_memory()),
                              "container", &error_abort);
@@ -85,16 +85,22 @@ void sysbus_pass_irq(SysBusDevice *dev, SysBusDevice *target)
 
 void sysbus_init_mmio(SysBusDevice *dev, MemoryRegion *memory)
 {
-    int n;
+    char *propname = g_strdup_printf("sysbus-mr-%d", dev->num_mmio++);
 
-    assert(dev->num_mmio < QDEV_MAX_MMIO);
-    n = dev->num_mmio++;
-    dev->mmio[n].memory = memory;
+    object_property_add_child(OBJECT(dev), propname, OBJECT(memory),
+                              &error_abort);
+    g_free(propname);
 }
 
 MemoryRegion *sysbus_mmio_get_region(SysBusDevice *dev, int n)
 {
-    return dev->mmio[n].memory;
+    MemoryRegion *ret;
+    char *propname = g_strdup_printf("sysbus-mr-%d", n);
+
+    ret = MEMORY_REGION(object_property_get_link(OBJECT(dev), propname,
+                        &error_abort));
+    g_free(propname);
+    return ret;
 }
 
 void sysbus_init_ioports(SysBusDevice *dev, pio_addr_t ioport, pio_addr_t size)
@@ -186,9 +192,9 @@ static void sysbus_dev_print(Monitor *mon, DeviceState *dev, int indent)
     int i;
 
     for (i = 0; i < s->num_mmio; i++) {
-        hwaddr addr = object_property_get_int(OBJECT(s->mmio[i].memory),
-                                              "addr", &error_abort);
-        size = memory_region_size(s->mmio[i].memory);
+        MemoryRegion *mr = sysbus_mmio_get_region(s, i);
+        hwaddr addr = object_property_get_int(OBJECT(mr), "addr", &error_abort);
+        size = memory_region_size(mr);
         monitor_printf(mon, "%*smmio " TARGET_FMT_plx "/" TARGET_FMT_plx "\n",
                        indent, "", addr, size);
     }
@@ -203,8 +209,9 @@ static char *sysbus_get_fw_dev_path(DeviceState *dev)
     off = snprintf(path, sizeof(path), "%s", qdev_fw_name(dev));
 
     if (s->num_mmio) {
-        hwaddr addr = object_property_get_int(OBJECT(s->mmio[0].memory),
-                                              "addr", &error_abort);
+        hwaddr addr;
+        addr = object_property_get_int(OBJECT(sysbus_mmio_get_region(s, 0)),
+                                       "addr", &error_abort);
         snprintf(path + off, sizeof(path) - off, "@"TARGET_FMT_plx, addr);
     } else if (s->num_pio) {
         snprintf(path + off, sizeof(path) - off, "@i%04x", s->pio[0]);
diff --git a/include/hw/sysbus.h b/include/hw/sysbus.h
index 4499020..1a8e527 100644
--- a/include/hw/sysbus.h
+++ b/include/hw/sysbus.h
@@ -6,7 +6,6 @@
 #include "hw/qdev.h"
 #include "exec/memory.h"
 
-#define QDEV_MAX_MMIO 32
 #define QDEV_MAX_PIO 32
 
 #define TYPE_SYSTEM_BUS "System"
@@ -49,9 +48,6 @@ struct SysBusDevice {
     /*< public >*/
 
     int num_mmio;
-    struct {
-        MemoryRegion *memory;
-    } mmio[QDEV_MAX_MMIO];
     int num_pio;
     pio_addr_t pio[QDEV_MAX_PIO];
 };
-- 
1.9.3.1.ga73a6ad

[Qemu-devel] [RFC v1 20/25] sysbus: Enable hotplug.

[Peter Crosthwaite]

If your willing to set a few props, it can work. All the
connections this comment is referring to are QOMified now, so sysbus
hotplug is a full reality.

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---

 hw/core/sysbus.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/hw/core/sysbus.c b/hw/core/sysbus.c
index 6858336..a56a74d 100644
--- a/hw/core/sysbus.c
+++ b/hw/core/sysbus.c
@@ -241,13 +241,6 @@ static void sysbus_device_class_init(ObjectClass *klass, void *data)
     DeviceClass *k = DEVICE_CLASS(klass);
     k->init = sysbus_device_init;
     k->bus_type = TYPE_SYSTEM_BUS;
-    /*
-     * device_add plugs devices into suitable bus.  For "real" buses,
-     * that actually connects the device.  For sysbus, the connections
-     * need to be made separately, and device_add can't do that.  The
-     * device would be left unconnected, and could not possibly work.
-     */
-    k->cannot_instantiate_with_device_add_yet = true;
 }
 
 static const TypeInfo sysbus_device_type_info = {
-- 
1.9.3.1.ga73a6ad

[Qemu-devel] [RFC v1 21/25] microblaze: s3adsp: Expand UART creator

[Peter Crosthwaite]

To not use sysbus_create_simple. This prepares for setting the device
parent.

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---

 hw/microblaze/petalogix_s3adsp1800_mmu.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/hw/microblaze/petalogix_s3adsp1800_mmu.c b/hw/microblaze/petalogix_s3adsp1800_mmu.c
index 6c45e20..9bc6928 100644
--- a/hw/microblaze/petalogix_s3adsp1800_mmu.c
+++ b/hw/microblaze/petalogix_s3adsp1800_mmu.c
@@ -107,8 +107,10 @@ petalogix_s3adsp1800_init(QEMUMachineInitArgs *args)
         irq[i] = qdev_get_gpio_in(dev, i);
     }
 
-    sysbus_create_simple("xlnx.xps-uartlite", UARTLITE_BASEADDR,
-                         irq[UARTLITE_IRQ]);
+    dev = qdev_create(NULL, "xlnx.xps-uartlite");
+    qdev_init_nofail(dev);
+    sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, UARTLITE_BASEADDR);
+    sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, irq[UARTLITE_IRQ]);
 
     /* 2 timers at irq 2 @ 62 Mhz.  */
     dev = qdev_create(NULL, "xlnx.xps-timer");
-- 
1.9.3.1.ga73a6ad

[Qemu-devel] [RFC v1 22/25] microblaze: s3adsp: Parent devices with sane names

[Peter Crosthwaite]

For ease of reference by users of their canonical paths.

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---

 hw/microblaze/petalogix_s3adsp1800_mmu.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/hw/microblaze/petalogix_s3adsp1800_mmu.c b/hw/microblaze/petalogix_s3adsp1800_mmu.c
index 9bc6928..8557254 100644
--- a/hw/microblaze/petalogix_s3adsp1800_mmu.c
+++ b/hw/microblaze/petalogix_s3adsp1800_mmu.c
@@ -97,6 +97,8 @@ petalogix_s3adsp1800_init(QEMUMachineInitArgs *args)
                           1, 0x89, 0x18, 0x0000, 0x0, 1);
 
     dev = qdev_create(NULL, "xlnx.xps-intc");
+    object_property_add_child(qdev_get_machine(), "intc", OBJECT(dev),
+                              &error_abort);
     qdev_prop_set_uint32(dev, "kind-of-intr",
                          1 << ETHLITE_IRQ | 1 << UARTLITE_IRQ);
     qdev_init_nofail(dev);
@@ -108,12 +110,16 @@ petalogix_s3adsp1800_init(QEMUMachineInitArgs *args)
     }
 
     dev = qdev_create(NULL, "xlnx.xps-uartlite");
+    object_property_add_child(qdev_get_machine(), "uart", OBJECT(dev),
+                              &error_abort);
     qdev_init_nofail(dev);
     sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, UARTLITE_BASEADDR);
     sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, irq[UARTLITE_IRQ]);
 
     /* 2 timers at irq 2 @ 62 Mhz.  */
     dev = qdev_create(NULL, "xlnx.xps-timer");
+    object_property_add_child(qdev_get_machine(), "timer", OBJECT(dev),
+                              &error_abort);
     qdev_prop_set_uint32(dev, "one-timer-only", 0);
     qdev_prop_set_uint32(dev, "clock-frequency", 62 * 1000000);
     qdev_init_nofail(dev);
@@ -122,6 +128,8 @@ petalogix_s3adsp1800_init(QEMUMachineInitArgs *args)
 
     qemu_check_nic_model(&nd_table[0], "xlnx.xps-ethernetlite");
     dev = qdev_create(NULL, "xlnx.xps-ethernetlite");
+    object_property_add_child(qdev_get_machine(), "enet", OBJECT(dev),
+                              &error_abort);
     qdev_set_nic_properties(dev, &nd_table[0]);
     qdev_prop_set_uint32(dev, "tx-ping-pong", 0);
     qdev_prop_set_uint32(dev, "rx-ping-pong", 0);
-- 
1.9.3.1.ga73a6ad

[Qemu-devel] [RFC v1 23/25] timer: xilinx_timer: Convert to realize()

[Peter Crosthwaite]

SysBusDevice::init is depracated. Convert to Object::init and
Device::realize as prescribed by QOM conventions.

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---

 hw/timer/xilinx_timer.c | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/hw/timer/xilinx_timer.c b/hw/timer/xilinx_timer.c
index 3ff1da9..cdb3355 100644
--- a/hw/timer/xilinx_timer.c
+++ b/hw/timer/xilinx_timer.c
@@ -204,14 +204,11 @@ static void timer_hit(void *opaque)
     timer_update_irq(t);
 }
 
-static int xilinx_timer_init(SysBusDevice *dev)
+static void xilinx_timer_realize(DeviceState *dev, Error **errp)
 {
     struct timerblock *t = XILINX_TIMER(dev);
     unsigned int i;
 
-    /* All timers share a single irq line.  */
-    sysbus_init_irq(dev, &t->irq);
-
     /* Init all the ptimers.  */
     t->timers = g_malloc0(sizeof t->timers[0] * num_timers(t));
     for (i = 0; i < num_timers(t); i++) {
@@ -226,8 +223,15 @@ static int xilinx_timer_init(SysBusDevice *dev)
 
     memory_region_init_io(&t->mmio, OBJECT(t), &timer_ops, t, "xlnx.xps-timer",
                           R_MAX * 4 * num_timers(t));
-    sysbus_init_mmio(dev, &t->mmio);
-    return 0;
+    sysbus_init_mmio(SYS_BUS_DEVICE(dev), &t->mmio);
+}
+
+static void xilinx_timer_init(Object *obj)
+{
+    struct timerblock *t = XILINX_TIMER(obj);
+
+    /* All timers share a single irq line.  */
+    sysbus_init_irq(SYS_BUS_DEVICE(obj), &t->irq);
 }
 
 static Property xilinx_timer_properties[] = {
@@ -240,9 +244,8 @@ static Property xilinx_timer_properties[] = {
 static void xilinx_timer_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
-    SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
 
-    k->init = xilinx_timer_init;
+    dc->realize = xilinx_timer_realize;
     dc->props = xilinx_timer_properties;
 }
 
@@ -250,6 +253,7 @@ static const TypeInfo xilinx_timer_info = {
     .name          = TYPE_XILINX_TIMER,
     .parent        = TYPE_SYS_BUS_DEVICE,
     .instance_size = sizeof(struct timerblock),
+    .instance_init = xilinx_timer_init,
     .class_init    = xilinx_timer_class_init,
 };
 
-- 
1.9.3.1.ga73a6ad

[Qemu-devel] [RFC v1 24/25] timer: xilinx_timer: init MMIO ASAP

[Peter Crosthwaite]

So the memory regions exist pre-realize.

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---

 hw/timer/xilinx_timer.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/hw/timer/xilinx_timer.c b/hw/timer/xilinx_timer.c
index cdb3355..1b0e2a1 100644
--- a/hw/timer/xilinx_timer.c
+++ b/hw/timer/xilinx_timer.c
@@ -221,9 +221,8 @@ static void xilinx_timer_realize(DeviceState *dev, Error **errp)
         ptimer_set_freq(xt->ptimer, t->freq_hz);
     }
 
-    memory_region_init_io(&t->mmio, OBJECT(t), &timer_ops, t, "xlnx.xps-timer",
-                          R_MAX * 4 * num_timers(t));
-    sysbus_init_mmio(SYS_BUS_DEVICE(dev), &t->mmio);
+    object_property_set_int(OBJECT(&t->mmio), R_MAX * 4 * num_timers(t),
+                            "size", &error_abort);
 }
 
 static void xilinx_timer_init(Object *obj)
@@ -232,6 +231,9 @@ static void xilinx_timer_init(Object *obj)
 
     /* All timers share a single irq line.  */
     sysbus_init_irq(SYS_BUS_DEVICE(obj), &t->irq);
+    memory_region_init_io(&t->mmio, OBJECT(t), &timer_ops, t,
+                          "xlnx.xps-timer", 0);
+    sysbus_init_mmio(SYS_BUS_DEVICE(obj), &t->mmio);
 }
 
 static Property xilinx_timer_properties[] = {
-- 
1.9.3.1.ga73a6ad

[Qemu-devel] [RFC v1 25/25] TEST: microblaze: s3adsp: Remove timer

[Peter Crosthwaite]

To make it easy to test sysbus hotplug with the published Microblaze
test image. The timer can be added using device_add etc.

On your command line:

-device xlnx.xps-timer,\
sysbus-mr-0.container=/machine/sysmem/root-mr,\
sysbus-mr-0.addr=0x83c00000,\
sysbus-irq-0=/machine/intc/unnamed-gpio-0

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---

 hw/microblaze/petalogix_s3adsp1800_mmu.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/microblaze/petalogix_s3adsp1800_mmu.c b/hw/microblaze/petalogix_s3adsp1800_mmu.c
index 8557254..74d7f05 100644
--- a/hw/microblaze/petalogix_s3adsp1800_mmu.c
+++ b/hw/microblaze/petalogix_s3adsp1800_mmu.c
@@ -116,6 +116,7 @@ petalogix_s3adsp1800_init(QEMUMachineInitArgs *args)
     sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, UARTLITE_BASEADDR);
     sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, irq[UARTLITE_IRQ]);
 
+#if 0
     /* 2 timers at irq 2 @ 62 Mhz.  */
     dev = qdev_create(NULL, "xlnx.xps-timer");
     object_property_add_child(qdev_get_machine(), "timer", OBJECT(dev),
@@ -125,6 +126,7 @@ petalogix_s3adsp1800_init(QEMUMachineInitArgs *args)
     qdev_init_nofail(dev);
     sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, TIMER_BASEADDR);
     sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, irq[TIMER_IRQ]);
+#endif
 
     qemu_check_nic_model(&nd_table[0], "xlnx.xps-ethernetlite");
     dev = qdev_create(NULL, "xlnx.xps-ethernetlite");
-- 
1.9.3.1.ga73a6ad

Re: [Qemu-devel] [RFC v1 11/25] irq: Slim conversion of qemu_irq to QOM [WIP]

[Peter Crosthwaite]

On Fri, May 16, 2014 at 11:56 AM, Peter Crosthwaite
<peter.crosthwaite@xilinx.com> wrote:
> From: Andreas Färber <afaerber@suse.de>
>
> As a prequel to any big Pin refactoring plans, do an in-place conversion
> of qemu_irq to an Object, so that we can reference it in link<> properties.
>
> Signed-off-by: Andreas Färber <afaerber@suse.de>
> ---
>
>  hw/core/irq.c    | 44 +++++++++++++++++++++++++++++++++++++++++---
>  include/hw/irq.h |  2 ++
>  2 files changed, 43 insertions(+), 3 deletions(-)
>
> diff --git a/hw/core/irq.c b/hw/core/irq.c
> index 03c8cb3..0bcd27b 100644
> --- a/hw/core/irq.c
> +++ b/hw/core/irq.c
> @@ -23,8 +23,13 @@
>   */
>  #include "qemu-common.h"
>  #include "hw/irq.h"
> +#include "qom/object.h"
> +
> +#define IRQ(obj) OBJECT_CHECK(struct IRQState, (obj), TYPE_IRQ)
>
>  struct IRQState {
> +    Object parent_obj;
> +
>      qemu_irq_handler handler;
>      void *opaque;
>      int n;
> @@ -38,6 +43,14 @@ void qemu_set_irq(qemu_irq irq, int level)
>      irq->handler(irq->opaque, irq->n, level);
>  }
>
> +static void irq_nonfirst_free(void *obj)
> +{
> +    struct IRQState *s = obj;
> +
> +    /* Unreference the first IRQ in this array */
> +    object_unref(OBJECT(s - s->n));
> +}
> +
>  qemu_irq *qemu_extend_irqs(qemu_irq *old, int n_old, qemu_irq_handler handler,
>                             void *opaque, int n)
>  {
> @@ -51,11 +64,23 @@ qemu_irq *qemu_extend_irqs(qemu_irq *old, int n_old, qemu_irq_handler handler,
>      s = old ? g_renew(qemu_irq, old, n + n_old) : g_new(qemu_irq, n);
>      p = old ? g_renew(struct IRQState, s[0], n + n_old) :
>                  g_new(struct IRQState, n);

So using g_renew on the actual object storage is very fragile, as it
means you cannot reliably take pointers to the objects. I think
however this whole issue could be simplified by de-arrayifying the
whole thing, and treating IRQs individually (interdiff):

 qemu_irq *qemu_extend_irqs(qemu_irq *old, int n_old, qemu_irq_handler handler,
                            void *opaque, int n)
 {
     qemu_irq *s;
-    struct IRQState *p;
     int i;

     if (!old) {
         n_old = 0;
     }
     s = old ? g_renew(qemu_irq, old, n + n_old) : g_new(qemu_irq, n);
-    p = old ? g_renew(struct IRQState, s[0], n + n_old) :
-                g_new(struct IRQState, n);
-    memset(p + n_old, 0, n * sizeof(*p));
     for (i = 0; i < n + n_old; i++) {
         if (i >= n_old) {
-            Object *obj;
-
-            object_initialize(p, sizeof(*p), TYPE_IRQ);
-            p->handler = handler;
-            p->opaque = opaque;
-            p->n = i;
-            obj = OBJECT(p);
-            /* Let the first IRQ clean them all up */
-            if (unlikely(i == 0)) {
-                obj->free = g_free;
-            } else {
-                object_ref(OBJECT(s[0]));
-                obj->free = irq_nonfirst_free;
-            }
+            s[i] = qemu_allocate_irq(handler, opaque, i);
         }
-        s[i] = p;
-        p++;
     }
     return s;

The system for freeing may need some thought, and I wonder if their
are API clients out there assuming the IRQ storage elements are
contiguous (if there are they have too much internal knowledge and
need to be fixed).

But with this change these objects are now usable with links and canon
paths etc.

Will roll into V2 of this patch.

Regards,
Peter

Re: [Qemu-devel] [RFC v1 17/25] sysbus: Use TYPE_DEVICE GPIO functionality

[Peter Crosthwaite]

On Fri, May 16, 2014 at 11:59 AM, Peter Crosthwaite
<peter.crosthwaite@xilinx.com> wrote:
> Re-implement the Sysbus GPIOs to use the existing TYPE_DEVICE
> GPIO named framework. A constant string name is chosen to avoid
> conflicts with existing unnamed GPIOs.
>
> This unifies GPIOs are IRQs for sysbus devices and allows removal
> of all Sysbus state for GPIOs.
>
> Any existing and future-added functionality for GPIOs is now
> also available for sysbus IRQs.
>
> For the anti-sysbus campaigners, this patch brings us one step
> closer to deleting the abstraction completely.
>
> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
> ---
>
>  hw/core/sysbus.c    | 20 +++-----------------
>  include/hw/sysbus.h |  7 +++----
>  2 files changed, 6 insertions(+), 21 deletions(-)
>
> diff --git a/hw/core/sysbus.c b/hw/core/sysbus.c
> index f4e760d..2fae2bd 100644
> --- a/hw/core/sysbus.c
> +++ b/hw/core/sysbus.c
> @@ -41,11 +41,7 @@ static const TypeInfo system_bus_info = {
>
>  void sysbus_connect_irq(SysBusDevice *dev, int n, qemu_irq irq)
>  {
> -    assert(n >= 0 && n < dev->num_irq);
> -    dev->irqs[n] = NULL;
> -    if (dev->irqp[n]) {
> -        *dev->irqp[n] = irq;
> -    }
> +    qdev_connect_gpio_out_named(DEVICE(dev), SYSBUS_DEVICE_GPIO_IRQ, n, irq);
>  }
>
>  static void sysbus_mmio_map_common(SysBusDevice *dev, int n, hwaddr addr,
> @@ -89,22 +85,13 @@ void sysbus_mmio_map_overlap(SysBusDevice *dev, int n, hwaddr addr,
>  /* Request an IRQ source.  The actual IRQ object may be populated later.  */
>  void sysbus_init_irq(SysBusDevice *dev, qemu_irq *p)
>  {
> -    int n;
> -
> -    assert(dev->num_irq < QDEV_MAX_IRQ);
> -    n = dev->num_irq++;
> -    dev->irqp[n] = p;
> +    qdev_init_gpio_out_named(DEVICE(dev), p, SYSBUS_DEVICE_GPIO_IRQ, 1);

This is not repeatable as qemu_init_gpio_out() doesnt allow repeated
calls for the same GPIO output set (doh!) - that feature only exists
for GPIO inputs. Rather than investing further into unnamed GPIOs on
the DEVICE level though, I think it's better to just name each sysbus
IRQ individually. If you really really want arrayified IRQs then you
can still use the DEVICE GPIO API directly. To fix in V2:

 /* Request an IRQ source.  The actual IRQ object may be populated later.  */
 void sysbus_init_irq(SysBusDevice *dev, qemu_irq *p)
 {
-    qdev_init_gpio_out_named(DEVICE(dev), p, SYSBUS_DEVICE_GPIO_IRQ, 1);
+    char *irq_name = g_strdup_printf(SYSBUS_DEVICE_GPIO_IRQ "-%d",
+                                     dev->num_irq++);
+    qdev_init_gpio_out_named(DEVICE(dev), p, irq_name, 1);
+    g_free(irq_name);
 }

Regards,
Peter

Re: [Qemu-devel] [RFC v1 11/25] irq: Slim conversion of qemu_irq to QOM [WIP]

[Andreas Färber]

Am 19.05.2014 03:52, schrieb Peter Crosthwaite:
> On Fri, May 16, 2014 at 11:56 AM, Peter Crosthwaite
> <peter.crosthwaite@xilinx.com> wrote:
>> From: Andreas Färber <afaerber@suse.de>
>>
>> As a prequel to any big Pin refactoring plans, do an in-place conversion
>> of qemu_irq to an Object, so that we can reference it in link<> properties.
>>
>> Signed-off-by: Andreas Färber <afaerber@suse.de>

(Note that you forgot to sign off here.)

>> ---
>>
>>  hw/core/irq.c    | 44 +++++++++++++++++++++++++++++++++++++++++---
>>  include/hw/irq.h |  2 ++
>>  2 files changed, 43 insertions(+), 3 deletions(-)
>>
>> diff --git a/hw/core/irq.c b/hw/core/irq.c
>> index 03c8cb3..0bcd27b 100644
>> --- a/hw/core/irq.c
>> +++ b/hw/core/irq.c
>> @@ -23,8 +23,13 @@
>>   */
>>  #include "qemu-common.h"
>>  #include "hw/irq.h"
>> +#include "qom/object.h"
>> +
>> +#define IRQ(obj) OBJECT_CHECK(struct IRQState, (obj), TYPE_IRQ)
>>
>>  struct IRQState {
>> +    Object parent_obj;
>> +
>>      qemu_irq_handler handler;
>>      void *opaque;
>>      int n;
>> @@ -38,6 +43,14 @@ void qemu_set_irq(qemu_irq irq, int level)
>>      irq->handler(irq->opaque, irq->n, level);
>>  }
>>
>> +static void irq_nonfirst_free(void *obj)
>> +{
>> +    struct IRQState *s = obj;
>> +
>> +    /* Unreference the first IRQ in this array */
>> +    object_unref(OBJECT(s - s->n));
>> +}
>> +
>>  qemu_irq *qemu_extend_irqs(qemu_irq *old, int n_old, qemu_irq_handler handler,
>>                             void *opaque, int n)
>>  {
>> @@ -51,11 +64,23 @@ qemu_irq *qemu_extend_irqs(qemu_irq *old, int n_old, qemu_irq_handler handler,
>>      s = old ? g_renew(qemu_irq, old, n + n_old) : g_new(qemu_irq, n);
>>      p = old ? g_renew(struct IRQState, s[0], n + n_old) :
>>                  g_new(struct IRQState, n);
> 
> So using g_renew on the actual object storage is very fragile, as it
> means you cannot reliably take pointers to the objects. I think
> however this whole issue could be simplified by de-arrayifying the
> whole thing, and treating IRQs individually (interdiff):
> 
>  qemu_irq *qemu_extend_irqs(qemu_irq *old, int n_old, qemu_irq_handler handler,
>                             void *opaque, int n)
>  {
>      qemu_irq *s;
> -    struct IRQState *p;
>      int i;
> 
>      if (!old) {
>          n_old = 0;
>      }
>      s = old ? g_renew(qemu_irq, old, n + n_old) : g_new(qemu_irq, n);
> -    p = old ? g_renew(struct IRQState, s[0], n + n_old) :
> -                g_new(struct IRQState, n);
> -    memset(p + n_old, 0, n * sizeof(*p));
>      for (i = 0; i < n + n_old; i++) {
>          if (i >= n_old) {
> -            Object *obj;
> -
> -            object_initialize(p, sizeof(*p), TYPE_IRQ);
> -            p->handler = handler;
> -            p->opaque = opaque;
> -            p->n = i;
> -            obj = OBJECT(p);
> -            /* Let the first IRQ clean them all up */
> -            if (unlikely(i == 0)) {
> -                obj->free = g_free;
> -            } else {
> -                object_ref(OBJECT(s[0]));
> -                obj->free = irq_nonfirst_free;
> -            }
> +            s[i] = qemu_allocate_irq(handler, opaque, i);
>          }
> -        s[i] = p;
> -        p++;
>      }
>      return s;
> 
> The system for freeing may need some thought, and I wonder if their
> are API clients out there assuming the IRQ storage elements are
> contiguous (if there are they have too much internal knowledge and
> need to be fixed).
> 
> But with this change these objects are now usable with links and canon
> paths etc.
> 
> Will roll into V2 of this patch.

Negative!

I was well aware of the two g_renew()s, one of which affects the
objects. However I figured, as long as no one has a pointer to those
objects it should just continue work (otherwise the pre-QOM pointers
would've broken, too -> separate issue) - and so far I haven't found a
test case that doesn't work as is.

So while I agree that we should refactor this, your series does iirc not
include my genuine qemu_irq* fixes either, and I reported at least two
callers of qemu_free_irqs() remaining, in serial-pci.c among others, so
your diff above is not yet okay - it would leak any non-first IRQState.
Which is why I do these odd object_{ref,unref}() tricks -
qemu_free_irqs() cannot tell how many IRQs there are to be freed. And
while your use of qemu_allocate_irq() gives them the normal oc->free =
g_free for freeing them individually, you do not unref them anywhere, so
it will never happen.

Instead of squashing this into my patch I suggest you build upon master
plus my first two cleanup patches and get rid of the remaining
qemu_free_irqs()s altogether, then we can much more sanely refactor this
code and get it in shortly.

Cheers,
Andreas

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

Re: [Qemu-devel] [RFC v1 11/25] irq: Slim conversion of qemu_irq to QOM [WIP]

[Peter Crosthwaite]

On Mon, May 19, 2014 at 7:13 PM, Andreas Färber <afaerber@suse.de> wrote:
> Am 19.05.2014 03:52, schrieb Peter Crosthwaite:
>> On Fri, May 16, 2014 at 11:56 AM, Peter Crosthwaite
>> <peter.crosthwaite@xilinx.com> wrote:
>>> From: Andreas Färber <afaerber@suse.de>
>>>
>>> As a prequel to any big Pin refactoring plans, do an in-place conversion
>>> of qemu_irq to an Object, so that we can reference it in link<> properties.
>>>
>>> Signed-off-by: Andreas Färber <afaerber@suse.de>
>
> (Note that you forgot to sign off here.)
>
>>> ---
>>>
>>>  hw/core/irq.c    | 44 +++++++++++++++++++++++++++++++++++++++++---
>>>  include/hw/irq.h |  2 ++
>>>  2 files changed, 43 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/hw/core/irq.c b/hw/core/irq.c
>>> index 03c8cb3..0bcd27b 100644
>>> --- a/hw/core/irq.c
>>> +++ b/hw/core/irq.c
>>> @@ -23,8 +23,13 @@
>>>   */
>>>  #include "qemu-common.h"
>>>  #include "hw/irq.h"
>>> +#include "qom/object.h"
>>> +
>>> +#define IRQ(obj) OBJECT_CHECK(struct IRQState, (obj), TYPE_IRQ)
>>>
>>>  struct IRQState {
>>> +    Object parent_obj;
>>> +
>>>      qemu_irq_handler handler;
>>>      void *opaque;
>>>      int n;
>>> @@ -38,6 +43,14 @@ void qemu_set_irq(qemu_irq irq, int level)
>>>      irq->handler(irq->opaque, irq->n, level);
>>>  }
>>>
>>> +static void irq_nonfirst_free(void *obj)
>>> +{
>>> +    struct IRQState *s = obj;
>>> +
>>> +    /* Unreference the first IRQ in this array */
>>> +    object_unref(OBJECT(s - s->n));
>>> +}
>>> +
>>>  qemu_irq *qemu_extend_irqs(qemu_irq *old, int n_old, qemu_irq_handler handler,
>>>                             void *opaque, int n)
>>>  {
>>> @@ -51,11 +64,23 @@ qemu_irq *qemu_extend_irqs(qemu_irq *old, int n_old, qemu_irq_handler handler,
>>>      s = old ? g_renew(qemu_irq, old, n + n_old) : g_new(qemu_irq, n);
>>>      p = old ? g_renew(struct IRQState, s[0], n + n_old) :
>>>                  g_new(struct IRQState, n);
>>
>> So using g_renew on the actual object storage is very fragile, as it
>> means you cannot reliably take pointers to the objects. I think
>> however this whole issue could be simplified by de-arrayifying the
>> whole thing, and treating IRQs individually (interdiff):
>>
>>  qemu_irq *qemu_extend_irqs(qemu_irq *old, int n_old, qemu_irq_handler handler,
>>                             void *opaque, int n)
>>  {
>>      qemu_irq *s;
>> -    struct IRQState *p;
>>      int i;
>>
>>      if (!old) {
>>          n_old = 0;
>>      }
>>      s = old ? g_renew(qemu_irq, old, n + n_old) : g_new(qemu_irq, n);
>> -    p = old ? g_renew(struct IRQState, s[0], n + n_old) :
>> -                g_new(struct IRQState, n);
>> -    memset(p + n_old, 0, n * sizeof(*p));
>>      for (i = 0; i < n + n_old; i++) {
>>          if (i >= n_old) {
>> -            Object *obj;
>> -
>> -            object_initialize(p, sizeof(*p), TYPE_IRQ);
>> -            p->handler = handler;
>> -            p->opaque = opaque;
>> -            p->n = i;
>> -            obj = OBJECT(p);
>> -            /* Let the first IRQ clean them all up */
>> -            if (unlikely(i == 0)) {
>> -                obj->free = g_free;
>> -            } else {
>> -                object_ref(OBJECT(s[0]));
>> -                obj->free = irq_nonfirst_free;
>> -            }
>> +            s[i] = qemu_allocate_irq(handler, opaque, i);
>>          }
>> -        s[i] = p;
>> -        p++;
>>      }
>>      return s;
>>
>> The system for freeing may need some thought, and I wonder if their
>> are API clients out there assuming the IRQ storage elements are
>> contiguous (if there are they have too much internal knowledge and
>> need to be fixed).
>>
>> But with this change these objects are now usable with links and canon
>> paths etc.
>>
>> Will roll into V2 of this patch.
>
> Negative!
>
> I was well aware of the two g_renew()s, one of which affects the
> objects. However I figured, as long as no one has a pointer to those
> objects it should just continue work (otherwise the pre-QOM pointers

Well I introduce the issue 13-15 of this series. They are dependant on
pointer stability.

> would've broken, too -> separate issue) - and so far I haven't found a
> test case that doesn't work as is.
>
> So while I agree that we should refactor this, your series does iirc not
> include my genuine qemu_irq* fixes either, and I reported at least two
> callers of qemu_free_irqs() remaining, in serial-pci.c among others, so
> your diff above is not yet okay -

Understand. I have seen your predecessor patches in your branch. I
have left out such patches from this RFC (there are others like it for
other features) to avoid a monster series - this is really just for
comments on the whole idea. I probably already have a TLDR problem
here so i'm trying to not make it worse!

 it would leak any non-first IRQState.
> Which is why I do these odd object_{ref,unref}() tricks -
> qemu_free_irqs() cannot tell how many IRQs there are to be freed.

There are so few call sites, they can probably just keep track of
numbers and pass the count to the freer. That or removal as you have
started in your short series.

 And
> while your use of qemu_allocate_irq() gives them the normal oc->free =
> g_free for freeing them individually, you do not unref them anywhere, so
> it will never happen.
>

Working on it. Left that issue alone just to get something working
today. By v2 I mean a v2 RFC :)

> Instead of squashing this into my patch I suggest you build upon master
> plus my first two cleanup patches and get rid of the remaining
> qemu_free_irqs()s altogether, then we can much more sanely refactor this
> code and get it in shortly.
>

Ok. Thanks for the review.

Regards,
Peter

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

Re: [Qemu-devel] [RFC v1 08/25] memory: MemoryRegion: Add may-overlap and priority props

[Peter Crosthwaite]

On Fri, May 16, 2014 at 11:54 AM, Peter Crosthwaite
<peter.crosthwaite@xilinx.com> wrote:
> QOM propertyify the .may-overlap and .priority fields. The setters
> will re-add the memory as a subregion if needed (i.e. the values change
> when the memory region is already contained).
>
> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
> ---
>
>  include/exec/memory.h |  2 +-
>  memory.c              | 58 ++++++++++++++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 58 insertions(+), 2 deletions(-)
>
> diff --git a/include/exec/memory.h b/include/exec/memory.h
> index 2bb074f..1f12c15 100644
> --- a/include/exec/memory.h
> +++ b/include/exec/memory.h
> @@ -158,7 +158,7 @@ struct MemoryRegion {
>      bool flush_coalesced_mmio;
>      MemoryRegion *alias;
>      hwaddr alias_offset;
> -    int priority;
> +    uint32_t priority;

It appears as if -ve memory region priorities are valid and work. Will
preserve this and change to this type to int32_t.

Regards,
Peter

>      bool may_overlap;
>      QTAILQ_HEAD(subregions, MemoryRegion) subregions;
>      QTAILQ_ENTRY(MemoryRegion) subregions_link;
> diff --git a/memory.c b/memory.c
> index 9b9cfad..e1236c1 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -859,7 +859,6 @@ void memory_region_init(MemoryRegion *mr,
>      mr->name = g_strdup(name);
>  }
>
>  static uint64_t unassigned_mem_read(void *opaque, hwaddr addr,
> --
> 1.9.3.1.ga73a6ad
>