On 5/8/26 9:16 PM, Eric Farman wrote:
On Mon, 2026-05-04 at 18:16 -0400, [email protected] wrote:
From: Jared Rossi <[email protected]>

Define a new IPLB format for SCSI PCI devices and add cases to handle it.

Signed-off-by: Jared Rossi <[email protected]>
---
  include/hw/s390x/ipl/qipl.h    | 15 +++++++++++++++
  pc-bios/s390-ccw/main.c        | 11 ++++++++++-
  pc-bios/s390-ccw/virtio-pci.c  | 29 +++++++++++++++++++++++++++++
  pc-bios/s390-ccw/virtio-scsi.c | 14 +++++++++++++-
  pc-bios/s390-ccw/virtio.c      |  5 +++++
  5 files changed, 72 insertions(+), 2 deletions(-)

diff --git a/include/hw/s390x/ipl/qipl.h b/include/hw/s390x/ipl/qipl.h
index 67db54c964..951c5e2d8e 100644
--- a/include/hw/s390x/ipl/qipl.h
+++ b/include/hw/s390x/ipl/qipl.h
@@ -25,6 +25,7 @@ enum S390IplType {
      S390_IPL_TYPE_CCW = 0x02,
      S390_IPL_TYPE_PCI = 0x04,
      S390_IPL_TYPE_PV = 0x05,
+    S390_IPL_TYPE_PCI_SCSI = 0xfe,
      S390_IPL_TYPE_CCW_SCSI = 0xff
  };
  typedef enum S390IplType S390IplType;
@@ -117,6 +118,19 @@ struct IplBlockPci {
  } QEMU_PACKED;
  typedef struct IplBlockPci IplBlockPci;
+struct IplBlockPciScsi {
+    uint32_t lun;
+    uint16_t target;
+    uint16_t channel;
+    uint8_t  reserved0[74];
+    uint8_t  opt;
+    uint8_t  reserved1[3];
+    uint32_t fid;
+    uint8_t  ssid;
+    uint16_t devno;
+} QEMU_PACKED;
+typedef struct IplBlockPciScsi IplBlockPciScsi;
Now that I get to the point where I see why you renamed the ...QemuScsi struct 
in patch one, I'm
struggling.

This combines the PCI stuff in IplBlockPci, with the CCW (!) and SCSI stuff in 
IplBlockCcwScsi (nee
IplBlockQemuScsi). Some of the stuff (ssid, devno) isn't needed for PCI, and I 
find myself wondering
what the "right" struct should look like...
The ssid and devno should be removed from the PCI version, it was just an
oversight to leave them there.
Can we add the PCI stuff to a (generalized) struct that's just "IplBlockScsi" 
and have fields that
are specific to -ccw or -pci? That's basically what IplBlockPciScsi is, even 
though it's not used
that way.
Yes, it is possible.  At first I did not think it was the proper approach, but
thinking about it again now I can see the benefit.  I will reconsider it.

Or is it better that there's overlap between Ccw/CcwScsi (e.g., 
offsetof(Ccw.ssid) ==
offsetof(CcwScsi.ssid)) and Pci/PciScsi? If the latter, there's quite a bit of 
difference in their
layouts, e.g., the location of the opt and fid fields. That worries me...
You are correct that the offsets are wrong. At minimum I will fix the offsets to
match, otherwise I will rework it into a combined IPLB as described above.

Thanks for catching this error.

Regards,
Jared Rossi

Reply via email to