Hello,
On Thu, 8 Oct 2026, Utkarsh Verma wrote:
[...]
diff --git a/hw/ppc/vof.c b/hw/ppc/vof.c
index 6f6d424347..d4b57b3b46 100644
--- a/hw/ppc/vof.c
+++ b/hw/ppc/vof.c
@@ -11,6 +11,7 @@
#include CONFIG_DEVICES /* CONFIG_PSERIES */
#include "qemu/osdep.h"
+#include "qemu/cutils.h"
#include "qemu/ctype.h"
#include "qemu/timer.h"
#include "qemu/range.h"
@@ -25,6 +26,9 @@
#include "trace.h"
#include "hw/ppc/spapr_vio.h"
+#include "hw/scsi/scsi.h"
+#include "system/block-backend.h"
+#include "block/block-io.h"
#include <libfdt.h>
/*
@@ -38,6 +42,7 @@
#define VOF_MAX_METHODLEN 256
#define VOF_MAX_FORTHCODE 256
#define VOF_VTY_BUF_SIZE 256
+#define VOF_BLK_BUF_SIZE (64 * 1024)
* KiB instead of 1024?
Sure, will use KiB and fix it for SPAPR_VOF_PREP_MAX_SIZE in spapr_vof.c as
well in v3.
Sorry for taking that long to review which caused you needing to prepare
another version but I have little time for QEMU now and generally don't
like to be first to review AI generated patches so I hoped somebody else
could do a review. Since the list is already high volume, if you agree
with a change it's OK to just include it in next version and you don't
have to reply for those with "OK, I'll do that" here, unless you have a
comment or question about it or answering a question asked.
typedef struct {
uint64_t start;
@@ -47,6 +52,8 @@ typedef struct {
typedef struct {
char *path; /* the path used to open the instance */
uint32_t phandle;
+ BlockBackend *blk;
+ uint64_t pos; /* current position for seek operations */
void *vty;
} OfInstance;
@@ -138,6 +145,8 @@ static int path_offset(const void *fdt, const char
*path)
{
g_autofree char *p = NULL;
char *at;
+ char *last_slash;
+ int offset;
/*
* https://www.devicetree.org/open-firmware/bindings/ppc/release/
ppc-2_1.html#HDR16
@@ -156,7 +165,76 @@ static int path_offset(const void *fdt, const char
*path)
}
}
- return fdt_path_offset(fdt, p);
+ offset = fdt_path_offset(fdt, p);
+ if (offset >= 0) {
+ return offset;
+ }
+
+ /*
+ * Apply the OF wildcard rule for vSCSI disk paths. A path ending in
+ * disk@<srp-lun> has no per-LUN FDT node, instead the FDT contains a
single
+ * generic "disk" child (no srp-lun appended to the path). The
phandle is
+ * shared by all LUNs on the adapter, matching the Open Firmware
behaviour.
+ */
+ last_slash = strrchr(p, '/');
+ if (last_slash && strncmp(last_slash + 1, "disk@", 5) == 0) {
+ int parent_off;
+
+ *last_slash = '\0';
+ parent_off = fdt_path_offset(fdt, p);
+ if (parent_off >= 0) {
+ int child_off = fdt_subnode_offset(fdt, parent_off, "disk");
I'm not sure how this should work but what if there are multiple disk nodes
such as disk@0 disk@1 or similar? Should this look for the original ending
of the path instead of just "disk"?
There is only one "disk" child node under the vSCSI adapter regardless of how
many physical disks are attached. It seems a little weird that only one disk
node is created even for multiple vSCSI disks, but that is the actual open
firmware behaviour, I cross checked with the device tree data on a pseries
LPAR machine and with the SLOF implementation of it as well.
So, the GRUB only searches for a "disk" child node under the vscsi adapter
and then calls the vscsi-report-luns method to get the full path of the
individual disks and passes the same path to firmware while calling open. So,
GRUB doesn't rely on FDT disk data to search the disk, it calls the
vscs-report-luns method to get the full disk path.
This is why I strip the @<srp-lun> part in the path before searching the FDT,
the node it needs to resolve to is always the shared "disk" child node.
We're in in the common vof.c file here which is a generic OpenFirmware
implementation and is shared with pegasos (and later could be used by
other machines needing OpenFirmware). On pegasos2 for example we can have
/pci/ide/disk@0,0, /pci/ide/disk@0,1, /pci/ide/disk@1,0 and
/pci/ide/disk@1,1 for 4 IDE disks so this change may break clients
accessing those device nodes. So likely you need to avoid changing this
function but maybe by moving all the other stuff to spapr_vof.c this will
not be needed here any more and can be handled in spapr_vof.c. If you find
you're trying to change vof.c for something specific to spapr then think
again because anything spapr specific should be in spapr_vof.c.
I've also noticed pegasos1 sets boot-device to /pci/ide/disk@0,0:0 where
:0 is showing the partition so that is a generic OF thing that could be
handled here in vof.c but that could be fixed later as well. To make it
easy to use this block device support by other machines in the future, the
functions to be added to VofMachineIfClass should be chosen to do the
machine specific mapping between OpenFirmware nodes to generic QEMU
constructs and then the code here can work on those without needing to be
machine specific. E.g. I'm not sure requiring machines to implement
vty_open/read/write is necessary and if it could be done more generically
with just one function mapping an OF device node to something that can be
used here like for block device the mapping function returns a blk device
for an OF device node. That would also make machine code simpler as it
does not have to talk to the device just return the QEMU struct to vof and
reduce code duplication between machines. But I don't know what would be
the right QEMU construct for a vty, maybe a chardev?
Regards,
BALATON Zoltan
