Re: [PATCH v2 3/6] hw/riscv: Make FDT optional for MPFS

[Alistair Francis <alistair23@gmail.com>]

On Tue, Feb 25, 2025 at 10:55 AM Sebastian Huber
<sebastian.huber@embedded-brains.de> wrote:
>
> Real-time kernels such as RTEMS or Zephyr may use a static device tree
> built into the kernel image.  Do not require to use the -dtb option if
> -kernel is used for the microchip-icicle-kit machine.  Issue a warning
> if no device tree is provided by the user since the machine does not
> generate one.
>
> Signed-off-by: Sebastian Huber <sebastian.huber@embedded-brains.de>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  hw/riscv/microchip_pfsoc.c | 56 +++++++++++++++++++-------------------
>  1 file changed, 28 insertions(+), 28 deletions(-)
>
> diff --git a/hw/riscv/microchip_pfsoc.c b/hw/riscv/microchip_pfsoc.c
> index f477d2791e..844dc0545c 100644
> --- a/hw/riscv/microchip_pfsoc.c
> +++ b/hw/riscv/microchip_pfsoc.c
> @@ -516,7 +516,6 @@ static void microchip_icicle_kit_machine_init(MachineState *machine)
>      uint64_t mem_low_size, mem_high_size;
>      hwaddr firmware_load_addr;
>      const char *firmware_name;
> -    bool kernel_as_payload = false;
>      target_ulong firmware_end_addr, kernel_start_addr;
>      uint64_t kernel_entry;
>      uint64_t fdt_load_addr;
> @@ -589,25 +588,12 @@ static void microchip_icicle_kit_machine_init(MachineState *machine)
>       *
>       * This ensures backwards compatibility with how we used to expose -bios
>       * to users but allows them to run through direct kernel booting as well.
> -     *
> -     * When -kernel is used for direct boot, -dtb must be present to provide
> -     * a valid device tree for the board, as we don't generate device tree.
>       */
>
> -    if (machine->kernel_filename && machine->dtb) {
> -        int fdt_size;
> -        machine->fdt = load_device_tree(machine->dtb, &fdt_size);
> -        if (!machine->fdt) {
> -            error_report("load_device_tree() failed");
> -            exit(1);
> -        }
> -
> +    if (machine->kernel_filename) {
>          firmware_name = RISCV64_BIOS_BIN;
>          firmware_load_addr = memmap[MICROCHIP_PFSOC_DRAM_LO].base;
> -        kernel_as_payload = true;
> -    }
> -
> -    if (!kernel_as_payload) {
> +    } else {
>          firmware_name = BIOS_FILENAME;
>          firmware_load_addr = RESET_VECTOR;
>      }
> @@ -617,7 +603,7 @@ static void microchip_icicle_kit_machine_init(MachineState *machine)
>                                                       &firmware_load_addr, NULL);
>
>      riscv_boot_info_init(&boot_info, &s->soc.u_cpus);
> -    if (kernel_as_payload) {
> +    if (machine->kernel_filename) {
>          kernel_start_addr = riscv_calc_kernel_start_addr(&boot_info,
>                                                           firmware_end_addr);
>
> @@ -625,19 +611,33 @@ static void microchip_icicle_kit_machine_init(MachineState *machine)
>                            true, NULL);
>          kernel_entry = boot_info.image_low_addr;
>
> -        /* Compute the fdt load address in dram */
> -        hwaddr kernel_ram_base = memmap[MICROCHIP_PFSOC_DRAM_LO].base;
> -        hwaddr kernel_ram_size = memmap[MICROCHIP_PFSOC_DRAM_LO].size;
> -
> -        if (kernel_entry - kernel_ram_base >= kernel_ram_size) {
> -            kernel_ram_base = memmap[MICROCHIP_PFSOC_DRAM_HI].base;
> -            kernel_ram_size = mem_high_size;
> +        if (machine->dtb) {
> +            int fdt_size;
> +            machine->fdt = load_device_tree(machine->dtb, &fdt_size);
> +            if (!machine->fdt) {
> +                error_report("load_device_tree() failed");
> +                exit(1);
> +            }
> +
> +            /* Compute the FDT load address in DRAM */
> +            hwaddr kernel_ram_base = memmap[MICROCHIP_PFSOC_DRAM_LO].base;
> +            hwaddr kernel_ram_size = memmap[MICROCHIP_PFSOC_DRAM_LO].size;
> +
> +            if (kernel_entry - kernel_ram_base >= kernel_ram_size) {
> +                kernel_ram_base = memmap[MICROCHIP_PFSOC_DRAM_HI].base;
> +                kernel_ram_size = mem_high_size;
> +            }
> +
> +            fdt_load_addr = riscv_compute_fdt_addr(kernel_ram_base, kernel_ram_size,
> +                                                   machine, &boot_info);
> +            riscv_load_fdt(fdt_load_addr, machine->fdt);
> +        } else {
> +            warn_report_once("The QEMU microchip-icicle-kit machine does not "
> +                             "generate a device tree, so no device tree is "
> +                             "being provided to the guest.");
> +            fdt_load_addr = 0;
>          }
>
> -        fdt_load_addr = riscv_compute_fdt_addr(kernel_ram_base, kernel_ram_size,
> -                                               machine, &boot_info);
> -        riscv_load_fdt(fdt_load_addr, machine->fdt);
> -
>          /* Load the reset vector */
>          riscv_setup_rom_reset_vec(machine, &s->soc.u_cpus, firmware_load_addr,
>                                    memmap[MICROCHIP_PFSOC_ENVM_DATA].base,
> --
> 2.43.0
>