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

[Peter Crosthwaite]

On Fri, May 16, 2014 at 12:00 PM, Peter Crosthwaite
<address@hidden> 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 <address@hidden>
> ---
>
>  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