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

[Igor Mammedov]

On Thu, 16 Jan 2020 19:22:08 +0100
Philippe Mathieu-Daudé <address@hidden> 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 <address@hidden>
> > ---
> > v3:
> >   * rephrase commit message in nicer way
> >     ("Chubb, Peter (Data61, Kensington NSW)" <address@hidden>)
> >   * reword error message and use size_to_str() to pretty print suggested 
> > size
> >     ("Chubb, Peter (Data61, Kensington NSW)" <address@hidden>)
> > 
> > CC: address@hidden
> > CC: address@hidden
> > CC: address@hidden
> > ---
> >   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é <address@hidden>
> 
> > +        g_free(sz);
> > +        exit(EXIT_FAILURE);
> >       }
> >   
> >       memory_region_allocate_system_memory(&s->ram, NULL, "kzm.ram",
> >   
>