[PATCH v2 0/7] s390x: Add support for virtio-net-pci boot device

[PATCH v2 0/7] s390x: Add support for virtio-net-pci boot device

[Zhuoying Cai]

changlog v1->v2

pc-bios/s390-ccw: Move CCW net setup to virtio-ccw
- Removed the virtio_net_setup() dispatcher; netmain.c now calls
  virtio_ccw_net_setup() directly
- Improved the error message to be CCW-specific

pc-bios/s390-ccw: Add dynamic net header size handling
- Defined VirtioNetHdrMrgRxbuf, which includes VirtioNetHdr and a
  num_buffers field
- Defined header size constants using the sizeof() each struct

pc-bios/s390-ccw: Introduce virtio_tswap helpers
- Edited the commit message to mention that only 16-bit helper function
  is used in this commit
- Reviewed-by: Jason J. Herne <jjherne@linux.ibm.com>
- Reviewed-by: Matthew Rosato <mjrosato@linux.ibm.com>

pc-bios/s390-ccw: Add support for virtio-net-pci IPL
- Updated the commit message to change "adds" to "extends" when
  describing endianness handling
- Removed net specific setup/find for PCI device
- Checked for virtio initialization failures during network device
  initialization

hw/virtio: Add "loadparm" property to virtio net PCI devices booting on s390x
- Edited the commit message to avoid implying that PCI netboot will fail
  if a valid loadparm is not provided.

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
- Tested-by: Joshua Daley <jdaley@linux.ibm.com>

----------------------------------------------------------------------

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

Refactor virtio-net to separate virtio-ccw specific logic 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: Move CCW net setup to virtio-ccw
  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              | 74 ++++---------------------
 pc-bios/s390-ccw/virtio-ccw.c           | 62 +++++++++++++++++++++
 pc-bios/s390-ccw/virtio-ccw.h           |  1 +
 pc-bios/s390-ccw/virtio-net.c           | 67 ++++++++++++++++++----
 pc-bios/s390-ccw/virtio-pci.c           | 18 ++++++
 pc-bios/s390-ccw/virtio.c               | 27 +++++++--
 pc-bios/s390-ccw/virtio.h               |  4 ++
 tests/functional/s390x/test_pxelinux.py | 71 ++++++++++++++++++++----
 tests/qtest/meson.build                 |  1 +
 tests/qtest/pxe-test.c                  |  1 +
 12 files changed, 234 insertions(+), 94 deletions(-)

-- 
2.55.0

[PATCH v2 1/7] pc-bios/s390-ccw: Move CCW net setup to virtio-ccw

[Zhuoying Cai]

Move CCW-specific virtio_setup() and find_net_dev() to virtio-ccw.c
to decouple the generic net boot logic from the CCW transport.
Also update the error message on failure to specify "ccw net device".

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

diff --git a/pc-bios/s390-ccw/netmain.c b/pc-bios/s390-ccw/netmain.c
index ccd1c30013..bcb62bf6ca 100644
--- a/pc-bios/s390-ccw/netmain.c
+++ b/pc-bios/s390-ccw/netmain.c
@@ -35,6 +35,7 @@
 #include "s390-ccw.h"
 #include "cio.h"
 #include "virtio.h"
+#include "virtio-ccw.h"
 #include "s390-time.h"
 
 #define DEFAULT_BOOT_RETRIES 10
@@ -489,68 +490,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,8 +498,8 @@ int netmain(void)
     sclp_setup();
     puts("Network boot starting...");
 
-    if (!virtio_setup()) {
-        puts("No virtio net device found.");
+    if (!virtio_ccw_net_setup()) {
+        puts("No valid virtio ccw 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..5bf67738e4 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
-- 
2.55.0

[PATCH v2 2/7] pc-bios/s390-ccw: Add dynamic net header size handling

[Zhuoying Cai]

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 | 30 ++++++++++++++++++++++++------
 1 file changed, 24 insertions(+), 6 deletions(-)

diff --git a/pc-bios/s390-ccw/virtio-net.c b/pc-bios/s390-ccw/virtio-net.c
index f58f7ffc55..afa728bc32 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,6 +29,7 @@
 #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 */
@@ -43,7 +45,18 @@ struct VirtioNetHdr {
 };
 typedef struct VirtioNetHdr VirtioNetHdr;
 
+struct VirtioNetHdrMrgRxbuf {
+    struct VirtioNetHdr hdr;
+    uint16_t num_buffers; /* Only with VIRTIO_NET_F_MRG_RXBUF or VIRTIO1 */
+};
+typedef struct VirtioNetHdrMrgRxbuf VirtioNetHdrMrgRxbuf;
+
+/* Header sizes for different modes */
+#define VIRTIO_NET_HDR_SIZE_LEGACY  sizeof(VirtioNetHdr)
+#define VIRTIO_NET_HDR_SIZE_V1      sizeof(VirtioNetHdrMrgRxbuf)
+
 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 +75,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);
@@ -77,14 +95,14 @@ int virtio_net_init(void *mac_addr)
 
 int send(int fd, const void *buf, int len, int flags)
 {
-    VirtioNetHdr tx_hdr;
+    VirtioNetHdrMrgRxbuf tx_hdr;
     VDev *vdev = virtio_get_device();
     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));
 
-    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 +126,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

[PATCH v2 3/7] pc-bios/s390-ccw: Introduce virtio_tswap helpers

[Zhuoying Cai]

Replace the repeated pattern:

    be_ipl() ? x : bswapN(x)

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

While only the 16-bit helper is used immediately, the 32-bit and 64-bit
variants are added here for later use.

Signed-off-by: Zhuoying Cai <zycai@linux.ibm.com>
Reviewed-by: Jason J. Herne <jjherne@linux.ibm.com>
Reviewed-by: Matthew Rosato <mjrosato@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 aa307025e0..645dc288b1 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

[PATCH v2 4/7] pc-bios/s390-ccw: Add support for virtio-net-pci IPL

[Zhuoying Cai]

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

This patch also extends 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/netmain.c    |  9 +++++++-
 pc-bios/s390-ccw/virtio-net.c | 41 ++++++++++++++++++++++++++++-------
 pc-bios/s390-ccw/virtio-pci.c | 18 +++++++++++++++
 4 files changed, 60 insertions(+), 9 deletions(-)

diff --git a/pc-bios/s390-ccw/main.c b/pc-bios/s390-ccw/main.c
index 8fc1cbf653..28cab3b411 100644
--- a/pc-bios/s390-ccw/main.c
+++ b/pc-bios/s390-ccw/main.c
@@ -332,6 +332,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/netmain.c b/pc-bios/s390-ccw/netmain.c
index bcb62bf6ca..e42e049063 100644
--- a/pc-bios/s390-ccw/netmain.c
+++ b/pc-bios/s390-ccw/netmain.c
@@ -498,7 +498,14 @@ int netmain(void)
     sclp_setup();
     puts("Network boot starting...");
 
-    if (!virtio_ccw_net_setup()) {
+    /*
+     * CCW devices require subchannel enumeration here.
+     * PCI devices don't require specific setup: the transport is already
+     * initialized by ipl_pci_device(), and virtio_is_supported() is checked
+     * later in virtio_net_init().
+     */
+    if (virtio_get_device()->ipl_type == S390_IPL_TYPE_CCW &&
+        !virtio_ccw_net_setup()) {
         puts("No valid virtio ccw net device found.");
         return -1;
     }
diff --git a/pc-bios/s390-ccw/virtio-net.c b/pc-bios/s390-ccw/virtio-net.c
index afa728bc32..418759565d 100644
--- a/pc-bios/s390-ccw/virtio-net.c
+++ b/pc-bios/s390-ccw/virtio-net.c
@@ -68,7 +68,22 @@ 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:
+        if (virtio_ccw_setup(vdev)) {
+            puts("Setup failed for virtio-net-ccw");
+            return -1;
+        }
+        break;
+    case S390_IPL_TYPE_PCI:
+        if (virtio_pci_setup(vdev)) {
+            puts("Setup failed for virtio-net-pci");
+            return -1;
+        }
+        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");
@@ -121,22 +136,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) {
@@ -150,8 +173,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 */
diff --git a/pc-bios/s390-ccw/virtio-pci.c b/pc-bios/s390-ccw/virtio-pci.c
index f501252c81..147a82895f 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;
-- 
2.55.0

[PATCH v2 5/7] hw/virtio: Add "loadparm" property to virtio net PCI devices booting on s390x

[Zhuoying Cai]

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

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

[PATCH v2 6/7] tests/qtest: Add s390x virtio net PCI test to pxe-test.c

[Zhuoying Cai]

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>
Tested-by: Joshua Daley <jdaley@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 50a1a5272f..654e145fbe 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -289,6 +289,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 e85dec5a4e..bfa3b98cad 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

[PATCH v2 7/7] tests/functional/s390x: Add tests for virtio net PCI in test_pxelinux.py

[Zhuoying Cai]

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>
Tested-by: Joshua Daley <jdaley@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

Re: [PATCH v2 2/7] pc-bios/s390-ccw: Add dynamic net header size handling

[Jared Rossi]

On 9/3/26 12:24 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 | 30 ++++++++++++++++++++++++------
>   1 file changed, 24 insertions(+), 6 deletions(-)
>
> diff --git a/pc-bios/s390-ccw/virtio-net.c b/pc-bios/s390-ccw/virtio-net.c
> index f58f7ffc55..afa728bc32 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,6 +29,7 @@
>   #endif
>   
>   #define VIRTIO_NET_F_MAC_BIT  (1 << 5)
> +#define VIRTIO_NET_F_MRG_RXBUF_BIT (1 << 15)

We define this new feature bit, but it looks like we never set it.

The existing virtio_net_init() code has only:

vdev->guest_features[0] = VIRTIO_NET_F_MAC_BIT;

Should it be updated to VIRTIO_NET_F_MAC_BIT | VIRTIO_NET_F_MRG_RXBUF_BIT?

Is the new feature bit something we want to unconditionally request, only
request for PCI, or do we simply not care about it?
>   
>   #define VQ_RX 0         /* Receive queue */
>   #define VQ_TX 1         /* Transmit queue */
> @@ -43,7 +45,18 @@ struct VirtioNetHdr {
>   };
>   typedef struct VirtioNetHdr VirtioNetHdr;
>   
> +struct VirtioNetHdrMrgRxbuf {
> +    struct VirtioNetHdr hdr;
> +    uint16_t num_buffers; /* Only with VIRTIO_NET_F_MRG_RXBUF or VIRTIO1 */
> +};
> +typedef struct VirtioNetHdrMrgRxbuf VirtioNetHdrMrgRxbuf;
> +
> +/* Header sizes for different modes */
> +#define VIRTIO_NET_HDR_SIZE_LEGACY  sizeof(VirtioNetHdr)
> +#define VIRTIO_NET_HDR_SIZE_V1      sizeof(VirtioNetHdrMrgRxbuf)
> +
>   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 +75,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))

Because we do not set VIRTIO_NET_F_MRG_RXBUF in net_init(), only the first
half of this check can ever return true.

Maybe that is correct, but in that case we can simplify this to just
vdev->guest_features[1] & VIRTIO_F_VERSION_1, right?

Regards,
Jared Rossi
> +                          ? 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);
> @@ -77,14 +95,14 @@ int virtio_net_init(void *mac_addr)
>   
>   int send(int fd, const void *buf, int len, int flags)
>   {
> -    VirtioNetHdr tx_hdr;
> +    VirtioNetHdrMrgRxbuf tx_hdr;
>       VDev *vdev = virtio_get_device();
>       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));
>   
> -    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 +126,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;

Re: [PATCH v2 1/7] pc-bios/s390-ccw: Move CCW net setup to virtio-ccw

[Jason J. Herne]

On 9/3/26 12:24 PM, Zhuoying Cai wrote:
> Move CCW-specific virtio_setup() and find_net_dev() to virtio-ccw.c
> to decouple the generic net boot logic from the CCW transport.
> Also update the error message on failure to specify "ccw net device".
> 
> Signed-off-by: Zhuoying Cai <zycai@linux.ibm.com>
> ---
>   pc-bios/s390-ccw/netmain.c    | 67 ++---------------------------------
>   pc-bios/s390-ccw/virtio-ccw.c | 62 ++++++++++++++++++++++++++++++++
>   pc-bios/s390-ccw/virtio-ccw.h |  1 +
>   3 files changed, 66 insertions(+), 64 deletions(-)
> 
LGTM

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

Re: [PATCH v2 4/7] pc-bios/s390-ccw: Add support for virtio-net-pci IPL

[Jason J. Herne]

On 9/3/26 12:24 PM, Zhuoying Cai wrote:
> Enable network booting via virtio-net-pci by implementing PCI transport
> support for virtio-net.
> 
> This patch also extends 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/netmain.c    |  9 +++++++-
>   pc-bios/s390-ccw/virtio-net.c | 41 ++++++++++++++++++++++++++++-------
>   pc-bios/s390-ccw/virtio-pci.c | 18 +++++++++++++++
>   4 files changed, 60 insertions(+), 9 deletions(-)
> 
> diff --git a/pc-bios/s390-ccw/main.c b/pc-bios/s390-ccw/main.c
> index 8fc1cbf653..28cab3b411 100644
> --- a/pc-bios/s390-ccw/main.c
> +++ b/pc-bios/s390-ccw/main.c
> @@ -332,6 +332,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/netmain.c b/pc-bios/s390-ccw/netmain.c
> index bcb62bf6ca..e42e049063 100644
> --- a/pc-bios/s390-ccw/netmain.c
> +++ b/pc-bios/s390-ccw/netmain.c
> @@ -498,7 +498,14 @@ int netmain(void)
>       sclp_setup();
>       puts("Network boot starting...");
>   
> -    if (!virtio_ccw_net_setup()) {
> +    /*
> +     * CCW devices require subchannel enumeration here.
> +     * PCI devices don't require specific setup: the transport is already
> +     * initialized by ipl_pci_device(), and virtio_is_supported() is checked
> +     * later in virtio_net_init().
> +     */
> +    if (virtio_get_device()->ipl_type == S390_IPL_TYPE_CCW &&
> +        !virtio_ccw_net_setup()) {
>           puts("No valid virtio ccw net device found.");
>           return -1;
>       }
> diff --git a/pc-bios/s390-ccw/virtio-net.c b/pc-bios/s390-ccw/virtio-net.c
> index afa728bc32..418759565d 100644
> --- a/pc-bios/s390-ccw/virtio-net.c
> +++ b/pc-bios/s390-ccw/virtio-net.c
> @@ -68,7 +68,22 @@ 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:
> +        if (virtio_ccw_setup(vdev)) {
> +            puts("Setup failed for virtio-net-ccw");
> +            return -1;
> +        }
> +        break;
> +    case S390_IPL_TYPE_PCI:
> +        if (virtio_pci_setup(vdev)) {
> +            puts("Setup failed for virtio-net-pci");
> +            return -1;
> +        }
> +        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");
> @@ -121,22 +136,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) {
> @@ -150,8 +173,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 */
> diff --git a/pc-bios/s390-ccw/virtio-pci.c b/pc-bios/s390-ccw/virtio-pci.c
> index f501252c81..147a82895f 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;
> +}
> +

Simplify to this:

static int virtio_pci_get_net_config(void)
{
     VirtioNetConfig *cfg = &virtio_get_device()->config.net;

     return vpci_read_flex(d_cap.off, d_cap.bar, cfg, 
sizeof(VirtioNetConfig));
}


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

cmd_vr_idx is never actually used for net devices. I'm thinking we want 
to skip setting it here. It's actually already set to 0 above, in common 
code, before we know what device type we're dealing with.

Re: [PATCH v2 5/7] hw/virtio: Add "loadparm" property to virtio net PCI devices booting on s390x

[Jason J. Herne]

On 9/3/26 12:24 PM, Zhuoying Cai wrote:
> Add optional loadparm property to virtio-net-pci to allow boot entry
> selection on s390x, consistent with virtio-blk-pci.
> 
> 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;
>   }
>   

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

Re: [PATCH v2 2/7] pc-bios/s390-ccw: Add dynamic net header size handling

[Zhuoying Cai]

On 9/8/26 2:32 PM, Jared Rossi wrote:
> 
> 
> On 9/3/26 12:24 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 | 30 ++++++++++++++++++++++++------
>>   1 file changed, 24 insertions(+), 6 deletions(-)
>>
>> diff --git a/pc-bios/s390-ccw/virtio-net.c b/pc-bios/s390-ccw/virtio-net.c
>> index f58f7ffc55..afa728bc32 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,6 +29,7 @@
>>   #endif
>>   
>>   #define VIRTIO_NET_F_MAC_BIT  (1 << 5)
>> +#define VIRTIO_NET_F_MRG_RXBUF_BIT (1 << 15)
> 
> We define this new feature bit, but it looks like we never set it.
> 
> The existing virtio_net_init() code has only:
> 
> vdev->guest_features[0] = VIRTIO_NET_F_MAC_BIT;
> 
> Should it be updated to VIRTIO_NET_F_MAC_BIT | VIRTIO_NET_F_MRG_RXBUF_BIT?
> 
> Is the new feature bit something we want to unconditionally request, only
> request for PCI, or do we simply not care about it?

As of now, we do not care about VIRTIO_NET_F_MRG_RXBUF_BIT. I added it
because the num_buffers field in VirtioNetHdrMrgRxbuf is present when
either condition is true, and we might want to use it in the future.

>>   
>>   #define VQ_RX 0         /* Receive queue */
>>   #define VQ_TX 1         /* Transmit queue */
>> @@ -43,7 +45,18 @@ struct VirtioNetHdr {
>>   };
>>   typedef struct VirtioNetHdr VirtioNetHdr;
>>   
>> +struct VirtioNetHdrMrgRxbuf {
>> +    struct VirtioNetHdr hdr;
>> +    uint16_t num_buffers; /* Only with VIRTIO_NET_F_MRG_RXBUF or VIRTIO1 */
>> +};
>> +typedef struct VirtioNetHdrMrgRxbuf VirtioNetHdrMrgRxbuf;
>> +
>> +/* Header sizes for different modes */
>> +#define VIRTIO_NET_HDR_SIZE_LEGACY  sizeof(VirtioNetHdr)
>> +#define VIRTIO_NET_HDR_SIZE_V1      sizeof(VirtioNetHdrMrgRxbuf)
>> +
>>   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 +75,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))
> 
> Because we do not set VIRTIO_NET_F_MRG_RXBUF in net_init(), only the first
> half of this check can ever return true.
> 
> Maybe that is correct, but in that case we can simplify this to just
> vdev->guest_features[1] & VIRTIO_F_VERSION_1, right?
> 

Since VIRTIO_NET_F_MRG_RXBUF is neither negotiated nor used today, I can
remove it entirely and simplify the check to vdev->guest_features[1] &
VIRTIO_F_VERSION_1.

> Regards,
> Jared Rossi
>> +                          ? 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);
>> @@ -77,14 +95,14 @@ int virtio_net_init(void *mac_addr)
>>   
>>   int send(int fd, const void *buf, int len, int flags)
>>   {
>> -    VirtioNetHdr tx_hdr;
>> +    VirtioNetHdrMrgRxbuf tx_hdr;
>>       VDev *vdev = virtio_get_device();
>>       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));
>>   
>> -    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 +126,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;
> 

Re: [PATCH v2 2/7] pc-bios/s390-ccw: Add dynamic net header size handling

[Matthew Rosato]

On 9/3/26 12:24 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.
> 
Since you're already touching this patch based on Jared's comment, maybe
add a small blurb here that you are replicating the same structure
mappings as defined in the uapi.

Thanks,
Matt

Re: [PATCH v2 1/7] pc-bios/s390-ccw: Move CCW net setup to virtio-ccw

[Matthew Rosato]

On 9/3/26 12:24 PM, Zhuoying Cai wrote:
> Move CCW-specific virtio_setup() and find_net_dev() to virtio-ccw.c
> to decouple the generic net boot logic from the CCW transport.
> Also update the error message on failure to specify "ccw net device".
> 
> Signed-off-by: Zhuoying Cai <zycai@linux.ibm.com>

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

Re: [PATCH v2 1/7] pc-bios/s390-ccw: Move CCW net setup to virtio-ccw

[Jared Rossi]

On 9/3/26 12:24 PM, Zhuoying Cai wrote:
> Move CCW-specific virtio_setup() and find_net_dev() to virtio-ccw.c
> to decouple the generic net boot logic from the CCW transport.
> Also update the error message on failure to specify "ccw net device".
>
> Signed-off-by: Zhuoying Cai <zycai@linux.ibm.com>

Reviewed-by: Jared Rossi <jrossi@linux.ibm.com>

Re: [PATCH v2 5/7] hw/virtio: Add "loadparm" property to virtio net PCI devices booting on s390x

[Jared Rossi]

On 9/3/26 12:24 PM, Zhuoying Cai wrote:
> Add optional loadparm property to virtio-net-pci to allow boot entry
> selection on s390x, consistent with virtio-blk-pci.
>
> Signed-off-by: Zhuoying Cai <zycai@linux.ibm.com>
> ---

Reviewed-by: Jared Rossi <jrossi@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;
>   }
>   

Re: [PATCH v2 3/7] pc-bios/s390-ccw: Introduce virtio_tswap helpers

[Jared Rossi]

On 9/3/26 12:24 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.
>
> While only the 16-bit helper is used immediately, the 32-bit and 64-bit
> variants are added here for later use.
>
> Signed-off-by: Zhuoying Cai <zycai@linux.ibm.com>
> Reviewed-by: Jason J. Herne <jjherne@linux.ibm.com>
> Reviewed-by: Matthew Rosato <mjrosato@linux.ibm.com>
> ---

Reviewed-by: Jared Rossi <jrossi@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 aa307025e0..645dc288b1 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);

Re: [PATCH v2 6/7] tests/qtest: Add s390x virtio net PCI test to pxe-test.c

[Matthew Rosato]

On 9/3/26 12:24 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>
> Tested-by: Joshua Daley <jdaley@linux.ibm.com>

Realized I shared some findings off-list, but never included them here.

> ---
>  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 50a1a5272f..654e145fbe 100644
> --- a/tests/qtest/meson.build
> +++ b/tests/qtest/meson.build
> @@ -289,6 +289,7 @@ qtests_aarch64 = \
>     'migration-test']
>  
>  qtests_s390x = \
> +  (slirp.found() ? ['pxe-test'] : []) + \

AFAICT this patch not only adds pci testing, but re-enables pxe-test for
virtio-net-ccw.  Sounds like this got accidentally removed along the way?

If that's the case I think this should be its own fix patch.

Alternatively, at least mention it in the commit subject/message.

>    qtests_filter + \
>    ['boot-serial-test',
>     'drive_del-test',
> diff --git a/tests/qtest/pxe-test.c b/tests/qtest/pxe-test.c
> index e85dec5a4e..bfa3b98cad 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" },

I noticed that if I attempt to run this test using a bios that is not
built with this series included, the virtio-net-pci test does not fail
gracefully.

Joy investigated, turns out instead of detecting the condition (no
suitable device found) the tests simply get killed by timeout; would be
good to detect this condition for s390x if possible.

Thanks,
Matt

>      { NULL },
>  };
>