Re: [PATCH v5 44/79] lm32/lm32_boards: use memdev for RAM

["Philippe Mathieu-Daudé" <philmd@redhat.com>]

On 2/17/20 6:34 PM, 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>
> ---
> v2:
>    * fix format string causing build failure on 32-bit host
>      (Philippe Mathieu-Daudé <philmd@redhat.com>)
> 
> CC: michael@walle.cc
> CC: philmd@redhat.com
> ---
>   hw/lm32/lm32_boards.c | 39 ++++++++++++++++++++++++++-------------
>   1 file changed, 26 insertions(+), 13 deletions(-)
> 
> diff --git a/hw/lm32/lm32_boards.c b/hw/lm32/lm32_boards.c
> index d1894adab8..4e0a98c117 100644
> --- a/hw/lm32/lm32_boards.c
> +++ b/hw/lm32/lm32_boards.c
> @@ -19,6 +19,7 @@
>   
>   #include "qemu/osdep.h"
>   #include "qemu/units.h"
> +#include "qemu/cutils.h"
>   #include "qemu/error-report.h"
>   #include "cpu.h"
>   #include "hw/sysbus.h"
> @@ -75,22 +76,28 @@ static void main_cpu_reset(void *opaque)
>   
>   static void lm32_evr_init(MachineState *machine)
>   {
> +    MachineClass *mc = MACHINE_GET_CLASS(machine);
>       const char *kernel_filename = machine->kernel_filename;
>       LM32CPU *cpu;
>       CPULM32State *env;
>       DriveInfo *dinfo;
>       MemoryRegion *address_space_mem =  get_system_memory();
> -    MemoryRegion *phys_ram = g_new(MemoryRegion, 1);
>       qemu_irq irq[32];
>       ResetInfo *reset_info;
>       int i;
>   
> +    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);
> +    }
> +
>       /* memory map */
>       hwaddr flash_base  = 0x04000000;
>       size_t flash_sector_size       = 256 * KiB;
>       size_t flash_size              = 32 * MiB;
>       hwaddr ram_base    = 0x08000000;
> -    size_t ram_size                = 64 * MiB;
>       hwaddr timer0_base = 0x80002000;
>       hwaddr uart0_base  = 0x80006000;
>       hwaddr timer1_base = 0x8000a000;
> @@ -107,9 +114,7 @@ static void lm32_evr_init(MachineState *machine)
>   
>       reset_info->flash_base = flash_base;
>   
> -    memory_region_allocate_system_memory(phys_ram, NULL, "lm32_evr.sdram",
> -                                         ram_size);
> -    memory_region_add_subregion(address_space_mem, ram_base, phys_ram);
> +    memory_region_add_subregion(address_space_mem, ram_base, machine->ram);
>   
>       dinfo = drive_get(IF_PFLASH, 0, 0);
>       /* Spansion S29NS128P */
> @@ -144,7 +149,7 @@ static void lm32_evr_init(MachineState *machine)
>   
>           if (kernel_size < 0) {
>               kernel_size = load_image_targphys(kernel_filename, ram_base,
> -                                              ram_size);
> +                                              machine->ram_size);
>               reset_info->bootstrap_pc = ram_base;
>           }
>   
> @@ -159,6 +164,7 @@ static void lm32_evr_init(MachineState *machine)
>   
>   static void lm32_uclinux_init(MachineState *machine)
>   {
> +    MachineClass *mc = MACHINE_GET_CLASS(machine);
>       const char *kernel_filename = machine->kernel_filename;
>       const char *kernel_cmdline = machine->kernel_cmdline;
>       const char *initrd_filename = machine->initrd_filename;
> @@ -166,18 +172,23 @@ static void lm32_uclinux_init(MachineState *machine)
>       CPULM32State *env;
>       DriveInfo *dinfo;
>       MemoryRegion *address_space_mem =  get_system_memory();
> -    MemoryRegion *phys_ram = g_new(MemoryRegion, 1);
>       qemu_irq irq[32];
>       HWSetup *hw;
>       ResetInfo *reset_info;
>       int i;
>   
> +    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);
> +    }
> +
>       /* memory map */
>       hwaddr flash_base   = 0x04000000;
>       size_t flash_sector_size        = 256 * KiB;
>       size_t flash_size               = 32 * MiB;
>       hwaddr ram_base     = 0x08000000;
> -    size_t ram_size                 = 64 * MiB;
>       hwaddr uart0_base   = 0x80000000;
>       hwaddr timer0_base  = 0x80002000;
>       hwaddr timer1_base  = 0x80010000;
> @@ -200,9 +211,7 @@ static void lm32_uclinux_init(MachineState *machine)
>   
>       reset_info->flash_base = flash_base;
>   
> -    memory_region_allocate_system_memory(phys_ram, NULL,
> -                                         "lm32_uclinux.sdram", ram_size);
> -    memory_region_add_subregion(address_space_mem, ram_base, phys_ram);
> +    memory_region_add_subregion(address_space_mem, ram_base, machine->ram);
>   
>       dinfo = drive_get(IF_PFLASH, 0, 0);
>       /* Spansion S29NS128P */
> @@ -238,7 +247,7 @@ static void lm32_uclinux_init(MachineState *machine)
>   
>           if (kernel_size < 0) {
>               kernel_size = load_image_targphys(kernel_filename, ram_base,
> -                                              ram_size);
> +                                              machine->ram_size);
>               reset_info->bootstrap_pc = ram_base;
>           }
>   
> @@ -252,7 +261,7 @@ static void lm32_uclinux_init(MachineState *machine)
>       hw = hwsetup_init();
>       hwsetup_add_cpu(hw, "LM32", 75000000);
>       hwsetup_add_flash(hw, "flash", flash_base, flash_size);
> -    hwsetup_add_ddr_sdram(hw, "ddr_sdram", ram_base, ram_size);
> +    hwsetup_add_ddr_sdram(hw, "ddr_sdram", ram_base, machine->ram_size);
>       hwsetup_add_timer(hw, "timer0", timer0_base, timer0_irq);
>       hwsetup_add_timer(hw, "timer1_dev_only", timer1_base, timer1_irq);
>       hwsetup_add_timer(hw, "timer2_dev_only", timer2_base, timer2_irq);
> @@ -288,6 +297,8 @@ static void lm32_evr_class_init(ObjectClass *oc, void *data)
>       mc->init = lm32_evr_init;
>       mc->is_default = 1;
>       mc->default_cpu_type = LM32_CPU_TYPE_NAME("lm32-full");
> +    mc->default_ram_size = 64 * MiB;
> +    mc->default_ram_id = "lm32_evr.sdram";
>   }
>   
>   static const TypeInfo lm32_evr_type = {
> @@ -304,6 +315,8 @@ static void lm32_uclinux_class_init(ObjectClass *oc, void *data)
>       mc->init = lm32_uclinux_init;
>       mc->is_default = 0;
>       mc->default_cpu_type = LM32_CPU_TYPE_NAME("lm32-full");
> +    mc->default_ram_size = 64 * MiB;
> +    mc->default_ram_id = "lm32_uclinux.sdram";
>   }
>   
>   static const TypeInfo lm32_uclinux_type = {
> 

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>