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

[Daniel Henrique Barboza]

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));



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) {