[2/3] memory: Provide 'base address' argument to mtree_print_mr()

Currently AdressSpace are display in 'info mtree' based on
the physical address of their first MemoryRegion. This is
rather confusing.

Provide a 'base' address argument to mtree_print_mr() and
use it in mtree_info() to display AdressSpace always based
at address 0.

Display behavior of MemoryRegions and FlatViews is not modified.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 softmmu/memory.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

Phil,

On Sat, Mar 06, 2021 at 12:54:13AM +0100, Philippe Mathieu-Daudé wrote:
> @@ -3188,14 +3188,15 @@ void mtree_info(bool flatview, bool dispatch_tree, bool owner, bool disabled)
>  
>      QTAILQ_FOREACH(as, &address_spaces, address_spaces_link) {
>          qemu_printf("address-space: %s\n", as->name);
> -        mtree_print_mr(as->root, 1, 0, &ml_head, owner, disabled);
> +        mtree_print_mr(as->root, 1, 0, as->root->addr,

Root MR of any address space should have mr->addr==0, right?

I'm slightly confused on what this patch wanted to do if so, since then "base"
will always be zero..  Or am I wrong?

Thanks,

> +                       &ml_head, owner, disabled);
>          qemu_printf("\n");
>      }
>  
>      /* print aliased regions */
>      QTAILQ_FOREACH(ml, &ml_head, mrqueue) {
>          qemu_printf("memory-region: %s\n", memory_region_name(ml->mr));
> -        mtree_print_mr(ml->mr, 1, 0, &ml_head, owner, disabled);
> +        mtree_print_mr(ml->mr, 1, 0, 0, &ml_head, owner, disabled);
>          qemu_printf("\n");
>      }
>  
> -- 
> 2.26.2
>

Hi Peter,

On 3/9/21 12:40 AM, Peter Xu wrote:> On Sat, Mar 06, 2021 at 12:54:13AM
+0100, Philippe Mathieu-Daudé wrote:
>> @@ -3188,14 +3188,15 @@ void mtree_info(bool flatview, bool dispatch_tree, bool owner, bool disabled)
>>  
>>      QTAILQ_FOREACH(as, &address_spaces, address_spaces_link) {
>>          qemu_printf("address-space: %s\n", as->name);
>> -        mtree_print_mr(as->root, 1, 0, &ml_head, owner, disabled);
>> +        mtree_print_mr(as->root, 1, 0, as->root->addr,
> 
> Root MR of any address space should have mr->addr==0, right?
> 
> I'm slightly confused on what this patch wanted to do if so, since then "base"
> will always be zero..  Or am I wrong?

That is what I am expecting too... Maybe the problem is elsewhere
when I create the address space... The simpler way to
figure it out is add an assertion. I haven't figure out my
issue yet, I'll follow up later with a proof-of-concept series
which triggers the assertion.

FYI I also have to modify:

-- >8 --
diff --git a/softmmu/physmem.c b/softmmu/physmem.c
index e19bc9f1c19..41a77e15752 100644
--- a/softmmu/physmem.c
+++ b/softmmu/physmem.c
@@ -2889,7 +2889,7 @@ MemTxResult address_space_read_full(AddressSpace
*as, hwaddr addr,
     if (len > 0) {
         RCU_READ_LOCK_GUARD();
         fv = address_space_to_flatview(as);
-        result = flatview_read(fv, addr, attrs, buf, len);
+        result = flatview_read(fv, as->root->addr + addr, attrs, buf, len);
     }

     return result;
@@ -2905,7 +2905,7 @@ MemTxResult address_space_write(AddressSpace *as,
hwaddr addr,
     if (len > 0) {
         RCU_READ_LOCK_GUARD();
         fv = address_space_to_flatview(as);
-        result = flatview_write(fv, addr, attrs, buf, len);
+        result = flatview_write(fv, as->root->addr + addr, attrs, buf,
len);
     }

     return result;
@@ -3117,7 +3117,8 @@ bool address_space_access_valid(AddressSpace *as,
hwaddr addr,

     RCU_READ_LOCK_GUARD();
     fv = address_space_to_flatview(as);
-    result = flatview_access_valid(fv, addr, len, is_write, attrs);
+    result = flatview_access_valid(fv, as->root->addr + addr, len,
+                                   is_write, attrs);
     return result;
 }
---

+Peter/Mark/Edgar for SoC modelling

On 3/9/21 10:39 AM, Philippe Mathieu-Daudé wrote:
> Hi Peter,
> 
> On 3/9/21 12:40 AM, Peter Xu wrote:> On Sat, Mar 06, 2021 at 12:54:13AM
> +0100, Philippe Mathieu-Daudé wrote:
>>> @@ -3188,14 +3188,15 @@ void mtree_info(bool flatview, bool dispatch_tree, bool owner, bool disabled)
>>>  
>>>      QTAILQ_FOREACH(as, &address_spaces, address_spaces_link) {
>>>          qemu_printf("address-space: %s\n", as->name);
>>> -        mtree_print_mr(as->root, 1, 0, &ml_head, owner, disabled);
>>> +        mtree_print_mr(as->root, 1, 0, as->root->addr,
>>
>> Root MR of any address space should have mr->addr==0, right?
>>
>> I'm slightly confused on what this patch wanted to do if so, since then "base"
>> will always be zero..  Or am I wrong?
> 
> That is what I am expecting too... Maybe the problem is elsewhere
> when I create the address space... The simpler way to
> figure it out is add an assertion. I haven't figure out my
> issue yet, I'll follow up later with a proof-of-concept series
> which triggers the assertion.

To trigger I simply use:

mydevice_realize()
{
  memory_region_init(&mr, obj, name, size);

  address_space_init(&as, &mr, name);
  // here we have as.root.addr = 0

  sysbus_init_mmio(sbd, &mr);
}

soc_realize()
{
    ...
    sysbus_realize(SYS_BUS_DEVICE(&mydevice), &error_abort);
    sysbus_mmio_map(SYS_BUS_DEVICE(&mydevice), 0, NONZERO_ADDRESS);
    ...
}

sysbus_mmio_map
-> sysbus_mmio_map_common
   -> memory_region_add_subregion
      -> memory_region_add_subregion_common

Which does:

static void memory_region_add_subregion_common(MemoryRegion *mr,
                                               hwaddr offset,
                                               MemoryRegion *subregion)
{
    assert(!subregion->container);
    subregion->container = mr;
    subregion->addr = offset;
    // subregion = &as.root
    // offset = NONZERO_ADDRESS
    // so here as.root.addr becomes non-zero

Is that use case incorrect? If so:
- where to add the proper assertions to avoid having address spaces
  with non-zero base address?
- what is the proper design?

Else what should we fix?

Thanks,

Phil.

Phil,

On Tue, Mar 09, 2021 at 10:54:18PM +0100, Philippe Mathieu-Daudé wrote:
> +Peter/Mark/Edgar for SoC modelling
> 
> On 3/9/21 10:39 AM, Philippe Mathieu-Daudé wrote:
> > Hi Peter,
> > 
> > On 3/9/21 12:40 AM, Peter Xu wrote:> On Sat, Mar 06, 2021 at 12:54:13AM
> > +0100, Philippe Mathieu-Daudé wrote:
> >>> @@ -3188,14 +3188,15 @@ void mtree_info(bool flatview, bool dispatch_tree, bool owner, bool disabled)
> >>>  
> >>>      QTAILQ_FOREACH(as, &address_spaces, address_spaces_link) {
> >>>          qemu_printf("address-space: %s\n", as->name);
> >>> -        mtree_print_mr(as->root, 1, 0, &ml_head, owner, disabled);
> >>> +        mtree_print_mr(as->root, 1, 0, as->root->addr,
> >>
> >> Root MR of any address space should have mr->addr==0, right?
> >>
> >> I'm slightly confused on what this patch wanted to do if so, since then "base"
> >> will always be zero..  Or am I wrong?
> > 
> > That is what I am expecting too... Maybe the problem is elsewhere
> > when I create the address space... The simpler way to
> > figure it out is add an assertion. I haven't figure out my
> > issue yet, I'll follow up later with a proof-of-concept series
> > which triggers the assertion.
> 
> To trigger I simply use:
> 
> mydevice_realize()
> {
>   memory_region_init(&mr, obj, name, size);
> 
>   address_space_init(&as, &mr, name);

Could I ask why you need to set this sysbus mmio region as root MR of as?
What's AS used for here?

Btw, normally I see these regions should be initialized with
memory_region_init_io(), since normally that MR will need a MemoryRegionOps
bound to it to trap MMIOs, iiuc.

Thanks,

>   // here we have as.root.addr = 0
> 
>   sysbus_init_mmio(sbd, &mr);
> }

On 3/10/21 6:06 PM, Peter Xu wrote:
> Phil,
> 
> On Tue, Mar 09, 2021 at 10:54:18PM +0100, Philippe Mathieu-Daudé wrote:
>> +Peter/Mark/Edgar for SoC modelling
>>
>> On 3/9/21 10:39 AM, Philippe Mathieu-Daudé wrote:
>>> Hi Peter,
>>>
>>> On 3/9/21 12:40 AM, Peter Xu wrote:> On Sat, Mar 06, 2021 at 12:54:13AM
>>> +0100, Philippe Mathieu-Daudé wrote:
>>>>> @@ -3188,14 +3188,15 @@ void mtree_info(bool flatview, bool dispatch_tree, bool owner, bool disabled)
>>>>>  
>>>>>      QTAILQ_FOREACH(as, &address_spaces, address_spaces_link) {
>>>>>          qemu_printf("address-space: %s\n", as->name);
>>>>> -        mtree_print_mr(as->root, 1, 0, &ml_head, owner, disabled);
>>>>> +        mtree_print_mr(as->root, 1, 0, as->root->addr,
>>>>
>>>> Root MR of any address space should have mr->addr==0, right?
>>>>
>>>> I'm slightly confused on what this patch wanted to do if so, since then "base"
>>>> will always be zero..  Or am I wrong?
>>>
>>> That is what I am expecting too... Maybe the problem is elsewhere
>>> when I create the address space... The simpler way to
>>> figure it out is add an assertion. I haven't figure out my
>>> issue yet, I'll follow up later with a proof-of-concept series
>>> which triggers the assertion.
>>
>> To trigger I simply use:
>>
>> mydevice_realize()
>> {
>>   memory_region_init(&mr, obj, name, size);
>>
>>   address_space_init(&as, &mr, name);
> 
> Could I ask why you need to set this sysbus mmio region as root MR of as?
> What's AS used for here?
> 
> Btw, normally I see these regions should be initialized with
> memory_region_init_io(), since normally that MR will need a MemoryRegionOps
> bound to it to trap MMIOs, iiuc.

This is the pattern for buses:

static PCIDevice *do_pci_register_device(PCIDevice *pci_dev,
                                         const char *name, int devfn,
                                         Error **errp)
{
    ...
    memory_region_init(&pci_dev->bus_master_container_region,
                       OBJECT(pci_dev),
                       "bus master container", UINT64_MAX);
    address_space_init(&pci_dev->bus_master_as,
                       &pci_dev->bus_master_container_region,
                       pci_dev->name);
    ....

AUXBus *aux_bus_init(DeviceState *parent, const char *name)
{
    AUXBus *bus;
    Object *auxtoi2c;

    bus = AUX_BUS(qbus_create(TYPE_AUX_BUS, parent, name));
    auxtoi2c = object_new_with_props(TYPE_AUXTOI2C, OBJECT(bus), "i2c",
                                     &error_abort, NULL);

    bus->bridge = AUXTOI2C(auxtoi2c);

    /* Memory related. */
    bus->aux_io = g_malloc(sizeof(*bus->aux_io));
    memory_region_init(bus->aux_io, OBJECT(bus), "aux-io", 1 * MiB);
    address_space_init(&bus->aux_addr_space, bus->aux_io, "aux-io");
    return bus;
}

static void artist_realizefn(DeviceState *dev, Error **errp)
{
    ...
    memory_region_init(&s->mem_as_root, OBJECT(dev), "artist", ~0ull);
    address_space_init(&s->as, &s->mem_as_root, "artist");

PCI host:

PCIBus *gt64120_register(qemu_irq *pic)
{
    ...
    memory_region_init(&d->pci0_mem, OBJECT(dev), "pci0-mem", 4 * GiB);
    address_space_init(&d->pci0_mem_as, &d->pci0_mem, "pci0-mem");

Also:

static void pnv_xive_realize(DeviceState *dev, Error **errp)
{
    ...
    /*
     * The XiveSource and XiveENDSource objects are realized with the
     * maximum allowed HW configuration. The ESB MMIO regions will be
     * resized dynamically when the controller is configured by the FW
     * to limit accesses to resources not provisioned.
     */
    memory_region_init(&xive->ipi_mmio, OBJECT(xive), "xive-vc-ipi",
                       PNV9_XIVE_VC_SIZE);
    address_space_init(&xive->ipi_as, &xive->ipi_mmio, "xive-vc-ipi");
    memory_region_init(&xive->end_mmio, OBJECT(xive), "xive-vc-end",
                       PNV9_XIVE_VC_SIZE);
    address_space_init(&xive->end_as, &xive->end_mmio, "xive-vc-end");

And:

static void memory_map_init(void)
{
    ...
    memory_region_init(system_memory, NULL, "system", UINT64_MAX);
    address_space_init(&address_space_memory, system_memory, "memory");

Or:

static AddressSpace *amdvi_host_dma_iommu(PCIBus *bus, void *opaque,
                                          int devfn)
{
    ...
    /*
     * Memory region relationships looks like (Address range shows
     * only lower 32 bits to make it short in length...):
     *
     * |-----------------+-------------------+----------|
     * | Name            | Address range     | Priority |
     * |-----------------+-------------------+----------+
     * | amdvi_root      | 00000000-ffffffff |        0 |
     * |  amdvi_iommu    | 00000000-ffffffff |        1 |
     * |  amdvi_iommu_ir | fee00000-feefffff |       64 |
     * |-----------------+-------------------+----------|
     */
    memory_region_init(&amdvi_dev_as->root, OBJECT(s),
                       "amdvi_root", UINT64_MAX);
    address_space_init(&amdvi_dev_as->as, &amdvi_dev_as->root, name);
    memory_region_init_io(&amdvi_dev_as->iommu_ir, OBJECT(s),
                          &amdvi_ir_ops, s, "amd_iommu_ir",
                          AMDVI_INT_ADDR_SIZE);
    memory_region_add_subregion_overlap(&amdvi_dev_as->root,
                                        AMDVI_INT_ADDR_FIRST,
                                        &amdvi_dev_as->iommu_ir,
                                        64);
    memory_region_add_subregion_overlap(&amdvi_dev_as->root, 0,
                           MEMORY_REGION(&amdvi_dev_as->iommu), 1);

I'll send a reproducer.

On Wed, Mar 10, 2021 at 08:09:59PM +0100, Philippe Mathieu-Daudé wrote:
> On 3/10/21 6:06 PM, Peter Xu wrote:
> > Phil,
> > 
> > On Tue, Mar 09, 2021 at 10:54:18PM +0100, Philippe Mathieu-Daudé wrote:
> >> +Peter/Mark/Edgar for SoC modelling
> >>
> >> On 3/9/21 10:39 AM, Philippe Mathieu-Daudé wrote:
> >>> Hi Peter,
> >>>
> >>> On 3/9/21 12:40 AM, Peter Xu wrote:> On Sat, Mar 06, 2021 at 12:54:13AM
> >>> +0100, Philippe Mathieu-Daudé wrote:
> >>>>> @@ -3188,14 +3188,15 @@ void mtree_info(bool flatview, bool dispatch_tree, bool owner, bool disabled)
> >>>>>  
> >>>>>      QTAILQ_FOREACH(as, &address_spaces, address_spaces_link) {
> >>>>>          qemu_printf("address-space: %s\n", as->name);
> >>>>> -        mtree_print_mr(as->root, 1, 0, &ml_head, owner, disabled);
> >>>>> +        mtree_print_mr(as->root, 1, 0, as->root->addr,
> >>>>
> >>>> Root MR of any address space should have mr->addr==0, right?
> >>>>
> >>>> I'm slightly confused on what this patch wanted to do if so, since then "base"
> >>>> will always be zero..  Or am I wrong?
> >>>
> >>> That is what I am expecting too... Maybe the problem is elsewhere
> >>> when I create the address space... The simpler way to
> >>> figure it out is add an assertion. I haven't figure out my
> >>> issue yet, I'll follow up later with a proof-of-concept series
> >>> which triggers the assertion.
> >>
> >> To trigger I simply use:
> >>
> >> mydevice_realize()
> >> {
> >>   memory_region_init(&mr, obj, name, size);
> >>
> >>   address_space_init(&as, &mr, name);
> > 
> > Could I ask why you need to set this sysbus mmio region as root MR of as?
> > What's AS used for here?
> > 
> > Btw, normally I see these regions should be initialized with
> > memory_region_init_io(), since normally that MR will need a MemoryRegionOps
> > bound to it to trap MMIOs, iiuc.
> 
> This is the pattern for buses:
> 
> static PCIDevice *do_pci_register_device(PCIDevice *pci_dev,
>                                          const char *name, int devfn,
>                                          Error **errp)
> {
>     ...
>     memory_region_init(&pci_dev->bus_master_container_region,
>                        OBJECT(pci_dev),
>                        "bus master container", UINT64_MAX);
>     address_space_init(&pci_dev->bus_master_as,
>                        &pci_dev->bus_master_container_region,
>                        pci_dev->name);
>     ....

This one is the pci bus address space container as its name shows, IMHO it
should never be added as subregion of another MR, but only the top parent.

> 
> AUXBus *aux_bus_init(DeviceState *parent, const char *name)
> {
>     AUXBus *bus;
>     Object *auxtoi2c;
> 
>     bus = AUX_BUS(qbus_create(TYPE_AUX_BUS, parent, name));
>     auxtoi2c = object_new_with_props(TYPE_AUXTOI2C, OBJECT(bus), "i2c",
>                                      &error_abort, NULL);
> 
>     bus->bridge = AUXTOI2C(auxtoi2c);
> 
>     /* Memory related. */
>     bus->aux_io = g_malloc(sizeof(*bus->aux_io));
>     memory_region_init(bus->aux_io, OBJECT(bus), "aux-io", 1 * MiB);
>     address_space_init(&bus->aux_addr_space, bus->aux_io, "aux-io");
>     return bus;
> }

This one seems similar - it's only used in aux_map_slave() as the parent MR, so
its mr->addr should still always be zero.

> 
> static void artist_realizefn(DeviceState *dev, Error **errp)
> {
>     ...
>     memory_region_init(&s->mem_as_root, OBJECT(dev), "artist", ~0ull);
>     address_space_init(&s->as, &s->mem_as_root, "artist");

This one is similar with above - mem_as_root is the top MR.

> 
> PCI host:
> 
> PCIBus *gt64120_register(qemu_irq *pic)
> {
>     ...
>     memory_region_init(&d->pci0_mem, OBJECT(dev), "pci0-mem", 4 * GiB);
>     address_space_init(&d->pci0_mem_as, &d->pci0_mem, "pci0-mem");

For this one, I even think pci0_mem_as can be removed since it's never
referenced after initialized..

> 
> Also:
> 
> static void pnv_xive_realize(DeviceState *dev, Error **errp)
> {
>     ...
>     /*
>      * The XiveSource and XiveENDSource objects are realized with the
>      * maximum allowed HW configuration. The ESB MMIO regions will be
>      * resized dynamically when the controller is configured by the FW
>      * to limit accesses to resources not provisioned.
>      */
>     memory_region_init(&xive->ipi_mmio, OBJECT(xive), "xive-vc-ipi",
>                        PNV9_XIVE_VC_SIZE);
>     address_space_init(&xive->ipi_as, &xive->ipi_mmio, "xive-vc-ipi");
>     memory_region_init(&xive->end_mmio, OBJECT(xive), "xive-vc-end",
>                        PNV9_XIVE_VC_SIZE);
>     address_space_init(&xive->end_as, &xive->end_mmio, "xive-vc-end");

Similar here - both are top MRs.

> 
> And:
> 
> static void memory_map_init(void)
> {
>     ...
>     memory_region_init(system_memory, NULL, "system", UINT64_MAX);
>     address_space_init(&address_space_memory, system_memory, "memory");

This is the system address space, system_memory should never be added as a
subregion to other memory region, afaiu..

> 
> Or:
> 
> static AddressSpace *amdvi_host_dma_iommu(PCIBus *bus, void *opaque,
>                                           int devfn)
> {
>     ...
>     /*
>      * Memory region relationships looks like (Address range shows
>      * only lower 32 bits to make it short in length...):
>      *
>      * |-----------------+-------------------+----------|
>      * | Name            | Address range     | Priority |
>      * |-----------------+-------------------+----------+
>      * | amdvi_root      | 00000000-ffffffff |        0 |
>      * |  amdvi_iommu    | 00000000-ffffffff |        1 |
>      * |  amdvi_iommu_ir | fee00000-feefffff |       64 |
>      * |-----------------+-------------------+----------|
>      */
>     memory_region_init(&amdvi_dev_as->root, OBJECT(s),
>                        "amdvi_root", UINT64_MAX);
>     address_space_init(&amdvi_dev_as->as, &amdvi_dev_as->root, name);
>     memory_region_init_io(&amdvi_dev_as->iommu_ir, OBJECT(s),
>                           &amdvi_ir_ops, s, "amd_iommu_ir",
>                           AMDVI_INT_ADDR_SIZE);
>     memory_region_add_subregion_overlap(&amdvi_dev_as->root,
>                                         AMDVI_INT_ADDR_FIRST,
>                                         &amdvi_dev_as->iommu_ir,
>                                         64);
>     memory_region_add_subregion_overlap(&amdvi_dev_as->root, 0,
>                            MEMORY_REGION(&amdvi_dev_as->iommu), 1);

I think this is similar too.

The thing is in your example you passed over the root MR to sysbus_init_mmio()
where it would be further added into the system memory layout as sub-mr.  Maybe
you should create two MRs, one as root MR, the other one as the one passed into
sysbus_init_mmio()?

On 09/03/2021 21:54, Philippe Mathieu-Daudé wrote:

> +Peter/Mark/Edgar for SoC modelling
> 
> On 3/9/21 10:39 AM, Philippe Mathieu-Daudé wrote:
>> Hi Peter,
>>
>> On 3/9/21 12:40 AM, Peter Xu wrote:> On Sat, Mar 06, 2021 at 12:54:13AM
>> +0100, Philippe Mathieu-Daudé wrote:
>>>> @@ -3188,14 +3188,15 @@ void mtree_info(bool flatview, bool dispatch_tree, bool owner, bool disabled)
>>>>   
>>>>       QTAILQ_FOREACH(as, &address_spaces, address_spaces_link) {
>>>>           qemu_printf("address-space: %s\n", as->name);
>>>> -        mtree_print_mr(as->root, 1, 0, &ml_head, owner, disabled);
>>>> +        mtree_print_mr(as->root, 1, 0, as->root->addr,
>>>
>>> Root MR of any address space should have mr->addr==0, right?
>>>
>>> I'm slightly confused on what this patch wanted to do if so, since then "base"
>>> will always be zero..  Or am I wrong?
>>
>> That is what I am expecting too... Maybe the problem is elsewhere
>> when I create the address space... The simpler way to
>> figure it out is add an assertion. I haven't figure out my
>> issue yet, I'll follow up later with a proof-of-concept series
>> which triggers the assertion.
> 
> To trigger I simply use:
> 
> mydevice_realize()
> {
>    memory_region_init(&mr, obj, name, size);
> 
>    address_space_init(&as, &mr, name);
>    // here we have as.root.addr = 0
> 
>    sysbus_init_mmio(sbd, &mr);
> }
> 
> soc_realize()
> {
>      ...
>      sysbus_realize(SYS_BUS_DEVICE(&mydevice), &error_abort);
>      sysbus_mmio_map(SYS_BUS_DEVICE(&mydevice), 0, NONZERO_ADDRESS);
>      ...
> }
> 
> sysbus_mmio_map
> -> sysbus_mmio_map_common
>     -> memory_region_add_subregion
>        -> memory_region_add_subregion_common
> 
> Which does:
> 
> static void memory_region_add_subregion_common(MemoryRegion *mr,
>                                                 hwaddr offset,
>                                                 MemoryRegion *subregion)
> {
>      assert(!subregion->container);
>      subregion->container = mr;
>      subregion->addr = offset;
>      // subregion = &as.root
>      // offset = NONZERO_ADDRESS
>      // so here as.root.addr becomes non-zero
> 
> Is that use case incorrect? If so:
> - where to add the proper assertions to avoid having address spaces
>    with non-zero base address?
> - what is the proper design?
> 
> Else what should we fix?

I think I've seen something like this before with the sun4u IOMMU - I tried to get 
the offset of the IOMMU address space memory region from its parent in 
sun4u_translate_iommu() but I ended up getting back an absolute address instead. I 
think the conclusion I came to was that it wouldn't work with a memory region at the 
root of an address space, so I'd be interested to know what the behaviour here should be.

In terms of soc_realize() I believe there are 2 options for getting the memory 
region: 1) use object_resolve_path_component() to retrieve the memory region as a QOM 
property or 2) use sysbus_mmio_get_region(). Once you have a reference to the memory 
region you can add it to the parent using memory_region_add_subregion() as needed. 
There is an existing example of 2) in hw/sparc64/sun4u.c in ebus_realize().


ATB,

Mark.