The cells of 'initrd-start/end' should follow the '#address-cell'.
QEMU API could support 1 and 2 cells.

Signed-off-by: Jim Shu <jim.shu@sifive.com>
---
 hw/riscv/boot.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c
index ad45bd7a6a..76b099c696 100644
--- a/hw/riscv/boot.c
+++ b/hw/riscv/boot.c
@@ -182,6 +182,7 @@ static void riscv_load_initrd(MachineState *machine, uint64_t kernel_entry)
     void *fdt = machine->fdt;
     hwaddr start, end;
     ssize_t size;
+    uint32_t acells;
 
     g_assert(filename != NULL);
 
@@ -209,9 +210,18 @@ static void riscv_load_initrd(MachineState *machine, uint64_t kernel_entry)
 
     /* Some RISC-V machines (e.g. opentitan) don't have a fdt. */
     if (fdt) {
+        acells = qemu_fdt_getprop_cell(fdt, "/", "#address-cells",
+                                       NULL, NULL);
+        if (acells == 0) {
+            error_report("dtb file invalid (#address-cells 0)");
+            exit(1);
+        }
+
         end = start + size;
-        qemu_fdt_setprop_u64(fdt, "/chosen", "linux,initrd-start", start);
-        qemu_fdt_setprop_u64(fdt, "/chosen", "linux,initrd-end", end);
+        qemu_fdt_setprop_sized_cells(fdt, "/chosen", "linux,initrd-start",
+                                     acells, start);
+        qemu_fdt_setprop_sized_cells(fdt, "/chosen", "linux,initrd-end",
+                                     acells, end);
     }
 }
 
-- 
2.17.1

On 10/21/24 1:09 AM, Jim Shu wrote:
> The cells of 'initrd-start/end' should follow the '#address-cell'.
> QEMU API could support 1 and 2 cells.
> 
> Signed-off-by: Jim Shu <jim.shu@sifive.com>
> ---
>   hw/riscv/boot.c | 14 ++++++++++++--
>   1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c
> index ad45bd7a6a..76b099c696 100644
> --- a/hw/riscv/boot.c
> +++ b/hw/riscv/boot.c
> @@ -182,6 +182,7 @@ static void riscv_load_initrd(MachineState *machine, uint64_t kernel_entry)
>       void *fdt = machine->fdt;
>       hwaddr start, end;
>       ssize_t size;
> +    uint32_t acells;
>   
>       g_assert(filename != NULL);
>   
> @@ -209,9 +210,18 @@ static void riscv_load_initrd(MachineState *machine, uint64_t kernel_entry)
>   
>       /* Some RISC-V machines (e.g. opentitan) don't have a fdt. */
>       if (fdt) {
> +        acells = qemu_fdt_getprop_cell(fdt, "/", "#address-cells",
> +                                       NULL, NULL);
> +        if (acells == 0) {
> +            error_report("dtb file invalid (#address-cells 0)");
> +            exit(1);
> +        }
> +
>           end = start + size;
> -        qemu_fdt_setprop_u64(fdt, "/chosen", "linux,initrd-start", start);
> -        qemu_fdt_setprop_u64(fdt, "/chosen", "linux,initrd-end", end);
> +        qemu_fdt_setprop_sized_cells(fdt, "/chosen", "linux,initrd-start",
> +                                     acells, start);
> +        qemu_fdt_setprop_sized_cells(fdt, "/chosen", "linux,initrd-end",
> +                                     acells, end);
>       }

Is this a legal format for linux,initrd-start and linux,initrd-end?

This link:

https://www.kernel.org/doc/Documentation/devicetree/bindings/chosen.txt

Defines both attributes as:

"These properties hold the physical start and end address of an initrd that's
loaded by the bootloader."

So I'm not sure if this format you're using here is valid.


Conor, care to weight in? Thanks,

Daniel

>   }
>