Re: [PATCH v5 24/79] arm/musicpal: use memdev for RAM

[Richard Henderson <richard.henderson@linaro.org>]

On 2/17/20 9:33 AM, 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.
> 
> PS:
>  while at it add check for user supplied RAM size and error
>  out if it mismatches board expected value.
> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> Reviewed-by: Andrew Jones <drjones@redhat.com>
> ---

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

> @@ -1589,16 +1590,21 @@ static void musicpal_init(MachineState *machine)
>      int i;
>      unsigned long flash_size;
>      DriveInfo *dinfo;
> +    MachineClass *mc = MACHINE_GET_CLASS(machine);
>      MemoryRegion *address_space_mem = get_system_memory();
> -    MemoryRegion *ram = g_new(MemoryRegion, 1);
>      MemoryRegion *sram = g_new(MemoryRegion, 1);
>  
> +    /* For now we use a fixed - the original - RAM size */
> +    if (machine->ram_size != mc->default_ram_size) {
> +        char *sz = size_to_str(mc->default_ram_size);
> +        error_report("Invalid RAM size, should be %s", sz);
> +        g_free(sz);
> +        exit(EXIT_FAILURE);
> +    }

If for some reason you need to re-spin this series again, and considering my
comment re arm/imx25_pdk, I think it would be worthwhile to create a common
helper for this:

void machine_memory_check_fixed_size(MachineState *machine)
{
    MachineClass *mc = MACHINE_GET_CLASS(machine);

    if (machine->ram_size != mc->default_ram_size) {
        char *sz = size_to_str(mc->default_ram_size);
        error_report("Invalid RAM size, should be %s", sz);
        g_free(sz);
        exit(EXIT_FAILURE);
    }
}

That would keep the language consistent across the boards.


r~