Re: [Qemu-devel] [PATCH 2.1 17/28] memory: move mem_path handling to memory_region_allocate_system_memory

[Hu Tao]

On Tue, Mar 04, 2014 at 03:00:45PM +0100, Paolo Bonzini wrote:
> Like the previous patch did in exec.c, split memory_region_init_ram and
> memory_region_init_ram_from_file, and push mem_path one step further up.
> Other RAM regions than system memory will now be backed by regular RAM.

This changes qemu's behaviour regarding using hugetlbfs, especially when
size of other RAM regions is significant compared to system memory. Will
this a problem?(compatibilities, user configurations...)

> 
> Also, boards that do not use memory_region_allocate_system_memory will
> not support -mem-path anymore.  This can be changed before the patches
> are merged by migrating boards to use the function.

IIUC, memory_region_allocate_system_memory() is only called once at
board initialization time. In most cases, it just works. But there
are cases that system memory is not initialized by a single call to
memory_region_allocate_system_memory() that total memory is splitted
into banks each of which is initialized individually, see
ppc4xx_sdram_adjust(). How to convert to
memory_region_allocate_system_memory() in this case? Should we map
banks into numa nodes?

> 
> Signed-off-by: Paolo Bonzini <address@hidden>
> ---
>  exec.c                | 10 ++--------
>  include/exec/memory.h | 18 ++++++++++++++++++
>  memory.c              | 21 ++++++++++++++++-----
>  numa.c                | 11 ++++++++++-
>  4 files changed, 46 insertions(+), 14 deletions(-)
> 
> diff --git a/exec.c b/exec.c
> index 0aa4947..4f05584 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -1123,14 +1123,6 @@ static void *file_ram_alloc(RAMBlock *block,
>      block->fd = fd;
>      return area;
>  }
> -#else
> -static void *file_ram_alloc(RAMBlock *block,
> -                            ram_addr_t memory,
> -                            const char *path)
> -{
> -    fprintf(stderr, "-mem-path not supported on this host\n");
> -    exit(1);
> -}
>  #endif
>  
>  static ram_addr_t find_ram_offset(ram_addr_t size)
> @@ -1304,6 +1296,7 @@ static ram_addr_t ram_block_add(RAMBlock *new_block)
>      return new_block->offset;
>  }
>  
> +#ifdef __linux__
>  ram_addr_t qemu_ram_alloc_from_file(ram_addr_t size, MemoryRegion *mr,
>                                      const char *mem_path)
>  {
> @@ -1332,6 +1325,7 @@ ram_addr_t qemu_ram_alloc_from_file(ram_addr_t size, 
> MemoryRegion *mr,
>      new_block->host = file_ram_alloc(new_block, size, mem_path);
>      return ram_block_add(new_block);
>  }
> +#endif
>  
>  ram_addr_t qemu_ram_alloc_from_ptr(ram_addr_t size, void *host,
>                                     MemoryRegion *mr)
> diff --git a/include/exec/memory.h b/include/exec/memory.h
> index 9101fc3..54bdb4d 100644
> --- a/include/exec/memory.h
> +++ b/include/exec/memory.h
> @@ -311,6 +311,24 @@ void memory_region_init_ram(MemoryRegion *mr,
>                              const char *name,
>                              uint64_t size);
>  
> +#ifdef __linux__
> +/**
> + * memory_region_init_ram_from_file:  Initialize RAM memory region with a
> + *                                    mmap-ed backend.
> + *
> + * @mr: the #MemoryRegion to be initialized.
> + * @owner: the object that tracks the region's reference count
> + * @name: the name of the region.
> + * @size: size of the region.
> + * @path: the path in which to allocate the RAM.
> + */
> +void memory_region_init_ram_from_file(MemoryRegion *mr,
> +                                      struct Object *owner,
> +                                      const char *name,
> +                                      uint64_t size,
> +                                      const char *path);
> +#endif
> +
>  /**
>   * memory_region_init_ram_ptr:  Initialize RAM memory region from a
>   *                              user-provided pointer.  Accesses into the
> diff --git a/memory.c b/memory.c
> index 32b17a8..1636351 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -1017,13 +1017,24 @@ void memory_region_init_ram(MemoryRegion *mr,
>      mr->ram = true;
>      mr->terminates = true;
>      mr->destructor = memory_region_destructor_ram;
> -    if (mem_path) {
> -        mr->ram_addr = qemu_ram_alloc_from_file(size, mr, mem_path);
> -    } else {
> -        mr->ram_addr = qemu_ram_alloc(size, mr);
> -    }
> +    mr->ram_addr = qemu_ram_alloc(size, mr);
>  }
>  
> +#ifdef __linux__
> +void memory_region_init_ram_from_file(MemoryRegion *mr,
> +                                      struct Object *owner,
> +                                      const char *name,
> +                                      uint64_t size,
> +                                      const char *path)
> +{
> +    memory_region_init(mr, owner, name, size);
> +    mr->ram = true;
> +    mr->terminates = true;
> +    mr->destructor = memory_region_destructor_ram;
> +    mr->ram_addr = qemu_ram_alloc_from_file(size, mr, path);
> +}
> +#endif
> +
>  void memory_region_init_ram_ptr(MemoryRegion *mr,
>                                  Object *owner,
>                                  const char *name,
> diff --git a/numa.c b/numa.c
> index b00ef90..1afa017 100644
> --- a/numa.c
> +++ b/numa.c
> @@ -228,7 +228,16 @@ static void allocate_system_memory_nonnuma(MemoryRegion 
> *mr, Object *owner,
>  {
>      uint64_t ram_size = args->ram_size;
>  
> -    memory_region_init_ram(mr, owner, name, ram_size);
> +    if (mem_path) {
> +#ifdef __linux__
> +        memory_region_init_ram_from_file(mr, owner, name, ram_size, 
> mem_path);
> +#else
> +        fprintf(stderr, "-mem-path not supported on this host\n");
> +        exit(1);
> +#endif
> +    } else {
> +        memory_region_init_ram(mr, owner, name, ram_size);
> +    }
>      vmstate_register_ram_global(mr);
>  }
>  
> -- 
> 1.8.5.3
>