All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] vhost-user-blk: add seg-max-adjust flag
@ 2026-04-01 23:05 Sergei Heifetz
  2026-04-15 15:24 ` Raphael Norwitz
  0 siblings, 1 reply; 5+ messages in thread
From: Sergei Heifetz @ 2026-04-01 23:05 UTC (permalink / raw)
  To: qemu-devel
  Cc: qemu-block, Michael S. Tsirkin, Kevin Wolf, Hanna Reitz,
	Stefano Garzarella, Raphael Norwitz, Sergei Heifetz

The virtio specification is not completely clear about seg_max and its
relationship with queue_size. Some drivers (for example, the modern
Linux kernel driver) rely on seg_max to set the maximum number of
segments used in a request. If seg_max is set larger than queue_size,
such a driver might overwhelm a virtqueue by trying to send more
segments than it can handle. As a result, it either hangs or faults.

One might argue that it is the vhost-user server's responsibility to set
a valid seg_max value. However, due to the issue described in the
previous paragraph, this value should generally depend on queue_size.
That's why it may be necessary to control it on QEMU's side.

This patch adds the seg-max-adjust flag. A flag with the same name
already exists for virtio-blk (and it exists to solve the same problem,
except that here we have a vhost-user server to consult).

If seg-max-adjust is set, the final seg_max is the minimum of the value
provided by the vhost-user server and (queue_size - 2). It is not
enabled by default.

Signed-off-by: Sergei Heifetz <heifetz@yandex-team.com>
---
 hw/block/vhost-user-blk.c          | 11 +++++++++++
 include/hw/virtio/vhost-user-blk.h |  1 +
 2 files changed, 12 insertions(+)

diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
index c151e836770..23f910d9fe3 100644
--- a/hw/block/vhost-user-blk.c
+++ b/hw/block/vhost-user-blk.c
@@ -65,6 +65,11 @@ static void vhost_user_blk_update_config(VirtIODevice *vdev, uint8_t *config)
     /* Our num_queues overrides the device backend */
     virtio_stw_p(vdev, &s->blkcfg.num_queues, s->num_queues);
 
+    if (s->seg_max_adjust) {
+        uint32_t seg_max = MIN(s->blkcfg.seg_max, s->queue_size - 2);
+        virtio_stl_p(vdev, &s->blkcfg.seg_max, seg_max);
+    }
+
     memcpy(config, &s->blkcfg, vdev->config_len);
 }
 
@@ -474,6 +479,10 @@ static void vhost_user_blk_device_realize(DeviceState *dev, Error **errp)
         error_setg(errp, "queue size must be non-zero");
         return;
     }
+    if (s->queue_size < 4 && s->seg_max_adjust) {
+        error_setg(errp, "queue size must be >= 4 when seg-max-adjust is set");
+        return;
+    }
     if (s->queue_size > VIRTQUEUE_MAX_SIZE) {
         error_setg(errp, "queue size must not exceed %d",
                    VIRTQUEUE_MAX_SIZE);
@@ -608,6 +617,8 @@ static const Property vhost_user_blk_properties[] = {
     DEFINE_PROP_UINT16("num-queues", VHostUserBlk, num_queues,
                        VHOST_USER_BLK_AUTO_NUM_QUEUES),
     DEFINE_PROP_UINT32("queue-size", VHostUserBlk, queue_size, 128),
+    DEFINE_PROP_BOOL("seg-max-adjust", VHostUserBlk, seg_max_adjust,
+                      false),
     DEFINE_PROP_BIT64("config-wce", VHostUserBlk, parent_obj.host_features,
                       VIRTIO_BLK_F_CONFIG_WCE, true),
     DEFINE_PROP_BIT64("discard", VHostUserBlk, parent_obj.host_features,
diff --git a/include/hw/virtio/vhost-user-blk.h b/include/hw/virtio/vhost-user-blk.h
index 1e41a2bcdf6..dee848cfd81 100644
--- a/include/hw/virtio/vhost-user-blk.h
+++ b/include/hw/virtio/vhost-user-blk.h
@@ -34,6 +34,7 @@ struct VHostUserBlk {
     struct virtio_blk_config blkcfg;
     uint16_t num_queues;
     uint32_t queue_size;
+    bool seg_max_adjust;
     struct vhost_dev dev;
     struct vhost_inflight *inflight;
     VhostUserState vhost_user;
-- 
2.53.0



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] vhost-user-blk: add seg-max-adjust flag
  2026-04-01 23:05 [PATCH] vhost-user-blk: add seg-max-adjust flag Sergei Heifetz
@ 2026-04-15 15:24 ` Raphael Norwitz
  2026-06-15 17:19   ` Vladimir Sementsov-Ogievskiy
  2026-07-04  9:17   ` Vladimir Sementsov-Ogievskiy
  0 siblings, 2 replies; 5+ messages in thread
From: Raphael Norwitz @ 2026-04-15 15:24 UTC (permalink / raw)
  To: Sergei Heifetz, qemu-devel
  Cc: qemu-block, Michael S. Tsirkin, Kevin Wolf, Hanna Reitz,
	Stefano Garzarella

Looks in keeping with how virtio-blk does things with seg_max_adjust, so 
I’m happy in principle.

Just a nit and a question.

On 4/2/26 1:05 AM, Sergei Heifetz wrote:
> [You don't often get email from heifetz@yandex-team.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
> 
> The virtio specification is not completely clear about seg_max and its
> relationship with queue_size. Some drivers (for example, the modern
> Linux kernel driver) rely on seg_max to set the maximum number of
> segments used in a request. If seg_max is set larger than queue_size,
> such a driver might overwhelm a virtqueue by trying to send more
> segments than it can handle. As a result, it either hangs or faults.
> 
> One might argue that it is the vhost-user server's responsibility to set
> a valid seg_max value. However, due to the issue described in the
> previous paragraph, this value should generally depend on queue_size.
> That's why it may be necessary to control it on QEMU's side.
> 
> This patch adds the seg-max-adjust flag. A flag with the same name
> already exists for virtio-blk (and it exists to solve the same problem,
> except that here we have a vhost-user server to consult).
> 
> If seg-max-adjust is set, the final seg_max is the minimum of the value
> provided by the vhost-user server and (queue_size - 2). It is not
> enabled by default.
> 
> Signed-off-by: Sergei Heifetz <heifetz@yandex-team.com>
> ---
>   hw/block/vhost-user-blk.c          | 11 +++++++++++
>   include/hw/virtio/vhost-user-blk.h |  1 +
>   2 files changed, 12 insertions(+)
> 
> diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
> index c151e836770..23f910d9fe3 100644
> --- a/hw/block/vhost-user-blk.c
> +++ b/hw/block/vhost-user-blk.c
> @@ -65,6 +65,11 @@ static void vhost_user_blk_update_config(VirtIODevice *vdev, uint8_t *config)
>       /* Our num_queues overrides the device backend */
>       virtio_stw_p(vdev, &s->blkcfg.num_queues, s->num_queues);
> 
> +    if (s->seg_max_adjust) {

NIT: declaration at the top of the function

> +        uint32_t seg_max = MIN(s->blkcfg.seg_max, s->queue_size - 2);
> +        virtio_stl_p(vdev, &s->blkcfg.seg_max, seg_max);
> +    }
> +
>       memcpy(config, &s->blkcfg, vdev->config_len);
>   }
> 
> @@ -474,6 +479,10 @@ static void vhost_user_blk_device_realize(DeviceState *dev, Error **errp)
>           error_setg(errp, "queue size must be non-zero");
>           return;
>       }

Can you explain why queue size must be greater than 4 if seg_max_adjust 
is set? Sorry if I'm missing something obvious.

> +    if (s->queue_size < 4 && s->seg_max_adjust) {
> +        error_setg(errp, "queue size must be >= 4 when seg-max-adjust is set");
> +        return;
> +    }
>       if (s->queue_size > VIRTQUEUE_MAX_SIZE) {
>           error_setg(errp, "queue size must not exceed %d",
>                      VIRTQUEUE_MAX_SIZE);
> @@ -608,6 +617,8 @@ static const Property vhost_user_blk_properties[] = {
>       DEFINE_PROP_UINT16("num-queues", VHostUserBlk, num_queues,
>                          VHOST_USER_BLK_AUTO_NUM_QUEUES),
>       DEFINE_PROP_UINT32("queue-size", VHostUserBlk, queue_size, 128),
> +    DEFINE_PROP_BOOL("seg-max-adjust", VHostUserBlk, seg_max_adjust,
> +                      false),
>       DEFINE_PROP_BIT64("config-wce", VHostUserBlk, parent_obj.host_features,
>                         VIRTIO_BLK_F_CONFIG_WCE, true),
>       DEFINE_PROP_BIT64("discard", VHostUserBlk, parent_obj.host_features,
> diff --git a/include/hw/virtio/vhost-user-blk.h b/include/hw/virtio/vhost-user-blk.h
> index 1e41a2bcdf6..dee848cfd81 100644
> --- a/include/hw/virtio/vhost-user-blk.h
> +++ b/include/hw/virtio/vhost-user-blk.h
> @@ -34,6 +34,7 @@ struct VHostUserBlk {
>       struct virtio_blk_config blkcfg;
>       uint16_t num_queues;
>       uint32_t queue_size;
> +    bool seg_max_adjust;
>       struct vhost_dev dev;
>       struct vhost_inflight *inflight;
>       VhostUserState vhost_user;
> --
> 2.53.0
> 



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] vhost-user-blk: add seg-max-adjust flag
  2026-04-15 15:24 ` Raphael Norwitz
@ 2026-06-15 17:19   ` Vladimir Sementsov-Ogievskiy
  2026-07-04  9:17   ` Vladimir Sementsov-Ogievskiy
  1 sibling, 0 replies; 5+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2026-06-15 17:19 UTC (permalink / raw)
  To: Raphael Norwitz, qemu-devel
  Cc: qemu-block, Michael S. Tsirkin, Kevin Wolf, Hanna Reitz,
	Stefano Garzarella, Daniil Tatianin

15.04.26 18:24, Raphael Norwitz пишет:
> Looks in keeping with how virtio-blk does things with seg_max_adjust, so I’m happy in principle.
> 
> Just a nit and a question.
> 
> On 4/2/26 1:05 AM, Sergei Heifetz wrote:
>> [You don't often get email from heifetz@yandex-team.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>>
>> The virtio specification is not completely clear about seg_max and its
>> relationship with queue_size. Some drivers (for example, the modern
>> Linux kernel driver) rely on seg_max to set the maximum number of
>> segments used in a request. If seg_max is set larger than queue_size,
>> such a driver might overwhelm a virtqueue by trying to send more
>> segments than it can handle. As a result, it either hangs or faults.
>>
>> One might argue that it is the vhost-user server's responsibility to set
>> a valid seg_max value. However, due to the issue described in the
>> previous paragraph, this value should generally depend on queue_size.
>> That's why it may be necessary to control it on QEMU's side.
>>
>> This patch adds the seg-max-adjust flag. A flag with the same name
>> already exists for virtio-blk (and it exists to solve the same problem,
>> except that here we have a vhost-user server to consult).
>>
>> If seg-max-adjust is set, the final seg_max is the minimum of the value
>> provided by the vhost-user server and (queue_size - 2). It is not
>> enabled by default.
>>
>> Signed-off-by: Sergei Heifetz <heifetz@yandex-team.com>
>> ---
>>   hw/block/vhost-user-blk.c          | 11 +++++++++++
>>   include/hw/virtio/vhost-user-blk.h |  1 +
>>   2 files changed, 12 insertions(+)
>>
>> diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
>> index c151e836770..23f910d9fe3 100644
>> --- a/hw/block/vhost-user-blk.c
>> +++ b/hw/block/vhost-user-blk.c
>> @@ -65,6 +65,11 @@ static void vhost_user_blk_update_config(VirtIODevice *vdev, uint8_t *config)
>>       /* Our num_queues overrides the device backend */
>>       virtio_stw_p(vdev, &s->blkcfg.num_queues, s->num_queues);
>>
>> +    if (s->seg_max_adjust) {
> 
> NIT: declaration at the top of the function
> 
>> +        uint32_t seg_max = MIN(s->blkcfg.seg_max, s->queue_size - 2);
>> +        virtio_stl_p(vdev, &s->blkcfg.seg_max, seg_max);
>> +    }
>> +
>>       memcpy(config, &s->blkcfg, vdev->config_len);
>>   }
>>
>> @@ -474,6 +479,10 @@ static void vhost_user_blk_device_realize(DeviceState *dev, Error **errp)
>>           error_setg(errp, "queue size must be non-zero");
>>           return;
>>       }
> 
> Can you explain why queue size must be greater than 4 if seg_max_adjust is set? Sorry if I'm missing something obvious.


Because we want seg_max = MIN(s->blkcfg.seg_max, s->queue_size - 2),

and queue size must be power of two. So, for seg_max be > 0, we need queue_size at least 4.

> 
>> +    if (s->queue_size < 4 && s->seg_max_adjust) {
>> +        error_setg(errp, "queue size must be >= 4 when seg-max-adjust is set");
>> +        return;
>> +    }
>>       if (s->queue_size > VIRTQUEUE_MAX_SIZE) {
>>           error_setg(errp, "queue size must not exceed %d",
>>                      VIRTQUEUE_MAX_SIZE);
>> @@ -608,6 +617,8 @@ static const Property vhost_user_blk_properties[] = {
>>       DEFINE_PROP_UINT16("num-queues", VHostUserBlk, num_queues,
>>                          VHOST_USER_BLK_AUTO_NUM_QUEUES),
>>       DEFINE_PROP_UINT32("queue-size", VHostUserBlk, queue_size, 128),
>> +    DEFINE_PROP_BOOL("seg-max-adjust", VHostUserBlk, seg_max_adjust,
>> +                      false),
>>       DEFINE_PROP_BIT64("config-wce", VHostUserBlk, parent_obj.host_features,
>>                         VIRTIO_BLK_F_CONFIG_WCE, true),
>>       DEFINE_PROP_BIT64("discard", VHostUserBlk, parent_obj.host_features,
>> diff --git a/include/hw/virtio/vhost-user-blk.h b/include/hw/virtio/vhost-user-blk.h
>> index 1e41a2bcdf6..dee848cfd81 100644
>> --- a/include/hw/virtio/vhost-user-blk.h
>> +++ b/include/hw/virtio/vhost-user-blk.h
>> @@ -34,6 +34,7 @@ struct VHostUserBlk {
>>       struct virtio_blk_config blkcfg;
>>       uint16_t num_queues;
>>       uint32_t queue_size;
>> +    bool seg_max_adjust;
>>       struct vhost_dev dev;
>>       struct vhost_inflight *inflight;
>>       VhostUserState vhost_user;
>> -- 
>> 2.53.0
>>
> 
> 


-- 
Best regards,
Vladimir


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] vhost-user-blk: add seg-max-adjust flag
  2026-04-15 15:24 ` Raphael Norwitz
  2026-06-15 17:19   ` Vladimir Sementsov-Ogievskiy
@ 2026-07-04  9:17   ` Vladimir Sementsov-Ogievskiy
  2026-07-04  9:57     ` Michael S. Tsirkin
  1 sibling, 1 reply; 5+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2026-07-04  9:17 UTC (permalink / raw)
  To: Raphael Norwitz, Sergei Heifetz, qemu-devel
  Cc: qemu-block, Michael S. Tsirkin, Kevin Wolf, Hanna Reitz,
	Stefano Garzarella

On 15.04.26 18:24, Raphael Norwitz wrote:
>> diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
>> index c151e836770..23f910d9fe3 100644
>> --- a/hw/block/vhost-user-blk.c
>> +++ b/hw/block/vhost-user-blk.c
>> @@ -65,6 +65,11 @@ static void vhost_user_blk_update_config(VirtIODevice *vdev, uint8_t *config)
>>       /* Our num_queues overrides the device backend */
>>       virtio_stw_p(vdev, &s->blkcfg.num_queues, s->num_queues);
>>
>> +    if (s->seg_max_adjust) {
> 
> NIT: declaration at the top of the function

Hmm, no, accordingly to docs/dev/style.rst, "at the beginning of blocks",
so, it's OK to declare it at top of if-block, it's often used in the code,
look at output of `git grep -B 3 '^        int '` for example.

> 
>> +        uint32_t seg_max = MIN(s->blkcfg.seg_max, s->queue_size - 2);
>> +        virtio_stl_p(vdev, &s->blkcfg.seg_max, seg_max);
>> +    } 


-- 
Best regards,
Vladimir


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] vhost-user-blk: add seg-max-adjust flag
  2026-07-04  9:17   ` Vladimir Sementsov-Ogievskiy
@ 2026-07-04  9:57     ` Michael S. Tsirkin
  0 siblings, 0 replies; 5+ messages in thread
From: Michael S. Tsirkin @ 2026-07-04  9:57 UTC (permalink / raw)
  To: Vladimir Sementsov-Ogievskiy
  Cc: Raphael Norwitz, Sergei Heifetz, qemu-devel, qemu-block,
	Kevin Wolf, Hanna Reitz, Stefano Garzarella

On Sat, Jul 04, 2026 at 12:17:40PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> On 15.04.26 18:24, Raphael Norwitz wrote:
> > > diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
> > > index c151e836770..23f910d9fe3 100644
> > > --- a/hw/block/vhost-user-blk.c
> > > +++ b/hw/block/vhost-user-blk.c
> > > @@ -65,6 +65,11 @@ static void vhost_user_blk_update_config(VirtIODevice *vdev, uint8_t *config)
> > >       /* Our num_queues overrides the device backend */
> > >       virtio_stw_p(vdev, &s->blkcfg.num_queues, s->num_queues);
> > > 
> > > +    if (s->seg_max_adjust) {
> > 
> > NIT: declaration at the top of the function
> 
> Hmm, no, accordingly to docs/dev/style.rst, "at the beginning of blocks",
> so, it's OK to declare it at top of if-block, it's often used in the code,
> look at output of `git grep -B 3 '^        int '` for example.

indeed.

> > 
> > > +        uint32_t seg_max = MIN(s->blkcfg.seg_max, s->queue_size - 2);

if you respon pls add an empty line here.

> > > +        virtio_stl_p(vdev, &s->blkcfg.seg_max, seg_max);
> > > +    }
> 
> 
> -- 
> Best regards,
> Vladimir



^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-07-04  9:58 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-01 23:05 [PATCH] vhost-user-blk: add seg-max-adjust flag Sergei Heifetz
2026-04-15 15:24 ` Raphael Norwitz
2026-06-15 17:19   ` Vladimir Sementsov-Ogievskiy
2026-07-04  9:17   ` Vladimir Sementsov-Ogievskiy
2026-07-04  9:57     ` Michael S. Tsirkin

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.