Re: [PATCH v2 48/86] lm32:milkymist: use memdev for RAM

[Igor Mammedov]

On Wed, 15 Jan 2020 19:32:19 +0100
Philippe Mathieu-Daudé <address@hidden> wrote:

> On 1/15/20 4:07 PM, Igor Mammedov wrote:
> > memory_region_allocate_system_memory() API is going away, so
> > replace it with memdev allocated MemoryRegion. The later is
> > initialized by generic code, so board only needs to opt in
> > to memdev scheme by providing
> >    MachineClass::default_ram_id
> > and using MachineState::ram instead of manually initializing
> > RAM memory region.
> > 
> > PS:
> >   while at it add check for user supplied RAM size and error
> >   out if it mismatches board expected value.
> > 
> > Signed-off-by: Igor Mammedov <address@hidden>
> > ---
> > v2:
> >    * fix format string causing build failure on 32-bit host
> >      (Philippe Mathieu-Daudé <address@hidden>)
> > 
> > CC: address@hidden
> > ---
> >   hw/lm32/milkymist.c | 19 ++++++++++++-------
> >   1 file changed, 12 insertions(+), 7 deletions(-)
> > 
> > diff --git a/hw/lm32/milkymist.c b/hw/lm32/milkymist.c
> > index 460d322..73c28f4 100644
> > --- a/hw/lm32/milkymist.c
> > +++ b/hw/lm32/milkymist.c
> > @@ -82,6 +82,7 @@ static void main_cpu_reset(void *opaque)
> >   static void
> >   milkymist_init(MachineState *machine)
> >   {
> > +    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;
> > @@ -90,22 +91,26 @@ milkymist_init(MachineState *machine)
> >       int kernel_size;
> >       DriveInfo *dinfo;
> >       MemoryRegion *address_space_mem = get_system_memory();
> > -    MemoryRegion *phys_sdram = g_new(MemoryRegion, 1);
> >       qemu_irq irq[32];
> >       int i;
> >       char *bios_filename;
> >       ResetInfo *reset_info;
> >   
> > +    if (machine->ram_size != mc->default_ram_size) {
> > +        error_report("Invalid RAM size, should be " RAM_ADDR_UFMT " Bytes",
> > +                     mc->default_ram_size);  
> 
> I'd be displayed nicer using size_to_str(). Can be improved later.
I'll use it on respin, including similar places on other boards

> 
> Reviewed-by: Philippe Mathieu-Daudé <address@hidden>
> 
> > +        exit(EXIT_FAILURE);
> > +    }
> > +
> >       /* memory map */
> >       hwaddr flash_base   = 0x00000000;
> >       size_t flash_sector_size        = 128 * KiB;
> >       size_t flash_size               = 32 * MiB;
> >       hwaddr sdram_base   = 0x40000000;
> > -    size_t sdram_size               = 128 * MiB;
> >   
> >       hwaddr initrd_base  = sdram_base + 0x1002000;
> >       hwaddr cmdline_base = sdram_base + 0x1000000;
> > -    size_t initrd_max = sdram_size - 0x1002000;
> > +    size_t initrd_max = machine->ram_size - 0x1002000;
> >   
> >       reset_info = g_malloc0(sizeof(ResetInfo));
> >   
> > @@ -116,9 +121,7 @@ milkymist_init(MachineState *machine)
> >   
> >       cpu_lm32_set_phys_msb_ignore(env, 1);
> >   
> > -    memory_region_allocate_system_memory(phys_sdram, NULL, 
> > "milkymist.sdram",
> > -                                         sdram_size);
> > -    memory_region_add_subregion(address_space_mem, sdram_base, phys_sdram);
> > +    memory_region_add_subregion(address_space_mem, sdram_base, 
> > machine->ram);
> >   
> >       dinfo = drive_get(IF_PFLASH, 0, 0);
> >       /* Numonyx JS28F256J3F105 */
> > @@ -183,7 +186,7 @@ milkymist_init(MachineState *machine)
> >   
> >           if (kernel_size < 0) {
> >               kernel_size = load_image_targphys(kernel_filename, sdram_base,
> > -                                              sdram_size);
> > +                                              machine->ram_size);
> >               reset_info->bootstrap_pc = sdram_base;
> >           }
> >   
> > @@ -216,6 +219,8 @@ static void milkymist_machine_init(MachineClass *mc)
> >       mc->init = milkymist_init;
> >       mc->is_default = 0;
> >       mc->default_cpu_type = LM32_CPU_TYPE_NAME("lm32-full");
> > +    mc->default_ram_size = 128 * MiB;
> > +    mc->default_ram_id = "milkymist.sdram";
> >   }
> >   
> >   DEFINE_MACHINE("milkymist", milkymist_machine_init)
> >   
>