Re: [PATCH v2 64/86] ppc:ppc405_boards: add RAM size checks

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 7048 bytes --]

On Wed, Jan 15, 2020 at 04:07:19PM +0100, Igor Mammedov wrote:
> If user provided non-sense RAM size, board will ignore it
> and continue running with fixed RAM size.
> 
> Also RAM is going to be allocated by generic code, so it
> won't be possible for board to fix CLI.
> 
> Make it error message and exit to force user fix CLI,
> instead of accepting non-sense CLI values.
> 
> PS:
> move fixed RAM size into mc->default_ram_size, so that
> generic code will know how much to allocate.
> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>

Wow, that's spectacularly broken.

Acked-by: David Gibson <david@gibson.dropbear.id.au>

> ---
> v2:
>   * fix format string causing build failure on 32-bit host
>     (Philippe Mathieu-Daudé <philmd@redhat.com>)
> 
> CC: david@gibson.dropbear.id.au
> CC: qemu-ppc@nongnu.org
> ---
>  hw/ppc/ppc405_boards.c | 36 +++++++++++++++++++++++-------------
>  1 file changed, 23 insertions(+), 13 deletions(-)
> 
> diff --git a/hw/ppc/ppc405_boards.c b/hw/ppc/ppc405_boards.c
> index 1f721fe..a7a432d 100644
> --- a/hw/ppc/ppc405_boards.c
> +++ b/hw/ppc/ppc405_boards.c
> @@ -137,7 +137,7 @@ static void ref405ep_fpga_init(MemoryRegion *sysmem, uint32_t base)
>  
>  static void ref405ep_init(MachineState *machine)
>  {
> -    ram_addr_t ram_size = machine->ram_size;
> +    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;
> @@ -161,15 +161,20 @@ static void ref405ep_init(MachineState *machine)
>      DriveInfo *dinfo;
>      MemoryRegion *sysmem = get_system_memory();
>  
> +    if (machine->ram_size != mc->default_ram_size) {
> +        error_report("Invalid RAM size, should be " RAM_ADDR_UFMT " Bytes",
> +                     mc->default_ram_size);
> +        exit(EXIT_FAILURE);
> +    }
> +
>      /* XXX: fix this */
>      memory_region_allocate_system_memory(&ram_memories[0], NULL, "ef405ep.ram",
> -                                         0x08000000);
> +                                         machine->ram_size);
>      ram_bases[0] = 0;
> -    ram_sizes[0] = 0x08000000;
> +    ram_sizes[0] = machine->ram_size;
>      memory_region_init(&ram_memories[1], NULL, "ef405ep.ram1", 0);
>      ram_bases[1] = 0x00000000;
>      ram_sizes[1] = 0x00000000;
> -    ram_size = 128 * MiB;
>      env = ppc405ep_init(sysmem, ram_memories, ram_bases, ram_sizes,
>                          33333333, &pic, kernel_filename == NULL ? 0 : 1);
>      /* allocate SRAM */
> @@ -227,7 +232,7 @@ static void ref405ep_init(MachineState *machine)
>      if (linux_boot) {
>          memset(&bd, 0, sizeof(bd));
>          bd.bi_memstart = 0x00000000;
> -        bd.bi_memsize = ram_size;
> +        bd.bi_memsize = machine->ram_size;
>          bd.bi_flashstart = -bios_size;
>          bd.bi_flashsize = -bios_size;
>          bd.bi_flashoffset = 0;
> @@ -255,7 +260,7 @@ static void ref405ep_init(MachineState *machine)
>          kernel_base = KERNEL_LOAD_ADDR;
>          /* now we can load the kernel */
>          kernel_size = load_image_targphys(kernel_filename, kernel_base,
> -                                          ram_size - kernel_base);
> +                                          machine->ram_size - kernel_base);
>          if (kernel_size < 0) {
>              error_report("could not load kernel '%s'", kernel_filename);
>              exit(1);
> @@ -266,7 +271,7 @@ static void ref405ep_init(MachineState *machine)
>          if (initrd_filename) {
>              initrd_base = INITRD_LOAD_ADDR;
>              initrd_size = load_image_targphys(initrd_filename, initrd_base,
> -                                              ram_size - initrd_base);
> +                                              machine->ram_size - initrd_base);
>              if (initrd_size < 0) {
>                  error_report("could not load initial ram disk '%s'",
>                               initrd_filename);
> @@ -304,6 +309,7 @@ static void ref405ep_class_init(ObjectClass *oc, void *data)
>  
>      mc->desc = "ref405ep";
>      mc->init = ref405ep_init;
> +    mc->default_ram_size = 0x08000000;
>  }
>  
>  static const TypeInfo ref405ep_type = {
> @@ -408,7 +414,7 @@ static void taihu_cpld_init(MemoryRegion *sysmem, uint32_t base)
>  
>  static void taihu_405ep_init(MachineState *machine)
>  {
> -    ram_addr_t ram_size = machine->ram_size;
> +    MachineClass *mc = MACHINE_GET_CLASS(machine);
>      const char *kernel_filename = machine->kernel_filename;
>      const char *initrd_filename = machine->initrd_filename;
>      char *filename;
> @@ -425,10 +431,13 @@ static void taihu_405ep_init(MachineState *machine)
>      int fl_idx;
>      DriveInfo *dinfo;
>  
> -    /* RAM is soldered to the board so the size cannot be changed */
> -    ram_size = 0x08000000;
> +    if (machine->ram_size != mc->default_ram_size) {
> +        error_report("Invalid RAM size, should be " RAM_ADDR_UFMT " Bytes",
> +                     mc->default_ram_size);
> +        exit(EXIT_FAILURE);
> +    }
>      memory_region_allocate_system_memory(ram, NULL, "taihu_405ep.ram",
> -                                         ram_size);
> +                                         machine->ram_size);
>  
>      ram_bases[0] = 0;
>      ram_sizes[0] = 0x04000000;
> @@ -500,7 +509,7 @@ static void taihu_405ep_init(MachineState *machine)
>          kernel_base = KERNEL_LOAD_ADDR;
>          /* now we can load the kernel */
>          kernel_size = load_image_targphys(kernel_filename, kernel_base,
> -                                          ram_size - kernel_base);
> +                                          machine->ram_size - kernel_base);
>          if (kernel_size < 0) {
>              error_report("could not load kernel '%s'", kernel_filename);
>              exit(1);
> @@ -509,7 +518,7 @@ static void taihu_405ep_init(MachineState *machine)
>          if (initrd_filename) {
>              initrd_base = INITRD_LOAD_ADDR;
>              initrd_size = load_image_targphys(initrd_filename, initrd_base,
> -                                              ram_size - initrd_base);
> +                                              machine->ram_size - initrd_base);
>              if (initrd_size < 0) {
>                  error_report("could not load initial ram disk '%s'",
>                               initrd_filename);
> @@ -533,6 +542,7 @@ static void taihu_class_init(ObjectClass *oc, void *data)
>  
>      mc->desc = "taihu";
>      mc->init = taihu_405ep_init;
> +    mc->default_ram_size = 0x08000000;
>  }
>  
>  static const TypeInfo taihu_type = {

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]