All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 00/15]  s390x: Add support for virtio-blk-pci IPL device
@ 2026-03-04  2:59 jrossi
  2026-03-04  2:59 ` [PATCH v4 01/15] pc-bios/s390-ccw: Fix misattributed function prototypes jrossi
                   ` (14 more replies)
  0 siblings, 15 replies; 35+ messages in thread
From: jrossi @ 2026-03-04  2:59 UTC (permalink / raw)
  To: qemu-devel, qemu-s390x, thuth, mst
  Cc: jjherne, alifm, farman, mjrosato, jrossi, zycai

From: Jared Rossi <jrossi@linux.ibm.com>

This patch series introduces an IPLB subtype to support PCI devices, which may
be built if a device has been assigned a boot index and is identified as a PCI
device with a corresponding s390 PCI Bus device.

Boot support is only added for virtio-blk-pci at this time and is limited to
devices with an assigned bootindex.

A "loadparm" property is added to virtio-blk-pci boot devices on s390x.

A simple test to check basic functionality is added to the cdrom-tests in qtest.

Changes v3 -> v4:
    - Reset virtio device after failed boot (not PCI specific)
    - Use defined constant for CLP block size
    - Enable PCI bus master bit
    - Refactor where some byte swaps happen
    - Fix an incorrect offset during PCI config read
    - Update maintainers when moving CLP definitions
    - Improve various error messages to include more info/codes

Jared Rossi (15):
  pc-bios/s390-ccw: Fix misattributed function prototypes
  pc-bios/s390-ccw: Remove redundant vring schid attribute
  pc-bios/s390-ccw: Always reset virtio device on failed boot attempt
  s390x: Remove duplicate definitions of IPL types
  pc-bios/s390-ccw: Store device type independent of sense data
  pc-bios/s390-ccw: Split virtio-ccw and generic virtio
  include/hw/s390x: Move CLP definitions for easier BIOS access
  pc-bios/s390-ccw: Introduce CLP Architecture
  s390x: Add definitions for PCI IPL type
  pc-bios/s390-ccw: Introduce PCI device
  pc-bios/s390-ccw: Introduce virtio-pci functions
  pc-bios/s390-ccw: Add support for virtio-blk-pci IPL
  s390x: Build IPLB for virtio-pci devices
  hw: Add "loadparm" property to virtio block PCI devices booting on
    s390x
  tests/qtest: Add s390x PCI boot test to cdrom-test.c

 MAINTAINERS                               |   1 +
 hw/pci/pci.c                              |  38 ++
 hw/s390x/ipl.c                            |  63 +++-
 hw/s390x/ipl.h                            |   8 +-
 hw/s390x/s390-pci-bus.c                   |   3 +-
 hw/s390x/s390-pci-vfio.c                  |   2 +-
 hw/virtio/virtio-blk-pci.c                |   1 +
 include/hw/pci/pci.h                      |   1 +
 include/hw/pci/pci_device.h               |   3 +
 include/hw/s390x/ipl/qipl.h               |  20 +
 include/hw/s390x/{ => ipl}/s390-pci-clp.h |   0
 include/hw/s390x/s390-pci-bus.h           |   4 +-
 pc-bios/s390-ccw/Makefile                 |   3 +-
 pc-bios/s390-ccw/bootmap.c                |   2 +-
 pc-bios/s390-ccw/clp.c                    |  99 +++++
 pc-bios/s390-ccw/clp.h                    |  24 ++
 pc-bios/s390-ccw/iplb.h                   |   4 -
 pc-bios/s390-ccw/main.c                   |  83 ++++-
 pc-bios/s390-ccw/netmain.c                |   2 +-
 pc-bios/s390-ccw/pci.c                    | 118 ++++++
 pc-bios/s390-ccw/pci.h                    |  42 +++
 pc-bios/s390-ccw/s390-ccw.h               |   7 -
 pc-bios/s390-ccw/virtio-blkdev.c          |  76 ++--
 pc-bios/s390-ccw/virtio-ccw.c             | 241 ++++++++++++
 pc-bios/s390-ccw/virtio-ccw.h             |  24 ++
 pc-bios/s390-ccw/virtio-net.c             |   5 +-
 pc-bios/s390-ccw/virtio-pci.c             | 432 ++++++++++++++++++++++
 pc-bios/s390-ccw/virtio-pci.h             |  82 ++++
 pc-bios/s390-ccw/virtio-scsi.c            |   8 +-
 pc-bios/s390-ccw/virtio-scsi.h            |   2 +-
 pc-bios/s390-ccw/virtio.c                 | 293 +++++----------
 pc-bios/s390-ccw/virtio.h                 |  21 +-
 tests/qtest/cdrom-test.c                  |   7 +
 33 files changed, 1448 insertions(+), 271 deletions(-)
 rename include/hw/s390x/{ => ipl}/s390-pci-clp.h (100%)
 create mode 100644 pc-bios/s390-ccw/clp.c
 create mode 100644 pc-bios/s390-ccw/clp.h
 create mode 100644 pc-bios/s390-ccw/pci.c
 create mode 100644 pc-bios/s390-ccw/pci.h
 create mode 100644 pc-bios/s390-ccw/virtio-ccw.c
 create mode 100644 pc-bios/s390-ccw/virtio-ccw.h
 create mode 100644 pc-bios/s390-ccw/virtio-pci.c
 create mode 100644 pc-bios/s390-ccw/virtio-pci.h

-- 
2.52.0



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

* [PATCH v4 01/15] pc-bios/s390-ccw: Fix misattributed function prototypes
  2026-03-04  2:59 [PATCH v4 00/15] s390x: Add support for virtio-blk-pci IPL device jrossi
@ 2026-03-04  2:59 ` jrossi
  2026-03-04  2:59 ` [PATCH v4 02/15] pc-bios/s390-ccw: Remove redundant vring schid attribute jrossi
                   ` (13 subsequent siblings)
  14 siblings, 0 replies; 35+ messages in thread
From: jrossi @ 2026-03-04  2:59 UTC (permalink / raw)
  To: qemu-devel, qemu-s390x, thuth, mst
  Cc: jjherne, alifm, farman, mjrosato, jrossi, zycai

From: Jared Rossi <jrossi@linux.ibm.com>

The virtio-blkdev functions are incorrectly listed in s390-ccw.h as belonging to
virtio.c.  Additionally, virtio_load_direct() has an unused subchan_id argument.

Remove the unused argument and move the prototypes to virtio.h so that they are
independent from the CCW bus.

Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Eric Farman <farman@linux.ibm.com>
Reviewed-by: Farhan Ali <alifm@linux.ibm.com>
Signed-off-by: Jared Rossi <jrossi@linux.ibm.com>
---
 pc-bios/s390-ccw/bootmap.c       | 2 +-
 pc-bios/s390-ccw/s390-ccw.h      | 4 ----
 pc-bios/s390-ccw/virtio-blkdev.c | 2 +-
 pc-bios/s390-ccw/virtio.h        | 7 +++++++
 4 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/pc-bios/s390-ccw/bootmap.c b/pc-bios/s390-ccw/bootmap.c
index 0f8baa0198..420ee32eff 100644
--- a/pc-bios/s390-ccw/bootmap.c
+++ b/pc-bios/s390-ccw/bootmap.c
@@ -662,7 +662,7 @@ static int zipl_load_segment(ComponentEntry *entry)
                  */
                 break;
             }
-            address = virtio_load_direct(cur_desc[0], cur_desc[1], 0,
+            address = virtio_load_direct(cur_desc[0], cur_desc[1],
                                          (void *)address);
             if (!address) {
                 puts("zIPL load segment failed");
diff --git a/pc-bios/s390-ccw/s390-ccw.h b/pc-bios/s390-ccw/s390-ccw.h
index b1dc35cded..47ea66bd4d 100644
--- a/pc-bios/s390-ccw/s390-ccw.h
+++ b/pc-bios/s390-ccw/s390-ccw.h
@@ -67,11 +67,7 @@ void sclp_get_loadparm_ascii(char *loadparm);
 int sclp_read(char *str, size_t count);
 
 /* virtio.c */
-unsigned long virtio_load_direct(unsigned long rec_list1, unsigned long rec_list2,
-                                 unsigned long subchan_id, void *load_addr);
 bool virtio_is_supported(SubChannelId schid);
-int virtio_blk_setup_device(SubChannelId schid);
-int virtio_read(unsigned long sector, void *load_addr);
 
 /* bootmap.c */
 void zipl_load(void);
diff --git a/pc-bios/s390-ccw/virtio-blkdev.c b/pc-bios/s390-ccw/virtio-blkdev.c
index 7b2d1e20f4..4b819dd80f 100644
--- a/pc-bios/s390-ccw/virtio-blkdev.c
+++ b/pc-bios/s390-ccw/virtio-blkdev.c
@@ -64,7 +64,7 @@ int virtio_read_many(unsigned long sector, void *load_addr, int sec_num)
 }
 
 unsigned long virtio_load_direct(unsigned long rec_list1, unsigned long rec_list2,
-                                 unsigned long subchan_id, void *load_addr)
+                                 void *load_addr)
 {
     u8 status;
     int sec = rec_list1;
diff --git a/pc-bios/s390-ccw/virtio.h b/pc-bios/s390-ccw/virtio.h
index 5c5e808a50..597bd42358 100644
--- a/pc-bios/s390-ccw/virtio.h
+++ b/pc-bios/s390-ccw/virtio.h
@@ -277,7 +277,14 @@ int virtio_run(VDev *vdev, int vqid, VirtioCmd *cmd);
 int virtio_reset(VDev *vdev);
 int virtio_setup_ccw(VDev *vdev);
 
+/* virtio-net.c */
 int virtio_net_init(void *mac_addr);
 void virtio_net_deinit(void);
 
+/* virtio-blkdev.c */
+int virtio_blk_setup_device(SubChannelId schid);
+int virtio_read(unsigned long sector, void *load_addr);
+unsigned long virtio_load_direct(unsigned long rec_list1, unsigned long rec_list2,
+                                 void *load_addr);
+
 #endif /* VIRTIO_H */
-- 
2.52.0



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

* [PATCH v4 02/15] pc-bios/s390-ccw: Remove redundant vring schid attribute
  2026-03-04  2:59 [PATCH v4 00/15] s390x: Add support for virtio-blk-pci IPL device jrossi
  2026-03-04  2:59 ` [PATCH v4 01/15] pc-bios/s390-ccw: Fix misattributed function prototypes jrossi
@ 2026-03-04  2:59 ` jrossi
  2026-03-04  2:59 ` [PATCH v4 03/15] pc-bios/s390-ccw: Always reset virtio device on failed boot attempt jrossi
                   ` (12 subsequent siblings)
  14 siblings, 0 replies; 35+ messages in thread
From: jrossi @ 2026-03-04  2:59 UTC (permalink / raw)
  To: qemu-devel, qemu-s390x, thuth, mst
  Cc: jjherne, alifm, farman, mjrosato, jrossi, zycai

From: Jared Rossi <jrossi@linux.ibm.com>

The schid is already stored as an attribute of the VDev itself and any other
instances are copies of this same value.  To avoid CCW specific attributes in
the VRing let's just access the existing VDev schid attribute as needed.

Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Eric Farman <farman@linux.ibm.com>
Reviewed-by: Farhan Ali<alifm@linux.ibm.com>
Signed-off-by: Jared Rossi <jrossi@linux.ibm.com>
---
 pc-bios/s390-ccw/virtio-blkdev.c | 2 +-
 pc-bios/s390-ccw/virtio-net.c    | 2 +-
 pc-bios/s390-ccw/virtio.c        | 9 ++++-----
 pc-bios/s390-ccw/virtio.h        | 3 +--
 4 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/pc-bios/s390-ccw/virtio-blkdev.c b/pc-bios/s390-ccw/virtio-blkdev.c
index 4b819dd80f..019c2718b1 100644
--- a/pc-bios/s390-ccw/virtio-blkdev.c
+++ b/pc-bios/s390-ccw/virtio-blkdev.c
@@ -42,7 +42,7 @@ static int virtio_blk_read_many(VDev *vdev, unsigned long sector, void *load_add
     /* Now we can tell the host to read */
     vring_wait_reply();
 
-    if (drain_irqs(vr->schid)) {
+    if (drain_irqs()) {
         /* Well, whatever status is supposed to contain... */
         status = 1;
     }
diff --git a/pc-bios/s390-ccw/virtio-net.c b/pc-bios/s390-ccw/virtio-net.c
index 301445bf97..7eb0850069 100644
--- a/pc-bios/s390-ccw/virtio-net.c
+++ b/pc-bios/s390-ccw/virtio-net.c
@@ -88,7 +88,7 @@ int send(int fd, const void *buf, int len, int flags)
     while (!vr_poll(txvq)) {
         yield();
     }
-    if (drain_irqs(txvq->schid)) {
+    if (drain_irqs()) {
         puts("send: drain irqs failed");
         return -1;
     }
diff --git a/pc-bios/s390-ccw/virtio.c b/pc-bios/s390-ccw/virtio.c
index cd6c99c7e3..f384a990dc 100644
--- a/pc-bios/s390-ccw/virtio.c
+++ b/pc-bios/s390-ccw/virtio.c
@@ -72,14 +72,14 @@ static long virtio_notify(SubChannelId schid, int vq_idx, long cookie)
  *             Virtio functions                *
  ***********************************************/
 
-int drain_irqs(SubChannelId schid)
+int drain_irqs(void)
 {
     Irb irb = {};
     int r = 0;
 
     while (1) {
         /* FIXME: make use of TPI, for that enable subchannel and isc */
-        if (tsch(schid, &irb)) {
+        if (tsch(vdev.schid, &irb)) {
             /* Might want to differentiate error codes later on. */
             if (irb.scsw.cstat) {
                 r = -EIO;
@@ -134,7 +134,7 @@ static void vring_init(VRing *vr, VqInfo *info)
 
 bool vring_notify(VRing *vr)
 {
-    vr->cookie = virtio_notify(vr->schid, vr->id, vr->cookie);
+    vr->cookie = virtio_notify(vdev.schid, vr->id, vr->cookie);
     return vr->cookie >= 0;
 }
 
@@ -211,7 +211,7 @@ int virtio_run(VDev *vdev, int vqid, VirtioCmd *cmd)
     } while (cmd[i++].flags & VRING_DESC_F_NEXT);
 
     vring_wait_reply();
-    if (drain_irqs(vr->schid)) {
+    if (drain_irqs()) {
         return -1;
     }
     return 0;
@@ -316,7 +316,6 @@ int virtio_setup_ccw(VDev *vdev)
         }
         info.num = config.num;
         vring_init(&vdev->vrings[i], &info);
-        vdev->vrings[i].schid = vdev->schid;
         if (run_ccw(vdev, CCW_CMD_SET_VQ, &info, sizeof(info), false)) {
             puts("Cannot set VQ info");
             return -EIO;
diff --git a/pc-bios/s390-ccw/virtio.h b/pc-bios/s390-ccw/virtio.h
index 597bd42358..06ba4e45ac 100644
--- a/pc-bios/s390-ccw/virtio.h
+++ b/pc-bios/s390-ccw/virtio.h
@@ -103,7 +103,6 @@ struct VRing {
     VRingDesc *desc;
     VRingAvail *avail;
     VRingUsed *used;
-    SubChannelId schid;
     long cookie;
     int id;
 };
@@ -269,7 +268,7 @@ struct VirtioCmd {
 typedef struct VirtioCmd VirtioCmd;
 
 bool vring_notify(VRing *vr);
-int drain_irqs(SubChannelId schid);
+int drain_irqs(void);
 void vring_send_buf(VRing *vr, void *p, int len, int flags);
 int vr_poll(VRing *vr);
 int vring_wait_reply(void);
-- 
2.52.0



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

* [PATCH v4 03/15] pc-bios/s390-ccw: Always reset virtio device on failed boot attempt
  2026-03-04  2:59 [PATCH v4 00/15] s390x: Add support for virtio-blk-pci IPL device jrossi
  2026-03-04  2:59 ` [PATCH v4 01/15] pc-bios/s390-ccw: Fix misattributed function prototypes jrossi
  2026-03-04  2:59 ` [PATCH v4 02/15] pc-bios/s390-ccw: Remove redundant vring schid attribute jrossi
@ 2026-03-04  2:59 ` jrossi
  2026-03-04  8:23   ` Thomas Huth
  2026-03-04 13:39   ` Eric Farman
  2026-03-04  2:59 ` [PATCH v4 04/15] s390x: Remove duplicate definitions of IPL types jrossi
                   ` (11 subsequent siblings)
  14 siblings, 2 replies; 35+ messages in thread
From: jrossi @ 2026-03-04  2:59 UTC (permalink / raw)
  To: qemu-devel, qemu-s390x, thuth, mst
  Cc: jjherne, alifm, farman, mjrosato, jrossi, zycai

From: Jared Rossi <jrossi@linux.ibm.com>

The virtio spec necessitates that live virtqueues must not be altered.  Reset
the failed device so that the queues are not live before we attempt to boot any
fallback devices.

Signed-off-by: Jared Rossi <jrossi@linux.ibm.com>
---
 pc-bios/s390-ccw/main.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/pc-bios/s390-ccw/main.c b/pc-bios/s390-ccw/main.c
index 76bf743900..8e2c99bee1 100644
--- a/pc-bios/s390-ccw/main.c
+++ b/pc-bios/s390-ccw/main.c
@@ -277,7 +277,8 @@ static void ipl_boot_device(void)
         break;
     case CU_TYPE_VIRTIO:
         if (virtio_setup() == 0) {
-            zipl_load();
+            zipl_load(); /* only return on error */
+            virtio_reset(virtio_get_device());
         }
         break;
     default:
-- 
2.52.0



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

* [PATCH v4 04/15] s390x: Remove duplicate definitions of IPL types
  2026-03-04  2:59 [PATCH v4 00/15] s390x: Add support for virtio-blk-pci IPL device jrossi
                   ` (2 preceding siblings ...)
  2026-03-04  2:59 ` [PATCH v4 03/15] pc-bios/s390-ccw: Always reset virtio device on failed boot attempt jrossi
@ 2026-03-04  2:59 ` jrossi
  2026-03-04  8:34   ` Thomas Huth
  2026-03-04  2:59 ` [PATCH v4 05/15] pc-bios/s390-ccw: Store device type independent of sense data jrossi
                   ` (10 subsequent siblings)
  14 siblings, 1 reply; 35+ messages in thread
From: jrossi @ 2026-03-04  2:59 UTC (permalink / raw)
  To: qemu-devel, qemu-s390x, thuth, mst
  Cc: jjherne, alifm, farman, mjrosato, jrossi, zycai

From: Jared Rossi <jrossi@linux.ibm.com>

Remove the duplicate definitions from hw/s390x/ipl.h and pc-bios/s390-ccw/iplb.h
and add a shared definition.  The new definition is an enum to enforce default
handling in switches.

Because the IPL type is determined by the IPLB, and because an IPLB is not
strictly necessary, the IPL type is set to a default value if not otherwise
specified.  A default IPL type is required so future functionality may add
IPL new bus and/or device types that dictate specific behavior during IPL.

Signed-off-by: Jared Rossi <jrossi@linux.ibm.com>
---
 hw/s390x/ipl.h              |  5 -----
 include/hw/s390x/ipl/qipl.h | 10 ++++++++++
 pc-bios/s390-ccw/iplb.h     |  4 ----
 pc-bios/s390-ccw/main.c     |  9 +++++++--
 pc-bios/s390-ccw/virtio.h   |  1 +
 5 files changed, 18 insertions(+), 11 deletions(-)

diff --git a/hw/s390x/ipl.h b/hw/s390x/ipl.h
index 086e57681c..c542d30ce2 100644
--- a/hw/s390x/ipl.h
+++ b/hw/s390x/ipl.h
@@ -103,11 +103,6 @@ QEMU_BUILD_BUG_MSG(offsetof(S390IPLState, iplb) & 3, "alignment of iplb wrong");
 #define DIAG308_PV_STORE            9
 #define DIAG308_PV_START            10
 
-#define S390_IPL_TYPE_FCP 0x00
-#define S390_IPL_TYPE_CCW 0x02
-#define S390_IPL_TYPE_PV 0x05
-#define S390_IPL_TYPE_QEMU_SCSI 0xff
-
 #define S390_IPLB_HEADER_LEN 8
 #define S390_IPLB_MIN_PV_LEN 148
 #define S390_IPLB_MIN_CCW_LEN 200
diff --git a/include/hw/s390x/ipl/qipl.h b/include/hw/s390x/ipl/qipl.h
index 6824391111..6dc12dd859 100644
--- a/include/hw/s390x/ipl/qipl.h
+++ b/include/hw/s390x/ipl/qipl.h
@@ -20,6 +20,16 @@
 #define LOADPARM_LEN    8
 #define NO_LOADPARM "\0\0\0\0\0\0\0\0"
 
+enum S390IplType {
+    S390_IPL_TYPE_FCP = 0x00,
+    S390_IPL_TYPE_CCW = 0x02,
+    S390_IPL_TYPE_PV = 0x05,
+    S390_IPL_TYPE_QEMU_SCSI = 0xff
+};
+typedef enum S390IplType S390IplType;
+
+#define QEMU_DEFAULT_IPL S390_IPL_TYPE_CCW
+
 /*
  * The QEMU IPL Parameters will be stored at absolute address
  * 204 (0xcc) which means it is 32-bit word aligned but not
diff --git a/pc-bios/s390-ccw/iplb.h b/pc-bios/s390-ccw/iplb.h
index 08f259ff31..926e8eed5d 100644
--- a/pc-bios/s390-ccw/iplb.h
+++ b/pc-bios/s390-ccw/iplb.h
@@ -23,10 +23,6 @@ extern QemuIplParameters qipl;
 extern IplParameterBlock iplb __attribute__((__aligned__(PAGE_SIZE)));
 extern bool have_iplb;
 
-#define S390_IPL_TYPE_FCP 0x00
-#define S390_IPL_TYPE_CCW 0x02
-#define S390_IPL_TYPE_QEMU_SCSI 0xff
-
 static inline bool manage_iplb(IplParameterBlock *iplb, bool store)
 {
     register unsigned long addr asm("0") = (unsigned long) iplb;
diff --git a/pc-bios/s390-ccw/main.c b/pc-bios/s390-ccw/main.c
index 8e2c99bee1..ef5acc1985 100644
--- a/pc-bios/s390-ccw/main.c
+++ b/pc-bios/s390-ccw/main.c
@@ -162,11 +162,12 @@ static void menu_setup(void)
         return;
     }
 
-    switch (iplb.pbt) {
+    switch (virtio_get_device()->ipl_type) {
     case S390_IPL_TYPE_CCW:
     case S390_IPL_TYPE_QEMU_SCSI:
         menu_set_parms(qipl.qipl_flags & BOOT_MENU_FLAG_MASK,
                        qipl.boot_menu_timeout);
+    default:
         return;
     }
 }
@@ -190,6 +191,7 @@ static void css_setup(void)
 static void boot_setup(void)
 {
     char lpmsg[] = "LOADPARM=[________]\n";
+    VDev *vdev = virtio_get_device();
 
     if (have_iplb && memcmp(iplb.loadparm, NO_LOADPARM, LOADPARM_LEN) != 0) {
         ebcdic_to_ascii((char *) iplb.loadparm, loadparm_str, LOADPARM_LEN);
@@ -198,7 +200,10 @@ static void boot_setup(void)
     }
 
     if (have_iplb) {
+        vdev->ipl_type = iplb.pbt;
         menu_setup();
+    } else {
+        vdev->ipl_type = QEMU_DEFAULT_IPL;
     }
 
     memcpy(lpmsg + 10, loadparm_str, 8);
@@ -216,7 +221,7 @@ static bool find_boot_device(void)
     VDev *vdev = virtio_get_device();
     bool found = false;
 
-    switch (iplb.pbt) {
+    switch (vdev->ipl_type) {
     case S390_IPL_TYPE_CCW:
         vdev->scsi_device_selected = false;
         debug_print_int("device no. ", iplb.ccw.devno);
diff --git a/pc-bios/s390-ccw/virtio.h b/pc-bios/s390-ccw/virtio.h
index 06ba4e45ac..391d6ff2f7 100644
--- a/pc-bios/s390-ccw/virtio.h
+++ b/pc-bios/s390-ccw/virtio.h
@@ -238,6 +238,7 @@ struct VDev {
     VirtioGDN guessed_disk_nature;
     SubChannelId schid;
     SenseId senseid;
+    S390IplType ipl_type;
     union {
         VirtioBlkConfig blk;
         VirtioScsiConfig scsi;
-- 
2.52.0



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

* [PATCH v4 05/15] pc-bios/s390-ccw: Store device type independent of sense data
  2026-03-04  2:59 [PATCH v4 00/15] s390x: Add support for virtio-blk-pci IPL device jrossi
                   ` (3 preceding siblings ...)
  2026-03-04  2:59 ` [PATCH v4 04/15] s390x: Remove duplicate definitions of IPL types jrossi
@ 2026-03-04  2:59 ` jrossi
  2026-03-04  2:59 ` [PATCH v4 06/15] pc-bios/s390-ccw: Split virtio-ccw and generic virtio jrossi
                   ` (9 subsequent siblings)
  14 siblings, 0 replies; 35+ messages in thread
From: jrossi @ 2026-03-04  2:59 UTC (permalink / raw)
  To: qemu-devel, qemu-s390x, thuth, mst
  Cc: jjherne, alifm, farman, mjrosato, jrossi, zycai

From: Jared Rossi <jrossi@linux.ibm.com>

Store the device type (e.g. block) directly as an attribute of the VDev rather
than assume all devices can be identified by accessing CCW specific sense data.

Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Farhan Ali <alifm@linux.ibm.com>
Signed-off-by: Jared Rossi <jrossi@linux.ibm.com>
---
 pc-bios/s390-ccw/main.c          |  2 +-
 pc-bios/s390-ccw/virtio-blkdev.c | 39 +++++++++++++++++++-------------
 pc-bios/s390-ccw/virtio.c        | 11 ++++++---
 pc-bios/s390-ccw/virtio.h        |  1 +
 4 files changed, 33 insertions(+), 20 deletions(-)

diff --git a/pc-bios/s390-ccw/main.c b/pc-bios/s390-ccw/main.c
index ef5acc1985..b3b16f845b 100644
--- a/pc-bios/s390-ccw/main.c
+++ b/pc-bios/s390-ccw/main.c
@@ -250,7 +250,7 @@ static int virtio_setup(void)
     vdev->is_cdrom = false;
     int ret;
 
-    switch (vdev->senseid.cu_model) {
+    switch (vdev->dev_type) {
     case VIRTIO_ID_NET:
         puts("Network boot device detected");
         return 0;
diff --git a/pc-bios/s390-ccw/virtio-blkdev.c b/pc-bios/s390-ccw/virtio-blkdev.c
index 019c2718b1..9cc40e9108 100644
--- a/pc-bios/s390-ccw/virtio-blkdev.c
+++ b/pc-bios/s390-ccw/virtio-blkdev.c
@@ -53,14 +53,14 @@ int virtio_read_many(unsigned long sector, void *load_addr, int sec_num)
 {
     VDev *vdev = virtio_get_device();
 
-    switch (vdev->senseid.cu_model) {
+    switch (vdev->dev_type) {
     case VIRTIO_ID_BLOCK:
         return virtio_blk_read_many(vdev, sector, load_addr, sec_num);
     case VIRTIO_ID_SCSI:
         return virtio_scsi_read_many(vdev, sector, load_addr, sec_num);
+    default:
+        return -1;
     }
-
-    return -1;
 }
 
 unsigned long virtio_load_direct(unsigned long rec_list1, unsigned long rec_list2,
@@ -119,7 +119,7 @@ void virtio_assume_iso9660(void)
 {
     VDev *vdev = virtio_get_device();
 
-    switch (vdev->senseid.cu_model) {
+    switch (vdev->dev_type) {
     case VIRTIO_ID_BLOCK:
         vdev->guessed_disk_nature = VIRTIO_GDN_SCSI;
         vdev->config.blk.blk_size = VIRTIO_ISO_BLOCK_SIZE;
@@ -129,6 +129,8 @@ void virtio_assume_iso9660(void)
     case VIRTIO_ID_SCSI:
         vdev->scsi_block_size = VIRTIO_ISO_BLOCK_SIZE;
         break;
+    default:
+        return;
     }
 }
 
@@ -139,13 +141,15 @@ void virtio_assume_eckd(void)
     vdev->guessed_disk_nature = VIRTIO_GDN_DASD;
     vdev->blk_factor = 1;
     vdev->config.blk.physical_block_exp = 0;
-    switch (vdev->senseid.cu_model) {
+    switch (vdev->dev_type) {
     case VIRTIO_ID_BLOCK:
         vdev->config.blk.blk_size = VIRTIO_DASD_DEFAULT_BLOCK_SIZE;
         break;
     case VIRTIO_ID_SCSI:
         vdev->config.blk.blk_size = vdev->scsi_block_size;
         break;
+    default:
+        break;
     }
     vdev->config.blk.geometry.heads = 15;
     vdev->config.blk.geometry.sectors =
@@ -162,50 +166,52 @@ bool virtio_ipl_disk_is_valid(void)
         return true;
     }
 
-    return (vdev->senseid.cu_model == VIRTIO_ID_BLOCK ||
-            vdev->senseid.cu_model == VIRTIO_ID_SCSI) &&
-           blksize >= 512 && blksize <= 4096;
+    return (vdev->dev_type == VIRTIO_ID_BLOCK || vdev->dev_type == VIRTIO_ID_SCSI)
+            && blksize >= 512 && blksize <= 4096;
 }
 
 int virtio_get_block_size(void)
 {
     VDev *vdev = virtio_get_device();
 
-    switch (vdev->senseid.cu_model) {
+    switch (vdev->dev_type) {
     case VIRTIO_ID_BLOCK:
         return vdev->config.blk.blk_size;
     case VIRTIO_ID_SCSI:
         return vdev->scsi_block_size;
+    default:
+        return 0;
     }
-    return 0;
 }
 
 uint8_t virtio_get_heads(void)
 {
     VDev *vdev = virtio_get_device();
 
-    switch (vdev->senseid.cu_model) {
+    switch (vdev->dev_type) {
     case VIRTIO_ID_BLOCK:
         return vdev->config.blk.geometry.heads;
     case VIRTIO_ID_SCSI:
         return vdev->guessed_disk_nature == VIRTIO_GDN_DASD
                ? vdev->config.blk.geometry.heads : 255;
+    default:
+        return 0;
     }
-    return 0;
 }
 
 uint8_t virtio_get_sectors(void)
 {
     VDev *vdev = virtio_get_device();
 
-    switch (vdev->senseid.cu_model) {
+    switch (vdev->dev_type) {
     case VIRTIO_ID_BLOCK:
         return vdev->config.blk.geometry.sectors;
     case VIRTIO_ID_SCSI:
         return vdev->guessed_disk_nature == VIRTIO_GDN_DASD
                ? vdev->config.blk.geometry.sectors : 63;
+    default:
+        return 0;
     }
-    return 0;
 }
 
 uint64_t virtio_get_blocks(void)
@@ -213,13 +219,14 @@ uint64_t virtio_get_blocks(void)
     VDev *vdev = virtio_get_device();
     const uint64_t factor = virtio_get_block_size() / VIRTIO_SECTOR_SIZE;
 
-    switch (vdev->senseid.cu_model) {
+    switch (vdev->dev_type) {
     case VIRTIO_ID_BLOCK:
         return vdev->config.blk.capacity / factor;
     case VIRTIO_ID_SCSI:
         return vdev->scsi_last_block / factor;
+    default:
+        return 0;
     }
-    return 0;
 }
 
 int virtio_blk_setup_device(SubChannelId schid)
diff --git a/pc-bios/s390-ccw/virtio.c b/pc-bios/s390-ccw/virtio.c
index f384a990dc..5dd407d5c9 100644
--- a/pc-bios/s390-ccw/virtio.c
+++ b/pc-bios/s390-ccw/virtio.c
@@ -41,7 +41,7 @@ VDev *virtio_get_device(void)
 
 VirtioDevType virtio_get_device_type(void)
 {
-    return vdev.senseid.cu_model;
+    return vdev.dev_type;
 }
 
 /* virtio spec v1.0 para 4.3.3.2 */
@@ -248,7 +248,7 @@ int virtio_setup_ccw(VDev *vdev)
         return -EIO;
     }
 
-    switch (vdev->senseid.cu_model) {
+    switch (vdev->dev_type) {
     case VIRTIO_ID_NET:
         vdev->nr_vqs = 2;
         vdev->cmd_vr_idx = 0;
@@ -346,12 +346,17 @@ bool virtio_is_supported(SubChannelId schid)
                 true)) {
         return false;
     }
+
+    vdev.dev_type = vdev.senseid.cu_model;
+
     if (vdev.senseid.cu_type == 0x3832) {
-        switch (vdev.senseid.cu_model) {
+        switch (vdev.dev_type) {
         case VIRTIO_ID_BLOCK:
         case VIRTIO_ID_SCSI:
         case VIRTIO_ID_NET:
             return true;
+        default:
+            return false;
         }
     }
     return false;
diff --git a/pc-bios/s390-ccw/virtio.h b/pc-bios/s390-ccw/virtio.h
index 391d6ff2f7..39b507b221 100644
--- a/pc-bios/s390-ccw/virtio.h
+++ b/pc-bios/s390-ccw/virtio.h
@@ -239,6 +239,7 @@ struct VDev {
     SubChannelId schid;
     SenseId senseid;
     S390IplType ipl_type;
+    VirtioDevType dev_type;
     union {
         VirtioBlkConfig blk;
         VirtioScsiConfig scsi;
-- 
2.52.0



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

* [PATCH v4 06/15] pc-bios/s390-ccw: Split virtio-ccw and generic virtio
  2026-03-04  2:59 [PATCH v4 00/15] s390x: Add support for virtio-blk-pci IPL device jrossi
                   ` (4 preceding siblings ...)
  2026-03-04  2:59 ` [PATCH v4 05/15] pc-bios/s390-ccw: Store device type independent of sense data jrossi
@ 2026-03-04  2:59 ` jrossi
  2026-03-04  8:42   ` Thomas Huth
  2026-03-05 15:43   ` Eric Farman
  2026-03-04  2:59 ` [PATCH v4 07/15] include/hw/s390x: Move CLP definitions for easier BIOS access jrossi
                   ` (8 subsequent siblings)
  14 siblings, 2 replies; 35+ messages in thread
From: jrossi @ 2026-03-04  2:59 UTC (permalink / raw)
  To: qemu-devel, qemu-s390x, thuth, mst
  Cc: jjherne, alifm, farman, mjrosato, jrossi, zycai

From: Jared Rossi <jrossi@linux.ibm.com>

Separate the CCW specific virtio routines and create generic wrappers for easier
reuse of existing virtio functions with non-CCW devices.

Signed-off-by: Jared Rossi <jrossi@linux.ibm.com>
---
 pc-bios/s390-ccw/Makefile        |   3 +-
 pc-bios/s390-ccw/main.c          |   8 +-
 pc-bios/s390-ccw/netmain.c       |   2 +-
 pc-bios/s390-ccw/s390-ccw.h      |   3 -
 pc-bios/s390-ccw/virtio-blkdev.c |  15 +-
 pc-bios/s390-ccw/virtio-ccw.c    | 241 +++++++++++++++++++++++++++++++
 pc-bios/s390-ccw/virtio-ccw.h    |  24 +++
 pc-bios/s390-ccw/virtio-net.c    |   3 +-
 pc-bios/s390-ccw/virtio-scsi.c   |   8 +-
 pc-bios/s390-ccw/virtio-scsi.h   |   2 +-
 pc-bios/s390-ccw/virtio.c        | 239 +++++-------------------------
 pc-bios/s390-ccw/virtio.h        |   5 +-
 12 files changed, 332 insertions(+), 221 deletions(-)
 create mode 100644 pc-bios/s390-ccw/virtio-ccw.c
 create mode 100644 pc-bios/s390-ccw/virtio-ccw.h

diff --git a/pc-bios/s390-ccw/Makefile b/pc-bios/s390-ccw/Makefile
index a0f24c94a8..259cff09db 100644
--- a/pc-bios/s390-ccw/Makefile
+++ b/pc-bios/s390-ccw/Makefile
@@ -34,7 +34,8 @@ QEMU_DGFLAGS = -MMD -MP -MT $@ -MF $(@D)/$(*F).d
 .PHONY : all clean build-all distclean
 
 OBJECTS = start.o main.o bootmap.o jump2ipl.o sclp.o menu.o netmain.o \
-	  virtio.o virtio-net.o virtio-scsi.o virtio-blkdev.o cio.o dasd-ipl.o
+	  virtio.o virtio-net.o virtio-scsi.o virtio-blkdev.o cio.o dasd-ipl.o \
+	  virtio-ccw.o
 
 SLOF_DIR := $(SRC_PATH)/../../roms/SLOF
 
diff --git a/pc-bios/s390-ccw/main.c b/pc-bios/s390-ccw/main.c
index b3b16f845b..c5ee575385 100644
--- a/pc-bios/s390-ccw/main.c
+++ b/pc-bios/s390-ccw/main.c
@@ -71,6 +71,7 @@ static int is_dev_possibly_bootable(int dev_no, int sch_no)
     bool is_virtio;
     Schib schib;
     int r;
+    VDev *vdev = virtio_get_device();
 
     blk_schid.sch_no = sch_no;
     r = stsch_err(blk_schid, &schib);
@@ -91,7 +92,8 @@ static int is_dev_possibly_bootable(int dev_no, int sch_no)
      * Note: we always have to run virtio_is_supported() here to make
      * sure that the vdev.senseid data gets pre-initialized correctly
      */
-    is_virtio = virtio_is_supported(blk_schid);
+    vdev->schid = blk_schid;
+    is_virtio = virtio_is_supported(vdev);
 
     /* No specific devno given, just return whether the device is possibly bootable */
     if (dev_no < 0) {
@@ -255,10 +257,10 @@ static int virtio_setup(void)
         puts("Network boot device detected");
         return 0;
     case VIRTIO_ID_BLOCK:
-        ret = virtio_blk_setup_device(blk_schid);
+        ret = virtio_blk_setup_device(vdev);
         break;
     case VIRTIO_ID_SCSI:
-        ret = virtio_scsi_setup_device(blk_schid);
+        ret = virtio_scsi_setup_device(vdev);
         break;
     default:
         puts("\n! No IPL device available !\n");
diff --git a/pc-bios/s390-ccw/netmain.c b/pc-bios/s390-ccw/netmain.c
index a9521dff41..651cedf6ef 100644
--- a/pc-bios/s390-ccw/netmain.c
+++ b/pc-bios/s390-ccw/netmain.c
@@ -500,7 +500,7 @@ static bool find_net_dev(Schib *schib, int dev_no)
             continue;
         }
         enable_subchannel(net_schid);
-        if (!virtio_is_supported(net_schid)) {
+        if (!virtio_is_supported(virtio_get_device())) {
             continue;
         }
         if (virtio_get_device_type() != VIRTIO_ID_NET) {
diff --git a/pc-bios/s390-ccw/s390-ccw.h b/pc-bios/s390-ccw/s390-ccw.h
index 47ea66bd4d..ccd68ff0a4 100644
--- a/pc-bios/s390-ccw/s390-ccw.h
+++ b/pc-bios/s390-ccw/s390-ccw.h
@@ -66,9 +66,6 @@ void sclp_setup(void);
 void sclp_get_loadparm_ascii(char *loadparm);
 int sclp_read(char *str, size_t count);
 
-/* virtio.c */
-bool virtio_is_supported(SubChannelId schid);
-
 /* bootmap.c */
 void zipl_load(void);
 
diff --git a/pc-bios/s390-ccw/virtio-blkdev.c b/pc-bios/s390-ccw/virtio-blkdev.c
index 9cc40e9108..9722b6970f 100644
--- a/pc-bios/s390-ccw/virtio-blkdev.c
+++ b/pc-bios/s390-ccw/virtio-blkdev.c
@@ -12,6 +12,7 @@
 #include "s390-ccw.h"
 #include "virtio.h"
 #include "virtio-scsi.h"
+#include "virtio-ccw.h"
 
 #define VIRTIO_BLK_F_GEOMETRY   (1 << 4)
 #define VIRTIO_BLK_F_BLK_SIZE   (1 << 6)
@@ -229,15 +230,17 @@ uint64_t virtio_get_blocks(void)
     }
 }
 
-int virtio_blk_setup_device(SubChannelId schid)
+int virtio_blk_setup_device(VDev *vdev)
 {
-    VDev *vdev = virtio_get_device();
-
     vdev->guest_features[0] = VIRTIO_BLK_F_GEOMETRY | VIRTIO_BLK_F_BLK_SIZE;
-    vdev->schid = schid;
-    virtio_setup_ccw(vdev);
 
     puts("Using virtio-blk.");
 
-    return 0;
+    switch (vdev->ipl_type) {
+    case S390_IPL_TYPE_QEMU_SCSI:
+    case S390_IPL_TYPE_CCW:
+        return virtio_ccw_setup(vdev);
+    default:
+        return 1;
+    }
 }
diff --git a/pc-bios/s390-ccw/virtio-ccw.c b/pc-bios/s390-ccw/virtio-ccw.c
new file mode 100644
index 0000000000..ab98da90c3
--- /dev/null
+++ b/pc-bios/s390-ccw/virtio-ccw.c
@@ -0,0 +1,241 @@
+/*
+ * Virtio functionality for CCW devices
+ *
+ * Copyright (c) 2013 Alexander Graf <agraf@suse.de>
+ * Copyright 2025 IBM Corp.
+ *
+ * Author(s): Jared Rossi <jrossi@linux.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or (at
+ * your option) any later version. See the COPYING file in the top-level
+ * directory.
+ */
+
+#include <string.h>
+#include "s390-ccw.h"
+#include "cio.h"
+#include "virtio.h"
+#include "virtio-ccw.h"
+#include "virtio-scsi.h"
+#include "bswap.h"
+#include "helper.h"
+#include "s390-time.h"
+
+/* virtio spec v1.0 para 4.3.3.2 */
+static long kvm_hypercall(unsigned long nr, unsigned long param1,
+                          unsigned long param2, unsigned long param3)
+{
+    register unsigned long r_nr asm("1") = nr;
+    register unsigned long r_param1 asm("2") = param1;
+    register unsigned long r_param2 asm("3") = param2;
+    register unsigned long r_param3 asm("4") = param3;
+    register long retval asm("2");
+
+    asm volatile ("diag %%r2,%%r4,0x500"
+                  : "=d" (retval)
+                  : "d" (r_nr), "0" (r_param1), "r"(r_param2), "d"(r_param3)
+                  : "memory", "cc");
+
+    return retval;
+}
+
+static int run_ccw(VDev *vdev, int cmd, void *ptr, int len, bool sli)
+{
+    Ccw1 ccw = {};
+
+    ccw.cmd_code = cmd;
+    ccw.cda = (long)ptr;
+    ccw.count = len;
+
+    if (sli) {
+        ccw.flags |= CCW_FLAG_SLI;
+    }
+
+    return do_cio(vdev->schid, vdev->senseid.cu_type, ptr2u32(&ccw), CCW_FMT1);
+}
+
+bool virtio_ccw_is_supported(VDev *vdev)
+{
+    memset(&vdev->senseid, 0, sizeof(vdev->senseid));
+
+    /*
+     * Run sense id command.
+     * The size of the senseid data differs between devices (notably,
+     * between virtio devices and dasds), so specify the largest possible
+     * size and suppress the incorrect length indication for smaller sizes.
+     */
+    if (run_ccw(vdev, CCW_CMD_SENSE_ID, &vdev->senseid, sizeof(vdev->senseid),
+                true)) {
+        return false;
+    }
+
+    vdev->dev_type = vdev->senseid.cu_model;
+
+    if (vdev->senseid.cu_type == 0x3832) {
+        switch (vdev->dev_type) {
+        case VIRTIO_ID_BLOCK:
+        case VIRTIO_ID_SCSI:
+        case VIRTIO_ID_NET:
+            return true;
+        default:
+            return false;
+        }
+    }
+    return false;
+}
+
+int drain_irqs_ccw(SubChannelId schid)
+{
+    Irb irb = {};
+    int r = 0;
+
+    while (1) {
+        /* FIXME: make use of TPI, for that enable subchannel and isc */
+        if (tsch(schid, &irb)) {
+            /* Might want to differentiate error codes later on. */
+            if (irb.scsw.cstat) {
+                r = -EIO;
+            } else if (irb.scsw.dstat != 0xc) {
+                r = -EIO;
+            }
+            return r;
+        }
+    }
+}
+
+long virtio_ccw_notify(SubChannelId schid, int vq_idx, long cookie)
+{
+    return kvm_hypercall(KVM_S390_VIRTIO_CCW_NOTIFY, *(u32 *)&schid,
+                         vq_idx, cookie);
+}
+
+int virtio_ccw_run(VDev *vdev, int vqid, VirtioCmd *cmd)
+{
+    VRing *vr = &vdev->vrings[vqid];
+    int i = 0;
+
+    do {
+        vring_send_buf(vr, cmd[i].data, cmd[i].size,
+                       cmd[i].flags | (i ? VRING_HIDDEN_IS_CHAIN : 0));
+    } while (cmd[i++].flags & VRING_DESC_F_NEXT);
+
+    vring_wait_reply();
+    if (drain_irqs()) {
+        return -1;
+    }
+    return 0;
+}
+
+int virtio_ccw_reset(VDev *vdev)
+{
+    return run_ccw(vdev, CCW_CMD_VDEV_RESET, NULL, 0, false);
+}
+
+int virtio_ccw_setup(VDev *vdev)
+{
+    int i, cfg_size = 0;
+    uint8_t status;
+    struct VirtioFeatureDesc {
+        uint32_t features;
+        uint8_t index;
+    } __attribute__((packed)) feats;
+
+    if (!virtio_ccw_is_supported(vdev)) {
+        puts("Virtio unsupported for this device ID");
+        return -ENODEV;
+    }
+    /* device ID has been established now */
+
+    vdev->config.blk.blk_size = 0; /* mark "illegal" - setup started... */
+    vdev->guessed_disk_nature = VIRTIO_GDN_NONE;
+
+    virtio_reset(vdev);
+
+    status = VIRTIO_CONFIG_S_ACKNOWLEDGE;
+    if (run_ccw(vdev, CCW_CMD_WRITE_STATUS, &status, sizeof(status), false)) {
+        puts("Could not write ACKNOWLEDGE status to host");
+        return -EIO;
+    }
+
+    switch (vdev->dev_type) {
+    case VIRTIO_ID_NET:
+        vdev->nr_vqs = 2;
+        vdev->cmd_vr_idx = 0;
+        cfg_size = sizeof(vdev->config.net);
+        break;
+    case VIRTIO_ID_BLOCK:
+        vdev->nr_vqs = 1;
+        vdev->cmd_vr_idx = 0;
+        cfg_size = sizeof(vdev->config.blk);
+        break;
+    case VIRTIO_ID_SCSI:
+        vdev->nr_vqs = 3;
+        vdev->cmd_vr_idx = VR_REQUEST;
+        cfg_size = sizeof(vdev->config.scsi);
+        break;
+    default:
+        puts("Unsupported virtio device");
+        return -ENODEV;
+    }
+
+    status |= VIRTIO_CONFIG_S_DRIVER;
+    if (run_ccw(vdev, CCW_CMD_WRITE_STATUS, &status, sizeof(status), false)) {
+        puts("Could not write DRIVER status to host");
+        return -EIO;
+    }
+
+    /* Feature negotiation */
+    for (i = 0; i < ARRAY_SIZE(vdev->guest_features); i++) {
+        feats.features = 0;
+        feats.index = i;
+        if (run_ccw(vdev, CCW_CMD_READ_FEAT, &feats, sizeof(feats), false)) {
+            puts("Could not get features bits");
+            return -EIO;
+        }
+
+        vdev->guest_features[i] &= bswap32(feats.features);
+        feats.features = bswap32(vdev->guest_features[i]);
+        if (run_ccw(vdev, CCW_CMD_WRITE_FEAT, &feats, sizeof(feats), false)) {
+            puts("Could not set features bits");
+            return -EIO;
+        }
+    }
+
+    if (run_ccw(vdev, CCW_CMD_READ_CONF, &vdev->config, cfg_size, false)) {
+        puts("Could not get virtio device configuration");
+        return -EIO;
+    }
+
+    for (i = 0; i < vdev->nr_vqs; i++) {
+        VqInfo info = {
+            .queue = (unsigned long long) virtio_get_ring_area(i),
+            .align = KVM_S390_VIRTIO_RING_ALIGN,
+            .index = i,
+            .num = 0,
+        };
+        VqConfig config = {
+            .index = i,
+            .num = 0,
+        };
+
+        if (run_ccw(vdev, CCW_CMD_READ_VQ_CONF, &config, sizeof(config),
+                false)) {
+            puts("Could not get virtio device VQ config");
+            return -EIO;
+        }
+        info.num = config.num;
+        vring_init(&vdev->vrings[i], &info);
+        if (run_ccw(vdev, CCW_CMD_SET_VQ, &info, sizeof(info), false)) {
+            puts("Cannot set VQ info");
+            return -EIO;
+        }
+    }
+
+    status |= VIRTIO_CONFIG_S_DRIVER_OK;
+    if (run_ccw(vdev, CCW_CMD_WRITE_STATUS, &status, sizeof(status), false)) {
+        puts("Could not write DRIVER_OK status to host");
+        return -EIO;
+    }
+
+    return 0;
+}
diff --git a/pc-bios/s390-ccw/virtio-ccw.h b/pc-bios/s390-ccw/virtio-ccw.h
new file mode 100644
index 0000000000..a506767eaa
--- /dev/null
+++ b/pc-bios/s390-ccw/virtio-ccw.h
@@ -0,0 +1,24 @@
+/*
+ * Virtio definitions for CCW devices
+ *
+ * Copyright 2025 IBM Corp.
+ * Author(s): Jared Rossi <jrossi@linux.ibm.com>
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#ifndef VIRTIO_CCW_H
+#define VIRTIO_CCW_H
+
+/* main.c */
+extern SubChannelId blk_schid;
+
+/* virtio-ccw.c */
+int drain_irqs_ccw(SubChannelId schid);
+bool virtio_ccw_is_supported(VDev *vdev);
+int virtio_ccw_run(VDev *vdev, int vqid, VirtioCmd *cmd);
+long virtio_ccw_notify(SubChannelId schid, int vq_idx, long cookie);
+int virtio_ccw_setup(VDev *vdev);
+int virtio_ccw_reset(VDev *vdev);
+
+#endif
diff --git a/pc-bios/s390-ccw/virtio-net.c b/pc-bios/s390-ccw/virtio-net.c
index 7eb0850069..f58f7ffc55 100644
--- a/pc-bios/s390-ccw/virtio-net.c
+++ b/pc-bios/s390-ccw/virtio-net.c
@@ -19,6 +19,7 @@
 #include <ethernet.h>
 #include "s390-ccw.h"
 #include "virtio.h"
+#include "virtio-ccw.h"
 #include "s390-time.h"
 #include "helper.h"
 
@@ -54,7 +55,7 @@ int virtio_net_init(void *mac_addr)
     rx_last_idx = 0;
 
     vdev->guest_features[0] = VIRTIO_NET_F_MAC_BIT;
-    virtio_setup_ccw(vdev);
+    virtio_ccw_setup(vdev);
 
     if (!(vdev->guest_features[0] & VIRTIO_NET_F_MAC_BIT)) {
         puts("virtio-net device does not support the MAC address feature");
diff --git a/pc-bios/s390-ccw/virtio-scsi.c b/pc-bios/s390-ccw/virtio-scsi.c
index 71db75ce7b..9ea00c6fe6 100644
--- a/pc-bios/s390-ccw/virtio-scsi.c
+++ b/pc-bios/s390-ccw/virtio-scsi.c
@@ -15,6 +15,7 @@
 #include "virtio.h"
 #include "scsi.h"
 #include "virtio-scsi.h"
+#include "virtio-ccw.h"
 #include "s390-time.h"
 #include "helper.h"
 
@@ -476,12 +477,9 @@ static int virtio_scsi_setup(VDev *vdev)
     return 0;
 }
 
-int virtio_scsi_setup_device(SubChannelId schid)
+int virtio_scsi_setup_device(VDev *vdev)
 {
-    VDev *vdev = virtio_get_device();
-
-    vdev->schid = schid;
-    virtio_setup_ccw(vdev);
+    virtio_ccw_setup(vdev);
 
     if (vdev->config.scsi.sense_size != VIRTIO_SCSI_SENSE_SIZE) {
         puts("Config: sense size mismatch");
diff --git a/pc-bios/s390-ccw/virtio-scsi.h b/pc-bios/s390-ccw/virtio-scsi.h
index c5612e16a2..070f24b7e5 100644
--- a/pc-bios/s390-ccw/virtio-scsi.h
+++ b/pc-bios/s390-ccw/virtio-scsi.h
@@ -69,6 +69,6 @@ static inline bool virtio_scsi_response_ok(const VirtioScsiCmdResp *r)
 
 int virtio_scsi_read_many(VDev *vdev,
                           unsigned long sector, void *load_addr, int sec_num);
-int virtio_scsi_setup_device(SubChannelId schid);
+int virtio_scsi_setup_device(VDev *vdev);
 
 #endif /* VIRTIO_SCSI_H */
diff --git a/pc-bios/s390-ccw/virtio.c b/pc-bios/s390-ccw/virtio.c
index 5dd407d5c9..956b34ff33 100644
--- a/pc-bios/s390-ccw/virtio.c
+++ b/pc-bios/s390-ccw/virtio.c
@@ -2,6 +2,9 @@
  * Virtio driver bits
  *
  * Copyright (c) 2013 Alexander Graf <agraf@suse.de>
+ * Copyright 2025 IBM Corp.
+ *
+ * Author(s): Jared Rossi <jrossi@linux.ibm.com>
  *
  * This work is licensed under the terms of the GNU GPL, version 2 or (at
  * your option) any later version. See the COPYING file in the top-level
@@ -13,6 +16,7 @@
 #include "cio.h"
 #include "virtio.h"
 #include "virtio-scsi.h"
+#include "virtio-ccw.h"
 #include "bswap.h"
 #include "helper.h"
 #include "s390-time.h"
@@ -44,28 +48,9 @@ VirtioDevType virtio_get_device_type(void)
     return vdev.dev_type;
 }
 
-/* virtio spec v1.0 para 4.3.3.2 */
-static long kvm_hypercall(unsigned long nr, unsigned long param1,
-                          unsigned long param2, unsigned long param3)
+char *virtio_get_ring_area(int ring_num)
 {
-    register unsigned long r_nr asm("1") = nr;
-    register unsigned long r_param1 asm("2") = param1;
-    register unsigned long r_param2 asm("3") = param2;
-    register unsigned long r_param3 asm("4") = param3;
-    register long retval asm("2");
-
-    asm volatile ("diag %%r2,%%r4,0x500"
-                  : "=d" (retval)
-                  : "d" (r_nr), "0" (r_param1), "r"(r_param2), "d"(r_param3)
-                  : "memory", "cc");
-
-    return retval;
-}
-
-static long virtio_notify(SubChannelId schid, int vq_idx, long cookie)
-{
-    return kvm_hypercall(KVM_S390_VIRTIO_CCW_NOTIFY, *(u32 *)&schid,
-                         vq_idx, cookie);
+    return ring_area + ring_num * VIRTIO_RING_SIZE;
 }
 
 /***********************************************
@@ -74,39 +59,27 @@ static long virtio_notify(SubChannelId schid, int vq_idx, long cookie)
 
 int drain_irqs(void)
 {
-    Irb irb = {};
-    int r = 0;
-
-    while (1) {
-        /* FIXME: make use of TPI, for that enable subchannel and isc */
-        if (tsch(vdev.schid, &irb)) {
-            /* Might want to differentiate error codes later on. */
-            if (irb.scsw.cstat) {
-                r = -EIO;
-            } else if (irb.scsw.dstat != 0xc) {
-                r = -EIO;
-            }
-            return r;
-        }
+    switch (vdev.ipl_type) {
+    case S390_IPL_TYPE_QEMU_SCSI:
+    case S390_IPL_TYPE_CCW:
+        return drain_irqs_ccw(vdev.schid);
+    default:
+        return 0;
     }
 }
 
-static int run_ccw(VDev *vdev, int cmd, void *ptr, int len, bool sli)
+int virtio_run(VDev *vdev, int vqid, VirtioCmd *cmd)
 {
-    Ccw1 ccw = {};
-
-    ccw.cmd_code = cmd;
-    ccw.cda = (long)ptr;
-    ccw.count = len;
-
-    if (sli) {
-        ccw.flags |= CCW_FLAG_SLI;
+    switch (vdev->ipl_type) {
+    case S390_IPL_TYPE_QEMU_SCSI:
+    case S390_IPL_TYPE_CCW:
+        return virtio_ccw_run(vdev, vqid, cmd);
+    default:
+        return -1;
     }
-
-    return do_cio(vdev->schid, vdev->senseid.cu_type, ptr2u32(&ccw), CCW_FMT1);
 }
 
-static void vring_init(VRing *vr, VqInfo *info)
+void vring_init(VRing *vr, VqInfo *info)
 {
     void *p = (void *) info->queue;
 
@@ -134,7 +107,15 @@ static void vring_init(VRing *vr, VqInfo *info)
 
 bool vring_notify(VRing *vr)
 {
-    vr->cookie = virtio_notify(vdev.schid, vr->id, vr->cookie);
+    switch (vdev.ipl_type) {
+    case S390_IPL_TYPE_QEMU_SCSI:
+    case S390_IPL_TYPE_CCW:
+        vr->cookie = virtio_ccw_notify(vdev.schid, vr->id, vr->cookie);
+        break;
+    default:
+        return 1;
+    }
+
     return vr->cookie >= 0;
 }
 
@@ -200,164 +181,24 @@ int vring_wait_reply(void)
     return 1;
 }
 
-int virtio_run(VDev *vdev, int vqid, VirtioCmd *cmd)
-{
-    VRing *vr = &vdev->vrings[vqid];
-    int i = 0;
-
-    do {
-        vring_send_buf(vr, cmd[i].data, cmd[i].size,
-                       cmd[i].flags | (i ? VRING_HIDDEN_IS_CHAIN : 0));
-    } while (cmd[i++].flags & VRING_DESC_F_NEXT);
-
-    vring_wait_reply();
-    if (drain_irqs()) {
-        return -1;
-    }
-    return 0;
-}
-
 int virtio_reset(VDev *vdev)
 {
-    return run_ccw(vdev, CCW_CMD_VDEV_RESET, NULL, 0, false);
-}
-
-int virtio_setup_ccw(VDev *vdev)
-{
-    int i, cfg_size = 0;
-    uint8_t status;
-    struct VirtioFeatureDesc {
-        uint32_t features;
-        uint8_t index;
-    } __attribute__((packed)) feats;
-
-    if (!virtio_is_supported(vdev->schid)) {
-        puts("Virtio unsupported for this device ID");
-        return -ENODEV;
-    }
-    /* device ID has been established now */
-
-    vdev->config.blk.blk_size = 0; /* mark "illegal" - setup started... */
-    vdev->guessed_disk_nature = VIRTIO_GDN_NONE;
-
-    virtio_reset(vdev);
-
-    status = VIRTIO_CONFIG_S_ACKNOWLEDGE;
-    if (run_ccw(vdev, CCW_CMD_WRITE_STATUS, &status, sizeof(status), false)) {
-        puts("Could not write ACKNOWLEDGE status to host");
-        return -EIO;
-    }
-
-    switch (vdev->dev_type) {
-    case VIRTIO_ID_NET:
-        vdev->nr_vqs = 2;
-        vdev->cmd_vr_idx = 0;
-        cfg_size = sizeof(vdev->config.net);
-        break;
-    case VIRTIO_ID_BLOCK:
-        vdev->nr_vqs = 1;
-        vdev->cmd_vr_idx = 0;
-        cfg_size = sizeof(vdev->config.blk);
-        break;
-    case VIRTIO_ID_SCSI:
-        vdev->nr_vqs = 3;
-        vdev->cmd_vr_idx = VR_REQUEST;
-        cfg_size = sizeof(vdev->config.scsi);
-        break;
+    switch (vdev->ipl_type) {
+    case S390_IPL_TYPE_QEMU_SCSI:
+    case S390_IPL_TYPE_CCW:
+        return virtio_ccw_reset(vdev);
     default:
-        puts("Unsupported virtio device");
-        return -ENODEV;
-    }
-
-    status |= VIRTIO_CONFIG_S_DRIVER;
-    if (run_ccw(vdev, CCW_CMD_WRITE_STATUS, &status, sizeof(status), false)) {
-        puts("Could not write DRIVER status to host");
-        return -EIO;
-    }
-
-    /* Feature negotiation */
-    for (i = 0; i < ARRAY_SIZE(vdev->guest_features); i++) {
-        feats.features = 0;
-        feats.index = i;
-        if (run_ccw(vdev, CCW_CMD_READ_FEAT, &feats, sizeof(feats), false)) {
-            puts("Could not get features bits");
-            return -EIO;
-        }
-
-        vdev->guest_features[i] &= bswap32(feats.features);
-        feats.features = bswap32(vdev->guest_features[i]);
-        if (run_ccw(vdev, CCW_CMD_WRITE_FEAT, &feats, sizeof(feats), false)) {
-            puts("Could not set features bits");
-            return -EIO;
-        }
-    }
-
-    if (run_ccw(vdev, CCW_CMD_READ_CONF, &vdev->config, cfg_size, false)) {
-        puts("Could not get virtio device configuration");
-        return -EIO;
-    }
-
-    for (i = 0; i < vdev->nr_vqs; i++) {
-        VqInfo info = {
-            .queue = (unsigned long long) ring_area + (i * VIRTIO_RING_SIZE),
-            .align = KVM_S390_VIRTIO_RING_ALIGN,
-            .index = i,
-            .num = 0,
-        };
-        VqConfig config = {
-            .index = i,
-            .num = 0,
-        };
-
-        if (run_ccw(vdev, CCW_CMD_READ_VQ_CONF, &config, sizeof(config),
-                false)) {
-            puts("Could not get virtio device VQ config");
-            return -EIO;
-        }
-        info.num = config.num;
-        vring_init(&vdev->vrings[i], &info);
-        if (run_ccw(vdev, CCW_CMD_SET_VQ, &info, sizeof(info), false)) {
-            puts("Cannot set VQ info");
-            return -EIO;
-        }
-    }
-
-    status |= VIRTIO_CONFIG_S_DRIVER_OK;
-    if (run_ccw(vdev, CCW_CMD_WRITE_STATUS, &status, sizeof(status), false)) {
-        puts("Could not write DRIVER_OK status to host");
-        return -EIO;
+        return -1;
     }
-
-    return 0;
 }
 
-bool virtio_is_supported(SubChannelId schid)
+bool virtio_is_supported(VDev *vdev)
 {
-    vdev.schid = schid;
-    memset(&vdev.senseid, 0, sizeof(vdev.senseid));
-
-    /*
-     * Run sense id command.
-     * The size of the senseid data differs between devices (notably,
-     * between virtio devices and dasds), so specify the largest possible
-     * size and suppress the incorrect length indication for smaller sizes.
-     */
-    if (run_ccw(&vdev, CCW_CMD_SENSE_ID, &vdev.senseid, sizeof(vdev.senseid),
-                true)) {
+    switch (vdev->ipl_type) {
+    case S390_IPL_TYPE_QEMU_SCSI:
+    case S390_IPL_TYPE_CCW:
+        return virtio_ccw_is_supported(vdev);
+    default:
         return false;
     }
-
-    vdev.dev_type = vdev.senseid.cu_model;
-
-    if (vdev.senseid.cu_type == 0x3832) {
-        switch (vdev.dev_type) {
-        case VIRTIO_ID_BLOCK:
-        case VIRTIO_ID_SCSI:
-        case VIRTIO_ID_NET:
-            return true;
-        default:
-            return false;
-        }
-    }
-    return false;
 }
diff --git a/pc-bios/s390-ccw/virtio.h b/pc-bios/s390-ccw/virtio.h
index 39b507b221..c3cb5a6ee3 100644
--- a/pc-bios/s390-ccw/virtio.h
+++ b/pc-bios/s390-ccw/virtio.h
@@ -108,6 +108,7 @@ struct VRing {
 };
 typedef struct VRing VRing;
 
+char *virtio_get_ring_area(int ring_num);
 
 /***********************************************
  *               Virtio block                  *
@@ -269,6 +270,8 @@ struct VirtioCmd {
 };
 typedef struct VirtioCmd VirtioCmd;
 
+void vring_init(VRing *vr, VqInfo *info);
+bool virtio_is_supported(VDev *vdev);
 bool vring_notify(VRing *vr);
 int drain_irqs(void);
 void vring_send_buf(VRing *vr, void *p, int len, int flags);
@@ -283,7 +286,7 @@ int virtio_net_init(void *mac_addr);
 void virtio_net_deinit(void);
 
 /* virtio-blkdev.c */
-int virtio_blk_setup_device(SubChannelId schid);
+int virtio_blk_setup_device(VDev *vdev);
 int virtio_read(unsigned long sector, void *load_addr);
 unsigned long virtio_load_direct(unsigned long rec_list1, unsigned long rec_list2,
                                  void *load_addr);
-- 
2.52.0



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

* [PATCH v4 07/15] include/hw/s390x: Move CLP definitions for easier BIOS access
  2026-03-04  2:59 [PATCH v4 00/15] s390x: Add support for virtio-blk-pci IPL device jrossi
                   ` (5 preceding siblings ...)
  2026-03-04  2:59 ` [PATCH v4 06/15] pc-bios/s390-ccw: Split virtio-ccw and generic virtio jrossi
@ 2026-03-04  2:59 ` jrossi
  2026-03-04  2:59 ` [PATCH v4 08/15] pc-bios/s390-ccw: Introduce CLP Architecture jrossi
                   ` (7 subsequent siblings)
  14 siblings, 0 replies; 35+ messages in thread
From: jrossi @ 2026-03-04  2:59 UTC (permalink / raw)
  To: qemu-devel, qemu-s390x, thuth, mst
  Cc: jjherne, alifm, farman, mjrosato, jrossi, zycai

From: Jared Rossi <jrossi@linux.ibm.com>

Move the s390-pci-clp definitions into the "ipl" sub-directory, which is visible
to the s390-bios.  This allows the bios to reuse the architected definitions and
prevents code duplication.

Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Eric Farman <farman@linux.ibm.com>
Reviewed-by: Matthew Rosato <mjrosato@linux.ibm.com>
Signed-off-by: Jared Rossi <jrossi@linux.ibm.com>
---
 MAINTAINERS                               | 1 +
 hw/s390x/s390-pci-vfio.c                  | 2 +-
 include/hw/s390x/{ => ipl}/s390-pci-clp.h | 0
 include/hw/s390x/s390-pci-bus.h           | 2 +-
 4 files changed, 3 insertions(+), 2 deletions(-)
 rename include/hw/s390x/{ => ipl}/s390-pci-clp.h (100%)

diff --git a/MAINTAINERS b/MAINTAINERS
index b8317fab31..54a600f76b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1892,6 +1892,7 @@ R: Eric Farman <farman@linux.ibm.com>
 S: Supported
 F: hw/s390x/s390-pci*
 F: include/hw/s390x/s390-pci*
+F: include/hw/s390x/ipl/s390-pci*
 F: util/s390x_pci_mmio.c
 L: qemu-s390x@nongnu.org
 
diff --git a/hw/s390x/s390-pci-vfio.c b/hw/s390x/s390-pci-vfio.c
index 9e31029d7a..8ce44dbecc 100644
--- a/hw/s390x/s390-pci-vfio.c
+++ b/hw/s390x/s390-pci-vfio.c
@@ -17,7 +17,7 @@
 
 #include "trace.h"
 #include "hw/s390x/s390-pci-bus.h"
-#include "hw/s390x/s390-pci-clp.h"
+#include "hw/s390x/ipl/s390-pci-clp.h"
 #include "hw/s390x/s390-pci-vfio.h"
 #include "hw/vfio/pci.h"
 #include "hw/vfio/vfio-container-legacy.h"
diff --git a/include/hw/s390x/s390-pci-clp.h b/include/hw/s390x/ipl/s390-pci-clp.h
similarity index 100%
rename from include/hw/s390x/s390-pci-clp.h
rename to include/hw/s390x/ipl/s390-pci-clp.h
diff --git a/include/hw/s390x/s390-pci-bus.h b/include/hw/s390x/s390-pci-bus.h
index 04944d4fed..f643e13057 100644
--- a/include/hw/s390x/s390-pci-bus.h
+++ b/include/hw/s390x/s390-pci-bus.h
@@ -19,7 +19,7 @@
 #include "hw/s390x/sclp.h"
 #include "hw/s390x/s390_flic.h"
 #include "hw/s390x/css.h"
-#include "hw/s390x/s390-pci-clp.h"
+#include "hw/s390x/ipl/s390-pci-clp.h"
 #include "qom/object.h"
 
 #define TYPE_S390_PCI_HOST_BRIDGE "s390-pcihost"
-- 
2.52.0



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

* [PATCH v4 08/15] pc-bios/s390-ccw: Introduce CLP Architecture
  2026-03-04  2:59 [PATCH v4 00/15] s390x: Add support for virtio-blk-pci IPL device jrossi
                   ` (6 preceding siblings ...)
  2026-03-04  2:59 ` [PATCH v4 07/15] include/hw/s390x: Move CLP definitions for easier BIOS access jrossi
@ 2026-03-04  2:59 ` jrossi
  2026-03-04 14:05   ` Eric Farman
  2026-03-04  2:59 ` [PATCH v4 09/15] s390x: Add definitions for PCI IPL type jrossi
                   ` (6 subsequent siblings)
  14 siblings, 1 reply; 35+ messages in thread
From: jrossi @ 2026-03-04  2:59 UTC (permalink / raw)
  To: qemu-devel, qemu-s390x, thuth, mst
  Cc: jjherne, alifm, farman, mjrosato, jrossi, zycai

From: Jared Rossi <jrossi@linux.ibm.com>

Call Logical Processor (CLP) Architecture is used for managing PCI functions on
s390x. Define and include the structures and routines needed to interact with
PCI devices during IPL.

Acked-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Jared Rossi <jrossi@linux.ibm.com>
---
 pc-bios/s390-ccw/Makefile |  2 +-
 pc-bios/s390-ccw/clp.c    | 99 +++++++++++++++++++++++++++++++++++++++
 pc-bios/s390-ccw/clp.h    | 24 ++++++++++
 3 files changed, 124 insertions(+), 1 deletion(-)
 create mode 100644 pc-bios/s390-ccw/clp.c
 create mode 100644 pc-bios/s390-ccw/clp.h

diff --git a/pc-bios/s390-ccw/Makefile b/pc-bios/s390-ccw/Makefile
index 259cff09db..9c29548f84 100644
--- a/pc-bios/s390-ccw/Makefile
+++ b/pc-bios/s390-ccw/Makefile
@@ -35,7 +35,7 @@ QEMU_DGFLAGS = -MMD -MP -MT $@ -MF $(@D)/$(*F).d
 
 OBJECTS = start.o main.o bootmap.o jump2ipl.o sclp.o menu.o netmain.o \
 	  virtio.o virtio-net.o virtio-scsi.o virtio-blkdev.o cio.o dasd-ipl.o \
-	  virtio-ccw.o
+	  virtio-ccw.o clp.o
 
 SLOF_DIR := $(SRC_PATH)/../../roms/SLOF
 
diff --git a/pc-bios/s390-ccw/clp.c b/pc-bios/s390-ccw/clp.c
new file mode 100644
index 0000000000..8c04738bbf
--- /dev/null
+++ b/pc-bios/s390-ccw/clp.c
@@ -0,0 +1,99 @@
+/*
+ * Call Logical Processor (CLP) architecture
+ *
+ * Copyright 2025 IBM Corp.
+ * Author(s): Jared Rossi <jrossi@linux.ibm.com>
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "clp.h"
+#include <stdio.h>
+#include <string.h>
+
+int clp_pci(void *data)
+{
+    struct { uint8_t _[CLP_BLK_SIZE]; } *req = data;
+    int cc = 3;
+
+    asm volatile (
+        "     .insn   rrf,0xb9a00000,0,%[req],0,2\n"
+        "     ipm     %[cc]\n"
+        "     srl     %[cc],28\n"
+        : [cc] "+d" (cc), "+m" (*req)
+        : [req] "a" (req)
+        : "cc");
+    if (cc) {
+        printf("CLP returned with non-zero condition code %d\n", cc);
+    }
+    return cc;
+}
+
+/*
+ * Get the PCI function entry for a given function ID
+ * Return 0 on success, 1 if the FID is not found, or a negative RC on error
+ */
+int find_pci_function(uint32_t fid, ClpFhListEntry *entry)
+{
+    int count = 0;
+    int limit = PCI_MAX_FUNCTIONS;
+    ClpReqRspListPci rrb;
+
+    rrb.request.hdr.len = sizeof(ClpReqListPci);
+    rrb.request.hdr.cmd = 0x02;
+    rrb.request.resume_token = 0;
+    rrb.response.hdr.len = sizeof(ClpRspListPci);
+
+    do {
+        if (clp_pci(&rrb) || rrb.response.hdr.rsp != 0x0010) {
+            puts("Failed to list PCI functions");
+            return -1;
+        }
+
+        /* Resume token set when max enteries are returned */
+        if (rrb.response.resume_token) {
+            count = CLP_FH_LIST_NR_ENTRIES;
+            rrb.request.resume_token = rrb.response.resume_token;
+        } else {
+            count = (rrb.response.hdr.len - 32) / sizeof(ClpFhListEntry);
+        }
+
+        limit -= count;
+
+        for (int i = 0; i < count; i++) {
+            if (rrb.response.fh_list[i].fid == fid) {
+                memcpy(entry, &rrb.response.fh_list[i], sizeof(ClpFhListEntry));
+                return 0;
+            }
+        }
+
+    } while (rrb.request.resume_token && limit > 0);
+
+    puts("No function entry found for FID!");
+
+    return 1;
+}
+
+/*
+ * Enable the PCI function associated with a given handle
+ * Return 0 on success or a negative RC on error
+ */
+int enable_pci_function(uint32_t *fhandle)
+{
+    ClpReqRspSetPci rrb;
+
+    rrb.request.hdr.len = sizeof(ClpReqSetPci);
+    rrb.request.hdr.cmd = 0x05;
+    rrb.request.fh = *fhandle;
+    rrb.request.oc = 0;
+    rrb.request.ndas = 1;
+    rrb.response.hdr.len = sizeof(ClpRspSetPci);
+
+    if (clp_pci(&rrb) || rrb.response.hdr.rsp != 0x0010) {
+        puts("Failed to enable PCI function");
+        return -1;
+    }
+
+    *fhandle = rrb.response.fh;
+    return 0;
+}
diff --git a/pc-bios/s390-ccw/clp.h b/pc-bios/s390-ccw/clp.h
new file mode 100644
index 0000000000..1ac2f8c177
--- /dev/null
+++ b/pc-bios/s390-ccw/clp.h
@@ -0,0 +1,24 @@
+/*
+ * Call Logical Processor (CLP) architecture definitions
+ *
+ * Copyright 2025 IBM Corp.
+ * Author(s): Jared Rossi <jrossi@linux.ibm.com>
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#ifndef CLP_H
+#define CLP_H
+
+#ifndef QEMU_PACKED
+#define QEMU_PACKED __attribute__((packed))
+#endif
+
+#include <stdint.h>
+#include <s390-pci-clp.h>
+
+int clp_pci(void *data);
+int find_pci_function(uint32_t fid, ClpFhListEntry *entry);
+int enable_pci_function(uint32_t *fhandle);
+
+#endif
-- 
2.52.0



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

* [PATCH v4 09/15] s390x: Add definitions for PCI IPL type
  2026-03-04  2:59 [PATCH v4 00/15] s390x: Add support for virtio-blk-pci IPL device jrossi
                   ` (7 preceding siblings ...)
  2026-03-04  2:59 ` [PATCH v4 08/15] pc-bios/s390-ccw: Introduce CLP Architecture jrossi
@ 2026-03-04  2:59 ` jrossi
  2026-03-04  2:59 ` [PATCH v4 10/15] pc-bios/s390-ccw: Introduce PCI device jrossi
                   ` (5 subsequent siblings)
  14 siblings, 0 replies; 35+ messages in thread
From: jrossi @ 2026-03-04  2:59 UTC (permalink / raw)
  To: qemu-devel, qemu-s390x, thuth, mst
  Cc: jjherne, alifm, farman, mjrosato, jrossi, zycai

From: Jared Rossi <jrossi@linux.ibm.com>

Define a new PBT code and IPLB layout in preparation for supporting PCI device
IPL on s390x.

Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Eric Farman <farman@linux.ibm.com>
Signed-off-by: Jared Rossi <jrossi@linux.ibm.com>
---
 include/hw/s390x/ipl/qipl.h | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/include/hw/s390x/ipl/qipl.h b/include/hw/s390x/ipl/qipl.h
index 6dc12dd859..8d3c83a80b 100644
--- a/include/hw/s390x/ipl/qipl.h
+++ b/include/hw/s390x/ipl/qipl.h
@@ -23,6 +23,7 @@
 enum S390IplType {
     S390_IPL_TYPE_FCP = 0x00,
     S390_IPL_TYPE_CCW = 0x02,
+    S390_IPL_TYPE_PCI = 0x04,
     S390_IPL_TYPE_PV = 0x05,
     S390_IPL_TYPE_QEMU_SCSI = 0xff
 };
@@ -108,6 +109,14 @@ struct IplBlockQemuScsi {
 } QEMU_PACKED;
 typedef struct IplBlockQemuScsi IplBlockQemuScsi;
 
+struct IplBlockPci {
+    uint32_t reserved0[76];
+    uint8_t  opt;
+    uint8_t  reserved1[3];
+    uint32_t fid;
+} QEMU_PACKED;
+typedef struct IplBlockPci IplBlockPci;
+
 union IplParameterBlock {
     struct {
         uint32_t len;
@@ -123,6 +132,7 @@ union IplParameterBlock {
             IplBlockFcp fcp;
             IPLBlockPV pv;
             IplBlockQemuScsi scsi;
+            IplBlockPci pci;
         };
     } QEMU_PACKED;
     struct {
-- 
2.52.0



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

* [PATCH v4 10/15] pc-bios/s390-ccw: Introduce PCI device
  2026-03-04  2:59 [PATCH v4 00/15] s390x: Add support for virtio-blk-pci IPL device jrossi
                   ` (8 preceding siblings ...)
  2026-03-04  2:59 ` [PATCH v4 09/15] s390x: Add definitions for PCI IPL type jrossi
@ 2026-03-04  2:59 ` jrossi
  2026-03-04  9:10   ` Thomas Huth
  2026-03-04  2:59 ` [PATCH v4 11/15] pc-bios/s390-ccw: Introduce virtio-pci functions jrossi
                   ` (4 subsequent siblings)
  14 siblings, 1 reply; 35+ messages in thread
From: jrossi @ 2026-03-04  2:59 UTC (permalink / raw)
  To: qemu-devel, qemu-s390x, thuth, mst
  Cc: jjherne, alifm, farman, mjrosato, jrossi, zycai

From: Jared Rossi <jrossi@linux.ibm.com>

Define selected s390x PCI instructions.

Signed-off-by: Jared Rossi <jrossi@linux.ibm.com>
---
 pc-bios/s390-ccw/Makefile |   2 +-
 pc-bios/s390-ccw/pci.c    | 118 ++++++++++++++++++++++++++++++++++++++
 pc-bios/s390-ccw/pci.h    |  39 +++++++++++++
 3 files changed, 158 insertions(+), 1 deletion(-)
 create mode 100644 pc-bios/s390-ccw/pci.c
 create mode 100644 pc-bios/s390-ccw/pci.h

diff --git a/pc-bios/s390-ccw/Makefile b/pc-bios/s390-ccw/Makefile
index 9c29548f84..a62fc9d766 100644
--- a/pc-bios/s390-ccw/Makefile
+++ b/pc-bios/s390-ccw/Makefile
@@ -35,7 +35,7 @@ QEMU_DGFLAGS = -MMD -MP -MT $@ -MF $(@D)/$(*F).d
 
 OBJECTS = start.o main.o bootmap.o jump2ipl.o sclp.o menu.o netmain.o \
 	  virtio.o virtio-net.o virtio-scsi.o virtio-blkdev.o cio.o dasd-ipl.o \
-	  virtio-ccw.o clp.o
+	  virtio-ccw.o clp.o pci.o
 
 SLOF_DIR := $(SRC_PATH)/../../roms/SLOF
 
diff --git a/pc-bios/s390-ccw/pci.c b/pc-bios/s390-ccw/pci.c
new file mode 100644
index 0000000000..36e86fcc71
--- /dev/null
+++ b/pc-bios/s390-ccw/pci.c
@@ -0,0 +1,118 @@
+/*
+ * s390x PCI functionality
+ *
+ * Copyright 2025 IBM Corp.
+ * Author(s): Jared Rossi <jrossi@linux.ibm.com>
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "clp.h"
+#include "pci.h"
+#include "bswap.h"
+#include <stdio.h>
+#include <stdbool.h>
+
+/* PCI load */
+static inline int pcilg(uint64_t *data, uint64_t req, uint64_t offset,
+                        uint8_t *status)
+{
+    union register_pair req_off = {.even = req, .odd = offset};
+    int cc = -1;
+    uint64_t __data;
+
+    asm volatile (
+        "     .insn   rre,0xb9d20000,%[data],%[req_off]\n"
+        "     ipm     %[cc]\n"
+        "     srl     %[cc],28\n"
+        : [cc] "+d" (cc), [data] "=d" (__data),
+          [req_off] "+d" (req_off.pair) :: "cc");
+    *status = req_off.even >> 24 & 0xff;
+    *data = __data;
+    return cc;
+}
+
+/* PCI store */
+static inline int pcistg(uint64_t data, uint64_t req, uint64_t offset,
+                         uint8_t *status)
+{
+    union register_pair req_off = {.even = req, .odd = offset};
+    int cc = -1;
+
+    asm volatile (
+        "     .insn   rre,0xb9d00000,%[data],%[req_off]\n"
+        "     ipm     %[cc]\n"
+        "     srl     %[cc],28\n"
+        : [cc] "+d" (cc), [req_off] "+d" (req_off.pair)
+        : [data] "d" (data)
+        : "cc");
+    *status = req_off.even >> 24 & 0xff;
+    return cc;
+}
+
+int pci_write(uint32_t fhandle, uint64_t offset, uint8_t pcias, uint64_t data,
+              uint8_t len)
+{
+
+    uint64_t req = ZPCI_CREATE_REQ(fhandle, pcias, len);
+    uint8_t status;
+    int rc;
+
+    /* len must be non-zero power of 2 with a maximum of 8 bytes per write */
+    switch (len) {
+    case 1:
+    case 2:
+    case 4:
+    case 8:
+        rc = pcistg(data, req, offset, &status);
+        break;
+    default:
+        return -1;
+    }
+
+    /* Error condition detected */
+    if (rc != 0) {
+        printf("PCI store failed with status condition %d, return code %d\n",
+               status, rc);
+        return -1;
+    }
+
+    return rc ? -1 : 0;
+}
+
+int pci_read(uint32_t fhandle, uint64_t offset, uint8_t pcias, void *buf,
+             uint8_t len)
+{
+    uint64_t req, data;
+    uint8_t status;
+    int rc;
+
+    req = ZPCI_CREATE_REQ(fhandle, pcias, len);
+    rc = pcilg(&data, req, offset, &status);
+
+    /* Error condition detected */
+    if (rc != 0) {
+        printf("PCI load failed with status condition %d, return code %d\n",
+               status, rc);
+        return -1;
+    }
+
+    switch (len) {
+    case 1:
+        *(uint8_t *)buf = data;
+        break;
+    case 2:
+        *(uint16_t *)buf = data;
+        break;
+    case 4:
+        *(uint32_t *)buf = data;
+        break;
+    case 8:
+        *(uint64_t *)buf = data;
+        break;
+    default:
+        return -1;
+    }
+
+    return rc ? -1 : 0;
+}
diff --git a/pc-bios/s390-ccw/pci.h b/pc-bios/s390-ccw/pci.h
new file mode 100644
index 0000000000..40a0bf9dcb
--- /dev/null
+++ b/pc-bios/s390-ccw/pci.h
@@ -0,0 +1,39 @@
+/*
+ * s390x PCI definitions
+ *
+ * Copyright 2025 IBM Corp.
+ * Author(s): Jared Rossi <jrossi@linux.ibm.com>
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#ifndef PCI_H
+#define PCI_H
+
+#include <stdint.h>
+#include <stdbool.h>
+#include "clp.h"
+
+#define ZPCI_CREATE_REQ(handle, space, len)                    \
+    ((uint64_t) handle << 32 | space << 16 | len)
+
+union register_pair {
+    unsigned __int128 pair;
+    struct {
+        unsigned long even;
+        unsigned long odd;
+    };
+};
+
+#define PCIST_DISABLED         0x0
+#define PCIST_ENABLED          0x1
+
+#define PCI_CFGBAR             0xF  /* Base Address Register for config space */
+#define PCI_CAPABILITY_LIST    0x34 /* Offset of first capability list entry */
+
+int pci_write(uint32_t fhandle, uint64_t offset, uint8_t pcias, uint64_t data,
+              uint8_t len);
+int pci_read(uint32_t fhandle, uint64_t offset, uint8_t pcias, void *buf,
+             uint8_t len);
+
+#endif
-- 
2.52.0



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

* [PATCH v4 11/15] pc-bios/s390-ccw: Introduce virtio-pci functions
  2026-03-04  2:59 [PATCH v4 00/15] s390x: Add support for virtio-blk-pci IPL device jrossi
                   ` (9 preceding siblings ...)
  2026-03-04  2:59 ` [PATCH v4 10/15] pc-bios/s390-ccw: Introduce PCI device jrossi
@ 2026-03-04  2:59 ` jrossi
  2026-03-04  9:18   ` Thomas Huth
  2026-03-05 18:37   ` Eric Farman
  2026-03-04  2:59 ` [PATCH v4 12/15] pc-bios/s390-ccw: Add support for virtio-blk-pci IPL jrossi
                   ` (3 subsequent siblings)
  14 siblings, 2 replies; 35+ messages in thread
From: jrossi @ 2026-03-04  2:59 UTC (permalink / raw)
  To: qemu-devel, qemu-s390x, thuth, mst
  Cc: jjherne, alifm, farman, mjrosato, jrossi, zycai

From: Jared Rossi <jrossi@linux.ibm.com>

Define common functionality for interacting with virtio-pci devices.

Signed-off-by: Jared Rossi <jrossi@linux.ibm.com>
---
 pc-bios/s390-ccw/Makefile     |   2 +-
 pc-bios/s390-ccw/virtio-pci.c | 167 ++++++++++++++++++++++++++++++++++
 pc-bios/s390-ccw/virtio-pci.h |  80 ++++++++++++++++
 pc-bios/s390-ccw/virtio.h     |   5 +
 4 files changed, 253 insertions(+), 1 deletion(-)
 create mode 100644 pc-bios/s390-ccw/virtio-pci.c
 create mode 100644 pc-bios/s390-ccw/virtio-pci.h

diff --git a/pc-bios/s390-ccw/Makefile b/pc-bios/s390-ccw/Makefile
index a62fc9d766..3e5dfb64d5 100644
--- a/pc-bios/s390-ccw/Makefile
+++ b/pc-bios/s390-ccw/Makefile
@@ -35,7 +35,7 @@ QEMU_DGFLAGS = -MMD -MP -MT $@ -MF $(@D)/$(*F).d
 
 OBJECTS = start.o main.o bootmap.o jump2ipl.o sclp.o menu.o netmain.o \
 	  virtio.o virtio-net.o virtio-scsi.o virtio-blkdev.o cio.o dasd-ipl.o \
-	  virtio-ccw.o clp.o pci.o
+	  virtio-ccw.o clp.o pci.o virtio-pci.o
 
 SLOF_DIR := $(SRC_PATH)/../../roms/SLOF
 
diff --git a/pc-bios/s390-ccw/virtio-pci.c b/pc-bios/s390-ccw/virtio-pci.c
new file mode 100644
index 0000000000..2cfb84bf66
--- /dev/null
+++ b/pc-bios/s390-ccw/virtio-pci.c
@@ -0,0 +1,167 @@
+/*
+ * Functionality for virtio-pci
+ *
+ * Copyright 2025 IBM Corp.
+ * Author(s): Jared Rossi <jrossi@linux.ibm.com>
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "clp.h"
+#include "pci.h"
+#include "helper.h"
+#include "virtio.h"
+#include "bswap.h"
+#include "virtio-pci.h"
+#include "s390-time.h"
+#include <stdio.h>
+
+/* Variable offsets used for reads/writes to modern memory regions */
+VirtioPciCap c_cap; /* Common capabilities  */
+VirtioPciCap d_cap; /* Device capabilities  */
+VirtioPciCap n_cap; /* Notify capabilities  */
+uint32_t notify_mult;
+uint16_t q_notify_offset;
+
+static int virtio_pci_set_status(uint8_t status)
+{
+    int rc = vpci_write_byte(c_cap.off + VPCI_C_OFFSET_STATUS, c_cap.bar, status);
+    if (rc) {
+        puts("Failed to write virtio-pci status");
+        return -EIO;
+    }
+
+    return 0;
+}
+
+static int virtio_pci_get_status(uint8_t *status)
+{
+    int rc = vpci_read_byte(c_cap.off + VPCI_C_OFFSET_STATUS, c_cap.bar, status);
+    if (rc) {
+        puts("Failed to read virtio-pci status");
+        return -EIO;
+    }
+
+    return 0;
+}
+
+/* virtio spec v1.3 section 4.1.2.1 */
+void virtio_pci_id2type(VDev *vdev, uint16_t device_id)
+{
+    switch (device_id) {
+    case 0x1402:
+    case 0x1001:
+        vdev->dev_type = VIRTIO_ID_BLOCK;
+        break;
+    default:
+        vdev->dev_type = 0;
+    }
+}
+
+int virtio_pci_reset(VDev *vdev)
+{
+    int rc;
+    uint8_t status = VIRTIO_CONFIG_S_RESET;
+
+    rc = virtio_pci_set_status(status);
+    rc |= virtio_pci_get_status(&status);
+
+    if (rc || status) {
+        puts("Failed to reset virtio-pci device");
+        return 1;
+    }
+
+    return 0;
+}
+
+long virtio_pci_notify(int vq_id)
+{
+    uint32_t offset = n_cap.off + notify_mult * q_notify_offset;
+    return vpci_bswap16_write(offset, n_cap.bar, (uint16_t) vq_id);
+}
+
+/*
+ * Wrappers to byte swap common data sizes then write
+ */
+int vpci_write_byte(uint64_t offset, uint8_t pcias, uint8_t data)
+{
+    return pci_write(virtio_get_device()->pci_fh, offset, pcias, (uint64_t) data, 1);
+}
+
+int vpci_bswap16_write(uint64_t offset, uint8_t pcias, uint16_t data)
+{
+    uint64_t le_data = bswap16(data);
+    return pci_write(virtio_get_device()->pci_fh, offset, pcias, le_data, 2);
+}
+
+int vpci_bswap32_write(uint64_t offset, uint8_t pcias, uint32_t data)
+{
+    uint64_t le_data = bswap32(data);
+    return pci_write(virtio_get_device()->pci_fh, offset, pcias, le_data, 4);
+}
+
+int vpci_bswap64_write(uint64_t offset, uint8_t pcias, uint64_t data)
+{
+    uint64_t le_data = bswap64(data);
+    return pci_write(virtio_get_device()->pci_fh, offset, pcias, le_data, 8);
+}
+
+/*
+ * Wrappers to read common data sizes then byte swap
+ */
+int vpci_read_byte(uint64_t offset, uint8_t pcias, uint8_t *buf)
+{
+    return pci_read(virtio_get_device()->pci_fh, offset, pcias, buf, 1);
+}
+
+int vpci_read_bswap16(uint64_t offset, uint8_t pcias, uint16_t *buf)
+{
+    int rc = pci_read(virtio_get_device()->pci_fh, offset, pcias, buf, 2);
+    *buf = bswap16(*buf);
+    return rc;
+}
+
+int vpci_read_bswap32(uint64_t offset, uint8_t pcias, uint32_t *buf)
+{
+    int rc = pci_read(virtio_get_device()->pci_fh, offset, pcias, buf, 4);
+    *buf = bswap32(*buf);
+    return rc;
+}
+
+int vpci_read_bswap64(uint64_t offset, uint8_t pcias, uint64_t *buf)
+{
+    int rc = pci_read(virtio_get_device()->pci_fh, offset, pcias, buf, 8);
+    *buf = bswap64(*buf);
+    return rc;
+}
+
+/*
+ * Read to an arbitrary length buffer without byte swapping
+ */
+int vpci_read_flex(uint64_t offset, uint8_t pcias, void *buf, int len)
+{
+    uint8_t readlen;
+    int rc;
+    int remaining = len;
+
+    /* Read bytes in powers of 2, up to a maximum of 8 bytes per read */
+    while (remaining) {
+        for (int i = 3; i >= 0; i--) {
+            readlen = 1 << i;
+            if (remaining >= readlen) {
+                break;
+            }
+        }
+
+        rc = pci_read(virtio_get_device()->pci_fh, offset, pcias, buf, readlen);
+        if (rc) {
+            return -1;
+        }
+
+        remaining -= readlen;
+        buf += readlen;
+        offset += readlen;
+    }
+
+    return 0;
+}
diff --git a/pc-bios/s390-ccw/virtio-pci.h b/pc-bios/s390-ccw/virtio-pci.h
new file mode 100644
index 0000000000..54c524f698
--- /dev/null
+++ b/pc-bios/s390-ccw/virtio-pci.h
@@ -0,0 +1,80 @@
+/*
+ * Definitions for virtio-pci
+ *
+ * Copyright 2025 IBM Corp.
+ * Author(s): Jared Rossi <jrossi@linux.ibm.com>
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#ifndef VIRTIO_PCI_H
+#define VIRTIO_PCI_H
+
+/* Common configuration */
+#define VPCI_CAP_COMMON_CFG          1
+/* Notifications */
+#define VPCI_CAP_NOTIFY_CFG          2
+/* ISR access */
+#define VPCI_CAP_ISR_CFG             3
+/* Device specific configuration */
+#define VPCI_CAP_DEVICE_CFG          4
+/* PCI configuration access */
+#define VPCI_CAP_PCI_CFG             5
+/* Additional shared memory capability */
+#define VPCI_CAP_SHARED_MEMORY_CFG   8
+/* PCI vendor data configuration */
+#define VPCI_CAP_VENDOR_CFG          9
+
+/* Offsets within capability header */
+#define VPCI_CAP_VNDR        0
+#define VPCI_CAP_NEXT        1
+#define VPCI_CAP_LEN         2
+#define VPCI_CAP_CFG_TYPE    3
+#define VPCI_CAP_BAR         4
+#define VPCI_CAP_OFFSET      8
+#define VPCI_CAP_LENGTH      12
+
+#define VPCI_N_CAP_MULT 16 /* Notify multiplier, VPCI_CAP_NOTIFY_CFG only */
+
+/* Common Area Offsets for virtio-pci queue */
+#define VPCI_C_OFFSET_DFSELECT      0
+#define VPCI_C_OFFSET_DF            4
+#define VPCI_C_OFFSET_GFSELECT      8
+#define VPCI_C_OFFSET_GF            12
+#define VPCI_C_COMMON_NUMQ          18
+#define VPCI_C_OFFSET_STATUS        20
+#define VPCI_C_OFFSET_Q_SELECT      22
+#define VPCI_C_OFFSET_Q_SIZE        24
+#define VPCI_C_OFFSET_Q_ENABLE      28
+#define VPCI_C_OFFSET_Q_NOFF        30
+#define VPCI_C_OFFSET_Q_DESCLO      32
+#define VPCI_C_OFFSET_Q_DESCHI      36
+#define VPCI_C_OFFSET_Q_AVAILLO     40
+#define VPCI_C_OFFSET_Q_AVAILHI     44
+#define VPCI_C_OFFSET_Q_USEDLO      48
+#define VPCI_C_OFFSET_Q_USEDHI      52
+
+#define VIRTIO_F_VERSION_1          1   /* Feature bit 32 */
+
+struct VirtioPciCap {
+    uint8_t bar;     /* Which PCIAS it's in */
+    uint32_t off;    /* Offset within bar */
+};
+typedef struct VirtioPciCap  VirtioPciCap;
+
+void virtio_pci_id2type(VDev *vdev, uint16_t device_id);
+int virtio_pci_reset(VDev *vdev);
+long virtio_pci_notify(int vq_id);
+
+int vpci_read_flex(uint64_t offset, uint8_t pcias, void *buf, int len);
+int vpci_read_bswap64(uint64_t offset, uint8_t pcias, uint64_t *buf);
+int vpci_read_bswap32(uint64_t offset, uint8_t pcias, uint32_t *buf);
+int vpci_read_bswap16(uint64_t offset, uint8_t pcias, uint16_t *buf);
+int vpci_read_byte(uint64_t offset, uint8_t pcias, uint8_t *buf);
+
+int vpci_bswap64_write(uint64_t offset, uint8_t pcias, uint64_t data);
+int vpci_bswap32_write(uint64_t offset, uint8_t pcias, uint32_t data);
+int vpci_bswap16_write(uint64_t offset, uint8_t pcias, uint16_t data);
+int vpci_write_byte(uint64_t offset, uint8_t pcias, uint8_t data);
+
+#endif
diff --git a/pc-bios/s390-ccw/virtio.h b/pc-bios/s390-ccw/virtio.h
index c3cb5a6ee3..0e7dbdb64c 100644
--- a/pc-bios/s390-ccw/virtio.h
+++ b/pc-bios/s390-ccw/virtio.h
@@ -18,6 +18,10 @@
 #define VIRTIO_CONFIG_S_DRIVER          2
 /* Driver has used its parts of the config, and is happy */
 #define VIRTIO_CONFIG_S_DRIVER_OK       4
+/* Feature negotiation complete */
+#define VIRTIO_CONFIG_S_FEATURES_OK     8
+/* Clear status byte */
+#define VIRTIO_CONFIG_S_RESET           0x00
 /* We've given up on this device. */
 #define VIRTIO_CONFIG_S_FAILED          0x80
 
@@ -255,6 +259,7 @@ struct VDev {
     uint8_t scsi_dev_heads;
     bool scsi_device_selected;
     ScsiDevice selected_scsi_device;
+    uint32_t pci_fh;
     uint32_t max_transfer;
     uint32_t guest_features[2];
 };
-- 
2.52.0



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

* [PATCH v4 12/15] pc-bios/s390-ccw: Add support for virtio-blk-pci IPL
  2026-03-04  2:59 [PATCH v4 00/15] s390x: Add support for virtio-blk-pci IPL device jrossi
                   ` (10 preceding siblings ...)
  2026-03-04  2:59 ` [PATCH v4 11/15] pc-bios/s390-ccw: Introduce virtio-pci functions jrossi
@ 2026-03-04  2:59 ` jrossi
  2026-03-04  9:53   ` Thomas Huth
  2026-03-05 22:25   ` Eric Farman
  2026-03-04  2:59 ` [PATCH v4 13/15] s390x: Build IPLB for virtio-pci devices jrossi
                   ` (2 subsequent siblings)
  14 siblings, 2 replies; 35+ messages in thread
From: jrossi @ 2026-03-04  2:59 UTC (permalink / raw)
  To: qemu-devel, qemu-s390x, thuth, mst
  Cc: jjherne, alifm, farman, mjrosato, jrossi, zycai

From: Jared Rossi <jrossi@linux.ibm.com>

Add little-endian virt-queue configuration and support for virtio-blk-pci IPL
devices.

Signed-off-by: Jared Rossi <jrossi@linux.ibm.com>
---
 pc-bios/s390-ccw/main.c          |  61 ++++++-
 pc-bios/s390-ccw/pci.h           |   3 +
 pc-bios/s390-ccw/virtio-blkdev.c |  18 +++
 pc-bios/s390-ccw/virtio-pci.c    | 265 +++++++++++++++++++++++++++++++
 pc-bios/s390-ccw/virtio-pci.h    |   2 +
 pc-bios/s390-ccw/virtio.c        |  54 ++++++-
 pc-bios/s390-ccw/virtio.h        |   1 +
 7 files changed, 399 insertions(+), 5 deletions(-)

diff --git a/pc-bios/s390-ccw/main.c b/pc-bios/s390-ccw/main.c
index c5ee575385..0e59ee3ea1 100644
--- a/pc-bios/s390-ccw/main.c
+++ b/pc-bios/s390-ccw/main.c
@@ -18,6 +18,8 @@
 #include "virtio.h"
 #include "virtio-scsi.h"
 #include "dasd-ipl.h"
+#include "clp.h"
+#include "virtio-pci.h"
 
 static SubChannelId blk_schid = { .one = 1 };
 static char loadparm_str[LOADPARM_LEN + 1];
@@ -151,6 +153,21 @@ static bool find_subch(int dev_no)
     return false;
 }
 
+static bool find_fid(uint32_t fid)
+{
+    ClpFhListEntry entry;
+    VDev *vdev = virtio_get_device();
+
+    if (find_pci_function(fid, &entry)) {
+        return false;
+    }
+
+    vdev->pci_fh = entry.fh;
+    virtio_pci_id2type(vdev, entry.device_id);
+
+    return vdev->dev_type != 0;
+}
+
 static void menu_setup(void)
 {
     if (memcmp(loadparm_str, LOADPARM_PROMPT, LOADPARM_LEN) == 0) {
@@ -239,6 +256,9 @@ static bool find_boot_device(void)
         blk_schid.ssid = iplb.scsi.ssid & 0x3;
         found = find_subch(iplb.scsi.devno);
         break;
+     case S390_IPL_TYPE_PCI:
+        found = find_fid(iplb.pci.fid);
+        break;
     default:
         puts("Unsupported IPLB");
     }
@@ -275,7 +295,7 @@ static int virtio_setup(void)
     return ret;
 }
 
-static void ipl_boot_device(void)
+static void ipl_ccw_device(void)
 {
     switch (cutype) {
     case CU_TYPE_DASD_3990:
@@ -289,7 +309,44 @@ static void ipl_boot_device(void)
         }
         break;
     default:
-        printf("Attempting to boot from unexpected device type 0x%X\n", cutype);
+        printf("Cannot boot CCW device with cu type 0x%X\n", cutype);
+    }
+}
+
+static void ipl_pci_device(void)
+{
+    VDev *vdev = virtio_get_device();
+    vdev->is_cdrom = false;
+    vdev->scsi_device_selected = false;
+
+    if (virtio_pci_setup_device()) {
+        return;
+    }
+
+    switch (vdev->dev_type) {
+    case VIRTIO_ID_BLOCK:
+        if (virtio_setup() == 0) {
+            zipl_load(); /* only return on error */
+            virtio_reset(virtio_get_device());
+        }
+        break;
+    default:
+        printf("Cannot boot PCI device type 0x%X\n", vdev->dev_type);
+    }
+}
+
+static void ipl_boot_device(void)
+{
+    switch (virtio_get_device()->ipl_type) {
+    case S390_IPL_TYPE_QEMU_SCSI:
+    case S390_IPL_TYPE_CCW:
+        ipl_ccw_device();
+        break;
+    case S390_IPL_TYPE_PCI:
+        ipl_pci_device();
+        break;
+    default:
+        puts("Unrecognized IPL type!");
     }
 }
 
diff --git a/pc-bios/s390-ccw/pci.h b/pc-bios/s390-ccw/pci.h
index 40a0bf9dcb..63825dd21c 100644
--- a/pc-bios/s390-ccw/pci.h
+++ b/pc-bios/s390-ccw/pci.h
@@ -29,8 +29,11 @@ union register_pair {
 #define PCIST_ENABLED          0x1
 
 #define PCI_CFGBAR             0xF  /* Base Address Register for config space */
+#define PCI_CMD_REG            0x4  /* Offset of command register */
 #define PCI_CAPABILITY_LIST    0x34 /* Offset of first capability list entry */
 
+#define PCI_BUS_MASTER_MASK    0x0020 /* LE bit 3 of 16 bit register */
+
 int pci_write(uint32_t fhandle, uint64_t offset, uint8_t pcias, uint64_t data,
               uint8_t len);
 int pci_read(uint32_t fhandle, uint64_t offset, uint8_t pcias, void *buf,
diff --git a/pc-bios/s390-ccw/virtio-blkdev.c b/pc-bios/s390-ccw/virtio-blkdev.c
index 9722b6970f..98b6cec3a0 100644
--- a/pc-bios/s390-ccw/virtio-blkdev.c
+++ b/pc-bios/s390-ccw/virtio-blkdev.c
@@ -13,10 +13,22 @@
 #include "virtio.h"
 #include "virtio-scsi.h"
 #include "virtio-ccw.h"
+#include "virtio-pci.h"
+#include "bswap.h"
 
 #define VIRTIO_BLK_F_GEOMETRY   (1 << 4)
 #define VIRTIO_BLK_F_BLK_SIZE   (1 << 6)
 
+/*
+ * Format header for little endian IPL
+ */
+static void fmt_blk_hdr_le(VirtioBlkOuthdr *hdr)
+{
+    hdr->type = bswap32(hdr->type);
+    hdr->ioprio = bswap32(hdr->ioprio);
+    hdr->sector = bswap64(hdr->sector);
+}
+
 static int virtio_blk_read_many(VDev *vdev, unsigned long sector, void *load_addr,
                                 int sec_num)
 {
@@ -29,6 +41,10 @@ static int virtio_blk_read_many(VDev *vdev, unsigned long sector, void *load_add
     out_hdr.ioprio = 99;
     out_hdr.sector = virtio_sector_adjust(sector);
 
+    if (!be_ipl()) {
+        fmt_blk_hdr_le(&out_hdr);
+    }
+
     vring_send_buf(vr, &out_hdr, sizeof(out_hdr), VRING_DESC_F_NEXT);
 
     /* This is where we want to receive data */
@@ -240,6 +256,8 @@ int virtio_blk_setup_device(VDev *vdev)
     case S390_IPL_TYPE_QEMU_SCSI:
     case S390_IPL_TYPE_CCW:
         return virtio_ccw_setup(vdev);
+    case S390_IPL_TYPE_PCI:
+        return virtio_pci_setup(vdev);
     default:
         return 1;
     }
diff --git a/pc-bios/s390-ccw/virtio-pci.c b/pc-bios/s390-ccw/virtio-pci.c
index 2cfb84bf66..beb97961d0 100644
--- a/pc-bios/s390-ccw/virtio-pci.c
+++ b/pc-bios/s390-ccw/virtio-pci.c
@@ -165,3 +165,268 @@ int vpci_read_flex(uint64_t offset, uint8_t pcias, void *buf, int len)
 
     return 0;
 }
+
+static int vpci_set_selected_vq(uint16_t queue_num)
+{
+    return vpci_bswap16_write(c_cap.off + VPCI_C_OFFSET_Q_SELECT, c_cap.bar, queue_num);
+}
+
+static int vpci_set_queue_enable(uint16_t enabled)
+{
+    return vpci_bswap16_write(c_cap.off + VPCI_C_OFFSET_Q_ENABLE, c_cap.bar, enabled);
+}
+
+static int set_pci_vq_addr(uint64_t config_off, void *addr)
+{
+    return vpci_bswap64_write(c_cap.off + config_off, c_cap.bar, (uint64_t) addr);
+}
+
+static int virtio_pci_get_blk_config(void)
+{
+    VirtioBlkConfig *cfg = &virtio_get_device()->config.blk;
+    int rc = vpci_read_flex(d_cap.off, d_cap.bar, cfg, sizeof(VirtioBlkConfig));
+
+    /* single byte fields are not touched */
+    cfg->capacity = bswap64(cfg->capacity);
+    cfg->size_max = bswap32(cfg->size_max);
+    cfg->seg_max = bswap32(cfg->seg_max);
+
+    cfg->geometry.cylinders = bswap16(cfg->geometry.cylinders);
+
+    cfg->blk_size = bswap32(cfg->blk_size);
+    cfg->min_io_size = bswap16(cfg->min_io_size);
+    cfg->opt_io_size = bswap32(cfg->opt_io_size);
+
+    return rc;
+}
+
+static int virtio_pci_negotiate(void)
+{
+    int i, rc;
+    VDev *vdev = virtio_get_device();
+    struct VirtioFeatureDesc {
+        uint32_t features;
+        uint8_t index;
+    } __attribute__((packed)) feats;
+
+    for (i = 0; i < ARRAY_SIZE(vdev->guest_features); i++) {
+        feats.features = 0;
+        feats.index = i;
+
+        rc = vpci_bswap32_write(c_cap.off + VPCI_C_OFFSET_DFSELECT, c_cap.bar,
+                                feats.index);
+        rc |= vpci_read_flex(c_cap.off + VPCI_C_OFFSET_DF, c_cap.bar, &feats, 4);
+
+        vdev->guest_features[i] &= bswap32(feats.features);
+        feats.features = vdev->guest_features[i];
+
+
+        rc |= vpci_bswap32_write(c_cap.off + VPCI_C_OFFSET_GFSELECT, c_cap.bar,
+                                 feats.index);
+        rc |= vpci_bswap32_write(c_cap.off + VPCI_C_OFFSET_GF, c_cap.bar,
+                                 feats.features);
+    }
+
+    return rc;
+}
+
+/*
+ * Find the position of the capability config within PCI configuration
+ * space for a given cfg type.  Return the position if found, otherwise 0.
+ */
+static uint8_t virtio_pci_find_cap_pos(uint8_t cfg_type)
+{
+    uint8_t next, cfg;
+    int rc;
+
+    rc = vpci_read_byte(PCI_CAPABILITY_LIST, PCI_CFGBAR, &next);
+    rc |= vpci_read_byte(next + 3, PCI_CFGBAR, &cfg);
+
+    while (!rc && (cfg != cfg_type) && next) {
+        rc = vpci_read_byte(next + 1, PCI_CFGBAR, &next);
+        rc |= vpci_read_byte(next + 3, PCI_CFGBAR, &cfg);
+    }
+
+    return rc ? 0 : next;
+}
+
+/*
+ * Read PCI configuration space to find the offset of the Common, Device, and
+ * Notification memory regions within the modern memory space.
+ * Returns 0 if success, 1 if a capability could not be located, or a
+ * negative RC if the configuration read failed.
+ */
+static int virtio_pci_read_pci_cap_config(void)
+{
+    uint8_t pos;
+    int rc;
+
+    /* Common capabilities */
+    pos = virtio_pci_find_cap_pos(VPCI_CAP_COMMON_CFG);
+    if (!pos) {
+        puts("Failed to locate PCI common configuration");
+        return 1;
+    }
+
+    rc = vpci_read_byte(pos + VPCI_CAP_BAR, PCI_CFGBAR, &c_cap.bar);
+    if (rc || vpci_read_bswap32(pos + VPCI_CAP_OFFSET, PCI_CFGBAR, &c_cap.off)) {
+        puts("Failed to read PCI common configuration");
+        return -EIO;
+    }
+
+    /* Device capabilities */
+    pos = virtio_pci_find_cap_pos(VPCI_CAP_DEVICE_CFG);
+    if (!pos) {
+        puts("Failed to locate PCI device configuration");
+        return 1;
+    }
+
+    rc = vpci_read_byte(pos + VPCI_CAP_BAR, PCI_CFGBAR, &d_cap.bar);
+    if (rc || vpci_read_bswap32(pos + VPCI_CAP_OFFSET, PCI_CFGBAR, &d_cap.off)) {
+        puts("Failed to read PCI device configuration");
+        return -EIO;
+    }
+
+    /* Notification capabilities */
+    pos = virtio_pci_find_cap_pos(VPCI_CAP_NOTIFY_CFG);
+    if (!pos) {
+        puts("Failed to locate PCI notification configuration");
+        return 1;
+    }
+
+    rc = vpci_read_byte(pos + VPCI_CAP_BAR, PCI_CFGBAR, &n_cap.bar);
+    if (rc || vpci_read_bswap32(pos + VPCI_CAP_OFFSET, PCI_CFGBAR, &n_cap.off)) {
+        puts("Failed to read PCI notification configuration");
+        return -EIO;
+    }
+
+    rc = vpci_read_bswap32(pos + VPCI_N_CAP_MULT, PCI_CFGBAR, &notify_mult);
+    if (rc || vpci_read_bswap16(c_cap.off + VPCI_C_OFFSET_Q_NOFF, c_cap.bar,
+                                &q_notify_offset)) {
+        puts("Failed to read notification queue configuration");
+        return -EIO;
+    }
+
+    return 0;
+}
+
+static int enable_pci_bus_master(void) {
+    uint16_t cmd_reg;
+
+    if (vpci_read_bswap16(PCI_CMD_REG, PCI_CFGBAR, &cmd_reg)) {
+        puts("Failed to read PCI command register");
+        return -EIO;
+    }
+
+    if (vpci_bswap16_write(PCI_CMD_REG, PCI_CFGBAR, cmd_reg | PCI_BUS_MASTER_MASK)) {
+        puts("Failed to enable PCI bus mastering");
+        return -EIO;
+    }
+
+    return 0;
+}
+
+int virtio_pci_setup(VDev *vdev)
+{
+    VRing *vr;
+    int rc;
+    uint8_t status;
+    uint16_t vq_size;
+    int i = 0;
+
+    vdev->guessed_disk_nature = VIRTIO_GDN_NONE;
+    vdev->cmd_vr_idx = 0;
+
+    if (virtio_pci_read_pci_cap_config()) {
+        puts("Invalid virtio PCI capabilities");
+        return -EIO;
+    }
+
+    if (enable_pci_bus_master()) {
+        return -EIO;
+    }
+
+    if (virtio_reset(vdev)) {
+        return -EIO;
+    }
+
+    status = VIRTIO_CONFIG_S_ACKNOWLEDGE;
+    if (virtio_pci_set_status(status)) {
+        puts("Virtio-pci device Failed to ACKNOWLEDGE");
+        return -EIO;
+    }
+
+    vdev->guest_features[1] = VIRTIO_F_VERSION_1;
+    if (virtio_pci_negotiate()) {
+        panic("Virtio feature negotation failed!");
+    }
+
+    switch (vdev->dev_type) {
+    case VIRTIO_ID_BLOCK:
+        vdev->nr_vqs = 1;
+        vdev->cmd_vr_idx = 0;
+        virtio_pci_get_blk_config();
+        break;
+    default:
+        puts("Unsupported virtio device");
+        return -ENODEV;
+    }
+
+    status |= VIRTIO_CONFIG_S_DRIVER;
+    rc = virtio_pci_set_status(status);
+    if (rc) {
+        puts("Set status failed");
+        return -EIO;
+    }
+
+    if (vpci_read_bswap16(VPCI_C_OFFSET_Q_SIZE, c_cap.bar, &vq_size)) {
+        puts("Failed to read virt-queue configuration");
+        return -EIO;
+    }
+
+    /* Configure virt-queues for pci */
+    for (i = 0; i < vdev->nr_vqs; i++) {
+        VqInfo info = {
+            .queue = (unsigned long long) virtio_get_ring_area(i),
+            .align = KVM_S390_VIRTIO_RING_ALIGN,
+            .index = i,
+            .num = vq_size,
+        };
+
+        vr = &vdev->vrings[i];
+        vring_init(vr, &info);
+
+        if (vpci_set_selected_vq(vr->id)) {
+            puts("Failed to set selected virt-queue");
+            return -EIO;
+        }
+
+        rc = set_pci_vq_addr(VPCI_C_OFFSET_Q_DESCLO, vr->desc);
+        rc |= set_pci_vq_addr(VPCI_C_OFFSET_Q_AVAILLO, vr->avail);
+        rc |= set_pci_vq_addr(VPCI_C_OFFSET_Q_USEDLO, vr->used);
+        if (rc) {
+            puts("Failed to configure virt-queue address");
+            return -EIO;
+        }
+
+        if (vpci_set_queue_enable(true)) {
+            puts("Failed to set virt-queue enabled");
+            return -EIO;
+        }
+    }
+
+    status |= VIRTIO_CONFIG_S_FEATURES_OK | VIRTIO_CONFIG_S_DRIVER_OK;
+    return virtio_pci_set_status(status);
+}
+
+int virtio_pci_setup_device(void)
+{
+    VDev *vdev = virtio_get_device();
+
+    if (enable_pci_function(&vdev->pci_fh)) {
+        puts("Failed to enable PCI function");
+        return -ENODEV;
+    }
+
+    return 0;
+}
diff --git a/pc-bios/s390-ccw/virtio-pci.h b/pc-bios/s390-ccw/virtio-pci.h
index 54c524f698..90d07cb9a7 100644
--- a/pc-bios/s390-ccw/virtio-pci.h
+++ b/pc-bios/s390-ccw/virtio-pci.h
@@ -65,6 +65,8 @@ typedef struct VirtioPciCap  VirtioPciCap;
 void virtio_pci_id2type(VDev *vdev, uint16_t device_id);
 int virtio_pci_reset(VDev *vdev);
 long virtio_pci_notify(int vq_id);
+int virtio_pci_setup(VDev *vdev);
+int virtio_pci_setup_device(void);
 
 int vpci_read_flex(uint64_t offset, uint8_t pcias, void *buf, int len);
 int vpci_read_bswap64(uint64_t offset, uint8_t pcias, uint64_t *buf);
diff --git a/pc-bios/s390-ccw/virtio.c b/pc-bios/s390-ccw/virtio.c
index 956b34ff33..390b55c7b9 100644
--- a/pc-bios/s390-ccw/virtio.c
+++ b/pc-bios/s390-ccw/virtio.c
@@ -17,6 +17,7 @@
 #include "virtio.h"
 #include "virtio-scsi.h"
 #include "virtio-ccw.h"
+#include "virtio-pci.h"
 #include "bswap.h"
 #include "helper.h"
 #include "s390-time.h"
@@ -96,7 +97,7 @@ void vring_init(VRing *vr, VqInfo *info)
     vr->avail->idx = 0;
 
     /* We're running with interrupts off anyways, so don't bother */
-    vr->used->flags = VRING_USED_F_NO_NOTIFY;
+    vr->used->flags = be_ipl() ? VRING_USED_F_NO_NOTIFY : bswap16(VRING_USED_F_NO_NOTIFY);
     vr->used->idx = 0;
     vr->used_idx = 0;
     vr->next_idx = 0;
@@ -112,6 +113,8 @@ bool vring_notify(VRing *vr)
     case S390_IPL_TYPE_CCW:
         vr->cookie = virtio_ccw_notify(vdev.schid, vr->id, vr->cookie);
         break;
+    case S390_IPL_TYPE_PCI:
+        vr->cookie = virtio_pci_notify(vr->id);
     default:
         return 1;
     }
@@ -119,11 +122,45 @@ bool vring_notify(VRing *vr)
     return vr->cookie >= 0;
 }
 
+/*
+ * Get endienness of the IPL type
+ * Return true for s390x native big-endian
+ */
+bool be_ipl(void)
+{
+    switch (virtio_get_device()->ipl_type) {
+    case S390_IPL_TYPE_QEMU_SCSI:
+    case S390_IPL_TYPE_CCW:
+        return true;
+    case S390_IPL_TYPE_PCI:
+        return false;
+    default:
+        return true;
+    }
+}
+
+/*
+ * Format the virtio ring descriptor endianness
+ * Return the available index increment in the appropriate endianness
+ */
+static void vr_bswap_descriptor(VRingDesc *desc)
+{
+    desc->addr = bswap64(desc->addr);
+    desc->len = bswap32(desc->len);
+    desc->flags = bswap16(desc->flags);
+    desc->next = bswap16(desc->next);
+}
+
 void vring_send_buf(VRing *vr, void *p, int len, int flags)
 {
+    if (!be_ipl()) {
+        vr->avail->idx = bswap16(vr->avail->idx);
+    }
+
     /* For follow-up chains we need to keep the first entry point */
     if (!(flags & VRING_HIDDEN_IS_CHAIN)) {
-        vr->avail->ring[vr->avail->idx % vr->num] = vr->next_idx;
+        vr->avail->ring[vr->avail->idx % vr->num] = be_ipl() ? vr->next_idx :
+                                                               bswap16(vr->next_idx);
     }
 
     vr->desc[vr->next_idx].addr = (unsigned long)p;
@@ -131,12 +168,21 @@ void vring_send_buf(VRing *vr, void *p, int len, int flags)
     vr->desc[vr->next_idx].flags = flags & ~VRING_HIDDEN_IS_CHAIN;
     vr->desc[vr->next_idx].next = vr->next_idx;
     vr->desc[vr->next_idx].next++;
+
+    if (!be_ipl()) {
+        vr_bswap_descriptor(&vr->desc[vr->next_idx]);
+    }
+
     vr->next_idx++;
 
     /* Chains only have a single ID */
     if (!(flags & VRING_DESC_F_NEXT)) {
         vr->avail->idx++;
     }
+
+    if (!be_ipl()) {
+        vr->avail->idx = bswap16(vr->avail->idx);
+    }
 }
 
 int vr_poll(VRing *vr)
@@ -147,7 +193,7 @@ int vr_poll(VRing *vr)
         return 0;
     }
 
-    vr->used_idx = vr->used->idx;
+    vr->used_idx = vr->used->idx; /* Endianness is preserved */
     vr->next_idx = 0;
     vr->desc[0].len = 0;
     vr->desc[0].flags = 0;
@@ -187,6 +233,8 @@ int virtio_reset(VDev *vdev)
     case S390_IPL_TYPE_QEMU_SCSI:
     case S390_IPL_TYPE_CCW:
         return virtio_ccw_reset(vdev);
+    case S390_IPL_TYPE_PCI:
+        return virtio_pci_reset(vdev);
     default:
         return -1;
     }
diff --git a/pc-bios/s390-ccw/virtio.h b/pc-bios/s390-ccw/virtio.h
index 0e7dbdb64c..18083b64cb 100644
--- a/pc-bios/s390-ccw/virtio.h
+++ b/pc-bios/s390-ccw/virtio.h
@@ -275,6 +275,7 @@ struct VirtioCmd {
 };
 typedef struct VirtioCmd VirtioCmd;
 
+bool be_ipl(void);
 void vring_init(VRing *vr, VqInfo *info);
 bool virtio_is_supported(VDev *vdev);
 bool vring_notify(VRing *vr);
-- 
2.52.0



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

* [PATCH v4 13/15] s390x: Build IPLB for virtio-pci devices
  2026-03-04  2:59 [PATCH v4 00/15] s390x: Add support for virtio-blk-pci IPL device jrossi
                   ` (11 preceding siblings ...)
  2026-03-04  2:59 ` [PATCH v4 12/15] pc-bios/s390-ccw: Add support for virtio-blk-pci IPL jrossi
@ 2026-03-04  2:59 ` jrossi
  2026-03-05 22:30   ` Eric Farman
  2026-03-04  2:59 ` [PATCH v4 14/15] hw: Add "loadparm" property to virtio block PCI devices booting on s390x jrossi
  2026-03-04  2:59 ` [PATCH v4 15/15] tests/qtest: Add s390x PCI boot test to cdrom-test.c jrossi
  14 siblings, 1 reply; 35+ messages in thread
From: jrossi @ 2026-03-04  2:59 UTC (permalink / raw)
  To: qemu-devel, qemu-s390x, thuth, mst
  Cc: jjherne, alifm, farman, mjrosato, jrossi, zycai

From: Jared Rossi <jrossi@linux.ibm.com>

Search for a corresponding S390PCIBusDevice and build an IPLB if a device has
been indexed for boot but does not identify as a CCW device,

PCI devices are not yet included in boot probing (they must have a boot index).

Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Jared Rossi <jrossi@linux.ibm.com>
---
 hw/s390x/ipl.c                  | 56 ++++++++++++++++++++++++++++++---
 hw/s390x/ipl.h                  |  3 ++
 hw/s390x/s390-pci-bus.c         |  3 +-
 include/hw/s390x/s390-pci-bus.h |  2 ++
 4 files changed, 57 insertions(+), 7 deletions(-)

diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
index d34adb5522..53dd3deafa 100644
--- a/hw/s390x/ipl.c
+++ b/hw/s390x/ipl.c
@@ -28,6 +28,8 @@
 #include "hw/s390x/ebcdic.h"
 #include "hw/scsi/scsi.h"
 #include "hw/virtio/virtio-net.h"
+#include "hw/virtio/virtio-pci.h"
+#include "hw/s390x/s390-pci-bus.h"
 #include "exec/cpu-common.h"
 #include "ipl.h"
 #include "qemu/error-report.h"
@@ -340,7 +342,8 @@ static void s390_ipl_set_boot_menu(S390IPLState *ipl)
     ipl->qipl.boot_menu_timeout = cpu_to_be32(splash_time);
 }
 
-#define CCW_DEVTYPE_NONE        0x00
+#define S390_DEVTYPE_NONE       0x00
+
 #define CCW_DEVTYPE_VIRTIO      0x01
 #define CCW_DEVTYPE_VIRTIO_NET  0x02
 #define CCW_DEVTYPE_SCSI        0x03
@@ -349,7 +352,7 @@ static void s390_ipl_set_boot_menu(S390IPLState *ipl)
 static CcwDevice *s390_get_ccw_device(DeviceState *dev_st, int *devtype)
 {
     CcwDevice *ccw_dev = NULL;
-    int tmp_dt = CCW_DEVTYPE_NONE;
+    int tmp_dt = S390_DEVTYPE_NONE;
 
     if (dev_st) {
         VirtIONet *virtio_net_dev = (VirtIONet *)
@@ -396,6 +399,31 @@ static CcwDevice *s390_get_ccw_device(DeviceState *dev_st, int *devtype)
     return ccw_dev;
 }
 
+#define PCI_DEVTYPE_VIRTIO       0x05
+
+static S390PCIBusDevice *s390_get_pci_device(DeviceState *dev_st, int *devtype)
+{
+    S390PCIBusDevice *pbdev = NULL;
+    int tmp_dt = S390_DEVTYPE_NONE;
+
+    if (dev_st) {
+        PCIDevice *pci_dev = (PCIDevice *)
+            object_dynamic_cast(OBJECT(qdev_get_parent_bus(dev_st)->parent),
+                                       TYPE_VIRTIO_PCI);
+        if (pci_dev) {
+            pbdev = s390_pci_find_dev_by_pci(s390_get_phb(), pci_dev);
+            if (pbdev) {
+                tmp_dt = PCI_DEVTYPE_VIRTIO;
+            }
+        }
+    }
+    if (devtype) {
+        *devtype = tmp_dt;
+    }
+
+    return pbdev;
+}
+
 static uint64_t s390_ipl_map_iplb_chain(IplParameterBlock *iplb_chain)
 {
     S390IPLState *ipl = get_ipl_device();
@@ -428,14 +456,12 @@ void s390_ipl_convert_loadparm(char *ascii_lp, uint8_t *ebcdic_lp)
 static bool s390_build_iplb(DeviceState *dev_st, IplParameterBlock *iplb)
 {
     CcwDevice *ccw_dev = NULL;
+    S390PCIBusDevice *pbdev = NULL;
     SCSIDevice *sd;
     int devtype;
     uint8_t *lp;
     g_autofree void *scsi_lp = NULL;
 
-    /*
-     * Currently allow IPL only from CCW devices.
-     */
     ccw_dev = s390_get_ccw_device(dev_st, &devtype);
     if (ccw_dev) {
         lp = ccw_dev->loadparm;
@@ -485,6 +511,26 @@ static bool s390_build_iplb(DeviceState *dev_st, IplParameterBlock *iplb)
         return true;
     }
 
+    pbdev = s390_get_pci_device(dev_st, &devtype);
+    if (pbdev) {
+        switch (devtype) {
+        case PCI_DEVTYPE_VIRTIO:
+            iplb->len = S390_IPLB_MIN_PCI_LEN;
+            iplb->pbt = S390_IPL_TYPE_PCI;
+            iplb->pci.fid = pbdev->fid;
+            break;
+        default:
+            return false;
+        }
+
+        /* Per-device loadparm not yet supported for non-ccw IPL */
+        lp = S390_CCW_MACHINE(qdev_get_machine())->loadparm;
+        s390_ipl_convert_loadparm((char *)lp, iplb->loadparm);
+        iplb->flags |= DIAG308_FLAGS_LP_VALID;
+
+        return true;
+    }
+
     return false;
 }
 
diff --git a/hw/s390x/ipl.h b/hw/s390x/ipl.h
index c542d30ce2..403cd08450 100644
--- a/hw/s390x/ipl.h
+++ b/hw/s390x/ipl.h
@@ -107,6 +107,7 @@ QEMU_BUILD_BUG_MSG(offsetof(S390IPLState, iplb) & 3, "alignment of iplb wrong");
 #define S390_IPLB_MIN_PV_LEN 148
 #define S390_IPLB_MIN_CCW_LEN 200
 #define S390_IPLB_MIN_FCP_LEN 384
+#define S390_IPLB_MIN_PCI_LEN 376
 #define S390_IPLB_MIN_QEMU_SCSI_LEN 200
 
 static inline bool iplb_valid_len(IplParameterBlock *iplb)
@@ -179,6 +180,8 @@ static inline bool iplb_valid(IplParameterBlock *iplb)
         return len >= S390_IPLB_MIN_FCP_LEN;
     case S390_IPL_TYPE_CCW:
         return len >= S390_IPLB_MIN_CCW_LEN;
+    case S390_IPL_TYPE_PCI:
+        return len >= S390_IPLB_MIN_PCI_LEN;
     case S390_IPL_TYPE_QEMU_SCSI:
     default:
         return false;
diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
index b438d63c44..ddf6efdc28 100644
--- a/hw/s390x/s390-pci-bus.c
+++ b/hw/s390x/s390-pci-bus.c
@@ -250,8 +250,7 @@ S390PCIBusDevice *s390_pci_find_dev_by_target(S390pciState *s,
     return NULL;
 }
 
-static S390PCIBusDevice *s390_pci_find_dev_by_pci(S390pciState *s,
-                                                  PCIDevice *pci_dev)
+S390PCIBusDevice *s390_pci_find_dev_by_pci(S390pciState *s, PCIDevice *pci_dev)
 {
     S390PCIBusDevice *pbdev;
 
diff --git a/include/hw/s390x/s390-pci-bus.h b/include/hw/s390x/s390-pci-bus.h
index f643e13057..9228523ce8 100644
--- a/include/hw/s390x/s390-pci-bus.h
+++ b/include/hw/s390x/s390-pci-bus.h
@@ -402,6 +402,8 @@ S390PCIBusDevice *s390_pci_find_dev_by_fh(S390pciState *s, uint32_t fh);
 S390PCIBusDevice *s390_pci_find_dev_by_fid(S390pciState *s, uint32_t fid);
 S390PCIBusDevice *s390_pci_find_dev_by_target(S390pciState *s,
                                               const char *target);
+S390PCIBusDevice *s390_pci_find_dev_by_pci(S390pciState *s,
+                                           PCIDevice *pci_dev);
 S390PCIBusDevice *s390_pci_find_next_avail_dev(S390pciState *s,
                                                S390PCIBusDevice *pbdev);
 void s390_pci_ism_reset(void);
-- 
2.52.0



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

* [PATCH v4 14/15] hw: Add "loadparm" property to virtio block PCI devices booting on s390x
  2026-03-04  2:59 [PATCH v4 00/15] s390x: Add support for virtio-blk-pci IPL device jrossi
                   ` (12 preceding siblings ...)
  2026-03-04  2:59 ` [PATCH v4 13/15] s390x: Build IPLB for virtio-pci devices jrossi
@ 2026-03-04  2:59 ` jrossi
  2026-03-04  9:59   ` Thomas Huth
  2026-03-04  2:59 ` [PATCH v4 15/15] tests/qtest: Add s390x PCI boot test to cdrom-test.c jrossi
  14 siblings, 1 reply; 35+ messages in thread
From: jrossi @ 2026-03-04  2:59 UTC (permalink / raw)
  To: qemu-devel, qemu-s390x, thuth, mst
  Cc: jjherne, alifm, farman, mjrosato, jrossi, zycai

From: Jared Rossi <jrossi@linux.ibm.com>

The loadparm is required on s390x to pass the information to the boot loader
such as which kernel should be started or whether the boot menu should be shown.

Because PCI devices do not naturally allocate space for this, the property is
added on an architecture specific basis for supported device types.

Signed-off-by: Jared Rossi <jrossi@linux.ibm.com>
---
 hw/pci/pci.c                | 38 +++++++++++++++++++++++++++++++++++++
 hw/s390x/ipl.c              | 11 +++++++++--
 hw/virtio/virtio-blk-pci.c  |  1 +
 include/hw/pci/pci.h        |  1 +
 include/hw/pci/pci_device.h |  3 +++
 5 files changed, 52 insertions(+), 2 deletions(-)

diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 90d6d71efd..04559f8312 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -36,6 +36,7 @@
 #include "migration/qemu-file-types.h"
 #include "migration/vmstate.h"
 #include "net/net.h"
+#include "system/arch_init.h"
 #include "system/numa.h"
 #include "system/runstate.h"
 #include "system/system.h"
@@ -2845,6 +2846,43 @@ int pci_qdev_find_device(const char *id, PCIDevice **pdev)
     return rc;
 }
 
+static char *pci_qdev_property_get_loadparm(Object *obj, Error **errp)
+{
+    return g_strdup(PCI_DEVICE(obj)->loadparm);
+}
+
+static void pci_qdev_property_set_loadparm(Object *obj, const char *value,
+                                       Error **errp)
+{
+    void *lp_str;
+
+    if (object_property_get_int(obj, "bootindex", NULL) < 0) {
+        error_setg(errp, "'loadparm' is only valid for boot devices");
+        return;
+    }
+
+    lp_str = g_malloc0(strlen(value) + 1);
+    if (!qdev_prop_sanitize_s390x_loadparm(lp_str, value, errp)) {
+        g_free(lp_str);
+        return;
+    }
+    PCI_DEVICE(obj)->loadparm = lp_str;
+}
+
+void pci_qdev_property_add_specifics(DeviceClass *dc)
+{
+    ObjectClass *oc = OBJECT_CLASS(dc);
+
+    /* The loadparm property is only supported on s390x */
+    if (qemu_arch_available(QEMU_ARCH_S390X)) {
+        object_class_property_add_str(oc, "loadparm",
+                                      pci_qdev_property_get_loadparm,
+                                      pci_qdev_property_set_loadparm);
+        object_class_property_set_description(oc, "loadparm",
+                                              "load parameter (s390x only)");
+    }
+}
+
 MemoryRegion *pci_address_space(PCIDevice *dev)
 {
     return pci_get_bus(dev)->address_space_mem;
diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
index 53dd3deafa..c231084c18 100644
--- a/hw/s390x/ipl.c
+++ b/hw/s390x/ipl.c
@@ -461,6 +461,7 @@ static bool s390_build_iplb(DeviceState *dev_st, IplParameterBlock *iplb)
     int devtype;
     uint8_t *lp;
     g_autofree void *scsi_lp = NULL;
+    g_autofree void *pci_lp = NULL;
 
     ccw_dev = s390_get_ccw_device(dev_st, &devtype);
     if (ccw_dev) {
@@ -513,6 +514,14 @@ static bool s390_build_iplb(DeviceState *dev_st, IplParameterBlock *iplb)
 
     pbdev = s390_get_pci_device(dev_st, &devtype);
     if (pbdev) {
+        pci_lp = object_property_get_str(OBJECT(pbdev->pdev), "loadparm", NULL);
+        if (pci_lp && strlen(pci_lp) > 0) {
+            lp = pci_lp;
+        } else {
+            /* Use machine loadparm as a place holder if PCI LP is unset */
+            lp = S390_CCW_MACHINE(qdev_get_machine())->loadparm;
+        }
+
         switch (devtype) {
         case PCI_DEVTYPE_VIRTIO:
             iplb->len = S390_IPLB_MIN_PCI_LEN;
@@ -523,8 +532,6 @@ static bool s390_build_iplb(DeviceState *dev_st, IplParameterBlock *iplb)
             return false;
         }
 
-        /* Per-device loadparm not yet supported for non-ccw IPL */
-        lp = S390_CCW_MACHINE(qdev_get_machine())->loadparm;
         s390_ipl_convert_loadparm((char *)lp, iplb->loadparm);
         iplb->flags |= DIAG308_FLAGS_LP_VALID;
 
diff --git a/hw/virtio/virtio-blk-pci.c b/hw/virtio/virtio-blk-pci.c
index 64a434c81b..3eecc23a65 100644
--- a/hw/virtio/virtio-blk-pci.c
+++ b/hw/virtio/virtio-blk-pci.c
@@ -71,6 +71,7 @@ static void virtio_blk_pci_class_init(ObjectClass *klass, const void *data)
 
     set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
     device_class_set_props(dc, virtio_blk_pci_properties);
+    pci_qdev_property_add_specifics(dc);
     k->realize = virtio_blk_pci_realize;
     pcidev_k->vendor_id = PCI_VENDOR_ID_REDHAT_QUMRANET;
     pcidev_k->device_id = PCI_DEVICE_ID_VIRTIO_BLOCK;
diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index d9835dfd0d..53c6e87b14 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -382,6 +382,7 @@ const char *pci_root_bus_path(PCIDevice *dev);
 bool pci_bus_bypass_iommu(PCIBus *bus);
 PCIDevice *pci_find_device(PCIBus *bus, int bus_num, uint8_t devfn);
 int pci_qdev_find_device(const char *id, PCIDevice **pdev);
+void pci_qdev_property_add_specifics(DeviceClass *dc);
 void pci_bus_get_w64_range(PCIBus *bus, Range *range);
 
 void pci_device_deassert_intx(PCIDevice *dev);
diff --git a/include/hw/pci/pci_device.h b/include/hw/pci/pci_device.h
index 88ccea5011..5cac6e1688 100644
--- a/include/hw/pci/pci_device.h
+++ b/include/hw/pci/pci_device.h
@@ -62,6 +62,9 @@ struct PCIDevice {
     bool partially_hotplugged;
     bool enabled;
 
+    /* only for s390x */
+    char *loadparm;
+
     /* PCI config space */
     uint8_t *config;
 
-- 
2.52.0



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

* [PATCH v4 15/15] tests/qtest: Add s390x PCI boot test to cdrom-test.c
  2026-03-04  2:59 [PATCH v4 00/15] s390x: Add support for virtio-blk-pci IPL device jrossi
                   ` (13 preceding siblings ...)
  2026-03-04  2:59 ` [PATCH v4 14/15] hw: Add "loadparm" property to virtio block PCI devices booting on s390x jrossi
@ 2026-03-04  2:59 ` jrossi
  14 siblings, 0 replies; 35+ messages in thread
From: jrossi @ 2026-03-04  2:59 UTC (permalink / raw)
  To: qemu-devel, qemu-s390x, thuth, mst
  Cc: jjherne, alifm, farman, mjrosato, jrossi, zycai

From: Jared Rossi <jrossi@linux.ibm.com>

Add a rudimentary test for s390x IPL to verify that a guest may boot using
virtio-blk-pci device.

Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Jared Rossi <jrossi@linux.ibm.com>
---
 tests/qtest/cdrom-test.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/tests/qtest/cdrom-test.c b/tests/qtest/cdrom-test.c
index 56e2d283a9..a65854d2bc 100644
--- a/tests/qtest/cdrom-test.c
+++ b/tests/qtest/cdrom-test.c
@@ -246,6 +246,13 @@ static void add_s390x_tests(void)
                             "-drive if=none,id=d2,media=cdrom,file=",
                             test_cdboot);
     }
+    if (qtest_has_device("virtio-blk-pci")) {
+        qtest_add_data_func("cdrom/boot/pci-bus-with-bootindex",
+                            "-device virtio-scsi -device virtio-serial "
+                            "-device virtio-blk-pci,drive=d1,bootindex=1 "
+                            "-drive if=none,id=d1,media=cdrom,file=",
+                            test_cdboot);
+    }
 }
 
 int main(int argc, char **argv)
-- 
2.52.0



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

* Re: [PATCH v4 03/15] pc-bios/s390-ccw: Always reset virtio device on failed boot attempt
  2026-03-04  2:59 ` [PATCH v4 03/15] pc-bios/s390-ccw: Always reset virtio device on failed boot attempt jrossi
@ 2026-03-04  8:23   ` Thomas Huth
  2026-03-04 13:39   ` Eric Farman
  1 sibling, 0 replies; 35+ messages in thread
From: Thomas Huth @ 2026-03-04  8:23 UTC (permalink / raw)
  To: jrossi, qemu-devel, qemu-s390x, mst
  Cc: jjherne, alifm, farman, mjrosato, zycai

On 04/03/2026 03.59, jrossi@linux.ibm.com wrote:
> From: Jared Rossi <jrossi@linux.ibm.com>
> 
> The virtio spec necessitates that live virtqueues must not be altered.  Reset
> the failed device so that the queues are not live before we attempt to boot any
> fallback devices.
> 
> Signed-off-by: Jared Rossi <jrossi@linux.ibm.com>
> ---
>   pc-bios/s390-ccw/main.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/pc-bios/s390-ccw/main.c b/pc-bios/s390-ccw/main.c
> index 76bf743900..8e2c99bee1 100644
> --- a/pc-bios/s390-ccw/main.c
> +++ b/pc-bios/s390-ccw/main.c
> @@ -277,7 +277,8 @@ static void ipl_boot_device(void)
>           break;
>       case CU_TYPE_VIRTIO:
>           if (virtio_setup() == 0) {
> -            zipl_load();
> +            zipl_load(); /* only return on error */
> +            virtio_reset(virtio_get_device());
>           }
>           break;
>       default:

Good idea! (Reminds me of 68c95ed1db070f7545e487e742715f01a545aab0 where we 
had to do something similar for virtio-net already).

Reviewed-by: Thomas Huth <thuth@redhat.com>



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

* Re: [PATCH v4 04/15] s390x: Remove duplicate definitions of IPL types
  2026-03-04  2:59 ` [PATCH v4 04/15] s390x: Remove duplicate definitions of IPL types jrossi
@ 2026-03-04  8:34   ` Thomas Huth
  0 siblings, 0 replies; 35+ messages in thread
From: Thomas Huth @ 2026-03-04  8:34 UTC (permalink / raw)
  To: jrossi, qemu-devel, qemu-s390x, mst
  Cc: jjherne, alifm, farman, mjrosato, zycai

On 04/03/2026 03.59, jrossi@linux.ibm.com wrote:
> From: Jared Rossi <jrossi@linux.ibm.com>
> 
> Remove the duplicate definitions from hw/s390x/ipl.h and pc-bios/s390-ccw/iplb.h
> and add a shared definition.  The new definition is an enum to enforce default
> handling in switches.
> 
> Because the IPL type is determined by the IPLB, and because an IPLB is not
> strictly necessary, the IPL type is set to a default value if not otherwise
> specified.  A default IPL type is required so future functionality may add
> IPL new bus and/or device types that dictate specific behavior during IPL.
> 
> Signed-off-by: Jared Rossi <jrossi@linux.ibm.com>
> ---
>   hw/s390x/ipl.h              |  5 -----
>   include/hw/s390x/ipl/qipl.h | 10 ++++++++++
>   pc-bios/s390-ccw/iplb.h     |  4 ----
>   pc-bios/s390-ccw/main.c     |  9 +++++++--
>   pc-bios/s390-ccw/virtio.h   |  1 +
>   5 files changed, 18 insertions(+), 11 deletions(-)
> 
> diff --git a/hw/s390x/ipl.h b/hw/s390x/ipl.h
> index 086e57681c..c542d30ce2 100644
> --- a/hw/s390x/ipl.h
> +++ b/hw/s390x/ipl.h
> @@ -103,11 +103,6 @@ QEMU_BUILD_BUG_MSG(offsetof(S390IPLState, iplb) & 3, "alignment of iplb wrong");
>   #define DIAG308_PV_STORE            9
>   #define DIAG308_PV_START            10
>   
> -#define S390_IPL_TYPE_FCP 0x00
> -#define S390_IPL_TYPE_CCW 0x02
> -#define S390_IPL_TYPE_PV 0x05
> -#define S390_IPL_TYPE_QEMU_SCSI 0xff
> -
>   #define S390_IPLB_HEADER_LEN 8
>   #define S390_IPLB_MIN_PV_LEN 148
>   #define S390_IPLB_MIN_CCW_LEN 200
> diff --git a/include/hw/s390x/ipl/qipl.h b/include/hw/s390x/ipl/qipl.h
> index 6824391111..6dc12dd859 100644
> --- a/include/hw/s390x/ipl/qipl.h
> +++ b/include/hw/s390x/ipl/qipl.h
> @@ -20,6 +20,16 @@
>   #define LOADPARM_LEN    8
>   #define NO_LOADPARM "\0\0\0\0\0\0\0\0"
>   
> +enum S390IplType {
> +    S390_IPL_TYPE_FCP = 0x00,
> +    S390_IPL_TYPE_CCW = 0x02,
> +    S390_IPL_TYPE_PV = 0x05,
> +    S390_IPL_TYPE_QEMU_SCSI = 0xff
> +};
> +typedef enum S390IplType S390IplType;
> +
> +#define QEMU_DEFAULT_IPL S390_IPL_TYPE_CCW
> +
>   /*
>    * The QEMU IPL Parameters will be stored at absolute address
>    * 204 (0xcc) which means it is 32-bit word aligned but not
> diff --git a/pc-bios/s390-ccw/iplb.h b/pc-bios/s390-ccw/iplb.h
> index 08f259ff31..926e8eed5d 100644
> --- a/pc-bios/s390-ccw/iplb.h
> +++ b/pc-bios/s390-ccw/iplb.h
> @@ -23,10 +23,6 @@ extern QemuIplParameters qipl;
>   extern IplParameterBlock iplb __attribute__((__aligned__(PAGE_SIZE)));
>   extern bool have_iplb;
>   
> -#define S390_IPL_TYPE_FCP 0x00
> -#define S390_IPL_TYPE_CCW 0x02
> -#define S390_IPL_TYPE_QEMU_SCSI 0xff
> -
>   static inline bool manage_iplb(IplParameterBlock *iplb, bool store)
>   {
>       register unsigned long addr asm("0") = (unsigned long) iplb;
> diff --git a/pc-bios/s390-ccw/main.c b/pc-bios/s390-ccw/main.c
> index 8e2c99bee1..ef5acc1985 100644
> --- a/pc-bios/s390-ccw/main.c
> +++ b/pc-bios/s390-ccw/main.c
> @@ -162,11 +162,12 @@ static void menu_setup(void)
>           return;
>       }
>   
> -    switch (iplb.pbt) {
> +    switch (virtio_get_device()->ipl_type) {

Cosmetics: Since you added a "vdev = virtio_get_device()" to boot_setup() 
and boot_setup() is the only caller of menu_setup(), you could also pass the 
vdev via parameter to menu_setup() now and get rid of the 
virtio_get_device() call here.

>       case S390_IPL_TYPE_CCW:
>       case S390_IPL_TYPE_QEMU_SCSI:
>           menu_set_parms(qipl.qipl_flags & BOOT_MENU_FLAG_MASK,
>                          qipl.boot_menu_timeout);

I'm a little bit surprised that this does not trigger any 
"-Wimplicit-fallthrough" warnings ... ah, we only enable "-Wall" in the 
Makefile, but not "-Wimplicit-fallthrough" is not included in that switch.

Anyway, I'd suggest to either add another "return;" statement here, or a "/* 
fallthrough */" comment.

> +    default:
>           return;
>       }
>   }

With the nits fixed:
Reviewed-by: Thomas Huth <thuth@redhat.com>



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

* Re: [PATCH v4 06/15] pc-bios/s390-ccw: Split virtio-ccw and generic virtio
  2026-03-04  2:59 ` [PATCH v4 06/15] pc-bios/s390-ccw: Split virtio-ccw and generic virtio jrossi
@ 2026-03-04  8:42   ` Thomas Huth
  2026-03-05 15:43   ` Eric Farman
  1 sibling, 0 replies; 35+ messages in thread
From: Thomas Huth @ 2026-03-04  8:42 UTC (permalink / raw)
  To: jrossi, qemu-devel, qemu-s390x, mst
  Cc: jjherne, alifm, farman, mjrosato, zycai

On 04/03/2026 03.59, jrossi@linux.ibm.com wrote:
> From: Jared Rossi <jrossi@linux.ibm.com>
> 
> Separate the CCW specific virtio routines and create generic wrappers for easier
> reuse of existing virtio functions with non-CCW devices.
> 
> Signed-off-by: Jared Rossi <jrossi@linux.ibm.com>
> ---
>   pc-bios/s390-ccw/Makefile        |   3 +-
>   pc-bios/s390-ccw/main.c          |   8 +-
>   pc-bios/s390-ccw/netmain.c       |   2 +-
>   pc-bios/s390-ccw/s390-ccw.h      |   3 -
>   pc-bios/s390-ccw/virtio-blkdev.c |  15 +-
>   pc-bios/s390-ccw/virtio-ccw.c    | 241 +++++++++++++++++++++++++++++++
>   pc-bios/s390-ccw/virtio-ccw.h    |  24 +++
>   pc-bios/s390-ccw/virtio-net.c    |   3 +-
>   pc-bios/s390-ccw/virtio-scsi.c   |   8 +-
>   pc-bios/s390-ccw/virtio-scsi.h   |   2 +-
>   pc-bios/s390-ccw/virtio.c        | 239 +++++-------------------------
>   pc-bios/s390-ccw/virtio.h        |   5 +-
>   12 files changed, 332 insertions(+), 221 deletions(-)
>   create mode 100644 pc-bios/s390-ccw/virtio-ccw.c
>   create mode 100644 pc-bios/s390-ccw/virtio-ccw.h

Reviewed-by: Thomas Huth <thuth@redhat.com>



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

* Re: [PATCH v4 10/15] pc-bios/s390-ccw: Introduce PCI device
  2026-03-04  2:59 ` [PATCH v4 10/15] pc-bios/s390-ccw: Introduce PCI device jrossi
@ 2026-03-04  9:10   ` Thomas Huth
  2026-03-04 14:35     ` Jared Rossi
  0 siblings, 1 reply; 35+ messages in thread
From: Thomas Huth @ 2026-03-04  9:10 UTC (permalink / raw)
  To: jrossi, qemu-devel, qemu-s390x, mst
  Cc: jjherne, alifm, farman, mjrosato, zycai

On 04/03/2026 03.59, jrossi@linux.ibm.com wrote:
> From: Jared Rossi <jrossi@linux.ibm.com>
> 
> Define selected s390x PCI instructions.
> 
> Signed-off-by: Jared Rossi <jrossi@linux.ibm.com>
> ---
...
> diff --git a/pc-bios/s390-ccw/pci.c b/pc-bios/s390-ccw/pci.c
> new file mode 100644
> index 0000000000..36e86fcc71
> --- /dev/null
> +++ b/pc-bios/s390-ccw/pci.c
> @@ -0,0 +1,118 @@
> +/*
> + * s390x PCI functionality
> + *
> + * Copyright 2025 IBM Corp.
> + * Author(s): Jared Rossi <jrossi@linux.ibm.com>
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#include "clp.h"
> +#include "pci.h"
> +#include "bswap.h"
> +#include <stdio.h>
> +#include <stdbool.h>
> +
> +/* PCI load */
> +static inline int pcilg(uint64_t *data, uint64_t req, uint64_t offset,
> +                        uint8_t *status)
> +{
> +    union register_pair req_off = {.even = req, .odd = offset};
> +    int cc = -1;
> +    uint64_t __data;
> +
> +    asm volatile (
> +        "     .insn   rre,0xb9d20000,%[data],%[req_off]\n"
> +        "     ipm     %[cc]\n"
> +        "     srl     %[cc],28\n"
> +        : [cc] "+d" (cc), [data] "=d" (__data),
> +          [req_off] "+d" (req_off.pair) :: "cc");
> +    *status = req_off.even >> 24 & 0xff;
> +    *data = __data;
> +    return cc;
> +}
> +
> +/* PCI store */
> +static inline int pcistg(uint64_t data, uint64_t req, uint64_t offset,
> +                         uint8_t *status)
> +{
> +    union register_pair req_off = {.even = req, .odd = offset};
> +    int cc = -1;
> +
> +    asm volatile (
> +        "     .insn   rre,0xb9d00000,%[data],%[req_off]\n"
> +        "     ipm     %[cc]\n"
> +        "     srl     %[cc],28\n"
> +        : [cc] "+d" (cc), [req_off] "+d" (req_off.pair)
> +        : [data] "d" (data)
> +        : "cc");
> +    *status = req_off.even >> 24 & 0xff;
> +    return cc;
> +}
> +
> +int pci_write(uint32_t fhandle, uint64_t offset, uint8_t pcias, uint64_t data,
> +              uint8_t len)
> +{
> +
> +    uint64_t req = ZPCI_CREATE_REQ(fhandle, pcias, len);
> +    uint8_t status;
> +    int rc;
> +
> +    /* len must be non-zero power of 2 with a maximum of 8 bytes per write */
> +    switch (len) {
> +    case 1:
> +    case 2:
> +    case 4:
> +    case 8:
> +        rc = pcistg(data, req, offset, &status);
> +        break;
> +    default:
> +        return -1;
> +    }
> +
> +    /* Error condition detected */
> +    if (rc != 0) {
> +        printf("PCI store failed with status condition %d, return code %d\n",
> +               status, rc);
> +        return -1;

Here you already return -1 if rc != 0 ...

> +    }
> +
> +    return rc ? -1 : 0;

... so the check here is redundant. Please simply always return 0 here instead.

> +}
> +
> +int pci_read(uint32_t fhandle, uint64_t offset, uint8_t pcias, void *buf,
> +             uint8_t len)
> +{
> +    uint64_t req, data;
> +    uint8_t status;
> +    int rc;
> +
> +    req = ZPCI_CREATE_REQ(fhandle, pcias, len);
> +    rc = pcilg(&data, req, offset, &status);
> +
> +    /* Error condition detected */
> +    if (rc != 0) {
> +        printf("PCI load failed with status condition %d, return code %d\n",
> +               status, rc);
> +        return -1;
> +    }
> +
> +    switch (len) {
> +    case 1:
> +        *(uint8_t *)buf = data;
> +        break;
> +    case 2:
> +        *(uint16_t *)buf = data;
> +        break;
> +    case 4:
> +        *(uint32_t *)buf = data;
> +        break;
> +    case 8:
> +        *(uint64_t *)buf = data;
> +        break;
> +    default:
> +        return -1;
> +    }
> +
> +    return rc ? -1 : 0;

dito, you checked for rc != 0 already earlier in this function.

  Thomas



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

* Re: [PATCH v4 11/15] pc-bios/s390-ccw: Introduce virtio-pci functions
  2026-03-04  2:59 ` [PATCH v4 11/15] pc-bios/s390-ccw: Introduce virtio-pci functions jrossi
@ 2026-03-04  9:18   ` Thomas Huth
  2026-03-04 14:38     ` Jared Rossi
  2026-03-05 18:37   ` Eric Farman
  1 sibling, 1 reply; 35+ messages in thread
From: Thomas Huth @ 2026-03-04  9:18 UTC (permalink / raw)
  To: jrossi, qemu-devel, qemu-s390x, mst
  Cc: jjherne, alifm, farman, mjrosato, zycai

On 04/03/2026 03.59, jrossi@linux.ibm.com wrote:
> From: Jared Rossi <jrossi@linux.ibm.com>
> 
> Define common functionality for interacting with virtio-pci devices.
> 
> Signed-off-by: Jared Rossi <jrossi@linux.ibm.com>
> ---
...
> diff --git a/pc-bios/s390-ccw/virtio-pci.c b/pc-bios/s390-ccw/virtio-pci.c
> new file mode 100644
> index 0000000000..2cfb84bf66
> --- /dev/null
> +++ b/pc-bios/s390-ccw/virtio-pci.c
> @@ -0,0 +1,167 @@
> +/*
> + * Functionality for virtio-pci
> + *
> + * Copyright 2025 IBM Corp.
> + * Author(s): Jared Rossi <jrossi@linux.ibm.com>
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#include "clp.h"
> +#include "pci.h"
> +#include "helper.h"
> +#include "virtio.h"
> +#include "bswap.h"
> +#include "virtio-pci.h"
> +#include "s390-time.h"
> +#include <stdio.h>
> +
> +/* Variable offsets used for reads/writes to modern memory regions */
> +VirtioPciCap c_cap; /* Common capabilities  */
> +VirtioPciCap d_cap; /* Device capabilities  */
> +VirtioPciCap n_cap; /* Notify capabilities  */
> +uint32_t notify_mult;
> +uint16_t q_notify_offset;
> +
> +static int virtio_pci_set_status(uint8_t status)
> +{
> +    int rc = vpci_write_byte(c_cap.off + VPCI_C_OFFSET_STATUS, c_cap.bar, status);
> +    if (rc) {
> +        puts("Failed to write virtio-pci status");
> +        return -EIO;
> +    }
> +
> +    return 0;
> +}
> +
> +static int virtio_pci_get_status(uint8_t *status)
> +{
> +    int rc = vpci_read_byte(c_cap.off + VPCI_C_OFFSET_STATUS, c_cap.bar, status);
> +    if (rc) {
> +        puts("Failed to read virtio-pci status");
> +        return -EIO;
> +    }
> +
> +    return 0;
> +}
> +
> +/* virtio spec v1.3 section 4.1.2.1 */
> +void virtio_pci_id2type(VDev *vdev, uint16_t device_id)
> +{
> +    switch (device_id) {
> +    case 0x1402:

I think it's 0x1042, not 0x1402 ?

> +    case 0x1001:
> +        vdev->dev_type = VIRTIO_ID_BLOCK;
> +        break;
> +    default:
> +        vdev->dev_type = 0;
> +    }
> +}

  Thomas



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

* Re: [PATCH v4 12/15] pc-bios/s390-ccw: Add support for virtio-blk-pci IPL
  2026-03-04  2:59 ` [PATCH v4 12/15] pc-bios/s390-ccw: Add support for virtio-blk-pci IPL jrossi
@ 2026-03-04  9:53   ` Thomas Huth
  2026-03-04 14:29     ` Jared Rossi
  2026-03-05 22:25   ` Eric Farman
  1 sibling, 1 reply; 35+ messages in thread
From: Thomas Huth @ 2026-03-04  9:53 UTC (permalink / raw)
  To: jrossi, qemu-devel, qemu-s390x, mst
  Cc: jjherne, alifm, farman, mjrosato, zycai

On 04/03/2026 03.59, jrossi@linux.ibm.com wrote:
> From: Jared Rossi <jrossi@linux.ibm.com>
> 
> Add little-endian virt-queue configuration and support for virtio-blk-pci IPL
> devices.
> 
> Signed-off-by: Jared Rossi <jrossi@linux.ibm.com>
> ---
...
> +static int virtio_pci_get_blk_config(void)
> +{
> +    VirtioBlkConfig *cfg = &virtio_get_device()->config.blk;
> +    int rc = vpci_read_flex(d_cap.off, d_cap.bar, cfg, sizeof(VirtioBlkConfig));
> +
> +    /* single byte fields are not touched */
> +    cfg->capacity = bswap64(cfg->capacity);
> +    cfg->size_max = bswap32(cfg->size_max);
> +    cfg->seg_max = bswap32(cfg->seg_max);
> +
> +    cfg->geometry.cylinders = bswap16(cfg->geometry.cylinders);
> +
> +    cfg->blk_size = bswap32(cfg->blk_size);
> +    cfg->min_io_size = bswap16(cfg->min_io_size);
> +    cfg->opt_io_size = bswap32(cfg->opt_io_size);

So it looks like you read a bunch of optional config fields here ...

> +    return rc;
> +}
...
> +int virtio_pci_setup(VDev *vdev)
> +{
> +    VRing *vr;
> +    int rc;
> +    uint8_t status;
> +    uint16_t vq_size;
> +    int i = 0;
> +
> +    vdev->guessed_disk_nature = VIRTIO_GDN_NONE;
> +    vdev->cmd_vr_idx = 0;
> +
> +    if (virtio_pci_read_pci_cap_config()) {
> +        puts("Invalid virtio PCI capabilities");
> +        return -EIO;
> +    }
> +
> +    if (enable_pci_bus_master()) {
> +        return -EIO;
> +    }
> +
> +    if (virtio_reset(vdev)) {
> +        return -EIO;
> +    }
> +
> +    status = VIRTIO_CONFIG_S_ACKNOWLEDGE;
> +    if (virtio_pci_set_status(status)) {
> +        puts("Virtio-pci device Failed to ACKNOWLEDGE");
> +        return -EIO;
> +    }

... so I think you should enable the corresponding feature bits in 
vdev->guest_features[0] here? QEMU might be very forgiving and provide you 
with the fields anyway, but let's better play safe. Something like:

     vdev->guest_features[0] = VIRTIO_BLK_F_SIZE_MAX |
                               VIRTIO_BLK_F_SEG_MAX |
                               VIRTIO_BLK_F_GEOMETRY |
                               VIRTIO_BLK_F_BLK_SIZE;

?

> +    vdev->guest_features[1] = VIRTIO_F_VERSION_1;
> +    if (virtio_pci_negotiate()) {
> +        panic("Virtio feature negotation failed!");
> +    }

Maybe double-check whether VIRTIO_F_VERSION_1 has really been negotiated? 
Otherwise, what happens if a user runs QEMU with "-device 
virtio-blk-pci,disable-modern=on" ?

  Thomas




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

* Re: [PATCH v4 14/15] hw: Add "loadparm" property to virtio block PCI devices booting on s390x
  2026-03-04  2:59 ` [PATCH v4 14/15] hw: Add "loadparm" property to virtio block PCI devices booting on s390x jrossi
@ 2026-03-04  9:59   ` Thomas Huth
  0 siblings, 0 replies; 35+ messages in thread
From: Thomas Huth @ 2026-03-04  9:59 UTC (permalink / raw)
  To: jrossi, qemu-devel, qemu-s390x, mst
  Cc: jjherne, alifm, farman, mjrosato, zycai

On 04/03/2026 03.59, jrossi@linux.ibm.com wrote:
> From: Jared Rossi <jrossi@linux.ibm.com>
> 
> The loadparm is required on s390x to pass the information to the boot loader
> such as which kernel should be started or whether the boot menu should be shown.
> 
> Because PCI devices do not naturally allocate space for this, the property is
> added on an architecture specific basis for supported device types.
> 
> Signed-off-by: Jared Rossi <jrossi@linux.ibm.com>
> ---
>   hw/pci/pci.c                | 38 +++++++++++++++++++++++++++++++++++++
>   hw/s390x/ipl.c              | 11 +++++++++--
>   hw/virtio/virtio-blk-pci.c  |  1 +
>   include/hw/pci/pci.h        |  1 +
>   include/hw/pci/pci_device.h |  3 +++
>   5 files changed, 52 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index 90d6d71efd..04559f8312 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -36,6 +36,7 @@
>   #include "migration/qemu-file-types.h"
>   #include "migration/vmstate.h"
>   #include "net/net.h"
> +#include "system/arch_init.h"
>   #include "system/numa.h"
>   #include "system/runstate.h"
>   #include "system/system.h"
> @@ -2845,6 +2846,43 @@ int pci_qdev_find_device(const char *id, PCIDevice **pdev)
>       return rc;
>   }
>   
> +static char *pci_qdev_property_get_loadparm(Object *obj, Error **errp)
> +{
> +    return g_strdup(PCI_DEVICE(obj)->loadparm);
> +}
> +
> +static void pci_qdev_property_set_loadparm(Object *obj, const char *value,
> +                                       Error **errp)
> +{
> +    void *lp_str;
> +
> +    if (object_property_get_int(obj, "bootindex", NULL) < 0) {
> +        error_setg(errp, "'loadparm' is only valid for boot devices");
> +        return;
> +    }
> +
> +    lp_str = g_malloc0(strlen(value) + 1);
> +    if (!qdev_prop_sanitize_s390x_loadparm(lp_str, value, errp)) {
> +        g_free(lp_str);
> +        return;
> +    }
> +    PCI_DEVICE(obj)->loadparm = lp_str;
> +}
> +
> +void pci_qdev_property_add_specifics(DeviceClass *dc)
> +{
> +    ObjectClass *oc = OBJECT_CLASS(dc);
> +
> +    /* The loadparm property is only supported on s390x */
> +    if (qemu_arch_available(QEMU_ARCH_S390X)) {

Philippe replaced qemu_arch_available(QEMU_ARCH_S390X) with target_s390x() 
in commit 1afc7da7dbc3f4c3b8cf310ff30a08f6f02587c8 ... I guess it's better 
to do the same here now, too?

With that changed:
Reviewed-by: Thomas Huth <thuth@redhat.com>




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

* Re: [PATCH v4 03/15] pc-bios/s390-ccw: Always reset virtio device on failed boot attempt
  2026-03-04  2:59 ` [PATCH v4 03/15] pc-bios/s390-ccw: Always reset virtio device on failed boot attempt jrossi
  2026-03-04  8:23   ` Thomas Huth
@ 2026-03-04 13:39   ` Eric Farman
  1 sibling, 0 replies; 35+ messages in thread
From: Eric Farman @ 2026-03-04 13:39 UTC (permalink / raw)
  To: jrossi, qemu-devel, qemu-s390x, thuth, mst
  Cc: jjherne, alifm, mjrosato, zycai

On Tue, 2026-03-03 at 21:59 -0500, jrossi@linux.ibm.com wrote:
> From: Jared Rossi <jrossi@linux.ibm.com>
> 
> The virtio spec necessitates that live virtqueues must not be altered.  Reset
> the failed device so that the queues are not live before we attempt to boot any
> fallback devices.
> 
> Signed-off-by: Jared Rossi <jrossi@linux.ibm.com>
> ---
>  pc-bios/s390-ccw/main.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

Reviewed-by: Eric Farman <farman@linux.ibm.com>

> 
> diff --git a/pc-bios/s390-ccw/main.c b/pc-bios/s390-ccw/main.c
> index 76bf743900..8e2c99bee1 100644
> --- a/pc-bios/s390-ccw/main.c
> +++ b/pc-bios/s390-ccw/main.c
> @@ -277,7 +277,8 @@ static void ipl_boot_device(void)
>          break;
>      case CU_TYPE_VIRTIO:
>          if (virtio_setup() == 0) {
> -            zipl_load();
> +            zipl_load(); /* only return on error */
> +            virtio_reset(virtio_get_device());
>          }
>          break;
>      default:


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

* Re: [PATCH v4 08/15] pc-bios/s390-ccw: Introduce CLP Architecture
  2026-03-04  2:59 ` [PATCH v4 08/15] pc-bios/s390-ccw: Introduce CLP Architecture jrossi
@ 2026-03-04 14:05   ` Eric Farman
  0 siblings, 0 replies; 35+ messages in thread
From: Eric Farman @ 2026-03-04 14:05 UTC (permalink / raw)
  To: jrossi, qemu-devel, qemu-s390x, thuth, mst
  Cc: jjherne, alifm, mjrosato, zycai

On Tue, 2026-03-03 at 21:59 -0500, jrossi@linux.ibm.com wrote:
> From: Jared Rossi <jrossi@linux.ibm.com>
> 
> Call Logical Processor (CLP) Architecture is used for managing PCI functions on
> s390x. Define and include the structures and routines needed to interact with
> PCI devices during IPL.
> 
> Acked-by: Thomas Huth <thuth@redhat.com>
> Signed-off-by: Jared Rossi <jrossi@linux.ibm.com>
> ---
>  pc-bios/s390-ccw/Makefile |  2 +-
>  pc-bios/s390-ccw/clp.c    | 99 +++++++++++++++++++++++++++++++++++++++
>  pc-bios/s390-ccw/clp.h    | 24 ++++++++++
>  3 files changed, 124 insertions(+), 1 deletion(-)
>  create mode 100644 pc-bios/s390-ccw/clp.c
>  create mode 100644 pc-bios/s390-ccw/clp.h

Reviewed-by: Eric Farman <farman@linux.ibm.com>


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

* Re: [PATCH v4 12/15] pc-bios/s390-ccw: Add support for virtio-blk-pci IPL
  2026-03-04  9:53   ` Thomas Huth
@ 2026-03-04 14:29     ` Jared Rossi
  2026-03-04 14:39       ` Thomas Huth
  0 siblings, 1 reply; 35+ messages in thread
From: Jared Rossi @ 2026-03-04 14:29 UTC (permalink / raw)
  To: Thomas Huth, qemu-devel, qemu-s390x, mst
  Cc: jjherne, alifm, farman, mjrosato, zycai



On 3/4/26 4:53 AM, Thomas Huth wrote:
> On 04/03/2026 03.59, jrossi@linux.ibm.com wrote:
>> From: Jared Rossi <jrossi@linux.ibm.com>
>>
>> Add little-endian virt-queue configuration and support for 
>> virtio-blk-pci IPL
>> devices.
>>
>> Signed-off-by: Jared Rossi <jrossi@linux.ibm.com>
>> ---
> ...
>> +static int virtio_pci_get_blk_config(void)
>> +{
>> +    VirtioBlkConfig *cfg = &virtio_get_device()->config.blk;
>> +    int rc = vpci_read_flex(d_cap.off, d_cap.bar, cfg, 
>> sizeof(VirtioBlkConfig));
>> +
>> +    /* single byte fields are not touched */
>> +    cfg->capacity = bswap64(cfg->capacity);
>> +    cfg->size_max = bswap32(cfg->size_max);
>> +    cfg->seg_max = bswap32(cfg->seg_max);
>> +
>> +    cfg->geometry.cylinders = bswap16(cfg->geometry.cylinders);
>> +
>> +    cfg->blk_size = bswap32(cfg->blk_size);
>> +    cfg->min_io_size = bswap16(cfg->min_io_size);
>> +    cfg->opt_io_size = bswap32(cfg->opt_io_size);
>
> So it looks like you read a bunch of optional config fields here ...
>
>> +    return rc;
>> +}
> ...
>> +int virtio_pci_setup(VDev *vdev)
>> +{
>> +    VRing *vr;
>> +    int rc;
>> +    uint8_t status;
>> +    uint16_t vq_size;
>> +    int i = 0;
>> +
>> +    vdev->guessed_disk_nature = VIRTIO_GDN_NONE;
>> +    vdev->cmd_vr_idx = 0;
>> +
>> +    if (virtio_pci_read_pci_cap_config()) {
>> +        puts("Invalid virtio PCI capabilities");
>> +        return -EIO;
>> +    }
>> +
>> +    if (enable_pci_bus_master()) {
>> +        return -EIO;
>> +    }
>> +
>> +    if (virtio_reset(vdev)) {
>> +        return -EIO;
>> +    }
>> +
>> +    status = VIRTIO_CONFIG_S_ACKNOWLEDGE;
>> +    if (virtio_pci_set_status(status)) {
>> +        puts("Virtio-pci device Failed to ACKNOWLEDGE");
>> +        return -EIO;
>> +    }
>
> ... so I think you should enable the corresponding feature bits in 
> vdev->guest_features[0] here? QEMU might be very forgiving and provide 
> you with the fields anyway, but let's better play safe. Something like:
>
>     vdev->guest_features[0] = VIRTIO_BLK_F_SIZE_MAX |
>                               VIRTIO_BLK_F_SEG_MAX |
>                               VIRTIO_BLK_F_GEOMETRY |
>                               VIRTIO_BLK_F_BLK_SIZE;
>
> ?

VIRTIO_BLK_F_GEOMETRY and VIRTIO_BLK_F_BLK_SIZE are already set during 
the virtio-blk setup.  I actually don't think the other two fields are 
used, I jut swapped any fields larger than 1 byte.  I suppose the 
feature bits should be enabled though... otherwise we could just just 
not touch the unused fields at all?

>
>> +    vdev->guest_features[1] = VIRTIO_F_VERSION_1;
>> +    if (virtio_pci_negotiate()) {
>> +        panic("Virtio feature negotation failed!");
>> +    }
>
> Maybe double-check whether VIRTIO_F_VERSION_1 has really been 
> negotiated? Otherwise, what happens if a user runs QEMU with "-device 
> virtio-blk-pci,disable-modern=on" ?

I haven't tried running it with "disable-modern=on" (I will test that 
next) but the config is big endian if I don't negotiate that feature 
bit, and little endian if I do, which I think is the expected behavior 
when VIRTIO_F_VERSION_1 is set.

Just for my understanding, do you see something that makes you suspect 
the negotiation isn't actually happening?  I will try running with 
"disable-modern=on" and let you know the results.

Thanks for the reviews,
Jared Rossi


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

* Re: [PATCH v4 10/15] pc-bios/s390-ccw: Introduce PCI device
  2026-03-04  9:10   ` Thomas Huth
@ 2026-03-04 14:35     ` Jared Rossi
  0 siblings, 0 replies; 35+ messages in thread
From: Jared Rossi @ 2026-03-04 14:35 UTC (permalink / raw)
  To: Thomas Huth, qemu-devel, qemu-s390x, mst
  Cc: jjherne, alifm, farman, mjrosato, zycai


> ...
>> +}
>> +
>> +int pci_read(uint32_t fhandle, uint64_t offset, uint8_t pcias, void 
>> *buf,
>> +             uint8_t len)
>> +{
>> +    uint64_t req, data;
>> +    uint8_t status;
>> +    int rc;
>> +
>> +    req = ZPCI_CREATE_REQ(fhandle, pcias, len);
>> +    rc = pcilg(&data, req, offset, &status);
>> +
>> +    /* Error condition detected */
>> +    if (rc != 0) {
>> +        printf("PCI load failed with status condition %d, return 
>> code %d\n",
>> +               status, rc);
>> +        return -1;
>> +    }
>> +
>> +    switch (len) {
>> +    case 1:
>> +        *(uint8_t *)buf = data;
>> +        break;
>> +    case 2:
>> +        *(uint16_t *)buf = data;
>> +        break;
>> +    case 4:
>> +        *(uint32_t *)buf = data;
>> +        break;
>> +    case 8:
>> +        *(uint64_t *)buf = data;
>> +        break;
>> +    default:
>> +        return -1;
>> +    }
>> +
>> +    return rc ? -1 : 0;
>
> dito, you checked for rc != 0 already earlier in this function.

My bad.  The previous version didn't check "rc != 0" but when I modified 
the status/error message I forgot that it made the later check 
redundant.  I'll fix it.

Thanks,
Jared Rossi



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

* Re: [PATCH v4 11/15] pc-bios/s390-ccw: Introduce virtio-pci functions
  2026-03-04  9:18   ` Thomas Huth
@ 2026-03-04 14:38     ` Jared Rossi
  0 siblings, 0 replies; 35+ messages in thread
From: Jared Rossi @ 2026-03-04 14:38 UTC (permalink / raw)
  To: Thomas Huth, qemu-devel, qemu-s390x, mst
  Cc: jjherne, alifm, farman, mjrosato, zycai



On 3/4/26 4:18 AM, Thomas Huth wrote:
> On 04/03/2026 03.59, jrossi@linux.ibm.com wrote:
>> From: Jared Rossi <jrossi@linux.ibm.com>
>>
>> Define common functionality for interacting with virtio-pci devices.
>>
>> Signed-off-by: Jared Rossi <jrossi@linux.ibm.com>
>> ---
> ...
>> diff --git a/pc-bios/s390-ccw/virtio-pci.c 
>> b/pc-bios/s390-ccw/virtio-pci.c
>> new file mode 100644
>> index 0000000000..2cfb84bf66
>> --- /dev/null
>> +++ b/pc-bios/s390-ccw/virtio-pci.c
>> @@ -0,0 +1,167 @@
>> +/*
>> + * Functionality for virtio-pci
>> + *
>> + * Copyright 2025 IBM Corp.
>> + * Author(s): Jared Rossi <jrossi@linux.ibm.com>
>> + *
>> + * SPDX-License-Identifier: GPL-2.0-or-later
>> + */
>> +
>> +#include "clp.h"
>> +#include "pci.h"
>> +#include "helper.h"
>> +#include "virtio.h"
>> +#include "bswap.h"
>> +#include "virtio-pci.h"
>> +#include "s390-time.h"
>> +#include <stdio.h>
>> +
>> +/* Variable offsets used for reads/writes to modern memory regions */
>> +VirtioPciCap c_cap; /* Common capabilities  */
>> +VirtioPciCap d_cap; /* Device capabilities  */
>> +VirtioPciCap n_cap; /* Notify capabilities  */
>> +uint32_t notify_mult;
>> +uint16_t q_notify_offset;
>> +
>> +static int virtio_pci_set_status(uint8_t status)
>> +{
>> +    int rc = vpci_write_byte(c_cap.off + VPCI_C_OFFSET_STATUS, 
>> c_cap.bar, status);
>> +    if (rc) {
>> +        puts("Failed to write virtio-pci status");
>> +        return -EIO;
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>> +static int virtio_pci_get_status(uint8_t *status)
>> +{
>> +    int rc = vpci_read_byte(c_cap.off + VPCI_C_OFFSET_STATUS, 
>> c_cap.bar, status);
>> +    if (rc) {
>> +        puts("Failed to read virtio-pci status");
>> +        return -EIO;
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>> +/* virtio spec v1.3 section 4.1.2.1 */
>> +void virtio_pci_id2type(VDev *vdev, uint16_t device_id)
>> +{
>> +    switch (device_id) {
>> +    case 0x1402:
>
> I think it's 0x1042, not 0x1402 ?

Good catch.  You are correct.  I'll fix it.

Thanks again,
Jared Rossi


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

* Re: [PATCH v4 12/15] pc-bios/s390-ccw: Add support for virtio-blk-pci IPL
  2026-03-04 14:29     ` Jared Rossi
@ 2026-03-04 14:39       ` Thomas Huth
  2026-03-04 20:17         ` Jared Rossi
  0 siblings, 1 reply; 35+ messages in thread
From: Thomas Huth @ 2026-03-04 14:39 UTC (permalink / raw)
  To: Jared Rossi, qemu-devel, qemu-s390x, mst
  Cc: jjherne, alifm, farman, mjrosato, zycai

On 04/03/2026 15.29, Jared Rossi wrote:
> 
> 
> On 3/4/26 4:53 AM, Thomas Huth wrote:
>> On 04/03/2026 03.59, jrossi@linux.ibm.com wrote:
>>> From: Jared Rossi <jrossi@linux.ibm.com>
>>>
>>> Add little-endian virt-queue configuration and support for virtio-blk-pci 
>>> IPL
>>> devices.
>>>
>>> Signed-off-by: Jared Rossi <jrossi@linux.ibm.com>
>>> ---
>> ...
>>> +static int virtio_pci_get_blk_config(void)
>>> +{
>>> +    VirtioBlkConfig *cfg = &virtio_get_device()->config.blk;
>>> +    int rc = vpci_read_flex(d_cap.off, d_cap.bar, cfg, 
>>> sizeof(VirtioBlkConfig));
>>> +
>>> +    /* single byte fields are not touched */
>>> +    cfg->capacity = bswap64(cfg->capacity);
>>> +    cfg->size_max = bswap32(cfg->size_max);
>>> +    cfg->seg_max = bswap32(cfg->seg_max);
>>> +
>>> +    cfg->geometry.cylinders = bswap16(cfg->geometry.cylinders);
>>> +
>>> +    cfg->blk_size = bswap32(cfg->blk_size);
>>> +    cfg->min_io_size = bswap16(cfg->min_io_size);
>>> +    cfg->opt_io_size = bswap32(cfg->opt_io_size);
>>
>> So it looks like you read a bunch of optional config fields here ...
>>
>>> +    return rc;
>>> +}
>> ...
>>> +int virtio_pci_setup(VDev *vdev)
>>> +{
>>> +    VRing *vr;
>>> +    int rc;
>>> +    uint8_t status;
>>> +    uint16_t vq_size;
>>> +    int i = 0;
>>> +
>>> +    vdev->guessed_disk_nature = VIRTIO_GDN_NONE;
>>> +    vdev->cmd_vr_idx = 0;
>>> +
>>> +    if (virtio_pci_read_pci_cap_config()) {
>>> +        puts("Invalid virtio PCI capabilities");
>>> +        return -EIO;
>>> +    }
>>> +
>>> +    if (enable_pci_bus_master()) {
>>> +        return -EIO;
>>> +    }
>>> +
>>> +    if (virtio_reset(vdev)) {
>>> +        return -EIO;
>>> +    }
>>> +
>>> +    status = VIRTIO_CONFIG_S_ACKNOWLEDGE;
>>> +    if (virtio_pci_set_status(status)) {
>>> +        puts("Virtio-pci device Failed to ACKNOWLEDGE");
>>> +        return -EIO;
>>> +    }
>>
>> ... so I think you should enable the corresponding feature bits in vdev- 
>> >guest_features[0] here? QEMU might be very forgiving and provide you with 
>> the fields anyway, but let's better play safe. Something like:
>>
>>     vdev->guest_features[0] = VIRTIO_BLK_F_SIZE_MAX |
>>                               VIRTIO_BLK_F_SEG_MAX |
>>                               VIRTIO_BLK_F_GEOMETRY |
>>                               VIRTIO_BLK_F_BLK_SIZE;
>>
>> ?
> 
> VIRTIO_BLK_F_GEOMETRY and VIRTIO_BLK_F_BLK_SIZE are already set during the 
> virtio-blk setup.  I actually don't think the other two fields are used, I 
> jut swapped any fields larger than 1 byte.  I suppose the feature bits 
> should be enabled though... otherwise we could just just not touch the 
> unused fields at all?

Ah, right, I missed the initialization in virtio_blk_setup_device(), so we 
should be fine here, indeed!

>>
>>> +    vdev->guest_features[1] = VIRTIO_F_VERSION_1;
>>> +    if (virtio_pci_negotiate()) {
>>> +        panic("Virtio feature negotation failed!");
>>> +    }
>>
>> Maybe double-check whether VIRTIO_F_VERSION_1 has really been negotiated? 
>> Otherwise, what happens if a user runs QEMU with "-device virtio-blk- 
>> pci,disable-modern=on" ?
> 
> I haven't tried running it with "disable-modern=on" (I will test that next) 
> but the config is big endian if I don't negotiate that feature bit, and 
> little endian if I do, which I think is the expected behavior when 
> VIRTIO_F_VERSION_1 is set.
> 
> Just for my understanding, do you see something that makes you suspect the 
> negotiation isn't actually happening?  I will try running with "disable- 
> modern=on" and let you know the results.

No, I think it's fine for the default case. I'm just wondering what happens 
when someone uses "disable-modern=on" ... I guess the code will currently 
behave in weird ways since the endianness is wrong ... thus I thought it 
might be better to check VIRTIO_F_VERSION_1 again and emit a proper error 
message in this case?

  Thomas



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

* Re: [PATCH v4 12/15] pc-bios/s390-ccw: Add support for virtio-blk-pci IPL
  2026-03-04 14:39       ` Thomas Huth
@ 2026-03-04 20:17         ` Jared Rossi
  2026-03-05  6:16           ` Thomas Huth
  0 siblings, 1 reply; 35+ messages in thread
From: Jared Rossi @ 2026-03-04 20:17 UTC (permalink / raw)
  To: Thomas Huth, qemu-devel, qemu-s390x, mst
  Cc: jjherne, alifm, farman, mjrosato, zycai



On 3/4/26 9:39 AM, Thomas Huth wrote:
> On 04/03/2026 15.29, Jared Rossi wrote:
>>
>>
>> On 3/4/26 4:53 AM, Thomas Huth wrote:
>>> On 04/03/2026 03.59, jrossi@linux.ibm.com wrote:
>>>> From: Jared Rossi <jrossi@linux.ibm.com>
>>>>
>>>> Add little-endian virt-queue configuration and support for 
>>>> virtio-blk-pci IPL
>>>> devices.
>>>>
>>>> Signed-off-by: Jared Rossi <jrossi@linux.ibm.com>
>>>> ---
>>> ...
>>>> +static int virtio_pci_get_blk_config(void)
>>>> +{
>>>> +    VirtioBlkConfig *cfg = &virtio_get_device()->config.blk;
>>>> +    int rc = vpci_read_flex(d_cap.off, d_cap.bar, cfg, 
>>>> sizeof(VirtioBlkConfig));
>>>> +
>>>> +    /* single byte fields are not touched */
>>>> +    cfg->capacity = bswap64(cfg->capacity);
>>>> +    cfg->size_max = bswap32(cfg->size_max);
>>>> +    cfg->seg_max = bswap32(cfg->seg_max);
>>>> +
>>>> +    cfg->geometry.cylinders = bswap16(cfg->geometry.cylinders);
>>>> +
>>>> +    cfg->blk_size = bswap32(cfg->blk_size);
>>>> +    cfg->min_io_size = bswap16(cfg->min_io_size);
>>>> +    cfg->opt_io_size = bswap32(cfg->opt_io_size);
>>>
>>> So it looks like you read a bunch of optional config fields here ...
>>>
>>>> +    return rc;
>>>> +}
>>> ...
>>>> +int virtio_pci_setup(VDev *vdev)
>>>> +{
>>>> +    VRing *vr;
>>>> +    int rc;
>>>> +    uint8_t status;
>>>> +    uint16_t vq_size;
>>>> +    int i = 0;
>>>> +
>>>> +    vdev->guessed_disk_nature = VIRTIO_GDN_NONE;
>>>> +    vdev->cmd_vr_idx = 0;
>>>> +
>>>> +    if (virtio_pci_read_pci_cap_config()) {
>>>> +        puts("Invalid virtio PCI capabilities");
>>>> +        return -EIO;
>>>> +    }
>>>> +
>>>> +    if (enable_pci_bus_master()) {
>>>> +        return -EIO;
>>>> +    }
>>>> +
>>>> +    if (virtio_reset(vdev)) {
>>>> +        return -EIO;
>>>> +    }
>>>> +
>>>> +    status = VIRTIO_CONFIG_S_ACKNOWLEDGE;
>>>> +    if (virtio_pci_set_status(status)) {
>>>> +        puts("Virtio-pci device Failed to ACKNOWLEDGE");
>>>> +        return -EIO;
>>>> +    }
>>>
>>> ... so I think you should enable the corresponding feature bits in 
>>> vdev- >guest_features[0] here? QEMU might be very forgiving and 
>>> provide you with the fields anyway, but let's better play safe. 
>>> Something like:
>>>
>>>     vdev->guest_features[0] = VIRTIO_BLK_F_SIZE_MAX |
>>>                               VIRTIO_BLK_F_SEG_MAX |
>>>                               VIRTIO_BLK_F_GEOMETRY |
>>>                               VIRTIO_BLK_F_BLK_SIZE;
>>>
>>> ?
>>
>> VIRTIO_BLK_F_GEOMETRY and VIRTIO_BLK_F_BLK_SIZE are already set 
>> during the virtio-blk setup.  I actually don't think the other two 
>> fields are used, I jut swapped any fields larger than 1 byte.  I 
>> suppose the feature bits should be enabled though... otherwise we 
>> could just just not touch the unused fields at all?
>
> Ah, right, I missed the initialization in virtio_blk_setup_device(), 
> so we should be fine here, indeed!
>
>>>
>>>> +    vdev->guest_features[1] = VIRTIO_F_VERSION_1;
>>>> +    if (virtio_pci_negotiate()) {
>>>> +        panic("Virtio feature negotation failed!");
>>>> +    }
>>>
>>> Maybe double-check whether VIRTIO_F_VERSION_1 has really been 
>>> negotiated? Otherwise, what happens if a user runs QEMU with 
>>> "-device virtio-blk- pci,disable-modern=on" ?
>>
>> I haven't tried running it with "disable-modern=on" (I will test that 
>> next) but the config is big endian if I don't negotiate that feature 
>> bit, and little endian if I do, which I think is the expected 
>> behavior when VIRTIO_F_VERSION_1 is set.
>>
>> Just for my understanding, do you see something that makes you 
>> suspect the negotiation isn't actually happening?  I will try running 
>> with "disable- modern=on" and let you know the results.
>
> No, I think it's fine for the default case. I'm just wondering what 
> happens when someone uses "disable-modern=on" ... I guess the code 
> will currently behave in weird ways since the endianness is wrong ... 
> thus I thought it might be better to check VIRTIO_F_VERSION_1 again 
> and emit a proper error message in this case?
>

I tried running with "disable-moden=on" and it failed very early in the 
virtio-pci setup when trying to read the PCI configuration space.

    Failed to locate PCI common configuration
    Invalid virtio PCI capabilities
    ERROR: No suitable device for IPL. Halting...


Actually that happens before we even try to negotiate 
VIRTIO_F_VERSION_1.  From the virtio spec, it looks like the legacy 
interface requires the common configuration to be in BAR0 (4.1.4.10), 
while we normally expect BAR15 to specify the layout. Typically we need 
to read the capabilities config in BAR15 to determine which BAR the 
common config is in, then that location is used when negotiating the 
features, etc.  My guess is BAR15 isn't populated when 
"disable-modern=on" so it bails out when there is no capabilities 
configuration.

But as far as I can tell it isn't an endianness issue since we are 
trying to read single byte fields at this point anyway.  What are your 
thoughts?

Regards,
Jared Rossi




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

* Re: [PATCH v4 12/15] pc-bios/s390-ccw: Add support for virtio-blk-pci IPL
  2026-03-04 20:17         ` Jared Rossi
@ 2026-03-05  6:16           ` Thomas Huth
  0 siblings, 0 replies; 35+ messages in thread
From: Thomas Huth @ 2026-03-05  6:16 UTC (permalink / raw)
  To: Jared Rossi, qemu-devel, qemu-s390x, mst
  Cc: jjherne, alifm, farman, mjrosato, zycai

On 04/03/2026 21.17, Jared Rossi wrote:
> 
> 
> On 3/4/26 9:39 AM, Thomas Huth wrote:
>> On 04/03/2026 15.29, Jared Rossi wrote:
>>>
>>>
>>> On 3/4/26 4:53 AM, Thomas Huth wrote:
>>>> On 04/03/2026 03.59, jrossi@linux.ibm.com wrote:
>>>>> From: Jared Rossi <jrossi@linux.ibm.com>
>>>>>
>>>>> Add little-endian virt-queue configuration and support for virtio-blk- 
>>>>> pci IPL
>>>>> devices.
>>>>>
>>>>> Signed-off-by: Jared Rossi <jrossi@linux.ibm.com>
>>>>> ---
>>>> ...
>>>>> +static int virtio_pci_get_blk_config(void)
>>>>> +{
>>>>> +    VirtioBlkConfig *cfg = &virtio_get_device()->config.blk;
>>>>> +    int rc = vpci_read_flex(d_cap.off, d_cap.bar, cfg, 
>>>>> sizeof(VirtioBlkConfig));
>>>>> +
>>>>> +    /* single byte fields are not touched */
>>>>> +    cfg->capacity = bswap64(cfg->capacity);
>>>>> +    cfg->size_max = bswap32(cfg->size_max);
>>>>> +    cfg->seg_max = bswap32(cfg->seg_max);
>>>>> +
>>>>> +    cfg->geometry.cylinders = bswap16(cfg->geometry.cylinders);
>>>>> +
>>>>> +    cfg->blk_size = bswap32(cfg->blk_size);
>>>>> +    cfg->min_io_size = bswap16(cfg->min_io_size);
>>>>> +    cfg->opt_io_size = bswap32(cfg->opt_io_size);
>>>>
>>>> So it looks like you read a bunch of optional config fields here ...
>>>>
>>>>> +    return rc;
>>>>> +}
>>>> ...
>>>>> +int virtio_pci_setup(VDev *vdev)
>>>>> +{
>>>>> +    VRing *vr;
>>>>> +    int rc;
>>>>> +    uint8_t status;
>>>>> +    uint16_t vq_size;
>>>>> +    int i = 0;
>>>>> +
>>>>> +    vdev->guessed_disk_nature = VIRTIO_GDN_NONE;
>>>>> +    vdev->cmd_vr_idx = 0;
>>>>> +
>>>>> +    if (virtio_pci_read_pci_cap_config()) {
>>>>> +        puts("Invalid virtio PCI capabilities");
>>>>> +        return -EIO;
>>>>> +    }
>>>>> +
>>>>> +    if (enable_pci_bus_master()) {
>>>>> +        return -EIO;
>>>>> +    }
>>>>> +
>>>>> +    if (virtio_reset(vdev)) {
>>>>> +        return -EIO;
>>>>> +    }
>>>>> +
>>>>> +    status = VIRTIO_CONFIG_S_ACKNOWLEDGE;
>>>>> +    if (virtio_pci_set_status(status)) {
>>>>> +        puts("Virtio-pci device Failed to ACKNOWLEDGE");
>>>>> +        return -EIO;
>>>>> +    }
>>>>
>>>> ... so I think you should enable the corresponding feature bits in vdev- 
>>>> >guest_features[0] here? QEMU might be very forgiving and provide you 
>>>> with the fields anyway, but let's better play safe. Something like:
>>>>
>>>>     vdev->guest_features[0] = VIRTIO_BLK_F_SIZE_MAX |
>>>>                               VIRTIO_BLK_F_SEG_MAX |
>>>>                               VIRTIO_BLK_F_GEOMETRY |
>>>>                               VIRTIO_BLK_F_BLK_SIZE;
>>>>
>>>> ?
>>>
>>> VIRTIO_BLK_F_GEOMETRY and VIRTIO_BLK_F_BLK_SIZE are already set during 
>>> the virtio-blk setup.  I actually don't think the other two fields are 
>>> used, I jut swapped any fields larger than 1 byte.  I suppose the feature 
>>> bits should be enabled though... otherwise we could just just not touch 
>>> the unused fields at all?
>>
>> Ah, right, I missed the initialization in virtio_blk_setup_device(), so we 
>> should be fine here, indeed!
>>
>>>>
>>>>> +    vdev->guest_features[1] = VIRTIO_F_VERSION_1;
>>>>> +    if (virtio_pci_negotiate()) {
>>>>> +        panic("Virtio feature negotation failed!");
>>>>> +    }
>>>>
>>>> Maybe double-check whether VIRTIO_F_VERSION_1 has really been 
>>>> negotiated? Otherwise, what happens if a user runs QEMU with "-device 
>>>> virtio-blk- pci,disable-modern=on" ?
>>>
>>> I haven't tried running it with "disable-modern=on" (I will test that 
>>> next) but the config is big endian if I don't negotiate that feature bit, 
>>> and little endian if I do, which I think is the expected behavior when 
>>> VIRTIO_F_VERSION_1 is set.
>>>
>>> Just for my understanding, do you see something that makes you suspect 
>>> the negotiation isn't actually happening?  I will try running with 
>>> "disable- modern=on" and let you know the results.
>>
>> No, I think it's fine for the default case. I'm just wondering what 
>> happens when someone uses "disable-modern=on" ... I guess the code will 
>> currently behave in weird ways since the endianness is wrong ... thus I 
>> thought it might be better to check VIRTIO_F_VERSION_1 again and emit a 
>> proper error message in this case?
>>
> 
> I tried running with "disable-moden=on" and it failed very early in the 
> virtio-pci setup when trying to read the PCI configuration space.
> 
>     Failed to locate PCI common configuration
>     Invalid virtio PCI capabilities
>     ERROR: No suitable device for IPL. Halting...
> 
> 
> Actually that happens before we even try to negotiate VIRTIO_F_VERSION_1.  
>  From the virtio spec, it looks like the legacy interface requires the 
> common configuration to be in BAR0 (4.1.4.10), while we normally expect 
> BAR15 to specify the layout. Typically we need to read the capabilities 
> config in BAR15 to determine which BAR the common config is in, then that 
> location is used when negotiating the features, etc.  My guess is BAR15 
> isn't populated when "disable-modern=on" so it bails out when there is no 
> capabilities configuration.
> 
> But as far as I can tell it isn't an endianness issue since we are trying to 
> read single byte fields at this point anyway.  What are your thoughts?

Ok, having the error message "Failed to locate ..." sounds good to me, so 
I'm fine if you keep this patch as it is. I was just worried that we end up 
with the bios crashing in weird ways due to the endianness issues, and that 
you could not tell by the output on the serial console what was going on. 
But since we have a clear error message instead of a crash, I think we're fine.

Thanks for checking!

  Thomas



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

* Re: [PATCH v4 06/15] pc-bios/s390-ccw: Split virtio-ccw and generic virtio
  2026-03-04  2:59 ` [PATCH v4 06/15] pc-bios/s390-ccw: Split virtio-ccw and generic virtio jrossi
  2026-03-04  8:42   ` Thomas Huth
@ 2026-03-05 15:43   ` Eric Farman
  1 sibling, 0 replies; 35+ messages in thread
From: Eric Farman @ 2026-03-05 15:43 UTC (permalink / raw)
  To: jrossi, qemu-devel, qemu-s390x, thuth, mst
  Cc: jjherne, alifm, mjrosato, zycai

On Tue, 2026-03-03 at 21:59 -0500, jrossi@linux.ibm.com wrote:
> From: Jared Rossi <jrossi@linux.ibm.com>
> 
> Separate the CCW specific virtio routines and create generic wrappers for easier
> reuse of existing virtio functions with non-CCW devices.
> 
> Signed-off-by: Jared Rossi <jrossi@linux.ibm.com>
> ---
>  pc-bios/s390-ccw/Makefile        |   3 +-
>  pc-bios/s390-ccw/main.c          |   8 +-
>  pc-bios/s390-ccw/netmain.c       |   2 +-
>  pc-bios/s390-ccw/s390-ccw.h      |   3 -
>  pc-bios/s390-ccw/virtio-blkdev.c |  15 +-
>  pc-bios/s390-ccw/virtio-ccw.c    | 241 +++++++++++++++++++++++++++++++
>  pc-bios/s390-ccw/virtio-ccw.h    |  24 +++
>  pc-bios/s390-ccw/virtio-net.c    |   3 +-
>  pc-bios/s390-ccw/virtio-scsi.c   |   8 +-
>  pc-bios/s390-ccw/virtio-scsi.h   |   2 +-
>  pc-bios/s390-ccw/virtio.c        | 239 +++++-------------------------
>  pc-bios/s390-ccw/virtio.h        |   5 +-
>  12 files changed, 332 insertions(+), 221 deletions(-)
>  create mode 100644 pc-bios/s390-ccw/virtio-ccw.c
>  create mode 100644 pc-bios/s390-ccw/virtio-ccw.h

This is some good untangling!

Reviewed-by: Eric Farman <farman@linux.ibm.com>


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

* Re: [PATCH v4 11/15] pc-bios/s390-ccw: Introduce virtio-pci functions
  2026-03-04  2:59 ` [PATCH v4 11/15] pc-bios/s390-ccw: Introduce virtio-pci functions jrossi
  2026-03-04  9:18   ` Thomas Huth
@ 2026-03-05 18:37   ` Eric Farman
  1 sibling, 0 replies; 35+ messages in thread
From: Eric Farman @ 2026-03-05 18:37 UTC (permalink / raw)
  To: jrossi, qemu-devel, qemu-s390x, thuth, mst
  Cc: jjherne, alifm, mjrosato, zycai

On Tue, 2026-03-03 at 21:59 -0500, jrossi@linux.ibm.com wrote:
> From: Jared Rossi <jrossi@linux.ibm.com>
> 
> Define common functionality for interacting with virtio-pci devices.
> 
> Signed-off-by: Jared Rossi <jrossi@linux.ibm.com>
> ---
>  pc-bios/s390-ccw/Makefile     |   2 +-
>  pc-bios/s390-ccw/virtio-pci.c | 167 ++++++++++++++++++++++++++++++++++
>  pc-bios/s390-ccw/virtio-pci.h |  80 ++++++++++++++++
>  pc-bios/s390-ccw/virtio.h     |   5 +
>  4 files changed, 253 insertions(+), 1 deletion(-)
>  create mode 100644 pc-bios/s390-ccw/virtio-pci.c
>  create mode 100644 pc-bios/s390-ccw/virtio-pci.h
> 

...snip...
(Only comment I had in here was the s/0x1402/0x1042/ that Thomas pointed out.)


> diff --git a/pc-bios/s390-ccw/virtio-pci.h b/pc-bios/s390-ccw/virtio-pci.h
> new file mode 100644
> index 0000000000..54c524f698
> --- /dev/null
> +++ b/pc-bios/s390-ccw/virtio-pci.h
> @@ -0,0 +1,80 @@
> +/*
> + * Definitions for virtio-pci
> + *
> + * Copyright 2025 IBM Corp.
> + * Author(s): Jared Rossi <jrossi@linux.ibm.com>
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#ifndef VIRTIO_PCI_H
> +#define VIRTIO_PCI_H
> +
> +/* Common configuration */
> +#define VPCI_CAP_COMMON_CFG          1
> +/* Notifications */
> +#define VPCI_CAP_NOTIFY_CFG          2
> +/* ISR access */
> +#define VPCI_CAP_ISR_CFG             3
> +/* Device specific configuration */
> +#define VPCI_CAP_DEVICE_CFG          4
> +/* PCI configuration access */
> +#define VPCI_CAP_PCI_CFG             5
> +/* Additional shared memory capability */
> +#define VPCI_CAP_SHARED_MEMORY_CFG   8
> +/* PCI vendor data configuration */
> +#define VPCI_CAP_VENDOR_CFG          9
> +
> +/* Offsets within capability header */
> +#define VPCI_CAP_VNDR        0
> +#define VPCI_CAP_NEXT        1
> +#define VPCI_CAP_LEN         2
> +#define VPCI_CAP_CFG_TYPE    3
> +#define VPCI_CAP_BAR         4
> +#define VPCI_CAP_OFFSET      8
> +#define VPCI_CAP_LENGTH      12
> +
> +#define VPCI_N_CAP_MULT 16 /* Notify multiplier, VPCI_CAP_NOTIFY_CFG only */
> +
> +/* Common Area Offsets for virtio-pci queue */
> +#define VPCI_C_OFFSET_DFSELECT      0
> +#define VPCI_C_OFFSET_DF            4
> +#define VPCI_C_OFFSET_GFSELECT      8
> +#define VPCI_C_OFFSET_GF            12
> +#define VPCI_C_COMMON_NUMQ          18
> +#define VPCI_C_OFFSET_STATUS        20
> +#define VPCI_C_OFFSET_Q_SELECT      22
> +#define VPCI_C_OFFSET_Q_SIZE        24
> +#define VPCI_C_OFFSET_Q_ENABLE      28
> +#define VPCI_C_OFFSET_Q_NOFF        30
> +#define VPCI_C_OFFSET_Q_DESCLO      32
> +#define VPCI_C_OFFSET_Q_DESCHI      36
> +#define VPCI_C_OFFSET_Q_AVAILLO     40
> +#define VPCI_C_OFFSET_Q_AVAILHI     44
> +#define VPCI_C_OFFSET_Q_USEDLO      48
> +#define VPCI_C_OFFSET_Q_USEDHI      52
> +
> +#define VIRTIO_F_VERSION_1          1   /* Feature bit 32 */
> +
> +struct VirtioPciCap {
> +    uint8_t bar;     /* Which PCIAS it's in */
> +    uint32_t off;    /* Offset within bar */
> +};
> +typedef struct VirtioPciCap  VirtioPciCap;
> +
> +void virtio_pci_id2type(VDev *vdev, uint16_t device_id);
> +int virtio_pci_reset(VDev *vdev);
> +long virtio_pci_notify(int vq_id);
> +
> +int vpci_read_flex(uint64_t offset, uint8_t pcias, void *buf, int len);
> +int vpci_read_bswap64(uint64_t offset, uint8_t pcias, uint64_t *buf);
> +int vpci_read_bswap32(uint64_t offset, uint8_t pcias, uint32_t *buf);
> +int vpci_read_bswap16(uint64_t offset, uint8_t pcias, uint16_t *buf);
> +int vpci_read_byte(uint64_t offset, uint8_t pcias, uint8_t *buf);
> +
> +int vpci_bswap64_write(uint64_t offset, uint8_t pcias, uint64_t data);
> +int vpci_bswap32_write(uint64_t offset, uint8_t pcias, uint32_t data);
> +int vpci_bswap16_write(uint64_t offset, uint8_t pcias, uint16_t data);
> +int vpci_write_byte(uint64_t offset, uint8_t pcias, uint8_t data);

I know it's pedantic, but since the reads are vpci_read_bXXXX while the writes are vcpi_bXXXX_write
(except the single byte case) could we make them consistent, please? (Soft preference for
vpci_read/write_bXXXX, but I don't much care.)

> +
> +#endif
> diff --git a/pc-bios/s390-ccw/virtio.h b/pc-bios/s390-ccw/virtio.h
> index c3cb5a6ee3..0e7dbdb64c 100644
> --- a/pc-bios/s390-ccw/virtio.h
> +++ b/pc-bios/s390-ccw/virtio.h
> @@ -18,6 +18,10 @@
>  #define VIRTIO_CONFIG_S_DRIVER          2
>  /* Driver has used its parts of the config, and is happy */
>  #define VIRTIO_CONFIG_S_DRIVER_OK       4
> +/* Feature negotiation complete */
> +#define VIRTIO_CONFIG_S_FEATURES_OK     8
> +/* Clear status byte */
> +#define VIRTIO_CONFIG_S_RESET           0x00

Heh, in v3 I wondered if you could add the new status bit(s) here. Buuuut... :)

Virtio section 2.1 defines "NEEDS_RESET" as 64 (0x40). Of course it says that the status field
starts as zero and is reinitialized to zero BY a reset, which is what you end up doing. Its
placement here between FEATURES_OK and FAILED with their spec-accurate definitions, but with a
different definition had me puzzled. Maybe move it to the top of this list, and rename it to
..._S_INIT or ..._S_ZERO or something? (Or don't define it and just set it to zero in the code.)

>  /* We've given up on this device. */
>  #define VIRTIO_CONFIG_S_FAILED          0x80
>  
> @@ -255,6 +259,7 @@ struct VDev {
>      uint8_t scsi_dev_heads;
>      bool scsi_device_selected;
>      ScsiDevice selected_scsi_device;
> +    uint32_t pci_fh;
>      uint32_t max_transfer;
>      uint32_t guest_features[2];
>  };


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

* Re: [PATCH v4 12/15] pc-bios/s390-ccw: Add support for virtio-blk-pci IPL
  2026-03-04  2:59 ` [PATCH v4 12/15] pc-bios/s390-ccw: Add support for virtio-blk-pci IPL jrossi
  2026-03-04  9:53   ` Thomas Huth
@ 2026-03-05 22:25   ` Eric Farman
  1 sibling, 0 replies; 35+ messages in thread
From: Eric Farman @ 2026-03-05 22:25 UTC (permalink / raw)
  To: jrossi, qemu-devel, qemu-s390x, thuth, mst
  Cc: jjherne, alifm, mjrosato, zycai

On Tue, 2026-03-03 at 21:59 -0500, jrossi@linux.ibm.com wrote:
> From: Jared Rossi <jrossi@linux.ibm.com>
> 
> Add little-endian virt-queue configuration and support for virtio-blk-pci IPL
> devices.
> 
> Signed-off-by: Jared Rossi <jrossi@linux.ibm.com>
> ---
>  pc-bios/s390-ccw/main.c          |  61 ++++++-
>  pc-bios/s390-ccw/pci.h           |   3 +
>  pc-bios/s390-ccw/virtio-blkdev.c |  18 +++
>  pc-bios/s390-ccw/virtio-pci.c    | 265 +++++++++++++++++++++++++++++++
>  pc-bios/s390-ccw/virtio-pci.h    |   2 +
>  pc-bios/s390-ccw/virtio.c        |  54 ++++++-
>  pc-bios/s390-ccw/virtio.h        |   1 +
>  7 files changed, 399 insertions(+), 5 deletions(-)

I realize this makes use of vpci_bswapXX_write routines that I made a comment on earlier. Feel free
to disregard that.

Reviewed-by: Eric Farman <farman@linux.ibm.com>


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

* Re: [PATCH v4 13/15] s390x: Build IPLB for virtio-pci devices
  2026-03-04  2:59 ` [PATCH v4 13/15] s390x: Build IPLB for virtio-pci devices jrossi
@ 2026-03-05 22:30   ` Eric Farman
  0 siblings, 0 replies; 35+ messages in thread
From: Eric Farman @ 2026-03-05 22:30 UTC (permalink / raw)
  To: jrossi, qemu-devel, qemu-s390x, thuth, mst
  Cc: jjherne, alifm, mjrosato, zycai

On Tue, 2026-03-03 at 21:59 -0500, jrossi@linux.ibm.com wrote:
> From: Jared Rossi <jrossi@linux.ibm.com>
> 
> Search for a corresponding S390PCIBusDevice and build an IPLB if a device has
> been indexed for boot but does not identify as a CCW device,
> 
> PCI devices are not yet included in boot probing (they must have a boot index).
> 
> Reviewed-by: Thomas Huth <thuth@redhat.com>
> Signed-off-by: Jared Rossi <jrossi@linux.ibm.com>
> ---
>  hw/s390x/ipl.c                  | 56 ++++++++++++++++++++++++++++++---
>  hw/s390x/ipl.h                  |  3 ++
>  hw/s390x/s390-pci-bus.c         |  3 +-
>  include/hw/s390x/s390-pci-bus.h |  2 ++
>  4 files changed, 57 insertions(+), 7 deletions(-)

Reviewed-by: Eric Farman <farman@linux.ibm.com>


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

end of thread, other threads:[~2026-03-05 22:31 UTC | newest]

Thread overview: 35+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-04  2:59 [PATCH v4 00/15] s390x: Add support for virtio-blk-pci IPL device jrossi
2026-03-04  2:59 ` [PATCH v4 01/15] pc-bios/s390-ccw: Fix misattributed function prototypes jrossi
2026-03-04  2:59 ` [PATCH v4 02/15] pc-bios/s390-ccw: Remove redundant vring schid attribute jrossi
2026-03-04  2:59 ` [PATCH v4 03/15] pc-bios/s390-ccw: Always reset virtio device on failed boot attempt jrossi
2026-03-04  8:23   ` Thomas Huth
2026-03-04 13:39   ` Eric Farman
2026-03-04  2:59 ` [PATCH v4 04/15] s390x: Remove duplicate definitions of IPL types jrossi
2026-03-04  8:34   ` Thomas Huth
2026-03-04  2:59 ` [PATCH v4 05/15] pc-bios/s390-ccw: Store device type independent of sense data jrossi
2026-03-04  2:59 ` [PATCH v4 06/15] pc-bios/s390-ccw: Split virtio-ccw and generic virtio jrossi
2026-03-04  8:42   ` Thomas Huth
2026-03-05 15:43   ` Eric Farman
2026-03-04  2:59 ` [PATCH v4 07/15] include/hw/s390x: Move CLP definitions for easier BIOS access jrossi
2026-03-04  2:59 ` [PATCH v4 08/15] pc-bios/s390-ccw: Introduce CLP Architecture jrossi
2026-03-04 14:05   ` Eric Farman
2026-03-04  2:59 ` [PATCH v4 09/15] s390x: Add definitions for PCI IPL type jrossi
2026-03-04  2:59 ` [PATCH v4 10/15] pc-bios/s390-ccw: Introduce PCI device jrossi
2026-03-04  9:10   ` Thomas Huth
2026-03-04 14:35     ` Jared Rossi
2026-03-04  2:59 ` [PATCH v4 11/15] pc-bios/s390-ccw: Introduce virtio-pci functions jrossi
2026-03-04  9:18   ` Thomas Huth
2026-03-04 14:38     ` Jared Rossi
2026-03-05 18:37   ` Eric Farman
2026-03-04  2:59 ` [PATCH v4 12/15] pc-bios/s390-ccw: Add support for virtio-blk-pci IPL jrossi
2026-03-04  9:53   ` Thomas Huth
2026-03-04 14:29     ` Jared Rossi
2026-03-04 14:39       ` Thomas Huth
2026-03-04 20:17         ` Jared Rossi
2026-03-05  6:16           ` Thomas Huth
2026-03-05 22:25   ` Eric Farman
2026-03-04  2:59 ` [PATCH v4 13/15] s390x: Build IPLB for virtio-pci devices jrossi
2026-03-05 22:30   ` Eric Farman
2026-03-04  2:59 ` [PATCH v4 14/15] hw: Add "loadparm" property to virtio block PCI devices booting on s390x jrossi
2026-03-04  9:59   ` Thomas Huth
2026-03-04  2:59 ` [PATCH v4 15/15] tests/qtest: Add s390x PCI boot test to cdrom-test.c jrossi

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.