Re: [PATCH 2/2] content: Support enabling virtqueue after DRIVER_OK stage

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

On Sat, 9 Sep 2023 17:32:18 +0300, Parav Pandit <parav@nvidia.com> wrote:
> Currently, a virtqueue must be enabled before driver has set the
> DRIVER_OK status bit.
>
> spec citation to section "Driver Requirements: Device Initialization"
>
> "Perform device-specific setup, including discovery of virtqueues
> for the device, optional per-bus setup, reading and possibly writing
> the deviceâs virtio configuration space, and population of virtqueues."
>
> This implies that a virtqueue must be enabled before reaching the
> DRIVER_OK stage. There was no explicit mention about ability to
> enable the virtqueue after DRIVER_OK stage.
>
> In following usecases, creating a virtqueue after DRIVER_OK phase is
> desired.
>
> 1. Dynamically create aq when administrative commands to be used.
> 2. Dynamically create the net device tx/rxq when device is
>    opened when deploying for a container.
>    In a container, number of virtqueues to be used may be <= max queues.
> 3. Dynamically create flow filter queues of netdevice when
>    ARFS or ethtool filters are enabled as listed in [1].
> 4. Dynamically create rtc functionality related read virtqueue only
>    when net device when timestamping to be used.
> 5. When XDP program is set, one can create additional XDP specific
>    queues without affecting existing queues.
>
> Hence, This patch introduces an existing queue enable and disable
> (aka reset) facility and a new feature bit to explicitly indicate such
> support by the device.
>
> With this feature, drivers can skip optional queues creation during
> driver init time. For example, a Linux net device driver
> can create/destroy the transmit and receive queues when net device's
> ndo_open() and ndo_stop() callbacks are invoked respectively.
>
> As side effect, it also enables driver to not consume MSI-X vectors for
> the PCI device at driver load time and utilize(enable) individual vector
> when needed. This further helps on some cpus at high scale where a
> interrupt line may not be available from the platform.
>
> [1] https://lists.oasis-open.org/archives/virtio-comment/202308/msg00263.html
>
> Signed-off-by: Parav Pandit <parav@nvidia.com>
> ---
>  conformance.tex |  2 ++
>  content.tex     | 54 ++++++++++++++++++++++++++++++++++++++++++++++---
>  2 files changed, 53 insertions(+), 3 deletions(-)
>
> diff --git a/conformance.tex b/conformance.tex
> index 863f9c5..07b0b7b 100644
> --- a/conformance.tex
> +++ b/conformance.tex
> @@ -77,6 +77,7 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
>  \item \ref{drivernormative:Basic Facilities of a Virtio Device / Device Configuration Space}
>  \item \ref{drivernormative:Basic Facilities of a Virtio Device / Virtqueues}
>  \item \ref{drivernormative:Basic Facilities of a Virtio Device / Virtqueues / Virtqueue Reset / Virtqueue Reset}
> +\item \ref{devicenormative:Basic Facilities of a Virtio Device / Virtqueues / Dynamic Virtqueues / Enable Virtqueues}

s/devicenormative/drivernormative/


>  \item \ref{drivernormative:Basic Facilities of a Virtio Device / Message Framing}
>  \item \ref{drivernormative:Basic Facilities of a Virtio Device / Virtqueues / The Virtqueue Descriptor Table}
>  \item \ref{drivernormative:Basic Facilities of a Virtio Device / Virtqueues / The Virtqueue Descriptor Table / Indirect Descriptors}
> @@ -164,6 +165,7 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
>  \item \ref{devicenormative:Basic Facilities of a Virtio Device / Device Reset}
>  \item \ref{devicenormative:Basic Facilities of a Virtio Device / Device Configuration Space}
>  \item \ref{devicenormative:Basic Facilities of a Virtio Device / Virtqueues / Virtqueue Reset / Virtqueue Reset}
> +\item \ref{devicenormative:Basic Facilities of a Virtio Device / Virtqueues / Dynamic Virtqueues / Enable Virtqueues}
>  \item \ref{devicenormative:Basic Facilities of a Virtio Device / Message Framing}
>  \item \ref{devicenormative:Basic Facilities of a Virtio Device / Virtqueues / The Virtqueue Descriptor Table}
>  \item \ref{devicenormative:Basic Facilities of a Virtio Device / Virtqueues / The Virtqueue Descriptor Table / Indirect Descriptors}
> diff --git a/content.tex b/content.tex
> index 0a62dce..91aee25 100644
> --- a/content.tex
> +++ b/content.tex
> @@ -102,7 +102,7 @@ \section{Feature Bits}\label{sec:Basic Facilities of a Virtio Device / Feature B
>  \item[24 to 41] Feature bits reserved for extensions to the queue and
>    feature negotiation mechanisms
>
> -\item[42 to 49, and 128 and above] Feature bits reserved for future extensions.
> +\item[43 to 49, and 128 and above] Feature bits reserved for future extensions.
>  \end{description}
>
>  \begin{note}
> @@ -440,6 +440,42 @@ \subsubsection{Virtqueue Re-enable}\label{sec:Basic Facilities of a Virtio Devic
>  as during initial virtqueue discovery, but optionally with different
>  parameters.
>
> +\subsection{Dynamic Virtqueues}\label{sec:Basic Facilities of a Virtio Device / Virtqueues / Dynamic Virtqueues}
> +
> +When VIRTIO_F_RING_DYNAMIC is not negotiated, the virtqueues are
> +enabled when the device is initialized, i.e. after the driver has ensured that
> +the device has set the FEATURES_OK status bit and before the driver setting
> +the DRIVER_OK status bit.
> +
> +When VIRTIO_F_RING_DYNAMIC is negotiated, the driver can avoid enabling the queue
> +before setting the DRIVER_OK status bit; the driver can enable the queue after
> +the driver has set the DRIVER_OK status bit. The queue enable mechanism is
> +transport specific.
> +
> +\devicenormative{\paragraph}{Enable Virtqueues}{Basic Facilities of a Virtio Device / Virtqueues / Dynamic Virtqueues / Enable Virtqueues}
> +
> +When VIRTIO_F_RING_DYNAMIC is negotiated, the device MUST allow enabling the
> +virtqueue which was not enabled during the device initialization phase
> +i.e. after the device has set the FEATURES_OK status bit and before the
> +driver has set the DRIVER_OK status bit.
> +
> +\drivernormative{\paragraph}{Enable Virtqueues}{Basic Facilities of a Virtio Device / Virtqueues / Dynamic Virtqueues / Enable Virtqueues}
> +
> +When VIRTIO_F_RING_DYNAMIC is negotiated,
> +\begin{itemize}
> +\item
> +the driver MAY not enable all the virtqueues during the device initialization phase;

Remove the \n after \item

> +
> +\item the driver MAY enable some or all the virtqueues after the driver has set the
> +	DRIVER_OK status bit.
> +
> +\item the driver MAY enable all the virtqueues during the device initialization phase;
> +\end{itemize}
> +
> +The driver interested in enabling and disabling the virtqueue dynamically
> +after setting the DRIVER_OK status bit MUST negotiate VIRTIO_F_RING_DYNAMIC
> +and VIRTIO_F_RING_RESET features.
> +
>  \input{split-ring.tex}
>
>  \input{packed-ring.tex}
> @@ -537,7 +573,9 @@ \section{Device Initialization}\label{sec:General Initialization And Device Oper
>
>  \item\label{itm:General Initialization And Device Operation / Device Initialization / Device-specific Setup} Perform device-specific setup, including discovery of virtqueues for the
>     device, optional per-bus setup, reading and possibly writing the
> -   device's virtio configuration space, and population of virtqueues.
> +   device's virtio configuration space.
> +
> +\item\label{itm:General Initialization And Device Operation / Device Initialization / Virtqueue Setup} Configure and enable the virtqueues.
>
>  \item\label{itm:General Initialization And Device Operation / Device Initialization / Set DRIVER-OK} Set the DRIVER_OK status bit.  At this point the device is
>     ``live''.
> @@ -551,6 +589,10 @@ \section{Device Initialization}\label{sec:General Initialization And Device Oper
>  The driver MUST NOT send any buffer available notifications to
>  the device before setting DRIVER_OK.
>
> +The driver MAY skip step
> +\ref{itm:General Initialization And Device Operation / Device Initialization / Virtqueue Setup}
> +when driver has negotiated VIRTIO_F_RING_DYNAMIC feature.
> +
>  \subsection{Legacy Interface: Device Initialization}\label{sec:General Initialization And Device Operation / Device Initialization / Legacy Interface: Device Initialization}
>  Legacy devices did not support the FEATURES_OK status bit, and thus did
>  not have a graceful way for the device to indicate unsupported feature
> @@ -569,7 +611,7 @@ \subsection{Legacy Interface: Device Initialization}\label{sec:General Initializ
>  Initialization / Re-read FEATURES-OK} were omitted, and steps
>  \ref{itm:General Initialization And Device Operation /
>  Device Initialization / Read feature bits},
> -\ref{itm:General Initialization And Device Operation / Device Initialization / Device-specific Setup} and \ref{itm:General Initialization And Device Operation / Device Initialization / Set DRIVER-OK}
> +\ref{itm:General Initialization And Device Operation / Device Initialization / Device-specific Setup}, \ref{itm:General Initialization And Device Operation / Device Initialization / Virtqueue Setup} and \ref{itm:General Initialization And Device Operation / Device Initialization / Set DRIVER-OK}
>  were conflated.
>
>  Therefore, when using the legacy interface:
> @@ -872,8 +914,14 @@ \chapter{Reserved Feature Bits}\label{sec:Reserved Feature Bits}
>  	\ref{devicenormative:Basic Facilities of a Virtio Device / Feature Bits} for
>  	handling features reserved for future use.
>
> +  \item[VIRTIO_F_RING_DYNAMIC(42)] This feature indicates
> +  that the driver can enable a virtqueue individually after the driver has
> +  set the status bit of DRIVER_OK.
> +  See \ref{sec:Basic Facilities of a Virtio Device / Virtqueues / Dynamic Virtqueues}.
> +
>  \end{description}
>
> +

No add new empty line, if we do not need it.


Thanks.

>  \drivernormative{\section}{Reserved Feature Bits}{Reserved Feature Bits}
>
>  A driver MUST accept VIRTIO_F_VERSION_1 if it is offered.  A driver
> --
> 2.34.1
>