[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]
Subject: Re: [PATCH 2/5] Introduce VIRTIO_F_ADMIN_VQ_INDIRECT_DESC/VIRTIO_F_ADMIN_VQ_IN_ORDER
On Thu, Jan 13, 2022 at 04:51:00PM +0200, Max Gurtovoy wrote:
> These new features are parallel to VIRTIO_F_INDIRECT_DESC and
> VIRTIO_F_IN_ORDER. Some devices might support these features only for
> admin virtqueues and some might support them for both admin virtqueues
> and request virtqueues or only for non-admin virtqueues. Some
> optimization can be made for each type of virtqueue, thus we separate
> these features for the different virtqueue types.
>
> Reviewed-by: Parav Pandit <parav@nvidia.com>
> Signed-off-by: Max Gurtovoy <mgurtovoy@nvidia.com>
That seems vague as motivation.
Why do we need to optimize admin queues? Aren't they
fundamentally a control path feature?
Why would we want to special-case these features specifically?
Should we allow control of features per VQ generally?
> ---
> content.tex | 47 +++++++++++++++++++++++++++++++++++++++--------
> packed-ring.tex | 26 +++++++++++++-------------
> split-ring.tex | 35 +++++++++++++++++++++++------------
> 3 files changed, 75 insertions(+), 33 deletions(-)
>
> diff --git a/content.tex b/content.tex
> index c524fab..cc3e648 100644
> --- a/content.tex
> +++ b/content.tex
> @@ -99,10 +99,10 @@ \section{Feature Bits}\label{sec:Basic Facilities of a Virtio Device / Feature B
> \begin{description}
> \item[0 to 23] Feature bits for the specific device type
>
> -\item[24 to 41] Feature bits reserved for extensions to the queue and
> +\item[24 to 43] Feature bits reserved for extensions to the queue and
> feature negotiation mechanisms
>
> -\item[42 and above] Feature bits reserved for future extensions.
> +\item[44 and above] Feature bits reserved for future extensions.
> \end{description}
>
> \begin{note}
> @@ -318,8 +318,9 @@ \section{Virtqueues}\label{sec:Basic Facilities of a Virtio Device / Virtqueues}
>
> Some devices always use descriptors in the same order in which
> they have been made available. These devices can offer the
> -VIRTIO_F_IN_ORDER feature. If negotiated, this knowledge
> -might allow optimizations or simplify driver and/or device code.
> +VIRTIO_F_IN_ORDER and/or VIRTIO_F_ADMIN_VQ_IN_ORDER features.
> +If negotiated, this knowledge might allow optimizations or
> +simplify driver and/or device code.
>
> Each virtqueue can consist of up to 3 parts:
> \begin{itemize}
> @@ -6768,7 +6769,7 @@ \chapter{Reserved Feature Bits}\label{sec:Reserved Feature Bits}
> Device / Virtqueues / The Virtqueue Descriptor Table / Indirect
> Descriptors}~\nameref{sec:Basic Facilities of a Virtio Device /
> Virtqueues / The Virtqueue Descriptor Table / Indirect
> -Descriptors} and \ref{sec:Packed Virtqueues / Indirect Flag: Scatter-Gather Support}~\nameref{sec:Packed Virtqueues / Indirect Flag: Scatter-Gather Support}.
> +Descriptors} and \ref{sec:Packed Virtqueues / Indirect Flag: Scatter-Gather Support}~\nameref{sec:Packed Virtqueues / Indirect Flag: Scatter-Gather Support}. This excluding the descriptors sent via the admin virtqueue.
> \item[VIRTIO_F_EVENT_IDX(29)] This feature enables the \field{used_event}
> and the \field{avail_event} fields as described in
> \ref{sec:Basic Facilities of a Virtio Device / Virtqueues / Used Buffer Notification Suppression}, \ref{sec:Basic Facilities of a Virtio Device / Virtqueues / The Virtqueue Used Ring} and \ref{sec:Packed Virtqueues / Driver and Device Event Suppression}.
> @@ -6800,8 +6801,9 @@ \chapter{Reserved Feature Bits}\label{sec:Reserved Feature Bits}
> support for the packed virtqueue layout as described in
> \ref{sec:Basic Facilities of a Virtio Device / Packed Virtqueues}~\nameref{sec:Basic Facilities of a Virtio Device / Packed Virtqueues}.
> \item[VIRTIO_F_IN_ORDER(35)] This feature indicates
> - that all buffers are used by the device in the same
> - order in which they have been made available.
> + that all buffers are used by the device, excluding buffers used by
> + the admin virtqueue, in the same order in which they have been made
> + available.
> \item[VIRTIO_F_ORDER_PLATFORM(36)] This feature indicates
> that memory accesses by the driver and the device are ordered
> in a way described by the platform.
> @@ -6852,6 +6854,18 @@ \chapter{Reserved Feature Bits}\label{sec:Reserved Feature Bits}
> \item[VIRTIO_F_ADMIN_VQ (41)] This feature indicates that
> the device supports administration virtqueue negotiation.
>
> + \item[VIRTIO_F_ADMIN_VQ_INDIRECT_DESC (42)] Negotiating this feature
> + indicates that the driver can use descriptors with the VIRTQ_DESC_F_INDIRECT
> + flag set, as described in \ref{sec:Basic Facilities of a Virtio
> +Device / Virtqueues / The Virtqueue Descriptor Table / Indirect
> +Descriptors}~\nameref{sec:Basic Facilities of a Virtio Device /
> +Virtqueues / The Virtqueue Descriptor Table / Indirect
> +Descriptors} and \ref{sec:Packed Virtqueues / Indirect Flag: Scatter-Gather Support}~\nameref{sec:Packed Virtqueues / Indirect Flag: Scatter-Gather Support}. This refers to descriptors sent via the admin
> + virtqueue and excluding the descriptors that sent via other virtqueues.
> + \item[VIRTIO_F_ADMIN_VQ_IN_ORDER (43)] This feature indicates
> + that all buffers are used by the admin virtqueue of the device in
> + the same order in which they have been made available.
> +
> \end{description}
>
> \drivernormative{\section}{Reserved Feature Bits}{Reserved Feature Bits}
> @@ -6888,6 +6902,12 @@ \chapter{Reserved Feature Bits}\label{sec:Reserved Feature Bits}
>
> A driver SHOULD accept VIRTIO_F_NOTIF_CONFIG_DATA if it is offered.
>
> +A driver MAY accept VIRTIO_F_ADMIN_VQ_INDIRECT_DESC only if it accepts
> +VIRTIO_F_ADMIN_VQ.
> +
> +A driver MAY accept VIRTIO_F_ADMIN_VQ_IN_ORDER only if it accepts
> +VIRTIO_F_ADMIN_VQ.
> +
> \devicenormative{\section}{Reserved Feature Bits}{Reserved Feature Bits}
>
> A device MUST offer VIRTIO_F_VERSION_1. A device MAY fail to operate further
> @@ -6902,7 +6922,12 @@ \chapter{Reserved Feature Bits}\label{sec:Reserved Feature Bits}
> accepted.
>
> If VIRTIO_F_IN_ORDER has been negotiated, a device MUST use
> -buffers in the same order in which they have been available.
> +buffers in the same order in which they have been available. This refers
> +to buffers that are used by virtqueue that is not the admin virtqueue.
> +
> +If VIRTIO_F_ADMIN_VQ_IN_ORDER has been negotiated, a device MUST use
> +buffers in the same order in which they have been available. This refers
> +only for buffers that are used by the admin virtqueue.
>
> A device MAY fail to operate further if
> VIRTIO_F_ORDER_PLATFORM is offered but not accepted.
> @@ -6917,6 +6942,12 @@ \chapter{Reserved Feature Bits}\label{sec:Reserved Feature Bits}
> and presents a PCI SR-IOV capability structure, otherwise
> it MUST NOT offer VIRTIO_F_SR_IOV.
>
> +A device MAY offer VIRTIO_F_ADMIN_VQ_INDIRECT_DESC only if it offers
> +VIRTIO_F_ADMIN_VQ.
> +
> +A device MAY offer VIRTIO_F_ADMIN_VQ_IN_ORDER only if it offers
> +VIRTIO_F_ADMIN_VQ.
> +
> \section{Legacy Interface: Reserved Feature Bits}\label{sec:Reserved Feature Bits / Legacy Interface: Reserved Feature Bits}
>
> Transitional devices MAY offer the following:
> diff --git a/packed-ring.tex b/packed-ring.tex
> index a9e6c16..ef1dbc2 100644
> --- a/packed-ring.tex
> +++ b/packed-ring.tex
> @@ -240,13 +240,12 @@ \subsection{Indirect Flag: Scatter-Gather Support}
> \label{sec:Packed Virtqueues / Indirect Flag: Scatter-Gather Support}
>
> Some devices benefit by concurrently dispatching a large number
> -of large requests. The VIRTIO_F_INDIRECT_DESC feature allows this. To increase
> -ring capacity the driver can store a (read-only by the device) table of indirect
> -descriptors anywhere in memory, and insert a descriptor in the main
> -virtqueue (with \field{Flags} bit VIRTQ_DESC_F_INDIRECT on) that refers to
> -a buffer element
> -containing this indirect descriptor table; \field{addr} and \field{len}
> -refer to the indirect table address and length in bytes,
> +of large requests. The VIRTIO_F_INDIRECT_DESC and VIRTIO_F_ADMIN_VQ_INDIRECT_DESC
> +features allows this. To increase ring capacity the driver can store a (read-only
> +by the device) table of indirect descriptors anywhere in memory, and insert a
> +descriptor in the main virtqueue (with \field{Flags} bit VIRTQ_DESC_F_INDIRECT on)
> +that refers to a buffer element containing this indirect descriptor table;
> +\field{addr} and \field{len} refer to the indirect table address and length in bytes,
> respectively.
> \begin{lstlisting}
> /* This means the element contains a table of descriptors. */
> @@ -279,10 +278,11 @@ \subsection{In-order use of descriptors}
>
> Some devices always use descriptors in the same order in which
> they have been made available. These devices can offer the
> -VIRTIO_F_IN_ORDER feature. If negotiated, this knowledge allows
> -devices to notify the use of a batch of buffers to the driver by
> -only writing out a single used descriptor with the Buffer ID
> -corresponding to the last descriptor in the batch.
> +VIRTIO_F_IN_ORDER and/or VIRTIO_F_ADMIN_VQ_IN_ORDER features.
> +If negotiated, this knowledge allows devices to notify the use of
> +a batch of buffers to the driver by only writing out a single used
> +descriptor with the Buffer ID corresponding to the last descriptor
> +in the batch.
>
> The device then skips forward in the ring according to the size of
> the batch. The driver needs to look up the used Buffer ID and
> @@ -500,8 +500,8 @@ \subsection{Event Suppression Structure Format}\label{sec:Basic
>
> \drivernormative{\subsection}{Indirect Descriptors}{Basic Facilities of a Virtio Device / Packed Virtqueues / The Virtqueue Descriptor Table / Indirect Descriptors}
> The driver MUST NOT set the DESC_F_INDIRECT flag unless the
> -VIRTIO_F_INDIRECT_DESC feature was negotiated. The driver MUST NOT
> -set any flags except DESC_F_WRITE within an indirect descriptor.
> +VIRTIO_F_INDIRECT_DESC or VIRTIO_F_ADMIN_VQ_INDIRECT_DESC features were negotiated.
> +The driver MUST NOT set any flags except DESC_F_WRITE within an indirect descriptor.
>
> A driver MUST NOT create a descriptor chain longer than allowed
> by the device.
> diff --git a/split-ring.tex b/split-ring.tex
> index de94038..cd53840 100644
> --- a/split-ring.tex
> +++ b/split-ring.tex
> @@ -208,6 +208,10 @@ \subsection{The Virtqueue Descriptor Table}\label{sec:Basic Facilities of a Virt
> descriptors in ring order: starting from offset 0 in the table,
> and wrapping around at the end of the table.
>
> +If VIRTIO_F_ADMIN_VQ_IN_ORDER has been negotiated, driver uses
> +descriptors in admin virtqueue ring order: starting from offset 0 in the
> +table, and wrapping around at the end of the table.
> +
> \begin{note}
> The legacy \hyperref[intro:Virtio PCI Draft]{[Virtio PCI Draft]}
> referred to this structure as vring_desc, and the constants as
> @@ -223,16 +227,18 @@ \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!
>
> -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
> +If VIRTIO_F_ADMIN_VQ_IN_ORDER and/or 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
> \field{next} to $0$ for the last descriptor in the table
> (where $x = queue\_size - 1$) and to $x + 1$ for the rest of the descriptors.
> +This refers to admin virtqueue descriptors and rest other virtqueues types descriptors respectively.
>
> \subsubsection{Indirect Descriptors}\label{sec:Basic Facilities of a Virtio Device / Virtqueues / The Virtqueue Descriptor Table / Indirect Descriptors}
>
> Some devices benefit by concurrently dispatching a large number
> -of large requests. The VIRTIO_F_INDIRECT_DESC feature allows this (see \ref{sec:virtio-queue.h}~\nameref{sec:virtio-queue.h}). To increase
> +of large requests. The VIRTIO_F_INDIRECT_DESC and VIRTIO_F_ADMIN_VQ_INDIRECT_DESC
> +features allows this (see \ref{sec:virtio-queue.h}~\nameref{sec:virtio-queue.h}). To increase
> ring capacity the driver can store a table of indirect
> descriptors anywhere in memory, and insert a descriptor in main
> virtqueue (with \field{flags}\&VIRTQ_DESC_F_INDIRECT on) that refers to memory buffer
> @@ -258,15 +264,19 @@ \subsubsection{Indirect Descriptors}\label{sec:Basic Facilities of a Virtio Devi
> A single indirect descriptor
> table can include both device-readable and device-writable descriptors.
>
> -If VIRTIO_F_IN_ORDER has been negotiated, indirect descriptors
> -use sequential indices, in-order: index 0 followed by index 1
> +If VIRTIO_F_IN_ORDER has been negotiated, indirect descriptors,
> +for non admin virtqueue, use sequential indices, in-order: index 0 followed
> +by index 1 followed by index 2, etc.
> +
> +If VIRTIO_F_ADMIN_VQ_IN_ORDER has been negotiated, admin virtqueue indirect
> +descriptors use sequential indices, in-order: index 0 followed by index 1
> followed by index 2, etc.
>
> \drivernormative{\paragraph}{Indirect Descriptors}{Basic Facilities of a Virtio Device / Virtqueues / The Virtqueue Descriptor Table / Indirect Descriptors}
> The driver MUST NOT set the VIRTQ_DESC_F_INDIRECT flag unless the
> -VIRTIO_F_INDIRECT_DESC feature was negotiated. The driver MUST NOT
> -set the VIRTQ_DESC_F_INDIRECT flag within an indirect descriptor (ie. only
> -one table per descriptor).
> +VIRTIO_F_INDIRECT_DESC and/or VIRTIO_F_ADMIN_VQ_INDIRECT_DESC features were negotiated.
> +The driver MUST NOT set the VIRTQ_DESC_F_INDIRECT flag within an indirect
> +descriptor (ie. only one table per descriptor).
>
> A driver MUST NOT create a descriptor chain longer than the Queue Size of
> the device.
> @@ -274,9 +284,10 @@ \subsubsection{Indirect Descriptors}\label{sec:Basic Facilities of a Virtio Devi
> A driver MUST NOT set both VIRTQ_DESC_F_INDIRECT and VIRTQ_DESC_F_NEXT
> in \field{flags}.
>
> -If VIRTIO_F_IN_ORDER has been negotiated, indirect descriptors
> -MUST appear sequentially, with \field{next} taking the value
> -of 1 for the 1st descriptor, 2 for the 2nd one, etc.
> +If VIRTIO_F_ADMIN_VQ_IN_ORDER and/or VIRTIO_F_IN_ORDER has been negotiated,
> +indirect descriptors MUST appear sequentially, with \field{next} taking the
> +value of 1 for the 1st descriptor, 2 for the 2nd one, etc for admin virtqueue
> +and rest other virtqueues types respectively.
>
> \devicenormative{\paragraph}{Indirect Descriptors}{Basic Facilities of a Virtio Device / Virtqueues / The Virtqueue Descriptor Table / Indirect Descriptors}
> The device MUST ignore the write-only flag (\field{flags}\&VIRTQ_DESC_F_WRITE) in the descriptor that refers to an indirect table.
> --
> 2.21.0
[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]