On Fri, 23 Oct 2020 19:39:16 +0200
Philippe Mathieu-Daudé <f4...@amsat.org> wrote:
> On 10/23/20 5:43 PM, Igor Mammedov wrote:
> > On Mon, 19 Oct 2020 11:43:13 +0200
> > Philippe Mathieu-Daudé <f4...@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 <imamm...@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 <imamm...@redhat.com>
> >>> Reviewed-by: Andrew Jones <drjo...@redhat.com>
> >>> Reviewed-by: Richard Henderson <richard.hender...@linaro.org>
> >>> Message-Id: <20200219160953.13771-26-imamm...@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?
yep, some parts of it (fdt related) already use MachineState* so it's
complete rewrite. The same probably applies to the fields you've just
quoted.
>
