diff mbox series

[v5,34/79] arm/xilinx_zynq: drop RAM size fixup

Message ID 20200217173452.15243-35-imammedo@redhat.com (mailing list archive)
State New, archived
Headers show
Series refactor main RAM allocation to use hostmem backend | expand

Commit Message

Igor Mammedov Feb. 17, 2020, 5:34 p.m. UTC 
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>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
---
 hw/arm/xilinx_zynq.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

Comments

Richard Henderson Feb. 17, 2020, 7:23 p.m. UTC | #1 
On 2/17/20 9:34 AM, 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>
> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
> ---

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

> +    /* max 2GB ram */
> +    if (machine->ram_size > 0x80000000) {
> +        error_report("RAM size more than %d is not supported", 0x80000000);
> +        exit(EXIT_FAILURE);
> +    }

If you have occasion to re-spin, I think that a common function like

void machine_memory_check_max_size(MachineState *machine,
                                   ram_addr_t max);

akin to the machine_memory_check_fixed_size I proposed earlier, would help keep
the language consistent across the boards.


r~
Philippe Mathieu-Daudé Feb. 18, 2020, 5:23 p.m. UTC | #2 
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>
> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
> ---
>   hw/arm/xilinx_zynq.c | 16 ++++++++--------
>   1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/arm/xilinx_zynq.c b/hw/arm/xilinx_zynq.c
> index 3a0fa5b23f..df950fc400 100644
> --- a/hw/arm/xilinx_zynq.c
> +++ b/hw/arm/xilinx_zynq.c
> @@ -158,7 +158,6 @@ static inline void zynq_init_spi_flashes(uint32_t base_addr, qemu_irq irq,
>   
>   static void zynq_init(MachineState *machine)
>   {
> -    ram_addr_t ram_size = machine->ram_size;
>       ARMCPU *cpu;
>       MemoryRegion *address_space_mem = get_system_memory();
>       MemoryRegion *ext_ram = g_new(MemoryRegion, 1);
> @@ -168,6 +167,12 @@ static void zynq_init(MachineState *machine)
>       qemu_irq pic[64];
>       int n;
>   
> +    /* max 2GB ram */
> +    if (machine->ram_size > 0x80000000) {
> +        error_report("RAM size more than %d is not supported", 0x80000000);

Eh? Maybe:

        if (machine->ram_size > 2 * GiB) {
            error_report("RAM size more than 2 GiB is not supported");

> +        exit(EXIT_FAILURE);
> +    }
> +
>       cpu = ARM_CPU(object_new(machine->cpu_type));
>   
>       /* By default A9 CPUs have EL3 enabled.  This board does not
> @@ -184,14 +189,9 @@ static void zynq_init(MachineState *machine)
>                               &error_fatal);
>       object_property_set_bool(OBJECT(cpu), true, "realized", &error_fatal);
>   
> -    /* max 2GB ram */
> -    if (ram_size > 0x80000000) {
> -        ram_size = 0x80000000;
> -    }
> -
>       /* DDR remapped to address zero.  */
>       memory_region_allocate_system_memory(ext_ram, NULL, "zynq.ext_ram",
> -                                         ram_size);
> +                                         machine->ram_size);
>       memory_region_add_subregion(address_space_mem, 0, ext_ram);
>   
>       /* 256K of on-chip memory */
> @@ -300,7 +300,7 @@ static void zynq_init(MachineState *machine)
>       sysbus_connect_irq(busdev, 0, pic[40 - IRQ_OFFSET]);
>       sysbus_mmio_map(busdev, 0, 0xF8007000);
>   
> -    zynq_binfo.ram_size = ram_size;
> +    zynq_binfo.ram_size = machine->ram_size;
>       zynq_binfo.nb_cpus = 1;
>       zynq_binfo.board_id = 0xd32;
>       zynq_binfo.loader_start = 0;
>
Igor Mammedov Feb. 19, 2020, 1:44 p.m. UTC | #3 
On Tue, 18 Feb 2020 18:23:47 +0100
Philippe Mathieu-Daudé <philmd@redhat.com> wrote:

> 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>
> > Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
> > ---
> >   hw/arm/xilinx_zynq.c | 16 ++++++++--------
> >   1 file changed, 8 insertions(+), 8 deletions(-)
> > 
> > diff --git a/hw/arm/xilinx_zynq.c b/hw/arm/xilinx_zynq.c
> > index 3a0fa5b23f..df950fc400 100644
> > --- a/hw/arm/xilinx_zynq.c
> > +++ b/hw/arm/xilinx_zynq.c
> > @@ -158,7 +158,6 @@ static inline void zynq_init_spi_flashes(uint32_t base_addr, qemu_irq irq,
> >   
> >   static void zynq_init(MachineState *machine)
> >   {
> > -    ram_addr_t ram_size = machine->ram_size;
> >       ARMCPU *cpu;
> >       MemoryRegion *address_space_mem = get_system_memory();
> >       MemoryRegion *ext_ram = g_new(MemoryRegion, 1);
> > @@ -168,6 +167,12 @@ static void zynq_init(MachineState *machine)
> >       qemu_irq pic[64];
> >       int n;
> >   
> > +    /* max 2GB ram */
> > +    if (machine->ram_size > 0x80000000) {
> > +        error_report("RAM size more than %d is not supported", 0x80000000);  
> 
> Eh? Maybe:
> 
>         if (machine->ram_size > 2 * GiB) {
>             error_report("RAM size more than 2 GiB is not supported");

amended in v6

> > +        exit(EXIT_FAILURE);
> > +    }
> > +
> >       cpu = ARM_CPU(object_new(machine->cpu_type));
> >   
> >       /* By default A9 CPUs have EL3 enabled.  This board does not
> > @@ -184,14 +189,9 @@ static void zynq_init(MachineState *machine)
> >                               &error_fatal);
> >       object_property_set_bool(OBJECT(cpu), true, "realized", &error_fatal);
> >   
> > -    /* max 2GB ram */
> > -    if (ram_size > 0x80000000) {
> > -        ram_size = 0x80000000;
> > -    }
> > -
> >       /* DDR remapped to address zero.  */
> >       memory_region_allocate_system_memory(ext_ram, NULL, "zynq.ext_ram",
> > -                                         ram_size);
> > +                                         machine->ram_size);
> >       memory_region_add_subregion(address_space_mem, 0, ext_ram);
> >   
> >       /* 256K of on-chip memory */
> > @@ -300,7 +300,7 @@ static void zynq_init(MachineState *machine)
> >       sysbus_connect_irq(busdev, 0, pic[40 - IRQ_OFFSET]);
> >       sysbus_mmio_map(busdev, 0, 0xF8007000);
> >   
> > -    zynq_binfo.ram_size = ram_size;
> > +    zynq_binfo.ram_size = machine->ram_size;
> >       zynq_binfo.nb_cpus = 1;
> >       zynq_binfo.board_id = 0xd32;
> >       zynq_binfo.loader_start = 0;
> >   
>
Philippe Mathieu-Daudé Feb. 19, 2020, 2:07 p.m. UTC | #4 
On 2/19/20 2:44 PM, Igor Mammedov wrote:
> On Tue, 18 Feb 2020 18:23:47 +0100
> Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
> 
>> 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>
>>> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
>>> ---
>>>    hw/arm/xilinx_zynq.c | 16 ++++++++--------
>>>    1 file changed, 8 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/hw/arm/xilinx_zynq.c b/hw/arm/xilinx_zynq.c
>>> index 3a0fa5b23f..df950fc400 100644
>>> --- a/hw/arm/xilinx_zynq.c
>>> +++ b/hw/arm/xilinx_zynq.c
>>> @@ -158,7 +158,6 @@ static inline void zynq_init_spi_flashes(uint32_t base_addr, qemu_irq irq,
>>>    
>>>    static void zynq_init(MachineState *machine)
>>>    {
>>> -    ram_addr_t ram_size = machine->ram_size;
>>>        ARMCPU *cpu;
>>>        MemoryRegion *address_space_mem = get_system_memory();
>>>        MemoryRegion *ext_ram = g_new(MemoryRegion, 1);
>>> @@ -168,6 +167,12 @@ static void zynq_init(MachineState *machine)
>>>        qemu_irq pic[64];
>>>        int n;
>>>    
>>> +    /* max 2GB ram */
>>> +    if (machine->ram_size > 0x80000000) {
>>> +        error_report("RAM size more than %d is not supported", 0x80000000);
>>
>> Eh? Maybe:
>>
>>          if (machine->ram_size > 2 * GiB) {
>>              error_report("RAM size more than 2 GiB is not supported");
> 
> amended in v6

Then please add to v6:
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Thanks :)

> 
>>> +        exit(EXIT_FAILURE);
>>> +    }
>>> +
>>>        cpu = ARM_CPU(object_new(machine->cpu_type));
>>>    
>>>        /* By default A9 CPUs have EL3 enabled.  This board does not
>>> @@ -184,14 +189,9 @@ static void zynq_init(MachineState *machine)
>>>                                &error_fatal);
>>>        object_property_set_bool(OBJECT(cpu), true, "realized", &error_fatal);
>>>    
>>> -    /* max 2GB ram */
>>> -    if (ram_size > 0x80000000) {
>>> -        ram_size = 0x80000000;
>>> -    }
>>> -
>>>        /* DDR remapped to address zero.  */
>>>        memory_region_allocate_system_memory(ext_ram, NULL, "zynq.ext_ram",
>>> -                                         ram_size);
>>> +                                         machine->ram_size);
>>>        memory_region_add_subregion(address_space_mem, 0, ext_ram);
>>>    
>>>        /* 256K of on-chip memory */
>>> @@ -300,7 +300,7 @@ static void zynq_init(MachineState *machine)
>>>        sysbus_connect_irq(busdev, 0, pic[40 - IRQ_OFFSET]);
>>>        sysbus_mmio_map(busdev, 0, 0xF8007000);
>>>    
>>> -    zynq_binfo.ram_size = ram_size;
>>> +    zynq_binfo.ram_size = machine->ram_size;
>>>        zynq_binfo.nb_cpus = 1;
>>>        zynq_binfo.board_id = 0xd32;
>>>        zynq_binfo.loader_start = 0;
>>>    
>>
>
diff mbox series

Patch

diff --git a/hw/arm/xilinx_zynq.c b/hw/arm/xilinx_zynq.c
index 3a0fa5b23f..df950fc400 100644
--- a/hw/arm/xilinx_zynq.c
+++ b/hw/arm/xilinx_zynq.c
@@ -158,7 +158,6 @@  static inline void zynq_init_spi_flashes(uint32_t base_addr, qemu_irq irq,
 
 static void zynq_init(MachineState *machine)
 {
-    ram_addr_t ram_size = machine->ram_size;
     ARMCPU *cpu;
     MemoryRegion *address_space_mem = get_system_memory();
     MemoryRegion *ext_ram = g_new(MemoryRegion, 1);
@@ -168,6 +167,12 @@  static void zynq_init(MachineState *machine)
     qemu_irq pic[64];
     int n;
 
+    /* max 2GB ram */
+    if (machine->ram_size > 0x80000000) {
+        error_report("RAM size more than %d is not supported", 0x80000000);
+        exit(EXIT_FAILURE);
+    }
+
     cpu = ARM_CPU(object_new(machine->cpu_type));
 
     /* By default A9 CPUs have EL3 enabled.  This board does not
@@ -184,14 +189,9 @@  static void zynq_init(MachineState *machine)
                             &error_fatal);
     object_property_set_bool(OBJECT(cpu), true, "realized", &error_fatal);
 
-    /* max 2GB ram */
-    if (ram_size > 0x80000000) {
-        ram_size = 0x80000000;
-    }
-
     /* DDR remapped to address zero.  */
     memory_region_allocate_system_memory(ext_ram, NULL, "zynq.ext_ram",
-                                         ram_size);
+                                         machine->ram_size);
     memory_region_add_subregion(address_space_mem, 0, ext_ram);
 
     /* 256K of on-chip memory */
@@ -300,7 +300,7 @@  static void zynq_init(MachineState *machine)
     sysbus_connect_irq(busdev, 0, pic[40 - IRQ_OFFSET]);
     sysbus_mmio_map(busdev, 0, 0xF8007000);
 
-    zynq_binfo.ram_size = ram_size;
+    zynq_binfo.ram_size = machine->ram_size;
     zynq_binfo.nb_cpus = 1;
     zynq_binfo.board_id = 0xd32;
     zynq_binfo.loader_start = 0;