Re: [PATCH v4 43/80] x86/pc: use memdev for RAM

[Michael S. Tsirkin]

On Fri, Jan 31, 2020 at 04:09:13PM +0100, Igor Mammedov wrote:
> memory_region_allocate_system_memory() API is going away, so
> replace it with memdev allocated MemoryRegion. The later is
> initialized by generic code, so board only needs to opt in
> to memdev scheme by providing
>   MachineClass::default_ram_id
> and using MachineState::ram instead of manually initializing
> RAM memory region.
> 
> Signed-off-by: Igor Mammedov <address@hidden>


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

But see by other comment - I think a bool flag to opt in,
that can then be dropped for non-versioned boards would be nicer.

E.g. use_memdev_for_ram.



> ---
> CC: address@hidden
> CC: address@hidden
> CC: address@hidden
> CC: address@hidden
> ---
>  hw/i386/pc.c | 19 +++++++++----------
>  1 file changed, 9 insertions(+), 10 deletions(-)
> 
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 2ddce42..6ab4acb 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -937,7 +937,7 @@ void pc_memory_init(PCMachineState *pcms,
>                      MemoryRegion **ram_memory)
>  {
>      int linux_boot, i;
> -    MemoryRegion *ram, *option_rom_mr;
> +    MemoryRegion *option_rom_mr;
>      MemoryRegion *ram_below_4g, *ram_above_4g;
>      FWCfgState *fw_cfg;
>      MachineState *machine = MACHINE(pcms);
> @@ -950,22 +950,20 @@ void pc_memory_init(PCMachineState *pcms,
>  
>      linux_boot = (machine->kernel_filename != NULL);
>  
> -    /* Allocate RAM.  We allocate it as a single memory region and use
> -     * aliases to address portions of it, mostly for backwards compatibility
> -     * with older qemus that used qemu_ram_alloc().
> +    /*
> +     * Split single memory region and use aliases to address portions of it,
> +     * done for backwards compatibility with older qemus.
>       */
> -    ram = g_malloc(sizeof(*ram));
> -    memory_region_allocate_system_memory(ram, NULL, "pc.ram",
> -                                         machine->ram_size);
> -    *ram_memory = ram;
> +    *ram_memory = machine->ram;
>      ram_below_4g = g_malloc(sizeof(*ram_below_4g));
> -    memory_region_init_alias(ram_below_4g, NULL, "ram-below-4g", ram,
> +    memory_region_init_alias(ram_below_4g, NULL, "ram-below-4g", 
> machine->ram,
>                               0, x86ms->below_4g_mem_size);
>      memory_region_add_subregion(system_memory, 0, ram_below_4g);
>      e820_add_entry(0, x86ms->below_4g_mem_size, E820_RAM);
>      if (x86ms->above_4g_mem_size > 0) {
>          ram_above_4g = g_malloc(sizeof(*ram_above_4g));
> -        memory_region_init_alias(ram_above_4g, NULL, "ram-above-4g", ram,
> +        memory_region_init_alias(ram_above_4g, NULL, "ram-above-4g",
> +                                 machine->ram,
>                                   x86ms->below_4g_mem_size,
>                                   x86ms->above_4g_mem_size);
>          memory_region_add_subregion(system_memory, 0x100000000ULL,
> @@ -1952,6 +1950,7 @@ static void pc_machine_class_init(ObjectClass *oc, void 
> *data)
>      mc->default_cpu_type = TARGET_DEFAULT_CPU_TYPE;
>      mc->nvdimm_supported = true;
>      mc->numa_mem_supported = true;
> +    mc->default_ram_id = "pc.ram";
>  
>      object_class_property_add(oc, PC_MACHINE_DEVMEM_REGION_SIZE, "int",
>          pc_machine_get_device_memory_region_size, NULL,
> -- 
> 2.7.4
> 
>