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

[Schspa Shi <schspa@gmail.com>]

Peter Maydell <peter.maydell@linaro.org> writes:

> 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?
>

I have made a new patch & test it this way, it works perfectly.

> thanks
> -- PMM


-- 
BRs
Schspa Shi