On Sat, Apr 17, 2021 at 12:30:17PM +0200, Philippe Mathieu-Daudé wrote:
> AddressSpace are physical address view and shouldn't be using
> non-zero base address. The correct way to map a MR used as AS
> root is to use a MR alias.
Today when I rethink this, I figured another way (maybe easier?) to fix the
issue.
The major problem so far we had is that mr->addr can be anything for a root mr
if it's added as subregion of another mr.
E.g. in current implementation of mtree_print_mr() MR.addr is constantly used
as an offset value:
cur_start = base + mr->addr;
However afaict mr->addr is defined as "relative offset of this mr to its
container", as in memory_region_add_subregion_common(). Say, mr->addr is
undefined from that pov if mr->container==NULL, as this MR belongs to nobody.
And if it's defined, it's only meaning is in its container's context (or say,
address space) only.
That said, when we do mtree_print_mr(), another solution could be as simple as,
not referencing mr->addr if we _know_ we're working on the root mr, as this is
definitely _not_ in the context of the mr's container, even if it has one
container after all:
---8<---
diff --git a/softmmu/memory.c b/softmmu/memory.c
index d4493ef9e43..d71fb8ecc89 100644
--- a/softmmu/memory.c
+++ b/softmmu/memory.c
@@ -2940,7 +2940,7 @@ static void mtree_print_mr(const MemoryRegion *mr, unsigned int level,
return;
}
- cur_start = base + mr->addr;
+ cur_start = base + (level == 1) ? 0 : mr->addr;
cur_end = cur_start + MR_SIZE(mr->size);
/*
---8<---
Phil, do you think it'll work too to fix the strange offset value dumped in
"info mtree"?
I don't know (even if it works, perhaps I've missed something) which is better,
as current series seems cleaner, then any mr will either belong to a AS or a MR
(never both!), but just raise it up.
Thanks,
--
Peter Xu