Re: [PATCH] hw: riscv: Allow large kernels to boot by moving the initrd further way in RAM

[Alexandre Ghiti]

Hi Daniel,

On Mon, Feb 5, 2024 at 1:17 PM Daniel Henrique Barboza
<dbarboza@ventanamicro.com> wrote:
>
>
>
> On 2/5/24 04:00, Alexandre Ghiti wrote:
> > Currently, the initrd is placed at 128MB, which overlaps with the kernel
> > when it is large (for example syzbot kernels are). From the kernel side,
> > there is no reason we could not push the initrd further away in memory
> > to accomodate large kernels, so move the initrd at 512MB when possible.
>
> typo: accommodate
>
> >
> > Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com>
> > ---
>
> Patch looks good - just tested with an Ubuntu guest and nothing bad happened.
>
> But I wonder ... what if there's an even bigger kernel we have to deal with?
> Move initrd even further away to fit it in?
>
> Instead of making assumptions about where initrd starts, we could grab the 
> kernel
> size loaded in the board and use it as a reference. This would be done by 
> storing
> the return of load_elf_ram_sym/load_uimage_as/load_image_targphys_as from
> riscv_load_kernel() an passing as argument to riscv_load_initrd().
>
> initrd start would then be:
>
>      start = kernel_entry + MIN(mem_size / 2, kernel_size);
>
> However, I believe we would like to keep the existing 128Mb minimum initrd 
> start,
> even if the kernel is smaller than 128Mb, to avoid breaking existing configs 
> that
> might be making this assumption. initrd start would then become:
>
>
>      start = kernel_entry + MIN(mem_size / 2, MAX(kernel_size, 128 * MiB));

Great, I agree with you, thanks for the pointers. I'll just align the
size on a 2MB boundary to make sure the kernel mapping (which in the
case of Linux uses PMD) does not overlap with the initrd.

I'll get back soon with a v2.

Thanks again,

Alex

>
>
>
> Thanks,
>
>
>
> Daniel
>
>
> >   hw/riscv/boot.c | 12 ++++++------
> >   1 file changed, 6 insertions(+), 6 deletions(-)
> >
> > diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c
> > index 0ffca05189..9a367af2fa 100644
> > --- a/hw/riscv/boot.c
> > +++ b/hw/riscv/boot.c
> > @@ -188,13 +188,13 @@ static void riscv_load_initrd(MachineState *machine, 
> > uint64_t kernel_entry)
> >        * kernel is uncompressed it will not clobber the initrd. However
> >        * on boards without much RAM we must ensure that we still leave
> >        * enough room for a decent sized initrd, and on boards with large
> > -     * amounts of RAM we must avoid the initrd being so far up in RAM
> > -     * that it is outside lowmem and inaccessible to the kernel.
> > -     * So for boards with less  than 256MB of RAM we put the initrd
> > -     * halfway into RAM, and for boards with 256MB of RAM or more we put
> > -     * the initrd at 128MB.
> > +     * amounts of RAM, we put the initrd at 512MB to allow large kernels
> > +     * to boot.
> > +     * So for boards with less than 1GB of RAM we put the initrd
> > +     * halfway into RAM, and for boards with 1GB of RAM or more we put
> > +     * the initrd at 512MB.
> >        */
> > -    start = kernel_entry + MIN(mem_size / 2, 128 * MiB);
> > +    start = kernel_entry + MIN(mem_size / 2, 512 * MiB);
> >
> >       size = load_ramdisk(filename, start, mem_size - start);
> >       if (size == -1) {