Re: [PATCH v5 51/79] mips/mips_fulong2e: drop RAM size fixup

[Philippe Mathieu-Daudé]

On 2/17/20 6:34 PM, Igor Mammedov wrote:
> If user provided non-sense RAM size, board will complain and
> continue running with max RAM size supported.
> Also RAM is going to be allocated by generic code, so it won't be
> possible for board to fix things up for user.
>
> Make it error message and exit to force user fix CLI,
> instead of accepting non-sense CLI values.
>
> Signed-off-by: Igor Mammedov <address@hidden>
> ---
> v2:
>   * fix format string cousing build failure on 32-bit host
>     (Philippe Mathieu-Daudé <address@hidden>)
> v3:
>   * since size is ifxed, just hardcode 256Mb value as text
>     in error message
>     (BALATON Zoltan <address@hidden>)
>
> CC: address@hidden
> CC: address@hidden
> CC: address@hidden
> CC: address@hidden
> CC: address@hidden
> ---
>   hw/mips/mips_fulong2e.c | 13 ++++++++-----
>   1 file changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/hw/mips/mips_fulong2e.c b/hw/mips/mips_fulong2e.c
> index 2e043cbb98..cf00211bd2 100644
> --- a/hw/mips/mips_fulong2e.c
> +++ b/hw/mips/mips_fulong2e.c
> @@ -296,7 +296,6 @@ static void mips_fulong2e_init(MachineState *machine)
>       MemoryRegion *address_space_mem = get_system_memory();
>       MemoryRegion *ram = g_new(MemoryRegion, 1);
>       MemoryRegion *bios = g_new(MemoryRegion, 1);
> -    ram_addr_t ram_size = machine->ram_size;
>       long bios_size;
>       uint8_t *spd_data;
>       Error *err = NULL;
> @@ -315,10 +314,14 @@ static void mips_fulong2e_init(MachineState *machine)
>       qemu_register_reset(main_cpu_reset, cpu);
>   
>       /* TODO: support more than 256M RAM as highmem */
> -    ram_size = 256 * MiB;
> +    if (machine->ram_size != 256 * MiB) {
> +        error_report("Invalid RAM size, should be 256MB");
> +        exit(EXIT_FAILURE);
> +    }

Thanks you didn't remove the TODO.

The patch keeps the same behavior, so:
Reviewed-by: Philippe Mathieu-Daudé <address@hidden>

>   
>       /* allocate RAM */
> -    memory_region_allocate_system_memory(ram, NULL, "fulong2e.ram", ram_size);
> +    memory_region_allocate_system_memory(ram, NULL, "fulong2e.ram",
> +                                         machine->ram_size);
>       memory_region_init_ram(bios, NULL, "fulong2e.bios", BIOS_SIZE,
>                              &error_fatal);
>       memory_region_set_readonly(bios, true);
> @@ -332,7 +335,7 @@ static void mips_fulong2e_init(MachineState *machine)
>        */
>   
>       if (kernel_filename) {
> -        loaderparams.ram_size = ram_size;
> +        loaderparams.ram_size = machine->ram_size;
>           loaderparams.kernel_filename = kernel_filename;
>           loaderparams.kernel_cmdline = kernel_cmdline;
>           loaderparams.initrd_filename = initrd_filename;
> @@ -378,7 +381,7 @@ static void mips_fulong2e_init(MachineState *machine)
>       }
>   
>       /* Populate SPD eeprom data */
> -    spd_data = spd_data_generate(DDR, ram_size, &err);
> +    spd_data = spd_data_generate(DDR, machine->ram_size, &err);
>       if (err) {
>           warn_report_err(err);
>       }