Alex Williamson <alex.william...@redhat.com> writes: > When we're trying to get a newly registered phys memory client updated > with the current page mappings, we end up passing the region offset > (a ram_addr_t) as the start address rather than the actual guest > physical memory address (target_phys_addr_t). If your guest has less > than 3.5G of memory, these are coincidentally the same thing. If > there's more, the region offset for the memory above 4G starts over > at 0, so the set_memory client will overwrite it's lower memory entries. > > Instead, keep track of the guest phsyical address as we're walking the > tables and pass that to the set_memory client. > > Signed-off-by: Alex Williamson <alex.william...@redhat.com> > --- > > exec.c | 10 ++++++---- > 1 files changed, 6 insertions(+), 4 deletions(-) > > diff --git a/exec.c b/exec.c > index 4752af1..e670929 100644 > --- a/exec.c > +++ b/exec.c > @@ -1742,7 +1742,7 @@ static int cpu_notify_migration_log(int enable) > } > > static void phys_page_for_each_1(CPUPhysMemoryClient *client, > - int level, void **lp) > + int level, void **lp, target_phys_addr_t > addr) > { > int i; >
Aren't you abusing target_phys_addr_t here? It's not a physical address, it needs to be shifted left to become one. By how much depends on level. Please take pity on future maintainers and spell this out in a comment. Perhaps you can code it in a way that makes the parameter an address. Probably no need for a comment then. > @@ -1751,16 +1751,18 @@ static void phys_page_for_each_1(CPUPhysMemoryClient > *client, > } > if (level == 0) { > PhysPageDesc *pd = *lp; > + addr <<= L2_BITS + TARGET_PAGE_BITS; > for (i = 0; i < L2_SIZE; ++i) { > if (pd[i].phys_offset != IO_MEM_UNASSIGNED) { > - client->set_memory(client, pd[i].region_offset, > + client->set_memory(client, addr | i << TARGET_PAGE_BITS, > TARGET_PAGE_SIZE, pd[i].phys_offset); > } > } > } else { > void **pp = *lp; > for (i = 0; i < L2_SIZE; ++i) { > - phys_page_for_each_1(client, level - 1, pp + i); > + phys_page_for_each_1(client, level - 1, pp + i, > + (addr << L2_BITS) | i); > } > } > } > @@ -1770,7 +1772,7 @@ static void phys_page_for_each(CPUPhysMemoryClient > *client) > int i; > for (i = 0; i < P_L1_SIZE; ++i) { > phys_page_for_each_1(client, P_L1_SHIFT / L2_BITS - 1, > - l1_phys_map + i); > + l1_phys_map + i, i); > } > } > Fix makes sense to me, after some head scratching. A comment explaining the phys map data structure would be helpful. l1_phys_map[] has a comment, but it's devoid of detail.