diff mbox series

[v3,18/86] arm:kzm: drop RAM size fixup

Message ID 1579195564-95459-1-git-send-email-imammedo@redhat.com
State New
Headers show
Series None | expand

Commit Message

Igor Mammedov Jan. 16, 2020, 5:26 p.m. UTC 
If the user provided too large a RAM size, the code used to
complain and trim it to the max size.  Now tht RAM is allocated by
generic code, that's no longer possible, so generate an error and
exit instead.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
v3:
 * rephrase commit message in nicer way
   ("Chubb, Peter (Data61, Kensington NSW)" <Peter.Chubb@data61.csiro.au>)
 * reword error message and use size_to_str() to pretty print suggested size
   ("Chubb, Peter (Data61, Kensington NSW)" <Peter.Chubb@data61.csiro.au>)

CC: peter.chubb@nicta.com.au
CC: peter.maydell@linaro.org
CC: qemu-arm@nongnu.org
---
 hw/arm/kzm.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

Comments

Philippe Mathieu-Daudé Jan. 16, 2020, 6:22 p.m. UTC | #1 
On 1/16/20 6:26 PM, Igor Mammedov wrote:
> If the user provided too large a RAM size, the code used to
> complain and trim it to the max size.  Now tht RAM is allocated by
> generic code, that's no longer possible, so generate an error and
> exit instead.
> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
> v3:
>   * rephrase commit message in nicer way
>     ("Chubb, Peter (Data61, Kensington NSW)" <Peter.Chubb@data61.csiro.au>)
>   * reword error message and use size_to_str() to pretty print suggested size
>     ("Chubb, Peter (Data61, Kensington NSW)" <Peter.Chubb@data61.csiro.au>)
> 
> CC: peter.chubb@nicta.com.au
> CC: peter.maydell@linaro.org
> CC: qemu-arm@nongnu.org
> ---
>   hw/arm/kzm.c | 9 +++++----
>   1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/arm/kzm.c b/hw/arm/kzm.c
> index 1d5ef28..94cbac1 100644
> --- a/hw/arm/kzm.c
> +++ b/hw/arm/kzm.c
> @@ -25,6 +25,7 @@
>   #include "hw/char/serial.h"
>   #include "sysemu/qtest.h"
>   #include "sysemu/sysemu.h"
> +#include "qemu/cutils.h"
>   
>   /* Memory map for Kzm Emulation Baseboard:
>    * 0x00000000-0x7fffffff See i.MX31 SOC for support
> @@ -78,10 +79,10 @@ static void kzm_init(MachineState *machine)
>   
>       /* Check the amount of memory is compatible with the SOC */
>       if (machine->ram_size > (FSL_IMX31_SDRAM0_SIZE + FSL_IMX31_SDRAM1_SIZE)) {
> -        warn_report("RAM size " RAM_ADDR_FMT " above max supported, "
> -                    "reduced to %x", machine->ram_size,
> -                    FSL_IMX31_SDRAM0_SIZE + FSL_IMX31_SDRAM1_SIZE);
> -        machine->ram_size = FSL_IMX31_SDRAM0_SIZE + FSL_IMX31_SDRAM1_SIZE;
> +        char *sz = size_to_str(FSL_IMX31_SDRAM0_SIZE + FSL_IMX31_SDRAM1_SIZE);
> +        error_report("RAM size more than %s is not supported", sz);

Yay! Can you use this pattern the other patches too?

You might want to add:

#define FSL_IMX31_SDRAM_SIZE_MAX \
   (FSL_IMX31_SDRAM0_SIZE + FSL_IMX31_SDRAM1_SIZE)

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

> +        g_free(sz);
> +        exit(EXIT_FAILURE);
>       }
>   
>       memory_region_allocate_system_memory(&s->ram, NULL, "kzm.ram",
>
Igor Mammedov Jan. 17, 2020, 9:50 a.m. UTC | #2 
On Thu, 16 Jan 2020 19:22:08 +0100
Philippe Mathieu-Daudé <philmd@redhat.com> wrote:

> On 1/16/20 6:26 PM, Igor Mammedov wrote:
> > If the user provided too large a RAM size, the code used to
> > complain and trim it to the max size.  Now tht RAM is allocated by
> > generic code, that's no longer possible, so generate an error and
> > exit instead.
> > 
> > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > ---
> > v3:
> >   * rephrase commit message in nicer way
> >     ("Chubb, Peter (Data61, Kensington NSW)" <Peter.Chubb@data61.csiro.au>)
> >   * reword error message and use size_to_str() to pretty print suggested size
> >     ("Chubb, Peter (Data61, Kensington NSW)" <Peter.Chubb@data61.csiro.au>)
> > 
> > CC: peter.chubb@nicta.com.au
> > CC: peter.maydell@linaro.org
> > CC: qemu-arm@nongnu.org
> > ---
> >   hw/arm/kzm.c | 9 +++++----
> >   1 file changed, 5 insertions(+), 4 deletions(-)
> > 
> > diff --git a/hw/arm/kzm.c b/hw/arm/kzm.c
> > index 1d5ef28..94cbac1 100644
> > --- a/hw/arm/kzm.c
> > +++ b/hw/arm/kzm.c
> > @@ -25,6 +25,7 @@
> >   #include "hw/char/serial.h"
> >   #include "sysemu/qtest.h"
> >   #include "sysemu/sysemu.h"
> > +#include "qemu/cutils.h"
> >   
> >   /* Memory map for Kzm Emulation Baseboard:
> >    * 0x00000000-0x7fffffff See i.MX31 SOC for support
> > @@ -78,10 +79,10 @@ static void kzm_init(MachineState *machine)
> >   
> >       /* Check the amount of memory is compatible with the SOC */
> >       if (machine->ram_size > (FSL_IMX31_SDRAM0_SIZE + FSL_IMX31_SDRAM1_SIZE)) {
> > -        warn_report("RAM size " RAM_ADDR_FMT " above max supported, "
> > -                    "reduced to %x", machine->ram_size,
> > -                    FSL_IMX31_SDRAM0_SIZE + FSL_IMX31_SDRAM1_SIZE);
> > -        machine->ram_size = FSL_IMX31_SDRAM0_SIZE + FSL_IMX31_SDRAM1_SIZE;
> > +        char *sz = size_to_str(FSL_IMX31_SDRAM0_SIZE + FSL_IMX31_SDRAM1_SIZE);
> > +        error_report("RAM size more than %s is not supported", sz);  
> 
> Yay! Can you use this pattern the other patches too?

I plan to, as it's much neater and I can avoid adding RAM_ADDR_FMT

Would your acks still stand or should I drop your Reviewed-bys
on changed in such way patches?
 
> You might want to add:
> 
> #define FSL_IMX31_SDRAM_SIZE_MAX \
>    (FSL_IMX31_SDRAM0_SIZE + FSL_IMX31_SDRAM1_SIZE)
> 
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> 
> > +        g_free(sz);
> > +        exit(EXIT_FAILURE);
> >       }
> >   
> >       memory_region_allocate_system_memory(&s->ram, NULL, "kzm.ram",
> >   
>
Philippe Mathieu-Daudé Jan. 17, 2020, 1:07 p.m. UTC | #3 
On 1/17/20 10:50 AM, Igor Mammedov wrote:
> On Thu, 16 Jan 2020 19:22:08 +0100
> Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
> 
>> On 1/16/20 6:26 PM, Igor Mammedov wrote:
>>> If the user provided too large a RAM size, the code used to
>>> complain and trim it to the max size.  Now tht RAM is allocated by
>>> generic code, that's no longer possible, so generate an error and
>>> exit instead.
>>>
>>> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
>>> ---
>>> v3:
>>>    * rephrase commit message in nicer way
>>>      ("Chubb, Peter (Data61, Kensington NSW)" <Peter.Chubb@data61.csiro.au>)
>>>    * reword error message and use size_to_str() to pretty print suggested size
>>>      ("Chubb, Peter (Data61, Kensington NSW)" <Peter.Chubb@data61.csiro.au>)
>>>
>>> CC: peter.chubb@nicta.com.au
>>> CC: peter.maydell@linaro.org
>>> CC: qemu-arm@nongnu.org
>>> ---
>>>    hw/arm/kzm.c | 9 +++++----
>>>    1 file changed, 5 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/hw/arm/kzm.c b/hw/arm/kzm.c
>>> index 1d5ef28..94cbac1 100644
>>> --- a/hw/arm/kzm.c
>>> +++ b/hw/arm/kzm.c
>>> @@ -25,6 +25,7 @@
>>>    #include "hw/char/serial.h"
>>>    #include "sysemu/qtest.h"
>>>    #include "sysemu/sysemu.h"
>>> +#include "qemu/cutils.h"
>>>    
>>>    /* Memory map for Kzm Emulation Baseboard:
>>>     * 0x00000000-0x7fffffff See i.MX31 SOC for support
>>> @@ -78,10 +79,10 @@ static void kzm_init(MachineState *machine)
>>>    
>>>        /* Check the amount of memory is compatible with the SOC */
>>>        if (machine->ram_size > (FSL_IMX31_SDRAM0_SIZE + FSL_IMX31_SDRAM1_SIZE)) {
>>> -        warn_report("RAM size " RAM_ADDR_FMT " above max supported, "
>>> -                    "reduced to %x", machine->ram_size,
>>> -                    FSL_IMX31_SDRAM0_SIZE + FSL_IMX31_SDRAM1_SIZE);
>>> -        machine->ram_size = FSL_IMX31_SDRAM0_SIZE + FSL_IMX31_SDRAM1_SIZE;
>>> +        char *sz = size_to_str(FSL_IMX31_SDRAM0_SIZE + FSL_IMX31_SDRAM1_SIZE);
>>> +        error_report("RAM size more than %s is not supported", sz);
>>
>> Yay! Can you use this pattern the other patches too?
> 
> I plan to, as it's much neater and I can avoid adding RAM_ADDR_FMT
> 
> Would your acks still stand or should I drop your Reviewed-bys
> on changed in such way patches?

Yes please keep my Reviewed-by tag in the other patches too.

>> You might want to add:
>>
>> #define FSL_IMX31_SDRAM_SIZE_MAX \
>>     (FSL_IMX31_SDRAM0_SIZE + FSL_IMX31_SDRAM1_SIZE)
>>
>> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>>
>>> +        g_free(sz);
>>> +        exit(EXIT_FAILURE);
>>>        }
>>>    
>>>        memory_region_allocate_system_memory(&s->ram, NULL, "kzm.ram",
>>>    
>>
>
diff mbox series

Patch

diff --git a/hw/arm/kzm.c b/hw/arm/kzm.c
index 1d5ef28..94cbac1 100644
--- a/hw/arm/kzm.c
+++ b/hw/arm/kzm.c
@@ -25,6 +25,7 @@ 
 #include "hw/char/serial.h"
 #include "sysemu/qtest.h"
 #include "sysemu/sysemu.h"
+#include "qemu/cutils.h"
 
 /* Memory map for Kzm Emulation Baseboard:
  * 0x00000000-0x7fffffff See i.MX31 SOC for support
@@ -78,10 +79,10 @@  static void kzm_init(MachineState *machine)
 
     /* Check the amount of memory is compatible with the SOC */
     if (machine->ram_size > (FSL_IMX31_SDRAM0_SIZE + FSL_IMX31_SDRAM1_SIZE)) {
-        warn_report("RAM size " RAM_ADDR_FMT " above max supported, "
-                    "reduced to %x", machine->ram_size,
-                    FSL_IMX31_SDRAM0_SIZE + FSL_IMX31_SDRAM1_SIZE);
-        machine->ram_size = FSL_IMX31_SDRAM0_SIZE + FSL_IMX31_SDRAM1_SIZE;
+        char *sz = size_to_str(FSL_IMX31_SDRAM0_SIZE + FSL_IMX31_SDRAM1_SIZE);
+        error_report("RAM size more than %s is not supported", sz);
+        g_free(sz);
+        exit(EXIT_FAILURE);
     }
 
     memory_region_allocate_system_memory(&s->ram, NULL, "kzm.ram",