Re: [PULL 25/33] tests/acceptance: Add a test for the N800 and N810 arm machines

[Philippe Mathieu-Daudé]

On 10/23/20 5:43 PM, Igor Mammedov wrote:
> On Mon, 19 Oct 2020 11:43:13 +0200
> Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> > > > > FYI this test is failing:
> > > > >
> > > > > qemu-system-arm: kernel 'meego-arm-n8x0-1.0.80.20100712.1431-vml
> > > > > inuz-2.6.35~rc4-129.1-n8x0' is too large to fit in RAM (kernel size
> > > > > 1964608,
> > > > > RAM size 0)
> > >
> > > FWIW:
> > >
> > > 7998beb9c2e280f0b7424223747941f106e2e854 is the first bad commit
> > > commit 7998beb9c2e280f0b7424223747941f106e2e854
> > > Author: Igor Mammedov <imammedo@redhat.com>
> > > Date:   Wed Feb 19 11:08:59 2020 -0500
> > >
> > >       arm/nseries: use memdev for RAM
> > >
> > >       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 <imammedo@redhat.com>
> > >       Reviewed-by: Andrew Jones <drjones@redhat.com>
> > >       Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> > >       Message-Id: <20200219160953.13771-26-imammedo@redhat.com>
> >
> > This fixes the issue:
> >
> > -- >8 --
> > diff --git a/hw/arm/nseries.c b/hw/arm/nseries.c
> > index e48092ca047..76fd7fe9854 100644
> > --- a/hw/arm/nseries.c
> > +++ b/hw/arm/nseries.c
> > @@ -1318,6 +1318,7 @@ static void n8x0_init(MachineState *machine,
> >            g_free(sz);
> >            exit(EXIT_FAILURE);
> >        }
> > +    binfo->ram_size = machine->ram_size;
> >
> >        memory_region_add_subregion(get_system_memory(), OMAP2_Q2_BASE,
> >                                    machine->ram);
>
> we really should replace binfo->ram_size with machine->ram_size to avoid
> duplicating the same data, but as a quick fix this should fix issue.

Hmm this is the 'ARM kernel loader' API in "arm/boot.h":

struct arm_boot_info {
    uint64_t ram_size;
    const char *kernel_filename;
    const char *kernel_cmdline;
    const char *initrd_filename;
    const char *dtb_filename;

and:

  void (*write_secondary_boot)(ARMCPU *cpu,
                               const struct arm_boot_info *info);
  void (*secondary_cpu_reset_hook)(ARMCPU *cpu,
                                   const struct arm_boot_info *info);

Are you saying arm_boot_info should hold a pointer to MachineState*
instead of duplicating?