Re: [PATCH v2 10/86] arm:aspeed: use memdev for RAM

[Philippe Mathieu-Daudé]

On 1/16/20 10:24 AM, Cédric Le Goater wrote:
> On 1/15/20 4:06 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.
> >
> > Signed-off-by: Igor Mammedov <address@hidden>
>
> Reviewed-by: Cédric Le Goater <address@hidden>
>
>
> We need to check 'max_ram' is still needed. I remember that old firmwares
> were testing the RAM size by doing write/read checks at the top of the RAM.
> This was breaking QEMU sometime ago.

Do you remember any version we can use to test this? Also add to our 
acceptance tests.

> C.
>
> > ---
> > CC: address@hidden
> > CC: address@hidden
> > CC: address@hidden
> > CC: address@hidden
> > CC: address@hidden
> > ---
> >   hw/arm/aspeed.c | 5 ++---
> >   1 file changed, 2 insertions(+), 3 deletions(-)
> >
> > diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
> > index 525c547..330254b 100644
> > --- a/hw/arm/aspeed.c
> > +++ b/hw/arm/aspeed.c
> > @@ -35,7 +35,6 @@ static struct arm_boot_info aspeed_board_binfo = {
> >   struct AspeedBoardState {
> >       AspeedSoCState soc;
> >       MemoryRegion ram_container;
> > -    MemoryRegion ram;
> >       MemoryRegion max_ram;
> >   };
> >   
> > @@ -184,6 +183,7 @@ static void aspeed_machine_init(MachineState *machine)
> >   
> >       memory_region_init(&bmc->ram_container, NULL, "aspeed-ram-container",
> >                          UINT32_MAX);
> > +    memory_region_add_subregion(&bmc->ram_container, 0, machine->ram);
> >   
> >       object_initialize_child(OBJECT(machine), "soc", &bmc->soc,
> >                               (sizeof(bmc->soc)), amc->soc_name, &error_abort,
> > @@ -215,8 +215,6 @@ static void aspeed_machine_init(MachineState *machine)
> >       object_property_set_bool(OBJECT(&bmc->soc), true, "realized",
> >                                &error_fatal);
> >   
> > -    memory_region_allocate_system_memory(&bmc->ram, NULL, "ram", ram_size);
> > -    memory_region_add_subregion(&bmc->ram_container, 0, &bmc->ram);
> >       memory_region_add_subregion(get_system_memory(),
> >                                   sc->memmap[ASPEED_SDRAM],
> >                                   &bmc->ram_container);
> > @@ -393,6 +391,7 @@ static void aspeed_machine_class_init(ObjectClass *oc, void 
> > *data)
> >       mc->no_floppy = 1;
> >       mc->no_cdrom = 1;
> >       mc->no_parallel = 1;
> > +    mc->default_ram_id = "ram";
> >   }
> >   
> >   static void aspeed_machine_palmetto_class_init(ObjectClass *oc, void *data)