diff mbox

[RFC,v1,19/25] sysbus: Setup memory regions as dynamic props

Message ID 7ae1d126849aed1eee8a3bf2fb0021efdd38e635.1400204799.git.peter.crosthwaite@xilinx.com
State New
Headers show

Commit Message

Peter Crosthwaite May 16, 2014, 2 a.m. UTC
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(-)

Comments

Peter Crosthwaite May 29, 2014, 6:22 a.m. UTC | #1
On Fri, May 16, 2014 at 12:00 PM, Peter Crosthwaite
<peter.crosthwaite@xilinx.com> wrote:
> 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);

So relying on sysbus to do the MR parenting doesn't work too well when
you have a mix of sysbus and non-sysbus MRs in the system. It also
increases the sysbus API incumbency. I thinks better to let the Memory
API do the QOM parenting to the owner argument in memory_region_init
(It's an object after all). If you pass NULL to memory_region_init,
then you are responsible for parenting the MR yourself.

This child-adder is then converted to a regular link-adder for sysbus's sake.

Regards,
Peter
diff mbox

Patch

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];
 };