Re: [PATCH 2/5] pc-bios/s390-ccw: Add per-queue notification offset for multi-queue virtio configurations

[Eric Farman <farman@linux.ibm.com>]

On Mon, 2026-05-04 at 18:16 -0400, jrossi@linux.ibm.com wrote:
> From: Jared Rossi <jrossi@linux.ibm.com>
> 
> The initial support for virtio-blk-pci IPL devices used a single virt-queue, but
> other device types require multiple queues, and for PCI device types this also
> requires a per-queue notification offset.
> 
> Add a PCI notify field to the VRing struct so that each queue has a unique
> notify offset.  Also re-select the target queue before writing buffers to
> ensure the proper queue is active.

Why is this patch 2/5, when the series is n/6?

> 
> Signed-off-by: Jared Rossi <jrossi@linux.ibm.com>
> ---
>  pc-bios/s390-ccw/virtio-pci.c | 36 ++++++++++++++++++++++-------------
>  pc-bios/s390-ccw/virtio-pci.h |  3 ++-
>  pc-bios/s390-ccw/virtio.c     |  4 +++-
>  pc-bios/s390-ccw/virtio.h     |  1 +
>  4 files changed, 29 insertions(+), 15 deletions(-)
> 
> diff --git a/pc-bios/s390-ccw/virtio-pci.c b/pc-bios/s390-ccw/virtio-pci.c
> index 53bdb52e76..736869f4f5 100644
> --- a/pc-bios/s390-ccw/virtio-pci.c
> +++ b/pc-bios/s390-ccw/virtio-pci.c
> @@ -74,10 +74,10 @@ int virtio_pci_reset(VDev *vdev)
>      return 0;
>  }
>  
> -long virtio_pci_notify(int vq_id)
> +long virtio_pci_notify(VRing *vr)
>  {
> -    uint32_t offset = n_cap.off + notify_mult * q_notify_offset;
> -    return vpci_bswap16_write(offset, n_cap.bar, (uint16_t) vq_id);
> +    uint32_t offset = n_cap.off + notify_mult * vr->pci_notify;
> +    return vpci_bswap16_write(offset, n_cap.bar, (uint16_t) vr->id);

You no longer need the q_notify_offset variable nor the read into it.
(Ah, Joy already pointed this out; nice!)

>  }
>  
>  /*
> @@ -166,7 +166,7 @@ int vpci_read_flex(uint64_t offset, uint8_t pcias, void *buf, int len)
>      return 0;
>  }
>  
> -static int vpci_set_selected_vq(uint16_t queue_num)
> +int vpci_set_selected_vq(uint16_t queue_num)
>  {
>      return vpci_bswap16_write(c_cap.off + VPCI_C_OFFSET_Q_SELECT, c_cap.bar, queue_num);
>  }
> @@ -332,7 +332,6 @@ int virtio_pci_setup(VDev *vdev)
>      VRing *vr;
>      int rc;
>      uint8_t status;
> -    uint16_t vq_size;
>      int i = 0;
>  
>      vdev->guessed_disk_nature = VIRTIO_GDN_NONE;
> @@ -380,28 +379,39 @@ int virtio_pci_setup(VDev *vdev)
>          return -EIO;
>      }
>  
> -    if (vpci_read_bswap16(VPCI_C_OFFSET_Q_SIZE, c_cap.bar, &vq_size)) {
> -        puts("Failed to read virt-queue configuration");
> -        return -EIO;
> -    }
> -
>      /* Configure virt-queues for pci */
>      for (i = 0; i < vdev->nr_vqs; i++) {
> +        uint16_t vq_size;
> +        uint16_t vq_notify;
>          VqInfo info = {
>              .queue = (unsigned long long) virtio_get_ring_area(i),
>              .align = KVM_S390_VIRTIO_RING_ALIGN,
>              .index = i,
> -            .num = vq_size,
> +            .num = 0,
>          };
>  
>          vr = &vdev->vrings[i];
> -        vring_init(vr, &info);
>  
> -        if (vpci_set_selected_vq(vr->id)) {
> +        if (vpci_set_selected_vq(i)) {
>              puts("Failed to set selected virt-queue");
>              return -EIO;
>          }
>  
> +        if (vpci_read_bswap16(VPCI_C_OFFSET_Q_SIZE, c_cap.bar, &vq_size)) {
> +            puts("Failed to read virt-queue configuration");
> +            return -EIO;
> +        }
> +
> +        info.num = vq_size;
> +
> +        if (vpci_read_bswap16(c_cap.off + VPCI_C_OFFSET_Q_NOFF, c_cap.bar, &vq_notify)) {
> +            puts("Failed to read virt-queue configuration");
> +            return -EIO;
> +        }

For either of these error exits, do you need an identifier logged as to -which- virtqueue failed, as
you're in a loop?

> +
> +        vr->pci_notify = vq_notify;
> +        vring_init(vr, &info);
> +
>          rc = set_pci_vq_addr(VPCI_C_OFFSET_Q_DESCLO, vr->desc);
>          rc |= set_pci_vq_addr(VPCI_C_OFFSET_Q_AVAILLO, vr->avail);
>          rc |= set_pci_vq_addr(VPCI_C_OFFSET_Q_USEDLO, vr->used);
> diff --git a/pc-bios/s390-ccw/virtio-pci.h b/pc-bios/s390-ccw/virtio-pci.h
> index 90d07cb9a7..993fc285ac 100644
> --- a/pc-bios/s390-ccw/virtio-pci.h
> +++ b/pc-bios/s390-ccw/virtio-pci.h
> @@ -62,9 +62,10 @@ struct VirtioPciCap {
>  };
>  typedef struct VirtioPciCap  VirtioPciCap;
>  
> +int vpci_set_selected_vq(uint16_t queue_num);
>  void virtio_pci_id2type(VDev *vdev, uint16_t device_id);
>  int virtio_pci_reset(VDev *vdev);
> -long virtio_pci_notify(int vq_id);
> +long virtio_pci_notify(VRing *vr);
>  int virtio_pci_setup(VDev *vdev);
>  int virtio_pci_setup_device(void);
>  
> diff --git a/pc-bios/s390-ccw/virtio.c b/pc-bios/s390-ccw/virtio.c
> index 30e6b2bc16..00850acc2f 100644
> --- a/pc-bios/s390-ccw/virtio.c
> +++ b/pc-bios/s390-ccw/virtio.c
> @@ -114,7 +114,8 @@ bool vring_notify(VRing *vr)
>          vr->cookie = virtio_ccw_notify(vdev.schid, vr->id, vr->cookie);
>          break;
>      case S390_IPL_TYPE_PCI:
> -        vr->cookie = virtio_pci_notify(vr->id);
> +        vr->cookie = virtio_pci_notify(vr);
> +        break;
>      default:
>          return 1;
>      }
> @@ -154,6 +155,7 @@ static void vr_bswap_descriptor(VRingDesc *desc)
>  void vring_send_buf(VRing *vr, void *p, int len, int flags)
>  {
>      if (!be_ipl()) {
> +        vpci_set_selected_vq(vr->id);
>          vr->avail->idx = bswap16(vr->avail->idx);
>      }
>  
> diff --git a/pc-bios/s390-ccw/virtio.h b/pc-bios/s390-ccw/virtio.h
> index d32a4830ca..75ae5bdbc2 100644
> --- a/pc-bios/s390-ccw/virtio.h
> +++ b/pc-bios/s390-ccw/virtio.h
> @@ -107,6 +107,7 @@ struct VRing {
>      VRingUsed *used;
>      long cookie;
>      int id;
> +    uint16_t pci_notify;
>  };
>  typedef struct VRing VRing;
>