[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]
Subject: Re: [PATCH 2/2] Add common configuration field "queue_indirect_size"
On Montag, 24. Januar 2022 14:53:45 CET Stefan Hajnoczi wrote: > On Tue, Dec 14, 2021 at 04:13:17PM +0100, Christian Schoenebeck wrote: > > This new common configuration field allows to negotiate a more fine > > graded maximum lenght of indirect descriptor chains. > > > > Fixes: https://github.com/oasis-tcs/virtio-spec/issues/122 > > Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com> > > --- > > > > content.tex | 25 ++++++++++++++++++++++++- > > split-ring.tex | 3 +++ > > 2 files changed, 27 insertions(+), 1 deletion(-) > > > > diff --git a/content.tex b/content.tex Stefan, while preparing the next version of patches for this issue (#122), I noticed that I missed your email here ... > > index 0aa4842..e3cfcae 100644 > > --- a/content.tex > > +++ b/content.tex > > @@ -859,6 +859,7 @@ \subsubsection{Common configuration structure > > layout}\label{sec:Virtio Transport> > > le64 queue_driver; /* read-write */ > > le64 queue_device; /* read-write */ > > le16 queue_notify_data; /* read-only for driver */ > > > > + le32 queue_indirect_size; /* read-write */ > > I see no other unaligned fields in this struct, so I think a le16 > padding field is needed for safety: > > le64 queue_device; /* read-write */ > le16 queue_notify_data; /* read-only for driver */ > + le16 reserved; /* no access */ > + le32 queue_indirect_size; /* read-write */ > That's no longer needed as there's now the new 'queue_reset' field, i.e.: diff --git a/content.tex b/content.tex index 685525d..da57d5d 100644 --- a/content.tex +++ b/content.tex @@ -902,6 +902,7 @@ \subsubsection{Common configuration structure layout} \label{sec:Virtio Transport le64 queue_device; /* read-write */ le16 queue_notify_data; /* read-only for driver */ le16 queue_reset; /* read-write */ + le32 queue_indirect_size; /* read-write */ }; \end{lstlisting} > > }; > > \end{lstlisting} > > > > @@ -938,6 +939,16 @@ \subsubsection{Common configuration structure > > layout}\label{sec:Virtio Transport> > > may benefit from providing another value, for example an internal > > virtqueue > > identifier, or an internal offset related to the virtqueue > > number. > > \end{note} > > > > + > > +\item[\field{queue_indirect_size}] > > + This field is used to negotiate the maximum amount of descriptors > > per + vring slot as in \ref{sec:Basic Facilities of a Virtio > > Device / + Virtqueues / The Virtqueue Descriptor Table / Indirect > > Descriptors} if + and only if the VIRTIO_RING_F_INDIRECT_SIZE > > feature has been negotiated. + > > + The device specifies its maximum supported number of descriptors > > per + vring slot. If the driver requires fewer descriptors, it > > writes its + lower value to inform the device of the reduced > > resource requirements. > "vring slot" is a little vague, it means "indirect descriptor > table"? The two paragraphs could use "maximum number of descriptors per > indirect descriptor table" instead of referring to vring slots to imply > indirect descriptor tables. The intended phrasing was intended to reflect that "Queue Indirect Size" is meant to be the *sum* of all indirect descriptors: https://lists.oasis-open.org/archives/virtio-comment/202112/msg00008.html Just to avoid that I am missing something here; as of now, there can only be two indirect tables per round-trip message, right? Best regards, Christian Schoenebeck
[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]