Re: [Qemu-devel] [PATCH] Fix phys memory client - pass guest physical address not region offset

[Michael S. Tsirkin]

On Thu, Apr 28, 2011 at 09:15:23PM -0600, Alex Williamson wrote:
> 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 <address@hidden>

Acked-by: Michael S. Tsirkin <address@hidden>

Given all this, can yo tell how much time does
it take to hotplug a device with, say, a 40G RAM guest?
> ---
> 
>  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;
>  
> @@ -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);
>      }
>  }
>