[PATCH] virtio: add support for SIZE_MAX & SEG_MAX features

[Simon Glass]

Hi Christian,

On 2026-03-27T08:18:58, Christian Pötzsch
<christian.poetzsch at kernkonzept.com> wrote:
> virtio: add support for SIZE_MAX & SEG_MAX features
> virtio: add support for SIZE_MAX & SEG_MAX features
>
> Some virtio implementations may forward the virtio requests directly to
> the underlying hw. The hw may have some restrictions in how many and how
> big the requests can be. Therefore, the corresponding virtio device will
> announce this limitations with the SIZE_MAX & SEG_MAX feature.
>
> Add support for those features. Split an io request into multiple virtio
> requests if more than seg_max segments would be used. Also split a
> single buffer request into multiple segments if the buffer is bigger
> then size_max.
>
> Signed-off-by: Christian Pötzsch <christian.poetzsch at kernkonzept.com>
> Signed-off-by: Adam Lackorzynski <adam at l4re.org>

> diff --git a/drivers/virtio/virtio_blk.c b/drivers/virtio/virtio_blk.c
> @@ -119,7 +145,38 @@ static ulong virtio_blk_do_req(...)
> +err_free:
> +     for (i = 0; i < seg_cnt + 2; ++i)
> +             free(sgs[i]);
> +     free(sgs);
> +
> +     return status == VIRTIO_BLK_S_OK ? blkcnt : -EIO;

The err_free label is reached from both the success path (fall-through
after virtqueue_get_buf()) and error paths (goto from malloc()
failure, virtqueue_add() failure, or unknown type). On error paths,
status is uninitialised, so the final check has undefined behaviour.
Please can you separate the success path from the error cleanup, e.g.
with an explicit return before the err_free label.

> diff --git a/drivers/virtio/virtio_blk.c b/drivers/virtio/virtio_blk.c
> @@ -145,38 +145,38 @@ static ulong virtio_blk_do_req(...)
> +     while (i < blkcnt) {
> +             u32 blk_per_sg = min(blkcnt - i, seg_sec_cnt * priv->seg_max);
> +
> +             ret = virtio_blk_do_single_req(dev, sector + i, blk_per_sg,
> +                                            buffer + i * 512, type);
> +             if (!ret)
> +                     return ret;

The error check is inverted. virtio_blk_do_single_req() returns blkcnt
(positive) on success and -EIO on failure, so if (!ret) only triggers
when ret is zero. I suspect you want if (ret < 0) here.

BTW please add a function comment for virtio_blk_do_single_req() so
all this is clearer.

> diff --git a/drivers/virtio/virtio_blk.c b/drivers/virtio/virtio_blk.c
> @@ -67,50 +74,69 @@ static void virtio_blk_init_data_sg(...)
> +     // The virtio device may have constrains on the maximum segment size.

U-Boot follows the Linux kernel coding style which prefers /* */
comments over //. Please can you convert these.

Regards,
Simon