Re: [PATCH 07/10] pc-bios/s390-ccw: Add support for virtio-blk-pci IPL

2026-01-09 Thread Jared Rossi




On 1/9/26 6:32 AM, Thomas Huth wrote:

On 07/01/2026 19.32, Jared Rossi wrote:



On 1/7/26 9:44 AM, Thomas Huth wrote:

On 10/12/2025 21.54, [email protected] wrote:

From: Jared Rossi 

Enable virt-queue PCI configuration and add routines for 
virtio-blk-pci devices.


Signed-off-by: Jared Rossi 
---
  include/hw/s390x/ipl/qipl.h  |  10 +
  pc-bios/s390-ccw/virtio-pci.h    |  79 +++
  pc-bios/s390-ccw/virtio.h    |   1 +
  pc-bios/s390-ccw/main.c  |  60 +-
  pc-bios/s390-ccw/virtio-blkdev.c |   3 +
  pc-bios/s390-ccw/virtio-pci.c    | 360 
+++

  pc-bios/s390-ccw/virtio.c    |   5 +
  pc-bios/s390-ccw/Makefile    |   2 +-
  8 files changed, 517 insertions(+), 3 deletions(-)
  create mode 100644 pc-bios/s390-ccw/virtio-pci.h
  create mode 100644 pc-bios/s390-ccw/virtio-pci.c

diff --git a/include/hw/s390x/ipl/qipl.h b/include/hw/s390x/ipl/qipl.h
index 8199b839f0..5c7779a1c3 100644
--- a/include/hw/s390x/ipl/qipl.h
+++ b/include/hw/s390x/ipl/qipl.h
@@ -22,6 +22,7 @@
    #define S390_IPL_TYPE_FCP 0x00
  #define S390_IPL_TYPE_CCW 0x02
+#define S390_IPL_TYPE_PCI 0x04
  #define S390_IPL_TYPE_PV 0x05
  #define S390_IPL_TYPE_QEMU_SCSI 0xff
  @@ -105,6 +106,14 @@ struct IplBlockQemuScsi {
  } QEMU_PACKED;
  typedef struct IplBlockQemuScsi IplBlockQemuScsi;
  +struct IplBlockPci {
+    uint32_t reserved0[80];
+    uint8_t  opt;
+    uint8_t  reserved1[3];
+    uint32_t fid;
+} QEMU_PACKED;
+typedef struct IplBlockPci IplBlockPci;


Is this supposed to have the same positions for "fid" and "opt" as 
in the ipl_pb0_nvme structure in the kernel 
(arch/s390/include/uapi/asm/ipl.h)?


Yes, I think.  That was my intention anyway. Are you suggesting it 
shouldn't?


Having the same positions sound like a good idea. But if I'm counting 
right, it currently does not match:


In the code above, there are 80 * 4 = 320 bytes between the end of the 
loadparm[] array and the "opt" field.


In the kernel, there is the reserved2 field with 304 bytes between the 
end of the loadparm[] array and the "opt" field.


So either I'm counting wrong, or your reserved0 field should be 
decreased in size?


 Thomas


Ah, now I see.  Your are correct.  I will fix it.  Thanks for catching that!

Regards,
Jared Rossi



Re: [PATCH 07/10] pc-bios/s390-ccw: Add support for virtio-blk-pci IPL

2026-01-09 Thread Thomas Huth

On 07/01/2026 19.32, Jared Rossi wrote:



On 1/7/26 9:44 AM, Thomas Huth wrote:

On 10/12/2025 21.54, [email protected] wrote:

From: Jared Rossi 

Enable virt-queue PCI configuration and add routines for virtio-blk-pci 
devices.


Signed-off-by: Jared Rossi 
---
  include/hw/s390x/ipl/qipl.h  |  10 +
  pc-bios/s390-ccw/virtio-pci.h    |  79 +++
  pc-bios/s390-ccw/virtio.h    |   1 +
  pc-bios/s390-ccw/main.c  |  60 +-
  pc-bios/s390-ccw/virtio-blkdev.c |   3 +
  pc-bios/s390-ccw/virtio-pci.c    | 360 +++
  pc-bios/s390-ccw/virtio.c    |   5 +
  pc-bios/s390-ccw/Makefile    |   2 +-
  8 files changed, 517 insertions(+), 3 deletions(-)
  create mode 100644 pc-bios/s390-ccw/virtio-pci.h
  create mode 100644 pc-bios/s390-ccw/virtio-pci.c

diff --git a/include/hw/s390x/ipl/qipl.h b/include/hw/s390x/ipl/qipl.h
index 8199b839f0..5c7779a1c3 100644
--- a/include/hw/s390x/ipl/qipl.h
+++ b/include/hw/s390x/ipl/qipl.h
@@ -22,6 +22,7 @@
    #define S390_IPL_TYPE_FCP 0x00
  #define S390_IPL_TYPE_CCW 0x02
+#define S390_IPL_TYPE_PCI 0x04
  #define S390_IPL_TYPE_PV 0x05
  #define S390_IPL_TYPE_QEMU_SCSI 0xff
  @@ -105,6 +106,14 @@ struct IplBlockQemuScsi {
  } QEMU_PACKED;
  typedef struct IplBlockQemuScsi IplBlockQemuScsi;
  +struct IplBlockPci {
+    uint32_t reserved0[80];
+    uint8_t  opt;
+    uint8_t  reserved1[3];
+    uint32_t fid;
+} QEMU_PACKED;
+typedef struct IplBlockPci IplBlockPci;


Is this supposed to have the same positions for "fid" and "opt" as in the 
ipl_pb0_nvme structure in the kernel (arch/s390/include/uapi/asm/ipl.h)?


Yes, I think.  That was my intention anyway. Are you suggesting it shouldn't?


Having the same positions sound like a good idea. But if I'm counting right, 
it currently does not match:


In the code above, there are 80 * 4 = 320 bytes between the end of the 
loadparm[] array and the "opt" field.


In the kernel, there is the reserved2 field with 304 bytes between the end 
of the loadparm[] array and the "opt" field.


So either I'm counting wrong, or your reserved0 field should be decreased in 
size?


 Thomas




Re: [PATCH 07/10] pc-bios/s390-ccw: Add support for virtio-blk-pci IPL

2026-01-07 Thread Jared Rossi




On 1/7/26 3:39 PM, Zhuoying Cai wrote:

On 12/10/25 3:54 PM, [email protected] wrote:

+
+rc = pci_read_bswap32(vdev->pci_fh, pos + VPCI_N_CAP_MULT, PCI_CFGBAR, 
¬ify_mult);
+if (rc || pci_read_bswap16(vdev->pci_fh, d_cap.off + VPCI_C_OFFSET_Q_NOFF,
+   d_cap.bar, &q_notify_offset)) {

Hi Jared, I think this came up in the previous version -- should
q_notify_offset be taken from the common configuration structure?



Ah, you are correct.  I've missed fixing it.  My bad...  Thanks.

Regards,
Jared Rossi



Re: [PATCH 07/10] pc-bios/s390-ccw: Add support for virtio-blk-pci IPL

2026-01-07 Thread Zhuoying Cai
On 12/10/25 3:54 PM, [email protected] wrote:
> From: Jared Rossi 
> 
> Enable virt-queue PCI configuration and add routines for virtio-blk-pci 
> devices.
> 
> Signed-off-by: Jared Rossi 
> ---
>  include/hw/s390x/ipl/qipl.h  |  10 +
>  pc-bios/s390-ccw/virtio-pci.h|  79 +++
>  pc-bios/s390-ccw/virtio.h|   1 +
>  pc-bios/s390-ccw/main.c  |  60 +-
>  pc-bios/s390-ccw/virtio-blkdev.c |   3 +
>  pc-bios/s390-ccw/virtio-pci.c| 360 +++
>  pc-bios/s390-ccw/virtio.c|   5 +
>  pc-bios/s390-ccw/Makefile|   2 +-
>  8 files changed, 517 insertions(+), 3 deletions(-)
>  create mode 100644 pc-bios/s390-ccw/virtio-pci.h
>  create mode 100644 pc-bios/s390-ccw/virtio-pci.c
> 
> diff --git a/include/hw/s390x/ipl/qipl.h b/include/hw/s390x/ipl/qipl.h
> index 8199b839f0..5c7779a1c3 100644
> --- a/include/hw/s390x/ipl/qipl.h
> +++ b/include/hw/s390x/ipl/qipl.h
> @@ -22,6 +22,7 @@
>  
>  #define S390_IPL_TYPE_FCP 0x00
>  #define S390_IPL_TYPE_CCW 0x02
> +#define S390_IPL_TYPE_PCI 0x04
>  #define S390_IPL_TYPE_PV 0x05
>  #define S390_IPL_TYPE_QEMU_SCSI 0xff
>  
> @@ -105,6 +106,14 @@ struct IplBlockQemuScsi {
>  } QEMU_PACKED;
>  typedef struct IplBlockQemuScsi IplBlockQemuScsi;
>  
> +struct IplBlockPci {
> +uint32_t reserved0[80];
> +uint8_t  opt;
> +uint8_t  reserved1[3];
> +uint32_t fid;
> +} QEMU_PACKED;
> +typedef struct IplBlockPci IplBlockPci;
> +
>  union IplParameterBlock {
>  struct {
>  uint32_t len;
> @@ -120,6 +129,7 @@ union IplParameterBlock {
>  IplBlockFcp fcp;
>  IPLBlockPV pv;
>  IplBlockQemuScsi scsi;
> +IplBlockPci pci;
>  };
>  } QEMU_PACKED;
>  struct {
> diff --git a/pc-bios/s390-ccw/virtio-pci.h b/pc-bios/s390-ccw/virtio-pci.h
> new file mode 100644
> index 00..b203f37aea
> --- /dev/null
> +++ b/pc-bios/s390-ccw/virtio-pci.h
> @@ -0,0 +1,79 @@
> +/*
> + * Definitions for virtio-pci
> + *
> + * Copyright 2025 IBM Corp.
> + * Author(s): Jared Rossi 
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#ifndef VIRTIO_PCI_H
> +#define VIRTIO_PCI_H
> +
> +/* Common configuration */
> +#define VPCI_CAP_COMMON_CFG  1
> +/* Notifications */
> +#define VPCI_CAP_NOTIFY_CFG  2
> +/* ISR access */
> +#define VPCI_CAP_ISR_CFG 3
> +/* Device specific configuration */
> +#define VPCI_CAP_DEVICE_CFG  4
> +/* PCI configuration access */
> +#define VPCI_CAP_PCI_CFG 5
> +/* Additional shared memory capability */
> +#define VPCI_CAP_SHARED_MEMORY_CFG   8
> +/* PCI vendor data configuration */
> +#define VPCI_CAP_VENDOR_CFG  9
> +
> +/* Offsets within capability header */
> +#define VPCI_CAP_VNDR0
> +#define VPCI_CAP_NEXT1
> +#define VPCI_CAP_LEN 2
> +#define VPCI_CAP_CFG_TYPE3
> +#define VPCI_CAP_BAR 4
> +#define VPCI_CAP_OFFSET  8
> +#define VPCI_CAP_LENGTH  12
> +
> +#define VPCI_N_CAP_MULT 16 /* Notify multiplier, VPCI_CAP_NOTIFY_CFG only */
> +
> +/* Common Area Offsets for virtio-pci queue */
> +#define VPCI_C_OFFSET_DFSELECT  0
> +#define VPCI_C_OFFSET_DF4
> +#define VPCI_C_OFFSET_GFSELECT  8
> +#define VPCI_C_OFFSET_GF12
> +#define VPCI_C_COMMON_NUMQ  18
> +#define VPCI_C_OFFSET_STATUS20
> +#define VPCI_C_OFFSET_Q_SELECT  22
> +#define VPCI_C_OFFSET_Q_SIZE24
> +#define VPCI_C_OFFSET_Q_ENABLE  28
> +#define VPCI_C_OFFSET_Q_NOFF30
> +#define VPCI_C_OFFSET_Q_DESCLO  32
> +#define VPCI_C_OFFSET_Q_DESCHI  36
> +#define VPCI_C_OFFSET_Q_AVAILLO 40
> +#define VPCI_C_OFFSET_Q_AVAILHI 44
> +#define VPCI_C_OFFSET_Q_USEDLO  48
> +#define VPCI_C_OFFSET_Q_USEDHI  52
> +
> +#define VPCI_S_RESET0
> +#define VPCI_S_ACKNOWLEDGE  1
> +#define VPCI_S_DRIVER   2
> +#define VPCI_S_DRIVER_OK4
> +#define VPCI_S_FEATURES_OK  8
> +
> +#define VIRTIO_F_VERSION_1  (1 << (32 - 32)) /* Feature bit 32 */
> +
> +#define VIRT_Q_SIZE 16
> +
> +struct VirtioPciCap {
> +uint8_t bar; /* Which PCIAS it's in */
> +uint32_t off;/* Offset within bar */
> +};
> +typedef struct VirtioPciCap  VirtioPciCap;
> +
> +long virtio_pci_notify(uint32_t fhandle, int vq_id);
> +int virtio_pci_setup(VDev *vdev);
> +int virtio_pci_setup_device(void);
> +int virtio_pci_reset(VDev *vdev);
> +void virtio_pci_id2type(VDev *vdev, uint16_t device_id);
> +
> +#endif
> diff --git a/pc-bios/s390-ccw/virtio.h b/pc-bios/s390-ccw/virtio.h
> index e747891a2c..cc68c2f8a9 100644
> --- a/pc-bios/s390-ccw/virtio.h
> +++ b/pc-bios/s390-ccw/virtio.h
> @@ -256,6 +256,7 @@ struct VDev {
>  uint8_t scsi_dev_heads;
>  bool scsi_device_selected;
>  ScsiDevice selected_scsi_device;
> +uint32_t pci_fh;
>  uint32_t max_transfer;
>  uint32_t guest_features[2];
>  };
> diff --git a/pc-bios/s390-ccw/main.c b/pc-bios/s390-ccw/main.c
> 

Re: [PATCH 07/10] pc-bios/s390-ccw: Add support for virtio-blk-pci IPL

2026-01-07 Thread Jared Rossi




On 1/7/26 11:08 AM, Thomas Huth wrote:

On 07/01/2026 15.44, Thomas Huth wrote:

On 10/12/2025 21.54, [email protected] wrote:

From: Jared Rossi 

Enable virt-queue PCI configuration and add routines for 
virtio-blk-pci devices.


Signed-off-by: Jared Rossi 
---

...

+static int virtio_pci_set_gfeatures(VDev *vdev)
+{
+    int rc;
+
+    rc = pci_bswap32_write(vdev->pci_fh, c_cap.off + 
VPCI_C_OFFSET_GFSELECT,

+   c_cap.bar, 0);
+
+    rc |= pci_bswap32_write(vdev->pci_fh, c_cap.off + 
VPCI_C_OFFSET_GF,

+    c_cap.bar, vdev->guest_features[1]);
+
+    rc |= pci_bswap32_write(vdev->pci_fh, c_cap.off + 
VPCI_C_OFFSET_GFSELECT,

+    c_cap.bar, 1);
+
+    rc |= pci_bswap32_write(vdev->pci_fh, c_cap.off + 
VPCI_C_OFFSET_GF,

+    c_cap.bar, vdev->guest_features[0]);
+
+    if (rc) {
+    puts("Failed to set PCI feature bits");
+    return -EIO;
+    }
+
+    return 0;
+}
+
+static int virtio_pci_get_blk_config(VDev *vdev)
+{
+    return pci_read_flex(vdev->pci_fh, d_cap.off, d_cap.bar, &vdev- 
>config.blk,

+ sizeof(VirtioBlkConfig));


I'm a little bit surprised that there does not seem to be any 
endianess swapping for the config.blk data anywhere here ... isn't 
that config space data supposed to be in little endian?


... oh, wait, you're not negotiating VIRTIO_F_VERSION_1, are you? ... 
so the config space is still in big endian for legacy virtio? ... 
hmm, I guess it's ok for now, but in the long run, I think we should 
rather use VERSION_1 instead.
Thinking about this twice, could you please have a try to use 
VIRTIO_F_VERSION_1 right from the start? I already heard from some 
people that they'd rather want to get rid of legacy virtio in QEMU 
(e.g. for the universal binary project that includes multiple targets 
in a single QEMU binary), so I think it would be safer to immediately 
go with VERSION_1 here.


 Thanks,
  Thomas



Hi Thomas,

I touched on this in reply to your other message, but yes, I agree that 
it is better to do all of these negotiations in the most compatible way 
right from the start.  I will fix the feature negotiations and try to 
avoid using anything legacy.


Thanks again for your reviews,
Jared Rossi



Re: [PATCH 07/10] pc-bios/s390-ccw: Add support for virtio-blk-pci IPL

2026-01-07 Thread Jared Rossi




On 1/7/26 9:44 AM, Thomas Huth wrote:

On 10/12/2025 21.54, [email protected] wrote:

From: Jared Rossi 

Enable virt-queue PCI configuration and add routines for 
virtio-blk-pci devices.


Signed-off-by: Jared Rossi 
---
  include/hw/s390x/ipl/qipl.h  |  10 +
  pc-bios/s390-ccw/virtio-pci.h    |  79 +++
  pc-bios/s390-ccw/virtio.h    |   1 +
  pc-bios/s390-ccw/main.c  |  60 +-
  pc-bios/s390-ccw/virtio-blkdev.c |   3 +
  pc-bios/s390-ccw/virtio-pci.c    | 360 +++
  pc-bios/s390-ccw/virtio.c    |   5 +
  pc-bios/s390-ccw/Makefile    |   2 +-
  8 files changed, 517 insertions(+), 3 deletions(-)
  create mode 100644 pc-bios/s390-ccw/virtio-pci.h
  create mode 100644 pc-bios/s390-ccw/virtio-pci.c

diff --git a/include/hw/s390x/ipl/qipl.h b/include/hw/s390x/ipl/qipl.h
index 8199b839f0..5c7779a1c3 100644
--- a/include/hw/s390x/ipl/qipl.h
+++ b/include/hw/s390x/ipl/qipl.h
@@ -22,6 +22,7 @@
    #define S390_IPL_TYPE_FCP 0x00
  #define S390_IPL_TYPE_CCW 0x02
+#define S390_IPL_TYPE_PCI 0x04
  #define S390_IPL_TYPE_PV 0x05
  #define S390_IPL_TYPE_QEMU_SCSI 0xff
  @@ -105,6 +106,14 @@ struct IplBlockQemuScsi {
  } QEMU_PACKED;
  typedef struct IplBlockQemuScsi IplBlockQemuScsi;
  +struct IplBlockPci {
+    uint32_t reserved0[80];
+    uint8_t  opt;
+    uint8_t  reserved1[3];
+    uint32_t fid;
+} QEMU_PACKED;
+typedef struct IplBlockPci IplBlockPci;


Is this supposed to have the same positions for "fid" and "opt" as in 
the ipl_pb0_nvme structure in the kernel 
(arch/s390/include/uapi/asm/ipl.h)?


Yes, I think.  That was my intention anyway. Are you suggesting it 
shouldn't?





  union IplParameterBlock {
  struct {
  uint32_t len;
@@ -120,6 +129,7 @@ union IplParameterBlock {
  IplBlockFcp fcp;
  IPLBlockPV pv;
  IplBlockQemuScsi scsi;
+    IplBlockPci pci;
  };
  } QEMU_PACKED;
  struct {

...
diff --git a/pc-bios/s390-ccw/virtio-pci.c 
b/pc-bios/s390-ccw/virtio-pci.c

new file mode 100644
index 00..838231d86c
--- /dev/null
+++ b/pc-bios/s390-ccw/virtio-pci.c
@@ -0,0 +1,360 @@
+/*
+ * Functionality for virtio-pci
+ *
+ * Copyright 2025 IBM Corp.
+ * Author(s): Jared Rossi 
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "clp.h"
+#include "pci.h"
+#include "helper.h"
+#include "s390-ccw.h"
+#include "virtio.h"
+#include "bswap.h"
+#include "virtio-pci.h"
+#include "s390-time.h"
+#include 
+
+/* Variable offsets used for reads/writes to modern memory regions */
+VirtioPciCap c_cap; /* Common capabilities  */
+VirtioPciCap d_cap; /* Device capabilities  */
+VirtioPciCap n_cap; /* Notify capabilities  */
+uint32_t notify_mult;
+uint16_t q_notify_offset;
+
+static int virtio_pci_set_status(VDev *vdev, uint8_t status)
+{
+    int rc = pci_write_byte(vdev->pci_fh, c_cap.off + 
VPCI_C_OFFSET_STATUS,

+ c_cap.bar, status);
+    if (rc) {
+    puts("Failed to write virtio-pci status");
+    return -EIO;
+    }
+
+    return 0;
+}
+
+static int virtio_pci_get_status(VDev *vdev, uint8_t *status)
+{
+    int rc = pci_read_byte(vdev->pci_fh, c_cap.off + 
VPCI_C_OFFSET_STATUS,

+   c_cap.bar, status);
+    if (rc) {
+    puts("Failed to read virtio-pci status");
+    return -EIO;
+    }
+
+    return 0;
+}
+
+/*
+ * Find the position of the capability config within PCI configuration
+ * space for a given cfg type.  Return the position if found, 
otherwise 0.

+ */
+static uint8_t find_cap_pos(uint32_t fhandle, uint8_t cfg_type)
+{
+    uint8_t next, cfg;
+    int rc;
+
+    rc = pci_read_byte(fhandle, PCI_CAPABILITY_LIST, PCI_CFGBAR, 
&next);

+    rc |= pci_read_byte(fhandle, next + 3, PCI_CFGBAR, &cfg);
+
+    while (!rc && (cfg != cfg_type) && next) {
+    rc = pci_read_byte(fhandle, next + 1, PCI_CFGBAR, &next);
+    rc |= pci_read_byte(fhandle, next + 3, PCI_CFGBAR, &cfg);
+    }
+
+    return rc ? 0 : next;
+}
+
+static int virtio_pci_get_hfeatures(VDev *vdev, uint64_t *features)
+{
+    uint32_t feat0, feat1;
+    int rc;
+
+    rc = pci_bswap32_write(vdev->pci_fh, c_cap.off + 
VPCI_C_OFFSET_DFSELECT,

+   c_cap.bar, 0);
+
+    rc |= pci_read_bswap32(vdev->pci_fh, c_cap.off + VPCI_C_OFFSET_DF,
+   c_cap.bar, &feat0);
+
+    rc |= pci_bswap32_write(vdev->pci_fh, c_cap.off + 
VPCI_C_OFFSET_DFSELECT,

+    c_cap.bar, 1);
+
+    rc |= pci_read_bswap32(vdev->pci_fh, c_cap.off + VPCI_C_OFFSET_DF,
+   c_cap.bar, &feat1);
+
+    if (rc) {
+    puts("Failed to get PCI feature bits");
+    return -EIO;
+    }
+
+    *features = (uint64_t) feat1 << 32;
+    *features |= (uint64_t) feat0;
+
+    return 0;
+}
+
+static int virtio_pci_set_gfeatures(VDev *vdev)
+{
+    int rc;
+
+    rc = pci_bswap32_write(vdev->pci_fh, c_cap.off + 
VPCI_C_OFFSET_GFSELECT,

+   c_ca

Re: [PATCH 07/10] pc-bios/s390-ccw: Add support for virtio-blk-pci IPL

2026-01-07 Thread Thomas Huth

On 07/01/2026 15.44, Thomas Huth wrote:

On 10/12/2025 21.54, [email protected] wrote:

From: Jared Rossi 

Enable virt-queue PCI configuration and add routines for virtio-blk-pci 
devices.


Signed-off-by: Jared Rossi 
---

...

+static int virtio_pci_set_gfeatures(VDev *vdev)
+{
+    int rc;
+
+    rc = pci_bswap32_write(vdev->pci_fh, c_cap.off + VPCI_C_OFFSET_GFSELECT,
+   c_cap.bar, 0);
+
+    rc |= pci_bswap32_write(vdev->pci_fh, c_cap.off + VPCI_C_OFFSET_GF,
+    c_cap.bar, vdev->guest_features[1]);
+
+    rc |= pci_bswap32_write(vdev->pci_fh, c_cap.off + 
VPCI_C_OFFSET_GFSELECT,

+    c_cap.bar, 1);
+
+    rc |= pci_bswap32_write(vdev->pci_fh, c_cap.off + VPCI_C_OFFSET_GF,
+    c_cap.bar, vdev->guest_features[0]);
+
+    if (rc) {
+    puts("Failed to set PCI feature bits");
+    return -EIO;
+    }
+
+    return 0;
+}
+
+static int virtio_pci_get_blk_config(VDev *vdev)
+{
+    return pci_read_flex(vdev->pci_fh, d_cap.off, d_cap.bar, &vdev- 
>config.blk,

+ sizeof(VirtioBlkConfig));


I'm a little bit surprised that there does not seem to be any endianess 
swapping for the config.blk data anywhere here ... isn't that config space 
data supposed to be in little endian?


... oh, wait, you're not negotiating VIRTIO_F_VERSION_1, are you? ... so the 
config space is still in big endian for legacy virtio? ... hmm, I guess it's 
ok for now, but in the long run, I think we should rather use VERSION_1 
instead.
Thinking about this twice, could you please have a try to use 
VIRTIO_F_VERSION_1 right from the start? I already heard from some people 
that they'd rather want to get rid of legacy virtio in QEMU (e.g. for the 
universal binary project that includes multiple targets in a single QEMU 
binary), so I think it would be safer to immediately go with VERSION_1 here.


 Thanks,
  Thomas




Re: [PATCH 07/10] pc-bios/s390-ccw: Add support for virtio-blk-pci IPL

2026-01-07 Thread Thomas Huth

On 10/12/2025 21.54, [email protected] wrote:

From: Jared Rossi 

Enable virt-queue PCI configuration and add routines for virtio-blk-pci devices.

Signed-off-by: Jared Rossi 
---
  include/hw/s390x/ipl/qipl.h  |  10 +
  pc-bios/s390-ccw/virtio-pci.h|  79 +++
  pc-bios/s390-ccw/virtio.h|   1 +
  pc-bios/s390-ccw/main.c  |  60 +-
  pc-bios/s390-ccw/virtio-blkdev.c |   3 +
  pc-bios/s390-ccw/virtio-pci.c| 360 +++
  pc-bios/s390-ccw/virtio.c|   5 +
  pc-bios/s390-ccw/Makefile|   2 +-
  8 files changed, 517 insertions(+), 3 deletions(-)
  create mode 100644 pc-bios/s390-ccw/virtio-pci.h
  create mode 100644 pc-bios/s390-ccw/virtio-pci.c

diff --git a/include/hw/s390x/ipl/qipl.h b/include/hw/s390x/ipl/qipl.h
index 8199b839f0..5c7779a1c3 100644
--- a/include/hw/s390x/ipl/qipl.h
+++ b/include/hw/s390x/ipl/qipl.h
@@ -22,6 +22,7 @@
  
  #define S390_IPL_TYPE_FCP 0x00

  #define S390_IPL_TYPE_CCW 0x02
+#define S390_IPL_TYPE_PCI 0x04
  #define S390_IPL_TYPE_PV 0x05
  #define S390_IPL_TYPE_QEMU_SCSI 0xff
  
@@ -105,6 +106,14 @@ struct IplBlockQemuScsi {

  } QEMU_PACKED;
  typedef struct IplBlockQemuScsi IplBlockQemuScsi;
  
+struct IplBlockPci {

+uint32_t reserved0[80];
+uint8_t  opt;
+uint8_t  reserved1[3];
+uint32_t fid;
+} QEMU_PACKED;
+typedef struct IplBlockPci IplBlockPci;


Is this supposed to have the same positions for "fid" and "opt" as in the 
ipl_pb0_nvme structure in the kernel (arch/s390/include/uapi/asm/ipl.h)?



  union IplParameterBlock {
  struct {
  uint32_t len;
@@ -120,6 +129,7 @@ union IplParameterBlock {
  IplBlockFcp fcp;
  IPLBlockPV pv;
  IplBlockQemuScsi scsi;
+IplBlockPci pci;
  };
  } QEMU_PACKED;
  struct {

...

diff --git a/pc-bios/s390-ccw/virtio-pci.c b/pc-bios/s390-ccw/virtio-pci.c
new file mode 100644
index 00..838231d86c
--- /dev/null
+++ b/pc-bios/s390-ccw/virtio-pci.c
@@ -0,0 +1,360 @@
+/*
+ * Functionality for virtio-pci
+ *
+ * Copyright 2025 IBM Corp.
+ * Author(s): Jared Rossi 
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "clp.h"
+#include "pci.h"
+#include "helper.h"
+#include "s390-ccw.h"
+#include "virtio.h"
+#include "bswap.h"
+#include "virtio-pci.h"
+#include "s390-time.h"
+#include 
+
+/* Variable offsets used for reads/writes to modern memory regions */
+VirtioPciCap c_cap; /* Common capabilities  */
+VirtioPciCap d_cap; /* Device capabilities  */
+VirtioPciCap n_cap; /* Notify capabilities  */
+uint32_t notify_mult;
+uint16_t q_notify_offset;
+
+static int virtio_pci_set_status(VDev *vdev, uint8_t status)
+{
+int rc = pci_write_byte(vdev->pci_fh, c_cap.off + VPCI_C_OFFSET_STATUS,
+ c_cap.bar, status);
+if (rc) {
+puts("Failed to write virtio-pci status");
+return -EIO;
+}
+
+return 0;
+}
+
+static int virtio_pci_get_status(VDev *vdev, uint8_t *status)
+{
+int rc = pci_read_byte(vdev->pci_fh, c_cap.off + VPCI_C_OFFSET_STATUS,
+   c_cap.bar, status);
+if (rc) {
+puts("Failed to read virtio-pci status");
+return -EIO;
+}
+
+return 0;
+}
+
+/*
+ * Find the position of the capability config within PCI configuration
+ * space for a given cfg type.  Return the position if found, otherwise 0.
+ */
+static uint8_t find_cap_pos(uint32_t fhandle, uint8_t cfg_type)
+{
+uint8_t next, cfg;
+int rc;
+
+rc = pci_read_byte(fhandle, PCI_CAPABILITY_LIST, PCI_CFGBAR, &next);
+rc |= pci_read_byte(fhandle, next + 3, PCI_CFGBAR, &cfg);
+
+while (!rc && (cfg != cfg_type) && next) {
+rc = pci_read_byte(fhandle, next + 1, PCI_CFGBAR, &next);
+rc |= pci_read_byte(fhandle, next + 3, PCI_CFGBAR, &cfg);
+}
+
+return rc ? 0 : next;
+}
+
+static int virtio_pci_get_hfeatures(VDev *vdev, uint64_t *features)
+{
+uint32_t feat0, feat1;
+int rc;
+
+rc = pci_bswap32_write(vdev->pci_fh, c_cap.off + VPCI_C_OFFSET_DFSELECT,
+   c_cap.bar, 0);
+
+rc |= pci_read_bswap32(vdev->pci_fh, c_cap.off + VPCI_C_OFFSET_DF,
+   c_cap.bar, &feat0);
+
+rc |= pci_bswap32_write(vdev->pci_fh, c_cap.off + VPCI_C_OFFSET_DFSELECT,
+c_cap.bar, 1);
+
+rc |= pci_read_bswap32(vdev->pci_fh, c_cap.off + VPCI_C_OFFSET_DF,
+   c_cap.bar, &feat1);
+
+if (rc) {
+puts("Failed to get PCI feature bits");
+return -EIO;
+}
+
+*features = (uint64_t) feat1 << 32;
+*features |= (uint64_t) feat0;
+
+return 0;
+}
+
+static int virtio_pci_set_gfeatures(VDev *vdev)
+{
+int rc;
+
+rc = pci_bswap32_write(vdev->pci_fh, c_cap.off + VPCI_C_OFFSET_GFSELECT,
+   c_cap.bar, 0);
+
+rc |= pci_bswap32_write(vdev->pci_fh, c_cap.off + VPCI_C_OFFSET_GF,
+c_cap.bar, vdev->guest

[PATCH 07/10] pc-bios/s390-ccw: Add support for virtio-blk-pci IPL

2025-12-10 Thread jrossi
From: Jared Rossi 

Enable virt-queue PCI configuration and add routines for virtio-blk-pci devices.

Signed-off-by: Jared Rossi 
---
 include/hw/s390x/ipl/qipl.h  |  10 +
 pc-bios/s390-ccw/virtio-pci.h|  79 +++
 pc-bios/s390-ccw/virtio.h|   1 +
 pc-bios/s390-ccw/main.c  |  60 +-
 pc-bios/s390-ccw/virtio-blkdev.c |   3 +
 pc-bios/s390-ccw/virtio-pci.c| 360 +++
 pc-bios/s390-ccw/virtio.c|   5 +
 pc-bios/s390-ccw/Makefile|   2 +-
 8 files changed, 517 insertions(+), 3 deletions(-)
 create mode 100644 pc-bios/s390-ccw/virtio-pci.h
 create mode 100644 pc-bios/s390-ccw/virtio-pci.c

diff --git a/include/hw/s390x/ipl/qipl.h b/include/hw/s390x/ipl/qipl.h
index 8199b839f0..5c7779a1c3 100644
--- a/include/hw/s390x/ipl/qipl.h
+++ b/include/hw/s390x/ipl/qipl.h
@@ -22,6 +22,7 @@
 
 #define S390_IPL_TYPE_FCP 0x00
 #define S390_IPL_TYPE_CCW 0x02
+#define S390_IPL_TYPE_PCI 0x04
 #define S390_IPL_TYPE_PV 0x05
 #define S390_IPL_TYPE_QEMU_SCSI 0xff
 
@@ -105,6 +106,14 @@ struct IplBlockQemuScsi {
 } QEMU_PACKED;
 typedef struct IplBlockQemuScsi IplBlockQemuScsi;
 
+struct IplBlockPci {
+uint32_t reserved0[80];
+uint8_t  opt;
+uint8_t  reserved1[3];
+uint32_t fid;
+} QEMU_PACKED;
+typedef struct IplBlockPci IplBlockPci;
+
 union IplParameterBlock {
 struct {
 uint32_t len;
@@ -120,6 +129,7 @@ union IplParameterBlock {
 IplBlockFcp fcp;
 IPLBlockPV pv;
 IplBlockQemuScsi scsi;
+IplBlockPci pci;
 };
 } QEMU_PACKED;
 struct {
diff --git a/pc-bios/s390-ccw/virtio-pci.h b/pc-bios/s390-ccw/virtio-pci.h
new file mode 100644
index 00..b203f37aea
--- /dev/null
+++ b/pc-bios/s390-ccw/virtio-pci.h
@@ -0,0 +1,79 @@
+/*
+ * Definitions for virtio-pci
+ *
+ * Copyright 2025 IBM Corp.
+ * Author(s): Jared Rossi 
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#ifndef VIRTIO_PCI_H
+#define VIRTIO_PCI_H
+
+/* Common configuration */
+#define VPCI_CAP_COMMON_CFG  1
+/* Notifications */
+#define VPCI_CAP_NOTIFY_CFG  2
+/* ISR access */
+#define VPCI_CAP_ISR_CFG 3
+/* Device specific configuration */
+#define VPCI_CAP_DEVICE_CFG  4
+/* PCI configuration access */
+#define VPCI_CAP_PCI_CFG 5
+/* Additional shared memory capability */
+#define VPCI_CAP_SHARED_MEMORY_CFG   8
+/* PCI vendor data configuration */
+#define VPCI_CAP_VENDOR_CFG  9
+
+/* Offsets within capability header */
+#define VPCI_CAP_VNDR0
+#define VPCI_CAP_NEXT1
+#define VPCI_CAP_LEN 2
+#define VPCI_CAP_CFG_TYPE3
+#define VPCI_CAP_BAR 4
+#define VPCI_CAP_OFFSET  8
+#define VPCI_CAP_LENGTH  12
+
+#define VPCI_N_CAP_MULT 16 /* Notify multiplier, VPCI_CAP_NOTIFY_CFG only */
+
+/* Common Area Offsets for virtio-pci queue */
+#define VPCI_C_OFFSET_DFSELECT  0
+#define VPCI_C_OFFSET_DF4
+#define VPCI_C_OFFSET_GFSELECT  8
+#define VPCI_C_OFFSET_GF12
+#define VPCI_C_COMMON_NUMQ  18
+#define VPCI_C_OFFSET_STATUS20
+#define VPCI_C_OFFSET_Q_SELECT  22
+#define VPCI_C_OFFSET_Q_SIZE24
+#define VPCI_C_OFFSET_Q_ENABLE  28
+#define VPCI_C_OFFSET_Q_NOFF30
+#define VPCI_C_OFFSET_Q_DESCLO  32
+#define VPCI_C_OFFSET_Q_DESCHI  36
+#define VPCI_C_OFFSET_Q_AVAILLO 40
+#define VPCI_C_OFFSET_Q_AVAILHI 44
+#define VPCI_C_OFFSET_Q_USEDLO  48
+#define VPCI_C_OFFSET_Q_USEDHI  52
+
+#define VPCI_S_RESET0
+#define VPCI_S_ACKNOWLEDGE  1
+#define VPCI_S_DRIVER   2
+#define VPCI_S_DRIVER_OK4
+#define VPCI_S_FEATURES_OK  8
+
+#define VIRTIO_F_VERSION_1  (1 << (32 - 32)) /* Feature bit 32 */
+
+#define VIRT_Q_SIZE 16
+
+struct VirtioPciCap {
+uint8_t bar; /* Which PCIAS it's in */
+uint32_t off;/* Offset within bar */
+};
+typedef struct VirtioPciCap  VirtioPciCap;
+
+long virtio_pci_notify(uint32_t fhandle, int vq_id);
+int virtio_pci_setup(VDev *vdev);
+int virtio_pci_setup_device(void);
+int virtio_pci_reset(VDev *vdev);
+void virtio_pci_id2type(VDev *vdev, uint16_t device_id);
+
+#endif
diff --git a/pc-bios/s390-ccw/virtio.h b/pc-bios/s390-ccw/virtio.h
index e747891a2c..cc68c2f8a9 100644
--- a/pc-bios/s390-ccw/virtio.h
+++ b/pc-bios/s390-ccw/virtio.h
@@ -256,6 +256,7 @@ struct VDev {
 uint8_t scsi_dev_heads;
 bool scsi_device_selected;
 ScsiDevice selected_scsi_device;
+uint32_t pci_fh;
 uint32_t max_transfer;
 uint32_t guest_features[2];
 };
diff --git a/pc-bios/s390-ccw/main.c b/pc-bios/s390-ccw/main.c
index e82d60bbb7..562eeede63 100644
--- a/pc-bios/s390-ccw/main.c
+++ b/pc-bios/s390-ccw/main.c
@@ -15,7 +15,9 @@
 #include "s390-arch.h"
 #include "s390-ccw.h"
 #include "cio.h"
+#include "clp.h"
 #include "virtio.h"
+#include "virtio-pci.h"
 #include "virtio-scsi.h"
 #include "dasd-ipl.h"
 
@@ -151,6 +153,21 @@ static bool find_subch(int dev_no)
 r