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

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

[Parav Pandit]

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>
---
 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

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

[Michael S. Tsirkin]

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

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

[Parav Pandit]

> From: Michael S. Tsirkin <mst@redhat.com>
> Sent: Wednesday, September 11, 2024 12:02 AM
> 
> 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 don't see any motivation to do before feature bit.
Never before features ok, feature negotiation is two ways communication that indicates device, that queue notification is coming.

> 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.
> 
We discussed before that pci-sig discourages piling up vendor specific caps.
So better to avoid that.
An alternative would be to bump up the pci revision_id, which would align to pci-sig and its known early enough.
Though it does not take away the feature bit because device preparation is needed to setup early admin commands.
And feature bit can be a good hint to inform that.
Only revision_id (from device to driver) is not good enough as there can be large sw stack which may never use it and no point in enabling this in the device without driver using it.
So feature bit proposed here is self-contained, and its early enough.

> 
> 
> 
> 
> > ---
> >  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

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

[Michael S. Tsirkin]

On Tue, Sep 10, 2024 at 07:01:08PM +0000, Parav Pandit wrote:
> 
> 
> > From: Michael S. Tsirkin <mst@redhat.com>
> > Sent: Wednesday, September 11, 2024 12:02 AM
> > 
> > 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 don't see any motivation to do before feature bit.

I do. And I keep saying, we need to be proactive, address more than one
problem with each feature and try to predict what will be needed,
otherwise it will be a mess of a million conflicting features.

> Never before features ok, feature negotiation is two ways communication that indicates device, that queue notification is coming.
> 
> > 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.
> > 
> We discussed before that pci-sig discourages piling up vendor specific caps.
> So better to avoid that.

Well, that's a separate issue. Actually pci endpoint guys already want
a way to access device without messing up with capabilities.
So we'll likely create a way to put capabilities in a BAR.


> An alternative would be to bump up the pci revision_id, which would align to pci-sig and its known early enough.
> Though it does not take away the feature bit because device preparation is needed to setup early admin commands.
> And feature bit can be a good hint to inform that.
> Only revision_id (from device to driver) is not good enough as there can be large sw stack which may never use it and no point in enabling this in the device without driver using it.
> So feature bit proposed here is self-contained, and its early enough.


I don't really get what this revision_id discussion is about.

> > 
> > 
> > 
> > 
> > > ---
> > >  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

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

[Parav Pandit]

> From: Michael S. Tsirkin <mst@redhat.com>
> Sent: Wednesday, September 11, 2024 2:06 AM
> 
> On Tue, Sep 10, 2024 at 07:01:08PM +0000, Parav Pandit wrote:
> >
> >
> > > From: Michael S. Tsirkin <mst@redhat.com>
> > > Sent: Wednesday, September 11, 2024 12:02 AM
> > >
> > > 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

Also if you can explain what exactly is the mess, if there is one?

> > > want it before features ok, and we will keep piling up hacks.
> > I don't see any motivation to do before feature bit.
> 
> I do. 
Can you please share the motivation, why one wants to enable the AQ before features_ok?
And how would it configure all the basic things to operate a AQ before, for example, PACKED, VERSION, PLATFORM?

If the motivation to negotiate device specific features via admin command, then that is still possible with current proposal.

> And I keep saying, we need to be proactive, address more than one
> problem with each feature and try to predict what will be needed,
> otherwise it will be a mess of a million conflicting features.
> 
Sure. make sense.

> > Never before features ok, feature negotiation is two ways communication
> that indicates device, that queue notification is coming.
> >
> > > 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.
> > >
> > We discussed before that pci-sig discourages piling up vendor specific caps.
> > So better to avoid that.
> 
> Well, that's a separate issue. Actually pci endpoint guys already want
> a way to access device without messing up with capabilities.
> So we'll likely create a way to put capabilities in a BAR.
> 
So lets take the base line design requirement and see what more options do we have.

a. do not extend pci capability
b. utilize the current q config registers
c. have bidirectional negotiation if driver will use the AQ early enough or not.

With that what do you propose?
A new register in the BAR instead of feature bits?
How does the driver tell the device after reset and before features ok?

> 
> > An alternative would be to bump up the pci revision_id, which would align
> to pci-sig and its known early enough.
> > Though it does not take away the feature bit because device preparation is
> needed to setup early admin commands.
> > And feature bit can be a good hint to inform that.
> > Only revision_id (from device to driver) is not good enough as there can be
> large sw stack which may never use it and no point in enabling this in the
> device without driver using it.
> > So feature bit proposed here is self-contained, and its early enough.
> 
> 
> I don't really get what this revision_id discussion is about.
>
I will pause explaining the issue with rev_id for a moment and see what is the efficient way than proposed?
 
> > >
> > >
> > >
> > >
> > > > ---
> > > >  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

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

[Zhu Lingshan]

On 9/11/2024 11:37 AM, Parav Pandit wrote:
>
>> From: Michael S. Tsirkin <mst@redhat.com>
>> Sent: Wednesday, September 11, 2024 2:06 AM
>>
>> On Tue, Sep 10, 2024 at 07:01:08PM +0000, Parav Pandit wrote:
>>>
>>>> From: Michael S. Tsirkin <mst@redhat.com>
>>>> Sent: Wednesday, September 11, 2024 12:02 AM
>>>>
>>>> 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
> Also if you can explain what exactly is the mess, if there is one?
As far as I can see, the spec says:
Set the DRIVER_OK status bit. At this point the device is “live”.

And no vqs are active before DRIVER_OK. How can the device assume that
the driver is ready before DRIVER_OK? Why admin vq is special?
The common config space is not enough to init a device?
If you see a gap, how about fix it in the common cfg?

Thanks

>
>>>> want it before features ok, and we will keep piling up hacks.
>>> I don't see any motivation to do before feature bit.
>> I do. 
> Can you please share the motivation, why one wants to enable the AQ before features_ok?
> And how would it configure all the basic things to operate a AQ before, for example, PACKED, VERSION, PLATFORM?
>
> If the motivation to negotiate device specific features via admin command, then that is still possible with current proposal.
>
>> And I keep saying, we need to be proactive, address more than one
>> problem with each feature and try to predict what will be needed,
>> otherwise it will be a mess of a million conflicting features.
>>
> Sure. make sense.
>
>>> Never before features ok, feature negotiation is two ways communication
>> that indicates device, that queue notification is coming.
>>>> 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.
>>>>
>>> We discussed before that pci-sig discourages piling up vendor specific caps.
>>> So better to avoid that.
>> Well, that's a separate issue. Actually pci endpoint guys already want
>> a way to access device without messing up with capabilities.
>> So we'll likely create a way to put capabilities in a BAR.
>>
> So lets take the base line design requirement and see what more options do we have.
>
> a. do not extend pci capability
> b. utilize the current q config registers
> c. have bidirectional negotiation if driver will use the AQ early enough or not.
>
> With that what do you propose?
> A new register in the BAR instead of feature bits?
> How does the driver tell the device after reset and before features ok?
>
>>> An alternative would be to bump up the pci revision_id, which would align
>> to pci-sig and its known early enough.
>>> Though it does not take away the feature bit because device preparation is
>> needed to setup early admin commands.
>>> And feature bit can be a good hint to inform that.
>>> Only revision_id (from device to driver) is not good enough as there can be
>> large sw stack which may never use it and no point in enabling this in the
>> device without driver using it.
>>> So feature bit proposed here is self-contained, and its early enough.
>>
>> I don't really get what this revision_id discussion is about.
>>
> I will pause explaining the issue with rev_id for a moment and see what is the efficient way than proposed?
>  
>>>>
>>>>
>>>>
>>>>> ---
>>>>>  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
>

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

[Parav Pandit]

> From: Zhu Lingshan <lingshan.zhu@amd.com>
> Sent: Wednesday, September 11, 2024 9:30 AM
> To: Parav Pandit <parav@nvidia.com>; Michael S. Tsirkin <mst@redhat.com>
> Cc: virtio-comment@lists.linux.dev; cohuck@redhat.com; Shahaf Shuler
> <shahafs@nvidia.com>
> Subject: Re: [PATCH] admin: Enable driver to send admin commands before
> DRIVER_OK
> 
> 
> 
> On 9/11/2024 11:37 AM, Parav Pandit wrote:
> >
> >> From: Michael S. Tsirkin <mst@redhat.com>
> >> Sent: Wednesday, September 11, 2024 2:06 AM
> >>
> >> On Tue, Sep 10, 2024 at 07:01:08PM +0000, Parav Pandit wrote:
> >>>
> >>>> From: Michael S. Tsirkin <mst@redhat.com>
> >>>> Sent: Wednesday, September 11, 2024 12:02 AM
> >>>>
> >>>> 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
> > Also if you can explain what exactly is the mess, if there is one?
> As far as I can see, the spec says:
> Set the DRIVER_OK status bit. At this point the device is “live”.
> 
This patch upgrades this detail. Please read through.

> And no vqs are active before DRIVER_OK. How can the device assume that the
> driver is ready before DRIVER_OK? 
Device does not assume.
When the feature described in this patch is negotiated, device knows that AQ will be live before DRIVER_OK.

> Why admin vq is special?
Because it is used to do fundamental device and driver capabilities discovery.
Please read the commit log that describes the motivation.

> The common config space is not enough to init a device?
No.
> If you see a gap, how about fix it in the common cfg?
Admin command is the interface do perform administration work.

> 
> Thanks
> 
> >
> >>>> want it before features ok, and we will keep piling up hacks.
> >>> I don't see any motivation to do before feature bit.
> >> I do.
> > Can you please share the motivation, why one wants to enable the AQ
> before features_ok?
> > And how would it configure all the basic things to operate a AQ before, for
> example, PACKED, VERSION, PLATFORM?
> >
> > If the motivation to negotiate device specific features via admin command,
> then that is still possible with current proposal.
> >
> >> And I keep saying, we need to be proactive, address more than one
> >> problem with each feature and try to predict what will be needed,
> >> otherwise it will be a mess of a million conflicting features.
> >>
> > Sure. make sense.
> >
> >>> Never before features ok, feature negotiation is two ways
> >>> communication
> >> that indicates device, that queue notification is coming.
> >>>> 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.
> >>>>
> >>> We discussed before that pci-sig discourages piling up vendor specific
> caps.
> >>> So better to avoid that.
> >> Well, that's a separate issue. Actually pci endpoint guys already
> >> want a way to access device without messing up with capabilities.
> >> So we'll likely create a way to put capabilities in a BAR.
> >>
> > So lets take the base line design requirement and see what more options do
> we have.
> >
> > a. do not extend pci capability
> > b. utilize the current q config registers c. have bidirectional
> > negotiation if driver will use the AQ early enough or not.
> >
> > With that what do you propose?
> > A new register in the BAR instead of feature bits?
> > How does the driver tell the device after reset and before features ok?
> >
> >>> An alternative would be to bump up the pci revision_id, which would
> >>> align
> >> to pci-sig and its known early enough.
> >>> Though it does not take away the feature bit because device
> >>> preparation is
> >> needed to setup early admin commands.
> >>> And feature bit can be a good hint to inform that.
> >>> Only revision_id (from device to driver) is not good enough as there
> >>> can be
> >> large sw stack which may never use it and no point in enabling this
> >> in the device without driver using it.
> >>> So feature bit proposed here is self-contained, and its early enough.
> >>
> >> I don't really get what this revision_id discussion is about.
> >>
> > I will pause explaining the issue with rev_id for a moment and see what is the
> efficient way than proposed?
> >
> >>>>
> >>>>
> >>>>
> >>>>> ---
> >>>>>  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
> >

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

[Jason Wang]

On Wed, Sep 11, 2024 at 12:12 PM Parav Pandit <parav@nvidia.com> wrote:
>
>
> > From: Zhu Lingshan <lingshan.zhu@amd.com>
> > Sent: Wednesday, September 11, 2024 9:30 AM
> > To: Parav Pandit <parav@nvidia.com>; Michael S. Tsirkin <mst@redhat.com>
> > Cc: virtio-comment@lists.linux.dev; cohuck@redhat.com; Shahaf Shuler
> > <shahafs@nvidia.com>
> > Subject: Re: [PATCH] admin: Enable driver to send admin commands before
> > DRIVER_OK
> >
> >
> >
> > On 9/11/2024 11:37 AM, Parav Pandit wrote:
> > >
> > >> From: Michael S. Tsirkin <mst@redhat.com>
> > >> Sent: Wednesday, September 11, 2024 2:06 AM
> > >>
> > >> On Tue, Sep 10, 2024 at 07:01:08PM +0000, Parav Pandit wrote:
> > >>>
> > >>>> From: Michael S. Tsirkin <mst@redhat.com>
> > >>>> Sent: Wednesday, September 11, 2024 12:02 AM
> > >>>>
> > >>>> 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
> > > Also if you can explain what exactly is the mess, if there is one?
> > As far as I can see, the spec says:
> > Set the DRIVER_OK status bit. At this point the device is “live”.
> >
> This patch upgrades this detail. Please read through.
>
> > And no vqs are active before DRIVER_OK. How can the device assume that the
> > driver is ready before DRIVER_OK?
> Device does not assume.
> When the feature described in this patch is negotiated, device knows that AQ will be live before DRIVER_OK.
>
> > Why admin vq is special?
> Because it is used to do fundamental device and driver capabilities discovery.

Then you need another transport or a control device.

Thanks

> Please read the commit log that describes the motivation.
>
> > The common config space is not enough to init a device?
> No.
> > If you see a gap, how about fix it in the common cfg?
> Admin command is the interface do perform administration work.
>
> >
> > Thanks
> >
> > >
> > >>>> want it before features ok, and we will keep piling up hacks.
> > >>> I don't see any motivation to do before feature bit.
> > >> I do.
> > > Can you please share the motivation, why one wants to enable the AQ
> > before features_ok?
> > > And how would it configure all the basic things to operate a AQ before, for
> > example, PACKED, VERSION, PLATFORM?
> > >
> > > If the motivation to negotiate device specific features via admin command,
> > then that is still possible with current proposal.
> > >
> > >> And I keep saying, we need to be proactive, address more than one
> > >> problem with each feature and try to predict what will be needed,
> > >> otherwise it will be a mess of a million conflicting features.
> > >>
> > > Sure. make sense.
> > >
> > >>> Never before features ok, feature negotiation is two ways
> > >>> communication
> > >> that indicates device, that queue notification is coming.
> > >>>> 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.
> > >>>>
> > >>> We discussed before that pci-sig discourages piling up vendor specific
> > caps.
> > >>> So better to avoid that.
> > >> Well, that's a separate issue. Actually pci endpoint guys already
> > >> want a way to access device without messing up with capabilities.
> > >> So we'll likely create a way to put capabilities in a BAR.
> > >>
> > > So lets take the base line design requirement and see what more options do
> > we have.
> > >
> > > a. do not extend pci capability
> > > b. utilize the current q config registers c. have bidirectional
> > > negotiation if driver will use the AQ early enough or not.
> > >
> > > With that what do you propose?
> > > A new register in the BAR instead of feature bits?
> > > How does the driver tell the device after reset and before features ok?
> > >
> > >>> An alternative would be to bump up the pci revision_id, which would
> > >>> align
> > >> to pci-sig and its known early enough.
> > >>> Though it does not take away the feature bit because device
> > >>> preparation is
> > >> needed to setup early admin commands.
> > >>> And feature bit can be a good hint to inform that.
> > >>> Only revision_id (from device to driver) is not good enough as there
> > >>> can be
> > >> large sw stack which may never use it and no point in enabling this
> > >> in the device without driver using it.
> > >>> So feature bit proposed here is self-contained, and its early enough.
> > >>
> > >> I don't really get what this revision_id discussion is about.
> > >>
> > > I will pause explaining the issue with rev_id for a moment and see what is the
> > efficient way than proposed?
> > >
> > >>>>
> > >>>>
> > >>>>
> > >>>>> ---
> > >>>>>  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
> > >
>

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

[Parav Pandit]

> From: Jason Wang <jasowang@redhat.com>
> Sent: Wednesday, September 11, 2024 10:08 AM
> To: Parav Pandit <parav@nvidia.com>
> Cc: Zhu Lingshan <lingshan.zhu@amd.com>; Michael S. Tsirkin
> <mst@redhat.com>; virtio-comment@lists.linux.dev; cohuck@redhat.com;
> Shahaf Shuler <shahafs@nvidia.com>
> Subject: Re: [PATCH] admin: Enable driver to send admin commands before
> DRIVER_OK
> 
> On Wed, Sep 11, 2024 at 12:12 PM Parav Pandit <parav@nvidia.com> wrote:
> >
> >
> > > From: Zhu Lingshan <lingshan.zhu@amd.com>
> > > Sent: Wednesday, September 11, 2024 9:30 AM
> > > To: Parav Pandit <parav@nvidia.com>; Michael S. Tsirkin
> > > <mst@redhat.com>
> > > Cc: virtio-comment@lists.linux.dev; cohuck@redhat.com; Shahaf Shuler
> > > <shahafs@nvidia.com>
> > > Subject: Re: [PATCH] admin: Enable driver to send admin commands
> > > before DRIVER_OK
> > >
> > >
> > >
> > > On 9/11/2024 11:37 AM, Parav Pandit wrote:
> > > >
> > > >> From: Michael S. Tsirkin <mst@redhat.com>
> > > >> Sent: Wednesday, September 11, 2024 2:06 AM
> > > >>
> > > >> On Tue, Sep 10, 2024 at 07:01:08PM +0000, Parav Pandit wrote:
> > > >>>
> > > >>>> From: Michael S. Tsirkin <mst@redhat.com>
> > > >>>> Sent: Wednesday, September 11, 2024 12:02 AM
> > > >>>>
> > > >>>> 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
> > > > Also if you can explain what exactly is the mess, if there is one?
> > > As far as I can see, the spec says:
> > > Set the DRIVER_OK status bit. At this point the device is “live”.
> > >
> > This patch upgrades this detail. Please read through.
> >
> > > And no vqs are active before DRIVER_OK. How can the device assume
> > > that the driver is ready before DRIVER_OK?
> > Device does not assume.
> > When the feature described in this patch is negotiated, device knows that AQ
> will be live before DRIVER_OK.
> >
> > > Why admin vq is special?
> > Because it is used to do fundamental device and driver capabilities discovery.
> 
> Then you need another transport or a control device.
I don’t think so.
Please explain the motivation to complicate it.

> 
> Thanks
> 
> > Please read the commit log that describes the motivation.
> >
> > > The common config space is not enough to init a device?
> > No.
> > > If you see a gap, how about fix it in the common cfg?
> > Admin command is the interface do perform administration work.
> >
> > >
> > > Thanks
> > >
> > > >
> > > >>>> want it before features ok, and we will keep piling up hacks.
> > > >>> I don't see any motivation to do before feature bit.
> > > >> I do.
> > > > Can you please share the motivation, why one wants to enable the
> > > > AQ
> > > before features_ok?
> > > > And how would it configure all the basic things to operate a AQ
> > > > before, for
> > > example, PACKED, VERSION, PLATFORM?
> > > >
> > > > If the motivation to negotiate device specific features via admin
> > > > command,
> > > then that is still possible with current proposal.
> > > >
> > > >> And I keep saying, we need to be proactive, address more than one
> > > >> problem with each feature and try to predict what will be needed,
> > > >> otherwise it will be a mess of a million conflicting features.
> > > >>
> > > > Sure. make sense.
> > > >
> > > >>> Never before features ok, feature negotiation is two ways
> > > >>> communication
> > > >> that indicates device, that queue notification is coming.
> > > >>>> 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.
> > > >>>>
> > > >>> We discussed before that pci-sig discourages piling up vendor
> > > >>> specific
> > > caps.
> > > >>> So better to avoid that.
> > > >> Well, that's a separate issue. Actually pci endpoint guys already
> > > >> want a way to access device without messing up with capabilities.
> > > >> So we'll likely create a way to put capabilities in a BAR.
> > > >>
> > > > So lets take the base line design requirement and see what more
> > > > options do
> > > we have.
> > > >
> > > > a. do not extend pci capability
> > > > b. utilize the current q config registers c. have bidirectional
> > > > negotiation if driver will use the AQ early enough or not.
> > > >
> > > > With that what do you propose?
> > > > A new register in the BAR instead of feature bits?
> > > > How does the driver tell the device after reset and before features ok?
> > > >
> > > >>> An alternative would be to bump up the pci revision_id, which
> > > >>> would align
> > > >> to pci-sig and its known early enough.
> > > >>> Though it does not take away the feature bit because device
> > > >>> preparation is
> > > >> needed to setup early admin commands.
> > > >>> And feature bit can be a good hint to inform that.
> > > >>> Only revision_id (from device to driver) is not good enough as
> > > >>> there can be
> > > >> large sw stack which may never use it and no point in enabling
> > > >> this in the device without driver using it.
> > > >>> So feature bit proposed here is self-contained, and its early enough.
> > > >>
> > > >> I don't really get what this revision_id discussion is about.
> > > >>
> > > > I will pause explaining the issue with rev_id for a moment and see
> > > > what is the
> > > efficient way than proposed?
> > > >
> > > >>>>
> > > >>>>
> > > >>>>
> > > >>>>> ---
> > > >>>>>  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
> > > >
> >

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

[Zhu Lingshan]

On 9/11/2024 12:38 PM, Jason Wang wrote:
> On Wed, Sep 11, 2024 at 12:12 PM Parav Pandit <parav@nvidia.com> wrote:
>>
>>> From: Zhu Lingshan <lingshan.zhu@amd.com>
>>> Sent: Wednesday, September 11, 2024 9:30 AM
>>> To: Parav Pandit <parav@nvidia.com>; Michael S. Tsirkin <mst@redhat.com>
>>> Cc: virtio-comment@lists.linux.dev; cohuck@redhat.com; Shahaf Shuler
>>> <shahafs@nvidia.com>
>>> Subject: Re: [PATCH] admin: Enable driver to send admin commands before
>>> DRIVER_OK
>>>
>>>
>>>
>>> On 9/11/2024 11:37 AM, Parav Pandit wrote:
>>>>> From: Michael S. Tsirkin <mst@redhat.com>
>>>>> Sent: Wednesday, September 11, 2024 2:06 AM
>>>>>
>>>>> On Tue, Sep 10, 2024 at 07:01:08PM +0000, Parav Pandit wrote:
>>>>>>> From: Michael S. Tsirkin <mst@redhat.com>
>>>>>>> Sent: Wednesday, September 11, 2024 12:02 AM
>>>>>>>
>>>>>>> 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
>>>> Also if you can explain what exactly is the mess, if there is one?
>>> As far as I can see, the spec says:
>>> Set the DRIVER_OK status bit. At this point the device is “live”.
>>>
>> This patch upgrades this detail. Please read through.
>>
>>> And no vqs are active before DRIVER_OK. How can the device assume that the
>>> driver is ready before DRIVER_OK?
>> Device does not assume.
>> When the feature described in this patch is negotiated, device knows that AQ will be live before DRIVER_OK.
>>
>>> Why admin vq is special?
>> Because it is used to do fundamental device and driver capabilities discovery.
> Then you need another transport or a control device.
So Maybe it is a good chance for me to continue the "transport vq" work?
>
> Thanks
>
>> Please read the commit log that describes the motivation.
>>
>>> The common config space is not enough to init a device?
>> No.
>>> If you see a gap, how about fix it in the common cfg?
>> Admin command is the interface do perform administration work.
>>
>>> Thanks
>>>
>>>>>>> want it before features ok, and we will keep piling up hacks.
>>>>>> I don't see any motivation to do before feature bit.
>>>>> I do.
>>>> Can you please share the motivation, why one wants to enable the AQ
>>> before features_ok?
>>>> And how would it configure all the basic things to operate a AQ before, for
>>> example, PACKED, VERSION, PLATFORM?
>>>> If the motivation to negotiate device specific features via admin command,
>>> then that is still possible with current proposal.
>>>>> And I keep saying, we need to be proactive, address more than one
>>>>> problem with each feature and try to predict what will be needed,
>>>>> otherwise it will be a mess of a million conflicting features.
>>>>>
>>>> Sure. make sense.
>>>>
>>>>>> Never before features ok, feature negotiation is two ways
>>>>>> communication
>>>>> that indicates device, that queue notification is coming.
>>>>>>> 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.
>>>>>>>
>>>>>> We discussed before that pci-sig discourages piling up vendor specific
>>> caps.
>>>>>> So better to avoid that.
>>>>> Well, that's a separate issue. Actually pci endpoint guys already
>>>>> want a way to access device without messing up with capabilities.
>>>>> So we'll likely create a way to put capabilities in a BAR.
>>>>>
>>>> So lets take the base line design requirement and see what more options do
>>> we have.
>>>> a. do not extend pci capability
>>>> b. utilize the current q config registers c. have bidirectional
>>>> negotiation if driver will use the AQ early enough or not.
>>>>
>>>> With that what do you propose?
>>>> A new register in the BAR instead of feature bits?
>>>> How does the driver tell the device after reset and before features ok?
>>>>
>>>>>> An alternative would be to bump up the pci revision_id, which would
>>>>>> align
>>>>> to pci-sig and its known early enough.
>>>>>> Though it does not take away the feature bit because device
>>>>>> preparation is
>>>>> needed to setup early admin commands.
>>>>>> And feature bit can be a good hint to inform that.
>>>>>> Only revision_id (from device to driver) is not good enough as there
>>>>>> can be
>>>>> large sw stack which may never use it and no point in enabling this
>>>>> in the device without driver using it.
>>>>>> So feature bit proposed here is self-contained, and its early enough.
>>>>> I don't really get what this revision_id discussion is about.
>>>>>
>>>> I will pause explaining the issue with rev_id for a moment and see what is the
>>> efficient way than proposed?
>>>>>>>
>>>>>>>
>>>>>>>> ---
>>>>>>>>  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

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

[Parav Pandit]

> From: Zhu Lingshan <lingshan.zhu@amd.com>
> Sent: Wednesday, September 11, 2024 10:12 AM
> 
> On 9/11/2024 12:38 PM, Jason Wang wrote:
> > On Wed, Sep 11, 2024 at 12:12 PM Parav Pandit <parav@nvidia.com>
> wrote:
> >>
> >>> From: Zhu Lingshan <lingshan.zhu@amd.com>
> >>> Sent: Wednesday, September 11, 2024 9:30 AM
> >>> To: Parav Pandit <parav@nvidia.com>; Michael S. Tsirkin
> >>> <mst@redhat.com>
> >>> Cc: virtio-comment@lists.linux.dev; cohuck@redhat.com; Shahaf Shuler
> >>> <shahafs@nvidia.com>
> >>> Subject: Re: [PATCH] admin: Enable driver to send admin commands
> >>> before DRIVER_OK
> >>>
> >>>
> >>>
> >>> On 9/11/2024 11:37 AM, Parav Pandit wrote:
> >>>>> From: Michael S. Tsirkin <mst@redhat.com>
> >>>>> Sent: Wednesday, September 11, 2024 2:06 AM
> >>>>>
> >>>>> On Tue, Sep 10, 2024 at 07:01:08PM +0000, Parav Pandit wrote:
> >>>>>>> From: Michael S. Tsirkin <mst@redhat.com>
> >>>>>>> Sent: Wednesday, September 11, 2024 12:02 AM
> >>>>>>>
> >>>>>>> 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
> >>>> Also if you can explain what exactly is the mess, if there is one?
> >>> As far as I can see, the spec says:
> >>> Set the DRIVER_OK status bit. At this point the device is “live”.
> >>>
> >> This patch upgrades this detail. Please read through.
> >>
> >>> And no vqs are active before DRIVER_OK. How can the device assume
> >>> that the driver is ready before DRIVER_OK?
> >> Device does not assume.
> >> When the feature described in this patch is negotiated, device knows that
> AQ will be live before DRIVER_OK.
> >>
> >>> Why admin vq is special?
> >> Because it is used to do fundamental device and driver capabilities
> discovery.
> > Then you need another transport or a control device.
> So Maybe it is a good chance for me to continue the "transport vq" work?
Nop. 
AQ has delivered what all was required uniformly for sw based device, PCI PF, PCI VFs.
Lets first understand the motivation of the complication from Jason.

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

[Jason Wang]

On Wed, Sep 11, 2024 at 12:45 PM Parav Pandit <parav@nvidia.com> wrote:
>
>
> > From: Zhu Lingshan <lingshan.zhu@amd.com>
> > Sent: Wednesday, September 11, 2024 10:12 AM
> >
> > On 9/11/2024 12:38 PM, Jason Wang wrote:
> > > On Wed, Sep 11, 2024 at 12:12 PM Parav Pandit <parav@nvidia.com>
> > wrote:
> > >>
> > >>> From: Zhu Lingshan <lingshan.zhu@amd.com>
> > >>> Sent: Wednesday, September 11, 2024 9:30 AM
> > >>> To: Parav Pandit <parav@nvidia.com>; Michael S. Tsirkin
> > >>> <mst@redhat.com>
> > >>> Cc: virtio-comment@lists.linux.dev; cohuck@redhat.com; Shahaf Shuler
> > >>> <shahafs@nvidia.com>
> > >>> Subject: Re: [PATCH] admin: Enable driver to send admin commands
> > >>> before DRIVER_OK
> > >>>
> > >>>
> > >>>
> > >>> On 9/11/2024 11:37 AM, Parav Pandit wrote:
> > >>>>> From: Michael S. Tsirkin <mst@redhat.com>
> > >>>>> Sent: Wednesday, September 11, 2024 2:06 AM
> > >>>>>
> > >>>>> On Tue, Sep 10, 2024 at 07:01:08PM +0000, Parav Pandit wrote:
> > >>>>>>> From: Michael S. Tsirkin <mst@redhat.com>
> > >>>>>>> Sent: Wednesday, September 11, 2024 12:02 AM
> > >>>>>>>
> > >>>>>>> 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
> > >>>> Also if you can explain what exactly is the mess, if there is one?
> > >>> As far as I can see, the spec says:
> > >>> Set the DRIVER_OK status bit. At this point the device is “live”.
> > >>>
> > >> This patch upgrades this detail. Please read through.
> > >>
> > >>> And no vqs are active before DRIVER_OK. How can the device assume
> > >>> that the driver is ready before DRIVER_OK?
> > >> Device does not assume.
> > >> When the feature described in this patch is negotiated, device knows that
> > AQ will be live before DRIVER_OK.
> > >>
> > >>> Why admin vq is special?
> > >> Because it is used to do fundamental device and driver capabilities
> > discovery.
> > > Then you need another transport or a control device.
> > So Maybe it is a good chance for me to continue the "transport vq" work?
> Nop.
> AQ has delivered what all was required uniformly for sw based device, PCI PF, PCI VFs.
> Lets first understand the motivation of the complication from Jason.

It's not a complication but a simplification because "fundamental
device and driver capabilities discovery" has already been the charge
of the transport.

Thanks

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

[Parav Pandit]

> From: Jason Wang <jasowang@redhat.com>
> Sent: Wednesday, September 11, 2024 10:34 AM
> 
> On Wed, Sep 11, 2024 at 12:45 PM Parav Pandit <parav@nvidia.com> wrote:
> >
> >
> > > From: Zhu Lingshan <lingshan.zhu@amd.com>
> > > Sent: Wednesday, September 11, 2024 10:12 AM
> > >
> > > On 9/11/2024 12:38 PM, Jason Wang wrote:
> > > > On Wed, Sep 11, 2024 at 12:12 PM Parav Pandit <parav@nvidia.com>
> > > wrote:
> > > >>
> > > >>> From: Zhu Lingshan <lingshan.zhu@amd.com>
> > > >>> Sent: Wednesday, September 11, 2024 9:30 AM
> > > >>> To: Parav Pandit <parav@nvidia.com>; Michael S. Tsirkin
> > > >>> <mst@redhat.com>
> > > >>> Cc: virtio-comment@lists.linux.dev; cohuck@redhat.com; Shahaf
> > > >>> Shuler <shahafs@nvidia.com>
> > > >>> Subject: Re: [PATCH] admin: Enable driver to send admin commands
> > > >>> before DRIVER_OK
> > > >>>
> > > >>>
> > > >>>
> > > >>> On 9/11/2024 11:37 AM, Parav Pandit wrote:
> > > >>>>> From: Michael S. Tsirkin <mst@redhat.com>
> > > >>>>> Sent: Wednesday, September 11, 2024 2:06 AM
> > > >>>>>
> > > >>>>> On Tue, Sep 10, 2024 at 07:01:08PM +0000, Parav Pandit wrote:
> > > >>>>>>> From: Michael S. Tsirkin <mst@redhat.com>
> > > >>>>>>> Sent: Wednesday, September 11, 2024 12:02 AM
> > > >>>>>>>
> > > >>>>>>> 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
> > > >>>> Also if you can explain what exactly is the mess, if there is one?
> > > >>> As far as I can see, the spec says:
> > > >>> Set the DRIVER_OK status bit. At this point the device is “live”.
> > > >>>
> > > >> This patch upgrades this detail. Please read through.
> > > >>
> > > >>> And no vqs are active before DRIVER_OK. How can the device
> > > >>> assume that the driver is ready before DRIVER_OK?
> > > >> Device does not assume.
> > > >> When the feature described in this patch is negotiated, device
> > > >> knows that
> > > AQ will be live before DRIVER_OK.
> > > >>
> > > >>> Why admin vq is special?
> > > >> Because it is used to do fundamental device and driver
> > > >> capabilities
> > > discovery.
> > > > Then you need another transport or a control device.
> > > So Maybe it is a good chance for me to continue the "transport vq" work?
> > Nop.
> > AQ has delivered what all was required uniformly for sw based device, PCI
> PF, PCI VFs.
> > Lets first understand the motivation of the complication from Jason.
> 
> It's not a complication but a simplification because "fundamental device and
> driver capabilities discovery" has already been the charge of the transport.
This does not explain the motivation at all.
Admin command interface is part of it already to discover the capabilities both direction.

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

[Jason Wang]

On Wed, Sep 11, 2024 at 12:42 PM Zhu Lingshan <lingshan.zhu@amd.com> wrote:
>
>
>
> On 9/11/2024 12:38 PM, Jason Wang wrote:
> > On Wed, Sep 11, 2024 at 12:12 PM Parav Pandit <parav@nvidia.com> wrote:
> >>
> >>> From: Zhu Lingshan <lingshan.zhu@amd.com>
> >>> Sent: Wednesday, September 11, 2024 9:30 AM
> >>> To: Parav Pandit <parav@nvidia.com>; Michael S. Tsirkin <mst@redhat.com>
> >>> Cc: virtio-comment@lists.linux.dev; cohuck@redhat.com; Shahaf Shuler
> >>> <shahafs@nvidia.com>
> >>> Subject: Re: [PATCH] admin: Enable driver to send admin commands before
> >>> DRIVER_OK
> >>>
> >>>
> >>>
> >>> On 9/11/2024 11:37 AM, Parav Pandit wrote:
> >>>>> From: Michael S. Tsirkin <mst@redhat.com>
> >>>>> Sent: Wednesday, September 11, 2024 2:06 AM
> >>>>>
> >>>>> On Tue, Sep 10, 2024 at 07:01:08PM +0000, Parav Pandit wrote:
> >>>>>>> From: Michael S. Tsirkin <mst@redhat.com>
> >>>>>>> Sent: Wednesday, September 11, 2024 12:02 AM
> >>>>>>>
> >>>>>>> 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
> >>>> Also if you can explain what exactly is the mess, if there is one?
> >>> As far as I can see, the spec says:
> >>> Set the DRIVER_OK status bit. At this point the device is “live”.
> >>>
> >> This patch upgrades this detail. Please read through.
> >>
> >>> And no vqs are active before DRIVER_OK. How can the device assume that the
> >>> driver is ready before DRIVER_OK?
> >> Device does not assume.
> >> When the feature described in this patch is negotiated, device knows that AQ will be live before DRIVER_OK.
> >>
> >>> Why admin vq is special?
> >> Because it is used to do fundamental device and driver capabilities discovery.
> > Then you need another transport or a control device.
> So Maybe it is a good chance for me to continue the "transport vq" work?

I think this could be one way.

Thanks

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

[Michael S. Tsirkin]

> So Maybe it is a good chance for me to continue the "transport vq" work?

Transport over admin commands was envisioned more or less from day one.
I think it's useful.

But what Parav is asking for seems somewhat orthogonal.



-- 
MST

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

[Zhu Lingshan]

On 9/14/2024 3:03 AM, Michael S. Tsirkin wrote:
>> So Maybe it is a good chance for me to continue the "transport vq" work?
> Transport over admin commands was envisioned more or less from day one.
> I think it's useful.
>
> But what Parav is asking for seems somewhat orthogonal.
Thanks! However, admin vq is a control plane, and we are implementing
a new transport, so IMHO an individual transport vq may make more sense.

Thanks
Zhu Lingshan
>
>
>