On Wed, Jul 2, 2014 at 7:49 AM, Alexander Graf <address@hidden> wrote:
We want to give the user the ability to tell our machine file where he wants
to have devices mapped to. This patch adds code to create these hints
dynamically and expose them as object properties that can only be modified
before device realization.
Signed-off-by: Alexander Graf <address@hidden>
---
hw/core/sysbus.c | 73 +++++++++++++++++++++++++++++++++++++++++++++++++++--
include/hw/sysbus.h | 6 +++++
2 files changed, 77 insertions(+), 2 deletions(-)
diff --git a/hw/core/sysbus.c b/hw/core/sysbus.c
index f4e760d..84cd0cf 100644
--- a/hw/core/sysbus.c
+++ b/hw/core/sysbus.c
@@ -86,6 +86,54 @@ void sysbus_mmio_map_overlap(SysBusDevice *dev, int n,
hwaddr addr,
sysbus_mmio_map_common(dev, n, addr, true, priority);
}
+static void sysbus_property_set_uint32_ptr(Object *obj, Visitor *v,
+ void *opaque, const char *name,
+ Error **errp)
+{
+ DeviceState *dev = DEVICE(obj);
+
+ if (dev->realized) {
+ qdev_prop_set_after_realize(dev, name, errp);
+ return;
+ }
+
So this suggests your reasoning for side effected _ptr write is just
for validity checking. So another approach could be to add a "check"
function to the _ptr variants (rather than an open coded) setter. This
has the advantage of being consistent with what we already do for
object_property_add_link.
+ object_property_set_uint32_ptr(obj, v, opaque, name, errp);
+}
+
+static void sysbus_property_set_uint64_ptr(Object *obj, Visitor *v,
+ void *opaque, const char *name,
+ Error **errp)
+{
+ DeviceState *dev = DEVICE(obj);
+
+ if (dev->realized) {
+ qdev_prop_set_after_realize(dev, name, errp);
+ return;
+ }
+
+ object_property_set_uint64_ptr(obj, v, opaque, name, errp);
+}
+
+static void sysbus_init_prop(SysBusDevice *dev, const char *propstr, int n,
sysbus_init_int_prop.
+ object_property_add_uint32_ptr(obj, name, ptr,
+ sysbus_property_set_uint32_ptr, NULL);
+ break;
+ case sizeof(uint64_t):
+ object_property_add_uint64_ptr(obj, name, ptr,
+ sysbus_property_set_uint64_ptr, NULL);
+ break;
+ default:
+ g_assert_not_reached();
+ }
+}
+
/* Request an IRQ source. The actual IRQ object may be populated later. */
void sysbus_init_irq(SysBusDevice *dev, qemu_irq *p)
{
@@ -93,7 +141,13 @@ void sysbus_init_irq(SysBusDevice *dev, qemu_irq *p)
assert(dev->num_irq < QDEV_MAX_IRQ);
n = dev->num_irq++;
+ dev->user_irqs = g_realloc(dev->user_irqs,
+ sizeof(*dev->user_irqs) * (n + 1));
Will the QOM framework take references to this allocated area before
the final realloc? I had this problem with IRQs leading to this patch
to remove uses of realloc for QOM:
https://lists.gnu.org/archive/html/qemu-devel/2014-06/msg00051.html
+ dev->user_irqs[n] = (uint32_t)SYSBUS_DYNAMIC;
dev->irqp[n] = p;
+
+ sysbus_init_prop(dev, "irq[%d]", n, &dev->user_irqs[n],
+ sizeof(dev->user_irqs[n]));
You pass a ref to reallocable area here.
Perhaps a cleaner solution is to just malloc a single uint64_t here
locally and pass it off to QOM. Then free the malloc in the property
finalizer ...
for (i = 0; i < dev->num_irq; i++) {
- dev->irqp[i] = target->irqp[i];
+ sysbus_init_irq(dev, target->irqp[i]);
}
}
@@ -113,8 +167,14 @@ void sysbus_init_mmio(SysBusDevice *dev, MemoryRegion
*memory)
assert(dev->num_mmio < QDEV_MAX_MMIO);
n = dev->num_mmio++;
+ dev->user_mmios = g_realloc(dev->user_mmios,
+ sizeof(*dev->user_mmios) * (n + 1));
+ dev->user_mmios[n] = SYSBUS_DYNAMIC;
dev->mmio[n].addr = -1;
dev->mmio[n].memory = memory;
+
+ sysbus_init_prop(dev, "mmio[%d]", n, &dev->user_mmios[n],
+ sizeof(dev->user_mmios[n]));
You might be able to drop the %d once Paolo wildcard array property
adder stuff gets through.
}
MemoryRegion *sysbus_mmio_get_region(SysBusDevice *dev, int n)
@@ -127,8 +187,17 @@ void sysbus_init_ioports(SysBusDevice *dev, pio_addr_t
ioport, pio_addr_t size)
pio_addr_t i;
for (i = 0; i < size; i++) {
+ int n;
+
assert(dev->num_pio < QDEV_MAX_PIO);
- dev->pio[dev->num_pio++] = ioport++;
+ n = dev->num_pio++;
+ dev->user_pios = g_realloc(dev->user_pios,
+ sizeof(*dev->user_pios) * (n + 1));
+ dev->user_pios[n] = (uint32_t)SYSBUS_DYNAMIC;
+ dev->pio[n] = ioport++;
+
+ sysbus_init_prop(dev, "pio[%d]", n, &dev->user_pios[n],
+ sizeof(dev->user_pios[n]));
}
}
diff --git a/include/hw/sysbus.h b/include/hw/sysbus.h
index f5aaa05..870e7cc 100644
--- a/include/hw/sysbus.h
+++ b/include/hw/sysbus.h
@@ -9,6 +9,7 @@
#define QDEV_MAX_MMIO 32
#define QDEV_MAX_PIO 32
#define QDEV_MAX_IRQ 512
+#define SYSBUS_DYNAMIC -1ULL
#define TYPE_SYSTEM_BUS "System"
#define SYSTEM_BUS(obj) OBJECT_CHECK(IDEBus, (obj), TYPE_IDE_BUS)
@@ -56,6 +57,11 @@ struct SysBusDevice {
} mmio[QDEV_MAX_MMIO];
int num_pio;
pio_addr_t pio[QDEV_MAX_PIO];
+
+ /* These may be set by the user as hints where to map the device */
+ uint64_t *user_mmios;
+ uint32_t *user_irqs;
+ uint32_t *user_pios;
With the single malloc/free-on-finalise approach to the properties
there's no longer a need for this new state at all.