On 5/6/26 2:20 PM, Zhuoying Cai wrote:
On 5/4/26 6:16 PM, [email protected] wrote:From: Jared Rossi <[email protected]> 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. Signed-off-by: Jared Rossi <[email protected]> --- 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); }/*@@ -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)) {Small nit: c_cap.off + VPCI_C_OFFSET_Q_SIZE
Ah, this is actually not just a small nit! Although the common capabilitiesoffset is 0 in the current QEMU implementation there is nothing preventing it from being a different offset in the future. So it must be included, otherwise
a non-zero value would break it. Good catch.
+ 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; + }Since the notification offset is now read per queue, should we remove the same read in virtio_pci_read_pci_cap_config() as well as the global q_notify_offset variable? if (rc || vpci_read_bswap16(c_cap.off + VPCI_C_OFFSET_Q_NOFF, c_cap.bar, &q_notify_offset))
Yes, that should be removed. I will take care of it for V2. Thanks.
+ + 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.cindex 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.hindex 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;
Regards, Jared Rossi
