Re: [RFC PATCH 3/8] ppc/spapr: add baseline VOF disk boot support

[Narayana Murty N <nnmlinux@linux.ibm.com>]

On 17/08/26 3:57 PM, uverma@linux.ibm.com wrote:
> From: Utkarsh Verma <uverma@linux.ibm.com>
> 
> Add initial support for disk boot via VOF.
> Scan all the block devices, and for each device search for PReP partition,
> load the ELF payload (GRUB), and set the '/chosen' node in FDT with boot kernel
> and bootpath properties.
> 
> AI-used-for: code
> Signed-off-by: Utkarsh Verma <uverma@linux.ibm.com>
> ---
>   hw/ppc/spapr_vof.c   | 142 ++++++++++++++++++++++++++++++++++++++++---
>   hw/ppc/vof.c         |   3 +
>   include/hw/ppc/vof.h |   2 +
>   3 files changed, 139 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/ppc/spapr_vof.c b/hw/ppc/spapr_vof.c
> index 46d78756e6..5bf9613005 100644
> --- a/hw/ppc/spapr_vof.c
> +++ b/hw/ppc/spapr_vof.c
> @@ -10,8 +10,15 @@
>   #include "hw/ppc/spapr_cpu_core.h"
>   #include "hw/ppc/fdt.h"
>   #include "hw/ppc/vof.h"
> +#include "hw/ppc/spapr_vof.h"
> +#include "hw/core/qdev.h"
> +#include "hw/core/loader.h"
>   #include "system/system.h"
> +#include "system/block-backend.h"
> +#include "system/block-backend-global-state.h"
>   #include "qom/qom-qobject.h"
> +#include "target/ppc/cpu.h"
> +#include "elf.h"
>   #include "trace.h"
>   
>   target_ulong spapr_h_vof_client(PowerPCCPU *cpu, SpaprMachineState *spapr,
> @@ -32,10 +39,10 @@ void spapr_vof_client_dt_finalize(SpaprMachineState *spapr, void *fdt)
>   
>       vof_build_dt(fdt, spapr->vof);
>   
> -    if (spapr->vof->bootargs) {
> -        int chosen;
> +    int chosen;
> +    _FDT(chosen = fdt_path_offset(fdt, "/chosen"));
>   
> -        _FDT(chosen = fdt_path_offset(fdt, "/chosen"));
> +    if (spapr->vof->bootargs) {
>           /*
>            * If the client did not change "bootargs", spapr_dt_chosen() must have
>            * stored machine->kernel_cmdline in it before getting here.
> @@ -43,6 +50,22 @@ void spapr_vof_client_dt_finalize(SpaprMachineState *spapr, void *fdt)
>           _FDT(fdt_setprop_string(fdt, chosen, "bootargs", spapr->vof->bootargs));
>       }
>   
> +    if (spapr->vof->disk_boot) {
> +        /*
> +         * If disk boot is detected change the "qemu,boot-kernel" to hold
> +         * kernel_addr/kernel_size which contain the GRUB entry point and size
> +         */
> +        uint64_t kern[2];
> +        kern[0] = cpu_to_be64(spapr->kernel_addr);
> +        kern[1] = cpu_to_be64(spapr->kernel_size);
> +        _FDT(fdt_setprop(fdt, chosen, "qemu,boot-kernel", &kern, sizeof(kern)));
> +
> +        if (spapr->vof->bootpath) {
> +            _FDT(fdt_setprop_string(fdt, chosen, "bootpath",
> +                                    spapr->vof->bootpath));
> +        }
> +    }
> +
>       /*
>        * SLOF-less setup requires an open instance of stdout for early
>        * kernel printk. By now all phandles are settled so we can open
> @@ -54,11 +77,100 @@ void spapr_vof_client_dt_finalize(SpaprMachineState *spapr, void *fdt)
>       }
>   }
>   
> +static bool vof_elf_segment_cb(void *opaque,
> +                                uint64_t paddr, uint64_t vaddr,
> +                                uint64_t filesz, uint64_t memsz)
> +{
> +    Vof *vof = opaque;
> +
> +    if (memsz == 0) {
> +        return true;
> +    }
> +
> +    if (paddr != vaddr) {
> +        error_report("spapr_vof_load_elf: segment paddr/vaddr mismatch "
> +                     "(paddr=0x%" PRIx64 " vaddr=0x%" PRIx64 ")",
> +                     paddr, vaddr);
> +        return false;
> +    }
> +
> +    if (vof_claim(vof, paddr, memsz, 0) == -1) {
> +        error_report("spapr_vof_load_elf: vof_claim failed for "
> +                     "paddr=0x%" PRIx64 " size=0x%" PRIx64, paddr, memsz);
> +        return false;
> +    }
> +
> +    return true;
> +}
> +
> +static bool spapr_vof_try_prep_boot(SpaprMachineState *spapr, Vof *vof)
> +{
> +    BlockBackend *blk;
> +    DeviceState *dev;
> +    uint64_t partition_offset;
> +    uint64_t partition_size;
> +    uint8_t *prep_data;
> +    uint64_t entry_point;
> +    uint64_t load_size;
> +
> +    for (blk = blk_next(NULL); blk; blk = blk_next(blk)) {
> +
> +        if (!blk_is_inserted(blk)) {
> +            continue;
> +        }
> +
> +        partition_offset = 0;
> +        partition_size   = 0;
> +
> +        if (!spapr_vof_find_prep_partition(blk, &partition_offset,
> +                                           &partition_size)) {
> +            continue;
> +        }
> +
> +        prep_data = g_malloc(partition_size);
partition_size is derived directly from MBR/GPT data.
Before allocating this buffer, validate using overflow-safe arithmetic.

> +        if (blk_pread(blk, partition_offset, partition_size,
> +                      prep_data, 0) < 0) {
> +            g_free(prep_data);
> +            continue;
> +        }
> +
> +        uint64_t lowaddr = UINT64_MAX, highaddr = 0;
> +        entry_point = 0;
> +        ssize_t ret = load_elf_ram_sym_buf(prep_data, partition_size,
> +                                           NULL, NULL, NULL,
> +                                           &entry_point, &lowaddr, &highaddr,
> +                                           NULL, ELFDATA2MSB,
> +                                           PPC_ELF_MACHINE, 0, 0,
> +                                           NULL, false, NULL,
> +                                           vof_elf_segment_cb, vof);
> +        if (ret <= 0) {
> +            error_report("spapr_vof_try_prep_boot: %s", load_elf_strerror(ret));
> +            g_free(prep_data);
> +            continue;
> +        }
> +        load_size = highaddr - lowaddr;
> +
> +        g_free(prep_data);
> +
> +        spapr->kernel_addr = entry_point;
> +        spapr->kernel_size = load_size;
> +
> +        vof->disk_boot = true;
> +        dev = blk_get_attached_dev(blk);
> +        if (dev) {
> +            vof->bootpath = qdev_get_fw_dev_path(dev);
> +        }
> +        return true;
> +    }
In the above loop if partially PT_LOAD( blk 1, blk 2 succeeded but blk 3 
failed)
spapr_vof_try_prep_boot() then continues and tries the next 
BlockBackend, but I don't see anything undoing the first two claims.

Could a malformed first boot disk therefore poison the VOF allocation 
map and make a valid second disk fail to boot?

This probably needs either a validation-first/commit-second model or 
explicit claim rollback.

Thanks,
Narayana Murty N
> +    return false;
> +}
> +
>   void spapr_vof_reset(SpaprMachineState *spapr, void *fdt, Error **errp)
>   {
>       target_ulong stack_ptr;
>       Vof *vof = spapr->vof;
>       PowerPCCPU *first_ppc_cpu = POWERPC_CPU(first_cpu);
> +    MachineState *machine = MACHINE(spapr);
>   
>       vof_init(vof, spapr->rma_size, errp);
>   
> @@ -70,7 +182,7 @@ void spapr_vof_reset(SpaprMachineState *spapr, void *fdt, Error **errp)
>       /* Stack grows downwards plus reserve space for the minimum stack frame */
>       stack_ptr += VOF_STACK_SIZE - 0x20;
>   
> -    if (spapr->kernel_size &&
> +    if (machine->kernel_filename && spapr->kernel_size &&
>           vof_claim(vof, spapr->kernel_addr, spapr->kernel_size, 0) == -1) {
>           error_setg(errp, "Memory for kernel is in use");
>           return;
> @@ -82,6 +194,14 @@ void spapr_vof_reset(SpaprMachineState *spapr, void *fdt, Error **errp)
>           return;
>       }
>   
> +    /*
> +     * Disk boot: load GRUB from the PReP boot partition on the block device, if
> +     * no kernel/initrd are provided
> +     */
> +    if (!machine->kernel_filename) {
> +        spapr_vof_try_prep_boot(spapr, vof);
> +    }
> +
>       spapr_vof_client_dt_finalize(spapr, fdt);
>   
>       spapr_cpu_set_entry_state(first_ppc_cpu, SPAPR_ENTRY_POINT,
> @@ -91,10 +211,16 @@ void spapr_vof_reset(SpaprMachineState *spapr, void *fdt, Error **errp)
>       /*
>        * At this point the expected allocation map is:
>        *
> -     * 0..c38 - the initial firmware
> -     * 8000..10000 - stack
> -     * 400000.. - kernel
> -     * 3ea0000.. - initramdisk
> +     * Kernel + initrd boot:
> +     *   0..c38      - the initial firmware
> +     *   8000..10000 - stack
> +     *   400000..    - kernel
> +     *   3ea0000..   - initramdisk
> +     *
> +     * Disk (GRUB) boot:
> +     *   0..c38      - the initial firmware
> +     *   8000..10000 - stack
> +     *   400000..    - GRUB (loaded from PReP partition)
>        *
>        * We skip writing FDT as nothing expects it; OF client interface is
>        * going to be used for reading the device tree.
> diff --git a/hw/ppc/vof.c b/hw/ppc/vof.c
> index fa7b73159a..a78bb1f116 100644
> --- a/hw/ppc/vof.c
> +++ b/hw/ppc/vof.c
> @@ -1041,6 +1041,9 @@ void vof_cleanup(Vof *vof)
>       vof->of_instances = NULL;
>       vof->of_instance_last = 0;
>       vof->claimed_base = 0;
> +    g_free(vof->bootpath);
> +    vof->bootpath = NULL;
> +    vof->disk_boot = false;
>   }
>   
>   void vof_build_dt(void *fdt, Vof *vof)
> diff --git a/include/hw/ppc/vof.h b/include/hw/ppc/vof.h
> index 3a0fbffe54..e17779ee8a 100644
> --- a/include/hw/ppc/vof.h
> +++ b/include/hw/ppc/vof.h
> @@ -18,6 +18,8 @@ typedef struct Vof {
>       GHashTable *of_instances; /* ihandle -> SpaprOfInstance */
>       uint32_t of_instance_last;
>       char *bootargs;
> +    char *bootpath;
> +    bool disk_boot;
>       long fw_size;
>   } Vof;
>