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

[Peter Xu]

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

-- 
Peter Xu