Re: [PATCH] VIRTIO: Update the desc 'flag' fied last in packed ring.

[Xuan Zhuo <xuanzhuo@linux.alibaba.com>]

On Tue,  2 Jun 2026 12:31:23 +0800, yangjiale <yangjiale133@163.com> wrote:
> When a descriptor list spans across cache lines,
> updating the flag first can lead to a scenario where the device side
> perceives the flag as valid, yet the corresponding address and length
> fields remain unupdated—resulting in invalid values.
> Therefore, the flag field must be updated last.

Are you raising this based on your code review, or did you actually encounter an
issue during testing?

I don't think there is a problem here, as all operations are protected by the
head desc flag. Even if cacheline effects cause data to be written prematurely,
it shouldn't be an issue. The device must wait until the head desc flags
are updated before it can start any subsequent work.

Thanks.

>
> Signed-off-by: yangjiale <yangjiale133@163.com>
> ---
>  drivers/virtio/virtio_ring.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> index fbca7ce1c6bf..036b4f90d30f 100644
> --- a/drivers/virtio/virtio_ring.c
> +++ b/drivers/virtio/virtio_ring.c
> @@ -1688,6 +1688,10 @@ static inline int virtqueue_add_packed(struct vring_virtqueue *vq,
>  					     &addr, &len, premapped, attr))
>  				goto unmap_release;
>
> +			desc[i].addr = cpu_to_le64(addr);
> +			desc[i].len = cpu_to_le32(len);
> +			desc[i].id = cpu_to_le16(id);
> +
>  			flags = cpu_to_le16(vq->packed.avail_used_flags |
>  				    (++c == total_sg ? 0 : VRING_DESC_F_NEXT) |
>  				    (n < out_sgs ? 0 : VRING_DESC_F_WRITE));
> @@ -1696,10 +1700,6 @@ static inline int virtqueue_add_packed(struct vring_virtqueue *vq,
>  			else
>  				desc[i].flags = flags;
>
> -			desc[i].addr = cpu_to_le64(addr);
> -			desc[i].len = cpu_to_le32(len);
> -			desc[i].id = cpu_to_le16(id);
> -
>  			if (unlikely(vq->use_map_api)) {
>  				vq->packed.desc_extra[curr].addr = premapped ?
>  					DMA_MAPPING_ERROR : addr;
> --
> 2.25.1
>