Re: [Qemu-devel] [PATCH v2] arm_boot: Change initrd load address to "halfway through RAM"

[Aurelien Jarno <aurelien@aurel32.net>]

On Fri, Oct 26, 2012 at 04:29:38PM +0100, Peter Maydell wrote:
> To avoid continually having to bump the initrd load address
> to account for larger kernel images, put the initrd halfway
> through RAM. This allows large kernels on new boards with lots
> of RAM to work OK, without breaking existing usecases for
> boards with only 32MB of RAM.
> 
> Note that this change fixes in passing a bug where we were
> passing an overly large max_size to load_image_targphys()
> for the initrd, which meant that we wouldn't correctly refuse
> to load an enormous initrd that didn't actually fit into RAM.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> Changes v1->v2: put initrd at min(128M, ram_size / 2)
> rather than just at ram_size / 2.
> 
>  hw/arm-misc.h |    1 +
>  hw/arm_boot.c |   40 +++++++++++++++++++++++++---------------
>  2 files changed, 26 insertions(+), 15 deletions(-)
> 
> diff --git a/hw/arm-misc.h b/hw/arm-misc.h
> index d02f7f0..adb1665 100644
> --- a/hw/arm-misc.h
> +++ b/hw/arm-misc.h
> @@ -56,6 +56,7 @@ struct arm_boot_info {
>                                       const struct arm_boot_info *info);
>      /* Used internally by arm_boot.c */
>      int is_linux;
> +    hwaddr initrd_start;
>      hwaddr initrd_size;
>      hwaddr entry;
>  };
> diff --git a/hw/arm_boot.c b/hw/arm_boot.c
> index 09bf6c5..92e2cab 100644
> --- a/hw/arm_boot.c
> +++ b/hw/arm_boot.c
> @@ -18,7 +18,6 @@
>  
>  #define KERNEL_ARGS_ADDR 0x100
>  #define KERNEL_LOAD_ADDR 0x00010000
> -#define INITRD_LOAD_ADDR 0x00d00000
>  
>  /* The worlds second smallest bootloader.  Set r0-r2, then jump to kernel.  */
>  static uint32_t bootloader[] = {
> @@ -109,7 +108,7 @@ static void set_kernel_args(const struct arm_boot_info *info)
>          /* ATAG_INITRD2 */
>          WRITE_WORD(p, 4);
>          WRITE_WORD(p, 0x54420005);
> -        WRITE_WORD(p, info->loader_start + INITRD_LOAD_ADDR);
> +        WRITE_WORD(p, info->initrd_start);
>          WRITE_WORD(p, initrd_size);
>      }
>      if (info->kernel_cmdline && *info->kernel_cmdline) {
> @@ -185,10 +184,11 @@ static void set_kernel_args_old(const struct arm_boot_info *info)
>      /* pages_in_vram */
>      WRITE_WORD(p, 0);
>      /* initrd_start */
> -    if (initrd_size)
> -        WRITE_WORD(p, info->loader_start + INITRD_LOAD_ADDR);
> -    else
> +    if (initrd_size) {
> +        WRITE_WORD(p, info->initrd_start);
> +    } else {
>          WRITE_WORD(p, 0);
> +    }
>      /* initrd_size */
>      WRITE_WORD(p, initrd_size);
>      /* rd_start */
> @@ -281,14 +281,13 @@ static int load_dtb(hwaddr addr, const struct arm_boot_info *binfo)
>  
>      if (binfo->initrd_size) {
>          rc = qemu_devtree_setprop_cell(fdt, "/chosen", "linux,initrd-start",
> -                binfo->loader_start + INITRD_LOAD_ADDR);
> +                binfo->initrd_start);
>          if (rc < 0) {
>              fprintf(stderr, "couldn't set /chosen/linux,initrd-start\n");
>          }
>  
>          rc = qemu_devtree_setprop_cell(fdt, "/chosen", "linux,initrd-end",
> -                    binfo->loader_start + INITRD_LOAD_ADDR +
> -                    binfo->initrd_size);
> +                    binfo->initrd_start + binfo->initrd_size);
>          if (rc < 0) {
>              fprintf(stderr, "couldn't set /chosen/linux,initrd-end\n");
>          }
> @@ -375,6 +374,19 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info)
>      big_endian = 0;
>  #endif
>  
> +    /* We want to put the initrd far enough into RAM that when the
> +     * 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.
> +     */
> +    info->initrd_start = info->loader_start +
> +        MIN(info->ram_size / 2, 128 * 1024 * 1024);
> +
>      /* Assume that raw images are linux kernels, and ELF images are not.  */
>      kernel_size = load_elf(info->kernel_filename, NULL, NULL, &elf_entry,
>                             NULL, NULL, big_endian, ELF_MACHINE, 1);
> @@ -398,10 +410,9 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info)
>      if (is_linux) {
>          if (info->initrd_filename) {
>              initrd_size = load_image_targphys(info->initrd_filename,
> -                                              info->loader_start
> -                                              + INITRD_LOAD_ADDR,
> -                                              info->ram_size
> -                                              - INITRD_LOAD_ADDR);
> +                                              info->initrd_start,
> +                                              info->ram_size -
> +                                              info->initrd_start);
>              if (initrd_size < 0) {
>                  fprintf(stderr, "qemu: could not load initrd '%s'\n",
>                          info->initrd_filename);
> @@ -419,9 +430,8 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info)
>           */
>          if (info->dtb_filename) {
>              /* Place the DTB after the initrd in memory */
> -            hwaddr dtb_start = TARGET_PAGE_ALIGN(info->loader_start
> -                                                             + INITRD_LOAD_ADDR
> -                                                             + initrd_size);
> +            hwaddr dtb_start = TARGET_PAGE_ALIGN(info->initrd_start +
> +                                                 initrd_size);
>              if (load_dtb(dtb_start, info)) {
>                  exit(1);
>              }

Thanks, applied.

-- 
Aurelien Jarno                          GPG: 1024D/F1BCDB73
aurelien@aurel32.net                 http://www.aurel32.net