Re: [PATCH] admin: Enable driver to send admin commands before DRIVER_OK

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

On Tue, Sep 10, 2024 at 08:07:23PM +0300, Parav Pandit wrote:
> Currently the driver can operate administration commands using
> administration virtqueues. Administration virtqueues must be
> enabled before it reached DRIVER_OK stage similar to all other
> queues. This is a limiting factor.
> 
> Many of the device functionalties needs to be discovered and
> configured early enough before the driver reaches the
> DRIVER_OK stage. Some examples are:
> 
> a. driver wants to dynamically create the virtqueues of virtio-net
>    device with more parameters, for example header data split,
>    multiple physical addresses.
>    Here, the driver needs to tell PCI device early enough that it no
>    longer uses queue_* registers for non admin queues.
> 
> b. driver wants to discover these features and related attributes
>    early enough so it has chance to decide to proceed via admin
>    cmd interface or proceed to DRIVER_OK and follow the current flow.
> 
> To overcome these limitations, introduce a feature bit that enables
> the driver to send capabilities related admin commands before DRIVER_OK
> via the available interface such as administration virtqueue.
> 
> Signed-off-by: Parav Pandit <parav@nvidia.com>


We were there in 1.0 and it's messy. And next thing you do, you will
want it before features ok, and we will keep piling up hacks.  I think
that if we want to do admin commands before VQs are active, we should
just add a capability with registers for that. People wanted that
anyway.





> ---
>  admin.tex         | 16 ++++++++++++++++
>  content.tex       | 13 ++++++++++++-
>  transport-pci.tex |  6 ++++++
>  3 files changed, 34 insertions(+), 1 deletion(-)
> 
> diff --git a/admin.tex b/admin.tex
> index 39fc072..95054ed 100644
> --- a/admin.tex
> +++ b/admin.tex
> @@ -334,6 +334,11 @@ \subsection{Group administration commands}\label{sec:Basic Facilities of a Virti
>  supporting multiple group types, the list of supported commands
>  might differ between different group types.
>  
> +When the driver has negotiated the feature VIRTIO_F_EARLY_CAP_ADMIN_CMD, the
> +driver can use administration commands VIRTIO_ADMIN_CMD_LIST_QUERY,
> +VIRTIO_ADMIN_CMD_LIST_USE and commands related to device and driver capabilities
> +listed in \ref[sec:Basic Facilities of a Virtio Device / Device groups / Group administration commands / Device and driver capabilities]{device and driver capabilities} for the self-group type before the driver indicates DRIVER_OK status to the device.
> +
>  \input{admin-cmds-legacy-interface.tex}
>  \input{admin-cmds-capabilities.tex}
>  \input{admin-cmds-resource-objects.tex}
> @@ -608,6 +613,14 @@ \section{Administration Virtqueues}\label{sec:Basic Facilities of a Virtio Devic
>  or VIRTIO_ADMIN_STATUS_ENOMEM, then the command MUST NOT
>  have any side effects, making it safe to retry.
>  
> +If VIRTIO_F_EARLY_CAP_ADMIN_CMD feature is negotiated, the device MUST process
> +administration commands related to device and driver capabilities before the
> +driver indicates DRIVER_OK to the device.
> +
> +If VIRTIO_F_ADMIN_VQ and VIRTIO_F_EARLY_CAP_ADMIN_CMD features are negotiated,
> +the device MUST be able to generate notifications related to the administration
> +virtqueue before the driver indicates DRIVER_OK to the device.
> +
>  \drivernormative{\subsection}{Group administration commands}{Basic Facilities of a Virtio Device / Administration virtqueues}
>  
>  The driver MAY supply device-readable or device-writeable parts
> @@ -641,3 +654,6 @@ \section{Administration Virtqueues}\label{sec:Basic Facilities of a Virtio Devic
>  
>  The driver SHOULD retry a command that completed with
>  \field{status} VIRTIO_ADMIN_STATUS_EAGAIN.
> +
> +When the VIRTIO_F_EARLY_ADMIN_CMD feature is negotiated, the driver MAY send commands
> +only to the first administration queue defined by the specific transport.
> diff --git a/content.tex b/content.tex
> index c32c218..12f9224 100644
> --- a/content.tex
> +++ b/content.tex
> @@ -540,6 +540,8 @@ \section{Device Initialization}\label{sec:General Initialization And Device Oper
>     device, optional per-bus setup, reading and possibly writing the
>     device's virtio configuration space, and population of virtqueues.
>  
> +\item\label{itm:General Initialization And Device Operation / Device Initialization / Capabilities Access} Optionally get device capabilities and set driver capabilities if VIRTIO_F_EARLY_CAP_ADMIN_CMD is negotiated.
> +
>  \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''.
>  \end{enumerate}
> @@ -550,7 +552,9 @@ \section{Device Initialization}\label{sec:General Initialization And Device Oper
>  driver MUST NOT continue initialization in that case.
>  
>  The driver MUST NOT send any buffer available notifications to
> -the device before setting DRIVER_OK.
> +the device before setting DRIVER_OK; however, the driver MAY send
> +buffer available notifications before setting DRIVER_OK if
> +VIRTIO_F_EARLY_CAP_ADMIN_CMD is negotiated.
>  
>  \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
> @@ -879,6 +883,13 @@ \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_EARLY_CAP_ADMIN_CMD(42)] This feature indicates that the device
> +   exposes an administration command interface which is accessible to the driver
> +   before the driver indicates DRIVER_OK device status. When this feature is
> +   negotiated, once the the administration commands interface is configured, it can be
> +   used by the driver to issue administration commands related to device and driver
> +   capabilities.
> +
>  \end{description}
>  
>  \drivernormative{\section}{Reserved Feature Bits}{Reserved Feature Bits}
> diff --git a/transport-pci.tex b/transport-pci.tex
> index 95b08b8..a612b34 100644
> --- a/transport-pci.tex
> +++ b/transport-pci.tex
> @@ -495,6 +495,12 @@ \subsubsection{Common configuration structure layout}\label{sec:Virtio Transport
>  to ensure that indices of valid admin queues fit into
>  a 16 bit range beyond all other virtqueues.
>  
> +If VIRTIO_F_ADMIN_VQ and VIRTIO_F_EARLY_ADMIN_CMD has been negotiated,
> +the device MUST support administration commands through the
> +administration virtqueue identified by the \field{admin_queue_index}
> +after the administration virtqueue is enabled and
> +before the driver sets DRIVER_OK to the \field{device_status}.
> +
>  \drivernormative{\paragraph}{Common configuration structure layout}{Virtio Transport Options / Virtio Over PCI Bus / PCI Device Layout / Common configuration structure layout}
>  
>  The driver MUST NOT write to \field{device_feature}, \field{num_queues},
> -- 
> 2.34.1