Re: [RFC PATCH] ring: Forbid using descriptors in two chains at once

["Michael S. Tsirkin" <mst@redhat.com>]

On Thu, May 07, 2026 at 12:34:19AM -0400, Demi Marie Obenour wrote:
> Using the same descriptor in multiple chains at once creates
> difficulties.  If the descriptor is device-writeable, this is begging
> for data corruption due to conflicting writes from the device.  Even if
> the descriptor is device-readable, the mere possibility requires devices
> to use more sophisticated internal data structures.

The natural way to index requests is by using the avail ring head,
not descriptors, though? I suspect it's about device memory being
limited, etc. But pls make it more explicit.

> Require descriptors to be used in at most one chain at any given time.
> Indirect descriptors can be used if support for huge batches is needed.

does this imply indirect are exempt from this rule? the text
you propose makes no such exemptions.

> 
> Signed-off-by: Demi Marie Obenour <demiobenour@gmail.com>
> ---
> I have not checked whether this will break existing drivers, so I'm
> marking this as RFC for now.
> ---
>  packed-ring.tex | 6 ++++++
>  split-ring.tex  | 6 ++++++
>  2 files changed, 12 insertions(+)
> 
> diff --git a/packed-ring.tex b/packed-ring.tex
> index 3ee55a18d024d4da1a48d522e98ea563b9809b1c..8fd5a770c7ca4e6f60911b2861e00a273a69ea73 100644
> --- a/packed-ring.tex
> +++ b/packed-ring.tex
> @@ -473,6 +473,12 @@ \subsection{Event Suppression Structure Format}\label{sec:Basic
>  
>  This implies that loops in the descriptor list are forbidden!
>  
> +Drivers MUST NOT include a descriptor in a chain if the
> +descriptor is already part of a previous chain whose processing
> +has not completed.  This implies that the total number of
> +descriptors submitted to the device, but not yet processed, is
> +limited by the queue size.

I do not understand what is this trying to say for the packed ring.
How do you include a descriptor in a chain twice - the ring has to
wrap around, no?

For that matter, I do not think "This implies that loops in the
descriptor list are forbidden!" makes sense for packed either -
I think we copied it from split by mistake.


> +
>  The driver MUST place any device-writable descriptor elements after
>  any device-readable descriptor elements.
>  
> diff --git a/split-ring.tex b/split-ring.tex
> index de9403882df1e6d20e16ec983ecf0a46d39efa6b..014e279ef1da7653c7051e3a24c6169fe8d51983 100644
> --- a/split-ring.tex
> +++ b/split-ring.tex
> @@ -223,6 +223,12 @@ \subsection{The Virtqueue Descriptor Table}\label{sec:Basic Facilities of a Virt
>  Drivers MUST NOT add a descriptor chain longer than $2^{32}$ bytes in total;
>  this implies that loops in the descriptor chain are forbidden!
>  
> +Drivers MUST NOT include a descriptor in a chain if the
> +descriptor is already part of a previous chain whose processing
> +has not completed.

what specifically is "include a descriptor in a chain"? something to do
with making a head available? and what is "whose processing has not
completed"? something to do with a head not used?

>  This implies that the total number of
> +descriptors submitted to the device, but not yet processed, is
> +limited by the queue size.
> +
>  If VIRTIO_F_IN_ORDER has been negotiated, and when making a
>  descriptor with VRING_DESC_F_NEXT set in \field{flags} at offset
>  $x$ in the table available to the device, driver MUST set



We can't really introduce new mandatory requirements without
a feature bit.


> -- 
> 2.54.0
>