Re: [PATCH] hw/arm/boot: set initrd parameters to 64bit in fdt

[Peter Maydell]

On Tue, 8 Nov 2022 at 02:35, Schspa Shi <schspa@gmail.com> wrote:
>
> We use 32bit value for linux,initrd-[start/end], when we have
> loader_start > 4GB, there will be a wrong initrd_start passed
> to the kernel, and the kernel will report the following warning

> To fix it, we can change it to u64 type.
>
> Signed-off-by: Schspa Shi <schspa@gmail.com>
> ---
>  hw/arm/boot.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hw/arm/boot.c b/hw/arm/boot.c
> index 57efb61ee419..da719a4f8874 100644
> --- a/hw/arm/boot.c
> +++ b/hw/arm/boot.c
> @@ -638,14 +638,14 @@ int arm_load_dtb(hwaddr addr, const struct 
> arm_boot_info *binfo,
>      }
>
>      if (binfo->initrd_size) {
> -        rc = qemu_fdt_setprop_cell(fdt, "/chosen", "linux,initrd-start",
> +        rc = qemu_fdt_setprop_u64(fdt, "/chosen", "linux,initrd-start",
>                                     binfo->initrd_start);
>          if (rc < 0) {
>              fprintf(stderr, "couldn't set /chosen/linux,initrd-start\n");
>              goto fail;
>          }
>
> -        rc = qemu_fdt_setprop_cell(fdt, "/chosen", "linux,initrd-end",
> +        rc = qemu_fdt_setprop_u64(fdt, "/chosen", "linux,initrd-end",
>                                     binfo->initrd_start + binfo->initrd_size);
>          if (rc < 0) {
>              fprintf(stderr, "couldn't set /chosen/linux,initrd-end\n");

Hi; the device-tree folks got back to me a bit late on this one,
but apparently the intention is that these fields should be
of a size that matches the #address-cells (and they'll fix the
schema docs to say that at some point). Some future kernel
or dtb-schema-check might warn about this, and also since it is
what u-boot does:
https://github.com/u-boot/u-boot/blob/218e2c45af83f2cb7b1374b9023b4ced6eb0bb77/common/fdt_support.c#L248
following that same approach is the safest thing in terms of not
breaking existing code.

I think that to do this in QEMU we just need to call

   qemu_fdt_setprop_sized_cells(fdt, "/chosen", "linux,initrd-start",
                                acells, binfo->initrd_start);

and similarly for initrd-end.

Would you mind doing a respin and test of this patch that works
that way?

thanks
-- PMM