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

[Philippe Mathieu-Daudé]

+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.