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

Igor Mammedov posted 79 patches 5 years, 11 months ago
Maintainers: "Michael S. Tsirkin" <mst@redhat.com>, KONRAD Frederic <frederic.konrad@adacore.com>, Jan Kiszka <jan.kiszka@web.de>, Paolo Bonzini <pbonzini@redhat.com>, Aurelien Jarno <aurelien@aurel32.net>, Cornelia Huck <cohuck@redhat.com>, Laurent Vivier <lvivier@redhat.com>, Paul Burton <pburton@wavecomp.com>, David Hildenbrand <david@redhat.com>, Radoslaw Biernacki <radoslaw.biernacki@linaro.org>, Fabien Chouteau <chouteau@adacore.com>, Andrew Baumann <Andrew.Baumann@microsoft.com>, Michael Walle <michael@walle.cc>, Peter Chubb <peter.chubb@nicta.com.au>, Richard Henderson <rth@twiddle.net>, Laurent Vivier <laurent@vivier.eu>, Aleksandar Rikalo <aleksandar.rikalo@rt-rk.com>, Sergio Lopez <slp@redhat.com>, Thomas Huth <huth@tuxfamily.org>, Beniamino Galvani <b.galvani@gmail.com>, Eduardo Habkost <ehabkost@redhat.com>, Jean-Christophe Dubois <jcd@tribudubois.net>, Helge Deller <deller@gmx.de>, Igor Mammedov <imammedo@redhat.com>, David Gibson <david@gibson.dropbear.id.au>, Andrew Jeffery <andrew@aj.id.au>, Joel Stanley <joel@jms.id.au>, Alistair Francis <alistair@alistair23.me>, Christian Borntraeger <borntraeger@de.ibm.com>, Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>, Thomas Huth <thuth@redhat.com>, "Hervé Poussineau" <hpoussin@reactos.org>, Andrey Smirnov <andrew.smirnov@gmail.com>, Halil Pasic <pasic@linux.ibm.com>, BALATON Zoltan <balaton@eik.bme.hu>, Antony Pavlov <antonynpavlov@gmail.com>, Aleksandar Markovic <amarkovic@wavecomp.com>, Rob Herring <robh@kernel.org>, Peter Maydell <peter.maydell@linaro.org>, "Cédric Le Goater" <clg@kaod.org>, "Philippe Mathieu-Daudé" <philmd@redhat.com>, Andrzej Zaborowski <balrogg@gmail.com>, Leif Lindholm <leif@nuviainc.com>, "Edgar E. Iglesias" <edgar.iglesias@gmail.com>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, Artyom Tarasenko <atar4qemu@gmail.com>
There is a newer version of this series
[PATCH v5 51/79] mips/mips_fulong2e: drop RAM size fixup
Posted by Igor Mammedov 5 years, 11 months ago
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 <imammedo@redhat.com>
---
v2:
 * fix format string cousing build failure on 32-bit host
   (Philippe Mathieu-Daudé <philmd@redhat.com>)
v3:
 * since size is ifxed, just hardcode 256Mb value as text
   in error message
   (BALATON Zoltan <balaton@eik.bme.hu>)

CC: philmd@redhat.com
CC: amarkovic@wavecomp.com
CC: aurelien@aurel32.net
CC: aleksandar.rikalo@rt-rk.com
CC: balaton@eik.bme.hu
---
 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);
+    }
 
     /* 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);
     }
-- 
2.18.1


Re: [PATCH v5 51/79] mips/mips_fulong2e: drop RAM size fixup
Posted by Philippe Mathieu-Daudé 5 years, 11 months ago
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 <imammedo@redhat.com>
> ---
> v2:
>   * fix format string cousing build failure on 32-bit host
>     (Philippe Mathieu-Daudé <philmd@redhat.com>)
> v3:
>   * since size is ifxed, just hardcode 256Mb value as text
>     in error message
>     (BALATON Zoltan <balaton@eik.bme.hu>)
> 
> CC: philmd@redhat.com
> CC: amarkovic@wavecomp.com
> CC: aurelien@aurel32.net
> CC: aleksandar.rikalo@rt-rk.com
> CC: balaton@eik.bme.hu
> ---
>   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é <philmd@redhat.com>

>   
>       /* 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);
>       }
>