All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH 0/8] ppc/spapr: VOF disk image (qcow2) boot support
@ 2026-08-17 10:27 uverma
  2026-08-17 10:27 ` [RFC PATCH 1/8] ppc/spapr: add PReP boot partition detection uverma
                   ` (8 more replies)
  0 siblings, 9 replies; 25+ messages in thread
From: uverma @ 2026-08-17 10:27 UTC (permalink / raw)
  To: qemu-devel, qemu-ppc, aik
  Cc: pbonzini, th.huth, nnmlinux, sbhat, harshpb, amachhiw, rathc,
	balaton, philmd, npiggin, marcandre.lureau, fam, Utkarsh Verma

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

This series adds support for booting from a disk image under the Virtual Open
Firmware (VOF) path for sPAPR/pseries machines in QEMU.

The most common use case for pseries VMs is booting from a distro qcow2 disk
image. These images carry a PReP boot partition containing GRUB2, which in turn
loads the kernel and initrd via IEEE 1275 client interface calls.
VOF currently only supports direct -kernel/-initrd boot; there is no
firmware-mediated path to hand off to a bootloader on disk. This series
implements that missing path, making VOF capable of handling the standard
distro qcow2 boot flow without requiring SLOF.

This series takes reference from an earlier implementation by Alexey
Kardashevskiy [1] which explored the same concept but was not carried forward at
the time.

The existing load_elf function works on a file, whereas in the VOF the PReP
partition (GRUB) is read from the disk image and loaded in the memory, also
on loading the PReP partition in the guest memory the /memory@0/available FDT
needs to be properly updated for each PT_LOAD segment of the core.elf file
of the GRUB (PReP partition).

Implemented a new public elf loader API load_elf_ram_sym_buf() to resolve both
these issues. load_elf_ram_sym_buf() accepts a caller-supplied memory buffer
instead of a filename. Internally it writes the buffer to an anonymous memfd
and delegates to the existing load_elf32/load_elf64 paths, so all existing
behaviour is preserved. Also, added optional segment_fn_t callback in
load_elf_ram_sym_buf() through the internal elf_ops.h.inc template; it is
invoked once per PT_LOAD segment after the load address is resolved, allowing
callers to perform per-segment work such as VOF memory claims.

When no -kernel is given, spapr_vof_reset() calls a newly implemented function
spapr_vof_try_prep_boot() which iterates attached block backends, scans
partitions for a PReP boot partition, reads the partition contents into a
host buffer, calls load_elf_ram_sym_buf() to load the GRUB in the guest memory,
and on success stores the ELF entry point in spapr->kernel_addr.
spapr_vof_client_dt_finalize() then writes the entry point into
/chosen/qemu,boot-kernel so VOF then transfers control to GRUB.

- Partition scanner (spapr_vof_partition.c): scans MBR and GPT partition tables
  on a BlockBackend to locate the PReP boot partition.

- Core loader extension (hw/core/loader.c): new load_elf_ram_sym_buf() API loads
  a 32/64-bit ELF binary from a caller-supplied memory buffer, reusing the
  existing load_elf32/load_elf64 paths via a memfd. A new segment_fn_t callback
  in elf_ops.h.inc lets callers act on each PT_LOAD segment as it is resolved.

- read, seek OF services: two previously unimplemented IEEE 1275 client
  services, covering block device I/O needed by GRUB.

- vscsi-report-luns method: walks the SCSIBus and builds the LUN report table
  in guest memory in the format expected by GRUB's ofdisk driver, matching
  SLOF's dev-generate-srplun encoding.

- VTY write/read routing: routes console I/O through the VTY chardev backend;
  promotes vty_getchars to external linkage.

- FDT disk@<lun> child nodes (spapr_vscsi.c): emits a child FDT node per
  attached SCSI disk in VOF mode, giving GRUB a fully-qualified OF path to open.
  Gated on spapr->vof so SLOF boots are unaffected.

We used AI to build prototype with manual changes incorporated as identified
during debugging multiple issues faced while making it work, hence using below
tag as suggested in [2].
AI-used-for: Code

Testing:

The series has been tested end-to-end by booting a RHEL9.7 qcow2 image on a
pseries machine with VOF enabled (no SLOF).

build$ ./qemu-system-ppc64 -M pseries,x-vof=on -cpu power9 -m 2G \
-hda ../../rhel-guest-image-9.7-20251021.0.ppc64le.qcow2 -nographic
qemu-system-ppc64: warning: TCG doesn't support requested feature, cap-cfpc=workaround
qemu-system-ppc64: warning: TCG doesn't support requested feature, cap-ibs=workaround
qemu-system-ppc64: warning: TCG doesn't support requested feature, cap-ccf-assist=on
Welcome to GRUB!
<snip>
                               GRUB version 2.06

 +----------------------------------------------------------------------------+
 |*Red Hat Enterprise Linux (5.14.0-611.5.1.el9_7.ppc64le) 9.7 (Plow)         | 
 |                                                                            |
 |                                                                            |
 |                                                                            |
 |                                                                            |
 |                                                                            |
 |                                                                            |
 |                                                                            |
 |                                                                            |
 |                                                                            |
 |                                                                            |
 |                                                                            | 
 +----------------------------------------------------------------------------+

      Use the ^ and v keys to select which entry is highlighted.          
      Press enter to boot the selected OS, `e' to edit the commands       
      before booting or `c' for a command-line.                           

<snip>

Red Hat Enterprise Linux 9.7 (Plow)
Kernel 5.14.0-611.5.1.el9_7.ppc64le on an ppc64le

Activate the web console with: systemctl enable --now cockpit.socket

localhost login: root
Password: 
Last login: Fri Jul 10 01:45:44 on hvc0
[root@localhost ~]# 
[root@localhost ~]# ls /
afs  boot  etc	 lib	media  opt   root  sbin  sys  usr
bin  dev   home  lib64	mnt    proc  run   srv	 tmp  var
[root@localhost ~]# 
[root@localhost ~]# echo "Hello, World!" > hello.txt
[root@localhost ~]# cat hello.txt 
Hello, World!
[root@localhost ~]# 

[1] https://lore.kernel.org/qemu-devel/20200203032943.121178-1-aik@ozlabs.ru/#t
[2] https://lore.kernel.org/qemu-devel/20260529094619.1034458-1-pbonzini@redhat.com/

Utkarsh Verma (8):
  ppc/spapr: add PReP boot partition detection
  hw/loader: add load_elf_ram_sym_buf() for in-memory ELF loading
  ppc/spapr: add baseline VOF disk boot support
  ppc/spapr: add VTY backend support to OF read/write/open services
  ppc/spapr: add block device backend to VOF open/read/write/seek
    services
  spapr_vscsi: add VOF disk nodes to the device tree
  ppc/spapr: strip OF path argument suffix in path_offset
  ppc/spapr: implement vscsi-report-luns call-method for PAPR vSCSI

 hw/char/spapr_vty.c          |   2 +-
 hw/core/loader.c             |  77 ++++++-
 hw/ppc/meson.build           |   5 +-
 hw/ppc/spapr_vof.c           | 144 +++++++++++-
 hw/ppc/spapr_vof_partition.c | 176 ++++++++++++++
 hw/ppc/trace-events          |   4 +
 hw/ppc/vof.c                 | 429 +++++++++++++++++++++++++++++++++++
 hw/scsi/spapr_vscsi.c        |  40 ++++
 include/hw/core/loader.h     |  30 +++
 include/hw/elf_ops.h.inc     |  12 +-
 include/hw/ppc/spapr_vio.h   |   1 +
 include/hw/ppc/spapr_vof.h   |  14 ++
 include/hw/ppc/vof.h         |   2 +
 13 files changed, 923 insertions(+), 13 deletions(-)
 create mode 100644 hw/ppc/spapr_vof_partition.c
 create mode 100644 include/hw/ppc/spapr_vof.h

-- 
2.54.0



^ permalink raw reply	[flat|nested] 25+ messages in thread

* [RFC PATCH 1/8] ppc/spapr: add PReP boot partition detection
  2026-08-17 10:27 [RFC PATCH 0/8] ppc/spapr: VOF disk image (qcow2) boot support uverma
@ 2026-08-17 10:27 ` uverma
  2026-09-17 17:14   ` Narayana Murty N
  2026-08-17 10:27 ` [RFC PATCH 2/8] hw/loader: add load_elf_ram_sym_buf() for in-memory ELF loading uverma
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 25+ messages in thread
From: uverma @ 2026-08-17 10:27 UTC (permalink / raw)
  To: qemu-devel, qemu-ppc, aik
  Cc: pbonzini, th.huth, nnmlinux, sbhat, harshpb, amachhiw, rathc,
	balaton, philmd, npiggin, marcandre.lureau, fam, Utkarsh Verma

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

Add spapr_vof_partition.c which implements basic MBR and GPT partition
table scanning to locate the PReP boot partition during VOF enabled boot.

AI-used-for: code
Signed-off-by: Utkarsh Verma <uverma@linux.ibm.com>
---
 hw/ppc/meson.build           |   5 +-
 hw/ppc/spapr_vof_partition.c | 176 +++++++++++++++++++++++++++++++++++
 include/hw/ppc/spapr_vof.h   |  14 +++
 3 files changed, 194 insertions(+), 1 deletion(-)
 create mode 100644 hw/ppc/spapr_vof_partition.c
 create mode 100644 include/hw/ppc/spapr_vof.h

diff --git a/hw/ppc/meson.build b/hw/ppc/meson.build
index 37aa535db2..68a5d2622f 100644
--- a/hw/ppc/meson.build
+++ b/hw/ppc/meson.build
@@ -93,6 +93,9 @@ ppc_ss.add(when: 'CONFIG_AMIGAONE', if_true: files('amigaone.c'))
 ppc_ss.add(when: 'CONFIG_PEGASOS', if_true: files('pegasos.c'))
 
 ppc_ss.add(when: 'CONFIG_VOF', if_true: files('vof.c'))
-ppc_ss.add(when: ['CONFIG_VOF', 'CONFIG_PSERIES'], if_true: files('spapr_vof.c'))
+ppc_ss.add(when: ['CONFIG_VOF', 'CONFIG_PSERIES'], if_true: files(
+  'spapr_vof.c',
+  'spapr_vof_partition.c',
+))
 
 hw_arch += {'ppc': ppc_ss}
diff --git a/hw/ppc/spapr_vof_partition.c b/hw/ppc/spapr_vof_partition.c
new file mode 100644
index 0000000000..ced2818998
--- /dev/null
+++ b/hw/ppc/spapr_vof_partition.c
@@ -0,0 +1,176 @@
+/*
+ * QEMU PowerPC sPAPR VOF Partition Table Support.
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ *
+ * This implements partition table detection for VOF boot,
+ * supporting MBR and GPT partition tables with focus on PReP boot partition.
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/bswap.h"
+#include "qemu/error-report.h"
+#include "system/block-backend-io.h"
+#include "hw/ppc/spapr_vof.h"
+
+#define SECTOR_SIZE              512
+#define MBR_PARTITION_ENTRY_SIZE 16
+#define MBR_PARTITION_TABLE_OFFSET 446
+#define MBR_NUM_PARTITIONS       4
+
+#define PARTITION_TYPE_PREP      0x41  /* PReP Boot partition */
+#define PARTITION_TYPE_GPT       0xEE
+
+#define GPT_SIGNATURE            "EFI PART"
+#define GPT_SIGNATURE_SIZE       8
+
+/* PReP Boot: 9E1A2D38-C612-4316-AA26-8B49521E5A8B */
+static const uint8_t GUID_PREP_BOOT[16] = {
+    0x38, 0x2D, 0x1A, 0x9E, 0x12, 0xC6, 0x16, 0x43,
+    0xAA, 0x26, 0x8B, 0x49, 0x52, 0x1E, 0x5A, 0x8B
+};
+
+typedef struct MBRPartitionEntry {
+    uint8_t  boot_flag;
+    uint8_t  start_chs[3];
+    uint8_t  type;
+    uint8_t  end_chs[3];
+    uint32_t start_lba;
+    uint32_t num_sectors;
+} QEMU_PACKED MBRPartitionEntry;
+
+typedef struct GPTHeader {
+    char     signature[8];
+    uint32_t revision;
+    uint32_t header_size;
+    uint32_t header_crc32;
+    uint32_t reserved;
+    uint64_t current_lba;
+    uint64_t backup_lba;
+    uint64_t first_usable_lba;
+    uint64_t last_usable_lba;
+    uint8_t  disk_guid[16];
+    uint64_t partition_entries_lba;
+    uint32_t num_partition_entries;
+    uint32_t partition_entry_size;
+    uint32_t partition_array_crc32;
+} QEMU_PACKED GPTHeader;
+
+typedef struct GPTPartitionEntry {
+    uint8_t  type_guid[16];
+    uint8_t  partition_guid[16];
+    uint64_t first_lba;
+    uint64_t last_lba;
+    uint64_t attributes;
+    uint16_t name[36];
+} QEMU_PACKED GPTPartitionEntry;
+
+static bool find_prep_partition_gpt(BlockBackend *blk,
+                                    uint64_t *offset, uint64_t *size)
+{
+    uint8_t lba1[SECTOR_SIZE];
+    GPTHeader *header;
+    uint64_t entries_lba;
+    uint32_t num_entries;
+    uint32_t entry_size;
+    uint64_t current_entry_offset;
+    uint8_t *buf;
+    GPTPartitionEntry *current_entry;
+    uint32_t i;
+    int ret;
+
+    ret = blk_pread(blk, SECTOR_SIZE, SECTOR_SIZE, lba1, 0);
+    if (ret < 0) {
+        warn_report("GPT: failed to read LBA1 (ret=%d)", ret);
+        return false;
+    }
+
+    header = (GPTHeader *)lba1;
+
+    if (memcmp(header->signature, GPT_SIGNATURE, GPT_SIGNATURE_SIZE) != 0) {
+        return false;
+    }
+
+    entries_lba = le64_to_cpu(header->partition_entries_lba);
+    num_entries = le32_to_cpu(header->num_partition_entries);
+    entry_size = le32_to_cpu(header->partition_entry_size);
+
+    if (num_entries > 128) {
+        num_entries = 128;
+    }
+
+    if (entry_size < 128) {
+        return false;
+    }
+
+    buf = g_malloc(entry_size);
+    for (i = 0; i < num_entries; i++) {
+        current_entry_offset = (entries_lba * SECTOR_SIZE) +
+                               ((uint64_t)i * entry_size);
+
+        ret = blk_pread(blk, current_entry_offset, entry_size, buf, 0);
+        if (ret < 0) {
+            warn_report("GPT: failed to read partition entry %u "
+                        "(ret=%d)", i, ret);
+            g_free(buf);
+            return false;
+        }
+
+        current_entry = (GPTPartitionEntry *)buf;
+        if (memcmp(current_entry->type_guid, GUID_PREP_BOOT,
+                   sizeof(GUID_PREP_BOOT)) == 0) {
+            *offset = le64_to_cpu(current_entry->first_lba) * SECTOR_SIZE;
+            *size = (le64_to_cpu(current_entry->last_lba) -
+                     le64_to_cpu(current_entry->first_lba) + 1) * SECTOR_SIZE;
+            g_free(buf);
+            return true;
+        }
+    }
+    g_free(buf);
+    return false;
+}
+
+static bool find_prep_partition_mbr(BlockBackend *blk,
+                                    uint64_t *offset, uint64_t *size)
+{
+    uint8_t mbr[SECTOR_SIZE];
+    MBRPartitionEntry *entry;
+    int ret;
+    int i;
+
+    ret = blk_pread(blk, 0, SECTOR_SIZE, mbr, 0);
+    if (ret < 0) {
+        warn_report("MBR: failed to read MBR sector (ret=%d)", ret);
+        return false;
+    }
+
+    /* MBR boot signature: byte 510 = 0x55, byte 511 = 0xAA */
+    if (mbr[510] != 0x55 || mbr[511] != 0xAA) {
+        return false;
+    }
+
+    for (i = 0; i < MBR_NUM_PARTITIONS; i++) {
+        entry = (MBRPartitionEntry *)&mbr[MBR_PARTITION_TABLE_OFFSET +
+                                          i * MBR_PARTITION_ENTRY_SIZE];
+
+        if (entry->type == PARTITION_TYPE_GPT) {
+            return find_prep_partition_gpt(blk, offset, size);
+        }
+
+        if (entry->type == PARTITION_TYPE_PREP) {
+            *offset = (uint64_t)le32_to_cpu(entry->start_lba) * SECTOR_SIZE;
+            *size = (uint64_t)le32_to_cpu(entry->num_sectors) * SECTOR_SIZE;
+            return true;
+        }
+    }
+    return false;
+}
+
+bool spapr_vof_find_prep_partition(BlockBackend *blk,
+                                   uint64_t *offset, uint64_t *size)
+{
+    if (!blk) {
+        return false;
+    }
+    return find_prep_partition_mbr(blk, offset, size);
+}
diff --git a/include/hw/ppc/spapr_vof.h b/include/hw/ppc/spapr_vof.h
new file mode 100644
index 0000000000..08787d2aeb
--- /dev/null
+++ b/include/hw/ppc/spapr_vof.h
@@ -0,0 +1,14 @@
+/*
+ * QEMU PowerPC sPAPR VOF Support
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+#ifndef HW_SPAPR_VOF_H
+#define HW_SPAPR_VOF_H
+
+typedef struct BlockBackend BlockBackend;
+
+bool spapr_vof_find_prep_partition(BlockBackend *blk,
+                                   uint64_t *offset, uint64_t *size);
+
+#endif /* HW_SPAPR_VOF_H */
-- 
2.54.0



^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [RFC PATCH 2/8] hw/loader: add load_elf_ram_sym_buf() for in-memory ELF loading
  2026-08-17 10:27 [RFC PATCH 0/8] ppc/spapr: VOF disk image (qcow2) boot support uverma
  2026-08-17 10:27 ` [RFC PATCH 1/8] ppc/spapr: add PReP boot partition detection uverma
@ 2026-08-17 10:27 ` uverma
  2026-09-17 17:14   ` Narayana Murty N
  2026-08-17 10:27 ` [RFC PATCH 3/8] ppc/spapr: add baseline VOF disk boot support uverma
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 25+ messages in thread
From: uverma @ 2026-08-17 10:27 UTC (permalink / raw)
  To: qemu-devel, qemu-ppc, aik
  Cc: pbonzini, th.huth, nnmlinux, sbhat, harshpb, amachhiw, rathc,
	balaton, philmd, npiggin, marcandre.lureau, fam, Utkarsh Verma

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

Add a new public API load_elf_ram_sym_buf() that loads an ELF
image from a caller-supplied memory buffer rather than a file path.
The implementation writes the buffer to an anonymous memfd, then
reuses the existing load_elf32/load_elf64 paths.

To support per-segment inspection by callers (e.g. detecting VoF
firmware segments), extend the internal load_elf{32,64} template
(elf_ops.h.inc) with an optional segment_fn_t callback that is
invoked once for every PT_LOAD segment after its load address is
resolved. Returning false from the callback aborts the load with
ELF_LOAD_FAILED.

AI-used-for: code
Signed-off-by: Utkarsh Verma <uverma@linux.ibm.com>
---
 hw/core/loader.c         | 77 ++++++++++++++++++++++++++++++++++++++--
 include/hw/core/loader.h | 30 ++++++++++++++++
 include/hw/elf_ops.h.inc | 12 ++++++-
 3 files changed, 116 insertions(+), 3 deletions(-)

diff --git a/hw/core/loader.c b/hw/core/loader.c
index 5cbfba0a86..4e4e9764a3 100644
--- a/hw/core/loader.c
+++ b/hw/core/loader.c
@@ -43,6 +43,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "qemu/memfd.h"
 #include "qemu/datadir.h"
 #include "qemu/error-report.h"
 #include "qapi/error.h"
@@ -510,12 +511,14 @@ ssize_t load_elf_ram_sym(const char *filename,
         ret = load_elf64(filename, fd, elf_note_fn,
                          translate_fn, translate_opaque, must_swab,
                          pentry, lowaddr, highaddr, pflags, elf_machine,
-                         clear_lsb, data_swab, as, load_rom, sym_cb);
+                         clear_lsb, data_swab, as, load_rom, sym_cb,
+                         NULL, NULL);
     } else {
         ret = load_elf32(filename, fd, elf_note_fn,
                          translate_fn, translate_opaque, must_swab,
                          pentry, lowaddr, highaddr, pflags, elf_machine,
-                         clear_lsb, data_swab, as, load_rom, sym_cb);
+                         clear_lsb, data_swab, as, load_rom, sym_cb,
+                         NULL, NULL);
     }
 
     if (ret > 0) {
@@ -527,6 +530,76 @@ ssize_t load_elf_ram_sym(const char *filename,
     return ret;
 }
 
+ssize_t load_elf_ram_sym_buf(const uint8_t *buf, size_t buflen,
+                              uint64_t (*elf_note_fn)(void *, void *, bool),
+                              uint64_t (*translate_fn)(void *, uint64_t),
+                              void *translate_opaque, uint64_t *pentry,
+                              uint64_t *lowaddr, uint64_t *highaddr,
+                              uint32_t *pflags, int elf_data_order,
+                              int elf_machine, int clear_lsb, int data_swab,
+                              AddressSpace *as, bool load_rom,
+                              symbol_fn_t sym_cb,
+                              segment_fn_t segment_fn, void *segment_opaque)
+{
+    const int host_data_order = HOST_BIG_ENDIAN ? ELFDATA2MSB : ELFDATA2LSB;
+    int fd, must_swab;
+    ssize_t ret = ELF_LOAD_FAILED;
+    uint8_t e_ident[EI_NIDENT];
+
+    fd = memfd_create("qemu-elf-buf", MFD_CLOEXEC);
+    if (fd < 0) {
+        error_report("load_elf_ram_sym_buf: memfd_create: %s", strerror(errno));
+        return ELF_LOAD_FAILED;
+    }
+
+    if (write(fd, buf, buflen) != (ssize_t)buflen) {
+        error_report("load_elf_ram_sym_buf: write: %s", strerror(errno));
+        goto fail;
+    }
+
+    lseek(fd, 0, SEEK_SET);
+    if (read(fd, e_ident, sizeof(e_ident)) != sizeof(e_ident)) {
+        goto fail;
+    }
+    if (e_ident[0] != ELFMAG0 ||
+        e_ident[1] != ELFMAG1 ||
+        e_ident[2] != ELFMAG2 ||
+        e_ident[3] != ELFMAG3) {
+        ret = ELF_LOAD_NOT_ELF;
+        goto fail;
+    }
+
+    if (elf_data_order != ELFDATANONE && elf_data_order != e_ident[EI_DATA]) {
+        ret = ELF_LOAD_WRONG_ENDIAN;
+        goto fail;
+    }
+
+    must_swab = host_data_order != e_ident[EI_DATA];
+
+    lseek(fd, 0, SEEK_SET);
+    if (e_ident[EI_CLASS] == ELFCLASS64) {
+        ret = load_elf64("(buffer)", fd, elf_note_fn,
+                         translate_fn, translate_opaque, must_swab,
+                         pentry, lowaddr, highaddr, pflags, elf_machine,
+                         clear_lsb, data_swab, as, load_rom, sym_cb,
+                         segment_fn, segment_opaque);
+    } else {
+        ret = load_elf32("(buffer)", fd, elf_note_fn,
+                         translate_fn, translate_opaque, must_swab,
+                         pentry, lowaddr, highaddr, pflags, elf_machine,
+                         clear_lsb, data_swab, as, load_rom, sym_cb,
+                         segment_fn, segment_opaque);
+    }
+
+    if (ret > 0) {
+        debuginfo_report_elf("(buffer)", fd, 0);
+    }
+
+ fail:
+    close(fd);
+    return ret;
+}
+
 static void bswap_uboot_header(uboot_image_header_t *hdr)
 {
 #if !HOST_BIG_ENDIAN
diff --git a/include/hw/core/loader.h b/include/hw/core/loader.h
index d9431e8a8d..b67b2ca013 100644
--- a/include/hw/core/loader.h
+++ b/include/hw/core/loader.h
@@ -156,6 +156,36 @@ ssize_t load_elf_ram_sym(const char *filename,
                          int clear_lsb, int data_swab,
                          AddressSpace *as, bool load_rom, symbol_fn_t sym_cb);
 
+/*
+ * segment_fn_t:
+ * Per-PT_LOAD segment callback for load_elf_ram_sym_buf().
+ */
+typedef bool (*segment_fn_t)(void *opaque,
+                             uint64_t paddr, uint64_t vaddr,
+                             uint64_t filesz, uint64_t memsz);
+
+/*
+ * load_elf_ram_sym_buf:
+ * @buf: pointer to an in-memory ELF image
+ * @buflen: size of @buf in bytes
+ * @segment_fn: optional per-PT_LOAD callback
+ * @segment_opaque: opaque data passed to @segment_fn
+ *
+ * Identical to load_elf_ram_sym() but loads from a caller-supplied
+ * memory buffer instead of a file.  All other parameters have the
+ * same meaning as load_elf_ram_sym().
+ */
+ssize_t load_elf_ram_sym_buf(const uint8_t *buf, size_t buflen,
+                             uint64_t (*elf_note_fn)(void *, void *, bool),
+                             uint64_t (*translate_fn)(void *, uint64_t),
+                             void *translate_opaque, uint64_t *pentry,
+                             uint64_t *lowaddr, uint64_t *highaddr,
+                             uint32_t *pflags, int elf_data_order,
+                             int elf_machine, int clear_lsb, int data_swab,
+                             AddressSpace *as, bool load_rom,
+                             symbol_fn_t sym_cb,
+                             segment_fn_t segment_fn, void *segment_opaque);
+
 /** load_elf_as:
  * Same as load_elf_ram_sym(), but always loads the elf as ROM
  */
diff --git a/include/hw/elf_ops.h.inc b/include/hw/elf_ops.h.inc
index 044e72de2a..a2c9b52bf9 100644
--- a/include/hw/elf_ops.h.inc
+++ b/include/hw/elf_ops.h.inc
@@ -321,7 +321,9 @@ static ssize_t glue(load_elf, SZ)(const char *name, int fd,
                                   uint32_t *pflags, int elf_machine,
                                   int clear_lsb, int data_swab,
                                   AddressSpace *as, bool load_rom,
-                                  symbol_fn_t sym_cb)
+                                  symbol_fn_t sym_cb,
+                                  segment_fn_t segment_fn,
+                                  void *segment_opaque)
 {
     struct elfhdr ehdr;
     struct elf_phdr *phdr = NULL, *ph;
@@ -504,6 +506,14 @@ static ssize_t glue(load_elf, SZ)(const char *name, int fd,
                 addr = ph->p_paddr;
             }
 
+            if (segment_fn) {
+                if (!segment_fn(segment_opaque, addr, ph->p_vaddr,
+                                file_size, mem_size)) {
+                    ret = ELF_LOAD_FAILED;
+                    goto fail;
+                }
+            }
+
             if (data_swab) {
                 elf_word j;
                 for (j = 0; j < file_size; j += (1 << data_swab)) {
-- 
2.54.0



^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [RFC PATCH 3/8] ppc/spapr: add baseline VOF disk boot support
  2026-08-17 10:27 [RFC PATCH 0/8] ppc/spapr: VOF disk image (qcow2) boot support uverma
  2026-08-17 10:27 ` [RFC PATCH 1/8] ppc/spapr: add PReP boot partition detection uverma
  2026-08-17 10:27 ` [RFC PATCH 2/8] hw/loader: add load_elf_ram_sym_buf() for in-memory ELF loading uverma
@ 2026-08-17 10:27 ` uverma
  2026-09-17 17:33   ` Narayana Murty N
  2026-08-17 10:27 ` [RFC PATCH 4/8] ppc/spapr: add VTY backend support to OF read/write/open services uverma
                   ` (5 subsequent siblings)
  8 siblings, 1 reply; 25+ messages in thread
From: uverma @ 2026-08-17 10:27 UTC (permalink / raw)
  To: qemu-devel, qemu-ppc, aik
  Cc: pbonzini, th.huth, nnmlinux, sbhat, harshpb, amachhiw, rathc,
	balaton, philmd, npiggin, marcandre.lureau, fam, Utkarsh Verma

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);
+        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;
+    }
+    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;
 
-- 
2.54.0



^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [RFC PATCH 4/8] ppc/spapr: add VTY backend support to OF read/write/open services
  2026-08-17 10:27 [RFC PATCH 0/8] ppc/spapr: VOF disk image (qcow2) boot support uverma
                   ` (2 preceding siblings ...)
  2026-08-17 10:27 ` [RFC PATCH 3/8] ppc/spapr: add baseline VOF disk boot support uverma
@ 2026-08-17 10:27 ` uverma
  2026-08-17 10:27 ` [RFC PATCH 5/8] ppc/spapr: add block device backend to VOF open/read/write/seek services uverma
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 25+ messages in thread
From: uverma @ 2026-08-17 10:27 UTC (permalink / raw)
  To: qemu-devel, qemu-ppc, aik
  Cc: pbonzini, th.huth, nnmlinux, sbhat, harshpb, amachhiw, rathc,
	balaton, philmd, npiggin, marcandre.lureau, fam, Utkarsh Verma

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

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

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

Route the OF read and write services backed by vty_getchars() and vty_putchars()

Make vty_getchars() public similar to vty_putchars().

AI-used-for: code
Signed-off-by: Utkarsh Verma <uverma@linux.ibm.com>
---
 hw/char/spapr_vty.c        |  2 +-
 hw/ppc/spapr_vof.c         |  2 +
 hw/ppc/trace-events        |  2 +
 hw/ppc/vof.c               | 84 ++++++++++++++++++++++++++++++++++++++
 include/hw/ppc/spapr_vio.h |  1 +
 5 files changed, 90 insertions(+), 1 deletion(-)

diff --git a/hw/char/spapr_vty.c b/hw/char/spapr_vty.c
index 1dd9fb155c..2c97e0e027 100644
--- a/hw/char/spapr_vty.c
+++ b/hw/char/spapr_vty.c
@@ -52,7 +52,7 @@ static void vty_receive(void *opaque, const uint8_t *buf, int size)
     }
 }
 
-static int vty_getchars(SpaprVioDevice *sdev, uint8_t *buf, int max)
+int vty_getchars(SpaprVioDevice *sdev, uint8_t *buf, int max)
 {
     SpaprVioVty *dev = VIO_SPAPR_VTY_DEVICE(sdev);
     int n = 0;
diff --git a/hw/ppc/spapr_vof.c b/hw/ppc/spapr_vof.c
index 5bf9613005..a08d45c6a5 100644
--- a/hw/ppc/spapr_vof.c
+++ b/hw/ppc/spapr_vof.c
@@ -74,6 +74,8 @@ void spapr_vof_client_dt_finalize(SpaprMachineState *spapr, void *fdt)
     if (stdout_path) {
         _FDT(vof_client_open_store(fdt, spapr->vof, "/chosen", "stdout",
                                    stdout_path));
+        _FDT(vof_client_open_store(fdt, spapr->vof, "/chosen", "stdin",
+                                   stdout_path));
     }
 }
 
diff --git a/hw/ppc/trace-events b/hw/ppc/trace-events
index 1f125ce841..dbdcfada26 100644
--- a/hw/ppc/trace-events
+++ b/hw/ppc/trace-events
@@ -79,6 +79,7 @@ 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_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"
 vof_claim(uint32_t virt, uint32_t size, uint32_t align, uint32_t ret) "virt=0x%x size=0x%x align=0x%x => 0x%x"
 vof_release(uint32_t virt, uint32_t size, uint32_t ret) "virt=0x%x size=0x%x => 0x%x"
@@ -92,6 +93,7 @@ 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_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
 vof_claimed(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 a78bb1f116..45c197df9f 100644
--- a/hw/ppc/vof.c
+++ b/hw/ppc/vof.c
@@ -9,6 +9,7 @@
  * SPDX-License-Identifier: GPL-2.0-or-later
  */
 
+#include CONFIG_DEVICES /* CONFIG_PSERIES */
 #include "qemu/osdep.h"
 #include "qemu/timer.h"
 #include "qemu/range.h"
@@ -22,6 +23,7 @@
 #include "qom/qom-qobject.h"
 #include "trace.h"
 
+#include "hw/ppc/spapr_vio.h"
 #include <libfdt.h>
 
 /*
@@ -44,6 +46,7 @@ typedef struct {
 typedef struct {
     char *path; /* the path used to open the instance */
     uint32_t phandle;
+    void *vty;
 } OfInstance;
 
 static int readstr(hwaddr pa, char *buf, int size)
@@ -458,6 +461,28 @@ 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->vty = NULL;
+
+#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) {
+            inst->vty = spapr_vty_get_default(spapr->vio_bus);
+            if (inst->vty) {
+                /* Flush any stale data from the VTY input buffer */
+                while (vty_getchars(inst->vty, discard_buf,
+                                    sizeof(discard_buf)) > 0) {
+                    /* discard */
+                }
+            }
+        }
+    }
+#endif
+
     g_hash_table_insert(vof->of_instances,
                         GINT_TO_POINTER(vof->of_instance_last),
                         inst);
@@ -576,6 +601,23 @@ static uint32_t vof_write(Vof *vof, uint32_t ihandle, uint32_t buf,
         return PROM_ERROR;
     }
 
+#ifdef CONFIG_PSERIES
+    if (inst->vty) {
+        uint32_t total_written = 0;
+
+        for ( ; len > 0; len -= cb) {
+            cb = MIN(len, sizeof(tmp));
+            if (VOF_MEM_READ(buf, tmp, cb) != MEMTX_OK) {
+                return PROM_ERROR;
+            }
+            vty_putchars(inst->vty, (uint8_t *)tmp, cb);
+            buf += cb;
+            total_written += cb;
+        }
+        return total_written;
+    }
+#endif
+
     for ( ; len > 0; len -= cb) {
         cb = MIN(len, sizeof(tmp) - 1);
         if (VOF_MEM_READ(buf, tmp, cb) != MEMTX_OK) {
@@ -593,6 +635,46 @@ static uint32_t vof_write(Vof *vof, uint32_t ihandle, uint32_t buf,
     return len;
 }
 
+static uint32_t vof_read(Vof *vof, uint32_t ihandle, uint32_t buf,
+                         uint32_t len)
+{
+    OfInstance *inst = (OfInstance *)
+        g_hash_table_lookup(vof->of_instances, GINT_TO_POINTER(ihandle));
+
+    if (!inst) {
+        trace_vof_error_read(ihandle);
+        return PROM_ERROR;
+    }
+
+#ifdef CONFIG_PSERIES
+    if (inst->vty) {
+        uint8_t tmp[VOF_VTY_BUF_SIZE];
+        unsigned cb = MIN(len, sizeof(tmp));
+        uint32_t bytes_read = vty_getchars(inst->vty, tmp, cb);
+        if (bytes_read > 0) {
+            if (VOF_MEM_WRITE(buf, tmp, bytes_read) != MEMTX_OK) {
+                trace_vof_error_read(ihandle);
+                return PROM_ERROR;
+            }
+        }
+        if (trace_event_get_state(TRACE_VOF_READ) &&
+            qemu_loglevel_mask(LOG_TRACE)) {
+            char trace_buf[VOF_VTY_BUF_SIZE + 1];
+            memcpy(trace_buf, tmp, bytes_read);
+            trace_buf[bytes_read] = '\0';
+            trace_vof_read(ihandle, bytes_read, trace_buf);
+        }
+        return bytes_read;
+    }
+#endif
+
+    /*
+     * For other devices, return 0 to indicate no data available.
+     * This allows GRUB to continue without blocking on input.
+     */
+    return 0;
+}
+
 static void vof_claimed_dump(GArray *claimed)
 {
     int i;
@@ -905,6 +987,8 @@ static uint32_t vof_client_handle(MachineState *ms, void *fdt, Vof *vof,
         ret = vof_instance_to_path(fdt, 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)) {
+        ret = vof_read(vof, args[0], args[1], args[2]);
     } else if (cmpserv("claim", 3, 1)) {
         uint64_t ret64 = vof_claim(vof, args[0], args[1], args[2]);
 
diff --git a/include/hw/ppc/spapr_vio.h b/include/hw/ppc/spapr_vio.h
index 0ea0dbae8b..81e7c0b91b 100644
--- a/include/hw/ppc/spapr_vio.h
+++ b/include/hw/ppc/spapr_vio.h
@@ -136,6 +136,7 @@ static inline int spapr_vio_dma_set(SpaprVioDevice *dev, uint64_t taddr,
 int spapr_vio_send_crq(SpaprVioDevice *dev, uint8_t *crq);
 
 SpaprVioDevice *vty_lookup(SpaprMachineState *spapr, target_ulong reg);
+int vty_getchars(SpaprVioDevice *sdev, uint8_t *buf, int max);
 void vty_putchars(SpaprVioDevice *sdev, uint8_t *buf, int len);
 void spapr_vty_create(SpaprVioBus *bus, Chardev *chardev);
 void spapr_vlan_create(SpaprVioBus *bus, NICInfo *nd);
-- 
2.54.0



^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [RFC PATCH 5/8] ppc/spapr: add block device backend to VOF open/read/write/seek services
  2026-08-17 10:27 [RFC PATCH 0/8] ppc/spapr: VOF disk image (qcow2) boot support uverma
                   ` (3 preceding siblings ...)
  2026-08-17 10:27 ` [RFC PATCH 4/8] ppc/spapr: add VTY backend support to OF read/write/open services uverma
@ 2026-08-17 10:27 ` uverma
  2026-09-18  5:23   ` Narayana Murty N
  2026-08-17 10:27 ` [RFC PATCH 6/8] spapr_vscsi: add VOF disk nodes to the device tree uverma
                   ` (3 subsequent siblings)
  8 siblings, 1 reply; 25+ messages in thread
From: uverma @ 2026-08-17 10:27 UTC (permalink / raw)
  To: qemu-devel, qemu-ppc, aik
  Cc: pbonzini, th.huth, nnmlinux, sbhat, harshpb, amachhiw, rathc,
	balaton, philmd, npiggin, marcandre.lureau, fam, Utkarsh Verma

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 FDT nodes named "disk@<srp-lun>" and resolve
the SRP LUN encoding to the matching SCSIDevice/BlockBackend by scanning
id/channel/lun.

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>
---
 hw/ppc/trace-events |   2 +
 hw/ppc/vof.c        | 116 +++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 117 insertions(+), 1 deletion(-)

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 45c197df9f..b1c574aabf 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/timer.h"
 #include "qemu/range.h"
 #include "qemu/units.h"
@@ -24,6 +25,8 @@
 #include "trace.h"
 
 #include "hw/ppc/spapr_vio.h"
+#include "hw/scsi/scsi.h"
+#include "system/block-backend.h"
 #include <libfdt.h>
 
 /*
@@ -46,6 +49,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;
 
@@ -449,6 +454,7 @@ 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;
 
     if (vof->of_instance_last == 0xFFFFFFFF) {
         /* We do not recycle ihandles yet */
@@ -461,10 +467,42 @@ 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);
+
+    if (node_name && strncmp(node_name, "disk@", 5) == 0) {
+        uint64_t srp_lun;
+        uint32_t id, channel, lun;
+        BlockBackend *blk;
+
+        if (qemu_strtou64(node_name + 5, NULL, 16, &srp_lun) == 0) {
+            id      = (srp_lun >> 56) & 0x3f;
+            channel = (srp_lun >> 53) & 0x7;
+            lun     = (srp_lun >> 48) & 0x1f;
+
+            for (blk = blk_next(NULL); blk; blk = blk_next(blk)) {
+                DeviceState *attached = blk_get_attached_dev(blk);
+                SCSIDevice *sdev;
+
+                if (!attached) {
+                    continue;
+                }
+                sdev = (SCSIDevice *)object_dynamic_cast(OBJECT(attached),
+                                                         TYPE_SCSI_DEVICE);
+                if (sdev && sdev->id == (int)id &&
+                    sdev->channel == (int)channel &&
+                    sdev->lun == (int)lun) {
+                    inst->blk = blk;
+                    break;
+                }
+            }
+        }
+    }
+
 #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());
@@ -601,6 +639,25 @@ static uint32_t vof_write(Vof *vof, uint32_t ihandle, uint32_t buf,
         return PROM_ERROR;
     }
 
+    if (inst->blk) {
+        g_autofree uint8_t *blkbuf = g_malloc(len);
+        int ret;
+
+        if (VOF_MEM_READ(buf, blkbuf, len) != MEMTX_OK) {
+            trace_vof_error_write(ihandle);
+            return PROM_ERROR;
+        }
+        ret = blk_pwrite(inst->blk, inst->pos, len, blkbuf, 0);
+        if (ret < 0) {
+            trace_vof_error_write(ihandle);
+            return PROM_ERROR;
+        }
+        blk_flush(inst->blk);
+        inst->pos += len;
+        trace_vof_write(ihandle, len, "(disk)");
+        return len;
+    }
+
 #ifdef CONFIG_PSERIES
     if (inst->vty) {
         uint32_t total_written = 0;
@@ -646,6 +703,26 @@ 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(len);
+        int ret;
+
+        ret = blk_pread(inst->blk, inst->pos, len, tmp, 0);
+        if (ret < 0) {
+            trace_vof_error_read(ihandle);
+            return PROM_ERROR;
+        }
+
+        if (VOF_MEM_WRITE(buf, tmp, len) != MEMTX_OK) {
+            trace_vof_error_read(ihandle);
+            return PROM_ERROR;
+        }
+
+        inst->pos += len;
+        trace_vof_read(ihandle, len, "(disk)");
+        return len;
+    }
+
 #ifdef CONFIG_PSERIES
     if (inst->vty) {
         uint8_t tmp[VOF_VTY_BUF_SIZE];
@@ -675,6 +752,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;
@@ -985,6 +1097,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)) {
-- 
2.54.0



^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [RFC PATCH 6/8] spapr_vscsi: add VOF disk nodes to the device tree
  2026-08-17 10:27 [RFC PATCH 0/8] ppc/spapr: VOF disk image (qcow2) boot support uverma
                   ` (4 preceding siblings ...)
  2026-08-17 10:27 ` [RFC PATCH 5/8] ppc/spapr: add block device backend to VOF open/read/write/seek services uverma
@ 2026-08-17 10:27 ` uverma
  2026-09-18  5:24   ` Narayana Murty N
  2026-08-17 10:27 ` [RFC PATCH 7/8] ppc/spapr: strip OF path argument suffix in path_offset uverma
                   ` (2 subsequent siblings)
  8 siblings, 1 reply; 25+ messages in thread
From: uverma @ 2026-08-17 10:27 UTC (permalink / raw)
  To: qemu-devel, qemu-ppc, aik
  Cc: pbonzini, th.huth, nnmlinux, sbhat, harshpb, amachhiw, rathc,
	balaton, philmd, npiggin, marcandre.lureau, fam, Utkarsh Verma

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

Populate the vSCSI device tree node with child disk nodes for VOF
guests.

This lets Open Firmware clients such as GRUB open fully-qualified
paths like /vdevice/v-scsi@.../disk@<srp-lun> for attached vSCSI
disks.

Each child node is named from the encoded SRP LUN and includes a reg
property with the 64-bit LUN value, along with device_type set to
"block".

AI-used-for: code
Signed-off-by: Utkarsh Verma <uverma@linux.ibm.com>
---
 hw/scsi/spapr_vscsi.c | 40 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/hw/scsi/spapr_vscsi.c b/hw/scsi/spapr_vscsi.c
index b4c8f94d22..03f5f27f5f 100644
--- a/hw/scsi/spapr_vscsi.c
+++ b/hw/scsi/spapr_vscsi.c
@@ -1243,6 +1243,8 @@ void spapr_vscsi_create(SpaprVioBus *bus)
 
 static int spapr_vscsi_devnode(SpaprVioDevice *dev, void *fdt, int node_off)
 {
+    VSCSIState *s = VIO_SPAPR_VSCSI_DEVICE(dev);
+    BusChild *kid;
     int ret;
 
     ret = fdt_setprop_cell(fdt, node_off, "#address-cells", 2);
@@ -1255,6 +1257,44 @@ static int spapr_vscsi_devnode(SpaprVioDevice *dev, void *fdt, int node_off)
         return ret;
     }
 
+    /*
+     * In VOF mode, add a child FDT node for each attached SCSI disk so that OF
+     * clients (e.g. GRUB via VOF) can open a fully-qualified path like
+     * /vdevice/v-scsi@.../disk@<srp-lun>.
+     */
+    SpaprMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
+    if (spapr->vof) {
+        QTAILQ_FOREACH(kid, &s->bus.qbus.children, sibling) {
+            SCSIDevice *sdev = SCSI_DEVICE(kid->child);
+            char disk_name[32];
+            uint64_t srp_lun;
+            uint32_t reg[2];
+            int disk_off;
+
+            srp_lun = ((uint64_t)(0x8000 | (sdev->id << 8) |
+                                   (sdev->channel << 5) | sdev->lun)) << 48;
+
+            snprintf(disk_name, sizeof(disk_name), "disk@%"PRIx64, srp_lun);
+
+            disk_off = fdt_add_subnode(fdt, node_off, disk_name);
+            if (disk_off < 0) {
+                return disk_off;
+            }
+
+            reg[0] = cpu_to_be32((uint32_t)(srp_lun >> 32));
+            reg[1] = cpu_to_be32((uint32_t)(srp_lun & 0xFFFFFFFF));
+            ret = fdt_setprop(fdt, disk_off, "reg", reg, sizeof(reg));
+            if (ret < 0) {
+                return ret;
+            }
+
+            ret = fdt_setprop_string(fdt, disk_off, "device_type", "block");
+            if (ret < 0) {
+                return ret;
+            }
+        }
+    }
+
     return 0;
 }
 
-- 
2.54.0



^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [RFC PATCH 7/8] ppc/spapr: strip OF path argument suffix in path_offset
  2026-08-17 10:27 [RFC PATCH 0/8] ppc/spapr: VOF disk image (qcow2) boot support uverma
                   ` (5 preceding siblings ...)
  2026-08-17 10:27 ` [RFC PATCH 6/8] spapr_vscsi: add VOF disk nodes to the device tree uverma
@ 2026-08-17 10:27 ` uverma
  2026-09-18  5:25   ` Narayana Murty N
  2026-08-17 10:27 ` [RFC PATCH 8/8] ppc/spapr: implement vscsi-report-luns call-method for PAPR vSCSI uverma
  2026-08-19  6:55 ` [RFC PATCH 0/8] ppc/spapr: VOF disk image (qcow2) boot support Utkarsh Verma
  8 siblings, 1 reply; 25+ messages in thread
From: uverma @ 2026-08-17 10:27 UTC (permalink / raw)
  To: qemu-devel, qemu-ppc, aik
  Cc: pbonzini, th.huth, nnmlinux, sbhat, harshpb, amachhiw, rathc,
	balaton, philmd, npiggin, marcandre.lureau, fam, Utkarsh Verma

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=y, Size: 1550 bytes --]

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

OF paths can include a ":argument" suffix on the last component (e.g.
"disk@8000000000000000:0"), which is an OF path argument such as a
partition number.  This is not part of the FDT node name and causes
fdt_path_offset() to fail.

Strip any ":<suffix>" from the last path component only, leaving
intermediate components untouched.

AI-used-for: code
Signed-off-by: Utkarsh Verma <uverma@linux.ibm.com>
---
 hw/ppc/vof.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/hw/ppc/vof.c b/hw/ppc/vof.c
index b1c574aabf..d3b0df30e4 100644
--- a/hw/ppc/vof.c
+++ b/hw/ppc/vof.c
@@ -142,6 +142,8 @@ static int path_offset(const void *fdt, const char *path)
 {
     g_autofree char *p = NULL;
     char *at;
+    char *last_slash;
+    char *colon;
 
     /*
      * https://www.devicetree.org/open-firmware/bindings/ppc/release/ppc-2_1.html#HDR16
@@ -151,6 +153,20 @@ static int path_offset(const void *fdt, const char *path)
      * suppressing leading zeros".
      */
     p = g_strdup(path);
+
+    /*
+     * Strip any ":argument" suffix from the last path component (e.g.
+     * "disk@8000000000000000:0") — it is an OF path argument, not part of
+     * the FDT node name.
+     */
+    last_slash = strrchr(p, '/');
+    if (last_slash) {
+        colon = strchr(last_slash, ':');
+        if (colon) {
+            *colon = '\0';
+        }
+    }
+
     for (at = strchr(p, '@'); at && *at; ) {
             if (*at == '/') {
                 at = strchr(at, '@');
-- 
2.54.0



^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [RFC PATCH 8/8] ppc/spapr: implement vscsi-report-luns call-method for PAPR vSCSI
  2026-08-17 10:27 [RFC PATCH 0/8] ppc/spapr: VOF disk image (qcow2) boot support uverma
                   ` (6 preceding siblings ...)
  2026-08-17 10:27 ` [RFC PATCH 7/8] ppc/spapr: strip OF path argument suffix in path_offset uverma
@ 2026-08-17 10:27 ` uverma
  2026-09-18  5:18   ` Narayana Murty N
  2026-09-18  5:26   ` Narayana Murty N
  2026-08-19  6:55 ` [RFC PATCH 0/8] ppc/spapr: VOF disk image (qcow2) boot support Utkarsh Verma
  8 siblings, 2 replies; 25+ messages in thread
From: uverma @ 2026-08-17 10:27 UTC (permalink / raw)
  To: qemu-devel, qemu-ppc, aik
  Cc: pbonzini, th.huth, nnmlinux, sbhat, harshpb, amachhiw, rathc,
	balaton, philmd, npiggin, marcandre.lureau, fam, Utkarsh Verma

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

GRUB (grub-core/disk/ieee1275/ofdisk.c) calls "vscsi-report-luns" on
any PAPR virtual SCSI controller ihandle to enumerate attached LUNs
before attempting to boot from disk. Without this method VOF returns
PROM_ERROR and GRUB cannot find vSCSI disks.

Add vof_find_vscsi_bus() to locate the SCSIBus from a v-scsi@<reg>
instance path, and vof_vscsi_report_luns() to build the response table
in the VOF firmware region (0..fw_size), which GRUB never claims.

The table layout and SRP LUN encoding follow SLOF (vio-vscsi.fs
dev-generate-srplun) each entry is an 8-byte big-endian cell whose
low 4 bytes hold a guest pointer to a null-terminated list of SRP LUNs
for that target; GRUB reads the pointer at offset table + 4 + 8*i.

AI-used-for: code
Signed-off-by: Utkarsh Verma <uverma@linux.ibm.com>
---
 hw/ppc/vof.c | 212 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 212 insertions(+)

diff --git a/hw/ppc/vof.c b/hw/ppc/vof.c
index d3b0df30e4..b1bf5b1af0 100644
--- a/hw/ppc/vof.c
+++ b/hw/ppc/vof.c
@@ -991,6 +991,198 @@ static void vof_instantiate_rtas(Error **errp)
     error_setg(errp, "The firmware should have instantiated RTAS");
 }
 
+#ifdef CONFIG_PSERIES
+/* Combined (channel<<6)|id index space, matches SLOF dev-max-target. */
+#define VSCSI_MAX_TARGETS 512
+
+/*
+ * Build the vscsi-report-luns response in guest memory.
+ *
+ * Returns catch_result (0 = success).  On success, *nentries_out and
+ * *table_addr_out describe the result table written into the VOF firmware
+ * region (0..fw_size), which GRUB never claims.
+ *
+ * Table layout (big-endian, 8 bytes per entry):
+ *   [table_addr + 8*i + 0..3] : 0x00000000
+ *   [table_addr + 8*i + 4..7] : guest pointer to null-terminated SRP LUN list
+ *
+ * GRUB reads each pointer as: *(uint32_t *)(table + 4 + 8 * i)
+ *
+ * SRP LUN encoding (SLOF dev-generate-srplun):
+ *   srplun = (0x8000 | bus | target | lun) << 48
+ *   where bus    = (combined_target >> 1) & 0x70
+ *         target = (combined_target & 0x3f) << 8
+ *         combined_target = (channel << 6) | id
+ */
+static uint32_t vof_vscsi_report_luns(Vof *vof,
+                                       SCSIBus *sbus,
+                                       uint32_t *nentries_out,
+                                       uint32_t *table_addr_out)
+{
+    uint64_t *lun_lists[VSCSI_MAX_TARGETS];
+    int lun_counts[VSCSI_MAX_TARGETS];
+    uint32_t ptr_table_size;
+    uint32_t lun_data_size;
+    uint32_t total_size;
+    uint32_t base_addr;
+    uint32_t lun_data_base;
+    uint32_t lun_data_off;
+    uint32_t table_idx;
+    uint32_t table_addr;
+    BusChild *kid;
+    int nentries = 0;
+    int t;
+
+    memset(lun_lists, 0, sizeof(lun_lists));
+    memset(lun_counts, 0, sizeof(lun_counts));
+
+    QTAILQ_FOREACH(kid, &sbus->qbus.children, sibling) {
+        SCSIDevice *dev = SCSI_DEVICE(kid->child);
+        int combined_target;
+        uint64_t bus_field;
+        uint64_t target_field;
+        uint64_t lun_field;
+        uint64_t srplun;
+        int cnt;
+
+        /* combined_target = (channel << 6) | id, matching SLOF bus+target */
+        combined_target = ((dev->channel & 0x7) << 6) | (dev->id & 0x3f);
+        if (combined_target >= VSCSI_MAX_TARGETS) {
+            continue;
+        }
+
+        bus_field    = ((uint64_t)combined_target >> 1) & 0x70ULL;
+        target_field = ((uint64_t)combined_target & 0x3fULL) << 8;
+        lun_field    = (uint64_t)(dev->lun & 0x1f);
+        srplun = (0x8000ULL | bus_field | target_field | lun_field) << 48;
+
+        cnt = lun_counts[combined_target];
+        lun_lists[combined_target] = g_realloc(lun_lists[combined_target],
+                                               (cnt + 2) * sizeof(uint64_t));
+        lun_lists[combined_target][cnt]     = cpu_to_be64(srplun);
+        lun_lists[combined_target][cnt + 1] = 0;
+        lun_counts[combined_target]++;
+        if (cnt == 0) {
+            nentries++;
+        }
+    }
+
+    if (nentries == 0) {
+        *nentries_out   = 0;
+        *table_addr_out = 0;
+        return 0;
+    }
+
+    ptr_table_size = nentries * sizeof(uint64_t);
+    lun_data_size  = 0;
+    for (t = 0; t < VSCSI_MAX_TARGETS; t++) {
+        if (lun_lists[t]) {
+            lun_data_size += (lun_counts[t] + 1) * sizeof(uint64_t);
+        }
+    }
+    total_size = ptr_table_size + lun_data_size;
+
+    /*
+     * Place the table in the VOF firmware region (0..fw_size), which GRUB
+     * never claims.  Fit into the top of the first 4 KB page, 8-byte aligned.
+     */
+    if (total_size <= 0x1000 - (uint32_t)vof->fw_size) {
+        base_addr = (0x1000 - total_size) & ~7U;
+    } else {
+        base_addr = 0x800;
+    }
+
+    table_addr    = base_addr;
+    lun_data_base = base_addr + ptr_table_size;
+    lun_data_off  = 0;
+    table_idx     = 0;
+
+    for (t = 0; t < VSCSI_MAX_TARGETS; t++) {
+        uint32_t lun_buf_size;
+        uint32_t lun_buf_addr;
+        uint64_t cell_be64;
+
+        if (!lun_lists[t]) {
+            continue;
+        }
+
+        lun_buf_size = (lun_counts[t] + 1) * sizeof(uint64_t);
+        lun_buf_addr = lun_data_base + lun_data_off;
+
+        if (VOF_MEM_WRITE(lun_buf_addr, lun_lists[t], lun_buf_size)
+                != MEMTX_OK) {
+            goto write_err;
+        }
+
+        cell_be64 = cpu_to_be64((uint64_t)lun_buf_addr);
+        if (VOF_MEM_WRITE(table_addr + table_idx * sizeof(uint64_t),
+                          &cell_be64, sizeof(cell_be64)) != MEMTX_OK) {
+            goto write_err;
+        }
+
+        lun_data_off += lun_buf_size;
+        table_idx++;
+    }
+
+    *nentries_out   = table_idx;
+    *table_addr_out = table_addr;
+
+    for (t = 0; t < VSCSI_MAX_TARGETS; t++) {
+        g_free(lun_lists[t]);
+    }
+    return 0;
+
+write_err:
+    for (t = 0; t < VSCSI_MAX_TARGETS; t++) {
+        g_free(lun_lists[t]);
+    }
+    return PROM_ERROR;
+}
+
+/*
+ * Return the SCSIBus for a v-scsi@<reg> path, or NULL if not found.
+ * Handles both "/vdevice/v-scsi@<reg>" and ".../v-scsi@<reg>/disk@..." paths.
+ */
+static SCSIBus *vof_find_vscsi_bus(MachineState *ms, const char *path)
+{
+    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);
+        }
+    }
+    return NULL;
+}
+#endif /* CONFIG_PSERIES */
+
 static uint32_t vof_call_method(MachineState *ms, Vof *vof, uint32_t methodaddr,
                                 uint32_t ihandle, uint32_t param1,
                                 uint32_t param2, uint32_t param3,
@@ -1014,6 +1206,26 @@ static uint32_t vof_call_method(MachineState *ms, Vof *vof, uint32_t methodaddr,
         goto trace_exit;
     }
 
+#ifdef CONFIG_PSERIES
+    /* vscsi-report-luns: enumerate LUNs for GRUB */
+    if (strcmp(method, "vscsi-report-luns") == 0) {
+        SCSIBus *sbus = vof_find_vscsi_bus(ms, inst->path);
+
+        if (sbus) {
+            uint32_t nentries = 0, table_addr = 0;
+            ret = vof_vscsi_report_luns(vof, sbus, &nentries, &table_addr);
+            ret2[0] = nentries;
+            ret2[1] = table_addr;
+        } else {
+            ret = 1;
+            ret2[0] = 0;
+            ret2[1] = 0;
+        }
+        trace_vof_method(ihandle, method, param1, ret, ret2[0]);
+        goto trace_exit;
+    }
+#endif /* CONFIG_PSERIES */
+
     if (strcmp(inst->path, "/") == 0) {
         if (strcmp(method, "ibm,client-architecture-support") == 0) {
             Object *vmo = object_dynamic_cast(OBJECT(ms), TYPE_VOF_MACHINE_IF);
-- 
2.54.0



^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: [RFC PATCH 0/8] ppc/spapr: VOF disk image (qcow2) boot support
  2026-08-17 10:27 [RFC PATCH 0/8] ppc/spapr: VOF disk image (qcow2) boot support uverma
                   ` (7 preceding siblings ...)
  2026-08-17 10:27 ` [RFC PATCH 8/8] ppc/spapr: implement vscsi-report-luns call-method for PAPR vSCSI uverma
@ 2026-08-19  6:55 ` Utkarsh Verma
  8 siblings, 0 replies; 25+ messages in thread
From: Utkarsh Verma @ 2026-08-19  6:55 UTC (permalink / raw)
  To: qemu-devel, qemu-ppc, aik, aik
  Cc: pbonzini, th.huth, nnmlinux, sbhat, harshpb, amachhiw, rathc,
	balaton, philmd, npiggin, marcandre.lureau, fam

+ Alexey (AMD)

Hi Alexey,
Will you continue to maintain VOF in QEMU?
Could you please share your feedback on this patch series?

Regards,
Utkarsh

On 8/17/26 3:57 PM, uverma@linux.ibm.com wrote:
> From: Utkarsh Verma <uverma@linux.ibm.com>
> 
> This series adds support for booting from a disk image under the Virtual Open
> Firmware (VOF) path for sPAPR/pseries machines in QEMU.
> 
> The most common use case for pseries VMs is booting from a distro qcow2 disk
> image. These images carry a PReP boot partition containing GRUB2, which in turn
> loads the kernel and initrd via IEEE 1275 client interface calls.
> VOF currently only supports direct -kernel/-initrd boot; there is no
> firmware-mediated path to hand off to a bootloader on disk. This series
> implements that missing path, making VOF capable of handling the standard
> distro qcow2 boot flow without requiring SLOF.
> 
> This series takes reference from an earlier implementation by Alexey
> Kardashevskiy [1] which explored the same concept but was not carried forward at
> the time.
> 
> The existing load_elf function works on a file, whereas in the VOF the PReP
> partition (GRUB) is read from the disk image and loaded in the memory, also
> on loading the PReP partition in the guest memory the /memory@0/available FDT
> needs to be properly updated for each PT_LOAD segment of the core.elf file
> of the GRUB (PReP partition).
> 
> Implemented a new public elf loader API load_elf_ram_sym_buf() to resolve both
> these issues. load_elf_ram_sym_buf() accepts a caller-supplied memory buffer
> instead of a filename. Internally it writes the buffer to an anonymous memfd
> and delegates to the existing load_elf32/load_elf64 paths, so all existing
> behaviour is preserved. Also, added optional segment_fn_t callback in
> load_elf_ram_sym_buf() through the internal elf_ops.h.inc template; it is
> invoked once per PT_LOAD segment after the load address is resolved, allowing
> callers to perform per-segment work such as VOF memory claims.
> 
> When no -kernel is given, spapr_vof_reset() calls a newly implemented function
> spapr_vof_try_prep_boot() which iterates attached block backends, scans
> partitions for a PReP boot partition, reads the partition contents into a
> host buffer, calls load_elf_ram_sym_buf() to load the GRUB in the guest memory,
> and on success stores the ELF entry point in spapr->kernel_addr.
> spapr_vof_client_dt_finalize() then writes the entry point into
> /chosen/qemu,boot-kernel so VOF then transfers control to GRUB.
> 
> - Partition scanner (spapr_vof_partition.c): scans MBR and GPT partition tables
>    on a BlockBackend to locate the PReP boot partition.
> 
> - Core loader extension (hw/core/loader.c): new load_elf_ram_sym_buf() API loads
>    a 32/64-bit ELF binary from a caller-supplied memory buffer, reusing the
>    existing load_elf32/load_elf64 paths via a memfd. A new segment_fn_t callback
>    in elf_ops.h.inc lets callers act on each PT_LOAD segment as it is resolved.
> 
> - read, seek OF services: two previously unimplemented IEEE 1275 client
>    services, covering block device I/O needed by GRUB.
> 
> - vscsi-report-luns method: walks the SCSIBus and builds the LUN report table
>    in guest memory in the format expected by GRUB's ofdisk driver, matching
>    SLOF's dev-generate-srplun encoding.
> 
> - VTY write/read routing: routes console I/O through the VTY chardev backend;
>    promotes vty_getchars to external linkage.
> 
> - FDT disk@<lun> child nodes (spapr_vscsi.c): emits a child FDT node per
>    attached SCSI disk in VOF mode, giving GRUB a fully-qualified OF path to open.
>    Gated on spapr->vof so SLOF boots are unaffected.
> 
> We used AI to build prototype with manual changes incorporated as identified
> during debugging multiple issues faced while making it work, hence using below
> tag as suggested in [2].
> AI-used-for: Code
> 
> Testing:
> 
> The series has been tested end-to-end by booting a RHEL9.7 qcow2 image on a
> pseries machine with VOF enabled (no SLOF).
> 
> build$ ./qemu-system-ppc64 -M pseries,x-vof=on -cpu power9 -m 2G \
> -hda ../../rhel-guest-image-9.7-20251021.0.ppc64le.qcow2 -nographic
> qemu-system-ppc64: warning: TCG doesn't support requested feature, cap-cfpc=workaround
> qemu-system-ppc64: warning: TCG doesn't support requested feature, cap-ibs=workaround
> qemu-system-ppc64: warning: TCG doesn't support requested feature, cap-ccf-assist=on
> Welcome to GRUB!
> <snip>
>                                 GRUB version 2.06
> 
>   +----------------------------------------------------------------------------+
>   |*Red Hat Enterprise Linux (5.14.0-611.5.1.el9_7.ppc64le) 9.7 (Plow)         |
>   |                                                                            |
>   |                                                                            |
>   |                                                                            |
>   |                                                                            |
>   |                                                                            |
>   |                                                                            |
>   |                                                                            |
>   |                                                                            |
>   |                                                                            |
>   |                                                                            |
>   |                                                                            |
>   +----------------------------------------------------------------------------+
> 
>        Use the ^ and v keys to select which entry is highlighted.
>        Press enter to boot the selected OS, `e' to edit the commands
>        before booting or `c' for a command-line.
> 
> <snip>
> 
> Red Hat Enterprise Linux 9.7 (Plow)
> Kernel 5.14.0-611.5.1.el9_7.ppc64le on an ppc64le
> 
> Activate the web console with: systemctl enable --now cockpit.socket
> 
> localhost login: root
> Password:
> Last login: Fri Jul 10 01:45:44 on hvc0
> [root@localhost ~]#
> [root@localhost ~]# ls /
> afs  boot  etc	 lib	media  opt   root  sbin  sys  usr
> bin  dev   home  lib64	mnt    proc  run   srv	 tmp  var
> [root@localhost ~]#
> [root@localhost ~]# echo "Hello, World!" > hello.txt
> [root@localhost ~]# cat hello.txt
> Hello, World!
> [root@localhost ~]#
> 
> [1] https://lore.kernel.org/qemu-devel/20200203032943.121178-1-aik@ozlabs.ru/#t
> [2] https://lore.kernel.org/qemu-devel/20260529094619.1034458-1-pbonzini@redhat.com/
> 
> Utkarsh Verma (8):
>    ppc/spapr: add PReP boot partition detection
>    hw/loader: add load_elf_ram_sym_buf() for in-memory ELF loading
>    ppc/spapr: add baseline VOF disk boot support
>    ppc/spapr: add VTY backend support to OF read/write/open services
>    ppc/spapr: add block device backend to VOF open/read/write/seek
>      services
>    spapr_vscsi: add VOF disk nodes to the device tree
>    ppc/spapr: strip OF path argument suffix in path_offset
>    ppc/spapr: implement vscsi-report-luns call-method for PAPR vSCSI
> 
>   hw/char/spapr_vty.c          |   2 +-
>   hw/core/loader.c             |  77 ++++++-
>   hw/ppc/meson.build           |   5 +-
>   hw/ppc/spapr_vof.c           | 144 +++++++++++-
>   hw/ppc/spapr_vof_partition.c | 176 ++++++++++++++
>   hw/ppc/trace-events          |   4 +
>   hw/ppc/vof.c                 | 429 +++++++++++++++++++++++++++++++++++
>   hw/scsi/spapr_vscsi.c        |  40 ++++
>   include/hw/core/loader.h     |  30 +++
>   include/hw/elf_ops.h.inc     |  12 +-
>   include/hw/ppc/spapr_vio.h   |   1 +
>   include/hw/ppc/spapr_vof.h   |  14 ++
>   include/hw/ppc/vof.h         |   2 +
>   13 files changed, 923 insertions(+), 13 deletions(-)
>   create mode 100644 hw/ppc/spapr_vof_partition.c
>   create mode 100644 include/hw/ppc/spapr_vof.h
> 



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC PATCH 1/8] ppc/spapr: add PReP boot partition detection
  2026-08-17 10:27 ` [RFC PATCH 1/8] ppc/spapr: add PReP boot partition detection uverma
@ 2026-09-17 17:14   ` Narayana Murty N
  2026-09-25  6:19     ` Utkarsh Verma
  0 siblings, 1 reply; 25+ messages in thread
From: Narayana Murty N @ 2026-09-17 17:14 UTC (permalink / raw)
  To: uverma, qemu-devel, qemu-ppc, aik
  Cc: pbonzini, th.huth, sbhat, harshpb, amachhiw, rathc, balaton,
	philmd, npiggin, marcandre.lureau, fam



On 17/08/26 3:57 PM, uverma@linux.ibm.com wrote:
> From: Utkarsh Verma <uverma@linux.ibm.com>
> 
> Add spapr_vof_partition.c which implements basic MBR and GPT partition
> table scanning to locate the PReP boot partition during VOF enabled boot.
> 
> AI-used-for: code
> Signed-off-by: Utkarsh Verma <uverma@linux.ibm.com>
> ---
>   hw/ppc/meson.build           |   5 +-
>   hw/ppc/spapr_vof_partition.c | 176 +++++++++++++++++++++++++++++++++++
>   include/hw/ppc/spapr_vof.h   |  14 +++
>   3 files changed, 194 insertions(+), 1 deletion(-)
>   create mode 100644 hw/ppc/spapr_vof_partition.c
>   create mode 100644 include/hw/ppc/spapr_vof.h
> 
> diff --git a/hw/ppc/meson.build b/hw/ppc/meson.build
> index 37aa535db2..68a5d2622f 100644
> --- a/hw/ppc/meson.build
> +++ b/hw/ppc/meson.build
> @@ -93,6 +93,9 @@ ppc_ss.add(when: 'CONFIG_AMIGAONE', if_true: files('amigaone.c'))
>   ppc_ss.add(when: 'CONFIG_PEGASOS', if_true: files('pegasos.c'))
>   
>   ppc_ss.add(when: 'CONFIG_VOF', if_true: files('vof.c'))
> -ppc_ss.add(when: ['CONFIG_VOF', 'CONFIG_PSERIES'], if_true: files('spapr_vof.c'))
> +ppc_ss.add(when: ['CONFIG_VOF', 'CONFIG_PSERIES'], if_true: files(
> +  'spapr_vof.c',
> +  'spapr_vof_partition.c',
> +))
>   
>   hw_arch += {'ppc': ppc_ss}
> diff --git a/hw/ppc/spapr_vof_partition.c b/hw/ppc/spapr_vof_partition.c
> new file mode 100644
> index 0000000000..ced2818998
> --- /dev/null
> +++ b/hw/ppc/spapr_vof_partition.c
> @@ -0,0 +1,176 @@
> +/*
> + * QEMU PowerPC sPAPR VOF Partition Table Support.
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + *
> + * This implements partition table detection for VOF boot,
> + * supporting MBR and GPT partition tables with focus on PReP boot partition.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qemu/bswap.h"
> +#include "qemu/error-report.h"
> +#include "system/block-backend-io.h"
> +#include "hw/ppc/spapr_vof.h"
> +
> +#define SECTOR_SIZE              512
> +#define MBR_PARTITION_ENTRY_SIZE 16
> +#define MBR_PARTITION_TABLE_OFFSET 446
> +#define MBR_NUM_PARTITIONS       4
> +
> +#define PARTITION_TYPE_PREP      0x41  /* PReP Boot partition */
> +#define PARTITION_TYPE_GPT       0xEE
> +
> +#define GPT_SIGNATURE            "EFI PART"
> +#define GPT_SIGNATURE_SIZE       8
> +
> +/* PReP Boot: 9E1A2D38-C612-4316-AA26-8B49521E5A8B */
> +static const uint8_t GUID_PREP_BOOT[16] = {
> +    0x38, 0x2D, 0x1A, 0x9E, 0x12, 0xC6, 0x16, 0x43,
> +    0xAA, 0x26, 0x8B, 0x49, 0x52, 0x1E, 0x5A, 0x8B
> +};
> +
> +typedef struct MBRPartitionEntry {
> +    uint8_t  boot_flag;
> +    uint8_t  start_chs[3];
> +    uint8_t  type;
> +    uint8_t  end_chs[3];
> +    uint32_t start_lba;
> +    uint32_t num_sectors;
> +} QEMU_PACKED MBRPartitionEntry;
> +
> +typedef struct GPTHeader {
> +    char     signature[8];
> +    uint32_t revision;
> +    uint32_t header_size;
> +    uint32_t header_crc32;
> +    uint32_t reserved;
> +    uint64_t current_lba;
> +    uint64_t backup_lba;
> +    uint64_t first_usable_lba;
> +    uint64_t last_usable_lba;
> +    uint8_t  disk_guid[16];
> +    uint64_t partition_entries_lba;
> +    uint32_t num_partition_entries;
> +    uint32_t partition_entry_size;
> +    uint32_t partition_array_crc32;
> +} QEMU_PACKED GPTHeader;
> +
> +typedef struct GPTPartitionEntry {
> +    uint8_t  type_guid[16];
> +    uint8_t  partition_guid[16];
> +    uint64_t first_lba;
> +    uint64_t last_lba;
> +    uint64_t attributes;
> +    uint16_t name[36];
> +} QEMU_PACKED GPTPartitionEntry;
> +
> +static bool find_prep_partition_gpt(BlockBackend *blk,
> +                                    uint64_t *offset, uint64_t *size)
> +{
> +    uint8_t lba1[SECTOR_SIZE];
> +    GPTHeader *header;
> +    uint64_t entries_lba;
> +    uint32_t num_entries;
> +    uint32_t entry_size;
> +    uint64_t current_entry_offset;
> +    uint8_t *buf;
> +    GPTPartitionEntry *current_entry;
> +    uint32_t i;
> +    int ret;
> +
> +    ret = blk_pread(blk, SECTOR_SIZE, SECTOR_SIZE, lba1, 0);
> +    if (ret < 0) {
> +        warn_report("GPT: failed to read LBA1 (ret=%d)", ret);
> +        return false;
> +    }
> +
> +    header = (GPTHeader *)lba1;
> +
> +    if (memcmp(header->signature, GPT_SIGNATURE, GPT_SIGNATURE_SIZE) != 0) {
> +        return false;
> +    }
> +
> +    entries_lba = le64_to_cpu(header->partition_entries_lba);
> +    num_entries = le32_to_cpu(header->num_partition_entries);
> +    entry_size = le32_to_cpu(header->partition_entry_size);
> +
> +    if (num_entries > 128) {
> +        num_entries = 128;
> +    }
Why is silently truncating to 128 the right behaviour?

This bounds the loop, which is good, but it also means that a valid PReP 
partition after entry 127 will not be found. If 128 is an intentional 
VOF limitation, can we document that? Otherwise it may be better to 
validate the table size and iterate the actual number of entries subject 
to an explicit resource limit.
> +
> +    if (entry_size < 128) {
> +        return false;
> +    }
> +
> +    buf = g_malloc(entry_size);
The "entry_size" comes directly from the GPT header, so checking only 
the lower bound means a malformed image can drive an arbitrarily large 
host allocation here.

Do we need to allocate "entry_size" bytes at all?

We only consume the fixed fields at the beginning of the GPT entry. It 
seems safer to use `entry_size` only as the stride between entries and 
read `sizeof(GPTPartitionEntry)` bytes for each entry after checking 
that `entry_size >= sizeof(GPTPartitionEntry)`.

That also avoids having to invent an arbitrary maximum GPT entry size.

> +    for (i = 0; i < num_entries; i++) {
> +        current_entry_offset = (entries_lba * SECTOR_SIZE) +
> +                               ((uint64_t)i * entry_size);
Both the multiplication and addition are based on image-controlled 
fields. Could we validate the complete partition-entry array against 
blk_getlength() using checked arithmetic before iterating it?
> +
> +        ret = blk_pread(blk, current_entry_offset, entry_size, buf, 0);
> +        if (ret < 0) {
> +            warn_report("GPT: failed to read partition entry %u "
> +                        "(ret=%d)", i, ret);
> +            g_free(buf);
> +            return false;
> +        }
> +
> +        current_entry = (GPTPartitionEntry *)buf;
> +        if (memcmp(current_entry->type_guid, GUID_PREP_BOOT,
> +                   sizeof(GUID_PREP_BOOT)) == 0) {
> +            *offset = le64_to_cpu(current_entry->first_lba) * SECTOR_SIZE;
> +            *size = (le64_to_cpu(current_entry->last_lba) -
> +                     le64_to_cpu(current_entry->first_lba) + 1) * SECTOR_SIZE;
I think this needs validation before returning the partition.

If last_lba < first_lba, the subtraction underflows and produces a very 
large partition size. There are also multiplication/partition-end 
overflow cases.

I'd expect something along the lines of:
	first_lba <= last_lba
	first_lba <= disk_last_lba
	last_lba <= disk_last_lba

> +            g_free(buf);
> +            return true;
> +        }
> +    }
> +    g_free(buf);
> +    return false;
> +}
> +
> +static bool find_prep_partition_mbr(BlockBackend *blk,
> +                                    uint64_t *offset, uint64_t *size)
> +{
> +    uint8_t mbr[SECTOR_SIZE];
> +    MBRPartitionEntry *entry;
> +    int ret;
> +    int i;
> +
> +    ret = blk_pread(blk, 0, SECTOR_SIZE, mbr, 0);
> +    if (ret < 0) {
> +        warn_report("MBR: failed to read MBR sector (ret=%d)", ret);
> +        return false;
> +    }
> +
> +    /* MBR boot signature: byte 510 = 0x55, byte 511 = 0xAA */
> +    if (mbr[510] != 0x55 || mbr[511] != 0xAA) {
> +        return false;
> +    }
> +
> +    for (i = 0; i < MBR_NUM_PARTITIONS; i++) {
> +        entry = (MBRPartitionEntry *)&mbr[MBR_PARTITION_TABLE_OFFSET +
> +                                          i * MBR_PARTITION_ENTRY_SIZE];
> +
> +        if (entry->type == PARTITION_TYPE_GPT) {
> +            return find_prep_partition_gpt(blk, offset, size);
> +        }
> +
> +        if (entry->type == PARTITION_TYPE_PREP) {
> +            *offset = (uint64_t)le32_to_cpu(entry->start_lba) * SECTOR_SIZE;
> +            *size = (uint64_t)le32_to_cpu(entry->num_sectors) * SECTOR_SIZE;
> +            return true;
> +        }
> +    }
> +    return false;
> +}
> +
> +bool spapr_vof_find_prep_partition(BlockBackend *blk,
> +                                   uint64_t *offset, uint64_t *size)
> +{
> +    if (!blk) {
> +        return false;
> +    }
> +    return find_prep_partition_mbr(blk, offset, size);
> +}
> diff --git a/include/hw/ppc/spapr_vof.h b/include/hw/ppc/spapr_vof.h
> new file mode 100644
> index 0000000000..08787d2aeb
> --- /dev/null
> +++ b/include/hw/ppc/spapr_vof.h
> @@ -0,0 +1,14 @@
> +/*
> + * QEMU PowerPC sPAPR VOF Support
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +#ifndef HW_SPAPR_VOF_H
> +#define HW_SPAPR_VOF_H
> +
> +typedef struct BlockBackend BlockBackend;
> +
> +bool spapr_vof_find_prep_partition(BlockBackend *blk,
> +                                   uint64_t *offset, uint64_t *size);
> +
> +#endif /* HW_SPAPR_VOF_H */

we parse header_size, CRC fields, usable-LBA fields, etc. into 
GPTHeader, but only check the signature.

I don't think CRC validation should replace range validation, but at 
least the header fields which determine the layout we consume should 
probably be validated before trusting the partition table.

check CRC validation posibilities .

Thanks,
Narayana Murty N



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC PATCH 2/8] hw/loader: add load_elf_ram_sym_buf() for in-memory ELF loading
  2026-08-17 10:27 ` [RFC PATCH 2/8] hw/loader: add load_elf_ram_sym_buf() for in-memory ELF loading uverma
@ 2026-09-17 17:14   ` Narayana Murty N
  2026-09-25  6:24     ` Utkarsh Verma
  0 siblings, 1 reply; 25+ messages in thread
From: Narayana Murty N @ 2026-09-17 17:14 UTC (permalink / raw)
  To: uverma, qemu-devel, qemu-ppc, aik
  Cc: pbonzini, th.huth, sbhat, harshpb, amachhiw, rathc, balaton,
	philmd, npiggin, marcandre.lureau, fam



On 17/08/26 3:57 PM, uverma@linux.ibm.com wrote:
> From: Utkarsh Verma <uverma@linux.ibm.com>
> 
> Add a new public API load_elf_ram_sym_buf() that loads an ELF
> image from a caller-supplied memory buffer rather than a file path.
> The implementation writes the buffer to an anonymous memfd, then
> reuses the existing load_elf32/load_elf64 paths.
> 
> To support per-segment inspection by callers (e.g. detecting VoF
> firmware segments), extend the internal load_elf{32,64} template
> (elf_ops.h.inc) with an optional segment_fn_t callback that is
> invoked once for every PT_LOAD segment after its load address is
> resolved. Returning false from the callback aborts the load with
> ELF_LOAD_FAILED.
> 
> AI-used-for: code
> Signed-off-by: Utkarsh Verma <uverma@linux.ibm.com>
> ---
>   hw/core/loader.c         | 77 ++++++++++++++++++++++++++++++++++++++--
>   include/hw/core/loader.h | 30 ++++++++++++++++
>   include/hw/elf_ops.h.inc | 12 ++++++-
>   3 files changed, 116 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/core/loader.c b/hw/core/loader.c
> index 5cbfba0a86..4e4e9764a3 100644
> --- a/hw/core/loader.c
> +++ b/hw/core/loader.c
> @@ -43,6 +43,7 @@
>    */
>   
>   #include "qemu/osdep.h"
> +#include "qemu/memfd.h"
>   #include "qemu/datadir.h"
>   #include "qemu/error-report.h"
>   #include "qapi/error.h"
> @@ -510,12 +511,14 @@ ssize_t load_elf_ram_sym(const char *filename,
>           ret = load_elf64(filename, fd, elf_note_fn,
>                            translate_fn, translate_opaque, must_swab,
>                            pentry, lowaddr, highaddr, pflags, elf_machine,
> -                         clear_lsb, data_swab, as, load_rom, sym_cb);
> +                         clear_lsb, data_swab, as, load_rom, sym_cb,
> +                         NULL, NULL);
>       } else {
>           ret = load_elf32(filename, fd, elf_note_fn,
>                            translate_fn, translate_opaque, must_swab,
>                            pentry, lowaddr, highaddr, pflags, elf_machine,
> -                         clear_lsb, data_swab, as, load_rom, sym_cb);
> +                         clear_lsb, data_swab, as, load_rom, sym_cb,
> +                         NULL, NULL);
>       }
>   
>       if (ret > 0) {
> @@ -527,6 +530,76 @@ ssize_t load_elf_ram_sym(const char *filename,
>       return ret;
>   }
>   
> +ssize_t load_elf_ram_sym_buf(const uint8_t *buf, size_t buflen,
> +                              uint64_t (*elf_note_fn)(void *, void *, bool),
> +                              uint64_t (*translate_fn)(void *, uint64_t),
> +                              void *translate_opaque, uint64_t *pentry,
> +                              uint64_t *lowaddr, uint64_t *highaddr,
> +                              uint32_t *pflags, int elf_data_order,
> +                              int elf_machine, int clear_lsb, int data_swab,
> +                              AddressSpace *as, bool load_rom,
> +                              symbol_fn_t sym_cb,
> +                              segment_fn_t segment_fn, void *segment_opaque)
> +{
> +    const int host_data_order = HOST_BIG_ENDIAN ? ELFDATA2MSB : ELFDATA2LSB;
> +    int fd, must_swab;
> +    ssize_t ret = ELF_LOAD_FAILED;
> +    uint8_t e_ident[EI_NIDENT];
> +
> +    fd = memfd_create("qemu-elf-buf", MFD_CLOEXEC);

The memfd_create() is Linux-specific.

qemu/memfd.h only exposes the direct memfd_create() declaration for 
Linux configurations, so this also appears to break non-Linux system builds.

Could this use an existing portable QEMU temporary-fd abstraction, or 
alternatively could the ELF loader be factored so the common parser 
accepts an fd/buffer directly?

It seems undesirable for a generic loader API to require Linux memfd 
support.

OR

If it is an arch specific design decision, then we need to document it.

> +    if (fd < 0) {
> +        error_report("load_elf_ram_sym_buf: memfd_create: %s", strerror(errno));
> +        return ELF_LOAD_FAILED;
> +    }
> +
> +    if (write(fd, buf, buflen) != (ssize_t)buflen) {

A single write() is not guaranteed to consume the whole buffer and can 
be interrupted.

Also, if buflen > SSIZE_MAX, the cast used for comparison is problematic.

Please use the appropriate QEMU full-write helper (or loop handling 
short writes/EINTR).

> +        error_report("load_elf_ram_sym_buf: write: %s", strerror(errno));
> +        goto fail;
> +    }
> +
> +    lseek(fd, 0, SEEK_SET);
> +    if (read(fd, e_ident, sizeof(e_ident)) != sizeof(e_ident)) {
> +        goto fail;
> +    }
> +    if (e_ident[0] != ELFMAG0 ||
> +        e_ident[1] != ELFMAG1 ||
> +        e_ident[2] != ELFMAG2 ||
> +        e_ident[3] != ELFMAG3) {
> +        ret = ELF_LOAD_NOT_ELF;
> +        goto fail;
> +    }
> +
> +    if (elf_data_order != ELFDATANONE && elf_data_order != e_ident[EI_DATA]) {
> +        ret = ELF_LOAD_WRONG_ENDIAN;
> +        goto fail;
> +    }
> +
> +    must_swab = host_data_order != e_ident[EI_DATA];
> +
> +    lseek(fd, 0, SEEK_SET);

both lseek call errors need to be handled.

> +    if (e_ident[EI_CLASS] == ELFCLASS64) {
> +        ret = load_elf64("(buffer)", fd, elf_note_fn,
> +                         translate_fn, translate_opaque, must_swab,
> +                         pentry, lowaddr, highaddr, pflags, elf_machine,
> +                         clear_lsb, data_swab, as, load_rom, sym_cb,
> +                         segment_fn, segment_opaque);
> +    } else {
> +        ret = load_elf32("(buffer)", fd, elf_note_fn,
> +                         translate_fn, translate_opaque, must_swab,
> +                         pentry, lowaddr, highaddr, pflags, elf_machine,
> +                         clear_lsb, data_swab, as, load_rom, sym_cb,
> +                         segment_fn, segment_opaque);
> +    }
> +
> +    if (ret > 0) {
> +        debuginfo_report_elf("(buffer)", fd, 0);
> +    }
> +
> + fail:
> +    close(fd);
> +    return ret;
> +}
> +
>   static void bswap_uboot_header(uboot_image_header_t *hdr)
>   {
>   #if !HOST_BIG_ENDIAN
> diff --git a/include/hw/core/loader.h b/include/hw/core/loader.h
> index d9431e8a8d..b67b2ca013 100644
> --- a/include/hw/core/loader.h
> +++ b/include/hw/core/loader.h
> @@ -156,6 +156,36 @@ ssize_t load_elf_ram_sym(const char *filename,
>                            int clear_lsb, int data_swab,
>                            AddressSpace *as, bool load_rom, symbol_fn_t sym_cb);
>   
> +/*
> + * segment_fn_t:
> + * Per-PT_LOAD segment callback for load_elf_ram_sym_buf().
> + */
> +typedef bool (*segment_fn_t)(void *opaque,
> +                             uint64_t paddr, uint64_t vaddr,
> +                             uint64_t filesz, uint64_t memsz);
> +
> +/*
> + * load_elf_ram_sym_buf:
> + * @buf: pointer to an in-memory ELF image
> + * @buflen: size of @buf in bytes
> + * @segment_fn: optional per-PT_LOAD callback
> + * @segment_opaque: opaque data passed to @segment_fn
> + *
> + * Identical to load_elf_ram_sym() but loads from a caller-supplied
> + * memory buffer instead of a file.  All other parameters have the
> + * same meaning as load_elf_ram_sym().
> + */
> +ssize_t load_elf_ram_sym_buf(const uint8_t *buf, size_t buflen,
> +                             uint64_t (*elf_note_fn)(void *, void *, bool),
> +                             uint64_t (*translate_fn)(void *, uint64_t),
> +                             void *translate_opaque, uint64_t *pentry,
> +                             uint64_t *lowaddr, uint64_t *highaddr,
> +                             uint32_t *pflags, int elf_data_order,
> +                             int elf_machine, int clear_lsb, int data_swab,
> +                             AddressSpace *as, bool load_rom,
> +                             symbol_fn_t sym_cb,
> +                             segment_fn_t segment_fn, void *segment_opaque);
> +
>   /** load_elf_as:
>    * Same as load_elf_ram_sym(), but always loads the elf as ROM
>    */
> diff --git a/include/hw/elf_ops.h.inc b/include/hw/elf_ops.h.inc
> index 044e72de2a..a2c9b52bf9 100644
> --- a/include/hw/elf_ops.h.inc
> +++ b/include/hw/elf_ops.h.inc
> @@ -321,7 +321,9 @@ static ssize_t glue(load_elf, SZ)(const char *name, int fd,
>                                     uint32_t *pflags, int elf_machine,
>                                     int clear_lsb, int data_swab,
>                                     AddressSpace *as, bool load_rom,
> -                                  symbol_fn_t sym_cb)
> +                                  symbol_fn_t sym_cb,
> +                                  segment_fn_t segment_fn,
> +                                  void *segment_opaque)
>   {
>       struct elfhdr ehdr;
>       struct elf_phdr *phdr = NULL, *ph;
> @@ -504,6 +506,14 @@ static ssize_t glue(load_elf, SZ)(const char *name, int fd,
>                   addr = ph->p_paddr;
>               }
>   
> +            if (segment_fn) {
> +                if (!segment_fn(segment_opaque, addr, ph->p_vaddr,
> +                                file_size, mem_size)) {
> +                    ret = ELF_LOAD_FAILED;
> +                    goto fail;
> +                }
> +            }
> +
>               if (data_swab) {
>                   elf_word j;
>                   for (j = 0; j < file_size; j += (1 << data_swab)) {

Thanks,
Narayana Murty N



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC PATCH 3/8] ppc/spapr: add baseline VOF disk boot support
  2026-08-17 10:27 ` [RFC PATCH 3/8] ppc/spapr: add baseline VOF disk boot support uverma
@ 2026-09-17 17:33   ` Narayana Murty N
  2026-09-25  6:28     ` Utkarsh Verma
  0 siblings, 1 reply; 25+ messages in thread
From: Narayana Murty N @ 2026-09-17 17:33 UTC (permalink / raw)
  To: uverma, qemu-devel, qemu-ppc, aik
  Cc: pbonzini, th.huth, sbhat, harshpb, amachhiw, rathc, balaton,
	philmd, npiggin, marcandre.lureau, fam



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



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC PATCH 8/8] ppc/spapr: implement vscsi-report-luns call-method for PAPR vSCSI
  2026-08-17 10:27 ` [RFC PATCH 8/8] ppc/spapr: implement vscsi-report-luns call-method for PAPR vSCSI uverma
@ 2026-09-18  5:18   ` Narayana Murty N
  2026-09-25  6:49     ` Utkarsh Verma
  2026-09-18  5:26   ` Narayana Murty N
  1 sibling, 1 reply; 25+ messages in thread
From: Narayana Murty N @ 2026-09-18  5:18 UTC (permalink / raw)
  To: uverma, qemu-devel, qemu-ppc, aik
  Cc: pbonzini, th.huth, sbhat, harshpb, amachhiw, rathc, balaton,
	philmd, npiggin, marcandre.lureau, fam, avnish



On 17/08/26 3:57 PM, uverma@linux.ibm.com wrote:
> From: Utkarsh Verma <uverma@linux.ibm.com>
> 
> GRUB (grub-core/disk/ieee1275/ofdisk.c) calls "vscsi-report-luns" on
> any PAPR virtual SCSI controller ihandle to enumerate attached LUNs
> before attempting to boot from disk. Without this method VOF returns
> PROM_ERROR and GRUB cannot find vSCSI disks.
> 
> Add vof_find_vscsi_bus() to locate the SCSIBus from a v-scsi@<reg>
> instance path, and vof_vscsi_report_luns() to build the response table
> in the VOF firmware region (0..fw_size), which GRUB never claims.
> 
> The table layout and SRP LUN encoding follow SLOF (vio-vscsi.fs
> dev-generate-srplun) each entry is an 8-byte big-endian cell whose
> low 4 bytes hold a guest pointer to a null-terminated list of SRP LUNs
> for that target; GRUB reads the pointer at offset table + 4 + 8*i.
> 
> AI-used-for: code
> Signed-off-by: Utkarsh Verma <uverma@linux.ibm.com>
> ---
>   hw/ppc/vof.c | 212 +++++++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 212 insertions(+)
> 
> diff --git a/hw/ppc/vof.c b/hw/ppc/vof.c
> index d3b0df30e4..b1bf5b1af0 100644
> --- a/hw/ppc/vof.c
> +++ b/hw/ppc/vof.c
> @@ -991,6 +991,198 @@ static void vof_instantiate_rtas(Error **errp)
>       error_setg(errp, "The firmware should have instantiated RTAS");
>   }
>   
> +#ifdef CONFIG_PSERIES
> +/* Combined (channel<<6)|id index space, matches SLOF dev-max-target. */
> +#define VSCSI_MAX_TARGETS 512
> +
> +/*
> + * Build the vscsi-report-luns response in guest memory.
> + *
> + * Returns catch_result (0 = success).  On success, *nentries_out and
> + * *table_addr_out describe the result table written into the VOF firmware
> + * region (0..fw_size), which GRUB never claims.
> + *
> + * Table layout (big-endian, 8 bytes per entry):
> + *   [table_addr + 8*i + 0..3] : 0x00000000
> + *   [table_addr + 8*i + 4..7] : guest pointer to null-terminated SRP LUN list
> + *
> + * GRUB reads each pointer as: *(uint32_t *)(table + 4 + 8 * i)
> + *
> + * SRP LUN encoding (SLOF dev-generate-srplun):
> + *   srplun = (0x8000 | bus | target | lun) << 48
> + *   where bus    = (combined_target >> 1) & 0x70
> + *         target = (combined_target & 0x3f) << 8
> + *         combined_target = (channel << 6) | id
> + */
> +static uint32_t vof_vscsi_report_luns(Vof *vof,
> +                                       SCSIBus *sbus,
> +                                       uint32_t *nentries_out,
> +                                       uint32_t *table_addr_out)
> +{
> +    uint64_t *lun_lists[VSCSI_MAX_TARGETS];
> +    int lun_counts[VSCSI_MAX_TARGETS];
> +    uint32_t ptr_table_size;
> +    uint32_t lun_data_size;
> +    uint32_t total_size;
> +    uint32_t base_addr;
> +    uint32_t lun_data_base;
> +    uint32_t lun_data_off;
> +    uint32_t table_idx;
> +    uint32_t table_addr;
> +    BusChild *kid;
> +    int nentries = 0;
> +    int t;
> +
> +    memset(lun_lists, 0, sizeof(lun_lists));
> +    memset(lun_counts, 0, sizeof(lun_counts));
> +
> +    QTAILQ_FOREACH(kid, &sbus->qbus.children, sibling) {
> +        SCSIDevice *dev = SCSI_DEVICE(kid->child);
> +        int combined_target;
> +        uint64_t bus_field;
> +        uint64_t target_field;
> +        uint64_t lun_field;
> +        uint64_t srplun;
> +        int cnt;
> +
> +        /* combined_target = (channel << 6) | id, matching SLOF bus+target */
> +        combined_target = ((dev->channel & 0x7) << 6) | (dev->id & 0x3f);
> +        if (combined_target >= VSCSI_MAX_TARGETS) {
> +            continue;
> +        }
> +
> +        bus_field    = ((uint64_t)combined_target >> 1) & 0x70ULL;
> +        target_field = ((uint64_t)combined_target & 0x3fULL) << 8;
> +        lun_field    = (uint64_t)(dev->lun & 0x1f);
> +        srplun = (0x8000ULL | bus_field | target_field | lun_field) << 48;
> +
> +        cnt = lun_counts[combined_target];
> +        lun_lists[combined_target] = g_realloc(lun_lists[combined_target],
> +                                               (cnt + 2) * sizeof(uint64_t));
> +        lun_lists[combined_target][cnt]     = cpu_to_be64(srplun);
> +        lun_lists[combined_target][cnt + 1] = 0;
> +        lun_counts[combined_target]++;
> +        if (cnt == 0) {
> +            nentries++;
> +        }
> +    }
> +
> +    if (nentries == 0) {
> +        *nentries_out   = 0;
> +        *table_addr_out = 0;
> +        return 0;
> +    }
> +
> +    ptr_table_size = nentries * sizeof(uint64_t);
> +    lun_data_size  = 0;
> +    for (t = 0; t < VSCSI_MAX_TARGETS; t++) {
> +        if (lun_lists[t]) {
> +            lun_data_size += (lun_counts[t] + 1) * sizeof(uint64_t);
> +        }
> +    }
> +    total_size = ptr_table_size + lun_data_size;
> +
> +    /*
> +     * Place the table in the VOF firmware region (0..fw_size), which GRUB
> +     * never claims.  Fit into the top of the first 4 KB page, 8-byte aligned.
> +     */
> +    if (total_size <= 0x1000 - (uint32_t)vof->fw_size) {
> +        base_addr = (0x1000 - total_size) & ~7U;
> +    } else {
> +        base_addr = 0x800;
> +    }
> +
I don't think the fallback to 0x800 is safe.

The VOF allocation map documented in spapr_vof_reset() currently has the 
initial firmware at approximately:

0 .. 0xc38

so 0x800 lies inside that firmware image.

For a sufficiently large LUN report, this branch therefore appears to 
write the table over active firmware memory.
> +    table_addr    = base_addr;
> +    lun_data_base = base_addr + ptr_table_size;
> +    lun_data_off  = 0;
> +    table_idx     = 0;
> +
> +    for (t = 0; t < VSCSI_MAX_TARGETS; t++) {
> +        uint32_t lun_buf_size;
> +        uint32_t lun_buf_addr;
> +        uint64_t cell_be64;
> +
> +        if (!lun_lists[t]) {
> +            continue;
> +        }
> +
> +        lun_buf_size = (lun_counts[t] + 1) * sizeof(uint64_t);
> +        lun_buf_addr = lun_data_base + lun_data_off;
> +
> +        if (VOF_MEM_WRITE(lun_buf_addr, lun_lists[t], lun_buf_size)
> +                != MEMTX_OK) {
> +            goto write_err;
> +        }
> +
> +        cell_be64 = cpu_to_be64((uint64_t)lun_buf_addr);
> +        if (VOF_MEM_WRITE(table_addr + table_idx * sizeof(uint64_t),
> +                          &cell_be64, sizeof(cell_be64)) != MEMTX_OK) {
> +            goto write_err;
> +        }
> +
> +        lun_data_off += lun_buf_size;
> +        table_idx++;
> +    }
> +
> +    *nentries_out   = table_idx;
> +    *table_addr_out = table_addr;
> +
> +    for (t = 0; t < VSCSI_MAX_TARGETS; t++) {
> +        g_free(lun_lists[t]);
> +    }
> +    return 0;
> +
> +write_err:
> +    for (t = 0; t < VSCSI_MAX_TARGETS; t++) {
> +        g_free(lun_lists[t]);
> +    }
> +    return PROM_ERROR;
> +}
> +
> +/*
> + * Return the SCSIBus for a v-scsi@<reg> path, or NULL if not found.
> + * Handles both "/vdevice/v-scsi@<reg>" and ".../v-scsi@<reg>/disk@..." paths.
> + */
> +static SCSIBus *vof_find_vscsi_bus(MachineState *ms, const char *path)
> +{
> +    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);
> +        }
> +    }
> +    return NULL;
> +}
> +#endif /* CONFIG_PSERIES */
> +
>   static uint32_t vof_call_method(MachineState *ms, Vof *vof, uint32_t methodaddr,
>                                   uint32_t ihandle, uint32_t param1,
>                                   uint32_t param2, uint32_t param3,
> @@ -1014,6 +1206,26 @@ static uint32_t vof_call_method(MachineState *ms, Vof *vof, uint32_t methodaddr,
>           goto trace_exit;
>       }
>   
> +#ifdef CONFIG_PSERIES
> +    /* vscsi-report-luns: enumerate LUNs for GRUB */
> +    if (strcmp(method, "vscsi-report-luns") == 0) {
> +        SCSIBus *sbus = vof_find_vscsi_bus(ms, inst->path);
> +
> +        if (sbus) {
> +            uint32_t nentries = 0, table_addr = 0;
> +            ret = vof_vscsi_report_luns(vof, sbus, &nentries, &table_addr);
> +            ret2[0] = nentries;
> +            ret2[1] = table_addr;
> +        } else {
> +            ret = 1;
> +            ret2[0] = 0;
> +            ret2[1] = 0;
> +        }
> +        trace_vof_method(ihandle, method, param1, ret, ret2[0]);
> +        goto trace_exit;
> +    }
> +#endif /* CONFIG_PSERIES */
> +
>       if (strcmp(inst->path, "/") == 0) {
>           if (strcmp(method, "ibm,client-architecture-support") == 0) {
>               Object *vmo = object_dynamic_cast(OBJECT(ms), TYPE_VOF_MACHINE_IF);

Over view:
In particular test malformed GPT fields (entry_size, reversed LBA 
ranges, overflowing table offsets), a very large OF read/write length, 
multiple vSCSI controllers using the same channel/id/lun tuple, IDs >= 
32/non-zero channels for SRP-LUN encoding, a malformed ELF after one or 
more successful PT_LOAD callbacks, and enough vSCSI targets to exceed 
the free space below 4 KiB.

Those cases exercise precisely the boundaries introduced by this series.

Thanks,
Narayana Murty N


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC PATCH 5/8] ppc/spapr: add block device backend to VOF open/read/write/seek services
  2026-08-17 10:27 ` [RFC PATCH 5/8] ppc/spapr: add block device backend to VOF open/read/write/seek services uverma
@ 2026-09-18  5:23   ` Narayana Murty N
  2026-09-25  6:33     ` Utkarsh Verma
  0 siblings, 1 reply; 25+ messages in thread
From: Narayana Murty N @ 2026-09-18  5:23 UTC (permalink / raw)
  To: uverma, qemu-devel, qemu-ppc, aik
  Cc: pbonzini, th.huth, sbhat, harshpb, amachhiw, rathc, balaton,
	philmd, npiggin, marcandre.lureau, fam, avnish

Hi Utkarsh,

On 17/08/26 3:57 PM, 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 FDT nodes named "disk@<srp-lun>" and resolve
> the SRP LUN encoding to the matching SCSIDevice/BlockBackend by scanning
> id/channel/lun.
> 
> 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>
> ---
>   hw/ppc/trace-events |   2 +
>   hw/ppc/vof.c        | 116 +++++++++++++++++++++++++++++++++++++++++++-
>   2 files changed, 117 insertions(+), 1 deletion(-)
> 
> 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 45c197df9f..b1c574aabf 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/timer.h"
>   #include "qemu/range.h"
>   #include "qemu/units.h"
> @@ -24,6 +25,8 @@
>   #include "trace.h"
>   
>   #include "hw/ppc/spapr_vio.h"
> +#include "hw/scsi/scsi.h"
> +#include "system/block-backend.h"
>   #include <libfdt.h>
>   
>   /*
> @@ -46,6 +49,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;
>   
> @@ -449,6 +454,7 @@ 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;
>   
>       if (vof->of_instance_last == 0xFFFFFFFF) {
>           /* We do not recycle ihandles yet */
> @@ -461,10 +467,42 @@ 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);
> +
> +    if (node_name && strncmp(node_name, "disk@", 5) == 0) {
> +        uint64_t srp_lun;
> +        uint32_t id, channel, lun;
> +        BlockBackend *blk;
> +
> +        if (qemu_strtou64(node_name + 5, NULL, 16, &srp_lun) == 0) {
> +            id      = (srp_lun >> 56) & 0x3f;
> +            channel = (srp_lun >> 53) & 0x7;
> +            lun     = (srp_lun >> 48) & 0x1f;
> +
> +            for (blk = blk_next(NULL); blk; blk = blk_next(blk)) {
> +                DeviceState *attached = blk_get_attached_dev(blk);
> +                SCSIDevice *sdev;
> +
> +                if (!attached) {
> +                    continue;
> +                }
> +                sdev = (SCSIDevice *)object_dynamic_cast(OBJECT(attached),
> +                                                         TYPE_SCSI_DEVICE);
> +                if (sdev && sdev->id == (int)id &&
> +                    sdev->channel == (int)channel &&
> +                    sdev->lun == (int)lun) {
> +                    inst->blk = blk;
> +                    break;
> +                }
> +            }
Shouldn't vof_do_open() first resolve that specific vSCSI 
controller/SCSIBus and then find the SCSIDevice on that bus, rather than 
searching the global BlockBackend list?

Patch 8 already has code for resolving a v-scsi path to its SCSIBus; it 
seems like the block-open path should use the same topology-aware lookup.

> +        }
> +    }
> +
>   #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());
> @@ -601,6 +639,25 @@ static uint32_t vof_write(Vof *vof, uint32_t ihandle, uint32_t buf,
>           return PROM_ERROR;
>       }
>   
> +    if (inst->blk) {
> +        g_autofree uint8_t *blkbuf = g_malloc(len);
> +        int ret;
> +
> +        if (VOF_MEM_READ(buf, blkbuf, len) != MEMTX_OK) {
> +            trace_vof_error_write(ihandle);
> +            return PROM_ERROR;
> +        }
> +        ret = blk_pwrite(inst->blk, inst->pos, len, blkbuf, 0);
Does VOF disk boot require block-device writes at all?

The series description talks about GRUB discovering the disk and reading 
the kernel/initrd. If writable OF disk instances are not required for 
boot, could we avoid exposing disk writes entirely?

If write support is required, this should also honour read-only backend 
state and propagate blk_flush() errors rather than discarding them.
> +        if (ret < 0) {
> +            trace_vof_error_write(ihandle);
> +            return PROM_ERROR;
> +        }
> +        blk_flush(inst->blk);
> +        inst->pos += len;
> +        trace_vof_write(ihandle, len, "(disk)");
> +        return len;
> +    }
> +
>   #ifdef CONFIG_PSERIES
>       if (inst->vty) {
>           uint32_t total_written = 0;
> @@ -646,6 +703,26 @@ 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(len);
len comes from the OF client call, i.e. from the guest.

This means the guest can request an allocation up to UINT32_MAX before 
either the block layer or guest-memory access is attempted.

I don't think blk_pread()/blk_pwrite() can be relied on for validating 
this because the potentially huge host allocation has already happened 
before those calls.

Could we instead use a small bounded bounce buffer and process the 
request in chunks?

The maximum should preferably come from an existing QEMU block/I/O 
convention rather than an arbitrary large allocation.

Thanks,
Narayana Murty N.
> +        int ret;
> +
> +        ret = blk_pread(inst->blk, inst->pos, len, tmp, 0);
> +        if (ret < 0) {
> +            trace_vof_error_read(ihandle);
> +            return PROM_ERROR;
> +        }
> +
> +        if (VOF_MEM_WRITE(buf, tmp, len) != MEMTX_OK) {
> +            trace_vof_error_read(ihandle);
> +            return PROM_ERROR;
> +        }
> +
> +        inst->pos += len;
> +        trace_vof_read(ihandle, len, "(disk)");
> +        return len;
> +    }
> +
>   #ifdef CONFIG_PSERIES
>       if (inst->vty) {
>           uint8_t tmp[VOF_VTY_BUF_SIZE];
> @@ -675,6 +752,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;
> @@ -985,6 +1097,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)) {



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC PATCH 6/8] spapr_vscsi: add VOF disk nodes to the device tree
  2026-08-17 10:27 ` [RFC PATCH 6/8] spapr_vscsi: add VOF disk nodes to the device tree uverma
@ 2026-09-18  5:24   ` Narayana Murty N
  2026-09-25  6:41     ` Utkarsh Verma
  0 siblings, 1 reply; 25+ messages in thread
From: Narayana Murty N @ 2026-09-18  5:24 UTC (permalink / raw)
  To: uverma, qemu-devel, qemu-ppc, aik
  Cc: pbonzini, th.huth, sbhat, harshpb, amachhiw, rathc, balaton,
	philmd, npiggin, marcandre.lureau, fam, avnish

Hi Utkarsh,

On 17/08/26 3:57 PM, uverma@linux.ibm.com wrote:
> From: Utkarsh Verma <uverma@linux.ibm.com>
> 
> Populate the vSCSI device tree node with child disk nodes for VOF
> guests.
> 
> This lets Open Firmware clients such as GRUB open fully-qualified
> paths like /vdevice/v-scsi@.../disk@<srp-lun> for attached vSCSI
> disks.
> 
> Each child node is named from the encoded SRP LUN and includes a reg
> property with the 64-bit LUN value, along with device_type set to
> "block".
> 
> AI-used-for: code
> Signed-off-by: Utkarsh Verma <uverma@linux.ibm.com>
> ---
>   hw/scsi/spapr_vscsi.c | 40 ++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 40 insertions(+)
> 
> diff --git a/hw/scsi/spapr_vscsi.c b/hw/scsi/spapr_vscsi.c
> index b4c8f94d22..03f5f27f5f 100644
> --- a/hw/scsi/spapr_vscsi.c
> +++ b/hw/scsi/spapr_vscsi.c
> @@ -1243,6 +1243,8 @@ void spapr_vscsi_create(SpaprVioBus *bus)
>   
>   static int spapr_vscsi_devnode(SpaprVioDevice *dev, void *fdt, int node_off)
>   {
> +    VSCSIState *s = VIO_SPAPR_VSCSI_DEVICE(dev);
> +    BusChild *kid;
>       int ret;
>   
>       ret = fdt_setprop_cell(fdt, node_off, "#address-cells", 2);
> @@ -1255,6 +1257,44 @@ static int spapr_vscsi_devnode(SpaprVioDevice *dev, void *fdt, int node_off)
>           return ret;
>       }
>   
> +    /*
> +     * In VOF mode, add a child FDT node for each attached SCSI disk so that OF
> +     * clients (e.g. GRUB via VOF) can open a fully-qualified path like
> +     * /vdevice/v-scsi@.../disk@<srp-lun>.
> +     */
> +    SpaprMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
> +    if (spapr->vof) {
> +        QTAILQ_FOREACH(kid, &s->bus.qbus.children, sibling) {
> +            SCSIDevice *sdev = SCSI_DEVICE(kid->child);
> +            char disk_name[32];
> +            uint64_t srp_lun;
> +            uint32_t reg[2];
> +            int disk_off;
> +
> +            srp_lun = ((uint64_t)(0x8000 | (sdev->id << 8) |
> +                                   (sdev->channel << 5) | sdev->lun)) << 48;
This matches the decoder introduced in patch 5 for the corresponding bit 
ranges, but patch 8 constructs SRP LUNs using a different expression.

Avoid hard coded values, define or use existing relative MACRO definitions.

> +
> +            snprintf(disk_name, sizeof(disk_name), "disk@%"PRIx64, srp_lun);
> +
> +            disk_off = fdt_add_subnode(fdt, node_off, disk_name);
> +            if (disk_off < 0) {
> +                return disk_off;
> +            }
> +
> +            reg[0] = cpu_to_be32((uint32_t)(srp_lun >> 32));
> +            reg[1] = cpu_to_be32((uint32_t)(srp_lun & 0xFFFFFFFF));
> +            ret = fdt_setprop(fdt, disk_off, "reg", reg, sizeof(reg));
> +            if (ret < 0) {
> +                return ret;
> +            }
> +
> +            ret = fdt_setprop_string(fdt, disk_off, "device_type", "block");
> +            if (ret < 0) {
> +                return ret;
> +            }
> +        }
> +    }
> +
>       return 0;
>   }
Could we please introduce a single helper for SRP-LUN encoding and 
decoding and use it in patches 5, 6 and 8?

Something along the lines of:

spapr_vscsi_encode_srp_lun(channel, id, lun)
spapr_vscsi_decode_srp_lun(srplun, ...)

That would also make it straightforward to add tests for:

id      = 0, 1, 31, 32, 63
channel = 0 .. 7
lun     = 0, 1, 31

At the moment the common channel=0/id=0 case can hide inconsistencies 
between the three implementations.


Thanks,
Narayana Murty N.
>   



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC PATCH 7/8] ppc/spapr: strip OF path argument suffix in path_offset
  2026-08-17 10:27 ` [RFC PATCH 7/8] ppc/spapr: strip OF path argument suffix in path_offset uverma
@ 2026-09-18  5:25   ` Narayana Murty N
  2026-09-25  6:45     ` Utkarsh Verma
  0 siblings, 1 reply; 25+ messages in thread
From: Narayana Murty N @ 2026-09-18  5:25 UTC (permalink / raw)
  To: uverma, qemu-devel, qemu-ppc, aik
  Cc: pbonzini, th.huth, sbhat, harshpb, amachhiw, rathc, balaton,
	philmd, npiggin, marcandre.lureau, fam, avnish

Hi Utkarsh,

On 17/08/26 3:57 PM, uverma@linux.ibm.com wrote:
> From: Utkarsh Verma <uverma@linux.ibm.com>
> 
> OF paths can include a ":argument" suffix on the last component (e.g.
> "disk@8000000000000000:0"), which is an OF path argument such as a
> partition number.  This is not part of the FDT node name and causes
> fdt_path_offset() to fail.
> 
> Strip any ":<suffix>" from the last path component only, leaving
> intermediate components untouched.
> 
> AI-used-for: code
> Signed-off-by: Utkarsh Verma <uverma@linux.ibm.com>
> ---
>   hw/ppc/vof.c | 16 ++++++++++++++++
>   1 file changed, 16 insertions(+)
> 
> diff --git a/hw/ppc/vof.c b/hw/ppc/vof.c
> index b1c574aabf..d3b0df30e4 100644
> --- a/hw/ppc/vof.c
> +++ b/hw/ppc/vof.c
> @@ -142,6 +142,8 @@ static int path_offset(const void *fdt, const char *path)
>   {
>       g_autofree char *p = NULL;
>       char *at;
> +    char *last_slash;
> +    char *colon;
>   
>       /*
>        * https://www.devicetree.org/open-firmware/bindings/ppc/release/ppc-2_1.html#HDR16
> @@ -151,6 +153,20 @@ static int path_offset(const void *fdt, const char *path)
>        * suppressing leading zeros".
>        */
>       p = g_strdup(path);
> +
> +    /*
> +     * Strip any ":argument" suffix from the last path component (e.g.
> +     * "disk@8000000000000000:0") — it is an OF path argument, not part of
> +     * the FDT node name.
> +     */
> +    last_slash = strrchr(p, '/');
> +    if (last_slash) {
> +        colon = strchr(last_slash, ':');
> +        if (colon) {
> +            *colon = '\0';
> +        }
> +    }
Is path_offset() guaranteed to always receive an absolute path?

If not, paths without /, such as disk@8000000000000000:0, won't have the 
suffix stripped. Either document the absolute-path requirement or handle 
the no-slash case as well.

Thanks,
Narayana Murty N
> +
>       for (at = strchr(p, '@'); at && *at; ) {
>               if (*at == '/') {
>                   at = strchr(at, '@');



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC PATCH 8/8] ppc/spapr: implement vscsi-report-luns call-method for PAPR vSCSI
  2026-08-17 10:27 ` [RFC PATCH 8/8] ppc/spapr: implement vscsi-report-luns call-method for PAPR vSCSI uverma
  2026-09-18  5:18   ` Narayana Murty N
@ 2026-09-18  5:26   ` Narayana Murty N
  1 sibling, 0 replies; 25+ messages in thread
From: Narayana Murty N @ 2026-09-18  5:26 UTC (permalink / raw)
  To: uverma, qemu-devel, qemu-ppc, aik
  Cc: pbonzini, th.huth, sbhat, harshpb, amachhiw, rathc, balaton,
	philmd, npiggin, marcandre.lureau, fam, avnish

Hi Utkarsh,

On 17/08/26 3:57 PM, uverma@linux.ibm.com wrote:
> From: Utkarsh Verma <uverma@linux.ibm.com>
> 
> GRUB (grub-core/disk/ieee1275/ofdisk.c) calls "vscsi-report-luns" on
> any PAPR virtual SCSI controller ihandle to enumerate attached LUNs
> before attempting to boot from disk. Without this method VOF returns
> PROM_ERROR and GRUB cannot find vSCSI disks.
> 
> Add vof_find_vscsi_bus() to locate the SCSIBus from a v-scsi@<reg>
> instance path, and vof_vscsi_report_luns() to build the response table
> in the VOF firmware region (0..fw_size), which GRUB never claims.
> 
> The table layout and SRP LUN encoding follow SLOF (vio-vscsi.fs
> dev-generate-srplun) each entry is an 8-byte big-endian cell whose
> low 4 bytes hold a guest pointer to a null-terminated list of SRP LUNs
> for that target; GRUB reads the pointer at offset table + 4 + 8*i.
> 
> AI-used-for: code
> Signed-off-by: Utkarsh Verma <uverma@linux.ibm.com>
> ---
>   hw/ppc/vof.c | 212 +++++++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 212 insertions(+)
> 
> diff --git a/hw/ppc/vof.c b/hw/ppc/vof.c
> index d3b0df30e4..b1bf5b1af0 100644
> --- a/hw/ppc/vof.c
> +++ b/hw/ppc/vof.c
> @@ -991,6 +991,198 @@ static void vof_instantiate_rtas(Error **errp)
>       error_setg(errp, "The firmware should have instantiated RTAS");
>   }
>   
> +#ifdef CONFIG_PSERIES
> +/* Combined (channel<<6)|id index space, matches SLOF dev-max-target. */
> +#define VSCSI_MAX_TARGETS 512
> +
> +/*
> + * Build the vscsi-report-luns response in guest memory.
> + *
> + * Returns catch_result (0 = success).  On success, *nentries_out and
> + * *table_addr_out describe the result table written into the VOF firmware
> + * region (0..fw_size), which GRUB never claims.
> + *
> + * Table layout (big-endian, 8 bytes per entry):
> + *   [table_addr + 8*i + 0..3] : 0x00000000
> + *   [table_addr + 8*i + 4..7] : guest pointer to null-terminated SRP LUN list
> + *
> + * GRUB reads each pointer as: *(uint32_t *)(table + 4 + 8 * i)
> + *
> + * SRP LUN encoding (SLOF dev-generate-srplun):
> + *   srplun = (0x8000 | bus | target | lun) << 48
> + *   where bus    = (combined_target >> 1) & 0x70
> + *         target = (combined_target & 0x3f) << 8
> + *         combined_target = (channel << 6) | id
> + */
> +static uint32_t vof_vscsi_report_luns(Vof *vof,
> +                                       SCSIBus *sbus,
> +                                       uint32_t *nentries_out,
> +                                       uint32_t *table_addr_out)
> +{
> +    uint64_t *lun_lists[VSCSI_MAX_TARGETS];
> +    int lun_counts[VSCSI_MAX_TARGETS];
> +    uint32_t ptr_table_size;
> +    uint32_t lun_data_size;
> +    uint32_t total_size;
> +    uint32_t base_addr;
> +    uint32_t lun_data_base;
> +    uint32_t lun_data_off;
> +    uint32_t table_idx;
> +    uint32_t table_addr;
> +    BusChild *kid;
> +    int nentries = 0;
> +    int t;
> +
> +    memset(lun_lists, 0, sizeof(lun_lists));
> +    memset(lun_counts, 0, sizeof(lun_counts));
> +
> +    QTAILQ_FOREACH(kid, &sbus->qbus.children, sibling) {
> +        SCSIDevice *dev = SCSI_DEVICE(kid->child);
> +        int combined_target;
> +        uint64_t bus_field;
> +        uint64_t target_field;
> +        uint64_t lun_field;
> +        uint64_t srplun;
> +        int cnt;
> +
> +        /* combined_target = (channel << 6) | id, matching SLOF bus+target */
> +        combined_target = ((dev->channel & 0x7) << 6) | (dev->id & 0x3f);
> +        if (combined_target >= VSCSI_MAX_TARGETS) {
> +            continue;
> +        }
> +
> +        bus_field    = ((uint64_t)combined_target >> 1) & 0x70ULL;
> +        target_field = ((uint64_t)combined_target & 0x3fULL) << 8;
> +        lun_field    = (uint64_t)(dev->lun & 0x1f);
> +        srplun = (0x8000ULL | bus_field | target_field | lun_field) << 48;
> +
> +        cnt = lun_counts[combined_target];
> +        lun_lists[combined_target] = g_realloc(lun_lists[combined_target],
> +                                               (cnt + 2) * sizeof(uint64_t));
> +        lun_lists[combined_target][cnt]     = cpu_to_be64(srplun);
> +        lun_lists[combined_target][cnt + 1] = 0;
> +        lun_counts[combined_target]++;
> +        if (cnt == 0) {
> +            nentries++;
> +        }
> +    }
> +
> +    if (nentries == 0) {
> +        *nentries_out   = 0;
> +        *table_addr_out = 0;
> +        return 0;
> +    }
> +
> +    ptr_table_size = nentries * sizeof(uint64_t);
> +    lun_data_size  = 0;
> +    for (t = 0; t < VSCSI_MAX_TARGETS; t++) {
> +        if (lun_lists[t]) {
> +            lun_data_size += (lun_counts[t] + 1) * sizeof(uint64_t);
> +        }
> +    }
> +    total_size = ptr_table_size + lun_data_size;
> +
> +    /*
> +     * Place the table in the VOF firmware region (0..fw_size), which GRUB
> +     * never claims.  Fit into the top of the first 4 KB page, 8-byte aligned.
> +     */
> +    if (total_size <= 0x1000 - (uint32_t)vof->fw_size) {
> +        base_addr = (0x1000 - total_size) & ~7U;
> +    } else {
> +        base_addr = 0x800;
> +    }
> +
I don't think the fallback to 0x800 is safe.

The VOF allocation map documented in spapr_vof_reset() currently has the 
initial firmware at approximately:

0 .. 0xc38

so 0x800 lies inside that firmware image.

For a sufficiently large LUN report, this branch therefore appears to 
write the table over active firmware memory.
> +    table_addr    = base_addr;
> +    lun_data_base = base_addr + ptr_table_size;
> +    lun_data_off  = 0;
> +    table_idx     = 0;
> +
> +    for (t = 0; t < VSCSI_MAX_TARGETS; t++) {
> +        uint32_t lun_buf_size;
> +        uint32_t lun_buf_addr;
> +        uint64_t cell_be64;
> +
> +        if (!lun_lists[t]) {
> +            continue;
> +        }
> +
> +        lun_buf_size = (lun_counts[t] + 1) * sizeof(uint64_t);
> +        lun_buf_addr = lun_data_base + lun_data_off;
> +
> +        if (VOF_MEM_WRITE(lun_buf_addr, lun_lists[t], lun_buf_size)
> +                != MEMTX_OK) {
> +            goto write_err;
> +        }
> +
> +        cell_be64 = cpu_to_be64((uint64_t)lun_buf_addr);
> +        if (VOF_MEM_WRITE(table_addr + table_idx * sizeof(uint64_t),
> +                          &cell_be64, sizeof(cell_be64)) != MEMTX_OK) {
> +            goto write_err;
> +        }
> +
> +        lun_data_off += lun_buf_size;
> +        table_idx++;
> +    }
> +
> +    *nentries_out   = table_idx;
> +    *table_addr_out = table_addr;
> +
> +    for (t = 0; t < VSCSI_MAX_TARGETS; t++) {
> +        g_free(lun_lists[t]);
> +    }
> +    return 0;
> +
> +write_err:
> +    for (t = 0; t < VSCSI_MAX_TARGETS; t++) {
> +        g_free(lun_lists[t]);
> +    }
> +    return PROM_ERROR;
> +}
> +
> +/*
> + * Return the SCSIBus for a v-scsi@<reg> path, or NULL if not found.
> + * Handles both "/vdevice/v-scsi@<reg>" and ".../v-scsi@<reg>/disk@..." paths.
> + */
> +static SCSIBus *vof_find_vscsi_bus(MachineState *ms, const char *path)
> +{
> +    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);
> +        }
> +    }
> +    return NULL;
> +}
> +#endif /* CONFIG_PSERIES */
> +
>   static uint32_t vof_call_method(MachineState *ms, Vof *vof, uint32_t methodaddr,
>                                   uint32_t ihandle, uint32_t param1,
>                                   uint32_t param2, uint32_t param3,
> @@ -1014,6 +1206,26 @@ static uint32_t vof_call_method(MachineState *ms, Vof *vof, uint32_t methodaddr,
>           goto trace_exit;
>       }
>   
> +#ifdef CONFIG_PSERIES
> +    /* vscsi-report-luns: enumerate LUNs for GRUB */
> +    if (strcmp(method, "vscsi-report-luns") == 0) {
> +        SCSIBus *sbus = vof_find_vscsi_bus(ms, inst->path);
> +
> +        if (sbus) {
> +            uint32_t nentries = 0, table_addr = 0;
> +            ret = vof_vscsi_report_luns(vof, sbus, &nentries, &table_addr);
> +            ret2[0] = nentries;
> +            ret2[1] = table_addr;
> +        } else {
> +            ret = 1;
> +            ret2[0] = 0;
> +            ret2[1] = 0;
> +        }
> +        trace_vof_method(ihandle, method, param1, ret, ret2[0]);
> +        goto trace_exit;
> +    }
> +#endif /* CONFIG_PSERIES */
> +
>       if (strcmp(inst->path, "/") == 0) {
>           if (strcmp(method, "ibm,client-architecture-support") == 0) {
>               Object *vmo = object_dynamic_cast(OBJECT(ms), TYPE_VOF_MACHINE_IF);



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC PATCH 1/8] ppc/spapr: add PReP boot partition detection
  2026-09-17 17:14   ` Narayana Murty N
@ 2026-09-25  6:19     ` Utkarsh Verma
  0 siblings, 0 replies; 25+ messages in thread
From: Utkarsh Verma @ 2026-09-25  6:19 UTC (permalink / raw)
  To: Narayana Murty N, qemu-devel, qemu-ppc, aik
  Cc: pbonzini, th.huth, sbhat, harshpb, amachhiw, rathc, balaton,
	philmd, npiggin, marcandre.lureau, fam

Hi Narayana,

On 9/17/26 10:44 PM, Narayana Murty N wrote:
> 
> 
> On 17/08/26 3:57 PM, uverma@linux.ibm.com wrote:
>> From: Utkarsh Verma <uverma@linux.ibm.com>
<snip>
>> +    entries_lba = le64_to_cpu(header->partition_entries_lba);
>> +    num_entries = le32_to_cpu(header->num_partition_entries);
>> +    entry_size = le32_to_cpu(header->partition_entry_size);
>> +
>> +    if (num_entries > 128) {
>> +        num_entries = 128;
>> +    }
> Why is silently truncating to 128 the right behaviour?
> 
> This bounds the loop, which is good, but it also means that a valid PReP 
> partition after entry 127 will not be found. If 128 is an intentional 
> VOF limitation, can we document that? Otherwise it may be better to 
> validate the table size and iterate the actual number of entries subject 
> to an explicit resource limit.

The PReP is usually the first partition in a disk and systems usually 
have max 128 partitions, though GPT spec doesn't have any upper limit. 
So, I capped the limit to 128, will add a comment here for this in the 
v2. I don't think iterating beyond 128 is necessary, but I'm open to 
changing this.

>> +
>> +    if (entry_size < 128) {
>> +        return false;
>> +    }
>> +
>> +    buf = g_malloc(entry_size);
> The "entry_size" comes directly from the GPT header, so checking only 
> the lower bound means a malformed image can drive an arbitrarily large 
> host allocation here.
> 
> Do we need to allocate "entry_size" bytes at all?
> 
> We only consume the fixed fields at the beginning of the GPT entry. It 
> seems safer to use `entry_size` only as the stride between entries and 
> read `sizeof(GPTPartitionEntry)` bytes for each entry after checking 
> that `entry_size >= sizeof(GPTPartitionEntry)`.
> 
> That also avoids having to invent an arbitrary maximum GPT entry size.
> 
Sure, will fix the allocation of buf to sizeof(GPTPartitionEntry) in v2.

>> +    for (i = 0; i < num_entries; i++) {
>> +        current_entry_offset = (entries_lba * SECTOR_SIZE) +
>> +                               ((uint64_t)i * entry_size);
> Both the multiplication and addition are based on image-controlled 
> fields. Could we validate the complete partition-entry array against 
> blk_getlength() using checked arithmetic before iterating it?

Sure, will fix in v2.

>> +
>> +        ret = blk_pread(blk, current_entry_offset, entry_size, buf, 0);
>> +        if (ret < 0) {
>> +            warn_report("GPT: failed to read partition entry %u "
>> +                        "(ret=%d)", i, ret);
>> +            g_free(buf);
>> +            return false;
>> +        }
>> +
>> +        current_entry = (GPTPartitionEntry *)buf;
>> +        if (memcmp(current_entry->type_guid, GUID_PREP_BOOT,
>> +                   sizeof(GUID_PREP_BOOT)) == 0) {
>> +            *offset = le64_to_cpu(current_entry->first_lba) * 
>> SECTOR_SIZE;
>> +            *size = (le64_to_cpu(current_entry->last_lba) -
>> +                     le64_to_cpu(current_entry->first_lba) + 1) * 
>> SECTOR_SIZE;
> I think this needs validation before returning the partition.
> 
> If last_lba < first_lba, the subtraction underflows and produces a very 
> large partition size. There are also multiplication/partition-end 
> overflow cases.
> 
> I'd expect something along the lines of:
>      first_lba <= last_lba
>      first_lba <= disk_last_lba
>      last_lba <= disk_last_lba

Sure, will add these checks in v2.
<snip>
>> +typedef struct BlockBackend BlockBackend;
>> +
>> +bool spapr_vof_find_prep_partition(BlockBackend *blk,
>> +                                   uint64_t *offset, uint64_t *size);
>> +
>> +#endif /* HW_SPAPR_VOF_H */
> 
> we parse header_size, CRC fields, usable-LBA fields, etc. into 
> GPTHeader, but only check the signature.
> 
> I don't think CRC validation should replace range validation, but at 
> least the header fields which determine the layout we consume should 
> probably be validated before trusting the partition table.
> 
> check CRC validation posibilities .
> 
Sure, will add the CRC validation for GPT header and partition entry 
array in v2.
It's worth noting that SLOF doesn't have many of these validations, but 
will add these additional sanity checks in VOF.

> Thanks,
> Narayana Murty N
> 
> 

Regards,
Utkarsh



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC PATCH 2/8] hw/loader: add load_elf_ram_sym_buf() for in-memory ELF loading
  2026-09-17 17:14   ` Narayana Murty N
@ 2026-09-25  6:24     ` Utkarsh Verma
  0 siblings, 0 replies; 25+ messages in thread
From: Utkarsh Verma @ 2026-09-25  6:24 UTC (permalink / raw)
  To: Narayana Murty N, qemu-devel, qemu-ppc, aik
  Cc: pbonzini, th.huth, sbhat, harshpb, amachhiw, rathc, balaton,
	philmd, npiggin, marcandre.lureau, fam, avnish



On 9/17/26 10:44 PM, Narayana Murty N wrote:
> 
> 
> On 17/08/26 3:57 PM, uverma@linux.ibm.com wrote:
>> From: Utkarsh Verma <uverma@linux.ibm.com>
>>
>> Add a new public API load_elf_ram_sym_buf() that loads an ELF
>> image from a caller-supplied memory buffer rather than a file path.
>> The implementation writes the buffer to an anonymous memfd, then
>> reuses the existing load_elf32/load_elf64 paths.
>>
>> To support per-segment inspection by callers (e.g. detecting VoF
>> firmware segments), extend the internal load_elf{32,64} template
>> (elf_ops.h.inc) with an optional segment_fn_t callback that is
>> invoked once for every PT_LOAD segment after its load address is
>> resolved. Returning false from the callback aborts the load with
>> ELF_LOAD_FAILED.
>>
>> AI-used-for: code
>> Signed-off-by: Utkarsh Verma <uverma@linux.ibm.com>
>> ---
>>   hw/core/loader.c         | 77 ++++++++++++++++++++++++++++++++++++++--
>>   include/hw/core/loader.h | 30 ++++++++++++++++
>>   include/hw/elf_ops.h.inc | 12 ++++++-
>>   3 files changed, 116 insertions(+), 3 deletions(-)
>>
>> diff --git a/hw/core/loader.c b/hw/core/loader.c
>> index 5cbfba0a86..4e4e9764a3 100644
>> --- a/hw/core/loader.c
>> +++ b/hw/core/loader.c
>> @@ -43,6 +43,7 @@
>>    */
>>   #include "qemu/osdep.h"
>> +#include "qemu/memfd.h"
>>   #include "qemu/datadir.h"
>>   #include "qemu/error-report.h"
>>   #include "qapi/error.h"
>> @@ -510,12 +511,14 @@ ssize_t load_elf_ram_sym(const char *filename,
>>           ret = load_elf64(filename, fd, elf_note_fn,
>>                            translate_fn, translate_opaque, must_swab,
>>                            pentry, lowaddr, highaddr, pflags, 
>> elf_machine,
>> -                         clear_lsb, data_swab, as, load_rom, sym_cb);
>> +                         clear_lsb, data_swab, as, load_rom, sym_cb,
>> +                         NULL, NULL);
>>       } else {
>>           ret = load_elf32(filename, fd, elf_note_fn,
>>                            translate_fn, translate_opaque, must_swab,
>>                            pentry, lowaddr, highaddr, pflags, 
>> elf_machine,
>> -                         clear_lsb, data_swab, as, load_rom, sym_cb);
>> +                         clear_lsb, data_swab, as, load_rom, sym_cb,
>> +                         NULL, NULL);
>>       }
>>       if (ret > 0) {
>> @@ -527,6 +530,76 @@ ssize_t load_elf_ram_sym(const char *filename,
>>       return ret;
>>   }
>> +ssize_t load_elf_ram_sym_buf(const uint8_t *buf, size_t buflen,
>> +                              uint64_t (*elf_note_fn)(void *, void *, 
>> bool),
>> +                              uint64_t (*translate_fn)(void *, 
>> uint64_t),
>> +                              void *translate_opaque, uint64_t *pentry,
>> +                              uint64_t *lowaddr, uint64_t *highaddr,
>> +                              uint32_t *pflags, int elf_data_order,
>> +                              int elf_machine, int clear_lsb, int 
>> data_swab,
>> +                              AddressSpace *as, bool load_rom,
>> +                              symbol_fn_t sym_cb,
>> +                              segment_fn_t segment_fn, void 
>> *segment_opaque)
>> +{
>> +    const int host_data_order = HOST_BIG_ENDIAN ? ELFDATA2MSB : 
>> ELFDATA2LSB;
>> +    int fd, must_swab;
>> +    ssize_t ret = ELF_LOAD_FAILED;
>> +    uint8_t e_ident[EI_NIDENT];
>> +
>> +    fd = memfd_create("qemu-elf-buf", MFD_CLOEXEC);
> 
> The memfd_create() is Linux-specific.
> 
> qemu/memfd.h only exposes the direct memfd_create() declaration for 
> Linux configurations, so this also appears to break non-Linux system 
> builds.
> 
> Could this use an existing portable QEMU temporary-fd abstraction, or 
> alternatively could the ELF loader be factored so the common parser 
> accepts an fd/buffer directly?
> 
> It seems undesirable for a generic loader API to require Linux memfd 
> support.
> 
> OR
> 
> If it is an arch specific design decision, then we need to document it.
> 
Yes, it breaks the non-Linux builds, I tried exploring the QEMU 
temporary-fd options, but all the approaches will make multiple copies 
of the same buffer. So, instead will add the logic to directly work on 
the buffer avoiding any fd, tempfile or extra copies. Though this will 
require new templates load_elf_buf32 and load_elf_buf64 implementation 
in elf_ops.h.inc. Will implement in v2.

>> +    if (fd < 0) {
>> +        error_report("load_elf_ram_sym_buf: memfd_create: %s", 
>> strerror(errno));
>> +        return ELF_LOAD_FAILED;
>> +    }
>> +
>> +    if (write(fd, buf, buflen) != (ssize_t)buflen) {
> 
> A single write() is not guaranteed to consume the whole buffer and can 
> be interrupted.
> 
> Also, if buflen > SSIZE_MAX, the cast used for comparison is problematic.
> 
> Please use the appropriate QEMU full-write helper (or loop handling 
> short writes/EINTR).
> 
This write won't be required in v2 with the direct buffer approach.

>> +        error_report("load_elf_ram_sym_buf: write: %s", 
>> strerror(errno));
>> +        goto fail;
>> +    }
>> +
>> +    lseek(fd, 0, SEEK_SET);
>> +    if (read(fd, e_ident, sizeof(e_ident)) != sizeof(e_ident)) {
>> +        goto fail;
>> +    }
>> +    if (e_ident[0] != ELFMAG0 ||
>> +        e_ident[1] != ELFMAG1 ||
>> +        e_ident[2] != ELFMAG2 ||
>> +        e_ident[3] != ELFMAG3) {
>> +        ret = ELF_LOAD_NOT_ELF;
>> +        goto fail;
>> +    }
>> +
>> +    if (elf_data_order != ELFDATANONE && elf_data_order != 
>> e_ident[EI_DATA]) {
>> +        ret = ELF_LOAD_WRONG_ENDIAN;
>> +        goto fail;
>> +    }
>> +
>> +    must_swab = host_data_order != e_ident[EI_DATA];
>> +
>> +    lseek(fd, 0, SEEK_SET);
> 
> both lseek call errors need to be handled.
> 
Actually all this logic was copied from the load_elf_ram_sym function. 
After creating the fd in this function, all the logic is taken as-is 
from the load_elf_ram_sym() and this unchecked lseek in load_elf_ram_sym 
has been there since the beginning. This lseek is done after 
successfully calling the open and read, so I don't think this could 
fail, which is why the load_elf_ram_sym also has this unchecked. But 
anyhow will add a sanity check in this load_elf_ram_sym_buf in v2.

>> +    if (e_ident[EI_CLASS] == ELFCLASS64) {
>> +        ret = load_elf64("(buffer)", fd, elf_note_fn,
>> +                         translate_fn, translate_opaque, must_swab,
>> +                         pentry, lowaddr, highaddr, pflags, elf_machine,
>> +                         clear_lsb, data_swab, as, load_rom, sym_cb,
>> +                         segment_fn, segment_opaque);
>> +    } else {
>> +        ret = load_elf32("(buffer)", fd, elf_note_fn,
>> +                         translate_fn, translate_opaque, must_swab,
>> +                         pentry, lowaddr, highaddr, pflags, elf_machine,
>> +                         clear_lsb, data_swab, as, load_rom, sym_cb,
>> +                         segment_fn, segment_opaque);
>> +    }
>> +
>> +    if (ret > 0) {
>> +        debuginfo_report_elf("(buffer)", fd, 0);
>> +    }
>> +
>> + fail:
>> +    close(fd);
>> +    return ret;
>> +}
>> +
>>   static void bswap_uboot_header(uboot_image_header_t *hdr)
>>   {
>>   #if !HOST_BIG_ENDIAN
>> diff --git a/include/hw/core/loader.h b/include/hw/core/loader.h
>> index d9431e8a8d..b67b2ca013 100644
>> --- a/include/hw/core/loader.h
>> +++ b/include/hw/core/loader.h
>> @@ -156,6 +156,36 @@ ssize_t load_elf_ram_sym(const char *filename,
>>                            int clear_lsb, int data_swab,
>>                            AddressSpace *as, bool load_rom, 
>> symbol_fn_t sym_cb);
>> +/*
>> + * segment_fn_t:
>> + * Per-PT_LOAD segment callback for load_elf_ram_sym_buf().
>> + */
>> +typedef bool (*segment_fn_t)(void *opaque,
>> +                             uint64_t paddr, uint64_t vaddr,
>> +                             uint64_t filesz, uint64_t memsz);
>> +
>> +/*
>> + * load_elf_ram_sym_buf:
>> + * @buf: pointer to an in-memory ELF image
>> + * @buflen: size of @buf in bytes
>> + * @segment_fn: optional per-PT_LOAD callback
>> + * @segment_opaque: opaque data passed to @segment_fn
>> + *
>> + * Identical to load_elf_ram_sym() but loads from a caller-supplied
>> + * memory buffer instead of a file.  All other parameters have the
>> + * same meaning as load_elf_ram_sym().
>> + */
>> +ssize_t load_elf_ram_sym_buf(const uint8_t *buf, size_t buflen,
>> +                             uint64_t (*elf_note_fn)(void *, void *, 
>> bool),
>> +                             uint64_t (*translate_fn)(void *, uint64_t),
>> +                             void *translate_opaque, uint64_t *pentry,
>> +                             uint64_t *lowaddr, uint64_t *highaddr,
>> +                             uint32_t *pflags, int elf_data_order,
>> +                             int elf_machine, int clear_lsb, int 
>> data_swab,
>> +                             AddressSpace *as, bool load_rom,
>> +                             symbol_fn_t sym_cb,
>> +                             segment_fn_t segment_fn, void 
>> *segment_opaque);
>> +
>>   /** load_elf_as:
>>    * Same as load_elf_ram_sym(), but always loads the elf as ROM
>>    */
>> diff --git a/include/hw/elf_ops.h.inc b/include/hw/elf_ops.h.inc
>> index 044e72de2a..a2c9b52bf9 100644
>> --- a/include/hw/elf_ops.h.inc
>> +++ b/include/hw/elf_ops.h.inc
>> @@ -321,7 +321,9 @@ static ssize_t glue(load_elf, SZ)(const char 
>> *name, int fd,
>>                                     uint32_t *pflags, int elf_machine,
>>                                     int clear_lsb, int data_swab,
>>                                     AddressSpace *as, bool load_rom,
>> -                                  symbol_fn_t sym_cb)
>> +                                  symbol_fn_t sym_cb,
>> +                                  segment_fn_t segment_fn,
>> +                                  void *segment_opaque)
>>   {
>>       struct elfhdr ehdr;
>>       struct elf_phdr *phdr = NULL, *ph;
>> @@ -504,6 +506,14 @@ static ssize_t glue(load_elf, SZ)(const char 
>> *name, int fd,
>>                   addr = ph->p_paddr;
>>               }
>> +            if (segment_fn) {
>> +                if (!segment_fn(segment_opaque, addr, ph->p_vaddr,
>> +                                file_size, mem_size)) {
>> +                    ret = ELF_LOAD_FAILED;
>> +                    goto fail;
>> +                }
>> +            }
>> +
>>               if (data_swab) {
>>                   elf_word j;
>>                   for (j = 0; j < file_size; j += (1 << data_swab)) {
> 
> Thanks,
> Narayana Murty N
> 
> 

Regards,
Utkarsh



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC PATCH 3/8] ppc/spapr: add baseline VOF disk boot support
  2026-09-17 17:33   ` Narayana Murty N
@ 2026-09-25  6:28     ` Utkarsh Verma
  0 siblings, 0 replies; 25+ messages in thread
From: Utkarsh Verma @ 2026-09-25  6:28 UTC (permalink / raw)
  To: Narayana Murty N, qemu-devel, qemu-ppc, aik
  Cc: pbonzini, th.huth, sbhat, harshpb, amachhiw, rathc, balaton,
	philmd, npiggin, marcandre.lureau, fam, avnish



On 9/17/26 11:03 PM, Narayana Murty N wrote:
> 
> 
> 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.
> 
Will add a sanity check in v2. Something like:
partition_size > 0 && \
partition_size <= 10 MB (PReP partition is usually 4–8 MB) && \
partition_size <= blk_getlength(blk)

>> +        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.
> 
Will fix in v2. I will add vof_release() which will free the entries in 
the vof->claimed map added during a failed attempt.

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

Regards,
Utkarsh



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC PATCH 5/8] ppc/spapr: add block device backend to VOF open/read/write/seek services
  2026-09-18  5:23   ` Narayana Murty N
@ 2026-09-25  6:33     ` Utkarsh Verma
  0 siblings, 0 replies; 25+ messages in thread
From: Utkarsh Verma @ 2026-09-25  6:33 UTC (permalink / raw)
  To: Narayana Murty N, qemu-devel, qemu-ppc, aik
  Cc: pbonzini, th.huth, sbhat, harshpb, amachhiw, rathc, balaton,
	philmd, npiggin, marcandre.lureau, fam, avnish



On 9/18/26 10:53 AM, Narayana Murty N wrote:
> Hi Utkarsh,
> 
> On 17/08/26 3:57 PM, 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 FDT nodes named "disk@<srp-lun>" and resolve
>> the SRP LUN encoding to the matching SCSIDevice/BlockBackend by scanning
>> id/channel/lun.
>>
>> 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>
>> ---
>>   hw/ppc/trace-events |   2 +
>>   hw/ppc/vof.c        | 116 +++++++++++++++++++++++++++++++++++++++++++-
>>   2 files changed, 117 insertions(+), 1 deletion(-)
>>
>> 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 45c197df9f..b1c574aabf 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/timer.h"
>>   #include "qemu/range.h"
>>   #include "qemu/units.h"
>> @@ -24,6 +25,8 @@
>>   #include "trace.h"
>>   #include "hw/ppc/spapr_vio.h"
>> +#include "hw/scsi/scsi.h"
>> +#include "system/block-backend.h"
>>   #include <libfdt.h>
>>   /*
>> @@ -46,6 +49,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;
>> @@ -449,6 +454,7 @@ 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;
>>       if (vof->of_instance_last == 0xFFFFFFFF) {
>>           /* We do not recycle ihandles yet */
>> @@ -461,10 +467,42 @@ 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);
>> +
>> +    if (node_name && strncmp(node_name, "disk@", 5) == 0) {
>> +        uint64_t srp_lun;
>> +        uint32_t id, channel, lun;
>> +        BlockBackend *blk;
>> +
>> +        if (qemu_strtou64(node_name + 5, NULL, 16, &srp_lun) == 0) {
>> +            id      = (srp_lun >> 56) & 0x3f;
>> +            channel = (srp_lun >> 53) & 0x7;
>> +            lun     = (srp_lun >> 48) & 0x1f;
>> +
>> +            for (blk = blk_next(NULL); blk; blk = blk_next(blk)) {
>> +                DeviceState *attached = blk_get_attached_dev(blk);
>> +                SCSIDevice *sdev;
>> +
>> +                if (!attached) {
>> +                    continue;
>> +                }
>> +                sdev = (SCSIDevice 
>> *)object_dynamic_cast(OBJECT(attached),
>> +                                                         
>> TYPE_SCSI_DEVICE);
>> +                if (sdev && sdev->id == (int)id &&
>> +                    sdev->channel == (int)channel &&
>> +                    sdev->lun == (int)lun) {
>> +                    inst->blk = blk;
>> +                    break;
>> +                }
>> +            }
> Shouldn't vof_do_open() first resolve that specific vSCSI controller/ 
> SCSIBus and then find the SCSIDevice on that bus, rather than searching 
> the global BlockBackend list?
> 
> Patch 8 already has code for resolving a v-scsi path to its SCSIBus; it 
> seems like the block-open path should use the same topology-aware lookup.
> 
Sorry, my bad. Will use the same vof_find_vscsi_bus() function to 
resolve the vSCSI bus and search for the matching id, channel and lun on 
that bus. Will fix in v2.

>> +        }
>> +    }
>> +
>>   #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());
>> @@ -601,6 +639,25 @@ static uint32_t vof_write(Vof *vof, uint32_t 
>> ihandle, uint32_t buf,
>>           return PROM_ERROR;
>>       }
>> +    if (inst->blk) {
>> +        g_autofree uint8_t *blkbuf = g_malloc(len);
>> +        int ret;
>> +
>> +        if (VOF_MEM_READ(buf, blkbuf, len) != MEMTX_OK) {
>> +            trace_vof_error_write(ihandle);
>> +            return PROM_ERROR;
>> +        }
>> +        ret = blk_pwrite(inst->blk, inst->pos, len, blkbuf, 0);
> Does VOF disk boot require block-device writes at all?
> 
> The series description talks about GRUB discovering the disk and reading 
> the kernel/initrd. If writable OF disk instances are not required for 
> boot, could we avoid exposing disk writes entirely?
> 
> If write support is required, this should also honour read-only backend 
> state and propagate blk_flush() errors rather than discarding them.

I added the write functionality because grub can record the selected 
menuentry in the saved_entry of grubenv file which requires a write 
operation, also editing the files from GRUB prompt requires the write 
functionality.
But will honour the backend's read-only state before attempting 
blk_pwrite(), and propagate blk_flush() errors through trace messages in v2.

>> +        if (ret < 0) {
>> +            trace_vof_error_write(ihandle);
>> +            return PROM_ERROR;
>> +        }
>> +        blk_flush(inst->blk);
>> +        inst->pos += len;
>> +        trace_vof_write(ihandle, len, "(disk)");
>> +        return len;
>> +    }
>> +
>>   #ifdef CONFIG_PSERIES
>>       if (inst->vty) {
>>           uint32_t total_written = 0;
>> @@ -646,6 +703,26 @@ 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(len);
> len comes from the OF client call, i.e. from the guest.
> 
> This means the guest can request an allocation up to UINT32_MAX before 
> either the block layer or guest-memory access is attempted.
> 
> I don't think blk_pread()/blk_pwrite() can be relied on for validating 
> this because the potentially huge host allocation has already happened 
> before those calls.
> 
> Could we instead use a small bounded bounce buffer and process the 
> request in chunks?
> 
> The maximum should preferably come from an existing QEMU block/I/O 
> convention rather than an arbitrary large allocation.
> 
Will fix in v2.

> Thanks,
> Narayana Murty N.
>> +        int ret;
>> +
>> +        ret = blk_pread(inst->blk, inst->pos, len, tmp, 0);
>> +        if (ret < 0) {
>> +            trace_vof_error_read(ihandle);
>> +            return PROM_ERROR;
>> +        }
>> +
>> +        if (VOF_MEM_WRITE(buf, tmp, len) != MEMTX_OK) {
>> +            trace_vof_error_read(ihandle);
>> +            return PROM_ERROR;
>> +        }
>> +
>> +        inst->pos += len;
>> +        trace_vof_read(ihandle, len, "(disk)");
>> +        return len;
>> +    }
>> +
>>   #ifdef CONFIG_PSERIES
>>       if (inst->vty) {
>>           uint8_t tmp[VOF_VTY_BUF_SIZE];
>> @@ -675,6 +752,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;
>> @@ -985,6 +1097,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



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC PATCH 6/8] spapr_vscsi: add VOF disk nodes to the device tree
  2026-09-18  5:24   ` Narayana Murty N
@ 2026-09-25  6:41     ` Utkarsh Verma
  0 siblings, 0 replies; 25+ messages in thread
From: Utkarsh Verma @ 2026-09-25  6:41 UTC (permalink / raw)
  To: Narayana Murty N, qemu-devel, qemu-ppc, aik
  Cc: pbonzini, th.huth, sbhat, harshpb, amachhiw, rathc, balaton,
	philmd, npiggin, marcandre.lureau, fam, avnish



On 9/18/26 10:54 AM, Narayana Murty N wrote:
> Hi Utkarsh,
> 
> On 17/08/26 3:57 PM, uverma@linux.ibm.com wrote:
>> From: Utkarsh Verma <uverma@linux.ibm.com>
>>
>> Populate the vSCSI device tree node with child disk nodes for VOF
>> guests.
>>
>> This lets Open Firmware clients such as GRUB open fully-qualified
>> paths like /vdevice/v-scsi@.../disk@<srp-lun> for attached vSCSI
>> disks.
>>
>> Each child node is named from the encoded SRP LUN and includes a reg
>> property with the 64-bit LUN value, along with device_type set to
>> "block".
>>
>> AI-used-for: code
>> Signed-off-by: Utkarsh Verma <uverma@linux.ibm.com>
>> ---
>>   hw/scsi/spapr_vscsi.c | 40 ++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 40 insertions(+)
>>
>> diff --git a/hw/scsi/spapr_vscsi.c b/hw/scsi/spapr_vscsi.c
>> index b4c8f94d22..03f5f27f5f 100644
>> --- a/hw/scsi/spapr_vscsi.c
>> +++ b/hw/scsi/spapr_vscsi.c
>> @@ -1243,6 +1243,8 @@ void spapr_vscsi_create(SpaprVioBus *bus)
>>   static int spapr_vscsi_devnode(SpaprVioDevice *dev, void *fdt, int 
>> node_off)
>>   {
>> +    VSCSIState *s = VIO_SPAPR_VSCSI_DEVICE(dev);
>> +    BusChild *kid;
>>       int ret;
>>       ret = fdt_setprop_cell(fdt, node_off, "#address-cells", 2);
>> @@ -1255,6 +1257,44 @@ static int spapr_vscsi_devnode(SpaprVioDevice 
>> *dev, void *fdt, int node_off)
>>           return ret;
>>       }
>> +    /*
>> +     * In VOF mode, add a child FDT node for each attached SCSI disk 
>> so that OF
>> +     * clients (e.g. GRUB via VOF) can open a fully-qualified path like
>> +     * /vdevice/v-scsi@.../disk@<srp-lun>.
>> +     */
>> +    SpaprMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
>> +    if (spapr->vof) {
>> +        QTAILQ_FOREACH(kid, &s->bus.qbus.children, sibling) {
>> +            SCSIDevice *sdev = SCSI_DEVICE(kid->child);
>> +            char disk_name[32];
>> +            uint64_t srp_lun;
>> +            uint32_t reg[2];
>> +            int disk_off;
>> +
>> +            srp_lun = ((uint64_t)(0x8000 | (sdev->id << 8) |
>> +                                   (sdev->channel << 5) | sdev->lun)) 
>> << 48;
> This matches the decoder introduced in patch 5 for the corresponding bit 
> ranges, but patch 8 constructs SRP LUNs using a different expression.
> 
> Avoid hard coded values, define or use existing relative MACRO definitions.
> 
Ack, will fix in v2.

>> +
>> +            snprintf(disk_name, sizeof(disk_name), "disk@%"PRIx64, 
>> srp_lun);
>> +
>> +            disk_off = fdt_add_subnode(fdt, node_off, disk_name);
>> +            if (disk_off < 0) {
>> +                return disk_off;
>> +            }
>> +
>> +            reg[0] = cpu_to_be32((uint32_t)(srp_lun >> 32));
>> +            reg[1] = cpu_to_be32((uint32_t)(srp_lun & 0xFFFFFFFF));
>> +            ret = fdt_setprop(fdt, disk_off, "reg", reg, sizeof(reg));
>> +            if (ret < 0) {
>> +                return ret;
>> +            }
>> +
>> +            ret = fdt_setprop_string(fdt, disk_off, "device_type", 
>> "block");
>> +            if (ret < 0) {
>> +                return ret;
>> +            }
>> +        }
>> +    }
>> +
>>       return 0;
>>   }
> Could we please introduce a single helper for SRP-LUN encoding and 
> decoding and use it in patches 5, 6 and 8?
> 
> Something along the lines of:
> 
> spapr_vscsi_encode_srp_lun(channel, id, lun)
> spapr_vscsi_decode_srp_lun(srplun, ...)
> 
> That would also make it straightforward to add tests for:
> 
> id      = 0, 1, 31, 32, 63
> channel = 0 .. 7
> lun     = 0, 1, 31
> 
> At the moment the common channel=0/id=0 case can hide inconsistencies 
> between the three implementations.
> 
Sure, will implement new encode and decode functions in this 
spapr_vscsi.c file and declare them in spapr_vio.h and use them acroos 
patch 5, 6, and 8.

> 
> Thanks,
> Narayana Murty N.
> 
> 

Regards,
Utkarsh



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC PATCH 7/8] ppc/spapr: strip OF path argument suffix in path_offset
  2026-09-18  5:25   ` Narayana Murty N
@ 2026-09-25  6:45     ` Utkarsh Verma
  0 siblings, 0 replies; 25+ messages in thread
From: Utkarsh Verma @ 2026-09-25  6:45 UTC (permalink / raw)
  To: Narayana Murty N, qemu-devel, qemu-ppc, aik
  Cc: pbonzini, th.huth, sbhat, harshpb, amachhiw, rathc, balaton,
	philmd, npiggin, marcandre.lureau, fam, avnish



On 9/18/26 10:55 AM, Narayana Murty N wrote:
> Hi Utkarsh,
> 
> On 17/08/26 3:57 PM, uverma@linux.ibm.com wrote:
>> From: Utkarsh Verma <uverma@linux.ibm.com>
>>
>> OF paths can include a ":argument" suffix on the last component (e.g.
>> "disk@8000000000000000:0"), which is an OF path argument such as a
>> partition number.  This is not part of the FDT node name and causes
>> fdt_path_offset() to fail.
>>
>> Strip any ":<suffix>" from the last path component only, leaving
>> intermediate components untouched.
>>
>> AI-used-for: code
>> Signed-off-by: Utkarsh Verma <uverma@linux.ibm.com>
>> ---
>>   hw/ppc/vof.c | 16 ++++++++++++++++
>>   1 file changed, 16 insertions(+)
>>
>> diff --git a/hw/ppc/vof.c b/hw/ppc/vof.c
>> index b1c574aabf..d3b0df30e4 100644
>> --- a/hw/ppc/vof.c
>> +++ b/hw/ppc/vof.c
>> @@ -142,6 +142,8 @@ static int path_offset(const void *fdt, const char 
>> *path)
>>   {
>>       g_autofree char *p = NULL;
>>       char *at;
>> +    char *last_slash;
>> +    char *colon;
>>       /*
>>        * https://www.devicetree.org/open-firmware/bindings/ppc/ 
>> release/ppc-2_1.html#HDR16
>> @@ -151,6 +153,20 @@ static int path_offset(const void *fdt, const 
>> char *path)
>>        * suppressing leading zeros".
>>        */
>>       p = g_strdup(path);
>> +
>> +    /*
>> +     * Strip any ":argument" suffix from the last path component (e.g.
>> +     * "disk@8000000000000000:0") — it is an OF path argument, not 
>> part of
>> +     * the FDT node name.
>> +     */
>> +    last_slash = strrchr(p, '/');
>> +    if (last_slash) {
>> +        colon = strchr(last_slash, ':');
>> +        if (colon) {
>> +            *colon = '\0';
>> +        }
>> +    }
> Is path_offset() guaranteed to always receive an absolute path?
> 
> If not, paths without /, such as disk@8000000000000000:0, won't have the 
> suffix stripped. Either document the absolute-path requirement or handle 
> the no-slash case as well.
> 
path_offset() is called from the vof_open() and vof_finddevice() which 
is the implementation of OF client services, and GRUB calls these API's 
with paths constructed from FDT nodes, which are absolute paths.
All these call chains usually work on absolute path, so path_offset() 
should also be receiving absolute path only. Will add a comment here in 
v2 to address this.

> Thanks,
> Narayana Murty N
>> +
>>       for (at = strchr(p, '@'); at && *at; ) {
>>               if (*at == '/') {
>>                   at = strchr(at, '@');
> 
> 

Regards,
Utkarsh



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC PATCH 8/8] ppc/spapr: implement vscsi-report-luns call-method for PAPR vSCSI
  2026-09-18  5:18   ` Narayana Murty N
@ 2026-09-25  6:49     ` Utkarsh Verma
  0 siblings, 0 replies; 25+ messages in thread
From: Utkarsh Verma @ 2026-09-25  6:49 UTC (permalink / raw)
  To: Narayana Murty N, qemu-devel, qemu-ppc, aik
  Cc: pbonzini, th.huth, sbhat, harshpb, amachhiw, rathc, balaton,
	philmd, npiggin, marcandre.lureau, fam, avnish



On 9/18/26 10:48 AM, Narayana Murty N wrote:
> 
> 
> On 17/08/26 3:57 PM, uverma@linux.ibm.com wrote:
>> From: Utkarsh Verma <uverma@linux.ibm.com>
>>
>> GRUB (grub-core/disk/ieee1275/ofdisk.c) calls "vscsi-report-luns" on
>> any PAPR virtual SCSI controller ihandle to enumerate attached LUNs
>> before attempting to boot from disk. Without this method VOF returns
>> PROM_ERROR and GRUB cannot find vSCSI disks.
>>
>> Add vof_find_vscsi_bus() to locate the SCSIBus from a v-scsi@<reg>
>> instance path, and vof_vscsi_report_luns() to build the response table
>> in the VOF firmware region (0..fw_size), which GRUB never claims.
>>
>> The table layout and SRP LUN encoding follow SLOF (vio-vscsi.fs
>> dev-generate-srplun) each entry is an 8-byte big-endian cell whose
>> low 4 bytes hold a guest pointer to a null-terminated list of SRP LUNs
>> for that target; GRUB reads the pointer at offset table + 4 + 8*i.
>>
>> AI-used-for: code
>> Signed-off-by: Utkarsh Verma <uverma@linux.ibm.com>
>> ---
>>   hw/ppc/vof.c | 212 +++++++++++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 212 insertions(+)
>>
>> diff --git a/hw/ppc/vof.c b/hw/ppc/vof.c
>> index d3b0df30e4..b1bf5b1af0 100644
>> --- a/hw/ppc/vof.c
>> +++ b/hw/ppc/vof.c
>> @@ -991,6 +991,198 @@ static void vof_instantiate_rtas(Error **errp)
>>       error_setg(errp, "The firmware should have instantiated RTAS");
>>   }
>> +#ifdef CONFIG_PSERIES
>> +/* Combined (channel<<6)|id index space, matches SLOF dev-max-target. */
>> +#define VSCSI_MAX_TARGETS 512
>> +
>> +/*
>> + * Build the vscsi-report-luns response in guest memory.
>> + *
>> + * Returns catch_result (0 = success).  On success, *nentries_out and
>> + * *table_addr_out describe the result table written into the VOF 
>> firmware
>> + * region (0..fw_size), which GRUB never claims.
>> + *
>> + * Table layout (big-endian, 8 bytes per entry):
>> + *   [table_addr + 8*i + 0..3] : 0x00000000
>> + *   [table_addr + 8*i + 4..7] : guest pointer to null-terminated SRP 
>> LUN list
>> + *
>> + * GRUB reads each pointer as: *(uint32_t *)(table + 4 + 8 * i)
>> + *
>> + * SRP LUN encoding (SLOF dev-generate-srplun):
>> + *   srplun = (0x8000 | bus | target | lun) << 48
>> + *   where bus    = (combined_target >> 1) & 0x70
>> + *         target = (combined_target & 0x3f) << 8
>> + *         combined_target = (channel << 6) | id
>> + */
>> +static uint32_t vof_vscsi_report_luns(Vof *vof,
>> +                                       SCSIBus *sbus,
>> +                                       uint32_t *nentries_out,
>> +                                       uint32_t *table_addr_out)
>> +{
>> +    uint64_t *lun_lists[VSCSI_MAX_TARGETS];
>> +    int lun_counts[VSCSI_MAX_TARGETS];
>> +    uint32_t ptr_table_size;
>> +    uint32_t lun_data_size;
>> +    uint32_t total_size;
>> +    uint32_t base_addr;
>> +    uint32_t lun_data_base;
>> +    uint32_t lun_data_off;
>> +    uint32_t table_idx;
>> +    uint32_t table_addr;
>> +    BusChild *kid;
>> +    int nentries = 0;
>> +    int t;
>> +
>> +    memset(lun_lists, 0, sizeof(lun_lists));
>> +    memset(lun_counts, 0, sizeof(lun_counts));
>> +
>> +    QTAILQ_FOREACH(kid, &sbus->qbus.children, sibling) {
>> +        SCSIDevice *dev = SCSI_DEVICE(kid->child);
>> +        int combined_target;
>> +        uint64_t bus_field;
>> +        uint64_t target_field;
>> +        uint64_t lun_field;
>> +        uint64_t srplun;
>> +        int cnt;
>> +
>> +        /* combined_target = (channel << 6) | id, matching SLOF 
>> bus+target */
>> +        combined_target = ((dev->channel & 0x7) << 6) | (dev->id & 
>> 0x3f);
>> +        if (combined_target >= VSCSI_MAX_TARGETS) {
>> +            continue;
>> +        }
>> +
>> +        bus_field    = ((uint64_t)combined_target >> 1) & 0x70ULL;
>> +        target_field = ((uint64_t)combined_target & 0x3fULL) << 8;
>> +        lun_field    = (uint64_t)(dev->lun & 0x1f);
>> +        srplun = (0x8000ULL | bus_field | target_field | lun_field) 
>> << 48;
>> +
>> +        cnt = lun_counts[combined_target];
>> +        lun_lists[combined_target] = 
>> g_realloc(lun_lists[combined_target],
>> +                                               (cnt + 2) * 
>> sizeof(uint64_t));
>> +        lun_lists[combined_target][cnt]     = cpu_to_be64(srplun);
>> +        lun_lists[combined_target][cnt + 1] = 0;
>> +        lun_counts[combined_target]++;
>> +        if (cnt == 0) {
>> +            nentries++;
>> +        }
>> +    }
>> +
>> +    if (nentries == 0) {
>> +        *nentries_out   = 0;
>> +        *table_addr_out = 0;
>> +        return 0;
>> +    }
>> +
>> +    ptr_table_size = nentries * sizeof(uint64_t);
>> +    lun_data_size  = 0;
>> +    for (t = 0; t < VSCSI_MAX_TARGETS; t++) {
>> +        if (lun_lists[t]) {
>> +            lun_data_size += (lun_counts[t] + 1) * sizeof(uint64_t);
>> +        }
>> +    }
>> +    total_size = ptr_table_size + lun_data_size;
>> +
>> +    /*
>> +     * Place the table in the VOF firmware region (0..fw_size), which 
>> GRUB
>> +     * never claims.  Fit into the top of the first 4 KB page, 8-byte 
>> aligned.
>> +     */
>> +    if (total_size <= 0x1000 - (uint32_t)vof->fw_size) {
>> +        base_addr = (0x1000 - total_size) & ~7U;
>> +    } else {
>> +        base_addr = 0x800;
>> +    }
>> +
> I don't think the fallback to 0x800 is safe.
> 
> The VOF allocation map documented in spapr_vof_reset() currently has the 
> initial firmware at approximately:
> 
> 0 .. 0xc38
> 
> so 0x800 lies inside that firmware image.
> 
> For a sufficiently large LUN report, this branch therefore appears to 
> write the table over active firmware memory.

Yes, will fix in v2. The comment at spapr_vof_reset() also needs to be 
updated, the firmware size is now 0xd80, will update that comment as 
well. There is almost 3.9 MB free space between the stack top 0x10000 
and the kernel/GRUB start address 0x400000, so will place the 
vscsi_report_luns table in that range and cap the table at 1 KB because 
32 targets × 1 LUN each needs ~768 bytes, so 1 KB should be sufficient 
for any pratical configuration. Also, if the table exceeds the 1 KB cap, 
then would return PROM_ERROR instead of writing to an unsafe address.

>> +    table_addr    = base_addr;
>> +    lun_data_base = base_addr + ptr_table_size;
>> +    lun_data_off  = 0;
>> +    table_idx     = 0;
>> +
>> +    for (t = 0; t < VSCSI_MAX_TARGETS; t++) {
>> +        uint32_t lun_buf_size;
>> +        uint32_t lun_buf_addr;
>> +        uint64_t cell_be64;
>> +
>> +        if (!lun_lists[t]) {
>> +            continue;
>> +        }
>> +
>> +        lun_buf_size = (lun_counts[t] + 1) * sizeof(uint64_t);
>> +        lun_buf_addr = lun_data_base + lun_data_off;
>> +
>> +        if (VOF_MEM_WRITE(lun_buf_addr, lun_lists[t], lun_buf_size)
>> +                != MEMTX_OK) {
>> +            goto write_err;
>> +        }
>> +
>> +        cell_be64 = cpu_to_be64((uint64_t)lun_buf_addr);
>> +        if (VOF_MEM_WRITE(table_addr + table_idx * sizeof(uint64_t),
>> +                          &cell_be64, sizeof(cell_be64)) != MEMTX_OK) {
>> +            goto write_err;
>> +        }
>> +
>> +        lun_data_off += lun_buf_size;
>> +        table_idx++;
>> +    }
>> +
>> +    *nentries_out   = table_idx;
>> +    *table_addr_out = table_addr;
>> +
>> +    for (t = 0; t < VSCSI_MAX_TARGETS; t++) {
>> +        g_free(lun_lists[t]);
>> +    }
>> +    return 0;
>> +
>> +write_err:
>> +    for (t = 0; t < VSCSI_MAX_TARGETS; t++) {
>> +        g_free(lun_lists[t]);
>> +    }
>> +    return PROM_ERROR;
>> +}
>> +
>> +/*
>> + * Return the SCSIBus for a v-scsi@<reg> path, or NULL if not found.
>> + * Handles both "/vdevice/v-scsi@<reg>" and ".../v-scsi@<reg>/ 
>> disk@..." paths.
>> + */
>> +static SCSIBus *vof_find_vscsi_bus(MachineState *ms, const char *path)
>> +{
>> +    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);
>> +        }
>> +    }
>> +    return NULL;
>> +}
>> +#endif /* CONFIG_PSERIES */
>> +
>>   static uint32_t vof_call_method(MachineState *ms, Vof *vof, uint32_t 
>> methodaddr,
>>                                   uint32_t ihandle, uint32_t param1,
>>                                   uint32_t param2, uint32_t param3,
>> @@ -1014,6 +1206,26 @@ static uint32_t vof_call_method(MachineState 
>> *ms, Vof *vof, uint32_t methodaddr,
>>           goto trace_exit;
>>       }
>> +#ifdef CONFIG_PSERIES
>> +    /* vscsi-report-luns: enumerate LUNs for GRUB */
>> +    if (strcmp(method, "vscsi-report-luns") == 0) {
>> +        SCSIBus *sbus = vof_find_vscsi_bus(ms, inst->path);
>> +
>> +        if (sbus) {
>> +            uint32_t nentries = 0, table_addr = 0;
>> +            ret = vof_vscsi_report_luns(vof, sbus, &nentries, 
>> &table_addr);
>> +            ret2[0] = nentries;
>> +            ret2[1] = table_addr;
>> +        } else {
>> +            ret = 1;
>> +            ret2[0] = 0;
>> +            ret2[1] = 0;
>> +        }
>> +        trace_vof_method(ihandle, method, param1, ret, ret2[0]);
>> +        goto trace_exit;
>> +    }
>> +#endif /* CONFIG_PSERIES */
>> +
>>       if (strcmp(inst->path, "/") == 0) {
>>           if (strcmp(method, "ibm,client-architecture-support") == 0) {
>>               Object *vmo = object_dynamic_cast(OBJECT(ms), 
>> TYPE_VOF_MACHINE_IF);
> 
> Over view:
> In particular test malformed GPT fields (entry_size, reversed LBA 
> ranges, overflowing table offsets), a very large OF read/write length, 
> multiple vSCSI controllers using the same channel/id/lun tuple, IDs >= 
> 32/non-zero channels for SRP-LUN encoding, a malformed ELF after one or 
> more successful PT_LOAD callbacks, and enough vSCSI targets to exceed 
> the free space below 4 KiB.
> 
> Those cases exercise precisely the boundaries introduced by this series.

Thanks Narayana for the thorough review and the comments :)
Will send v2 with all the fixes.>
> Thanks,
> Narayana Murty N
> 

Regards,
Utkarsh



^ permalink raw reply	[flat|nested] 25+ messages in thread

end of thread, other threads:[~2026-09-25  6:50 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-17 10:27 [RFC PATCH 0/8] ppc/spapr: VOF disk image (qcow2) boot support uverma
2026-08-17 10:27 ` [RFC PATCH 1/8] ppc/spapr: add PReP boot partition detection uverma
2026-09-17 17:14   ` Narayana Murty N
2026-09-25  6:19     ` Utkarsh Verma
2026-08-17 10:27 ` [RFC PATCH 2/8] hw/loader: add load_elf_ram_sym_buf() for in-memory ELF loading uverma
2026-09-17 17:14   ` Narayana Murty N
2026-09-25  6:24     ` Utkarsh Verma
2026-08-17 10:27 ` [RFC PATCH 3/8] ppc/spapr: add baseline VOF disk boot support uverma
2026-09-17 17:33   ` Narayana Murty N
2026-09-25  6:28     ` Utkarsh Verma
2026-08-17 10:27 ` [RFC PATCH 4/8] ppc/spapr: add VTY backend support to OF read/write/open services uverma
2026-08-17 10:27 ` [RFC PATCH 5/8] ppc/spapr: add block device backend to VOF open/read/write/seek services uverma
2026-09-18  5:23   ` Narayana Murty N
2026-09-25  6:33     ` Utkarsh Verma
2026-08-17 10:27 ` [RFC PATCH 6/8] spapr_vscsi: add VOF disk nodes to the device tree uverma
2026-09-18  5:24   ` Narayana Murty N
2026-09-25  6:41     ` Utkarsh Verma
2026-08-17 10:27 ` [RFC PATCH 7/8] ppc/spapr: strip OF path argument suffix in path_offset uverma
2026-09-18  5:25   ` Narayana Murty N
2026-09-25  6:45     ` Utkarsh Verma
2026-08-17 10:27 ` [RFC PATCH 8/8] ppc/spapr: implement vscsi-report-luns call-method for PAPR vSCSI uverma
2026-09-18  5:18   ` Narayana Murty N
2026-09-25  6:49     ` Utkarsh Verma
2026-09-18  5:26   ` Narayana Murty N
2026-08-19  6:55 ` [RFC PATCH 0/8] ppc/spapr: VOF disk image (qcow2) boot support Utkarsh Verma

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.