* [PATCH] hw: riscv: Allow large kernels to boot by moving the initrd further way in RAM @ 2024-02-05 7:00 Alexandre Ghiti 2024-02-05 12:17 ` Daniel Henrique Barboza 0 siblings, 1 reply; 5+ messages in thread From: Alexandre Ghiti @ 2024-02-05 7:00 UTC (permalink / raw) To: Palmer Dabbelt, Alistair Francis, Bin Meng, Weiwei Li, Daniel Henrique Barboza, Liu Zhiwei, qemu-riscv, qemu-devel Cc: Alexandre Ghiti 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. Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com> --- 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) { -- 2.39.2 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] hw: riscv: Allow large kernels to boot by moving the initrd further way in RAM 2024-02-05 7:00 [PATCH] hw: riscv: Allow large kernels to boot by moving the initrd further way in RAM Alexandre Ghiti @ 2024-02-05 12:17 ` Daniel Henrique Barboza 2024-02-05 13:36 ` Alexandre Ghiti 0 siblings, 1 reply; 5+ messages in thread From: Daniel Henrique Barboza @ 2024-02-05 12:17 UTC (permalink / raw) To: Alexandre Ghiti, Palmer Dabbelt, Alistair Francis, Bin Meng, Weiwei Li, Liu Zhiwei, qemu-riscv, qemu-devel 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) { ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] hw: riscv: Allow large kernels to boot by moving the initrd further way in RAM 2024-02-05 12:17 ` Daniel Henrique Barboza @ 2024-02-05 13:36 ` Alexandre Ghiti 2024-02-06 9:41 ` Alexandre Ghiti 0 siblings, 1 reply; 5+ messages in thread From: Alexandre Ghiti @ 2024-02-05 13:36 UTC (permalink / raw) To: Daniel Henrique Barboza Cc: Palmer Dabbelt, Alistair Francis, Bin Meng, Weiwei Li, Liu Zhiwei, qemu-riscv, qemu-devel 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) { ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] hw: riscv: Allow large kernels to boot by moving the initrd further way in RAM 2024-02-05 13:36 ` Alexandre Ghiti @ 2024-02-06 9:41 ` Alexandre Ghiti 2024-02-06 11:35 ` Daniel Henrique Barboza 0 siblings, 1 reply; 5+ messages in thread From: Alexandre Ghiti @ 2024-02-06 9:41 UTC (permalink / raw) To: Daniel Henrique Barboza Cc: Palmer Dabbelt, Alistair Francis, Bin Meng, Weiwei Li, Liu Zhiwei, qemu-riscv, qemu-devel Hi Daniel, On Mon, Feb 5, 2024 at 2:36 PM Alexandre Ghiti <alexghiti@rivosinc.com> wrote: > > 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(). So this does not work because the size returned by load_image_targphys_as() does not take into account the size of the BSS sections (and I guess other NOBITS sections) and then we end up loading the initrd there. I'm a bit surprised though because arm64 does just that https://elixir.bootlin.com/qemu/latest/source/hw/arm/boot.c#L1034. I also tried using the highaddr parameter, but that gives the same result. We could parse the Image header (a PE header) to get the "Virtual Size" of the .data section, but I did not find any PE header parser in qemu, so that would be overkill? Any idea? Thanks, Alex > > > > 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) { ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] hw: riscv: Allow large kernels to boot by moving the initrd further way in RAM 2024-02-06 9:41 ` Alexandre Ghiti @ 2024-02-06 11:35 ` Daniel Henrique Barboza 0 siblings, 0 replies; 5+ messages in thread From: Daniel Henrique Barboza @ 2024-02-06 11:35 UTC (permalink / raw) To: Alexandre Ghiti Cc: Palmer Dabbelt, Alistair Francis, Bin Meng, Weiwei Li, Liu Zhiwei, qemu-riscv, qemu-devel On 2/6/24 06:41, Alexandre Ghiti wrote: > Hi Daniel, > > On Mon, Feb 5, 2024 at 2:36 PM Alexandre Ghiti <alexghiti@rivosinc.com> wrote: >> >> 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(). > > So this does not work because the size returned by > load_image_targphys_as() does not take into account the size of the > BSS sections (and I guess other NOBITS sections) and then we end up > loading the initrd there. I'm a bit surprised though because arm64 > does just that https://elixir.bootlin.com/qemu/latest/source/hw/arm/boot.c#L1034. > I also tried using the highaddr parameter, but that gives the same > result. We could parse the Image header (a PE header) to get the > "Virtual Size" of the .data section, but I did not find any PE header > parser in qemu, so that would be overkill? > > Any idea? hmm that's unfortunate. The size retrieval in these cases might have to do with missing information from the image header, and we can't control that because we're not generating the images. It would be better if they return '0' in these cases (then we could special case them in the math), but if it's a non-zero return then there's not much we can do. If we can't reliably get the kernel size in RAM then I believe your patch is going to make it do (until we want to load an even bigger kernel hehe). Can you please document these findings (i.e. we can't retrieve the kernel size reliably) in the commit msg? Thanks, Daniel > > Thanks, > > Alex > >>> >>> 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) { ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2024-02-06 11:35 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-02-05 7:00 [PATCH] hw: riscv: Allow large kernels to boot by moving the initrd further way in RAM Alexandre Ghiti 2024-02-05 12:17 ` Daniel Henrique Barboza 2024-02-05 13:36 ` Alexandre Ghiti 2024-02-06 9:41 ` Alexandre Ghiti 2024-02-06 11:35 ` Daniel Henrique Barboza
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).