[RFC,v2,5/8] ppc/spapr: add block device backend to VOF open/read/write/seek services

From: Utkarsh Verma <uverma@linux.ibm.com>

Extend the VOF client interface to support block device I/O through the
sPAPR SCSI disk, which is needed by bootloaders such as GRUB during VOF
disk boot.

Add BlockBackend and position tracking fields to OfInstance and use them
in vof_seek(), vof_write() and vof_read() to extend their functionality
to handle block devices as well.

In vof_do_open(), detect "disk" FDT nodes, extract the SRP LUN from the
path string, and use scsi_device_find() + blk_by_dev() to obtain the
matching BlockBackend.

Add vof_seek() to implement the OpenFirmware "seek" client service,
which was previously missing.

AI-used-for: code
Signed-off-by: Utkarsh Verma <uverma@linux.ibm.com>
---
Changelog:
v2:
- vof_do_open(): replaced brute force blk_next() loop with
  vof_find_vscsi_bus() + scsi_device_find() + blk_by_dev() for a SCSI
  disk discovery.
- vof_write() block path: capped I/O to VOF_BLK_BUF_SIZE (64 KB) per
  iteration, checked blk_flush() return value, added bdrv_is_read_only()
  guard before any write attempt.
- vof_write() serial path: fixed an existing bug, which did not advance
  the buffer after each iteration and returned the remaining len instead
  of bytes written.
- vof_read() block path: added the same loop pattern as vof_write(),
  advancing buf and position correctly per iteration.
- path_offset(): added wildcard fallback so "disk@<srp-lun>" paths
  resolve to the parent's generic "disk" FDT subnode.
---
 hw/ppc/trace-events |   2 +
 hw/ppc/vof.c        | 217 +++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 215 insertions(+), 4 deletions(-)

On Fri, 2 Oct 2026, uverma@linux.ibm.com wrote:
> From: Utkarsh Verma <uverma@linux.ibm.com>
>
> Extend the VOF client interface to support block device I/O through the
> sPAPR SCSI disk, which is needed by bootloaders such as GRUB during VOF
> disk boot.
>
> Add BlockBackend and position tracking fields to OfInstance and use them
> in vof_seek(), vof_write() and vof_read() to extend their functionality
> to handle block devices as well.
>
> In vof_do_open(), detect "disk" FDT nodes, extract the SRP LUN from the
> path string, and use scsi_device_find() + blk_by_dev() to obtain the
> matching BlockBackend.
>
> Add vof_seek() to implement the OpenFirmware "seek" client service,
> which was previously missing.
>
> AI-used-for: code
> Signed-off-by: Utkarsh Verma <uverma@linux.ibm.com>
> ---
> Changelog:
> v2:
> - vof_do_open(): replaced brute force blk_next() loop with
>  vof_find_vscsi_bus() + scsi_device_find() + blk_by_dev() for a SCSI
>  disk discovery.
> - vof_write() block path: capped I/O to VOF_BLK_BUF_SIZE (64 KB) per
>  iteration, checked blk_flush() return value, added bdrv_is_read_only()
>  guard before any write attempt.
> - vof_write() serial path: fixed an existing bug, which did not advance
>  the buffer after each iteration and returned the remaining len instead
>  of bytes written.
> - vof_read() block path: added the same loop pattern as vof_write(),
>  advancing buf and position correctly per iteration.
> - path_offset(): added wildcard fallback so "disk@<srp-lun>" paths
>  resolve to the parent's generic "disk" FDT subnode.
> ---
> hw/ppc/trace-events |   2 +
> hw/ppc/vof.c        | 217 +++++++++++++++++++++++++++++++++++++++++++-
> 2 files changed, 215 insertions(+), 4 deletions(-)
>
> diff --git a/hw/ppc/trace-events b/hw/ppc/trace-events
> index dbdcfada26..a61398ecb2 100644
> --- a/hw/ppc/trace-events
> +++ b/hw/ppc/trace-events
> @@ -78,6 +78,7 @@ vof_error_unknown_service(const char *service, int nargs, int nret) "\"%s\" args
> vof_error_unknown_method(const char *method) "\"%s\""
> vof_error_unknown_ihandle_close(uint32_t ih) "ih=0x%x"
> vof_error_unknown_path(const char *path) "\"%s\""
> +vof_error_seek(uint32_t ih) "ih=0x%x"
> vof_error_write(uint32_t ih) "ih=0x%x"
> vof_error_read(uint32_t ih) "ih=0x%x"
> vof_finddevice(const char *path, uint32_t ph) "\"%s\" => ph=0x%x"
> @@ -92,6 +93,7 @@ vof_interpret(const char *cmd, uint32_t param1, uint32_t param2, uint32_t ret, u
> vof_package_to_path(uint32_t ph, const char *tmp, int ret) "ph=0x%x => %s len=%d"
> vof_instance_to_path(uint32_t ih, uint32_t ph, const char *tmp, int ret) "ih=0x%x ph=0x%x => %s len=%d"
> vof_instance_to_package(uint32_t ih, uint32_t ph) "ih=0x%x => ph=0x%x"
> +vof_seek(uint32_t ih, uint64_t pos) "ih=0x%x pos=0x%"PRIx64
> vof_write(uint32_t ih, unsigned cb, const char *msg) "ih=0x%x [%u] \"%s\""
> vof_read(uint32_t ih, unsigned cb, const char *msg) "ih=0x%x [%u] \"%s\""
> vof_avail(uint64_t start, uint64_t end, uint64_t size) "0x%"PRIx64"..0x%"PRIx64" size=0x%"PRIx64
> diff --git a/hw/ppc/vof.c b/hw/ppc/vof.c
> index 6f6d424347..d4b57b3b46 100644
> --- a/hw/ppc/vof.c
> +++ b/hw/ppc/vof.c
> @@ -11,6 +11,7 @@
>
> #include CONFIG_DEVICES /* CONFIG_PSERIES */
> #include "qemu/osdep.h"
> +#include "qemu/cutils.h"
> #include "qemu/ctype.h"
> #include "qemu/timer.h"
> #include "qemu/range.h"
> @@ -25,6 +26,9 @@
> #include "trace.h"
>
> #include "hw/ppc/spapr_vio.h"
> +#include "hw/scsi/scsi.h"
> +#include "system/block-backend.h"
> +#include "block/block-io.h"
> #include <libfdt.h>
>
> /*
> @@ -38,6 +42,7 @@
> #define VOF_MAX_METHODLEN   256
> #define VOF_MAX_FORTHCODE   256
> #define VOF_VTY_BUF_SIZE    256
> +#define VOF_BLK_BUF_SIZE    (64 * 1024)

* KiB instead of 1024?

>
> typedef struct {
>     uint64_t start;
> @@ -47,6 +52,8 @@ typedef struct {
> typedef struct {
>     char *path; /* the path used to open the instance */
>     uint32_t phandle;
> +    BlockBackend *blk;
> +    uint64_t pos; /* current position for seek operations */
>     void *vty;
> } OfInstance;
>
> @@ -138,6 +145,8 @@ static int path_offset(const void *fdt, const char *path)
> {
>     g_autofree char *p = NULL;
>     char *at;
> +    char *last_slash;
> +    int offset;
>
>     /*
>      * https://www.devicetree.org/open-firmware/bindings/ppc/release/ppc-2_1.html#HDR16
> @@ -156,7 +165,76 @@ static int path_offset(const void *fdt, const char *path)
>             }
>     }
>
> -    return fdt_path_offset(fdt, p);
> +    offset = fdt_path_offset(fdt, p);
> +    if (offset >= 0) {
> +        return offset;
> +    }
> +
> +    /*
> +     * Apply the OF wildcard rule for vSCSI disk paths. A path ending in
> +     * disk@<srp-lun> has no per-LUN FDT node, instead the FDT contains a single
> +     * generic "disk" child (no srp-lun appended to the path). The phandle is
> +     * shared by all LUNs on the adapter, matching the Open Firmware behaviour.
> +     */
> +    last_slash = strrchr(p, '/');
> +    if (last_slash && strncmp(last_slash + 1, "disk@", 5) == 0) {
> +        int parent_off;
> +
> +        *last_slash = '\0';
> +        parent_off = fdt_path_offset(fdt, p);
> +        if (parent_off >= 0) {
> +            int child_off = fdt_subnode_offset(fdt, parent_off, "disk");

I'm not sure how this should work but what if there are multiple disk 
nodes such as disk@0 disk@1 or similar? Should this look for the original 
ending of the path instead of just "disk"?

> +            if (child_off >= 0) {
> +                return child_off;
> +            }
> +        }
> +    }
> +
> +    return offset;
> +}
> +
> +/*
> + * Return the SCSIBus for a v-scsi@<reg> path, or NULL if not found.
> + */
> +static SCSIBus *vof_find_vscsi_bus(MachineState *ms, const char *path)
> +{
> +#ifdef CONFIG_PSERIES
> +    SpaprMachineState *spapr = SPAPR_MACHINE(ms);
> +    const char *at;
> +    const char *endptr;
> +    unsigned long reg;
> +    SpaprVioDevice *vdev;
> +    BusState *bus;
> +
> +    if (!spapr || !spapr->vio_bus) {
> +        return NULL;
> +    }
> +
> +    at = strstr(path, "v-scsi@");
> +    if (!at) {
> +        return NULL;
> +    }
> +    at = strchr(at, '@');
> +    if (!at) {
> +        return NULL;
> +    }
> +
> +    if (qemu_strtoul(at + 1, &endptr, 16, &reg) || endptr == at + 1) {
> +        return NULL;
> +    }
> +
> +    vdev = spapr_vio_find_by_reg(spapr->vio_bus, (uint32_t)reg);
> +    if (!vdev) {
> +        return NULL;
> +    }
> +
> +    QLIST_FOREACH(bus, &vdev->qdev.child_bus, sibling) {
> +        if (object_dynamic_cast(OBJECT(bus), TYPE_SCSI_BUS)) {
> +            return SCSI_BUS(bus);
> +        }
> +    }
> +#endif /* CONFIG_PSERIES */

No #ifdefs. This vof.c file is the generic machine independent parts, so 
this function should go to spapr_vof.c and maybe exported through the VOF 
machine class interface. Pegasos2 has IDE disks so if I want to later add 
support for this for that machine I should implement similar function 
there and this one here should call the implementation supplied by the 
machine not hard code spapr here. So this should be some generic find 
block device for device node function that the machine implements.

> +    return NULL;
> }
>
> static uint32_t vof_finddevice(const void *fdt, uint32_t nodeaddr)
> @@ -450,6 +528,8 @@ static uint32_t vof_do_open(void *fdt, Vof *vof, int offset, const char *path)
> {
>     uint32_t ret = PROM_ERROR;
>     OfInstance *inst = NULL;
> +    const char *node_name;
> +    MachineState *ms = MACHINE(qdev_get_machine());
>
>     if (vof->of_instance_last == 0xFFFFFFFF) {
>         /* We do not recycle ihandles yet */
> @@ -462,13 +542,43 @@ static uint32_t vof_do_open(void *fdt, Vof *vof, int offset, const char *path)
>     ++vof->of_instance_last;
>
>     inst->path = g_strdup(path);
> +    inst->blk = NULL;
> +    inst->pos = 0;
>     inst->vty = NULL;
>
> +    node_name = fdt_get_name(fdt, offset, NULL);
> +
> +    /*
> +     * vSCSI disk open.  The FDT contains only a generic "disk" child node (no
> +     * srp-lun appended to the path), matching Open Firmware behaviour.
> +     * Extract the LUN from the "disk@<hex>" component of the path string
> +     * and use it to find the corresponding QEMU block backend.
> +     */
> +    if (node_name && strcmp(node_name, "disk") == 0) {
> +        const char *disk_at = strstr(path, "/disk@");
> +        const char *endptr;
> +        uint64_t srp_lun;
> +
> +        if (disk_at &&
> +            qemu_strtou64(disk_at + 6, &endptr, 16, &srp_lun) == 0) {
> +            SCSIBus *sbus = vof_find_vscsi_bus(ms, path);
> +
> +            if (sbus) {
> +                uint32_t target  = (srp_lun >> 56) & 0x3f;
> +                uint32_t channel = (srp_lun >> 53) & 0x7;
> +                uint32_t lun     = (srp_lun >> 48) & 0x1f;
> +                SCSIDevice *sdev = scsi_device_find(sbus, (int)channel,
> +                                                    (int)target, (int)lun);
> +                if (sdev) {
> +                    inst->blk = blk_by_dev(sdev);
> +                }
> +            }
> +        }
> +    }
> +

This should be in spapr_vof.c too as it's specific to spapr. Just call the 
machine function that returns the blk for the node here. The rest below 
seems generic so those are OK to be here in vof.c.

Regards,
BALATON Zoltan

> #ifdef CONFIG_PSERIES
> -    const char *node_name = fdt_get_name(fdt, offset, NULL);
>     if (node_name && strncmp(node_name, "vty", 3) == 0) {
>         uint8_t discard_buf[VOF_VTY_BUF_SIZE];
> -        MachineState *ms = MACHINE(qdev_get_machine());
>         SpaprMachineState *spapr = SPAPR_MACHINE(ms);
>
>         if (spapr && spapr->vio_bus) {
> @@ -602,6 +712,38 @@ static uint32_t vof_write(Vof *vof, uint32_t ihandle, uint32_t buf,
>         return PROM_ERROR;
>     }
>
> +    if (inst->blk) {
> +        if (bdrv_is_read_only(blk_bs(inst->blk))) {
> +            trace_vof_error_write(ihandle);
> +            return PROM_ERROR;
> +        }
> +
> +        g_autofree uint8_t *blkbuf = g_malloc(MIN(len, (uint32_t)VOF_BLK_BUF_SIZE));
> +        uint32_t total = 0;
> +
> +        while (len > 0) {
> +            cb = MIN(len, (uint32_t)VOF_BLK_BUF_SIZE);
> +            if (VOF_MEM_READ(buf, blkbuf, cb) != MEMTX_OK) {
> +                trace_vof_error_write(ihandle);
> +                return PROM_ERROR;
> +            }
> +            if (blk_pwrite(inst->blk, inst->pos, cb, blkbuf, 0) < 0) {
> +                trace_vof_error_write(ihandle);
> +                return PROM_ERROR;
> +            }
> +            inst->pos += cb;
> +            buf += cb;
> +            len -= cb;
> +            total += cb;
> +        }
> +        if (blk_flush(inst->blk) < 0) {
> +            trace_vof_error_write(ihandle);
> +            return PROM_ERROR;
> +        }
> +        trace_vof_write(ihandle, total, "(disk)");
> +        return total;
> +    }
> +
> #ifdef CONFIG_PSERIES
>     if (inst->vty) {
>         uint32_t total_written = 0;
> @@ -619,6 +761,7 @@ static uint32_t vof_write(Vof *vof, uint32_t ihandle, uint32_t buf,
>     }
> #endif
>
> +    uint32_t total_traced = 0;
>     for ( ; len > 0; len -= cb) {
>         cb = MIN(len, sizeof(tmp) - 1);
>         if (VOF_MEM_READ(buf, tmp, cb) != MEMTX_OK) {
> @@ -631,9 +774,11 @@ static uint32_t vof_write(Vof *vof, uint32_t ihandle, uint32_t buf,
>             tmp[cb] = '\0';
>             trace_vof_write(ihandle, cb, tmp);
>         }
> +        buf += cb;
> +        total_traced += cb;
>     }
>
> -    return len;
> +    return total_traced;
> }
>
> static uint32_t vof_read(Vof *vof, uint32_t ihandle, uint32_t buf,
> @@ -647,6 +792,33 @@ static uint32_t vof_read(Vof *vof, uint32_t ihandle, uint32_t buf,
>         return PROM_ERROR;
>     }
>
> +    if (inst->blk) {
> +        g_autofree uint8_t *tmp = g_malloc(MIN(len, (uint32_t)VOF_BLK_BUF_SIZE));
> +        uint32_t total = 0;
> +
> +        while (len > 0) {
> +            unsigned cb = MIN(len, (uint32_t)VOF_BLK_BUF_SIZE);
> +            int ret = blk_pread(inst->blk, inst->pos, cb, tmp, 0);
> +            if (ret < 0) {
> +                trace_vof_error_read(ihandle);
> +                return PROM_ERROR;
> +            }
> +
> +            if (VOF_MEM_WRITE(buf, tmp, cb) != MEMTX_OK) {
> +                trace_vof_error_read(ihandle);
> +                return PROM_ERROR;
> +            }
> +
> +            inst->pos += cb;
> +            buf += cb;
> +            len -= cb;
> +            total += cb;
> +        }
> +
> +        trace_vof_read(ihandle, total, "(disk)");
> +        return total;
> +    }
> +
> #ifdef CONFIG_PSERIES
>     if (inst->vty) {
>         uint8_t tmp[VOF_VTY_BUF_SIZE];
> @@ -676,6 +848,41 @@ static uint32_t vof_read(Vof *vof, uint32_t ihandle, uint32_t buf,
>     return 0;
> }
>
> +static uint32_t vof_seek(Vof *vof, uint32_t ihandle, uint32_t pos_hi,
> +                         uint32_t pos_lo)
> +{
> +    OfInstance *inst = (OfInstance *)
> +        g_hash_table_lookup(vof->of_instances, GINT_TO_POINTER(ihandle));
> +    uint64_t pos = ((uint64_t)pos_hi << 32) | pos_lo;
> +
> +    if (!inst) {
> +        trace_vof_error_seek(ihandle);
> +        return PROM_ERROR;
> +    }
> +
> +    if (inst->blk) {
> +        int64_t size = blk_getlength(inst->blk);
> +
> +        if (size < 0) {
> +            trace_vof_error_seek(ihandle);
> +            return PROM_ERROR;
> +        }
> +
> +        if (pos > (uint64_t)size) {
> +            trace_vof_error_seek(ihandle);
> +            return PROM_ERROR;
> +        }
> +
> +        inst->pos = pos;
> +        trace_vof_seek(ihandle, pos);
> +        return 0;
> +    }
> +
> +    /* VTY and other devices don't support seek */
> +    trace_vof_error_seek(ihandle);
> +    return PROM_ERROR;
> +}
> +
> static void vof_claimed_dump(GArray *claimed)
> {
>     int i;
> @@ -986,6 +1193,8 @@ static uint32_t vof_client_handle(MachineState *ms, void *fdt, Vof *vof,
>         ret = vof_package_to_path(fdt, args[0], args[1], args[2]);
>     } else if (cmpserv("instance-to-path", 3, 1)) {
>         ret = vof_instance_to_path(fdt, vof, args[0], args[1], args[2]);
> +    } else if (cmpserv("seek", 3, 1)) {
> +        ret = vof_seek(vof, args[0], args[1], args[2]);
>     } else if (cmpserv("write", 3, 1)) {
>         ret = vof_write(vof, args[0], args[1], args[2]);
>     } else if (cmpserv("read", 3, 1)) {
>

On 10/7/26 11:50 PM, BALATON Zoltan wrote:
> On Fri, 2 Oct 2026, uverma@linux.ibm.com wrote:
>> From: Utkarsh Verma <uverma@linux.ibm.com>
>>
>> Extend the VOF client interface to support block device I/O through the
>> sPAPR SCSI disk, which is needed by bootloaders such as GRUB during VOF
>> disk boot.
>>
>> Add BlockBackend and position tracking fields to OfInstance and use them
>> in vof_seek(), vof_write() and vof_read() to extend their functionality
>> to handle block devices as well.
>>
>> In vof_do_open(), detect "disk" FDT nodes, extract the SRP LUN from the
>> path string, and use scsi_device_find() + blk_by_dev() to obtain the
>> matching BlockBackend.
>>
>> Add vof_seek() to implement the OpenFirmware "seek" client service,
>> which was previously missing.
>>
>> AI-used-for: code
>> Signed-off-by: Utkarsh Verma <uverma@linux.ibm.com>
>> ---
>> Changelog:
>> v2:
>> - vof_do_open(): replaced brute force blk_next() loop with
>>  vof_find_vscsi_bus() + scsi_device_find() + blk_by_dev() for a SCSI
>>  disk discovery.
>> - vof_write() block path: capped I/O to VOF_BLK_BUF_SIZE (64 KB) per
>>  iteration, checked blk_flush() return value, added bdrv_is_read_only()
>>  guard before any write attempt.
>> - vof_write() serial path: fixed an existing bug, which did not advance
>>  the buffer after each iteration and returned the remaining len instead
>>  of bytes written.
>> - vof_read() block path: added the same loop pattern as vof_write(),
>>  advancing buf and position correctly per iteration.
>> - path_offset(): added wildcard fallback so "disk@<srp-lun>" paths
>>  resolve to the parent's generic "disk" FDT subnode.
>> ---
>> hw/ppc/trace-events |   2 +
>> hw/ppc/vof.c        | 217 +++++++++++++++++++++++++++++++++++++++++++-
>> 2 files changed, 215 insertions(+), 4 deletions(-)
>>
>> diff --git a/hw/ppc/trace-events b/hw/ppc/trace-events
>> index dbdcfada26..a61398ecb2 100644
>> --- a/hw/ppc/trace-events
>> +++ b/hw/ppc/trace-events
>> @@ -78,6 +78,7 @@ vof_error_unknown_service(const char *service, int 
>> nargs, int nret) "\"%s\" args
>> vof_error_unknown_method(const char *method) "\"%s\""
>> vof_error_unknown_ihandle_close(uint32_t ih) "ih=0x%x"
>> vof_error_unknown_path(const char *path) "\"%s\""
>> +vof_error_seek(uint32_t ih) "ih=0x%x"
>> vof_error_write(uint32_t ih) "ih=0x%x"
>> vof_error_read(uint32_t ih) "ih=0x%x"
>> vof_finddevice(const char *path, uint32_t ph) "\"%s\" => ph=0x%x"
>> @@ -92,6 +93,7 @@ vof_interpret(const char *cmd, uint32_t param1, 
>> uint32_t param2, uint32_t ret, u
>> vof_package_to_path(uint32_t ph, const char *tmp, int ret) "ph=0x%x => 
>> %s len=%d"
>> vof_instance_to_path(uint32_t ih, uint32_t ph, const char *tmp, int 
>> ret) "ih=0x%x ph=0x%x => %s len=%d"
>> vof_instance_to_package(uint32_t ih, uint32_t ph) "ih=0x%x => ph=0x%x"
>> +vof_seek(uint32_t ih, uint64_t pos) "ih=0x%x pos=0x%"PRIx64
>> vof_write(uint32_t ih, unsigned cb, const char *msg) "ih=0x%x [%u] 
>> \"%s\""
>> vof_read(uint32_t ih, unsigned cb, const char *msg) "ih=0x%x [%u] \"%s\""
>> vof_avail(uint64_t start, uint64_t end, uint64_t size) 
>> "0x%"PRIx64"..0x%"PRIx64" size=0x%"PRIx64
>> diff --git a/hw/ppc/vof.c b/hw/ppc/vof.c
>> index 6f6d424347..d4b57b3b46 100644
>> --- a/hw/ppc/vof.c
>> +++ b/hw/ppc/vof.c
>> @@ -11,6 +11,7 @@
>>
>> #include CONFIG_DEVICES /* CONFIG_PSERIES */
>> #include "qemu/osdep.h"
>> +#include "qemu/cutils.h"
>> #include "qemu/ctype.h"
>> #include "qemu/timer.h"
>> #include "qemu/range.h"
>> @@ -25,6 +26,9 @@
>> #include "trace.h"
>>
>> #include "hw/ppc/spapr_vio.h"
>> +#include "hw/scsi/scsi.h"
>> +#include "system/block-backend.h"
>> +#include "block/block-io.h"
>> #include <libfdt.h>
>>
>> /*
>> @@ -38,6 +42,7 @@
>> #define VOF_MAX_METHODLEN   256
>> #define VOF_MAX_FORTHCODE   256
>> #define VOF_VTY_BUF_SIZE    256
>> +#define VOF_BLK_BUF_SIZE    (64 * 1024)
> 
> * KiB instead of 1024?
> 
Sure, will use KiB and fix it for SPAPR_VOF_PREP_MAX_SIZE in spapr_vof.c 
as well in v3.

>>
>> typedef struct {
>>     uint64_t start;
>> @@ -47,6 +52,8 @@ typedef struct {
>> typedef struct {
>>     char *path; /* the path used to open the instance */
>>     uint32_t phandle;
>> +    BlockBackend *blk;
>> +    uint64_t pos; /* current position for seek operations */
>>     void *vty;
>> } OfInstance;
>>
>> @@ -138,6 +145,8 @@ static int path_offset(const void *fdt, const char 
>> *path)
>> {
>>     g_autofree char *p = NULL;
>>     char *at;
>> +    char *last_slash;
>> +    int offset;
>>
>>     /*
>>      * https://www.devicetree.org/open-firmware/bindings/ppc/release/ 
>> ppc-2_1.html#HDR16
>> @@ -156,7 +165,76 @@ static int path_offset(const void *fdt, const 
>> char *path)
>>             }
>>     }
>>
>> -    return fdt_path_offset(fdt, p);
>> +    offset = fdt_path_offset(fdt, p);
>> +    if (offset >= 0) {
>> +        return offset;
>> +    }
>> +
>> +    /*
>> +     * Apply the OF wildcard rule for vSCSI disk paths. A path ending in
>> +     * disk@<srp-lun> has no per-LUN FDT node, instead the FDT 
>> contains a single
>> +     * generic "disk" child (no srp-lun appended to the path). The 
>> phandle is
>> +     * shared by all LUNs on the adapter, matching the Open Firmware 
>> behaviour.
>> +     */
>> +    last_slash = strrchr(p, '/');
>> +    if (last_slash && strncmp(last_slash + 1, "disk@", 5) == 0) {
>> +        int parent_off;
>> +
>> +        *last_slash = '\0';
>> +        parent_off = fdt_path_offset(fdt, p);
>> +        if (parent_off >= 0) {
>> +            int child_off = fdt_subnode_offset(fdt, parent_off, "disk");
> 
> I'm not sure how this should work but what if there are multiple disk 
> nodes such as disk@0 disk@1 or similar? Should this look for the 
> original ending of the path instead of just "disk"?
> 
There is only one "disk" child node under the vSCSI adapter regardless 
of how many physical disks are attached. It seems a little weird that 
only one disk node is created even for multiple vSCSI disks, but that is 
the actual open firmware behaviour, I cross checked with the device tree 
data on a pseries LPAR machine and with the SLOF implementation of it as 
well.
So, the GRUB only searches for a "disk" child node under the vscsi 
adapter and then calls the vscsi-report-luns method to get the full path 
of the individual disks and passes the same path to firmware while 
calling open. So, GRUB doesn't rely on FDT disk data to search the disk, 
it calls the vscs-report-luns method to get the full disk path.
This is why I strip the @<srp-lun> part in the path before searching the 
FDT, the node it needs to resolve to is always the shared "disk" child node.

>> +            if (child_off >= 0) {
>> +                return child_off;
>> +            }
>> +        }
>> +    }
>> +
>> +    return offset;
>> +}
>> +
>> +/*
>> + * Return the SCSIBus for a v-scsi@<reg> path, or NULL if not found.
>> + */
>> +static SCSIBus *vof_find_vscsi_bus(MachineState *ms, const char *path)
>> +{
>> +#ifdef CONFIG_PSERIES
>> +    SpaprMachineState *spapr = SPAPR_MACHINE(ms);
>> +    const char *at;
>> +    const char *endptr;
>> +    unsigned long reg;
>> +    SpaprVioDevice *vdev;
>> +    BusState *bus;
>> +
>> +    if (!spapr || !spapr->vio_bus) {
>> +        return NULL;
>> +    }
>> +
>> +    at = strstr(path, "v-scsi@");
>> +    if (!at) {
>> +        return NULL;
>> +    }
>> +    at = strchr(at, '@');
>> +    if (!at) {
>> +        return NULL;
>> +    }
>> +
>> +    if (qemu_strtoul(at + 1, &endptr, 16, &reg) || endptr == at + 1) {
>> +        return NULL;
>> +    }
>> +
>> +    vdev = spapr_vio_find_by_reg(spapr->vio_bus, (uint32_t)reg);
>> +    if (!vdev) {
>> +        return NULL;
>> +    }
>> +
>> +    QLIST_FOREACH(bus, &vdev->qdev.child_bus, sibling) {
>> +        if (object_dynamic_cast(OBJECT(bus), TYPE_SCSI_BUS)) {
>> +            return SCSI_BUS(bus);
>> +        }
>> +    }
>> +#endif /* CONFIG_PSERIES */
> 
> No #ifdefs. This vof.c file is the generic machine independent parts, so 
> this function should go to spapr_vof.c and maybe exported through the 
> VOF machine class interface. Pegasos2 has IDE disks so if I want to 
> later add support for this for that machine I should implement similar 
> function there and this one here should call the implementation supplied 
> by the machine not hard code spapr here. So this should be some generic 
> find block device for device node function that the machine implements.
> 
Agreed, I will move this vof_find_vscsi_bus function in spapr_vof.c

>> +    return NULL;
>> }
>>
>> static uint32_t vof_finddevice(const void *fdt, uint32_t nodeaddr)
>> @@ -450,6 +528,8 @@ static uint32_t vof_do_open(void *fdt, Vof *vof, 
>> int offset, const char *path)
>> {
>>     uint32_t ret = PROM_ERROR;
>>     OfInstance *inst = NULL;
>> +    const char *node_name;
>> +    MachineState *ms = MACHINE(qdev_get_machine());
>>
>>     if (vof->of_instance_last == 0xFFFFFFFF) {
>>         /* We do not recycle ihandles yet */
>> @@ -462,13 +542,43 @@ static uint32_t vof_do_open(void *fdt, Vof *vof, 
>> int offset, const char *path)
>>     ++vof->of_instance_last;
>>
>>     inst->path = g_strdup(path);
>> +    inst->blk = NULL;
>> +    inst->pos = 0;
>>     inst->vty = NULL;
>>
>> +    node_name = fdt_get_name(fdt, offset, NULL);
>> +
>> +    /*
>> +     * vSCSI disk open.  The FDT contains only a generic "disk" child 
>> node (no
>> +     * srp-lun appended to the path), matching Open Firmware behaviour.
>> +     * Extract the LUN from the "disk@<hex>" component of the path 
>> string
>> +     * and use it to find the corresponding QEMU block backend.
>> +     */
>> +    if (node_name && strcmp(node_name, "disk") == 0) {
>> +        const char *disk_at = strstr(path, "/disk@");
>> +        const char *endptr;
>> +        uint64_t srp_lun;
>> +
>> +        if (disk_at &&
>> +            qemu_strtou64(disk_at + 6, &endptr, 16, &srp_lun) == 0) {
>> +            SCSIBus *sbus = vof_find_vscsi_bus(ms, path);
>> +
>> +            if (sbus) {
>> +                uint32_t target  = (srp_lun >> 56) & 0x3f;
>> +                uint32_t channel = (srp_lun >> 53) & 0x7;
>> +                uint32_t lun     = (srp_lun >> 48) & 0x1f;
>> +                SCSIDevice *sdev = scsi_device_find(sbus, (int)channel,
>> +                                                    (int)target, 
>> (int)lun);
>> +                if (sdev) {
>> +                    inst->blk = blk_by_dev(sdev);
>> +                }
>> +            }
>> +        }
>> +    }
>> +
> 
> This should be in spapr_vof.c too as it's specific to spapr. Just call 
> the machine function that returns the blk for the node here. The rest 
> below seems generic so those are OK to be here in vof.c.
> 
Will add a new find_blk kind of a callback function in 
VofMachineIfClass, and move all this code inside that function in 
spapr_vof.c and just call hook from here.
  > Regards,
> BALATON Zoltan
> 
>> #ifdef CONFIG_PSERIES
>> -    const char *node_name = fdt_get_name(fdt, offset, NULL);
>>     if (node_name && strncmp(node_name, "vty", 3) == 0) {
>>         uint8_t discard_buf[VOF_VTY_BUF_SIZE];
>> -        MachineState *ms = MACHINE(qdev_get_machine());
>>         SpaprMachineState *spapr = SPAPR_MACHINE(ms);
>>
>>         if (spapr && spapr->vio_bus) {
>> @@ -602,6 +712,38 @@ static uint32_t vof_write(Vof *vof, uint32_t 
>> ihandle, uint32_t buf,
>>         return PROM_ERROR;
>>     }
>>
>> +    if (inst->blk) {
>> +        if (bdrv_is_read_only(blk_bs(inst->blk))) {
>> +            trace_vof_error_write(ihandle);
>> +            return PROM_ERROR;
>> +        }
>> +
>> +        g_autofree uint8_t *blkbuf = g_malloc(MIN(len, 
>> (uint32_t)VOF_BLK_BUF_SIZE));
>> +        uint32_t total = 0;
>> +
>> +        while (len > 0) {
>> +            cb = MIN(len, (uint32_t)VOF_BLK_BUF_SIZE);
>> +            if (VOF_MEM_READ(buf, blkbuf, cb) != MEMTX_OK) {
>> +                trace_vof_error_write(ihandle);
>> +                return PROM_ERROR;
>> +            }
>> +            if (blk_pwrite(inst->blk, inst->pos, cb, blkbuf, 0) < 0) {
>> +                trace_vof_error_write(ihandle);
>> +                return PROM_ERROR;
>> +            }
>> +            inst->pos += cb;
>> +            buf += cb;
>> +            len -= cb;
>> +            total += cb;
>> +        }
>> +        if (blk_flush(inst->blk) < 0) {
>> +            trace_vof_error_write(ihandle);
>> +            return PROM_ERROR;
>> +        }
>> +        trace_vof_write(ihandle, total, "(disk)");
>> +        return total;
>> +    }
>> +
>> #ifdef CONFIG_PSERIES
>>     if (inst->vty) {
>>         uint32_t total_written = 0;
>> @@ -619,6 +761,7 @@ static uint32_t vof_write(Vof *vof, uint32_t 
>> ihandle, uint32_t buf,
>>     }
>> #endif
>>
>> +    uint32_t total_traced = 0;
>>     for ( ; len > 0; len -= cb) {
>>         cb = MIN(len, sizeof(tmp) - 1);
>>         if (VOF_MEM_READ(buf, tmp, cb) != MEMTX_OK) {
>> @@ -631,9 +774,11 @@ static uint32_t vof_write(Vof *vof, uint32_t 
>> ihandle, uint32_t buf,
>>             tmp[cb] = '\0';
>>             trace_vof_write(ihandle, cb, tmp);
>>         }
>> +        buf += cb;
>> +        total_traced += cb;
>>     }
>>
>> -    return len;
>> +    return total_traced;
>> }
>>
>> static uint32_t vof_read(Vof *vof, uint32_t ihandle, uint32_t buf,
>> @@ -647,6 +792,33 @@ static uint32_t vof_read(Vof *vof, uint32_t 
>> ihandle, uint32_t buf,
>>         return PROM_ERROR;
>>     }
>>
>> +    if (inst->blk) {
>> +        g_autofree uint8_t *tmp = g_malloc(MIN(len, 
>> (uint32_t)VOF_BLK_BUF_SIZE));
>> +        uint32_t total = 0;
>> +
>> +        while (len > 0) {
>> +            unsigned cb = MIN(len, (uint32_t)VOF_BLK_BUF_SIZE);
>> +            int ret = blk_pread(inst->blk, inst->pos, cb, tmp, 0);
>> +            if (ret < 0) {
>> +                trace_vof_error_read(ihandle);
>> +                return PROM_ERROR;
>> +            }
>> +
>> +            if (VOF_MEM_WRITE(buf, tmp, cb) != MEMTX_OK) {
>> +                trace_vof_error_read(ihandle);
>> +                return PROM_ERROR;
>> +            }
>> +
>> +            inst->pos += cb;
>> +            buf += cb;
>> +            len -= cb;
>> +            total += cb;
>> +        }
>> +
>> +        trace_vof_read(ihandle, total, "(disk)");
>> +        return total;
>> +    }
>> +
>> #ifdef CONFIG_PSERIES
>>     if (inst->vty) {
>>         uint8_t tmp[VOF_VTY_BUF_SIZE];
>> @@ -676,6 +848,41 @@ static uint32_t vof_read(Vof *vof, uint32_t 
>> ihandle, uint32_t buf,
>>     return 0;
>> }
>>
>> +static uint32_t vof_seek(Vof *vof, uint32_t ihandle, uint32_t pos_hi,
>> +                         uint32_t pos_lo)
>> +{
>> +    OfInstance *inst = (OfInstance *)
>> +        g_hash_table_lookup(vof->of_instances, 
>> GINT_TO_POINTER(ihandle));
>> +    uint64_t pos = ((uint64_t)pos_hi << 32) | pos_lo;
>> +
>> +    if (!inst) {
>> +        trace_vof_error_seek(ihandle);
>> +        return PROM_ERROR;
>> +    }
>> +
>> +    if (inst->blk) {
>> +        int64_t size = blk_getlength(inst->blk);
>> +
>> +        if (size < 0) {
>> +            trace_vof_error_seek(ihandle);
>> +            return PROM_ERROR;
>> +        }
>> +
>> +        if (pos > (uint64_t)size) {
>> +            trace_vof_error_seek(ihandle);
>> +            return PROM_ERROR;
>> +        }
>> +
>> +        inst->pos = pos;
>> +        trace_vof_seek(ihandle, pos);
>> +        return 0;
>> +    }
>> +
>> +    /* VTY and other devices don't support seek */
>> +    trace_vof_error_seek(ihandle);
>> +    return PROM_ERROR;
>> +}
>> +
>> static void vof_claimed_dump(GArray *claimed)
>> {
>>     int i;
>> @@ -986,6 +1193,8 @@ static uint32_t vof_client_handle(MachineState 
>> *ms, void *fdt, Vof *vof,
>>         ret = vof_package_to_path(fdt, args[0], args[1], args[2]);
>>     } else if (cmpserv("instance-to-path", 3, 1)) {
>>         ret = vof_instance_to_path(fdt, vof, args[0], args[1], args[2]);
>> +    } else if (cmpserv("seek", 3, 1)) {
>> +        ret = vof_seek(vof, args[0], args[1], args[2]);
>>     } else if (cmpserv("write", 3, 1)) {
>>         ret = vof_write(vof, args[0], args[1], args[2]);
>>     } else if (cmpserv("read", 3, 1)) {
>>
> 
Regards,
Utkarsh

Hello,

On Thu, 8 Oct 2026, Utkarsh Verma wrote:
[...]
>>> diff --git a/hw/ppc/vof.c b/hw/ppc/vof.c
>>> index 6f6d424347..d4b57b3b46 100644
>>> --- a/hw/ppc/vof.c
>>> +++ b/hw/ppc/vof.c
>>> @@ -11,6 +11,7 @@
>>> 
>>> #include CONFIG_DEVICES /* CONFIG_PSERIES */
>>> #include "qemu/osdep.h"
>>> +#include "qemu/cutils.h"
>>> #include "qemu/ctype.h"
>>> #include "qemu/timer.h"
>>> #include "qemu/range.h"
>>> @@ -25,6 +26,9 @@
>>> #include "trace.h"
>>> 
>>> #include "hw/ppc/spapr_vio.h"
>>> +#include "hw/scsi/scsi.h"
>>> +#include "system/block-backend.h"
>>> +#include "block/block-io.h"
>>> #include <libfdt.h>
>>> 
>>> /*
>>> @@ -38,6 +42,7 @@
>>> #define VOF_MAX_METHODLEN   256
>>> #define VOF_MAX_FORTHCODE   256
>>> #define VOF_VTY_BUF_SIZE    256
>>> +#define VOF_BLK_BUF_SIZE    (64 * 1024)
>> 
>> * KiB instead of 1024?
>> 
> Sure, will use KiB and fix it for SPAPR_VOF_PREP_MAX_SIZE in spapr_vof.c as 
> well in v3.

Sorry for taking that long to review which caused you needing to prepare 
another version but I have little time for QEMU now and generally don't 
like to be first to review AI generated patches so I hoped somebody else 
could do a review. Since the list is already high volume, if you agree 
with a change it's OK to just include it in next version and you don't 
have to reply for those with "OK, I'll do that" here, unless you have a 
comment or question about it or answering a question asked.

>>> 
>>> typedef struct {
>>>     uint64_t start;
>>> @@ -47,6 +52,8 @@ typedef struct {
>>> typedef struct {
>>>     char *path; /* the path used to open the instance */
>>>     uint32_t phandle;
>>> +    BlockBackend *blk;
>>> +    uint64_t pos; /* current position for seek operations */
>>>     void *vty;
>>> } OfInstance;
>>> 
>>> @@ -138,6 +145,8 @@ static int path_offset(const void *fdt, const char 
>>> *path)
>>> {
>>>     g_autofree char *p = NULL;
>>>     char *at;
>>> +    char *last_slash;
>>> +    int offset;
>>> 
>>>     /*
>>>      * https://www.devicetree.org/open-firmware/bindings/ppc/release/ 
>>> ppc-2_1.html#HDR16
>>> @@ -156,7 +165,76 @@ static int path_offset(const void *fdt, const char 
>>> *path)
>>>             }
>>>     }
>>> 
>>> -    return fdt_path_offset(fdt, p);
>>> +    offset = fdt_path_offset(fdt, p);
>>> +    if (offset >= 0) {
>>> +        return offset;
>>> +    }
>>> +
>>> +    /*
>>> +     * Apply the OF wildcard rule for vSCSI disk paths. A path ending in
>>> +     * disk@<srp-lun> has no per-LUN FDT node, instead the FDT contains a 
>>> single
>>> +     * generic "disk" child (no srp-lun appended to the path). The 
>>> phandle is
>>> +     * shared by all LUNs on the adapter, matching the Open Firmware 
>>> behaviour.
>>> +     */
>>> +    last_slash = strrchr(p, '/');
>>> +    if (last_slash && strncmp(last_slash + 1, "disk@", 5) == 0) {
>>> +        int parent_off;
>>> +
>>> +        *last_slash = '\0';
>>> +        parent_off = fdt_path_offset(fdt, p);
>>> +        if (parent_off >= 0) {
>>> +            int child_off = fdt_subnode_offset(fdt, parent_off, "disk");
>> 
>> I'm not sure how this should work but what if there are multiple disk nodes 
>> such as disk@0 disk@1 or similar? Should this look for the original ending 
>> of the path instead of just "disk"?
>> 
> There is only one "disk" child node under the vSCSI adapter regardless of how 
> many physical disks are attached. It seems a little weird that only one disk 
> node is created even for multiple vSCSI disks, but that is the actual open 
> firmware behaviour, I cross checked with the device tree data on a pseries 
> LPAR machine and with the SLOF implementation of it as well.
> So, the GRUB only searches for a "disk" child node under the vscsi adapter 
> and then calls the vscsi-report-luns method to get the full path of the 
> individual disks and passes the same path to firmware while calling open. So, 
> GRUB doesn't rely on FDT disk data to search the disk, it calls the 
> vscs-report-luns method to get the full disk path.
> This is why I strip the @<srp-lun> part in the path before searching the FDT, 
> the node it needs to resolve to is always the shared "disk" child node.

We're in in the common vof.c file here which is a generic OpenFirmware 
implementation and is shared with pegasos (and later could be used by 
other machines needing OpenFirmware). On pegasos2 for example we can have 
/pci/ide/disk@0,0, /pci/ide/disk@0,1, /pci/ide/disk@1,0 and 
/pci/ide/disk@1,1 for 4 IDE disks so this change may break clients 
accessing those device nodes. So likely you need to avoid changing this 
function but maybe by moving all the other stuff to spapr_vof.c this will 
not be needed here any more and can be handled in spapr_vof.c. If you find 
you're trying to change vof.c for something specific to spapr then think 
again because anything spapr specific should be in spapr_vof.c.

I've also noticed pegasos1 sets boot-device to /pci/ide/disk@0,0:0 where 
:0 is showing the partition so that is a generic OF thing that could be 
handled here in vof.c but that could be fixed later as well. To make it 
easy to use this block device support by other machines in the future, the 
functions to be added to VofMachineIfClass should be chosen to do the 
machine specific mapping between OpenFirmware nodes to generic QEMU 
constructs and then the code here can work on those without needing to be 
machine specific. E.g. I'm not sure requiring machines to implement 
vty_open/read/write is necessary and if it could be done more generically 
with just one function mapping an OF device node to something that can be 
used here like for block device the mapping function returns a blk device 
for an OF device node. That would also make machine code simpler as it 
does not have to talk to the device just return the QEMU struct to vof and 
reduce code duplication between machines. But I don't know what would be 
the right QEMU construct for a vty, maybe a chardev?

Regards,
BALATON Zoltan