All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v1 0/7] s390x: Add support for virtio-net-pci boot device
@ 2026-08-18 20:53 Zhuoying Cai
  2026-08-18 20:53 ` [PATCH v1 1/7] pc-bios/s390-ccw: Split virtio-ccw and generic virtio net Zhuoying Cai
                   ` (6 more replies)
  0 siblings, 7 replies; 28+ messages in thread
From: Zhuoying Cai @ 2026-08-18 20:53 UTC (permalink / raw)
  To: qemu-devel, qemu-s390x
  Cc: mst, jrossi, zycai, borntraeger, jjherne, cohuck, farman,
	mjrosato, pasic, farosas, lvivier, pbonzini

This series enables network booting via virtio-net-pci on s390x.

Refactor virtio-net to separate virtio-ccw specific logic and
create generic virtio-net helpers, and add support for booting
from virtio-net-pci devices with proper endianness handling.
Add support for the loadparm property during network boot.

Add corresponding test coverage to verify correct network boot
functionality with virtio-net-pci devices.

Zhuoying Cai (7):
  pc-bios/s390-ccw: Split virtio-ccw and generic virtio net
  pc-bios/s390-ccw: Add dynamic net header size handling
  pc-bios/s390-ccw: Introduce virtio_tswap helpers
  pc-bios/s390-ccw: Add support for virtio-net-pci IPL
  hw/virtio: Add "loadparm" property to virtio net PCI devices booting
    on s390x
  tests/qtest: Add s390x virtio net PCI test to pxe-test.c
  tests/functional/s390x: Add tests for virtio net PCI in
    test_pxelinux.py

 hw/virtio/virtio-net-pci.c              |  1 +
 pc-bios/s390-ccw/main.c                 |  1 +
 pc-bios/s390-ccw/netmain.c              | 64 +---------------------
 pc-bios/s390-ccw/virtio-ccw.c           | 62 +++++++++++++++++++++
 pc-bios/s390-ccw/virtio-ccw.h           |  1 +
 pc-bios/s390-ccw/virtio-net.c           | 69 +++++++++++++++++++-----
 pc-bios/s390-ccw/virtio-pci.c           | 43 +++++++++++++++
 pc-bios/s390-ccw/virtio-pci.h           |  2 +
 pc-bios/s390-ccw/virtio.c               | 27 +++++++---
 pc-bios/s390-ccw/virtio.h               |  5 ++
 tests/functional/s390x/test_pxelinux.py | 71 ++++++++++++++++++++-----
 tests/qtest/meson.build                 |  1 +
 tests/qtest/pxe-test.c                  |  1 +
 13 files changed, 254 insertions(+), 94 deletions(-)

-- 
2.55.0



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

* [PATCH v1 1/7] pc-bios/s390-ccw: Split virtio-ccw and generic virtio net
  2026-08-18 20:53 [PATCH v1 0/7] s390x: Add support for virtio-net-pci boot device Zhuoying Cai
@ 2026-08-18 20:53 ` Zhuoying Cai
  2026-08-26 16:26   ` Jason J. Herne
  2026-08-26 17:33   ` Matthew Rosato
  2026-08-18 20:53 ` [PATCH v1 2/7] pc-bios/s390-ccw: Add dynamic net header size handling Zhuoying Cai
                   ` (5 subsequent siblings)
  6 siblings, 2 replies; 28+ messages in thread
From: Zhuoying Cai @ 2026-08-18 20:53 UTC (permalink / raw)
  To: qemu-devel, qemu-s390x
  Cc: mst, jrossi, zycai, borntraeger, jjherne, cohuck, farman,
	mjrosato, pasic, farosas, lvivier, pbonzini

Refactor network device setup to separate virtio-ccw from
generic virtio-net logic for easier reuse of existing virtio
functions with non-CCW devices.

Signed-off-by: Zhuoying Cai <zycai@linux.ibm.com>
---
 pc-bios/s390-ccw/netmain.c    | 64 +----------------------------------
 pc-bios/s390-ccw/virtio-ccw.c | 62 +++++++++++++++++++++++++++++++++
 pc-bios/s390-ccw/virtio-ccw.h |  1 +
 pc-bios/s390-ccw/virtio-net.c | 10 ++++++
 pc-bios/s390-ccw/virtio.h     |  1 +
 5 files changed, 75 insertions(+), 63 deletions(-)

diff --git a/pc-bios/s390-ccw/netmain.c b/pc-bios/s390-ccw/netmain.c
index 791854fce0..e2f2278b8d 100644
--- a/pc-bios/s390-ccw/netmain.c
+++ b/pc-bios/s390-ccw/netmain.c
@@ -489,68 +489,6 @@ static int net_try_direct_tftp_load(filename_ip_t *fn_ip)
     return rc;
 }
 
-static bool find_net_dev(Schib *schib, int dev_no)
-{
-    int i, r;
-
-    for (i = 0; i < 0x10000; i++) {
-        net_schid.sch_no = i;
-        r = stsch_err(net_schid, schib);
-        if (r == 3 || r == -EIO) {
-            break;
-        }
-        if (!schib->pmcw.dnv) {
-            continue;
-        }
-        enable_subchannel(net_schid);
-        if (!virtio_is_supported(virtio_get_device())) {
-            continue;
-        }
-        if (virtio_get_device_type() != VIRTIO_ID_NET) {
-            continue;
-        }
-        if (dev_no < 0 || schib->pmcw.dev == dev_no) {
-            return true;
-        }
-    }
-
-    return false;
-}
-
-static bool virtio_setup(void)
-{
-    Schib schib;
-    int ssid;
-    bool found = false;
-    uint16_t dev_no;
-
-    /*
-     * We unconditionally enable mss support. In every sane configuration,
-     * this will succeed; and even if it doesn't, stsch_err() can deal
-     * with the consequences.
-     */
-    enable_mss_facility();
-
-    if (have_iplb || store_iplb(&iplb)) {
-        IPL_assert(iplb.pbt == S390_IPL_TYPE_CCW, "IPL_TYPE_CCW expected");
-        dev_no = iplb.ccw.devno;
-        debug_print_int("device no. ", dev_no);
-        net_schid.ssid = iplb.ccw.ssid & 0x3;
-        debug_print_int("ssid ", net_schid.ssid);
-        found = find_net_dev(&schib, dev_no);
-    } else {
-        for (ssid = 0; ssid < 0x3; ssid++) {
-            net_schid.ssid = ssid;
-            found = find_net_dev(&schib, -1);
-            if (found) {
-                break;
-            }
-        }
-    }
-
-    return found;
-}
-
 int netmain(void)
 {
     filename_ip_t fn_ip;
@@ -559,7 +497,7 @@ int netmain(void)
     sclp_setup();
     puts("Network boot starting...");
 
-    if (!virtio_setup()) {
+    if (!virtio_net_setup()) {
         puts("No virtio net device found.");
         return -1;
     }
diff --git a/pc-bios/s390-ccw/virtio-ccw.c b/pc-bios/s390-ccw/virtio-ccw.c
index 5cb2158ed2..403bd8a785 100644
--- a/pc-bios/s390-ccw/virtio-ccw.c
+++ b/pc-bios/s390-ccw/virtio-ccw.c
@@ -237,3 +237,65 @@ int virtio_ccw_setup(VDev *vdev)
 
     return 0;
 }
+
+static bool find_ccw_net_dev(Schib *schib, int dev_no)
+{
+    int i, r;
+
+    for (i = 0; i < 0x10000; i++) {
+        net_schid.sch_no = i;
+        r = stsch_err(net_schid, schib);
+        if (r == 3 || r == -EIO) {
+            break;
+        }
+        if (!schib->pmcw.dnv) {
+            continue;
+        }
+        enable_subchannel(net_schid);
+        if (!virtio_is_supported(virtio_get_device())) {
+            continue;
+        }
+        if (virtio_get_device_type() != VIRTIO_ID_NET) {
+            continue;
+        }
+        if (dev_no < 0 || schib->pmcw.dev == dev_no) {
+            return true;
+        }
+    }
+
+    return false;
+}
+
+bool virtio_ccw_net_setup(void)
+{
+    Schib schib;
+    int ssid;
+    bool found = false;
+    uint16_t dev_no;
+
+    /*
+     * We unconditionally enable mss support. In every sane configuration,
+     * this will succeed; and even if it doesn't, stsch_err() can deal
+     * with the consequences.
+     */
+    enable_mss_facility();
+
+    if (have_iplb || store_iplb(&iplb)) {
+        IPL_assert(iplb.pbt == S390_IPL_TYPE_CCW, "IPL_TYPE_CCW expected");
+        dev_no = iplb.ccw.devno;
+        debug_print_int("device no. ", dev_no);
+        net_schid.ssid = iplb.ccw.ssid & 0x3;
+        debug_print_int("ssid ", net_schid.ssid);
+        found = find_ccw_net_dev(&schib, dev_no);
+    } else {
+        for (ssid = 0; ssid < 0x3; ssid++) {
+            net_schid.ssid = ssid;
+            found = find_ccw_net_dev(&schib, -1);
+            if (found) {
+                break;
+            }
+        }
+    }
+
+    return found;
+}
diff --git a/pc-bios/s390-ccw/virtio-ccw.h b/pc-bios/s390-ccw/virtio-ccw.h
index a506767eaa..c330e02f99 100644
--- a/pc-bios/s390-ccw/virtio-ccw.h
+++ b/pc-bios/s390-ccw/virtio-ccw.h
@@ -20,5 +20,6 @@ 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);
+bool virtio_ccw_net_setup(void);
 
 #endif
diff --git a/pc-bios/s390-ccw/virtio-net.c b/pc-bios/s390-ccw/virtio-net.c
index f58f7ffc55..0ee51653ab 100644
--- a/pc-bios/s390-ccw/virtio-net.c
+++ b/pc-bios/s390-ccw/virtio-net.c
@@ -146,3 +146,13 @@ void virtio_net_deinit(void)
 {
     virtio_reset(virtio_get_device());
 }
+
+bool virtio_net_setup(void)
+{
+    switch (virtio_get_device()->ipl_type) {
+    case S390_IPL_TYPE_CCW:
+        return virtio_ccw_net_setup();
+    default:
+        return false;
+    }
+}
diff --git a/pc-bios/s390-ccw/virtio.h b/pc-bios/s390-ccw/virtio.h
index aa307025e0..04dbc65dbd 100644
--- a/pc-bios/s390-ccw/virtio.h
+++ b/pc-bios/s390-ccw/virtio.h
@@ -290,6 +290,7 @@ int virtio_setup_ccw(VDev *vdev);
 /* virtio-net.c */
 int virtio_net_init(void *mac_addr);
 void virtio_net_deinit(void);
+bool virtio_net_setup(void);
 
 /* virtio-blkdev.c */
 int virtio_blk_setup_device(VDev *vdev);
-- 
2.55.0



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

* [PATCH v1 2/7] pc-bios/s390-ccw: Add dynamic net header size handling
  2026-08-18 20:53 [PATCH v1 0/7] s390x: Add support for virtio-net-pci boot device Zhuoying Cai
  2026-08-18 20:53 ` [PATCH v1 1/7] pc-bios/s390-ccw: Split virtio-ccw and generic virtio net Zhuoying Cai
@ 2026-08-18 20:53 ` Zhuoying Cai
  2026-08-26 17:01   ` Jason J. Herne
  2026-08-26 17:34   ` Matthew Rosato
  2026-08-18 20:53 ` [PATCH v1 3/7] pc-bios/s390-ccw: Introduce virtio_tswap helpers Zhuoying Cai
                   ` (4 subsequent siblings)
  6 siblings, 2 replies; 28+ messages in thread
From: Zhuoying Cai @ 2026-08-18 20:53 UTC (permalink / raw)
  To: qemu-devel, qemu-s390x
  Cc: mst, jrossi, zycai, borntraeger, jjherne, cohuck, farman,
	mjrosato, pasic, farosas, lvivier, pbonzini

The virtio-net device used a fixed header size that did not account for
the num_buffers field used in VirtIO 1.0 or for the mergeable receive
buffers feature.

Use dynamic header sizing: 10 bytes for legacy mode and 12 bytes for
VirtIO 1.0 or when VIRTIO_NET_F_MRG_RXBUF is enabled. This ensures
correct packet handling across different VirtIO configurations.

Signed-off-by: Zhuoying Cai <zycai@linux.ibm.com>
---
 pc-bios/s390-ccw/virtio-net.c | 26 +++++++++++++++++++-------
 1 file changed, 19 insertions(+), 7 deletions(-)

diff --git a/pc-bios/s390-ccw/virtio-net.c b/pc-bios/s390-ccw/virtio-net.c
index 0ee51653ab..3a9ae789cf 100644
--- a/pc-bios/s390-ccw/virtio-net.c
+++ b/pc-bios/s390-ccw/virtio-net.c
@@ -20,6 +20,7 @@
 #include "s390-ccw.h"
 #include "virtio.h"
 #include "virtio-ccw.h"
+#include "virtio-pci.h"
 #include "s390-time.h"
 #include "helper.h"
 
@@ -28,10 +29,15 @@
 #endif
 
 #define VIRTIO_NET_F_MAC_BIT  (1 << 5)
+#define VIRTIO_NET_F_MRG_RXBUF_BIT (1 << 15)
 
 #define VQ_RX 0         /* Receive queue */
 #define VQ_TX 1         /* Transmit queue */
 
+/* Header sizes for different modes */
+#define VIRTIO_NET_HDR_SIZE_LEGACY  10  /* Without num_buffers */
+#define VIRTIO_NET_HDR_SIZE_V1      12  /* With num_buffers */
+
 struct VirtioNetHdr {
     uint8_t flags;
     uint8_t gso_type;
@@ -39,11 +45,12 @@ struct VirtioNetHdr {
     uint16_t gso_size;
     uint16_t csum_start;
     uint16_t csum_offset;
-    /*uint16_t num_buffers;*/ /* Only with VIRTIO_NET_F_MRG_RXBUF or VIRTIO1 */
+    uint16_t num_buffers; /* Only with VIRTIO_NET_F_MRG_RXBUF or VIRTIO1 */
 };
 typedef struct VirtioNetHdr VirtioNetHdr;
 
 static uint16_t rx_last_idx;  /* Last index in receive queue "used" ring */
+static int virtio_net_hdr_size;
 
 int virtio_net_init(void *mac_addr)
 {
@@ -62,12 +69,17 @@ int virtio_net_init(void *mac_addr)
         return -1;
     }
 
+    virtio_net_hdr_size = ((vdev->guest_features[1] & VIRTIO_F_VERSION_1) ||
+                           (vdev->guest_features[0] & VIRTIO_NET_F_MRG_RXBUF_BIT))
+                          ? VIRTIO_NET_HDR_SIZE_V1
+                          : VIRTIO_NET_HDR_SIZE_LEGACY;
+
     memcpy(mac_addr, vdev->config.net.mac, ETH_ALEN);
 
     for (i = 0; i < 64; i++) {
-        buf = malloc(ETH_MTU_SIZE + sizeof(VirtioNetHdr));
+        buf = malloc(ETH_MTU_SIZE + virtio_net_hdr_size);
         IPL_assert(buf != NULL, "Can not allocate memory for receive buffers");
-        vring_send_buf(rxvq, buf, ETH_MTU_SIZE + sizeof(VirtioNetHdr),
+        vring_send_buf(rxvq, buf, ETH_MTU_SIZE + virtio_net_hdr_size,
                        VRING_DESC_F_WRITE);
     }
     vring_notify(rxvq);
@@ -82,9 +94,9 @@ int send(int fd, const void *buf, int len, int flags)
     VRing *txvq = &vdev->vrings[VQ_TX];
 
     /* Set up header - we do not use anything special, so simply clear it */
-    memset(&tx_hdr, 0, sizeof(tx_hdr));
+    memset(&tx_hdr, 0, virtio_net_hdr_size);
 
-    vring_send_buf(txvq, &tx_hdr, sizeof(tx_hdr), VRING_DESC_F_NEXT);
+    vring_send_buf(txvq, &tx_hdr, virtio_net_hdr_size, VRING_DESC_F_NEXT);
     vring_send_buf(txvq, (void *)buf, len, VRING_HIDDEN_IS_CHAIN);
     while (!vr_poll(txvq)) {
         yield();
@@ -108,13 +120,13 @@ int recv(int fd, void *buf, int maxlen, int flags)
         return 0;
     }
 
-    len = rxvq->used->ring[rx_last_idx % rxvq->num].len - sizeof(VirtioNetHdr);
+    len = rxvq->used->ring[rx_last_idx % rxvq->num].len - virtio_net_hdr_size;
     if (len > maxlen) {
         puts("virtio-net: Receive buffer too small");
         len = maxlen;
     }
     id = rxvq->used->ring[rx_last_idx % rxvq->num].id % rxvq->num;
-    pkt = (uint8_t *)(rxvq->desc[id].addr + sizeof(VirtioNetHdr));
+    pkt = (uint8_t *)(rxvq->desc[id].addr + virtio_net_hdr_size);
 
 #if DEBUG_VIRTIO_NET   /* Dump packet */
     int i;
-- 
2.55.0



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

* [PATCH v1 3/7] pc-bios/s390-ccw: Introduce virtio_tswap helpers
  2026-08-18 20:53 [PATCH v1 0/7] s390x: Add support for virtio-net-pci boot device Zhuoying Cai
  2026-08-18 20:53 ` [PATCH v1 1/7] pc-bios/s390-ccw: Split virtio-ccw and generic virtio net Zhuoying Cai
  2026-08-18 20:53 ` [PATCH v1 2/7] pc-bios/s390-ccw: Add dynamic net header size handling Zhuoying Cai
@ 2026-08-18 20:53 ` Zhuoying Cai
  2026-08-26 17:20   ` Jason J. Herne
  2026-08-26 17:44   ` Matthew Rosato
  2026-08-18 20:53 ` [PATCH v1 4/7] pc-bios/s390-ccw: Add support for virtio-net-pci IPL Zhuoying Cai
                   ` (3 subsequent siblings)
  6 siblings, 2 replies; 28+ messages in thread
From: Zhuoying Cai @ 2026-08-18 20:53 UTC (permalink / raw)
  To: qemu-devel, qemu-s390x
  Cc: mst, jrossi, zycai, borntraeger, jjherne, cohuck, farman,
	mjrosato, pasic, farosas, lvivier, pbonzini

Replace the repeated pattern:

    be_ipl() ? x : bswapN(x)

with helpers that conditionally byte-swap based on the virtio transport
endianness.

Signed-off-by: Zhuoying Cai <zycai@linux.ibm.com>
---
 pc-bios/s390-ccw/virtio.c | 27 +++++++++++++++++++++------
 pc-bios/s390-ccw/virtio.h |  4 ++++
 2 files changed, 25 insertions(+), 6 deletions(-)

diff --git a/pc-bios/s390-ccw/virtio.c b/pc-bios/s390-ccw/virtio.c
index a0d249db24..037c00a3e1 100644
--- a/pc-bios/s390-ccw/virtio.c
+++ b/pc-bios/s390-ccw/virtio.c
@@ -97,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 = be_ipl() ? VRING_USED_F_NO_NOTIFY : bswap16(VRING_USED_F_NO_NOTIFY);
+    vr->used->flags = virtio_tswap16(VRING_USED_F_NO_NOTIFY);
     vr->used->idx = 0;
     vr->used_idx = 0;
     vr->next_idx = 0;
@@ -140,6 +140,22 @@ bool be_ipl(void)
     }
 }
 
+/* Conditionally byte-swap between virtio-endian and s390x native big-endian. */
+uint16_t virtio_tswap16(uint16_t x)
+{
+    return be_ipl() ? x : bswap16(x);
+}
+
+uint32_t virtio_tswap32(uint32_t x)
+{
+    return be_ipl() ? x : bswap32(x);
+}
+
+uint64_t virtio_tswap64(uint64_t x)
+{
+    return be_ipl() ? x : bswap64(x);
+}
+
 /*
  * Format the virtio ring descriptor endianness
  * Return the available index increment in the appropriate endianness
@@ -156,12 +172,11 @@ void vring_send_buf(VRing *vr, void *p, int len, int flags)
 {
     uint16_t avail_idx;
 
-    avail_idx = be_ipl() ? vr->avail->idx : bswap16(vr->avail->idx);
+    avail_idx = virtio_tswap16(vr->avail->idx);
 
     /* For follow-up chains we need to keep the first entry point */
     if (!(flags & VRING_HIDDEN_IS_CHAIN)) {
-        vr->avail->ring[avail_idx % vr->num] = be_ipl() ? vr->next_idx :
-                                                          bswap16(vr->next_idx);
+        vr->avail->ring[avail_idx % vr->num] = virtio_tswap16(vr->next_idx);
     }
 
     vr->desc[vr->next_idx].addr = (unsigned long)p;
@@ -179,7 +194,7 @@ void vring_send_buf(VRing *vr, void *p, int len, int flags)
     /* Chains only have a single ID */
     if (!(flags & VRING_DESC_F_NEXT)) {
         avail_idx++;
-        vr->avail->idx = be_ipl() ? avail_idx : bswap16(avail_idx);
+        vr->avail->idx = virtio_tswap16(avail_idx);
     }
 }
 
@@ -187,7 +202,7 @@ int vr_poll(VRing *vr)
 {
     uint16_t used_idx;
 
-    used_idx = be_ipl() ? vr->used->idx : bswap16(vr->used->idx);
+    used_idx = virtio_tswap16(vr->used->idx);
     if (used_idx == vr->used_idx) {
         vring_notify(vr);
         yield();
diff --git a/pc-bios/s390-ccw/virtio.h b/pc-bios/s390-ccw/virtio.h
index 04dbc65dbd..3c84a480d9 100644
--- a/pc-bios/s390-ccw/virtio.h
+++ b/pc-bios/s390-ccw/virtio.h
@@ -287,6 +287,10 @@ int virtio_run(VDev *vdev, int vqid, VirtioCmd *cmd);
 int virtio_reset(VDev *vdev);
 int virtio_setup_ccw(VDev *vdev);
 
+uint16_t virtio_tswap16(uint16_t x);
+uint32_t virtio_tswap32(uint32_t x);
+uint64_t virtio_tswap64(uint64_t x);
+
 /* virtio-net.c */
 int virtio_net_init(void *mac_addr);
 void virtio_net_deinit(void);
-- 
2.55.0



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

* [PATCH v1 4/7] pc-bios/s390-ccw: Add support for virtio-net-pci IPL
  2026-08-18 20:53 [PATCH v1 0/7] s390x: Add support for virtio-net-pci boot device Zhuoying Cai
                   ` (2 preceding siblings ...)
  2026-08-18 20:53 ` [PATCH v1 3/7] pc-bios/s390-ccw: Introduce virtio_tswap helpers Zhuoying Cai
@ 2026-08-18 20:53 ` Zhuoying Cai
  2026-08-26 14:31   ` Zhuoying Cai
                     ` (2 more replies)
  2026-08-18 20:53 ` [PATCH v1 5/7] hw/virtio: Add "loadparm" property to virtio net PCI devices booting on s390x Zhuoying Cai
                   ` (2 subsequent siblings)
  6 siblings, 3 replies; 28+ messages in thread
From: Zhuoying Cai @ 2026-08-18 20:53 UTC (permalink / raw)
  To: qemu-devel, qemu-s390x
  Cc: mst, jrossi, zycai, borntraeger, jjherne, cohuck, farman,
	mjrosato, pasic, farosas, lvivier, pbonzini

Enable network booting via virtio-net-pci by implementing PCI transport
support for virtio-net.

This patch also adds endianness handling for virtio PCI ring operations in
little-endian to ensure correct behavior on s390x.

Signed-off-by: Zhuoying Cai <zycai@linux.ibm.com>
---
 pc-bios/s390-ccw/main.c       |  1 +
 pc-bios/s390-ccw/virtio-net.c | 37 +++++++++++++++++++++++-------
 pc-bios/s390-ccw/virtio-pci.c | 43 +++++++++++++++++++++++++++++++++++
 pc-bios/s390-ccw/virtio-pci.h |  2 ++
 4 files changed, 75 insertions(+), 8 deletions(-)

diff --git a/pc-bios/s390-ccw/main.c b/pc-bios/s390-ccw/main.c
index 8bc6e8eaa3..0eeb53a2bf 100644
--- a/pc-bios/s390-ccw/main.c
+++ b/pc-bios/s390-ccw/main.c
@@ -328,6 +328,7 @@ static void ipl_pci_device(void)
     }
 
     switch (vdev->dev_type) {
+    case VIRTIO_ID_NET:
     case VIRTIO_ID_BLOCK:
         if (virtio_setup() == 0) {
             zipl_load(); /* only return on error */
diff --git a/pc-bios/s390-ccw/virtio-net.c b/pc-bios/s390-ccw/virtio-net.c
index 3a9ae789cf..209f98a0e3 100644
--- a/pc-bios/s390-ccw/virtio-net.c
+++ b/pc-bios/s390-ccw/virtio-net.c
@@ -62,7 +62,16 @@ int virtio_net_init(void *mac_addr)
     rx_last_idx = 0;
 
     vdev->guest_features[0] = VIRTIO_NET_F_MAC_BIT;
-    virtio_ccw_setup(vdev);
+    switch (virtio_get_device()->ipl_type) {
+    case S390_IPL_TYPE_CCW:
+        virtio_ccw_setup(vdev);
+        break;
+    case S390_IPL_TYPE_PCI:
+        virtio_pci_setup(vdev);
+        break;
+    default:
+        return -1;
+    }
 
     if (!(vdev->guest_features[0] & VIRTIO_NET_F_MAC_BIT)) {
         puts("virtio-net device does not support the MAC address feature");
@@ -115,22 +124,30 @@ int recv(int fd, void *buf, int maxlen, int flags)
     VRing *rxvq = &vdev->vrings[VQ_RX];
     int len, id;
     uint8_t *pkt;
+    uint16_t rx_used_idx, rx_avail_idx;
+    uint32_t rx_used_len, rx_used_id;
+    uint64_t rx_desc_addr;
 
-    if (rx_last_idx == rxvq->used->idx) {
+    rx_used_idx = virtio_tswap16(rxvq->used->idx);
+    if (rx_last_idx == rx_used_idx) {
         return 0;
     }
 
-    len = rxvq->used->ring[rx_last_idx % rxvq->num].len - virtio_net_hdr_size;
+    rx_used_len = virtio_tswap32(rxvq->used->ring[rx_last_idx % rxvq->num].len);
+    rx_used_id = virtio_tswap32(rxvq->used->ring[rx_last_idx % rxvq->num].id);
+
+    len = rx_used_len - virtio_net_hdr_size;
     if (len > maxlen) {
         puts("virtio-net: Receive buffer too small");
         len = maxlen;
     }
-    id = rxvq->used->ring[rx_last_idx % rxvq->num].id % rxvq->num;
-    pkt = (uint8_t *)(rxvq->desc[id].addr + virtio_net_hdr_size);
+    id = rx_used_id % rxvq->num;
+    rx_desc_addr = virtio_tswap64(rxvq->desc[id].addr);
+    pkt = (uint8_t *)(rx_desc_addr + virtio_net_hdr_size);
 
 #if DEBUG_VIRTIO_NET   /* Dump packet */
     int i;
-    printf("\nbuf %p: len=%i\n", (void *)rxvq->desc[id].addr, len);
+    printf("\nbuf %p: len=%i\n", (void *)rx_desc_addr, len);
     for (i = 0; i < 64; i++) {
         printf(" %02x", pkt[i]);
         if ((i % 16) == 15) {
@@ -144,8 +161,10 @@ int recv(int fd, void *buf, int maxlen, int flags)
     memcpy(buf, pkt, len);
 
     /* Mark buffer as available to the host again */
-    rxvq->avail->ring[rxvq->avail->idx % rxvq->num] = id;
-    rxvq->avail->idx = rxvq->avail->idx + 1;
+    rx_avail_idx = virtio_tswap16(rxvq->avail->idx);
+    rxvq->avail->ring[rx_avail_idx % rxvq->num] = virtio_tswap16(id);
+    rx_avail_idx++;
+    rxvq->avail->idx = virtio_tswap16(rx_avail_idx);
     vring_notify(rxvq);
 
     /* Move index to next entry */
@@ -164,6 +183,8 @@ bool virtio_net_setup(void)
     switch (virtio_get_device()->ipl_type) {
     case S390_IPL_TYPE_CCW:
         return virtio_ccw_net_setup();
+    case S390_IPL_TYPE_PCI:
+        return virtio_pci_net_setup();
     default:
         return false;
     }
diff --git a/pc-bios/s390-ccw/virtio-pci.c b/pc-bios/s390-ccw/virtio-pci.c
index f501252c81..c7d62766b8 100644
--- a/pc-bios/s390-ccw/virtio-pci.c
+++ b/pc-bios/s390-ccw/virtio-pci.c
@@ -52,6 +52,10 @@ void virtio_pci_id2type(VDev *vdev, uint16_t device_id)
     case 0x1001:
         vdev->dev_type = VIRTIO_ID_BLOCK;
         break;
+    case 0x1041:
+    case 0x1000:
+        vdev->dev_type = VIRTIO_ID_NET;
+        break;
     default:
         vdev->dev_type = 0;
     }
@@ -199,6 +203,14 @@ static int virtio_pci_get_blk_config(void)
     return rc;
 }
 
+static int virtio_pci_get_net_config(void)
+{
+    VirtioNetConfig *cfg = &virtio_get_device()->config.net;
+    int rc = vpci_read_flex(d_cap.off, d_cap.bar, cfg, sizeof(VirtioNetConfig));
+
+    return rc;
+}
+
 static int virtio_pci_negotiate(void)
 {
     int i, rc;
@@ -330,6 +342,7 @@ bool virtio_pci_is_supported(VDev *vdev)
     if (vdev->vendor_id == PCI_VENDOR_VIRTIO) {
         switch (vdev->dev_type) {
         case VIRTIO_ID_BLOCK:
+        case VIRTIO_ID_NET:
             return true;
         default:
             return false;
@@ -384,6 +397,11 @@ int virtio_pci_setup(VDev *vdev)
         vdev->cmd_vr_idx = 0;
         virtio_pci_get_blk_config();
         break;
+    case VIRTIO_ID_NET:
+        vdev->nr_vqs = 2;
+        vdev->cmd_vr_idx = 0;
+        virtio_pci_get_net_config();
+        break;
     default:
         puts("Unsupported virtio device");
         return -ENODEV;
@@ -458,3 +476,28 @@ int virtio_pci_setup_device(void)
 
     return 0;
 }
+
+static bool find_pci_net_dev(void)
+{
+    if (!virtio_is_supported(virtio_get_device())) {
+        return false;
+    }
+
+    if (virtio_get_device_type() != VIRTIO_ID_NET) {
+        return false;
+    }
+
+    return true;
+}
+
+bool virtio_pci_net_setup(void)
+{
+    bool found = false;
+
+    if (have_iplb || store_iplb(&iplb)) {
+        IPL_assert(iplb.pbt == S390_IPL_TYPE_PCI, "IPL_TYPE_PCI expected");
+        found = find_pci_net_dev();
+    }
+
+    return found;
+}
diff --git a/pc-bios/s390-ccw/virtio-pci.h b/pc-bios/s390-ccw/virtio-pci.h
index 33b683bd92..2c3136fd05 100644
--- a/pc-bios/s390-ccw/virtio-pci.h
+++ b/pc-bios/s390-ccw/virtio-pci.h
@@ -70,6 +70,8 @@ long virtio_pci_notify(VRing *vr);
 bool virtio_pci_is_supported(VDev *vdev);
 int virtio_pci_setup(VDev *vdev);
 int virtio_pci_setup_device(void);
+bool virtio_pci_is_supported(VDev *vdev);
+bool virtio_pci_net_setup(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);
-- 
2.55.0



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

* [PATCH v1 5/7] hw/virtio: Add "loadparm" property to virtio net PCI devices booting on s390x
  2026-08-18 20:53 [PATCH v1 0/7] s390x: Add support for virtio-net-pci boot device Zhuoying Cai
                   ` (3 preceding siblings ...)
  2026-08-18 20:53 ` [PATCH v1 4/7] pc-bios/s390-ccw: Add support for virtio-net-pci IPL Zhuoying Cai
@ 2026-08-18 20:53 ` Zhuoying Cai
  2026-08-28 17:00   ` Jason J. Herne
  2026-08-18 20:53 ` [PATCH v1 6/7] tests/qtest: Add s390x virtio net PCI test to pxe-test.c Zhuoying Cai
  2026-08-18 20:53 ` [PATCH v1 7/7] tests/functional/s390x: Add tests for virtio net PCI in test_pxelinux.py Zhuoying Cai
  6 siblings, 1 reply; 28+ messages in thread
From: Zhuoying Cai @ 2026-08-18 20:53 UTC (permalink / raw)
  To: qemu-devel, qemu-s390x
  Cc: mst, jrossi, zycai, borntraeger, jjherne, cohuck, farman,
	mjrosato, pasic, farosas, lvivier, pbonzini

s390x PCI IPL devices require a "loadparm" property. Add it to virtio-net-pci
to support booting from it on s390x.

Signed-off-by: Zhuoying Cai <zycai@linux.ibm.com>
---
 hw/virtio/virtio-net-pci.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/virtio/virtio-net-pci.c b/hw/virtio/virtio-net-pci.c
index fd3e0303f5..893d1905a9 100644
--- a/hw/virtio/virtio-net-pci.c
+++ b/hw/virtio/virtio-net-pci.c
@@ -77,6 +77,7 @@ static void virtio_net_pci_class_init(ObjectClass *klass, const void *data)
     k->sriov_vf_user_creatable = true;
     set_bit(DEVICE_CATEGORY_NETWORK, dc->categories);
     device_class_set_props(dc, virtio_net_properties);
+    pci_qdev_property_add_specifics(dc);
     vpciklass->realize = virtio_net_pci_realize;
 }
 
-- 
2.55.0



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

* [PATCH v1 6/7] tests/qtest: Add s390x virtio net PCI test to pxe-test.c
  2026-08-18 20:53 [PATCH v1 0/7] s390x: Add support for virtio-net-pci boot device Zhuoying Cai
                   ` (4 preceding siblings ...)
  2026-08-18 20:53 ` [PATCH v1 5/7] hw/virtio: Add "loadparm" property to virtio net PCI devices booting on s390x Zhuoying Cai
@ 2026-08-18 20:53 ` Zhuoying Cai
  2026-08-26 18:28   ` Joshua Daley
  2026-08-18 20:53 ` [PATCH v1 7/7] tests/functional/s390x: Add tests for virtio net PCI in test_pxelinux.py Zhuoying Cai
  6 siblings, 1 reply; 28+ messages in thread
From: Zhuoying Cai @ 2026-08-18 20:53 UTC (permalink / raw)
  To: qemu-devel, qemu-s390x
  Cc: mst, jrossi, zycai, borntraeger, jjherne, cohuck, farman,
	mjrosato, pasic, farosas, lvivier, pbonzini

Add test coverage for virtio-net-pci network booting on s390x by
including it in the PXE test suite. The test verifies that PXE
network booting works correctly with virtio-net-pci devices.

Signed-off-by: Zhuoying Cai <zycai@linux.ibm.com>
---
 tests/qtest/meson.build | 1 +
 tests/qtest/pxe-test.c  | 1 +
 2 files changed, 2 insertions(+)

diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index f7c7d06620..94e2555935 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -281,6 +281,7 @@ qtests_aarch64 = \
    'migration-test']
 
 qtests_s390x = \
+  (slirp.found() ? ['pxe-test'] : []) + \
   qtests_filter + \
   ['boot-serial-test',
    'drive_del-test',
diff --git a/tests/qtest/pxe-test.c b/tests/qtest/pxe-test.c
index a3f900fbea..d4b74e28cd 100644
--- a/tests/qtest/pxe-test.c
+++ b/tests/qtest/pxe-test.c
@@ -60,6 +60,7 @@ static testdef_t ppc64_tests_slow[] = {
 
 static testdef_t s390x_tests[] = {
     { "s390-ccw-virtio", "virtio-net-ccw" },
+    { "s390-ccw-virtio", "virtio-net-pci" },
     { NULL },
 };
 
-- 
2.55.0



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

* [PATCH v1 7/7] tests/functional/s390x: Add tests for virtio net PCI in test_pxelinux.py
  2026-08-18 20:53 [PATCH v1 0/7] s390x: Add support for virtio-net-pci boot device Zhuoying Cai
                   ` (5 preceding siblings ...)
  2026-08-18 20:53 ` [PATCH v1 6/7] tests/qtest: Add s390x virtio net PCI test to pxe-test.c Zhuoying Cai
@ 2026-08-18 20:53 ` Zhuoying Cai
  2026-08-26 18:17   ` Joshua Daley
  6 siblings, 1 reply; 28+ messages in thread
From: Zhuoying Cai @ 2026-08-18 20:53 UTC (permalink / raw)
  To: qemu-devel, qemu-s390x
  Cc: mst, jrossi, zycai, borntraeger, jjherne, cohuck, farman,
	mjrosato, pasic, farosas, lvivier, pbonzini

Add test coverage for virtio-net-pci network booting on s390x by
extending the pxelinux functional tests.

This patch adds PCI variants of all existing pxelinux tests to verify
that virtio-net-pci devices work correctly for network booting.

Signed-off-by: Zhuoying Cai <zycai@linux.ibm.com>
---
 tests/functional/s390x/test_pxelinux.py | 71 ++++++++++++++++++++-----
 1 file changed, 59 insertions(+), 12 deletions(-)

diff --git a/tests/functional/s390x/test_pxelinux.py b/tests/functional/s390x/test_pxelinux.py
index c00cce6a5a..7223364852 100755
--- a/tests/functional/s390x/test_pxelinux.py
+++ b/tests/functional/s390x/test_pxelinux.py
@@ -50,7 +50,8 @@ class S390PxeLinux(QemuSystemTest):
          '/images/kernel.img'),
         '480859574f3f44caa6cd35c62d70e1ac0609134e22ce2a954bbed9b110c06e0b')
 
-    def pxelinux_launch(self, pl_name='default', extra_opts=None):
+    def pxelinux_launch(self, pl_name='default', extra_opts=None,
+                        dev='virtio-net-ccw'):
         '''Create a pxelinux.cfg file in the right location and launch QEMU'''
         self.require_netdev('user')
         self.set_machine('s390-ccw-virtio')
@@ -74,7 +75,7 @@ def pxelinux_launch(self, pl_name='default', extra_opts=None):
         with open(cfg_fname, 'w', encoding='utf-8') as f:
             f.write(PXELINUX_CFG_CONTENTS)
 
-        virtio_net_dev = 'virtio-net-ccw,netdev=n1,bootindex=1'
+        virtio_net_dev = f'{dev},netdev=n1,bootindex=1'
         if extra_opts:
             virtio_net_dev += ',' + extra_opts
 
@@ -85,9 +86,9 @@ def pxelinux_launch(self, pl_name='default', extra_opts=None):
         self.vm.launch()
 
 
-    def test_default(self):
+    def do_test_default(self, dev):
         '''Check whether the guest uses the "default" file name'''
-        self.pxelinux_launch()
+        self.pxelinux_launch(dev=dev)
         # The kernel prints its arguments to the console, so we can use
         # this to check whether the kernel parameters are correctly handled:
         wait_for_console_pattern(self, 'testoption=teststring')
@@ -95,36 +96,82 @@ def test_default(self):
         wait_for_console_pattern(self, 'Unpacking initramfs...')
         wait_for_console_pattern(self, 'Run /init as init process')
 
-    def test_mac(self):
+    def do_test_mac(self, dev):
         '''Check whether the guest uses file name based on its MAC address'''
         self.pxelinux_launch(pl_name='01-02-ca-fe-ba-be-42',
-                             extra_opts='mac=02:ca:fe:ba:be:42,loadparm=3')
+                             extra_opts='mac=02:ca:fe:ba:be:42,loadparm=3',
+                             dev=dev)
         wait_for_console_pattern(self, 'Linux version 5.3.7-301.fc31.s390x')
 
-    def test_uuid(self):
+    def do_test_uuid(self, dev):
         '''Check whether the guest uses file name based on its UUID'''
         # Also add a non-bootable disk to check the fallback to network boot:
         self.vm.add_args('-blockdev', 'null-co,size=65536,node-name=d1',
                          '-device', 'virtio-blk,drive=d1,bootindex=0,loadparm=1',
                          '-uuid', '550e8400-e29b-11d4-a716-446655441234')
-        self.pxelinux_launch(pl_name='550e8400-e29b-11d4-a716-446655441234')
+        self.pxelinux_launch(pl_name='550e8400-e29b-11d4-a716-446655441234',
+                             dev=dev)
         wait_for_console_pattern(self, 'Debian 4.19.146-1 (2020-09-17)')
 
-    def test_ip(self):
+    def do_test_ip(self, dev):
         '''Check whether the guest uses file name based on its IP address'''
         self.vm.add_args('-M', 'loadparm=3')
-        self.pxelinux_launch(pl_name='0A00020F')
+        self.pxelinux_launch(pl_name='0A00020F', dev=dev)
         wait_for_console_pattern(self, 'Linux version 5.3.7-301.fc31.s390x')
 
-    def test_menu(self):
+    def do_test_menu(self, dev):
         '''Check whether the boot menu works for pxelinux.cfg booting'''
         self.vm.add_args('-boot', 'menu=on,splash-time=10')
-        self.pxelinux_launch(pl_name='0A00')
+        self.pxelinux_launch(pl_name='0A00', dev=dev)
         wait_for_console_pattern(self, '[1] Nonexisting')
         wait_for_console_pattern(self, '[2] Debian')
         wait_for_console_pattern(self, '[3] Fedora')
         wait_for_console_pattern(self, 'Debian 4.19.146-1 (2020-09-17)')
 
+    def test_default(self):
+        '''pxelinux.cfg "default" lookup via virtio-net-ccw'''
+        self.do_test_default('virtio-net-ccw')
+
+    def test_default_pci(self):
+        '''pxelinux.cfg "default" lookup via virtio-net-pci'''
+        self.require_device('virtio-net-pci')
+        self.do_test_default('virtio-net-pci')
+
+    def test_mac(self):
+        '''pxelinux.cfg MAC-address-based file lookup via virtio-net-ccw'''
+        self.do_test_mac('virtio-net-ccw')
+
+    def test_mac_pci(self):
+        '''pxelinux.cfg MAC-address-based file lookup via virtio-net-pci'''
+        self.require_device('virtio-net-pci')
+        self.do_test_mac('virtio-net-pci')
+
+    def test_uuid(self):
+        '''pxelinux.cfg UUID-based file lookup via virtio-net-ccw'''
+        self.do_test_uuid('virtio-net-ccw')
+
+    def test_uuid_pci(self):
+        '''pxelinux.cfg UUID-based file lookup via virtio-net-pci'''
+        self.require_device('virtio-net-pci')
+        self.do_test_uuid('virtio-net-pci')
+
+    def test_ip(self):
+        '''pxelinux.cfg IP-address-based file lookup via virtio-net-ccw'''
+        self.do_test_ip('virtio-net-ccw')
+
+    def test_ip_pci(self):
+        '''pxelinux.cfg IP-address-based file lookup via virtio-net-pci'''
+        self.require_device('virtio-net-pci')
+        self.do_test_ip('virtio-net-pci')
+
+    def test_menu(self):
+        '''pxelinux.cfg interactive boot menu via virtio-net-ccw'''
+        self.do_test_menu('virtio-net-ccw')
+
+    def test_menu_pci(self):
+        '''pxelinux.cfg interactive boot menu via virtio-net-pci'''
+        self.require_device('virtio-net-pci')
+        self.do_test_menu('virtio-net-pci')
 
 if __name__ == '__main__':
     QemuSystemTest.main()
-- 
2.55.0



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

* Re: [PATCH v1 4/7] pc-bios/s390-ccw: Add support for virtio-net-pci IPL
  2026-08-18 20:53 ` [PATCH v1 4/7] pc-bios/s390-ccw: Add support for virtio-net-pci IPL Zhuoying Cai
@ 2026-08-26 14:31   ` Zhuoying Cai
  2026-08-26 16:44   ` Jared Rossi
  2026-08-28 16:51   ` Jason J. Herne
  2 siblings, 0 replies; 28+ messages in thread
From: Zhuoying Cai @ 2026-08-26 14:31 UTC (permalink / raw)
  To: qemu-devel, qemu-s390x
  Cc: mst, jrossi, borntraeger, jjherne, cohuck, farman, mjrosato,
	pasic, farosas, lvivier, pbonzini

On 8/18/26 4:53 PM, Zhuoying Cai wrote:
> Enable network booting via virtio-net-pci by implementing PCI transport
> support for virtio-net.
> 
> This patch also adds endianness handling for virtio PCI ring operations in
> little-endian to ensure correct behavior on s390x.
> 
> Signed-off-by: Zhuoying Cai <zycai@linux.ibm.com>
> ---
>  pc-bios/s390-ccw/main.c       |  1 +
>  pc-bios/s390-ccw/virtio-net.c | 37 +++++++++++++++++++++++-------
>  pc-bios/s390-ccw/virtio-pci.c | 43 +++++++++++++++++++++++++++++++++++
>  pc-bios/s390-ccw/virtio-pci.h |  2 ++
>  4 files changed, 75 insertions(+), 8 deletions(-)
> 
> diff --git a/pc-bios/s390-ccw/main.c b/pc-bios/s390-ccw/main.c
> index 8bc6e8eaa3..0eeb53a2bf 100644
> --- a/pc-bios/s390-ccw/main.c
> +++ b/pc-bios/s390-ccw/main.c
> @@ -328,6 +328,7 @@ static void ipl_pci_device(void)
>      }
>  
>      switch (vdev->dev_type) {
> +    case VIRTIO_ID_NET:
>      case VIRTIO_ID_BLOCK:
>          if (virtio_setup() == 0) {
>              zipl_load(); /* only return on error */
> diff --git a/pc-bios/s390-ccw/virtio-net.c b/pc-bios/s390-ccw/virtio-net.c
> index 3a9ae789cf..209f98a0e3 100644
> --- a/pc-bios/s390-ccw/virtio-net.c
> +++ b/pc-bios/s390-ccw/virtio-net.c
> @@ -62,7 +62,16 @@ int virtio_net_init(void *mac_addr)
>      rx_last_idx = 0;
>  
>      vdev->guest_features[0] = VIRTIO_NET_F_MAC_BIT;
> -    virtio_ccw_setup(vdev);
> +    switch (virtio_get_device()->ipl_type) {
> +    case S390_IPL_TYPE_CCW:
> +        virtio_ccw_setup(vdev);
> +        break;
> +    case S390_IPL_TYPE_PCI:
> +        virtio_pci_setup(vdev);
> +        break;
> +    default:
> +        return -1;
> +    }
>  
>      if (!(vdev->guest_features[0] & VIRTIO_NET_F_MAC_BIT)) {
>          puts("virtio-net device does not support the MAC address feature");
> @@ -115,22 +124,30 @@ int recv(int fd, void *buf, int maxlen, int flags)
>      VRing *rxvq = &vdev->vrings[VQ_RX];
>      int len, id;
>      uint8_t *pkt;
> +    uint16_t rx_used_idx, rx_avail_idx;
> +    uint32_t rx_used_len, rx_used_id;
> +    uint64_t rx_desc_addr;
>  
> -    if (rx_last_idx == rxvq->used->idx) {
> +    rx_used_idx = virtio_tswap16(rxvq->used->idx);
> +    if (rx_last_idx == rx_used_idx) {
>          return 0;
>      }
>  
> -    len = rxvq->used->ring[rx_last_idx % rxvq->num].len - virtio_net_hdr_size;
> +    rx_used_len = virtio_tswap32(rxvq->used->ring[rx_last_idx % rxvq->num].len);
> +    rx_used_id = virtio_tswap32(rxvq->used->ring[rx_last_idx % rxvq->num].id);
> +
> +    len = rx_used_len - virtio_net_hdr_size;
>      if (len > maxlen) {
>          puts("virtio-net: Receive buffer too small");
>          len = maxlen;
>      }
> -    id = rxvq->used->ring[rx_last_idx % rxvq->num].id % rxvq->num;
> -    pkt = (uint8_t *)(rxvq->desc[id].addr + virtio_net_hdr_size);
> +    id = rx_used_id % rxvq->num;
> +    rx_desc_addr = virtio_tswap64(rxvq->desc[id].addr);
> +    pkt = (uint8_t *)(rx_desc_addr + virtio_net_hdr_size);
>  
>  #if DEBUG_VIRTIO_NET   /* Dump packet */
>      int i;
> -    printf("\nbuf %p: len=%i\n", (void *)rxvq->desc[id].addr, len);
> +    printf("\nbuf %p: len=%i\n", (void *)rx_desc_addr, len);
>      for (i = 0; i < 64; i++) {
>          printf(" %02x", pkt[i]);
>          if ((i % 16) == 15) {
> @@ -144,8 +161,10 @@ int recv(int fd, void *buf, int maxlen, int flags)
>      memcpy(buf, pkt, len);
>  
>      /* Mark buffer as available to the host again */
> -    rxvq->avail->ring[rxvq->avail->idx % rxvq->num] = id;
> -    rxvq->avail->idx = rxvq->avail->idx + 1;
> +    rx_avail_idx = virtio_tswap16(rxvq->avail->idx);
> +    rxvq->avail->ring[rx_avail_idx % rxvq->num] = virtio_tswap16(id);
> +    rx_avail_idx++;
> +    rxvq->avail->idx = virtio_tswap16(rx_avail_idx);
>      vring_notify(rxvq);
>  
>      /* Move index to next entry */
> @@ -164,6 +183,8 @@ bool virtio_net_setup(void)
>      switch (virtio_get_device()->ipl_type) {
>      case S390_IPL_TYPE_CCW:
>          return virtio_ccw_net_setup();
> +    case S390_IPL_TYPE_PCI:
> +        return virtio_pci_net_setup();
>      default:
>          return false;
>      }
> diff --git a/pc-bios/s390-ccw/virtio-pci.c b/pc-bios/s390-ccw/virtio-pci.c
> index f501252c81..c7d62766b8 100644
> --- a/pc-bios/s390-ccw/virtio-pci.c
> +++ b/pc-bios/s390-ccw/virtio-pci.c
> @@ -52,6 +52,10 @@ void virtio_pci_id2type(VDev *vdev, uint16_t device_id)
>      case 0x1001:
>          vdev->dev_type = VIRTIO_ID_BLOCK;
>          break;
> +    case 0x1041:
> +    case 0x1000:
> +        vdev->dev_type = VIRTIO_ID_NET;
> +        break;
>      default:
>          vdev->dev_type = 0;
>      }
> @@ -199,6 +203,14 @@ static int virtio_pci_get_blk_config(void)
>      return rc;
>  }
>  
> +static int virtio_pci_get_net_config(void)
> +{
> +    VirtioNetConfig *cfg = &virtio_get_device()->config.net;
> +    int rc = vpci_read_flex(d_cap.off, d_cap.bar, cfg, sizeof(VirtioNetConfig));
> +
> +    return rc;
> +}
> +
>  static int virtio_pci_negotiate(void)
>  {
>      int i, rc;
> @@ -330,6 +342,7 @@ bool virtio_pci_is_supported(VDev *vdev)
>      if (vdev->vendor_id == PCI_VENDOR_VIRTIO) {
>          switch (vdev->dev_type) {
>          case VIRTIO_ID_BLOCK:
> +        case VIRTIO_ID_NET:
>              return true;
>          default:
>              return false;
> @@ -384,6 +397,11 @@ int virtio_pci_setup(VDev *vdev)
>          vdev->cmd_vr_idx = 0;
>          virtio_pci_get_blk_config();
>          break;
> +    case VIRTIO_ID_NET:
> +        vdev->nr_vqs = 2;
> +        vdev->cmd_vr_idx = 0;
> +        virtio_pci_get_net_config();
> +        break;
>      default:
>          puts("Unsupported virtio device");
>          return -ENODEV;
> @@ -458,3 +476,28 @@ int virtio_pci_setup_device(void)
>  
>      return 0;
>  }
> +
> +static bool find_pci_net_dev(void)
> +{
> +    if (!virtio_is_supported(virtio_get_device())) {
> +        return false;
> +    }
> +
> +    if (virtio_get_device_type() != VIRTIO_ID_NET) {
> +        return false;
> +    }
> +
> +    return true;
> +}
> +
> +bool virtio_pci_net_setup(void)
> +{
> +    bool found = false;
> +
> +    if (have_iplb || store_iplb(&iplb)) {
> +        IPL_assert(iplb.pbt == S390_IPL_TYPE_PCI, "IPL_TYPE_PCI expected");
> +        found = find_pci_net_dev();
> +    }
> +
> +    return found;
> +}
> diff --git a/pc-bios/s390-ccw/virtio-pci.h b/pc-bios/s390-ccw/virtio-pci.h
> index 33b683bd92..2c3136fd05 100644
> --- a/pc-bios/s390-ccw/virtio-pci.h
> +++ b/pc-bios/s390-ccw/virtio-pci.h
> @@ -70,6 +70,8 @@ long virtio_pci_notify(VRing *vr);
>  bool virtio_pci_is_supported(VDev *vdev);
>  int virtio_pci_setup(VDev *vdev);
>  int virtio_pci_setup_device(void);
> +bool virtio_pci_is_supported(VDev *vdev);

This is a duplicate declaration and will be removed in the next revision.

> +bool virtio_pci_net_setup(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);



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

* Re: [PATCH v1 1/7] pc-bios/s390-ccw: Split virtio-ccw and generic virtio net
  2026-08-18 20:53 ` [PATCH v1 1/7] pc-bios/s390-ccw: Split virtio-ccw and generic virtio net Zhuoying Cai
@ 2026-08-26 16:26   ` Jason J. Herne
  2026-08-26 17:33   ` Matthew Rosato
  1 sibling, 0 replies; 28+ messages in thread
From: Jason J. Herne @ 2026-08-26 16:26 UTC (permalink / raw)
  To: Zhuoying Cai, qemu-devel, qemu-s390x
  Cc: mst, jrossi, borntraeger, cohuck, farman, mjrosato, pasic,
	farosas, lvivier, pbonzini

On 8/18/26 4:53 PM, Zhuoying Cai wrote:
> Refactor network device setup to separate virtio-ccw from
> generic virtio-net logic for easier reuse of existing virtio
> functions with non-CCW devices.
> 
> Signed-off-by: Zhuoying Cai <zycai@linux.ibm.com>
> ---
>   pc-bios/s390-ccw/netmain.c    | 64 +----------------------------------
>   pc-bios/s390-ccw/virtio-ccw.c | 62 +++++++++++++++++++++++++++++++++
>   pc-bios/s390-ccw/virtio-ccw.h |  1 +
>   pc-bios/s390-ccw/virtio-net.c | 10 ++++++
>   pc-bios/s390-ccw/virtio.h     |  1 +
>   5 files changed, 75 insertions(+), 63 deletions(-)
> ...

Reviewed-by: Jason J. Herne <jjherne@linux.ibm.com>


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

* Re: [PATCH v1 4/7] pc-bios/s390-ccw: Add support for virtio-net-pci IPL
  2026-08-18 20:53 ` [PATCH v1 4/7] pc-bios/s390-ccw: Add support for virtio-net-pci IPL Zhuoying Cai
  2026-08-26 14:31   ` Zhuoying Cai
@ 2026-08-26 16:44   ` Jared Rossi
  2026-08-31 18:36     ` Zhuoying Cai
  2026-08-28 16:51   ` Jason J. Herne
  2 siblings, 1 reply; 28+ messages in thread
From: Jared Rossi @ 2026-08-26 16:44 UTC (permalink / raw)
  To: Zhuoying Cai, qemu-devel, qemu-s390x
  Cc: mst, borntraeger, jjherne, cohuck, farman, mjrosato, pasic,
	farosas, lvivier, pbonzini



On 8/18/26 4:53 PM, Zhuoying Cai wrote:
> Enable network booting via virtio-net-pci by implementing PCI transport
> support for virtio-net.
>
> This patch also adds endianness handling for virtio PCI ring operations in
> little-endian to ensure correct behavior on s390x.

Endianness handling was actually added in the previous patch.  Is this a
misplaced message? Or perhaps it should say extends, rather than adds?

> Signed-off-by: Zhuoying Cai <zycai@linux.ibm.com>
> ---
>   pc-bios/s390-ccw/main.c       |  1 +
>   pc-bios/s390-ccw/virtio-net.c | 37 +++++++++++++++++++++++-------
>   pc-bios/s390-ccw/virtio-pci.c | 43 +++++++++++++++++++++++++++++++++++
>   pc-bios/s390-ccw/virtio-pci.h |  2 ++
>   4 files changed, 75 insertions(+), 8 deletions(-)
>
> diff --git a/pc-bios/s390-ccw/main.c b/pc-bios/s390-ccw/main.c
> index 8bc6e8eaa3..0eeb53a2bf 100644
> --- a/pc-bios/s390-ccw/main.c
> +++ b/pc-bios/s390-ccw/main.c
> @@ -328,6 +328,7 @@ static void ipl_pci_device(void)
>       }
>   
>       switch (vdev->dev_type) {
> +    case VIRTIO_ID_NET:
>       case VIRTIO_ID_BLOCK:
>           if (virtio_setup() == 0) {
>               zipl_load(); /* only return on error */
> diff --git a/pc-bios/s390-ccw/virtio-net.c b/pc-bios/s390-ccw/virtio-net.c
> index 3a9ae789cf..209f98a0e3 100644
> --- a/pc-bios/s390-ccw/virtio-net.c
> +++ b/pc-bios/s390-ccw/virtio-net.c
> @@ -62,7 +62,16 @@ int virtio_net_init(void *mac_addr)
>       rx_last_idx = 0;
>   
>       vdev->guest_features[0] = VIRTIO_NET_F_MAC_BIT;
> -    virtio_ccw_setup(vdev);
> +    switch (virtio_get_device()->ipl_type) {
> +    case S390_IPL_TYPE_CCW:
> +        virtio_ccw_setup(vdev);
> +        break;
> +    case S390_IPL_TYPE_PCI:
> +        virtio_pci_setup(vdev);
> +        break;
> +    default:
> +        return -1;
> +    }
>   
>       if (!(vdev->guest_features[0] & VIRTIO_NET_F_MAC_BIT)) {
>           puts("virtio-net device does not support the MAC address feature");
> @@ -115,22 +124,30 @@ int recv(int fd, void *buf, int maxlen, int flags)
>       VRing *rxvq = &vdev->vrings[VQ_RX];
>       int len, id;
>       uint8_t *pkt;
> +    uint16_t rx_used_idx, rx_avail_idx;
> +    uint32_t rx_used_len, rx_used_id;
> +    uint64_t rx_desc_addr;
>   
> -    if (rx_last_idx == rxvq->used->idx) {
> +    rx_used_idx = virtio_tswap16(rxvq->used->idx);
> +    if (rx_last_idx == rx_used_idx) {
>           return 0;
>       }
>   
> -    len = rxvq->used->ring[rx_last_idx % rxvq->num].len - virtio_net_hdr_size;
> +    rx_used_len = virtio_tswap32(rxvq->used->ring[rx_last_idx % rxvq->num].len);
> +    rx_used_id = virtio_tswap32(rxvq->used->ring[rx_last_idx % rxvq->num].id);
> +
> +    len = rx_used_len - virtio_net_hdr_size;
>       if (len > maxlen) {
>           puts("virtio-net: Receive buffer too small");
>           len = maxlen;
>       }
> -    id = rxvq->used->ring[rx_last_idx % rxvq->num].id % rxvq->num;
> -    pkt = (uint8_t *)(rxvq->desc[id].addr + virtio_net_hdr_size);
> +    id = rx_used_id % rxvq->num;
> +    rx_desc_addr = virtio_tswap64(rxvq->desc[id].addr);
> +    pkt = (uint8_t *)(rx_desc_addr + virtio_net_hdr_size);
>   
>   #if DEBUG_VIRTIO_NET   /* Dump packet */
>       int i;
> -    printf("\nbuf %p: len=%i\n", (void *)rxvq->desc[id].addr, len);
> +    printf("\nbuf %p: len=%i\n", (void *)rx_desc_addr, len);
>       for (i = 0; i < 64; i++) {
>           printf(" %02x", pkt[i]);
>           if ((i % 16) == 15) {
> @@ -144,8 +161,10 @@ int recv(int fd, void *buf, int maxlen, int flags)
>       memcpy(buf, pkt, len);
>   
>       /* Mark buffer as available to the host again */
> -    rxvq->avail->ring[rxvq->avail->idx % rxvq->num] = id;
> -    rxvq->avail->idx = rxvq->avail->idx + 1;
> +    rx_avail_idx = virtio_tswap16(rxvq->avail->idx);
> +    rxvq->avail->ring[rx_avail_idx % rxvq->num] = virtio_tswap16(id);
> +    rx_avail_idx++;
> +    rxvq->avail->idx = virtio_tswap16(rx_avail_idx);
>       vring_notify(rxvq);
>   
>       /* Move index to next entry */
> @@ -164,6 +183,8 @@ bool virtio_net_setup(void)
>       switch (virtio_get_device()->ipl_type) {
>       case S390_IPL_TYPE_CCW:
>           return virtio_ccw_net_setup();
> +    case S390_IPL_TYPE_PCI:
> +        return virtio_pci_net_setup();
>       default:
>           return false;
>       }
> diff --git a/pc-bios/s390-ccw/virtio-pci.c b/pc-bios/s390-ccw/virtio-pci.c
> index f501252c81..c7d62766b8 100644
> --- a/pc-bios/s390-ccw/virtio-pci.c
> +++ b/pc-bios/s390-ccw/virtio-pci.c
> @@ -52,6 +52,10 @@ void virtio_pci_id2type(VDev *vdev, uint16_t device_id)
>       case 0x1001:
>           vdev->dev_type = VIRTIO_ID_BLOCK;
>           break;
> +    case 0x1041:
> +    case 0x1000:
> +        vdev->dev_type = VIRTIO_ID_NET;
> +        break;
>       default:
>           vdev->dev_type = 0;
>       }
> @@ -199,6 +203,14 @@ static int virtio_pci_get_blk_config(void)
>       return rc;
>   }
>   
> +static int virtio_pci_get_net_config(void)
> +{
> +    VirtioNetConfig *cfg = &virtio_get_device()->config.net;
> +    int rc = vpci_read_flex(d_cap.off, d_cap.bar, cfg, sizeof(VirtioNetConfig));
> +
> +    return rc;
> +}
> +
>   static int virtio_pci_negotiate(void)
>   {
>       int i, rc;
> @@ -330,6 +342,7 @@ bool virtio_pci_is_supported(VDev *vdev)
>       if (vdev->vendor_id == PCI_VENDOR_VIRTIO) {
>           switch (vdev->dev_type) {
>           case VIRTIO_ID_BLOCK:
> +        case VIRTIO_ID_NET:
>               return true;
>           default:
>               return false;
> @@ -384,6 +397,11 @@ int virtio_pci_setup(VDev *vdev)
>           vdev->cmd_vr_idx = 0;
>           virtio_pci_get_blk_config();
>           break;
> +    case VIRTIO_ID_NET:
> +        vdev->nr_vqs = 2;
> +        vdev->cmd_vr_idx = 0;
> +        virtio_pci_get_net_config();
> +        break;
>       default:
>           puts("Unsupported virtio device");
>           return -ENODEV;
> @@ -458,3 +476,28 @@ int virtio_pci_setup_device(void)
>   
>       return 0;
>   }
> +
> +static bool find_pci_net_dev(void)
> +{
> +    if (!virtio_is_supported(virtio_get_device())) {
> +        return false;
> +    }
> +
> +    if (virtio_get_device_type() != VIRTIO_ID_NET) {
> +        return false;
> +    }
> +
> +    return true;
> +}
> +
> +bool virtio_pci_net_setup(void)
> +{
> +    bool found = false;
> +
> +    if (have_iplb || store_iplb(&iplb)) {
> +        IPL_assert(iplb.pbt == S390_IPL_TYPE_PCI, "IPL_TYPE_PCI expected");
> +        found = find_pci_net_dev();
> +    }
> +
> +    return found;
> +}

As far as I can tell the net specific setup/find are not needed for PCI
devices.  By the time we reach the the call to virtio_pci_net_setup() we
know vdev->ipl_type == S390_IPL_TYPE_PCI because that is the prerequisite
to call the function in the first place, and vdev->ipl_type was previously
set based on the PBT.  We also already know that our device type is
VIRTIO_ID_NET because otherwise the call to virtio_setup() in main.c
would have fallen through to the default case and failed and/or netmain()
would not be called from bootmap.c.

Although we haven't checked virtio_is_supported() yet, we will do that
verification in virtio_net_init(), which is the very next step.

For PCI devices, everything checked from virtio_net_setup() appears to be
guaranteed by the call chain or validated by subsequent net code. Maybe
virtio_net_setup() only needs to be called for CCW devices?

Regards,
Jared Rossi
> diff --git a/pc-bios/s390-ccw/virtio-pci.h b/pc-bios/s390-ccw/virtio-pci.h
> index 33b683bd92..2c3136fd05 100644
> --- a/pc-bios/s390-ccw/virtio-pci.h
> +++ b/pc-bios/s390-ccw/virtio-pci.h
> @@ -70,6 +70,8 @@ long virtio_pci_notify(VRing *vr);
>   bool virtio_pci_is_supported(VDev *vdev);
>   int virtio_pci_setup(VDev *vdev);
>   int virtio_pci_setup_device(void);
> +bool virtio_pci_is_supported(VDev *vdev);
> +bool virtio_pci_net_setup(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);



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

* Re: [PATCH v1 2/7] pc-bios/s390-ccw: Add dynamic net header size handling
  2026-08-18 20:53 ` [PATCH v1 2/7] pc-bios/s390-ccw: Add dynamic net header size handling Zhuoying Cai
@ 2026-08-26 17:01   ` Jason J. Herne
  2026-08-26 17:34   ` Matthew Rosato
  1 sibling, 0 replies; 28+ messages in thread
From: Jason J. Herne @ 2026-08-26 17:01 UTC (permalink / raw)
  To: Zhuoying Cai, qemu-devel, qemu-s390x
  Cc: mst, jrossi, borntraeger, cohuck, farman, mjrosato, pasic,
	farosas, lvivier, pbonzini



On 8/18/26 4:53 PM, Zhuoying Cai wrote:
> The virtio-net device used a fixed header size that did not account for
> the num_buffers field used in VirtIO 1.0 or for the mergeable receive
> buffers feature.
> 
> Use dynamic header sizing: 10 bytes for legacy mode and 12 bytes for
> VirtIO 1.0 or when VIRTIO_NET_F_MRG_RXBUF is enabled. This ensures
> correct packet handling across different VirtIO configurations.
> 
> Signed-off-by: Zhuoying Cai <zycai@linux.ibm.com>
> ---
>   pc-bios/s390-ccw/virtio-net.c | 26 +++++++++++++++++++-------
>   1 file changed, 19 insertions(+), 7 deletions(-)
> 
> diff --git a/pc-bios/s390-ccw/virtio-net.c b/pc-bios/s390-ccw/virtio-net.c
> index 0ee51653ab..3a9ae789cf 100644
> --- a/pc-bios/s390-ccw/virtio-net.c
> +++ b/pc-bios/s390-ccw/virtio-net.c
> @@ -20,6 +20,7 @@
>   #include "s390-ccw.h"
>   #include "virtio.h"
>   #include "virtio-ccw.h"
> +#include "virtio-pci.h"
>   #include "s390-time.h"
>   #include "helper.h"
>   
> @@ -28,10 +29,15 @@
>   #endif
>   
>   #define VIRTIO_NET_F_MAC_BIT  (1 << 5)
> +#define VIRTIO_NET_F_MRG_RXBUF_BIT (1 << 15)
>   
>   #define VQ_RX 0         /* Receive queue */
>   #define VQ_TX 1         /* Transmit queue */
>   
> +/* Header sizes for different modes */
> +#define VIRTIO_NET_HDR_SIZE_LEGACY  10  /* Without num_buffers */
> +#define VIRTIO_NET_HDR_SIZE_V1      12  /* With num_buffers */
> +
>   struct VirtioNetHdr {
>       uint8_t flags;
>       uint8_t gso_type;
> @@ -39,11 +45,12 @@ struct VirtioNetHdr {
>       uint16_t gso_size;
>       uint16_t csum_start;
>       uint16_t csum_offset;
> -    /*uint16_t num_buffers;*/ /* Only with VIRTIO_NET_F_MRG_RXBUF or VIRTIO1 */
> +    uint16_t num_buffers; /* Only with VIRTIO_NET_F_MRG_RXBUF or VIRTIO1 */
>   };
>   typedef struct VirtioNetHdr VirtioNetHdr;
>   
>   static uint16_t rx_last_idx;  /* Last index in receive queue "used" ring */
> +static int virtio_net_hdr_size;
>   
>   int virtio_net_init(void *mac_addr)
>   {
> @@ -62,12 +69,17 @@ int virtio_net_init(void *mac_addr)
>           return -1;
>       }
>   
> +    virtio_net_hdr_size = ((vdev->guest_features[1] & VIRTIO_F_VERSION_1) ||
> +                           (vdev->guest_features[0] & VIRTIO_NET_F_MRG_RXBUF_BIT))
> +                          ? VIRTIO_NET_HDR_SIZE_V1
> +                          : VIRTIO_NET_HDR_SIZE_LEGACY;
> +
>       memcpy(mac_addr, vdev->config.net.mac, ETH_ALEN);
>   
>       for (i = 0; i < 64; i++) {
> -        buf = malloc(ETH_MTU_SIZE + sizeof(VirtioNetHdr));
> +        buf = malloc(ETH_MTU_SIZE + virtio_net_hdr_size);
>           IPL_assert(buf != NULL, "Can not allocate memory for receive buffers");
> -        vring_send_buf(rxvq, buf, ETH_MTU_SIZE + sizeof(VirtioNetHdr),
> +        vring_send_buf(rxvq, buf, ETH_MTU_SIZE + virtio_net_hdr_size,
>                          VRING_DESC_F_WRITE);
>       }
>       vring_notify(rxvq);
> @@ -82,9 +94,9 @@ int send(int fd, const void *buf, int len, int flags)
>       VRing *txvq = &vdev->vrings[VQ_TX];
>   
>       /* Set up header - we do not use anything special, so simply clear it */
> -    memset(&tx_hdr, 0, sizeof(tx_hdr));
> +    memset(&tx_hdr, 0, virtio_net_hdr_size);

I think you want to leave this line as-is. There's no harm in cleaning 
the entire struct's memory even if we never end up using the final 
field. But only partially cleaning the struct looks weird and could 
potentially cause problems if subsequent code changes introduce code 
that attempts to read uninitialized data from num_buffers later.

With that change made:
Reviewed-by: Jason J. Herne <jjherne@linux.ibm.com>


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

* Re: [PATCH v1 3/7] pc-bios/s390-ccw: Introduce virtio_tswap helpers
  2026-08-18 20:53 ` [PATCH v1 3/7] pc-bios/s390-ccw: Introduce virtio_tswap helpers Zhuoying Cai
@ 2026-08-26 17:20   ` Jason J. Herne
  2026-08-26 17:44   ` Matthew Rosato
  1 sibling, 0 replies; 28+ messages in thread
From: Jason J. Herne @ 2026-08-26 17:20 UTC (permalink / raw)
  To: Zhuoying Cai, qemu-devel, qemu-s390x
  Cc: mst, jrossi, borntraeger, cohuck, farman, mjrosato, pasic,
	farosas, lvivier, pbonzini



On 8/18/26 4:53 PM, Zhuoying Cai wrote:
> Replace the repeated pattern:
> 
>      be_ipl() ? x : bswapN(x)
> 
> with helpers that conditionally byte-swap based on the virtio transport
> endianness.
> 
> Signed-off-by: Zhuoying Cai <zycai@linux.ibm.com>
> ---
>   pc-bios/s390-ccw/virtio.c | 27 +++++++++++++++++++++------
>   pc-bios/s390-ccw/virtio.h |  4 ++++
>   2 files changed, 25 insertions(+), 6 deletions(-)
> 
> diff --git a/pc-bios/s390-ccw/virtio.c b/pc-bios/s390-ccw/virtio.c
> index a0d249db24..037c00a3e1 100644
> --- a/pc-bios/s390-ccw/virtio.c
> +++ b/pc-bios/s390-ccw/virtio.c
> @@ -97,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 = be_ipl() ? VRING_USED_F_NO_NOTIFY : bswap16(VRING_USED_F_NO_NOTIFY);
> +    vr->used->flags = virtio_tswap16(VRING_USED_F_NO_NOTIFY);
>       vr->used->idx = 0;
>       vr->used_idx = 0;
>       vr->next_idx = 0;
> @@ -140,6 +140,22 @@ bool be_ipl(void)
>       }
>   }
>   
> +/* Conditionally byte-swap between virtio-endian and s390x native big-endian. */
> +uint16_t virtio_tswap16(uint16_t x)
> +{
> +    return be_ipl() ? x : bswap16(x);
> +}
> +
> +uint32_t virtio_tswap32(uint32_t x)
> +{
> +    return be_ipl() ? x : bswap32(x);
> +}
> +
> +uint64_t virtio_tswap64(uint64_t x)
> +{
> +    return be_ipl() ? x : bswap64(x);
> +}
> +
>   /*
>    * Format the virtio ring descriptor endianness
>    * Return the available index increment in the appropriate endianness
> @@ -156,12 +172,11 @@ void vring_send_buf(VRing *vr, void *p, int len, int flags)
>   {
>       uint16_t avail_idx;
>   
> -    avail_idx = be_ipl() ? vr->avail->idx : bswap16(vr->avail->idx);
> +    avail_idx = virtio_tswap16(vr->avail->idx);
>   
>       /* For follow-up chains we need to keep the first entry point */
>       if (!(flags & VRING_HIDDEN_IS_CHAIN)) {
> -        vr->avail->ring[avail_idx % vr->num] = be_ipl() ? vr->next_idx :
> -                                                          bswap16(vr->next_idx);
> +        vr->avail->ring[avail_idx % vr->num] = virtio_tswap16(vr->next_idx);
>       }
>   
>       vr->desc[vr->next_idx].addr = (unsigned long)p;
> @@ -179,7 +194,7 @@ void vring_send_buf(VRing *vr, void *p, int len, int flags)
>       /* Chains only have a single ID */
>       if (!(flags & VRING_DESC_F_NEXT)) {
>           avail_idx++;
> -        vr->avail->idx = be_ipl() ? avail_idx : bswap16(avail_idx);
> +        vr->avail->idx = virtio_tswap16(avail_idx);
>       }
>   }
>   
> @@ -187,7 +202,7 @@ int vr_poll(VRing *vr)
>   {
>       uint16_t used_idx;
>   
> -    used_idx = be_ipl() ? vr->used->idx : bswap16(vr->used->idx);
> +    used_idx = virtio_tswap16(vr->used->idx);
>       if (used_idx == vr->used_idx) {
>           vring_notify(vr);
>           yield();
> diff --git a/pc-bios/s390-ccw/virtio.h b/pc-bios/s390-ccw/virtio.h
> index 04dbc65dbd..3c84a480d9 100644
> --- a/pc-bios/s390-ccw/virtio.h
> +++ b/pc-bios/s390-ccw/virtio.h
> @@ -287,6 +287,10 @@ int virtio_run(VDev *vdev, int vqid, VirtioCmd *cmd);
>   int virtio_reset(VDev *vdev);
>   int virtio_setup_ccw(VDev *vdev);
>   
> +uint16_t virtio_tswap16(uint16_t x);
> +uint32_t virtio_tswap32(uint32_t x);
> +uint64_t virtio_tswap64(uint64_t x);
> +
>   /* virtio-net.c */
>   int virtio_net_init(void *mac_addr);
>   void virtio_net_deinit(void);

Reviewed-by: Jason J. Herne <jjherne@linux.ibm.com>



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

* Re: [PATCH v1 1/7] pc-bios/s390-ccw: Split virtio-ccw and generic virtio net
  2026-08-18 20:53 ` [PATCH v1 1/7] pc-bios/s390-ccw: Split virtio-ccw and generic virtio net Zhuoying Cai
  2026-08-26 16:26   ` Jason J. Herne
@ 2026-08-26 17:33   ` Matthew Rosato
  2026-08-26 19:30     ` Jared Rossi
  1 sibling, 1 reply; 28+ messages in thread
From: Matthew Rosato @ 2026-08-26 17:33 UTC (permalink / raw)
  To: Zhuoying Cai, qemu-devel, qemu-s390x
  Cc: mst, jrossi, borntraeger, jjherne, cohuck, farman, pasic, farosas,
	lvivier, pbonzini


> +bool virtio_net_setup(void)
> +{
> +    switch (virtio_get_device()->ipl_type) {
> +    case S390_IPL_TYPE_CCW:
> +        return virtio_ccw_net_setup();
> +    default:
> +        return false;
> +    }
> +}

This patch is largely renaming, but this does seem to have a subtle
functional change right here.

AFAICT before this patch attempting to netboot with anything other than
a ccw device would hit
IPL_assert(iplb.pbt == S390_IPL_TYPE_CCW, "IPL_TYPE_CCW expected");

Now, we will never call virtio_ccw_net_setup(), return false and instead
bail out with
"No virtio net device found."

That is new behavior for !IPL_TYPE_CCW after this patch.  For
IPL_TYPE_PCI, patch 4 will change the behavior again.

That's not a deal-breaker, but I do think it's worth a mention in the
commit message.  I then also wonder if the message
"No virtio net device found"
would be more accurate if it instead read something like:
"No supported virtio net device found"

Thanks,
Matt



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

* Re: [PATCH v1 2/7] pc-bios/s390-ccw: Add dynamic net header size handling
  2026-08-18 20:53 ` [PATCH v1 2/7] pc-bios/s390-ccw: Add dynamic net header size handling Zhuoying Cai
  2026-08-26 17:01   ` Jason J. Herne
@ 2026-08-26 17:34   ` Matthew Rosato
  1 sibling, 0 replies; 28+ messages in thread
From: Matthew Rosato @ 2026-08-26 17:34 UTC (permalink / raw)
  To: Zhuoying Cai, qemu-devel, qemu-s390x
  Cc: mst, jrossi, borntraeger, jjherne, cohuck, farman, pasic, farosas,
	lvivier, pbonzini

On 8/18/26 4:53 PM, Zhuoying Cai wrote:
> The virtio-net device used a fixed header size that did not account for
> the num_buffers field used in VirtIO 1.0 or for the mergeable receive
> buffers feature.
> 
> Use dynamic header sizing: 10 bytes for legacy mode and 12 bytes for
> VirtIO 1.0 or when VIRTIO_NET_F_MRG_RXBUF is enabled. This ensures
> correct packet handling across different VirtIO configurations.
> 
> Signed-off-by: Zhuoying Cai <zycai@linux.ibm.com>
> ---
>  pc-bios/s390-ccw/virtio-net.c | 26 +++++++++++++++++++-------
>  1 file changed, 19 insertions(+), 7 deletions(-)
> 
> diff --git a/pc-bios/s390-ccw/virtio-net.c b/pc-bios/s390-ccw/virtio-net.c
> index 0ee51653ab..3a9ae789cf 100644
> --- a/pc-bios/s390-ccw/virtio-net.c
> +++ b/pc-bios/s390-ccw/virtio-net.c
> @@ -20,6 +20,7 @@
>  #include "s390-ccw.h"
>  #include "virtio.h"
>  #include "virtio-ccw.h"
> +#include "virtio-pci.h"
>  #include "s390-time.h"
>  #include "helper.h"
>  
> @@ -28,10 +29,15 @@
>  #endif
>  
>  #define VIRTIO_NET_F_MAC_BIT  (1 << 5)
> +#define VIRTIO_NET_F_MRG_RXBUF_BIT (1 << 15)
>  
>  #define VQ_RX 0         /* Receive queue */
>  #define VQ_TX 1         /* Transmit queue */
>  
> +/* Header sizes for different modes */
> +#define VIRTIO_NET_HDR_SIZE_LEGACY  10  /* Without num_buffers */
> +#define VIRTIO_NET_HDR_SIZE_V1      12  /* With num_buffers */
> +

See my comment on Jared virtio-scsi series; you already have a structure
defined for this, and this would just be a versioned variation on it;
can you basically make a split structure and then calculate these
constants using the sizeof() each struct, rather than using constant
values here that circumstantially match the size of well-defined structures?

FWIW, this looks to be exactly what the kernel does with virtio_net_hdr
vs virtio_net_hdr_mrg_rxbuf in linux/uapi/linux/virtio_net.h (or
qemu/include/standard-headers/linux/virtio_net.h)

I am not suggesting we pull the standard-headers into s390-ccw, but I
would recommend using that layout as a basis of the structures in this
patch.

Thanks,
Matt



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

* Re: [PATCH v1 3/7] pc-bios/s390-ccw: Introduce virtio_tswap helpers
  2026-08-18 20:53 ` [PATCH v1 3/7] pc-bios/s390-ccw: Introduce virtio_tswap helpers Zhuoying Cai
  2026-08-26 17:20   ` Jason J. Herne
@ 2026-08-26 17:44   ` Matthew Rosato
  1 sibling, 0 replies; 28+ messages in thread
From: Matthew Rosato @ 2026-08-26 17:44 UTC (permalink / raw)
  To: Zhuoying Cai, qemu-devel, qemu-s390x
  Cc: mst, jrossi, borntraeger, jjherne, cohuck, farman, pasic, farosas,
	lvivier, pbonzini

On 8/18/26 4:53 PM, Zhuoying Cai wrote:
> Replace the repeated pattern:
> 
>     be_ipl() ? x : bswapN(x)
> 
> with helpers that conditionally byte-swap based on the virtio transport
> endianness.
> 

Maybe worth a mention that while the 16 variant is used immediately, the
32 and 64 variants are setup for later use -- since they will
technically be dead code at this point in git history.

Otherwise:

Reviewed-by: Matthew Rosato <mjrosato@linux.ibm.com>




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

* Re: [PATCH v1 7/7] tests/functional/s390x: Add tests for virtio net PCI in test_pxelinux.py
  2026-08-18 20:53 ` [PATCH v1 7/7] tests/functional/s390x: Add tests for virtio net PCI in test_pxelinux.py Zhuoying Cai
@ 2026-08-26 18:17   ` Joshua Daley
  2026-08-26 18:22     ` Joshua Daley
  0 siblings, 1 reply; 28+ messages in thread
From: Joshua Daley @ 2026-08-26 18:17 UTC (permalink / raw)
  To: Zhuoying Cai, qemu-devel, qemu-s390x
  Cc: mst, jrossi, borntraeger, jjherne, cohuck, farman, mjrosato,
	pasic, farosas, lvivier, pbonzini

On 8/18/2026 4:53 PM, Zhuoying Cai wrote:
> Add test coverage for virtio-net-pci network booting on s390x by
> extending the pxelinux functional tests.
> 
> This patch adds PCI variants of all existing pxelinux tests to verify
> that virtio-net-pci devices work correctly for network booting.
> 
> Signed-off-by: Zhuoying Cai <zycai@linux.ibm.com>
> ---
>   tests/functional/s390x/test_pxelinux.py | 71 ++++++++++++++++++++-----
>   1 file changed, 59 insertions(+), 12 deletions(-)
> Tested-by: Joshua Daley <jdaley@linux.ibm.com>


make check-functional-s390x MTESTARGS="func-s390x-pxelinux"

...
ok 1 test_pxelinux.S390PxeLinux.test_default
ok 2 test_pxelinux.S390PxeLinux.test_default_pci
ok 3 test_pxelinux.S390PxeLinux.test_ip
ok 4 test_pxelinux.S390PxeLinux.test_ip_pci
ok 5 test_pxelinux.S390PxeLinux.test_mac
ok 6 test_pxelinux.S390PxeLinux.test_mac_pci
ok 7 test_pxelinux.S390PxeLinux.test_menu
ok 8 test_pxelinux.S390PxeLinux.test_menu_pci
ok 9 test_pxelinux.S390PxeLinux.test_uuid
ok 10 test_pxelinux.S390PxeLinux.test_uuid_pci


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

* Re: [PATCH v1 7/7] tests/functional/s390x: Add tests for virtio net PCI in test_pxelinux.py
  2026-08-26 18:17   ` Joshua Daley
@ 2026-08-26 18:22     ` Joshua Daley
  0 siblings, 0 replies; 28+ messages in thread
From: Joshua Daley @ 2026-08-26 18:22 UTC (permalink / raw)
  To: Zhuoying Cai, qemu-devel, qemu-s390x
  Cc: mst, jrossi, borntraeger, jjherne, cohuck, farman, mjrosato,
	pasic, farosas, lvivier, pbonzini

On 8/26/2026 2:17 PM, Joshua Daley wrote:
> On 8/18/2026 4:53 PM, Zhuoying Cai wrote:
>> Add test coverage for virtio-net-pci network booting on s390x by
>> extending the pxelinux functional tests.
>>
>> This patch adds PCI variants of all existing pxelinux tests to verify
>> that virtio-net-pci devices work correctly for network booting.
>>
>> Signed-off-by: Zhuoying Cai <zycai@linux.ibm.com>
>> ---
>>   tests/functional/s390x/test_pxelinux.py | 71 ++++++++++++++++++++-----
>>   1 file changed, 59 insertions(+), 12 deletions(-)
>> Tested-by: Joshua Daley <jdaley@linux.ibm.com>
> 
> 
> make check-functional-s390x MTESTARGS="func-s390x-pxelinux"
> 
> ...
> ok 1 test_pxelinux.S390PxeLinux.test_default
> ok 2 test_pxelinux.S390PxeLinux.test_default_pci
> ok 3 test_pxelinux.S390PxeLinux.test_ip
> ok 4 test_pxelinux.S390PxeLinux.test_ip_pci
> ok 5 test_pxelinux.S390PxeLinux.test_mac
> ok 6 test_pxelinux.S390PxeLinux.test_mac_pci
> ok 7 test_pxelinux.S390PxeLinux.test_menu
> ok 8 test_pxelinux.S390PxeLinux.test_menu_pci
> ok 9 test_pxelinux.S390PxeLinux.test_uuid
> ok 10 test_pxelinux.S390PxeLinux.test_uuid_pci
> 

Sorry for the formatting error.
Tested-by: Joshua Daley <jdaley@linux.ibm.com>


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

* Re: [PATCH v1 6/7] tests/qtest: Add s390x virtio net PCI test to pxe-test.c
  2026-08-18 20:53 ` [PATCH v1 6/7] tests/qtest: Add s390x virtio net PCI test to pxe-test.c Zhuoying Cai
@ 2026-08-26 18:28   ` Joshua Daley
  0 siblings, 0 replies; 28+ messages in thread
From: Joshua Daley @ 2026-08-26 18:28 UTC (permalink / raw)
  To: Zhuoying Cai, qemu-devel, qemu-s390x
  Cc: mst, jrossi, borntraeger, jjherne, cohuck, farman, mjrosato,
	pasic, farosas, lvivier, pbonzini

On 8/18/2026 4:53 PM, Zhuoying Cai wrote:
> Add test coverage for virtio-net-pci network booting on s390x by
> including it in the PXE test suite. The test verifies that PXE
> network booting works correctly with virtio-net-pci devices.
> 
> Signed-off-by: Zhuoying Cai <zycai@linux.ibm.com>
> ---
>   tests/qtest/meson.build | 1 +
>   tests/qtest/pxe-test.c  | 1 +
>   2 files changed, 2 insertions(+)
> 
> diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
> index f7c7d06620..94e2555935 100644
> --- a/tests/qtest/meson.build
> +++ b/tests/qtest/meson.build
> @@ -281,6 +281,7 @@ qtests_aarch64 = \
>      'migration-test']
>   
>   qtests_s390x = \
> +  (slirp.found() ? ['pxe-test'] : []) + \
>     qtests_filter + \
>     ['boot-serial-test',
>      'drive_del-test',
> diff --git a/tests/qtest/pxe-test.c b/tests/qtest/pxe-test.c
> index a3f900fbea..d4b74e28cd 100644
> --- a/tests/qtest/pxe-test.c
> +++ b/tests/qtest/pxe-test.c
> @@ -60,6 +60,7 @@ static testdef_t ppc64_tests_slow[] = {
>   
>   static testdef_t s390x_tests[] = {
>       { "s390-ccw-virtio", "virtio-net-ccw" },
> +    { "s390-ccw-virtio", "virtio-net-pci" },
>       { NULL },
>   };
>   
Tested-by: Joshua Daley <jdaley@linux.ibm.com>
Resent because I used "Reply List" instead of "Reply All." Sorry!


meson test --suite qtest-s390x 'qtest-s390x/pxe-test'

...
# starting QEMU: exec ./qemu-system-s390x -qtest unix:/tmp/qtest-170442.sock -qtest-log /dev/null -chardev socket,path=/tmp/qtest-170442.qmp,id=char0 -object monitor-qmp,id=qmp0,chardev=char0 -display none -audio none -run-with exit-with-parent=on -accel kvm -accel tcg -machine s390-ccw-virtio -nodefaults -boot order=n -netdev user,id=net0,tftp=./,bootfile=tests/pxe-test-disk-ivTCmO,ipv4=on,ipv6=off -device virtio-net-pci,bootindex=1,netdev=net0   -accel qtest
ok 2 /s390x/pxe/ipv4/s390-ccw-virtio/virtio-net-pci



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

* Re: [PATCH v1 1/7] pc-bios/s390-ccw: Split virtio-ccw and generic virtio net
  2026-08-26 17:33   ` Matthew Rosato
@ 2026-08-26 19:30     ` Jared Rossi
  2026-08-26 19:41       ` Matthew Rosato
  0 siblings, 1 reply; 28+ messages in thread
From: Jared Rossi @ 2026-08-26 19:30 UTC (permalink / raw)
  To: Matthew Rosato, Zhuoying Cai, qemu-devel, qemu-s390x
  Cc: mst, borntraeger, jjherne, cohuck, farman, pasic, farosas,
	lvivier, pbonzini



On 8/26/26 1:33 PM, Matthew Rosato wrote:
>> +bool virtio_net_setup(void)
>> +{
>> +    switch (virtio_get_device()->ipl_type) {
>> +    case S390_IPL_TYPE_CCW:
>> +        return virtio_ccw_net_setup();
>> +    default:
>> +        return false;
>> +    }
>> +}
> This patch is largely renaming, but this does seem to have a subtle
> functional change right here.
>
> AFAICT before this patch attempting to netboot with anything other than
> a ccw device would hit
> IPL_assert(iplb.pbt == S390_IPL_TYPE_CCW, "IPL_TYPE_CCW expected");
>
> Now, we will never call virtio_ccw_net_setup(), return false and instead
> bail out with
> "No virtio net device found."
>
> That is new behavior for !IPL_TYPE_CCW after this patch.  For
> IPL_TYPE_PCI, patch 4 will change the behavior again.
>
> That's not a deal-breaker, but I do think it's worth a mention in the
> commit message.  I then also wonder if the message
> "No virtio net device found"
> would be more accurate if it instead read something like:
> "No supported virtio net device found"
>
> Thanks,
> Matt
>

I’m not sure this is a valid concern.  A non-ccw net device would be
rejected before getting to virtio_net_setup() earlier at the
find_boot_device() step either way.  In the case of virtio-net-pci
specifically, it would fail because VIRTIO_ID_NET is not a supported PCI
type yet.  For some sort of non-ccw non-pci netboot device, I believe there
wouldn't ever be an IPLB built for it, so it wouldn’t be recognized as
boot eligible at all.  I don’t think this patch affects any of that.

Regards,
Jared Rossi


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

* Re: [PATCH v1 1/7] pc-bios/s390-ccw: Split virtio-ccw and generic virtio net
  2026-08-26 19:30     ` Jared Rossi
@ 2026-08-26 19:41       ` Matthew Rosato
  2026-08-26 20:23         ` Jared Rossi
  0 siblings, 1 reply; 28+ messages in thread
From: Matthew Rosato @ 2026-08-26 19:41 UTC (permalink / raw)
  To: Jared Rossi, Zhuoying Cai, qemu-devel, qemu-s390x
  Cc: mst, borntraeger, jjherne, cohuck, farman, pasic, farosas,
	lvivier, pbonzini

On 8/26/26 3:30 PM, Jared Rossi wrote:
> 
> 
> On 8/26/26 1:33 PM, Matthew Rosato wrote:
>>> +bool virtio_net_setup(void)
>>> +{
>>> +    switch (virtio_get_device()->ipl_type) {
>>> +    case S390_IPL_TYPE_CCW:
>>> +        return virtio_ccw_net_setup();
>>> +    default:
>>> +        return false;
>>> +    }
>>> +}
>> This patch is largely renaming, but this does seem to have a subtle
>> functional change right here.
>>
>> AFAICT before this patch attempting to netboot with anything other than
>> a ccw device would hit
>> IPL_assert(iplb.pbt == S390_IPL_TYPE_CCW, "IPL_TYPE_CCW expected");
>>
>> Now, we will never call virtio_ccw_net_setup(), return false and instead
>> bail out with
>> "No virtio net device found."
>>
>> That is new behavior for !IPL_TYPE_CCW after this patch.  For
>> IPL_TYPE_PCI, patch 4 will change the behavior again.
>>
>> That's not a deal-breaker, but I do think it's worth a mention in the
>> commit message.  I then also wonder if the message
>> "No virtio net device found"
>> would be more accurate if it instead read something like:
>> "No supported virtio net device found"
>>
>> Thanks,
>> Matt
>>
> 
> I’m not sure this is a valid concern.  A non-ccw net device would be
> rejected before getting to virtio_net_setup() earlier at the
> find_boot_device() step either way.  In the case of virtio-net-pci
> specifically, it would fail because VIRTIO_ID_NET is not a supported PCI
> type yet.  For some sort of non-ccw non-pci netboot device, I believe there
> wouldn't ever be an IPLB built for it, so it wouldn’t be recognized as
> boot eligible at all.  I don’t think this patch affects any of that.
>


Based on that description it sounds like we don't even ever to expect to
reach the new default: case then, as prior checks should have already
weeded out all but the supported IPL_TYPE_* values.

Should the default: case have an IPL_assert with its own message then?

Thanks,
Matt



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

* Re: [PATCH v1 1/7] pc-bios/s390-ccw: Split virtio-ccw and generic virtio net
  2026-08-26 19:41       ` Matthew Rosato
@ 2026-08-26 20:23         ` Jared Rossi
  2026-08-26 20:49           ` Matthew Rosato
  0 siblings, 1 reply; 28+ messages in thread
From: Jared Rossi @ 2026-08-26 20:23 UTC (permalink / raw)
  To: Matthew Rosato, Zhuoying Cai, qemu-devel, qemu-s390x
  Cc: mst, borntraeger, jjherne, cohuck, farman, pasic, farosas,
	lvivier, pbonzini



On 8/26/26 3:41 PM, Matthew Rosato wrote:
> On 8/26/26 3:30 PM, Jared Rossi wrote:
>>
>> On 8/26/26 1:33 PM, Matthew Rosato wrote:
>>>> +bool virtio_net_setup(void)
>>>> +{
>>>> +    switch (virtio_get_device()->ipl_type) {
>>>> +    case S390_IPL_TYPE_CCW:
>>>> +        return virtio_ccw_net_setup();
>>>> +    default:
>>>> +        return false;
>>>> +    }
>>>> +}
>>> This patch is largely renaming, but this does seem to have a subtle
>>> functional change right here.
>>>
>>> AFAICT before this patch attempting to netboot with anything other than
>>> a ccw device would hit
>>> IPL_assert(iplb.pbt == S390_IPL_TYPE_CCW, "IPL_TYPE_CCW expected");
>>>
>>> Now, we will never call virtio_ccw_net_setup(), return false and instead
>>> bail out with
>>> "No virtio net device found."
>>>
>>> That is new behavior for !IPL_TYPE_CCW after this patch.  For
>>> IPL_TYPE_PCI, patch 4 will change the behavior again.
>>>
>>> That's not a deal-breaker, but I do think it's worth a mention in the
>>> commit message.  I then also wonder if the message
>>> "No virtio net device found"
>>> would be more accurate if it instead read something like:
>>> "No supported virtio net device found"
>>>
>>> Thanks,
>>> Matt
>>>
>> I’m not sure this is a valid concern.  A non-ccw net device would be
>> rejected before getting to virtio_net_setup() earlier at the
>> find_boot_device() step either way.  In the case of virtio-net-pci
>> specifically, it would fail because VIRTIO_ID_NET is not a supported PCI
>> type yet.  For some sort of non-ccw non-pci netboot device, I believe there
>> wouldn't ever be an IPLB built for it, so it wouldn’t be recognized as
>> boot eligible at all.  I don’t think this patch affects any of that.
>>
>
> Based on that description it sounds like we don't even ever to expect to
> reach the new default: case then, as prior checks should have already
> weeded out all but the supported IPL_TYPE_* values.
>
> Should the default: case have an IPL_assert with its own message then?

Right, we shouldn’t ever hit the default case, but I don’t follow your
suggestion about an IPL_assert.  What condition might be asserted there?

In my opinion simply returning false is appropriate, which will just print
the error and move on to the next boot device if there one.  A full panic
could be justified since something weird would need to happen to get into
the default case at all, but I don’t think it’s really necessary.

If virtio_net_setup() does fail though, regardless of why, I do agree the
subsequent error message needs to be updated to clarify that no “valid” or
“supported” virtio network device was found rather than stating none was
found at all.

Regards,
Jared Rossi


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

* Re: [PATCH v1 1/7] pc-bios/s390-ccw: Split virtio-ccw and generic virtio net
  2026-08-26 20:23         ` Jared Rossi
@ 2026-08-26 20:49           ` Matthew Rosato
  2026-08-31 17:17             ` Zhuoying Cai
  0 siblings, 1 reply; 28+ messages in thread
From: Matthew Rosato @ 2026-08-26 20:49 UTC (permalink / raw)
  To: Jared Rossi, Zhuoying Cai, qemu-devel, qemu-s390x
  Cc: mst, borntraeger, jjherne, cohuck, farman, pasic, farosas,
	lvivier, pbonzini

On 8/26/26 4:23 PM, Jared Rossi wrote:
> 
> 
> On 8/26/26 3:41 PM, Matthew Rosato wrote:
>> On 8/26/26 3:30 PM, Jared Rossi wrote:
>>>
>>> On 8/26/26 1:33 PM, Matthew Rosato wrote:
>>>>> +bool virtio_net_setup(void)
>>>>> +{
>>>>> +    switch (virtio_get_device()->ipl_type) {
>>>>> +    case S390_IPL_TYPE_CCW:
>>>>> +        return virtio_ccw_net_setup();
>>>>> +    default:
>>>>> +        return false;
>>>>> +    }
>>>>> +}
>>>> This patch is largely renaming, but this does seem to have a subtle
>>>> functional change right here.
>>>>
>>>> AFAICT before this patch attempting to netboot with anything other than
>>>> a ccw device would hit
>>>> IPL_assert(iplb.pbt == S390_IPL_TYPE_CCW, "IPL_TYPE_CCW expected");
>>>>
>>>> Now, we will never call virtio_ccw_net_setup(), return false and
>>>> instead
>>>> bail out with
>>>> "No virtio net device found."
>>>>
>>>> That is new behavior for !IPL_TYPE_CCW after this patch.  For
>>>> IPL_TYPE_PCI, patch 4 will change the behavior again.
>>>>
>>>> That's not a deal-breaker, but I do think it's worth a mention in the
>>>> commit message.  I then also wonder if the message
>>>> "No virtio net device found"
>>>> would be more accurate if it instead read something like:
>>>> "No supported virtio net device found"
>>>>
>>>> Thanks,
>>>> Matt
>>>>
>>> I’m not sure this is a valid concern.  A non-ccw net device would be
>>> rejected before getting to virtio_net_setup() earlier at the
>>> find_boot_device() step either way.  In the case of virtio-net-pci
>>> specifically, it would fail because VIRTIO_ID_NET is not a supported PCI
>>> type yet.  For some sort of non-ccw non-pci netboot device, I believe
>>> there
>>> wouldn't ever be an IPLB built for it, so it wouldn’t be recognized as
>>> boot eligible at all.  I don’t think this patch affects any of that.
>>>
>>
>> Based on that description it sounds like we don't even ever to expect to
>> reach the new default: case then, as prior checks should have already
>> weeded out all but the supported IPL_TYPE_* values.
>>
>> Should the default: case have an IPL_assert with its own message then?
>
> Right, we shouldn’t ever hit the default case, but I don’t follow your
> suggestion about an IPL_assert.  What condition might be asserted there?

Since it's a switch statement looking at the type value would be
redundant, but something like
IPL_assert(false, "unexpected IPL type");
should work

> 
> In my opinion simply returning false is appropriate, which will just print
> the error and move on to the next boot device if there one.  A full panic
> could be justified since something weird would need to happen to get into
> the default case at all, but I don’t think it’s really necessary.

Yeah, my line of thinking was that if we shouldn't reach this code but
somehow do, then something unexpected happened but we would kind of mask
that by simply moving onto the next device.

> 
> If virtio_net_setup() does fail though, regardless of why, I do agree the
> subsequent error message needs to be updated to clarify that no “valid” or
> “supported” virtio network device was found rather than stating none was
> found at all.
> 
I leave it up to you and Joy to decide if you feel it makes sense to
assert or update the message to be more accurate, but please do at least
one of those.

Outside of that, this patch LGTM.

Thanks,
Matt


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

* Re: [PATCH v1 4/7] pc-bios/s390-ccw: Add support for virtio-net-pci IPL
  2026-08-18 20:53 ` [PATCH v1 4/7] pc-bios/s390-ccw: Add support for virtio-net-pci IPL Zhuoying Cai
  2026-08-26 14:31   ` Zhuoying Cai
  2026-08-26 16:44   ` Jared Rossi
@ 2026-08-28 16:51   ` Jason J. Herne
  2 siblings, 0 replies; 28+ messages in thread
From: Jason J. Herne @ 2026-08-28 16:51 UTC (permalink / raw)
  To: Zhuoying Cai, qemu-devel, qemu-s390x
  Cc: mst, jrossi, borntraeger, cohuck, farman, mjrosato, pasic,
	farosas, lvivier, pbonzini


On 8/18/26 4:53 PM, Zhuoying Cai wrote:
> Enable network booting via virtio-net-pci by implementing PCI transport
> support for virtio-net.
> 
> This patch also adds endianness handling for virtio PCI ring operations in
> little-endian to ensure correct behavior on s390x.
> 
> Signed-off-by: Zhuoying Cai <zycai@linux.ibm.com>
> ---
>   pc-bios/s390-ccw/main.c       |  1 +
>   pc-bios/s390-ccw/virtio-net.c | 37 +++++++++++++++++++++++-------
>   pc-bios/s390-ccw/virtio-pci.c | 43 +++++++++++++++++++++++++++++++++++
>   pc-bios/s390-ccw/virtio-pci.h |  2 ++
>   4 files changed, 75 insertions(+), 8 deletions(-)
> ...
> +static bool find_pci_net_dev(void)
> +{
> +    if (!virtio_is_supported(virtio_get_device())) {
> +        return false;
> +    }
> +
> +    if (virtio_get_device_type() != VIRTIO_ID_NET) {
> +        return false;
> +    }
> +
> +    return true;
> +}
> +
> +bool virtio_pci_net_setup(void)
> +{
> +    bool found = false;
> +
> +    if (have_iplb || store_iplb(&iplb)) {
> +        IPL_assert(iplb.pbt == S390_IPL_TYPE_PCI, "IPL_TYPE_PCI expected");
> +        found = find_pci_net_dev();

Just return find_pci_net_dev() here, and false below.
That eliminates the found variable entirely.

> +    }
> +
> +    return found;
> +}
> diff --git a/pc-bios/s390-ccw/virtio-pci.h b/pc-bios/s390-ccw/virtio-pci.h
> index 33b683bd92..2c3136fd05 100644
> --- a/pc-bios/s390-ccw/virtio-pci.h
> +++ b/pc-bios/s390-ccw/virtio-pci.h
> @@ -70,6 +70,8 @@ long virtio_pci_notify(VRing *vr);
>   bool virtio_pci_is_supported(VDev *vdev);
>   int virtio_pci_setup(VDev *vdev);
>   int virtio_pci_setup_device(void);
> +bool virtio_pci_is_supported(VDev *vdev);

Accidental copy and paste here? You added
virtio_pci_is_supported() but it exists above :)

> +bool virtio_pci_net_setup(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);

With both of those fixed:
Reviewed-by: Jason J. Herne <jjherne@linux.ibm.com>




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

* Re: [PATCH v1 5/7] hw/virtio: Add "loadparm" property to virtio net PCI devices booting on s390x
  2026-08-18 20:53 ` [PATCH v1 5/7] hw/virtio: Add "loadparm" property to virtio net PCI devices booting on s390x Zhuoying Cai
@ 2026-08-28 17:00   ` Jason J. Herne
  0 siblings, 0 replies; 28+ messages in thread
From: Jason J. Herne @ 2026-08-28 17:00 UTC (permalink / raw)
  To: Zhuoying Cai, qemu-devel, qemu-s390x
  Cc: mst, jrossi, borntraeger, cohuck, farman, mjrosato, pasic,
	farosas, lvivier, pbonzini

On 8/18/26 4:53 PM, Zhuoying Cai wrote:
> s390x PCI IPL devices require a "loadparm" property. Add it to virtio-net-pci
> to support booting from it on s390x.
> 
> Signed-off-by: Zhuoying Cai <zycai@linux.ibm.com>
> ---
>   hw/virtio/virtio-net-pci.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/hw/virtio/virtio-net-pci.c b/hw/virtio/virtio-net-pci.c
> index fd3e0303f5..893d1905a9 100644
> --- a/hw/virtio/virtio-net-pci.c
> +++ b/hw/virtio/virtio-net-pci.c
> @@ -77,6 +77,7 @@ static void virtio_net_pci_class_init(ObjectClass *klass, const void *data)
>       k->sriov_vf_user_creatable = true;
>       set_bit(DEVICE_CATEGORY_NETWORK, dc->categories);
>       device_class_set_props(dc, virtio_net_properties);
> +    pci_qdev_property_add_specifics(dc);
>       vpciklass->realize = virtio_net_pci_realize;
>   }
>   

Should this patch be integrated into the last patch? If pci netboot 
breaks without it, then the answer is likely yes.

If, however, loadparm is not required (I suspect this to be the case), 
then the commit message should be changed as it implies that pci netboot 
will fail without a valid loadparm:

Add optional loadparm property to virtio-net-pci to allow boot entry 
selection on s390x, consistent with virtio-blk-pci.


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

* Re: [PATCH v1 1/7] pc-bios/s390-ccw: Split virtio-ccw and generic virtio net
  2026-08-26 20:49           ` Matthew Rosato
@ 2026-08-31 17:17             ` Zhuoying Cai
  0 siblings, 0 replies; 28+ messages in thread
From: Zhuoying Cai @ 2026-08-31 17:17 UTC (permalink / raw)
  To: Matthew Rosato, Jared Rossi, qemu-devel, qemu-s390x
  Cc: mst, borntraeger, jjherne, cohuck, farman, pasic, farosas,
	lvivier, pbonzini

On 8/26/26 4:49 PM, Matthew Rosato wrote:
> On 8/26/26 4:23 PM, Jared Rossi wrote:
>>
>>
>> On 8/26/26 3:41 PM, Matthew Rosato wrote:
>>> On 8/26/26 3:30 PM, Jared Rossi wrote:
>>>>
>>>> On 8/26/26 1:33 PM, Matthew Rosato wrote:
>>>>>> +bool virtio_net_setup(void)
>>>>>> +{
>>>>>> +    switch (virtio_get_device()->ipl_type) {
>>>>>> +    case S390_IPL_TYPE_CCW:
>>>>>> +        return virtio_ccw_net_setup();
>>>>>> +    default:
>>>>>> +        return false;
>>>>>> +    }
>>>>>> +}
>>>>> This patch is largely renaming, but this does seem to have a subtle
>>>>> functional change right here.
>>>>>
>>>>> AFAICT before this patch attempting to netboot with anything other than
>>>>> a ccw device would hit
>>>>> IPL_assert(iplb.pbt == S390_IPL_TYPE_CCW, "IPL_TYPE_CCW expected");
>>>>>
>>>>> Now, we will never call virtio_ccw_net_setup(), return false and
>>>>> instead
>>>>> bail out with
>>>>> "No virtio net device found."
>>>>>
>>>>> That is new behavior for !IPL_TYPE_CCW after this patch.  For
>>>>> IPL_TYPE_PCI, patch 4 will change the behavior again.
>>>>>
>>>>> That's not a deal-breaker, but I do think it's worth a mention in the
>>>>> commit message.  I then also wonder if the message
>>>>> "No virtio net device found"
>>>>> would be more accurate if it instead read something like:
>>>>> "No supported virtio net device found"
>>>>>
>>>>> Thanks,
>>>>> Matt
>>>>>
>>>> I’m not sure this is a valid concern.  A non-ccw net device would be
>>>> rejected before getting to virtio_net_setup() earlier at the
>>>> find_boot_device() step either way.  In the case of virtio-net-pci
>>>> specifically, it would fail because VIRTIO_ID_NET is not a supported PCI
>>>> type yet.  For some sort of non-ccw non-pci netboot device, I believe
>>>> there
>>>> wouldn't ever be an IPLB built for it, so it wouldn’t be recognized as
>>>> boot eligible at all.  I don’t think this patch affects any of that.
>>>>
>>>
>>> Based on that description it sounds like we don't even ever to expect to
>>> reach the new default: case then, as prior checks should have already
>>> weeded out all but the supported IPL_TYPE_* values.
>>>
>>> Should the default: case have an IPL_assert with its own message then?
>>
>> Right, we shouldn’t ever hit the default case, but I don’t follow your
>> suggestion about an IPL_assert.  What condition might be asserted there?
> 
> Since it's a switch statement looking at the type value would be
> redundant, but something like
> IPL_assert(false, "unexpected IPL type");
> should work
> 
>>
>> In my opinion simply returning false is appropriate, which will just print
>> the error and move on to the next boot device if there one.  A full panic
>> could be justified since something weird would need to happen to get into
>> the default case at all, but I don’t think it’s really necessary.
> 
> Yeah, my line of thinking was that if we shouldn't reach this code but
> somehow do, then something unexpected happened but we would kind of mask
> that by simply moving onto the next device.
> 
>>
>> If virtio_net_setup() does fail though, regardless of why, I do agree the
>> subsequent error message needs to be updated to clarify that no “valid” or
>> “supported” virtio network device was found rather than stating none was
>> found at all.
>>
> I leave it up to you and Joy to decide if you feel it makes sense to
> assert or update the message to be more accurate, but please do at least
> one of those.
> 
> Outside of that, this patch LGTM.
> 
> Thanks,
> Matt

Thanks for the suggestion!

Based on the discussion, I'd prefer having an IPL_assert/panic in the
default case and updating the subsequent error message to: "No valid
virtio-net device found." Since the default path should be unreachable,
reaching it would indicate an unexpected condition and should be treated
as fatal.

Please let me know if you'd prefer a different approach.




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

* Re: [PATCH v1 4/7] pc-bios/s390-ccw: Add support for virtio-net-pci IPL
  2026-08-26 16:44   ` Jared Rossi
@ 2026-08-31 18:36     ` Zhuoying Cai
  2026-09-02 10:12       ` Eric Farman
  0 siblings, 1 reply; 28+ messages in thread
From: Zhuoying Cai @ 2026-08-31 18:36 UTC (permalink / raw)
  To: Jared Rossi, qemu-devel, qemu-s390x
  Cc: mst, borntraeger, jjherne, cohuck, farman, mjrosato, pasic,
	farosas, lvivier, pbonzini

Thanks for the feedback!

On 8/26/26 12:44 PM, Jared Rossi wrote:
> 
> 
> On 8/18/26 4:53 PM, Zhuoying Cai wrote:
>> Enable network booting via virtio-net-pci by implementing PCI transport
>> support for virtio-net.
>>
>> This patch also adds endianness handling for virtio PCI ring operations in
>> little-endian to ensure correct behavior on s390x.
> 
> Endianness handling was actually added in the previous patch.  Is this a
> misplaced message? Or perhaps it should say extends, rather than adds?
> 

Good catch. It should be "extends" rather than "adds". I'll update the
commit message.

>> Signed-off-by: Zhuoying Cai <zycai@linux.ibm.com>
>> ---
>>   pc-bios/s390-ccw/main.c       |  1 +
>>   pc-bios/s390-ccw/virtio-net.c | 37 +++++++++++++++++++++++-------
>>   pc-bios/s390-ccw/virtio-pci.c | 43 +++++++++++++++++++++++++++++++++++
>>   pc-bios/s390-ccw/virtio-pci.h |  2 ++
>>   4 files changed, 75 insertions(+), 8 deletions(-)
>>
>> diff --git a/pc-bios/s390-ccw/main.c b/pc-bios/s390-ccw/main.c
>> index 8bc6e8eaa3..0eeb53a2bf 100644
>> --- a/pc-bios/s390-ccw/main.c
>> +++ b/pc-bios/s390-ccw/main.c
>> @@ -328,6 +328,7 @@ static void ipl_pci_device(void)
>>       }
>>   
>>       switch (vdev->dev_type) {
>> +    case VIRTIO_ID_NET:
>>       case VIRTIO_ID_BLOCK:
>>           if (virtio_setup() == 0) {
>>               zipl_load(); /* only return on error */
>> diff --git a/pc-bios/s390-ccw/virtio-net.c b/pc-bios/s390-ccw/virtio-net.c
>> index 3a9ae789cf..209f98a0e3 100644
>> --- a/pc-bios/s390-ccw/virtio-net.c
>> +++ b/pc-bios/s390-ccw/virtio-net.c
>> @@ -62,7 +62,16 @@ int virtio_net_init(void *mac_addr)
>>       rx_last_idx = 0;
>>   
>>       vdev->guest_features[0] = VIRTIO_NET_F_MAC_BIT;
>> -    virtio_ccw_setup(vdev);
>> +    switch (virtio_get_device()->ipl_type) {
>> +    case S390_IPL_TYPE_CCW:
>> +        virtio_ccw_setup(vdev);
>> +        break;
>> +    case S390_IPL_TYPE_PCI:
>> +        virtio_pci_setup(vdev);
>> +        break;
>> +    default:
>> +        return -1;
>> +    }
>>   
>>       if (!(vdev->guest_features[0] & VIRTIO_NET_F_MAC_BIT)) {
>>           puts("virtio-net device does not support the MAC address feature");
>> @@ -115,22 +124,30 @@ int recv(int fd, void *buf, int maxlen, int flags)
>>       VRing *rxvq = &vdev->vrings[VQ_RX];
>>       int len, id;
>>       uint8_t *pkt;
>> +    uint16_t rx_used_idx, rx_avail_idx;
>> +    uint32_t rx_used_len, rx_used_id;
>> +    uint64_t rx_desc_addr;
>>   
>> -    if (rx_last_idx == rxvq->used->idx) {
>> +    rx_used_idx = virtio_tswap16(rxvq->used->idx);
>> +    if (rx_last_idx == rx_used_idx) {
>>           return 0;
>>       }
>>   
>> -    len = rxvq->used->ring[rx_last_idx % rxvq->num].len - virtio_net_hdr_size;
>> +    rx_used_len = virtio_tswap32(rxvq->used->ring[rx_last_idx % rxvq->num].len);
>> +    rx_used_id = virtio_tswap32(rxvq->used->ring[rx_last_idx % rxvq->num].id);
>> +
>> +    len = rx_used_len - virtio_net_hdr_size;
>>       if (len > maxlen) {
>>           puts("virtio-net: Receive buffer too small");
>>           len = maxlen;
>>       }
>> -    id = rxvq->used->ring[rx_last_idx % rxvq->num].id % rxvq->num;
>> -    pkt = (uint8_t *)(rxvq->desc[id].addr + virtio_net_hdr_size);
>> +    id = rx_used_id % rxvq->num;
>> +    rx_desc_addr = virtio_tswap64(rxvq->desc[id].addr);
>> +    pkt = (uint8_t *)(rx_desc_addr + virtio_net_hdr_size);
>>   
>>   #if DEBUG_VIRTIO_NET   /* Dump packet */
>>       int i;
>> -    printf("\nbuf %p: len=%i\n", (void *)rxvq->desc[id].addr, len);
>> +    printf("\nbuf %p: len=%i\n", (void *)rx_desc_addr, len);
>>       for (i = 0; i < 64; i++) {
>>           printf(" %02x", pkt[i]);
>>           if ((i % 16) == 15) {
>> @@ -144,8 +161,10 @@ int recv(int fd, void *buf, int maxlen, int flags)
>>       memcpy(buf, pkt, len);
>>   
>>       /* Mark buffer as available to the host again */
>> -    rxvq->avail->ring[rxvq->avail->idx % rxvq->num] = id;
>> -    rxvq->avail->idx = rxvq->avail->idx + 1;
>> +    rx_avail_idx = virtio_tswap16(rxvq->avail->idx);
>> +    rxvq->avail->ring[rx_avail_idx % rxvq->num] = virtio_tswap16(id);
>> +    rx_avail_idx++;
>> +    rxvq->avail->idx = virtio_tswap16(rx_avail_idx);
>>       vring_notify(rxvq);
>>   
>>       /* Move index to next entry */
>> @@ -164,6 +183,8 @@ bool virtio_net_setup(void)
>>       switch (virtio_get_device()->ipl_type) {
>>       case S390_IPL_TYPE_CCW:
>>           return virtio_ccw_net_setup();
>> +    case S390_IPL_TYPE_PCI:
>> +        return virtio_pci_net_setup();
>>       default:
>>           return false;
>>       }
>> diff --git a/pc-bios/s390-ccw/virtio-pci.c b/pc-bios/s390-ccw/virtio-pci.c
>> index f501252c81..c7d62766b8 100644
>> --- a/pc-bios/s390-ccw/virtio-pci.c
>> +++ b/pc-bios/s390-ccw/virtio-pci.c
>> @@ -52,6 +52,10 @@ void virtio_pci_id2type(VDev *vdev, uint16_t device_id)
>>       case 0x1001:
>>           vdev->dev_type = VIRTIO_ID_BLOCK;
>>           break;
>> +    case 0x1041:
>> +    case 0x1000:
>> +        vdev->dev_type = VIRTIO_ID_NET;
>> +        break;
>>       default:
>>           vdev->dev_type = 0;
>>       }
>> @@ -199,6 +203,14 @@ static int virtio_pci_get_blk_config(void)
>>       return rc;
>>   }
>>   
>> +static int virtio_pci_get_net_config(void)
>> +{
>> +    VirtioNetConfig *cfg = &virtio_get_device()->config.net;
>> +    int rc = vpci_read_flex(d_cap.off, d_cap.bar, cfg, sizeof(VirtioNetConfig));
>> +
>> +    return rc;
>> +}
>> +
>>   static int virtio_pci_negotiate(void)
>>   {
>>       int i, rc;
>> @@ -330,6 +342,7 @@ bool virtio_pci_is_supported(VDev *vdev)
>>       if (vdev->vendor_id == PCI_VENDOR_VIRTIO) {
>>           switch (vdev->dev_type) {
>>           case VIRTIO_ID_BLOCK:
>> +        case VIRTIO_ID_NET:
>>               return true;
>>           default:
>>               return false;
>> @@ -384,6 +397,11 @@ int virtio_pci_setup(VDev *vdev)
>>           vdev->cmd_vr_idx = 0;
>>           virtio_pci_get_blk_config();
>>           break;
>> +    case VIRTIO_ID_NET:
>> +        vdev->nr_vqs = 2;
>> +        vdev->cmd_vr_idx = 0;
>> +        virtio_pci_get_net_config();
>> +        break;
>>       default:
>>           puts("Unsupported virtio device");
>>           return -ENODEV;
>> @@ -458,3 +476,28 @@ int virtio_pci_setup_device(void)
>>   
>>       return 0;
>>   }
>> +
>> +static bool find_pci_net_dev(void)
>> +{
>> +    if (!virtio_is_supported(virtio_get_device())) {
>> +        return false;
>> +    }
>> +
>> +    if (virtio_get_device_type() != VIRTIO_ID_NET) {
>> +        return false;
>> +    }
>> +
>> +    return true;
>> +}
>> +
>> +bool virtio_pci_net_setup(void)
>> +{
>> +    bool found = false;
>> +
>> +    if (have_iplb || store_iplb(&iplb)) {
>> +        IPL_assert(iplb.pbt == S390_IPL_TYPE_PCI, "IPL_TYPE_PCI expected");
>> +        found = find_pci_net_dev();
>> +    }
>> +
>> +    return found;
>> +}
> 
> As far as I can tell the net specific setup/find are not needed for PCI
> devices.  By the time we reach the the call to virtio_pci_net_setup() we
> know vdev->ipl_type == S390_IPL_TYPE_PCI because that is the prerequisite
> to call the function in the first place, and vdev->ipl_type was previously
> set based on the PBT.  We also already know that our device type is
> VIRTIO_ID_NET because otherwise the call to virtio_setup() in main.c
> would have fallen through to the default case and failed and/or netmain()
> would not be called from bootmap.c.
> 
> Although we haven't checked virtio_is_supported() yet, we will do that
> verification in virtio_net_init(), which is the very next step.
> 
> For PCI devices, everything checked from virtio_net_setup() appears to be
> guaranteed by the call chain or validated by subsequent net code. Maybe
> virtio_net_setup() only needs to be called for CCW devices?
> 

I was trying to align the CCW setup flow with the PCI devices, but I
agree that it isn't really necessary.

If virtio_net_setup() only needs to be called for CCW devices, is the
generic virtio_net_setup() wrapper still needed? Something like the
following seems sufficient:

if (virtio_get_device()->ipl_type == S390_IPL_TYPE_CCW &&
    !virtio_ccw_net_setup()) {
    puts("No valid virtio ccw net device found.");
}

Alternatively, we could simply add return true for the S390_IPL_TYPE_PCI
case in virtio_net_setup().

One thing worth noting is that virtio_net_init() calls virtio_*_setup(),
which performs virtio_is_supported() as its first step. However, the
return value of virtio_*_setup() is currently ignored in
virtio_net_init(). This is pre-existing behavior and applies to both the
CCW and PCI paths. If setup fails, execution still proceeds to MAC
negotiation and buffer allocation.

Since we are removing the network-specific setup from the PCI path,
would it make sense to also fix virtio_net_init() to check and propagate
the return value of virtio_*_setup()? The error is already reported by
virtio_*_setup(), but execution currently continues into MAC negotiation
and buffer allocation even when device initialization has failed.

> Regards,
> Jared Rossi
>> diff --git a/pc-bios/s390-ccw/virtio-pci.h b/pc-bios/s390-ccw/virtio-pci.h
>> index 33b683bd92..2c3136fd05 100644
>> --- a/pc-bios/s390-ccw/virtio-pci.h
>> +++ b/pc-bios/s390-ccw/virtio-pci.h
>> @@ -70,6 +70,8 @@ long virtio_pci_notify(VRing *vr);
>>   bool virtio_pci_is_supported(VDev *vdev);
>>   int virtio_pci_setup(VDev *vdev);
>>   int virtio_pci_setup_device(void);
>> +bool virtio_pci_is_supported(VDev *vdev);
>> +bool virtio_pci_net_setup(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);
> 



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

* Re: [PATCH v1 4/7] pc-bios/s390-ccw: Add support for virtio-net-pci IPL
  2026-08-31 18:36     ` Zhuoying Cai
@ 2026-09-02 10:12       ` Eric Farman
  0 siblings, 0 replies; 28+ messages in thread
From: Eric Farman @ 2026-09-02 10:12 UTC (permalink / raw)
  To: Zhuoying Cai, Jared Rossi, qemu-devel, qemu-s390x
  Cc: mst, borntraeger, jjherne, cohuck, mjrosato, pasic, farosas,
	lvivier, pbonzini



On 8/31/26 2:36 PM, Zhuoying Cai wrote:
> One thing worth noting is that virtio_net_init() calls virtio_*_setup(),
> which performs virtio_is_supported() as its first step. However, the
> return value of virtio_*_setup() is currently ignored in
> virtio_net_init(). This is pre-existing behavior and applies to both the
> CCW and PCI paths. If setup fails, execution still proceeds to MAC
> negotiation and buffer allocation.
> 
> Since we are removing the network-specific setup from the PCI path,
> would it make sense to also fix virtio_net_init() to check and propagate
> the return value of virtio_*_setup()? The error is already reported by
> virtio_*_setup(), but execution currently continues into MAC negotiation
> and buffer allocation even when device initialization has failed.

Yes, I would recommend making that change here as well, just as Jared is 
doing in the virtio-scsi path.


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

end of thread, other threads:[~2026-09-02 10:12 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-18 20:53 [PATCH v1 0/7] s390x: Add support for virtio-net-pci boot device Zhuoying Cai
2026-08-18 20:53 ` [PATCH v1 1/7] pc-bios/s390-ccw: Split virtio-ccw and generic virtio net Zhuoying Cai
2026-08-26 16:26   ` Jason J. Herne
2026-08-26 17:33   ` Matthew Rosato
2026-08-26 19:30     ` Jared Rossi
2026-08-26 19:41       ` Matthew Rosato
2026-08-26 20:23         ` Jared Rossi
2026-08-26 20:49           ` Matthew Rosato
2026-08-31 17:17             ` Zhuoying Cai
2026-08-18 20:53 ` [PATCH v1 2/7] pc-bios/s390-ccw: Add dynamic net header size handling Zhuoying Cai
2026-08-26 17:01   ` Jason J. Herne
2026-08-26 17:34   ` Matthew Rosato
2026-08-18 20:53 ` [PATCH v1 3/7] pc-bios/s390-ccw: Introduce virtio_tswap helpers Zhuoying Cai
2026-08-26 17:20   ` Jason J. Herne
2026-08-26 17:44   ` Matthew Rosato
2026-08-18 20:53 ` [PATCH v1 4/7] pc-bios/s390-ccw: Add support for virtio-net-pci IPL Zhuoying Cai
2026-08-26 14:31   ` Zhuoying Cai
2026-08-26 16:44   ` Jared Rossi
2026-08-31 18:36     ` Zhuoying Cai
2026-09-02 10:12       ` Eric Farman
2026-08-28 16:51   ` Jason J. Herne
2026-08-18 20:53 ` [PATCH v1 5/7] hw/virtio: Add "loadparm" property to virtio net PCI devices booting on s390x Zhuoying Cai
2026-08-28 17:00   ` Jason J. Herne
2026-08-18 20:53 ` [PATCH v1 6/7] tests/qtest: Add s390x virtio net PCI test to pxe-test.c Zhuoying Cai
2026-08-26 18:28   ` Joshua Daley
2026-08-18 20:53 ` [PATCH v1 7/7] tests/functional/s390x: Add tests for virtio net PCI in test_pxelinux.py Zhuoying Cai
2026-08-26 18:17   ` Joshua Daley
2026-08-26 18:22     ` Joshua Daley

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.