In virtblk_add_req(), each request consumes 2 extra descriptors (out_hdr and in_hdr) in addition to the data scatter-gather segments.
When a hypervisor (e.g. QNX Hypervisor) advertises VIRTIO_BLK_F_SEG_MAX with seg_max = 1024 alongside a 1024-entry split ring (vring.num = 1024) and VIRTIO_RING_F_INDIRECT_DESC disabled, lim->max_segments is set to 1024. When the block layer submits requests with 1023 or 1024 data segments, total_sg reaches 1025 or 1026. This exceeds vring.num (1024), causing virtqueue_add_split() to return -ENOSPC and permanently wedge the blk-mq queue. Furthermore, the Virtio specification (2.7.5.3.1) requires that a descriptor chain never exceed the Queue Size, and virtqueue_add_split() falls back to direct descriptors if indirect table allocation fails. Fix this by: 1. Rejecting queues with ring_size < 3 at probe, or ring_size < (queue_max_segments + 2) during resume/reset recovery in init_vq(). 2. Unconditionally clamping sg_elems to (ring_size - 2) across all virtqueues in virtblk_read_limits(). Signed-off-by: Sergii Ushakov <[email protected]> --- Hi Michael, Thank you for the detailed review! I logged the values in virtblk_read_limits() on the QNX Hypervisor guest: virtio_blk virtio0: 1/0/0 default/read/poll queues virtio_blk virtio0: F_SEG_MAX=1 seg_max=1024 F_INDIRECT=0 vring_size=1024 max_segments=1024 dev_info(&vdev->dev, "virtio_blk debug: F_SEG_MAX=%d seg_max=%u F_INDIRECT=%d vring_size=%u max_segments=%u\n", virtio_has_feature(vdev, VIRTIO_BLK_F_SEG_MAX), sg_elems, virtio_has_feature(vdev, VIRTIO_RING_F_INDIRECT_DESC), virtqueue_get_vring_size(vblk->vqs[0].vq), lim->max_segments); So what actually happens: 1. The host *does* advertise VIRTIO_BLK_F_SEG_MAX=1 with seg_max=1024 (setting seg_max equal to the virtqueue size vring_size=1024), and does *not* offer VIRTIO_RING_F_INDIRECT_DESC (F_INDIRECT=0). 2. Because seg_max (1024) <= VIRTIO_BLK_MAX_SG_ELEMS - 2 (32766), virtblk_read_limits() sets lim->max_segments = 1024. 3. The block layer submits requests with 1023 or 1024 data segments. virtblk_add_req() adds 2 extra descriptors (out_hdr and in_hdr), resulting in total_sg > 1024. 4. Since total_sg > vq->split.vring.num and !vq->indirect, virtqueue_add_split() triggers: WARN_ON_ONCE(total_sg > vq->split.vring.num && !vq->indirect); In v3, I drop the !virtio_has_feature(vdev, VIRTIO_RING_F_INDIRECT_DESC) condition and clamp sg_elems to (ring_size - 2) unconditionally across all virtqueues. Also, init_vq() checks all virtqueues: - At initial probe (!vblk->disk), it enforces ring_size >= 3. - On resume/recovery, it enforces ring_size >= queue_max_segments(vblk->disk->queue) + 2. Logs after the v3 patch: virtio_blk virtio0: F_SEG_MAX=1 seg_max=1022 F_INDIRECT=0 vring_size=1024 max_segments=1022 drivers/block/virtio_blk.c | 35 ++++++++++++++++++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-) diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c index 32bf3ba07a9d..c263d0e3db95 100644 --- a/drivers/block/virtio_blk.c +++ b/drivers/block/virtio_blk.c @@ -1021,6 +1021,21 @@ static int init_vq(struct virtio_blk *vblk) goto out; for (i = 0; i < num_vqs; i++) { + unsigned int ring_size = virtqueue_get_vring_size(vqs[i]); + unsigned int min_ring_size = 3; + + if (vblk->disk) + min_ring_size = queue_max_segments(vblk->disk->queue) + 2; + + if (ring_size < min_ring_size) { + dev_err(&vdev->dev, + "virtqueue %u ring size %u is smaller than minimum %u\n", + i, ring_size, min_ring_size); + vdev->config->del_vqs(vdev); + err = -EINVAL; + goto out; + } + spin_lock_init(&vblk->vqs[i].lock); vblk->vqs[i].vq = vqs[i]; } @@ -1253,7 +1268,7 @@ static int virtblk_read_limits(struct virtio_blk *vblk, u16 min_io_size; u8 physical_block_exp, alignment_offset; size_t max_dma_size; - int err; + int err, i; /* We need to know how many segments before we allocate. */ err = virtio_cread_feature(vdev, VIRTIO_BLK_F_SEG_MAX, @@ -1267,6 +1282,23 @@ static int virtblk_read_limits(struct virtio_blk *vblk, /* Prevent integer overflows and honor max vq size */ sg_elems = min_t(u32, sg_elems, VIRTIO_BLK_MAX_SG_ELEMS - 2); + /* + * virtblk_add_req() uses separate outgoing and incoming header + * descriptors (out_hdr and in_hdr), consuming 2 extra descriptors + * per request in addition to the data segments. + * + * Per virtio specification (2.7.5.3.1), a driver MUST NOT create a + * descriptor chain longer than the Queue Size of the device. + * + * Clamp max_segments to (ring_size - 2) across all virtqueues so + * that a request never exceeds the ring size of any queue. + */ + for (i = 0; i < vblk->num_vqs; i++) { + u32 ring_size = virtqueue_get_vring_size(vblk->vqs[i].vq); + + sg_elems = min_t(u32, sg_elems, ring_size - 2); + } + /* We can handle whatever the host told us to handle. */ lim->max_segments = sg_elems; @@ -1466,6 +1498,7 @@ static int virtblk_probe(struct virtio_device *vdev) mutex_init(&vblk->vdev_mutex); vblk->vdev = vdev; + vblk->disk = NULL; INIT_WORK(&vblk->config_work, virtblk_config_changed_work); -- 2.55.0.1082.g2b9226bbc0-goog
