[virtio-comment] [PATCH v3 0/2] Support enabling virtqueue after DRIVER_OK

[virtio-comment] [PATCH v3 0/2] Support enabling virtqueue after DRIVER_OK

[Parav Pandit]

Summary:
========
This patch enables driver to create virtqueues after DRIVER_OK
status bit is set.

This patch take the inspiration from the thread [2] with credits to
Eugenio PÃrez.

Details:
========
Currently, a virtqueue must be enabled before driver has set the
DRIVER_OK status bit.

spec citation to section "Driver Requirements: Device Initialization"

"Perform device-specific setup, including discovery of virtqueues
for the device, optional per-bus setup, reading and possibly writing
the device’s virtio configuration space, and population of virtqueues."

This implies that a virtqueue must be enabled before reaching the
DRIVER_OK stage. There was no explicit mention about ability to
enable the virtqueue after DRIVER_OK stage.

In following usecases, creating a virtqueue after DRIVER_OK phase is
desired.

Use cases:
=========
1. Dynamically create aq when administrative commands to be used.
ate the net device tx/rxq when device is
   opened when deploying for a container.
   In a container, number of virtqueues to be used may be <= max queues.
3. Dynamically create flow filter queues of netdevice when
   ARFS or ethtool filters are enabled as listed in [1].
4. Dynamically create rtc functionality related read virtqueue only
   when net device when timestamping to be used.
5. When XDP program is set, one can create additional XDP specific
   queues without affecting existing queues.

Hence, This patch introduces an existing queue enable and disable
(aka reset) facility and a new feature bit to explicitly indicate such
support by the device.

With this feature, drivers can skip optional queues creation during
driver init time. For example, a Linux net device driver
can create/destroy the transmit and receive queues when net device's
ndo_open() and ndo_stop() callbacks are invoked respectively.

[1] https://lists.oasis-open.org/archives/virtio-comment/202308/msg00263.html
[2] https://lists.oasis-open.org/archives/virtio-comment/202306/msg00097.html

Patch summary:
==============
patch-1 fixes to add virtqueue reset conformance references
patch-2 adds VIRTIO_F_RING_DYNAMIC feature bit, requirements

Please review.

Fixes: https://github.com/oasis-tcs/virtio-spec/issues/177

---
changelog:
v2->v3:
- github issue link updated
v1->v2:
- fixed comments from Michael
- reduced device and driver normatives (simplified)
- replace 'a virtqueue' to 'the virtqueues'
- reworded to remove 'interested'
- avoided repeated 'device initialization text', replaced with
  DRIVER_OK status bit language
- avoided confusing text around 'MAY' in the requirements
- replaced 'the queue' to 'specific virtqueues'
- replaced queue to virtqueue
- simplified commit log to talk about msix
v0->v1:
- fixed comments from Xuan
- removed blank lines
- fixed wrong requirement link to device in driver section


Parav Pandit (2):
  conformance: Add missing virtqueue reset conformance references
  content: Support enabling virtqueue after DRIVER_OK stage

 conformance.tex |  4 ++++
 content.tex     | 49 ++++++++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 50 insertions(+), 3 deletions(-)

-- 
2.34.1


This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.

In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.

Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/

[virtio-comment] [PATCH v3 1/2] conformance: Add missing virtqueue reset conformance references

[Parav Pandit]

Add the missing references to the virtqueue reset related conformance
requirements.

Signed-off-by: Parav Pandit <parav@nvidia.com>
Reviewed-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
---
 conformance.tex | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/conformance.tex b/conformance.tex
index dc00e84..863f9c5 100644
--- a/conformance.tex
+++ b/conformance.tex
@@ -76,6 +76,7 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
 \item \ref{drivernormative:Basic Facilities of a Virtio Device / Device Reset}
 \item \ref{drivernormative:Basic Facilities of a Virtio Device / Device Configuration Space}
 \item \ref{drivernormative:Basic Facilities of a Virtio Device / Virtqueues}
+\item \ref{drivernormative:Basic Facilities of a Virtio Device / Virtqueues / Virtqueue Reset / Virtqueue Reset}
 \item \ref{drivernormative:Basic Facilities of a Virtio Device / Message Framing}
 \item \ref{drivernormative:Basic Facilities of a Virtio Device / Virtqueues / The Virtqueue Descriptor Table}
 \item \ref{drivernormative:Basic Facilities of a Virtio Device / Virtqueues / The Virtqueue Descriptor Table / Indirect Descriptors}
@@ -162,6 +163,7 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
 \item \ref{devicenormative:Basic Facilities of a Virtio Device / Feature Bits}
 \item \ref{devicenormative:Basic Facilities of a Virtio Device / Device Reset}
 \item \ref{devicenormative:Basic Facilities of a Virtio Device / Device Configuration Space}
+\item \ref{devicenormative:Basic Facilities of a Virtio Device / Virtqueues / Virtqueue Reset / Virtqueue Reset}
 \item \ref{devicenormative:Basic Facilities of a Virtio Device / Message Framing}
 \item \ref{devicenormative:Basic Facilities of a Virtio Device / Virtqueues / The Virtqueue Descriptor Table}
 \item \ref{devicenormative:Basic Facilities of a Virtio Device / Virtqueues / The Virtqueue Descriptor Table / Indirect Descriptors}
-- 
2.34.1


This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.

In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.

Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/

[virtio-comment] [PATCH v3 2/2] content: Support enabling virtqueue after DRIVER_OK stage

[Parav Pandit]

Currently, a virtqueue must be enabled before driver has set the
DRIVER_OK status bit.

spec citation to section "Driver Requirements: Device Initialization"

"Perform device-specific setup, including discovery of virtqueues
for the device, optional per-bus setup, reading and possibly writing
the device’s virtio configuration space, and population of virtqueues."

This implies that a virtqueue must be enabled before reaching the
DRIVER_OK stage. There was no explicit mention about ability to
enable the virtqueue after DRIVER_OK stage.

In following usecases, creating a virtqueue after DRIVER_OK phase is
desired.

1. Dynamically create aq when administrative commands to be used.
2. Dynamically create the net device tx/rxq when device is
   opened when deploying for a container.
   In a container, number of virtqueues to be used may be <= max queues.
3. Dynamically create flow filter queues of netdevice when
   ARFS or ethtool filters are enabled as listed in [1].
4. Dynamically create rtc functionality related read virtqueue only
   when net device when timestamping to be used.
5. When XDP program is set, one can create additional XDP specific
   queues without affecting existing queues.

Hence, This patch introduces an existing queue enable and disable
(aka reset) facility and a new feature bit to explicitly indicate such
support by the device.

With this feature, drivers can skip optional queues creation during
driver init time. For example, a Linux net device driver
can create/destroy the transmit and receive queues when net device's
ndo_open() and ndo_stop() callbacks are invoked respectively.

[1] https://lists.oasis-open.org/archives/virtio-comment/202308/msg00263.html

Fixes: https://github.com/oasis-tcs/virtio-spec/issues/177
Signed-off-by: Parav Pandit <parav@nvidia.com>
---
changelog:
v2->v3:
- github fixes tag added
v1->v2:
- fixed comments from Michael
- reduced device and driver normatives (simplified)
- replace 'a virtqueue' to 'the virtqueues'
- reworded to remove 'interested'
- avoided repeated 'device initialization text', replaced with
  DRIVER_OK status bit language
- avoided confusing text around 'MAY' in the requirements
- replaced 'the queue' to 'specific virtqueues'
- replaced queue to virtqueue
- simplified commit log to talk about msix
v0->v1:
- fixed comments from Xuan
- removed blank lines
- fixed wrong requirement link to device in driver section
---
 conformance.tex |  2 ++
 content.tex     | 49 ++++++++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 48 insertions(+), 3 deletions(-)

diff --git a/conformance.tex b/conformance.tex
index 863f9c5..75de58c 100644
--- a/conformance.tex
+++ b/conformance.tex
@@ -77,6 +77,7 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
 \item \ref{drivernormative:Basic Facilities of a Virtio Device / Device Configuration Space}
 \item \ref{drivernormative:Basic Facilities of a Virtio Device / Virtqueues}
 \item \ref{drivernormative:Basic Facilities of a Virtio Device / Virtqueues / Virtqueue Reset / Virtqueue Reset}
+\item \ref{drivernormative:Basic Facilities of a Virtio Device / Virtqueues / Dynamic Virtqueues / Enable Virtqueues}
 \item \ref{drivernormative:Basic Facilities of a Virtio Device / Message Framing}
 \item \ref{drivernormative:Basic Facilities of a Virtio Device / Virtqueues / The Virtqueue Descriptor Table}
 \item \ref{drivernormative:Basic Facilities of a Virtio Device / Virtqueues / The Virtqueue Descriptor Table / Indirect Descriptors}
@@ -164,6 +165,7 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
 \item \ref{devicenormative:Basic Facilities of a Virtio Device / Device Reset}
 \item \ref{devicenormative:Basic Facilities of a Virtio Device / Device Configuration Space}
 \item \ref{devicenormative:Basic Facilities of a Virtio Device / Virtqueues / Virtqueue Reset / Virtqueue Reset}
+\item \ref{devicenormative:Basic Facilities of a Virtio Device / Virtqueues / Dynamic Virtqueues / Enable Virtqueues}
 \item \ref{devicenormative:Basic Facilities of a Virtio Device / Message Framing}
 \item \ref{devicenormative:Basic Facilities of a Virtio Device / Virtqueues / The Virtqueue Descriptor Table}
 \item \ref{devicenormative:Basic Facilities of a Virtio Device / Virtqueues / The Virtqueue Descriptor Table / Indirect Descriptors}
diff --git a/content.tex b/content.tex
index 0a62dce..1a296a9 100644
--- a/content.tex
+++ b/content.tex
@@ -102,7 +102,7 @@ \section{Feature Bits}\label{sec:Basic Facilities of a Virtio Device / Feature B
 \item[24 to 41] Feature bits reserved for extensions to the queue and
   feature negotiation mechanisms
 
-\item[42 to 49, and 128 and above] Feature bits reserved for future extensions.
+\item[43 to 49, and 128 and above] Feature bits reserved for future extensions.
 \end{description}
 
 \begin{note}
@@ -440,6 +440,38 @@ \subsubsection{Virtqueue Re-enable}\label{sec:Basic Facilities of a Virtio Devic
 as during initial virtqueue discovery, but optionally with different
 parameters.
 
+\subsection{Dynamic Virtqueues}\label{sec:Basic Facilities of a Virtio Device / Virtqueues / Dynamic Virtqueues}
+
+When VIRTIO_F_RING_DYNAMIC is not negotiated, the driver enables the virtqueues
+during the device initialization sequence, i.e. after the device sets the
+FEATURES_OK status bit and before the driver setting the DRIVER_OK status bit.
+
+When VIRTIO_F_RING_DYNAMIC is negotiated, the driver can avoid enabling the
+virtqueues before setting the DRIVER_OK status bit; the driver can enable the
+specific virtqueues after the driver has set the DRIVER_OK status bit.
+The virtqueue enable mechanism is transport specific.
+
+\devicenormative{\paragraph}{Enable Virtqueues}{Basic Facilities of a Virtio Device / Virtqueues / Dynamic Virtqueues / Enable Virtqueues}
+
+When VIRTIO_F_RING_DYNAMIC is negotiated, the device MUST allow enabling the
+virtqueue which was not enabled during the device initialization phase
+i.e. after the device has set the FEATURES_OK status bit and before the
+driver has set the DRIVER_OK status bit.
+
+\drivernormative{\paragraph}{Enable Virtqueues}{Basic Facilities of a Virtio Device / Virtqueues / Dynamic Virtqueues / Enable Virtqueues}
+
+When VIRTIO_F_RING_DYNAMIC is negotiated,
+\begin{itemize}
+\item the driver MAY enable some or all the virtqueues after the driver has set the
+      DRIVER_OK status bit.
+
+\item the driver MAY enable some of the virtqueues or all the virtqueues or none of
+      the virtqueues before the driver has set the DRIVER_OK status bit.
+\end{itemize}
+
+When VIRTIO_F_RING_DYNAMIC is not negotiated, the driver MUST enable the
+required number of virtqueues before setting the DRIVER_OK status bit.
+
 \input{split-ring.tex}
 
 \input{packed-ring.tex}
@@ -537,7 +569,9 @@ \section{Device Initialization}\label{sec:General Initialization And Device Oper
 
 \item\label{itm:General Initialization And Device Operation / Device Initialization / Device-specific Setup} Perform device-specific setup, including discovery of virtqueues for the
    device, optional per-bus setup, reading and possibly writing the
-   device's virtio configuration space, and population of virtqueues.
+   device's virtio configuration space.
+
+\item\label{itm:General Initialization And Device Operation / Device Initialization / Virtqueue Setup} Configure and enable the virtqueues.
 
 \item\label{itm:General Initialization And Device Operation / Device Initialization / Set DRIVER-OK} Set the DRIVER_OK status bit.  At this point the device is
    ``live''.
@@ -551,6 +585,10 @@ \section{Device Initialization}\label{sec:General Initialization And Device Oper
 The driver MUST NOT send any buffer available notifications to
 the device before setting DRIVER_OK.
 
+The driver MAY skip step
+\ref{itm:General Initialization And Device Operation / Device Initialization / Virtqueue Setup}
+when driver has negotiated VIRTIO_F_RING_DYNAMIC feature.
+
 \subsection{Legacy Interface: Device Initialization}\label{sec:General Initialization And Device Operation / Device Initialization / Legacy Interface: Device Initialization}
 Legacy devices did not support the FEATURES_OK status bit, and thus did
 not have a graceful way for the device to indicate unsupported feature
@@ -569,7 +607,7 @@ \subsection{Legacy Interface: Device Initialization}\label{sec:General Initializ
 Initialization / Re-read FEATURES-OK} were omitted, and steps
 \ref{itm:General Initialization And Device Operation /
 Device Initialization / Read feature bits},
-\ref{itm:General Initialization And Device Operation / Device Initialization / Device-specific Setup} and \ref{itm:General Initialization And Device Operation / Device Initialization / Set DRIVER-OK}
+\ref{itm:General Initialization And Device Operation / Device Initialization / Device-specific Setup}, \ref{itm:General Initialization And Device Operation / Device Initialization / Virtqueue Setup} and \ref{itm:General Initialization And Device Operation / Device Initialization / Set DRIVER-OK}
 were conflated.
 
 Therefore, when using the legacy interface:
@@ -872,6 +910,11 @@ \chapter{Reserved Feature Bits}\label{sec:Reserved Feature Bits}
 	\ref{devicenormative:Basic Facilities of a Virtio Device / Feature Bits} for
 	handling features reserved for future use.
 
+  \item[VIRTIO_F_RING_DYNAMIC(42)] This feature indicates
+  that the driver can enable the virtqueues individually after the driver has
+  set the status bit of DRIVER_OK.
+  See \ref{sec:Basic Facilities of a Virtio Device / Virtqueues / Dynamic Virtqueues}.
+
 \end{description}
 
 \drivernormative{\section}{Reserved Feature Bits}{Reserved Feature Bits}
-- 
2.34.1


This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.

In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.

Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/

[virtio-comment] RE: [PATCH v3 0/2] Support enabling virtqueue after DRIVER_OK

[Parav Pandit]

Hi Michael, Cornelia,


> From: Parav Pandit <parav@nvidia.com>
> Sent: Monday, October 2, 2023 10:46 AM
> 
> Summary:
> ========
> This patch enables driver to create virtqueues after DRIVER_OK status bit is set.
> 
> This patch take the inspiration from the thread [2] with credits to Eugenio PÃrez.
> 
> Details:
> ========
> Currently, a virtqueue must be enabled before driver has set the DRIVER_OK
> status bit.
> 
> spec citation to section "Driver Requirements: Device Initialization"
> 
> "Perform device-specific setup, including discovery of virtqueues for the device,
> optional per-bus setup, reading and possibly writing the device’s virtio
> configuration space, and population of virtqueues."
> 
> This implies that a virtqueue must be enabled before reaching the DRIVER_OK
> stage. There was no explicit mention about ability to enable the virtqueue after
> DRIVER_OK stage.
> 
> In following usecases, creating a virtqueue after DRIVER_OK phase is desired.
> 
> Use cases:
> =========
> 1. Dynamically create aq when administrative commands to be used.
> ate the net device tx/rxq when device is
>    opened when deploying for a container.
>    In a container, number of virtqueues to be used may be <= max queues.
> 3. Dynamically create flow filter queues of netdevice when
>    ARFS or ethtool filters are enabled as listed in [1].
> 4. Dynamically create rtc functionality related read virtqueue only
>    when net device when timestamping to be used.
> 5. When XDP program is set, one can create additional XDP specific
>    queues without affecting existing queues.
> 
> Hence, This patch introduces an existing queue enable and disable (aka reset)
> facility and a new feature bit to explicitly indicate such support by the device.
> 
> With this feature, drivers can skip optional queues creation during driver init
> time. For example, a Linux net device driver can create/destroy the transmit and
> receive queues when net device's
> ndo_open() and ndo_stop() callbacks are invoked respectively.
> 
> [1] https://lists.oasis-open.org/archives/virtio-
> comment/202308/msg00263.html
> [2] https://lists.oasis-open.org/archives/virtio-
> comment/202306/msg00097.html
> 
> Patch summary:
> ==============
> patch-1 fixes to add virtqueue reset conformance references
> patch-2 adds VIRTIO_F_RING_DYNAMIC feature bit, requirements
> 
> Please review.
> 
> Fixes: https://github.com/oasis-tcs/virtio-spec/issues/177
> 
> ---
> changelog:
> v2->v3:
> - github issue link updated
> v1->v2:
> - fixed comments from Michael
> - reduced device and driver normatives (simplified)
> - replace 'a virtqueue' to 'the virtqueues'
> - reworded to remove 'interested'
> - avoided repeated 'device initialization text', replaced with
>   DRIVER_OK status bit language
> - avoided confusing text around 'MAY' in the requirements
> - replaced 'the queue' to 'specific virtqueues'
> - replaced queue to virtqueue
> - simplified commit log to talk about msix
> v0->v1:
> - fixed comments from Xuan
> - removed blank lines
> - fixed wrong requirement link to device in driver section
> 

I have updated v3 to link to the github issue.
Now that this small feature was up for few weeks, can you please create the ballot review?

Re: [virtio-comment] [PATCH v3 2/2] content: Support enabling virtqueue after DRIVER_OK stage

[Eugenio Perez Martin]

On Mon, Oct 2, 2023 at 7:17 AM Parav Pandit <parav@nvidia.com> wrote:
>
> Currently, a virtqueue must be enabled before driver has set the
> DRIVER_OK status bit.
>
> spec citation to section "Driver Requirements: Device Initialization"
>
> "Perform device-specific setup, including discovery of virtqueues
> for the device, optional per-bus setup, reading and possibly writing
> the device’s virtio configuration space, and population of virtqueues."
>
> This implies that a virtqueue must be enabled before reaching the
> DRIVER_OK stage. There was no explicit mention about ability to
> enable the virtqueue after DRIVER_OK stage.
>
> In following usecases, creating a virtqueue after DRIVER_OK phase is
> desired.
>
> 1. Dynamically create aq when administrative commands to be used.
> 2. Dynamically create the net device tx/rxq when device is
>    opened when deploying for a container.
>    In a container, number of virtqueues to be used may be <= max queues.
> 3. Dynamically create flow filter queues of netdevice when
>    ARFS or ethtool filters are enabled as listed in [1].
> 4. Dynamically create rtc functionality related read virtqueue only
>    when net device when timestamping to be used.
> 5. When XDP program is set, one can create additional XDP specific
>    queues without affecting existing queues.
>
> Hence, This patch introduces an existing queue enable and disable
> (aka reset) facility and a new feature bit to explicitly indicate such
> support by the device.
>
> With this feature, drivers can skip optional queues creation during
> driver init time. For example, a Linux net device driver
> can create/destroy the transmit and receive queues when net device's
> ndo_open() and ndo_stop() callbacks are invoked respectively.
>
> [1] https://lists.oasis-open.org/archives/virtio-comment/202308/msg00263.html
>
> Fixes: https://github.com/oasis-tcs/virtio-spec/issues/177

Acked-by: Eugenio Pérez <eperezma@redhat.com>

> Signed-off-by: Parav Pandit <parav@nvidia.com>
> ---
> changelog:
> v2->v3:
> - github fixes tag added
> v1->v2:
> - fixed comments from Michael
> - reduced device and driver normatives (simplified)
> - replace 'a virtqueue' to 'the virtqueues'
> - reworded to remove 'interested'
> - avoided repeated 'device initialization text', replaced with
>   DRIVER_OK status bit language
> - avoided confusing text around 'MAY' in the requirements
> - replaced 'the queue' to 'specific virtqueues'
> - replaced queue to virtqueue
> - simplified commit log to talk about msix
> v0->v1:
> - fixed comments from Xuan
> - removed blank lines
> - fixed wrong requirement link to device in driver section
> ---
>  conformance.tex |  2 ++
>  content.tex     | 49 ++++++++++++++++++++++++++++++++++++++++++++++---
>  2 files changed, 48 insertions(+), 3 deletions(-)
>
> diff --git a/conformance.tex b/conformance.tex
> index 863f9c5..75de58c 100644
> --- a/conformance.tex
> +++ b/conformance.tex
> @@ -77,6 +77,7 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
>  \item \ref{drivernormative:Basic Facilities of a Virtio Device / Device Configuration Space}
>  \item \ref{drivernormative:Basic Facilities of a Virtio Device / Virtqueues}
>  \item \ref{drivernormative:Basic Facilities of a Virtio Device / Virtqueues / Virtqueue Reset / Virtqueue Reset}
> +\item \ref{drivernormative:Basic Facilities of a Virtio Device / Virtqueues / Dynamic Virtqueues / Enable Virtqueues}
>  \item \ref{drivernormative:Basic Facilities of a Virtio Device / Message Framing}
>  \item \ref{drivernormative:Basic Facilities of a Virtio Device / Virtqueues / The Virtqueue Descriptor Table}
>  \item \ref{drivernormative:Basic Facilities of a Virtio Device / Virtqueues / The Virtqueue Descriptor Table / Indirect Descriptors}
> @@ -164,6 +165,7 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
>  \item \ref{devicenormative:Basic Facilities of a Virtio Device / Device Reset}
>  \item \ref{devicenormative:Basic Facilities of a Virtio Device / Device Configuration Space}
>  \item \ref{devicenormative:Basic Facilities of a Virtio Device / Virtqueues / Virtqueue Reset / Virtqueue Reset}
> +\item \ref{devicenormative:Basic Facilities of a Virtio Device / Virtqueues / Dynamic Virtqueues / Enable Virtqueues}
>  \item \ref{devicenormative:Basic Facilities of a Virtio Device / Message Framing}
>  \item \ref{devicenormative:Basic Facilities of a Virtio Device / Virtqueues / The Virtqueue Descriptor Table}
>  \item \ref{devicenormative:Basic Facilities of a Virtio Device / Virtqueues / The Virtqueue Descriptor Table / Indirect Descriptors}
> diff --git a/content.tex b/content.tex
> index 0a62dce..1a296a9 100644
> --- a/content.tex
> +++ b/content.tex
> @@ -102,7 +102,7 @@ \section{Feature Bits}\label{sec:Basic Facilities of a Virtio Device / Feature B
>  \item[24 to 41] Feature bits reserved for extensions to the queue and
>    feature negotiation mechanisms
>
> -\item[42 to 49, and 128 and above] Feature bits reserved for future extensions.
> +\item[43 to 49, and 128 and above] Feature bits reserved for future extensions.
>  \end{description}
>
>  \begin{note}
> @@ -440,6 +440,38 @@ \subsubsection{Virtqueue Re-enable}\label{sec:Basic Facilities of a Virtio Devic
>  as during initial virtqueue discovery, but optionally with different
>  parameters.
>
> +\subsection{Dynamic Virtqueues}\label{sec:Basic Facilities of a Virtio Device / Virtqueues / Dynamic Virtqueues}
> +
> +When VIRTIO_F_RING_DYNAMIC is not negotiated, the driver enables the virtqueues
> +during the device initialization sequence, i.e. after the device sets the
> +FEATURES_OK status bit and before the driver setting the DRIVER_OK status bit.
> +
> +When VIRTIO_F_RING_DYNAMIC is negotiated, the driver can avoid enabling the
> +virtqueues before setting the DRIVER_OK status bit; the driver can enable the
> +specific virtqueues after the driver has set the DRIVER_OK status bit.
> +The virtqueue enable mechanism is transport specific.
> +
> +\devicenormative{\paragraph}{Enable Virtqueues}{Basic Facilities of a Virtio Device / Virtqueues / Dynamic Virtqueues / Enable Virtqueues}
> +
> +When VIRTIO_F_RING_DYNAMIC is negotiated, the device MUST allow enabling the
> +virtqueue which was not enabled during the device initialization phase
> +i.e. after the device has set the FEATURES_OK status bit and before the
> +driver has set the DRIVER_OK status bit.
> +
> +\drivernormative{\paragraph}{Enable Virtqueues}{Basic Facilities of a Virtio Device / Virtqueues / Dynamic Virtqueues / Enable Virtqueues}
> +
> +When VIRTIO_F_RING_DYNAMIC is negotiated,
> +\begin{itemize}
> +\item the driver MAY enable some or all the virtqueues after the driver has set the
> +      DRIVER_OK status bit.
> +
> +\item the driver MAY enable some of the virtqueues or all the virtqueues or none of
> +      the virtqueues before the driver has set the DRIVER_OK status bit.
> +\end{itemize}
> +
> +When VIRTIO_F_RING_DYNAMIC is not negotiated, the driver MUST enable the
> +required number of virtqueues before setting the DRIVER_OK status bit.
> +
>  \input{split-ring.tex}
>
>  \input{packed-ring.tex}
> @@ -537,7 +569,9 @@ \section{Device Initialization}\label{sec:General Initialization And Device Oper
>
>  \item\label{itm:General Initialization And Device Operation / Device Initialization / Device-specific Setup} Perform device-specific setup, including discovery of virtqueues for the
>     device, optional per-bus setup, reading and possibly writing the
> -   device's virtio configuration space, and population of virtqueues.
> +   device's virtio configuration space.
> +
> +\item\label{itm:General Initialization And Device Operation / Device Initialization / Virtqueue Setup} Configure and enable the virtqueues.
>
>  \item\label{itm:General Initialization And Device Operation / Device Initialization / Set DRIVER-OK} Set the DRIVER_OK status bit.  At this point the device is
>     ``live''.
> @@ -551,6 +585,10 @@ \section{Device Initialization}\label{sec:General Initialization And Device Oper
>  The driver MUST NOT send any buffer available notifications to
>  the device before setting DRIVER_OK.
>
> +The driver MAY skip step
> +\ref{itm:General Initialization And Device Operation / Device Initialization / Virtqueue Setup}
> +when driver has negotiated VIRTIO_F_RING_DYNAMIC feature.
> +
>  \subsection{Legacy Interface: Device Initialization}\label{sec:General Initialization And Device Operation / Device Initialization / Legacy Interface: Device Initialization}
>  Legacy devices did not support the FEATURES_OK status bit, and thus did
>  not have a graceful way for the device to indicate unsupported feature
> @@ -569,7 +607,7 @@ \subsection{Legacy Interface: Device Initialization}\label{sec:General Initializ
>  Initialization / Re-read FEATURES-OK} were omitted, and steps
>  \ref{itm:General Initialization And Device Operation /
>  Device Initialization / Read feature bits},
> -\ref{itm:General Initialization And Device Operation / Device Initialization / Device-specific Setup} and \ref{itm:General Initialization And Device Operation / Device Initialization / Set DRIVER-OK}
> +\ref{itm:General Initialization And Device Operation / Device Initialization / Device-specific Setup}, \ref{itm:General Initialization And Device Operation / Device Initialization / Virtqueue Setup} and \ref{itm:General Initialization And Device Operation / Device Initialization / Set DRIVER-OK}
>  were conflated.
>
>  Therefore, when using the legacy interface:
> @@ -872,6 +910,11 @@ \chapter{Reserved Feature Bits}\label{sec:Reserved Feature Bits}
>         \ref{devicenormative:Basic Facilities of a Virtio Device / Feature Bits} for
>         handling features reserved for future use.
>
> +  \item[VIRTIO_F_RING_DYNAMIC(42)] This feature indicates
> +  that the driver can enable the virtqueues individually after the driver has
> +  set the status bit of DRIVER_OK.
> +  See \ref{sec:Basic Facilities of a Virtio Device / Virtqueues / Dynamic Virtqueues}.
> +
>  \end{description}
>
>  \drivernormative{\section}{Reserved Feature Bits}{Reserved Feature Bits}
> --
> 2.34.1
>
>
> This publicly archived list offers a means to provide input to the
> OASIS Virtual I/O Device (VIRTIO) TC.
>
> In order to verify user consent to the Feedback License terms and
> to minimize spam in the list archive, subscription is required
> before posting.
>
> Subscribe: virtio-comment-subscribe@lists.oasis-open.org
> Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
> List help: virtio-comment-help@lists.oasis-open.org
> List archive: https://lists.oasis-open.org/archives/virtio-comment/
> Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
> List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
> Committee: https://www.oasis-open.org/committees/virtio/
> Join OASIS: https://www.oasis-open.org/join/
>


This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.

In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.

Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/

Re: [virtio-comment] [PATCH v3 1/2] conformance: Add missing virtqueue reset conformance references

[Eugenio Perez Martin]

On Mon, Oct 2, 2023 at 7:16 AM Parav Pandit <parav@nvidia.com> wrote:
>
> Add the missing references to the virtqueue reset related conformance
> requirements.
>

acked-by was not carried from v1:

Acked-by: Eugenio Pérez <eperezma@redhat.com>

> Signed-off-by: Parav Pandit <parav@nvidia.com>
> Reviewed-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> ---
>  conformance.tex | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/conformance.tex b/conformance.tex
> index dc00e84..863f9c5 100644
> --- a/conformance.tex
> +++ b/conformance.tex
> @@ -76,6 +76,7 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
>  \item \ref{drivernormative:Basic Facilities of a Virtio Device / Device Reset}
>  \item \ref{drivernormative:Basic Facilities of a Virtio Device / Device Configuration Space}
>  \item \ref{drivernormative:Basic Facilities of a Virtio Device / Virtqueues}
> +\item \ref{drivernormative:Basic Facilities of a Virtio Device / Virtqueues / Virtqueue Reset / Virtqueue Reset}
>  \item \ref{drivernormative:Basic Facilities of a Virtio Device / Message Framing}
>  \item \ref{drivernormative:Basic Facilities of a Virtio Device / Virtqueues / The Virtqueue Descriptor Table}
>  \item \ref{drivernormative:Basic Facilities of a Virtio Device / Virtqueues / The Virtqueue Descriptor Table / Indirect Descriptors}
> @@ -162,6 +163,7 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
>  \item \ref{devicenormative:Basic Facilities of a Virtio Device / Feature Bits}
>  \item \ref{devicenormative:Basic Facilities of a Virtio Device / Device Reset}
>  \item \ref{devicenormative:Basic Facilities of a Virtio Device / Device Configuration Space}
> +\item \ref{devicenormative:Basic Facilities of a Virtio Device / Virtqueues / Virtqueue Reset / Virtqueue Reset}
>  \item \ref{devicenormative:Basic Facilities of a Virtio Device / Message Framing}
>  \item \ref{devicenormative:Basic Facilities of a Virtio Device / Virtqueues / The Virtqueue Descriptor Table}
>  \item \ref{devicenormative:Basic Facilities of a Virtio Device / Virtqueues / The Virtqueue Descriptor Table / Indirect Descriptors}
> --
> 2.34.1
>
>
> This publicly archived list offers a means to provide input to the
> OASIS Virtual I/O Device (VIRTIO) TC.
>
> In order to verify user consent to the Feedback License terms and
> to minimize spam in the list archive, subscription is required
> before posting.
>
> Subscribe: virtio-comment-subscribe@lists.oasis-open.org
> Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
> List help: virtio-comment-help@lists.oasis-open.org
> List archive: https://lists.oasis-open.org/archives/virtio-comment/
> Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
> List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
> Committee: https://www.oasis-open.org/committees/virtio/
> Join OASIS: https://www.oasis-open.org/join/
>


This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.

In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.

Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/

[virtio-comment] RE: [PATCH v3 0/2] Support enabling virtqueue after DRIVER_OK

[Parav Pandit]

Hi Michael, Cornelia,

> From: virtio-comment@lists.oasis-open.org <virtio-comment@lists.oasis-
> open.org> On Behalf Of Parav Pandit
> Sent: Monday, October 2, 2023 10:50 AM

> Hi Michael, Cornelia,
> 
> 
> > From: Parav Pandit <parav@nvidia.com>
> > Sent: Monday, October 2, 2023 10:46 AM
> >
> > Summary:
> > ========
> > This patch enables driver to create virtqueues after DRIVER_OK status bit is
> set.
> >
> > This patch take the inspiration from the thread [2] with credits to Eugenio
> PÃrez.
> >
> >
> > Use cases:
> > =========
> > 1. Dynamically create aq when administrative commands to be used.
> > ate the net device tx/rxq when device is
> >    opened when deploying for a container.
> >    In a container, number of virtqueues to be used may be <= max queues.
> > 3. Dynamically create flow filter queues of netdevice when
> >    ARFS or ethtool filters are enabled as listed in [1].
> > 4. Dynamically create rtc functionality related read virtqueue only
> >    when net device when timestamping to be used.
> > 5. When XDP program is set, one can create additional XDP specific
> >    queues without affecting existing queues.
> >
> > Hence, This patch introduces an existing queue enable and disable (aka
> > reset) facility and a new feature bit to explicitly indicate such support by the
> device.
> >
> > With this feature, drivers can skip optional queues creation during
> > driver init time. For example, a Linux net device driver can
> > create/destroy the transmit and receive queues when net device's
> > ndo_open() and ndo_stop() callbacks are invoked respectively.
> >
> > [1] https://lists.oasis-open.org/archives/virtio-
> > comment/202308/msg00263.html
> > [2] https://lists.oasis-open.org/archives/virtio-
> > comment/202306/msg00097.html
> >
> > Patch summary:
> > ==============
> > patch-1 fixes to add virtqueue reset conformance references
> > patch-2 adds VIRTIO_F_RING_DYNAMIC feature bit, requirements
> >
> > Please review.
> >
> > Fixes: https://github.com/oasis-tcs/virtio-spec/issues/177
 
> I have updated v3 to link to the github issue.
> Now that this small feature was up for few weeks, can you please create the
> ballot review?

Can you please create ballot review for 1.4?

[virtio-comment] Re: [PATCH v3 2/2] content: Support enabling virtqueue after DRIVER_OK stage

[Xuan Zhuo]

On Mon, 2 Oct 2023 08:16:01 +0300, Parav Pandit <parav@nvidia.com> wrote:
> Currently, a virtqueue must be enabled before driver has set the
> DRIVER_OK status bit.
>
> spec citation to section "Driver Requirements: Device Initialization"
>
> "Perform device-specific setup, including discovery of virtqueues
> for the device, optional per-bus setup, reading and possibly writing
> the device’s virtio configuration space, and population of virtqueues."
>
> This implies that a virtqueue must be enabled before reaching the
> DRIVER_OK stage. There was no explicit mention about ability to
> enable the virtqueue after DRIVER_OK stage.
>
> In following usecases, creating a virtqueue after DRIVER_OK phase is
> desired.
>
> 1. Dynamically create aq when administrative commands to be used.
> 2. Dynamically create the net device tx/rxq when device is
>    opened when deploying for a container.
>    In a container, number of virtqueues to be used may be <= max queues.
> 3. Dynamically create flow filter queues of netdevice when
>    ARFS or ethtool filters are enabled as listed in [1].
> 4. Dynamically create rtc functionality related read virtqueue only
>    when net device when timestamping to be used.
> 5. When XDP program is set, one can create additional XDP specific
>    queues without affecting existing queues.
>
> Hence, This patch introduces an existing queue enable and disable
> (aka reset) facility and a new feature bit to explicitly indicate such
> support by the device.
>
> With this feature, drivers can skip optional queues creation during
> driver init time. For example, a Linux net device driver
> can create/destroy the transmit and receive queues when net device's
> ndo_open() and ndo_stop() callbacks are invoked respectively.
>
> [1] https://lists.oasis-open.org/archives/virtio-comment/202308/msg00263.html
>
> Fixes: https://github.com/oasis-tcs/virtio-spec/issues/177
> Signed-off-by: Parav Pandit <parav@nvidia.com>

Acked-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>


> ---
> changelog:
> v2->v3:
> - github fixes tag added
> v1->v2:
> - fixed comments from Michael
> - reduced device and driver normatives (simplified)
> - replace 'a virtqueue' to 'the virtqueues'
> - reworded to remove 'interested'
> - avoided repeated 'device initialization text', replaced with
>   DRIVER_OK status bit language
> - avoided confusing text around 'MAY' in the requirements
> - replaced 'the queue' to 'specific virtqueues'
> - replaced queue to virtqueue
> - simplified commit log to talk about msix
> v0->v1:
> - fixed comments from Xuan
> - removed blank lines
> - fixed wrong requirement link to device in driver section
> ---
>  conformance.tex |  2 ++
>  content.tex     | 49 ++++++++++++++++++++++++++++++++++++++++++++++---
>  2 files changed, 48 insertions(+), 3 deletions(-)
>
> diff --git a/conformance.tex b/conformance.tex
> index 863f9c5..75de58c 100644
> --- a/conformance.tex
> +++ b/conformance.tex
> @@ -77,6 +77,7 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
>  \item \ref{drivernormative:Basic Facilities of a Virtio Device / Device Configuration Space}
>  \item \ref{drivernormative:Basic Facilities of a Virtio Device / Virtqueues}
>  \item \ref{drivernormative:Basic Facilities of a Virtio Device / Virtqueues / Virtqueue Reset / Virtqueue Reset}
> +\item \ref{drivernormative:Basic Facilities of a Virtio Device / Virtqueues / Dynamic Virtqueues / Enable Virtqueues}
>  \item \ref{drivernormative:Basic Facilities of a Virtio Device / Message Framing}
>  \item \ref{drivernormative:Basic Facilities of a Virtio Device / Virtqueues / The Virtqueue Descriptor Table}
>  \item \ref{drivernormative:Basic Facilities of a Virtio Device / Virtqueues / The Virtqueue Descriptor Table / Indirect Descriptors}
> @@ -164,6 +165,7 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
>  \item \ref{devicenormative:Basic Facilities of a Virtio Device / Device Reset}
>  \item \ref{devicenormative:Basic Facilities of a Virtio Device / Device Configuration Space}
>  \item \ref{devicenormative:Basic Facilities of a Virtio Device / Virtqueues / Virtqueue Reset / Virtqueue Reset}
> +\item \ref{devicenormative:Basic Facilities of a Virtio Device / Virtqueues / Dynamic Virtqueues / Enable Virtqueues}
>  \item \ref{devicenormative:Basic Facilities of a Virtio Device / Message Framing}
>  \item \ref{devicenormative:Basic Facilities of a Virtio Device / Virtqueues / The Virtqueue Descriptor Table}
>  \item \ref{devicenormative:Basic Facilities of a Virtio Device / Virtqueues / The Virtqueue Descriptor Table / Indirect Descriptors}
> diff --git a/content.tex b/content.tex
> index 0a62dce..1a296a9 100644
> --- a/content.tex
> +++ b/content.tex
> @@ -102,7 +102,7 @@ \section{Feature Bits}\label{sec:Basic Facilities of a Virtio Device / Feature B
>  \item[24 to 41] Feature bits reserved for extensions to the queue and
>    feature negotiation mechanisms
>
> -\item[42 to 49, and 128 and above] Feature bits reserved for future extensions.
> +\item[43 to 49, and 128 and above] Feature bits reserved for future extensions.
>  \end{description}
>
>  \begin{note}
> @@ -440,6 +440,38 @@ \subsubsection{Virtqueue Re-enable}\label{sec:Basic Facilities of a Virtio Devic
>  as during initial virtqueue discovery, but optionally with different
>  parameters.
>
> +\subsection{Dynamic Virtqueues}\label{sec:Basic Facilities of a Virtio Device / Virtqueues / Dynamic Virtqueues}
> +
> +When VIRTIO_F_RING_DYNAMIC is not negotiated, the driver enables the virtqueues
> +during the device initialization sequence, i.e. after the device sets the
> +FEATURES_OK status bit and before the driver setting the DRIVER_OK status bit.
> +
> +When VIRTIO_F_RING_DYNAMIC is negotiated, the driver can avoid enabling the
> +virtqueues before setting the DRIVER_OK status bit; the driver can enable the
> +specific virtqueues after the driver has set the DRIVER_OK status bit.
> +The virtqueue enable mechanism is transport specific.
> +
> +\devicenormative{\paragraph}{Enable Virtqueues}{Basic Facilities of a Virtio Device / Virtqueues / Dynamic Virtqueues / Enable Virtqueues}
> +
> +When VIRTIO_F_RING_DYNAMIC is negotiated, the device MUST allow enabling the
> +virtqueue which was not enabled during the device initialization phase
> +i.e. after the device has set the FEATURES_OK status bit and before the
> +driver has set the DRIVER_OK status bit.
> +
> +\drivernormative{\paragraph}{Enable Virtqueues}{Basic Facilities of a Virtio Device / Virtqueues / Dynamic Virtqueues / Enable Virtqueues}
> +
> +When VIRTIO_F_RING_DYNAMIC is negotiated,
> +\begin{itemize}
> +\item the driver MAY enable some or all the virtqueues after the driver has set the
> +      DRIVER_OK status bit.
> +
> +\item the driver MAY enable some of the virtqueues or all the virtqueues or none of
> +      the virtqueues before the driver has set the DRIVER_OK status bit.
> +\end{itemize}
> +
> +When VIRTIO_F_RING_DYNAMIC is not negotiated, the driver MUST enable the
> +required number of virtqueues before setting the DRIVER_OK status bit.
> +
>  \input{split-ring.tex}
>
>  \input{packed-ring.tex}
> @@ -537,7 +569,9 @@ \section{Device Initialization}\label{sec:General Initialization And Device Oper
>
>  \item\label{itm:General Initialization And Device Operation / Device Initialization / Device-specific Setup} Perform device-specific setup, including discovery of virtqueues for the
>     device, optional per-bus setup, reading and possibly writing the
> -   device's virtio configuration space, and population of virtqueues.
> +   device's virtio configuration space.
> +
> +\item\label{itm:General Initialization And Device Operation / Device Initialization / Virtqueue Setup} Configure and enable the virtqueues.
>
>  \item\label{itm:General Initialization And Device Operation / Device Initialization / Set DRIVER-OK} Set the DRIVER_OK status bit.  At this point the device is
>     ``live''.
> @@ -551,6 +585,10 @@ \section{Device Initialization}\label{sec:General Initialization And Device Oper
>  The driver MUST NOT send any buffer available notifications to
>  the device before setting DRIVER_OK.
>
> +The driver MAY skip step
> +\ref{itm:General Initialization And Device Operation / Device Initialization / Virtqueue Setup}
> +when driver has negotiated VIRTIO_F_RING_DYNAMIC feature.
> +
>  \subsection{Legacy Interface: Device Initialization}\label{sec:General Initialization And Device Operation / Device Initialization / Legacy Interface: Device Initialization}
>  Legacy devices did not support the FEATURES_OK status bit, and thus did
>  not have a graceful way for the device to indicate unsupported feature
> @@ -569,7 +607,7 @@ \subsection{Legacy Interface: Device Initialization}\label{sec:General Initializ
>  Initialization / Re-read FEATURES-OK} were omitted, and steps
>  \ref{itm:General Initialization And Device Operation /
>  Device Initialization / Read feature bits},
> -\ref{itm:General Initialization And Device Operation / Device Initialization / Device-specific Setup} and \ref{itm:General Initialization And Device Operation / Device Initialization / Set DRIVER-OK}
> +\ref{itm:General Initialization And Device Operation / Device Initialization / Device-specific Setup}, \ref{itm:General Initialization And Device Operation / Device Initialization / Virtqueue Setup} and \ref{itm:General Initialization And Device Operation / Device Initialization / Set DRIVER-OK}
>  were conflated.
>
>  Therefore, when using the legacy interface:
> @@ -872,6 +910,11 @@ \chapter{Reserved Feature Bits}\label{sec:Reserved Feature Bits}
>  	\ref{devicenormative:Basic Facilities of a Virtio Device / Feature Bits} for
>  	handling features reserved for future use.
>
> +  \item[VIRTIO_F_RING_DYNAMIC(42)] This feature indicates
> +  that the driver can enable the virtqueues individually after the driver has
> +  set the status bit of DRIVER_OK.
> +  See \ref{sec:Basic Facilities of a Virtio Device / Virtqueues / Dynamic Virtqueues}.
> +
>  \end{description}
>
>  \drivernormative{\section}{Reserved Feature Bits}{Reserved Feature Bits}
> --
> 2.34.1
>

This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.

In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.

Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/

[virtio-comment] RE: [PATCH v3 0/2] Support enabling virtqueue after DRIVER_OK

[Parav Pandit]

+ virtio TC.

Hi Michael, Cornelia,

Its been many days, the voting request is not raised for this small patch.

Are you receiving this email?
By any chance, this email has gone to spam/junk directory in your mailbox?

Xuan, Eugenio acked so I at least two mailboxes received in non-junk directory.
I noticed ballot request created 5 days ago for github issue 175, https://www.oasis-open.org/committees/ballot.php?id=3798.

I suspect that, this email is not reaching you for some reason.

Added virtio TC to see if the 3rd ping reaches you.

Flow filters feature is waiting for a while now which depends on this series as users are not able to scale their performance.

Is there anything more to take care in this patch?
Please let me know if I need to fix something in this patch.

> -----Original Message-----
> From: Parav Pandit <parav@nvidia.com>
> Sent: Tuesday, October 10, 2023 10:27 AM
> To: Parav Pandit <parav@nvidia.com>; virtio-comment@lists.oasis-open.org;
> mst@redhat.com; cohuck@redhat.com
> Cc: hengqi@linux.alibaba.com; xuanzhuo@linux.alibaba.com; Shahaf Shuler
> <shahafs@nvidia.com>
> Subject: RE: [PATCH v3 0/2] Support enabling virtqueue after DRIVER_OK
> 
> Hi Michael, Cornelia,
> 
> > From: virtio-comment@lists.oasis-open.org <virtio-comment@lists.oasis-
> > open.org> On Behalf Of Parav Pandit
> > Sent: Monday, October 2, 2023 10:50 AM
> 
> > Hi Michael, Cornelia,
> >
> >
> > > From: Parav Pandit <parav@nvidia.com>
> > > Sent: Monday, October 2, 2023 10:46 AM
> > >
> > > Summary:
> > > ========
> > > This patch enables driver to create virtqueues after DRIVER_OK
> > > status bit is
> > set.
> > >
> > > This patch take the inspiration from the thread [2] with credits to
> > > Eugenio
> > PÃrez.
> > >
> > >
> > > Use cases:
> > > =========
> > > 1. Dynamically create aq when administrative commands to be used.
> > > ate the net device tx/rxq when device is
> > >    opened when deploying for a container.
> > >    In a container, number of virtqueues to be used may be <= max queues.
> > > 3. Dynamically create flow filter queues of netdevice when
> > >    ARFS or ethtool filters are enabled as listed in [1].
> > > 4. Dynamically create rtc functionality related read virtqueue only
> > >    when net device when timestamping to be used.
> > > 5. When XDP program is set, one can create additional XDP specific
> > >    queues without affecting existing queues.
> > >
> > > Hence, This patch introduces an existing queue enable and disable
> > > (aka
> > > reset) facility and a new feature bit to explicitly indicate such
> > > support by the
> > device.
> > >
> > > With this feature, drivers can skip optional queues creation during
> > > driver init time. For example, a Linux net device driver can
> > > create/destroy the transmit and receive queues when net device's
> > > ndo_open() and ndo_stop() callbacks are invoked respectively.
> > >
> > > [1] https://lists.oasis-open.org/archives/virtio-
> > > comment/202308/msg00263.html
> > > [2] https://lists.oasis-open.org/archives/virtio-
> > > comment/202306/msg00097.html
> > >
> > > Patch summary:
> > > ==============
> > > patch-1 fixes to add virtqueue reset conformance references
> > > patch-2 adds VIRTIO_F_RING_DYNAMIC feature bit, requirements
> > >
> > > Please review.
> > >
> > > Fixes: https://github.com/oasis-tcs/virtio-spec/issues/177
> 
> > I have updated v3 to link to the github issue.
> > Now that this small feature was up for few weeks, can you please
> > create the ballot review?
> 
> Can you please create ballot review for 1.4?

Re: [virtio-comment] RE: [PATCH v3 0/2] Support enabling virtqueue after DRIVER_OK

[Cornelia Huck]

On Mon, Oct 16 2023, Parav Pandit <parav@nvidia.com> wrote:

> + virtio TC.
>
> Hi Michael, Cornelia,
>
> Its been many days, the voting request is not raised for this small patch.
>
> Are you receiving this email?
> By any chance, this email has gone to spam/junk directory in your mailbox?
>
> Xuan, Eugenio acked so I at least two mailboxes received in non-junk directory.
> I noticed ballot request created 5 days ago for github issue 175, https://www.oasis-open.org/committees/ballot.php?id=3798.
>
> I suspect that, this email is not reaching you for some reason.
>
> Added virtio TC to see if the 3rd ping reaches you.
>
> Flow filters feature is waiting for a while now which depends on this series as users are not able to scale their performance.
>
> Is there anything more to take care in this patch?
> Please let me know if I need to fix something in this patch.

This is the first time I noticed this, let me check.


This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.

In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.

Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/

[virtio-comment] Re: [PATCH v3 2/2] content: Support enabling virtqueue after DRIVER_OK stage

[Cornelia Huck]

On Mon, Oct 02 2023, Parav Pandit <parav@nvidia.com> wrote:

> Currently, a virtqueue must be enabled before driver has set the
> DRIVER_OK status bit.
>
> spec citation to section "Driver Requirements: Device Initialization"
>
> "Perform device-specific setup, including discovery of virtqueues
> for the device, optional per-bus setup, reading and possibly writing
> the device’s virtio configuration space, and population of virtqueues."
>
> This implies that a virtqueue must be enabled before reaching the
> DRIVER_OK stage. There was no explicit mention about ability to
> enable the virtqueue after DRIVER_OK stage.
>
> In following usecases, creating a virtqueue after DRIVER_OK phase is
> desired.
>
> 1. Dynamically create aq when administrative commands to be used.
> 2. Dynamically create the net device tx/rxq when device is
>    opened when deploying for a container.
>    In a container, number of virtqueues to be used may be <= max queues.
> 3. Dynamically create flow filter queues of netdevice when
>    ARFS or ethtool filters are enabled as listed in [1].
> 4. Dynamically create rtc functionality related read virtqueue only
>    when net device when timestamping to be used.
> 5. When XDP program is set, one can create additional XDP specific
>    queues without affecting existing queues.
>
> Hence, This patch introduces an existing queue enable and disable
> (aka reset) facility and a new feature bit to explicitly indicate such
> support by the device.
>
> With this feature, drivers can skip optional queues creation during
> driver init time. For example, a Linux net device driver
> can create/destroy the transmit and receive queues when net device's
> ndo_open() and ndo_stop() callbacks are invoked respectively.
>
> [1] https://lists.oasis-open.org/archives/virtio-comment/202308/msg00263.html
>
> Fixes: https://github.com/oasis-tcs/virtio-spec/issues/177
> Signed-off-by: Parav Pandit <parav@nvidia.com>
> ---
> changelog:
> v2->v3:
> - github fixes tag added
> v1->v2:
> - fixed comments from Michael
> - reduced device and driver normatives (simplified)
> - replace 'a virtqueue' to 'the virtqueues'
> - reworded to remove 'interested'
> - avoided repeated 'device initialization text', replaced with
>   DRIVER_OK status bit language
> - avoided confusing text around 'MAY' in the requirements
> - replaced 'the queue' to 'specific virtqueues'
> - replaced queue to virtqueue
> - simplified commit log to talk about msix
> v0->v1:
> - fixed comments from Xuan
> - removed blank lines
> - fixed wrong requirement link to device in driver section
> ---
>  conformance.tex |  2 ++
>  content.tex     | 49 ++++++++++++++++++++++++++++++++++++++++++++++---
>  2 files changed, 48 insertions(+), 3 deletions(-)
>

(...)

> @@ -440,6 +440,38 @@ \subsubsection{Virtqueue Re-enable}\label{sec:Basic Facilities of a Virtio Devic
>  as during initial virtqueue discovery, but optionally with different
>  parameters.
>  
> +\subsection{Dynamic Virtqueues}\label{sec:Basic Facilities of a Virtio Device / Virtqueues / Dynamic Virtqueues}
> +
> +When VIRTIO_F_RING_DYNAMIC is not negotiated, the driver enables the virtqueues
> +during the device initialization sequence, i.e. after the device sets the
> +FEATURES_OK status bit and before the driver setting the DRIVER_OK status bit.

_Or_ if a virtqueue has been reset and the driver wants to re-enable it,
right?

> +
> +When VIRTIO_F_RING_DYNAMIC is negotiated, the driver can avoid enabling the
> +virtqueues before setting the DRIVER_OK status bit; the driver can enable the
> +specific virtqueues after the driver has set the DRIVER_OK status bit.

"the driver is not required to enable every virtqueue it wants to use
before setting the DRIVER_OK status bit; it can choose to enable a
virtqueue even after it has set the DRIVER_OK status bit."

> +The virtqueue enable mechanism is transport specific.

Would that be the same mechanism as for re-enabling a queue after a
queue reset? I guess I'm missing the relationship here...

> +
> +\devicenormative{\paragraph}{Enable Virtqueues}{Basic Facilities of a Virtio Device / Virtqueues / Dynamic Virtqueues / Enable Virtqueues}
> +
> +When VIRTIO_F_RING_DYNAMIC is negotiated, the device MUST allow enabling the
> +virtqueue which was not enabled during the device initialization phase

s/the virtqueue/a virtqueue/

> +i.e. after the device has set the FEATURES_OK status bit and before the
> +driver has set the DRIVER_OK status bit.
> +
> +\drivernormative{\paragraph}{Enable Virtqueues}{Basic Facilities of a Virtio Device / Virtqueues / Dynamic Virtqueues / Enable Virtqueues}
> +
> +When VIRTIO_F_RING_DYNAMIC is negotiated,
> +\begin{itemize}
> +\item the driver MAY enable some or all the virtqueues after the driver has set the
> +      DRIVER_OK status bit.
> +
> +\item the driver MAY enable some of the virtqueues or all the virtqueues or none of
> +      the virtqueues before the driver has set the DRIVER_OK status bit.
> +\end{itemize}

Can we write this more concisely as

"When VIRTIO_F_RING_DYNAMIC is negotiated, the driver MAY enable any
virtqueue either before or after it has set the DRIVER_OK status bit."

?

> +
> +When VIRTIO_F_RING_DYNAMIC is not negotiated, the driver MUST enable the
> +required number of virtqueues before setting the DRIVER_OK status bit.

What does "required" mean here? It just chooses to enable the queues it
wants to use, right?

I'm also still not quite clear how this relates to re-enabling queues
after queue reset.

> +
>  \input{split-ring.tex}
>  
>  \input{packed-ring.tex}
> @@ -537,7 +569,9 @@ \section{Device Initialization}\label{sec:General Initialization And Device Oper
>  
>  \item\label{itm:General Initialization And Device Operation / Device Initialization / Device-specific Setup} Perform device-specific setup, including discovery of virtqueues for the
>     device, optional per-bus setup, reading and possibly writing the
> -   device's virtio configuration space, and population of virtqueues.
> +   device's virtio configuration space.
> +
> +\item\label{itm:General Initialization And Device Operation / Device Initialization / Virtqueue Setup} Configure and enable the virtqueues.
>  
>  \item\label{itm:General Initialization And Device Operation / Device Initialization / Set DRIVER-OK} Set the DRIVER_OK status bit.  At this point the device is
>     ``live''.
> @@ -551,6 +585,10 @@ \section{Device Initialization}\label{sec:General Initialization And Device Oper
>  The driver MUST NOT send any buffer available notifications to
>  the device before setting DRIVER_OK.
>  
> +The driver MAY skip step
> +\ref{itm:General Initialization And Device Operation / Device Initialization / Virtqueue Setup}
> +when driver has negotiated VIRTIO_F_RING_DYNAMIC feature.

That's confusing: It may not only skip it, but also enable only a subset
of the queues it wants to use later.

> +
>  \subsection{Legacy Interface: Device Initialization}\label{sec:General Initialization And Device Operation / Device Initialization / Legacy Interface: Device Initialization}
>  Legacy devices did not support the FEATURES_OK status bit, and thus did
>  not have a graceful way for the device to indicate unsupported feature
> @@ -569,7 +607,7 @@ \subsection{Legacy Interface: Device Initialization}\label{sec:General Initializ
>  Initialization / Re-read FEATURES-OK} were omitted, and steps
>  \ref{itm:General Initialization And Device Operation /
>  Device Initialization / Read feature bits},
> -\ref{itm:General Initialization And Device Operation / Device Initialization / Device-specific Setup} and \ref{itm:General Initialization And Device Operation / Device Initialization / Set DRIVER-OK}
> +\ref{itm:General Initialization And Device Operation / Device Initialization / Device-specific Setup}, \ref{itm:General Initialization And Device Operation / Device Initialization / Virtqueue Setup} and \ref{itm:General Initialization And Device Operation / Device Initialization / Set DRIVER-OK}
>  were conflated.
>  
>  Therefore, when using the legacy interface:
> @@ -872,6 +910,11 @@ \chapter{Reserved Feature Bits}\label{sec:Reserved Feature Bits}
>  	\ref{devicenormative:Basic Facilities of a Virtio Device / Feature Bits} for
>  	handling features reserved for future use.
>  
> +  \item[VIRTIO_F_RING_DYNAMIC(42)] This feature indicates
> +  that the driver can enable the virtqueues individually after the driver has

s/the virtqueues/virtqueues/

> +  set the status bit of DRIVER_OK.
> +  See \ref{sec:Basic Facilities of a Virtio Device / Virtqueues / Dynamic Virtqueues}.
> +
>  \end{description}
>  
>  \drivernormative{\section}{Reserved Feature Bits}{Reserved Feature Bits}

We currently have a device normative statement:

"The device MUST NOT consume buffers or send any used buffer
notifications to the driver before DRIVER_OK."

I guess we need to extend that to not doing that for not-yet-enabled
queues in the dynamic virtqueue case? There's a (transport-specific)
point in time when the driver tells the device that the queue is ready,
right?


This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.

In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.

Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/

[virtio-comment] RE: [PATCH v3 2/2] content: Support enabling virtqueue after DRIVER_OK stage

[Parav Pandit]

> From: Cornelia Huck <cohuck@redhat.com>
> Sent: Tuesday, October 17, 2023 5:55 PM
> 
> On Mon, Oct 02 2023, Parav Pandit <parav@nvidia.com> wrote:
> 
> > Currently, a virtqueue must be enabled before driver has set the
> > DRIVER_OK status bit.
> >
> > spec citation to section "Driver Requirements: Device Initialization"
> >
> > "Perform device-specific setup, including discovery of virtqueues for
> > the device, optional per-bus setup, reading and possibly writing the
> > device’s virtio configuration space, and population of virtqueues."
> >
> > This implies that a virtqueue must be enabled before reaching the
> > DRIVER_OK stage. There was no explicit mention about ability to enable
> > the virtqueue after DRIVER_OK stage.
> >
> > In following usecases, creating a virtqueue after DRIVER_OK phase is
> > desired.
> >
> > 1. Dynamically create aq when administrative commands to be used.
> > 2. Dynamically create the net device tx/rxq when device is
> >    opened when deploying for a container.
> >    In a container, number of virtqueues to be used may be <= max queues.
> > 3. Dynamically create flow filter queues of netdevice when
> >    ARFS or ethtool filters are enabled as listed in [1].
> > 4. Dynamically create rtc functionality related read virtqueue only
> >    when net device when timestamping to be used.
> > 5. When XDP program is set, one can create additional XDP specific
> >    queues without affecting existing queues.
> >
> > Hence, This patch introduces an existing queue enable and disable (aka
> > reset) facility and a new feature bit to explicitly indicate such
> > support by the device.
> >
> > With this feature, drivers can skip optional queues creation during
> > driver init time. For example, a Linux net device driver can
> > create/destroy the transmit and receive queues when net device's
> > ndo_open() and ndo_stop() callbacks are invoked respectively.
> >
> > [1]
> > https://lists.oasis-open.org/archives/virtio-comment/202308/msg00263.h
> > tml
> >
> > Fixes: https://github.com/oasis-tcs/virtio-spec/issues/177
> > Signed-off-by: Parav Pandit <parav@nvidia.com>
> > ---
> > changelog:
> > v2->v3:
> > - github fixes tag added
> > v1->v2:
> > - fixed comments from Michael
> > - reduced device and driver normatives (simplified)
> > - replace 'a virtqueue' to 'the virtqueues'
> > - reworded to remove 'interested'
> > - avoided repeated 'device initialization text', replaced with
> >   DRIVER_OK status bit language
> > - avoided confusing text around 'MAY' in the requirements
> > - replaced 'the queue' to 'specific virtqueues'
> > - replaced queue to virtqueue
> > - simplified commit log to talk about msix
> > v0->v1:
> > - fixed comments from Xuan
> > - removed blank lines
> > - fixed wrong requirement link to device in driver section
> > ---
> >  conformance.tex |  2 ++
> >  content.tex     | 49 ++++++++++++++++++++++++++++++++++++++++++++++--
> -
> >  2 files changed, 48 insertions(+), 3 deletions(-)
> >
> 
> (...)
> 
> > @@ -440,6 +440,38 @@ \subsubsection{Virtqueue
> > Re-enable}\label{sec:Basic Facilities of a Virtio Devic  as during
> > initial virtqueue discovery, but optionally with different  parameters.
> >
> > +\subsection{Dynamic Virtqueues}\label{sec:Basic Facilities of a
> > +Virtio Device / Virtqueues / Dynamic Virtqueues}
> > +
> > +When VIRTIO_F_RING_DYNAMIC is not negotiated, the driver enables the
> > +virtqueues during the device initialization sequence, i.e. after the
> > +device sets the FEATURES_OK status bit and before the driver setting the
> DRIVER_OK status bit.
> 
> _Or_ if a virtqueue has been reset and the driver wants to re-enable it, right?
> 
Well no. Because above text is for "enablement", and not re-enablement.
If the driver want to reset and re-enable, it must be enabled in first place before setting driver_ok.
VQ not enabled before driver ok, cannot be reset, and hence cannot be re-enabled.

So I think above text is fine because it says about "enables" and not "re-enables".

> > +
> > +When VIRTIO_F_RING_DYNAMIC is negotiated, the driver can avoid
> > +enabling the virtqueues before setting the DRIVER_OK status bit; the
> > +driver can enable the specific virtqueues after the driver has set the
> DRIVER_OK status bit.
> 
> "the driver is not required to enable every virtqueue it wants to use before
> setting the DRIVER_OK status bit; it can choose to enable a virtqueue even after
> it has set the DRIVER_OK status bit."
> 
Sounds good. Will change it.

> > +The virtqueue enable mechanism is transport specific.
> 
> Would that be the same mechanism as for re-enabling a queue after a queue
> reset? I guess I'm missing the relationship here...
> 
Yes, it is same.
There is no change in enabling/re-enabling the virtqueue after/before DRIVER_OK with/without _dynamic bit.
So no extra text added here.

> > +
> > +\devicenormative{\paragraph}{Enable Virtqueues}{Basic Facilities of a
> > +Virtio Device / Virtqueues / Dynamic Virtqueues / Enable Virtqueues}
> > +
> > +When VIRTIO_F_RING_DYNAMIC is negotiated, the device MUST allow
> > +enabling the virtqueue which was not enabled during the device
> > +initialization phase
> 
> s/the virtqueue/a virtqueue/
> 
Ack.

> > +i.e. after the device has set the FEATURES_OK status bit and before
> > +the driver has set the DRIVER_OK status bit.
> > +
> > +\drivernormative{\paragraph}{Enable Virtqueues}{Basic Facilities of a
> > +Virtio Device / Virtqueues / Dynamic Virtqueues / Enable Virtqueues}
> > +
> > +When VIRTIO_F_RING_DYNAMIC is negotiated, \begin{itemize} \item the
> > +driver MAY enable some or all the virtqueues after the driver has set the
> > +      DRIVER_OK status bit.
> > +
> > +\item the driver MAY enable some of the virtqueues or all the virtqueues or
> none of
> > +      the virtqueues before the driver has set the DRIVER_OK status bit.
> > +\end{itemize}
> 
> Can we write this more concisely as
> 
> "When VIRTIO_F_RING_DYNAMIC is negotiated, the driver MAY enable any
> virtqueue either before or after it has set the DRIVER_OK status bit."
> 
> ?
Looks fine to me. Will change.
> 
> > +
> > +When VIRTIO_F_RING_DYNAMIC is not negotiated, the driver MUST enable
> > +the required number of virtqueues before setting the DRIVER_OK status bit.
> 
> What does "required" mean here? It just chooses to enable the queues it wants
> to use, right?
Right.
Required meaning, whatever number of queues that driver choose to enable, those must be enabled before driver_ok.
So it is "required by the driver".
Would that be ok?

> 
> I'm also still not quite clear how this relates to re-enabling queues after queue
> reset.
Queue can be renabled (after queue reset) only if it was enabled before driver_ok.
This feature bit allows driver to not do such workaround to enable the queue before driver_ok, that it does not want to use in first place.
Not sure I answered, what you ask.
Did I?
	
> 
> > +
> >  \input{split-ring.tex}
> >
> >  \input{packed-ring.tex}
> > @@ -537,7 +569,9 @@ \section{Device Initialization}\label{sec:General
> > Initialization And Device Oper
> >
> >  \item\label{itm:General Initialization And Device Operation / Device
> Initialization / Device-specific Setup} Perform device-specific setup, including
> discovery of virtqueues for the
> >     device, optional per-bus setup, reading and possibly writing the
> > -   device's virtio configuration space, and population of virtqueues.
> > +   device's virtio configuration space.
> > +
> > +\item\label{itm:General Initialization And Device Operation / Device
> Initialization / Virtqueue Setup} Configure and enable the virtqueues.
> >
> >  \item\label{itm:General Initialization And Device Operation / Device
> Initialization / Set DRIVER-OK} Set the DRIVER_OK status bit.  At this point the
> device is
> >     ``live''.
> > @@ -551,6 +585,10 @@ \section{Device Initialization}\label{sec:General
> > Initialization And Device Oper  The driver MUST NOT send any buffer
> > available notifications to  the device before setting DRIVER_OK.
> >
> > +The driver MAY skip step
> > +\ref{itm:General Initialization And Device Operation / Device
> > +Initialization / Virtqueue Setup} when driver has negotiated
> VIRTIO_F_RING_DYNAMIC feature.
> 
> That's confusing: It may not only skip it, but also enable only a subset of the
> queues it wants to use later.
> 
I will add this extra text, that driver may skip or may enable subset of the queues.

> > +
> >  \subsection{Legacy Interface: Device
> > Initialization}\label{sec:General Initialization And Device Operation
> > / Device Initialization / Legacy Interface: Device Initialization}
> > Legacy devices did not support the FEATURES_OK status bit, and thus
> > did  not have a graceful way for the device to indicate unsupported
> > feature @@ -569,7 +607,7 @@ \subsection{Legacy Interface: Device
> > Initialization}\label{sec:General Initializ  Initialization / Re-read
> > FEATURES-OK} were omitted, and steps  \ref{itm:General Initialization
> > And Device Operation /  Device Initialization / Read feature bits},
> > -\ref{itm:General Initialization And Device Operation / Device
> > Initialization / Device-specific Setup} and \ref{itm:General
> > Initialization And Device Operation / Device Initialization / Set
> > DRIVER-OK}
> > +\ref{itm:General Initialization And Device Operation / Device
> > +Initialization / Device-specific Setup}, \ref{itm:General
> > +Initialization And Device Operation / Device Initialization /
> > +Virtqueue Setup} and \ref{itm:General Initialization And Device
> > +Operation / Device Initialization / Set DRIVER-OK}
> >  were conflated.
> >
> >  Therefore, when using the legacy interface:
> > @@ -872,6 +910,11 @@ \chapter{Reserved Feature Bits}\label{sec:Reserved
> Feature Bits}
> >  	\ref{devicenormative:Basic Facilities of a Virtio Device / Feature Bits}
> for
> >  	handling features reserved for future use.
> >
> > +  \item[VIRTIO_F_RING_DYNAMIC(42)] This feature indicates  that the
> > + driver can enable the virtqueues individually after the driver has
> 
> s/the virtqueues/virtqueues/
Ack

> 
> > +  set the status bit of DRIVER_OK.
> > +  See \ref{sec:Basic Facilities of a Virtio Device / Virtqueues / Dynamic
> Virtqueues}.
> > +
> >  \end{description}
> >
> >  \drivernormative{\section}{Reserved Feature Bits}{Reserved Feature
> > Bits}
> 
> We currently have a device normative statement:
> 
> "The device MUST NOT consume buffers or send any used buffer notifications to
> the driver before DRIVER_OK."
> 
> I guess we need to extend that to not doing that for not-yet-enabled queues in
> the dynamic virtqueue case? There's a (transport-specific) point in time when
> the driver tells the device that the queue is ready, right?

We don’t need extend this text because device does not know anything about the disabled queue, so it cannot consume any buffer from it anyway.
Above line only applied to the enabled virtqueue.

RE: [virtio-comment] RE: [PATCH v3 0/2] Support enabling virtqueue after DRIVER_OK

[Parav Pandit]

> From: Cornelia Huck <cohuck@redhat.com>
> Sent: Tuesday, October 17, 2023 5:28 PM
> 
> On Mon, Oct 16 2023, Parav Pandit <parav@nvidia.com> wrote:
> 
> > + virtio TC.
> >
> > Hi Michael, Cornelia,
> >
> > Its been many days, the voting request is not raised for this small patch.
> >
> > Are you receiving this email?
> > By any chance, this email has gone to spam/junk directory in your mailbox?
> >
> > Xuan, Eugenio acked so I at least two mailboxes received in non-junk
> directory.
> > I noticed ballot request created 5 days ago for github issue 175,
> https://www.oasis-open.org/committees/ballot.php?id=3798.
> >
> > I suspect that, this email is not reaching you for some reason.
> >
> > Added virtio TC to see if the 3rd ping reaches you.
> >
> > Flow filters feature is waiting for a while now which depends on this series as
> users are not able to scale their performance.
> >
> > Is there anything more to take care in this patch?
> > Please let me know if I need to fix something in this patch.
> 
> This is the first time I noticed this, let me check.

Can you please confirm, if you notice this on virtio list or virtio-comment list in your email client.
So that we can identify and fix the problem.
Eugenio and Xuan were able to see the email on virtio-comment.
It is also present on two mail bots too. So trying to find out what is wrong here.

This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.

In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.

Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/

Re: [virtio-comment] RE: [PATCH v3 2/2] content: Support enabling virtqueue after DRIVER_OK stage

[Cornelia Huck]

On Tue, Oct 17 2023, Parav Pandit <parav@nvidia.com> wrote:

>> From: Cornelia Huck <cohuck@redhat.com>
>> Sent: Tuesday, October 17, 2023 5:55 PM
>> 
>> On Mon, Oct 02 2023, Parav Pandit <parav@nvidia.com> wrote:
>> > @@ -440,6 +440,38 @@ \subsubsection{Virtqueue
>> > Re-enable}\label{sec:Basic Facilities of a Virtio Devic  as during
>> > initial virtqueue discovery, but optionally with different  parameters.
>> >
>> > +\subsection{Dynamic Virtqueues}\label{sec:Basic Facilities of a
>> > +Virtio Device / Virtqueues / Dynamic Virtqueues}
>> > +
>> > +When VIRTIO_F_RING_DYNAMIC is not negotiated, the driver enables the
>> > +virtqueues during the device initialization sequence, i.e. after the
>> > +device sets the FEATURES_OK status bit and before the driver setting the
>> DRIVER_OK status bit.
>> 
>> _Or_ if a virtqueue has been reset and the driver wants to re-enable it, right?
>> 
> Well no. Because above text is for "enablement", and not re-enablement.
> If the driver want to reset and re-enable, it must be enabled in first place before setting driver_ok.
> VQ not enabled before driver ok, cannot be reset, and hence cannot be re-enabled.
>
> So I think above text is fine because it says about "enables" and not "re-enables".
>
>> > +
>> > +When VIRTIO_F_RING_DYNAMIC is negotiated, the driver can avoid
>> > +enabling the virtqueues before setting the DRIVER_OK status bit; the
>> > +driver can enable the specific virtqueues after the driver has set the
>> DRIVER_OK status bit.
>> 
>> "the driver is not required to enable every virtqueue it wants to use before
>> setting the DRIVER_OK status bit; it can choose to enable a virtqueue even after
>> it has set the DRIVER_OK status bit."
>> 
> Sounds good. Will change it.
>
>> > +The virtqueue enable mechanism is transport specific.
>> 
>> Would that be the same mechanism as for re-enabling a queue after a queue
>> reset? I guess I'm missing the relationship here...
>> 
> Yes, it is same.
> There is no change in enabling/re-enabling the virtqueue after/before DRIVER_OK with/without _dynamic bit.
> So no extra text added here.

This is not really clear to me just from this text, especially if you
just wrote above that enabling or re-enabling is something
different... my understanding would be:

- if neither dynamic vqs nor queue reset are supported or negotiated,
  the only way to enable a vq is before DRIVER_OK, during setup
- both of these features rely on the transport supporting enabling
  individual queues (either a queue that has not been enabled before, or
  a queue that has been reset)
- the transport is supposed to use the same mechanism for either

Did I get it right? If so, I think we should make it a bit more clear.

(...)

>> > +When VIRTIO_F_RING_DYNAMIC is not negotiated, the driver MUST enable
>> > +the required number of virtqueues before setting the DRIVER_OK status bit.
>> 
>> What does "required" mean here? It just chooses to enable the queues it wants
>> to use, right?
> Right.
> Required meaning, whatever number of queues that driver choose to enable, those must be enabled before driver_ok.
> So it is "required by the driver".
> Would that be ok?

I'd write it as "the driver MUST enable any virtqueue it plans to use"
or something like that.

(...)

>> We currently have a device normative statement:
>> 
>> "The device MUST NOT consume buffers or send any used buffer notifications to
>> the driver before DRIVER_OK."
>> 
>> I guess we need to extend that to not doing that for not-yet-enabled queues in
>> the dynamic virtqueue case? There's a (transport-specific) point in time when
>> the driver tells the device that the queue is ready, right?
>
> We don’t need extend this text because device does not know anything about the disabled queue, so it cannot consume any buffer from it anyway.
> Above line only applied to the enabled virtqueue.

My point is: DRIVER_OK signals that the driver is done with its setup
and the device may start to interact with the queue. Depending on the
mechanism the transport uses for enabling the queue, it may "know" about
the queue before the driver is ready -- it might need to wait until the
driver has completed enabling of the queue. Not sure if that is worth
spelling out.


This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.

In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.

Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/

Re: [virtio-comment] RE: [PATCH v3 2/2] content: Support enabling virtqueue after DRIVER_OK stage

[Michael S. Tsirkin]

On Wed, Oct 18, 2023 at 12:25:23PM +0200, Cornelia Huck wrote:
> On Tue, Oct 17 2023, Parav Pandit <parav@nvidia.com> wrote:
> 
> >> From: Cornelia Huck <cohuck@redhat.com>
> >> Sent: Tuesday, October 17, 2023 5:55 PM
> >> 
> >> On Mon, Oct 02 2023, Parav Pandit <parav@nvidia.com> wrote:
> >> > @@ -440,6 +440,38 @@ \subsubsection{Virtqueue
> >> > Re-enable}\label{sec:Basic Facilities of a Virtio Devic  as during
> >> > initial virtqueue discovery, but optionally with different  parameters.
> >> >
> >> > +\subsection{Dynamic Virtqueues}\label{sec:Basic Facilities of a
> >> > +Virtio Device / Virtqueues / Dynamic Virtqueues}
> >> > +
> >> > +When VIRTIO_F_RING_DYNAMIC is not negotiated, the driver enables the
> >> > +virtqueues during the device initialization sequence, i.e. after the
> >> > +device sets the FEATURES_OK status bit and before the driver setting the
> >> DRIVER_OK status bit.
> >> 
> >> _Or_ if a virtqueue has been reset and the driver wants to re-enable it, right?
> >> 
> > Well no. Because above text is for "enablement", and not re-enablement.
> > If the driver want to reset and re-enable, it must be enabled in first place before setting driver_ok.
> > VQ not enabled before driver ok, cannot be reset, and hence cannot be re-enabled.
> >
> > So I think above text is fine because it says about "enables" and not "re-enables".
> >
> >> > +
> >> > +When VIRTIO_F_RING_DYNAMIC is negotiated, the driver can avoid
> >> > +enabling the virtqueues before setting the DRIVER_OK status bit; the
> >> > +driver can enable the specific virtqueues after the driver has set the
> >> DRIVER_OK status bit.
> >> 
> >> "the driver is not required to enable every virtqueue it wants to use before
> >> setting the DRIVER_OK status bit; it can choose to enable a virtqueue even after
> >> it has set the DRIVER_OK status bit."
> >> 
> > Sounds good. Will change it.
> >
> >> > +The virtqueue enable mechanism is transport specific.
> >> 
> >> Would that be the same mechanism as for re-enabling a queue after a queue
> >> reset? I guess I'm missing the relationship here...
> >> 
> > Yes, it is same.
> > There is no change in enabling/re-enabling the virtqueue after/before DRIVER_OK with/without _dynamic bit.
> > So no extra text added here.
> 
> This is not really clear to me just from this text, especially if you
> just wrote above that enabling or re-enabling is something
> different... my understanding would be:
> 
> - if neither dynamic vqs nor queue reset are supported or negotiated,
>   the only way to enable a vq is before DRIVER_OK, during setup
> - both of these features rely on the transport supporting enabling
>   individual queues (either a queue that has not been enabled before, or
>   a queue that has been reset)
> - the transport is supposed to use the same mechanism for either
> 
> Did I get it right? If so, I think we should make it a bit more clear.
> 
> (...)
> 
> >> > +When VIRTIO_F_RING_DYNAMIC is not negotiated, the driver MUST enable
> >> > +the required number of virtqueues before setting the DRIVER_OK status bit.
> >> 
> >> What does "required" mean here? It just chooses to enable the queues it wants
> >> to use, right?
> > Right.
> > Required meaning, whatever number of queues that driver choose to enable, those must be enabled before driver_ok.
> > So it is "required by the driver".
> > Would that be ok?
> 
> I'd write it as "the driver MUST enable any virtqueue it plans to use"
> or something like that.
> 
> (...)

It would have to be SHOULD - we can't add new MUST requirements not
contingent on a feature bit, we can give recommendation based on
existing installed base.

> >> We currently have a device normative statement:
> >> 
> >> "The device MUST NOT consume buffers or send any used buffer notifications to
> >> the driver before DRIVER_OK."
> >> 
> >> I guess we need to extend that to not doing that for not-yet-enabled queues in
> >> the dynamic virtqueue case? There's a (transport-specific) point in time when
> >> the driver tells the device that the queue is ready, right?
> >
> > We don’t need extend this text because device does not know anything about the disabled queue, so it cannot consume any buffer from it anyway.
> > Above line only applied to the enabled virtqueue.
> 
> My point is: DRIVER_OK signals that the driver is done with its setup
> and the device may start to interact with the queue. Depending on the
> mechanism the transport uses for enabling the queue, it may "know" about
> the queue before the driver is ready -- it might need to wait until the
> driver has completed enabling of the queue. Not sure if that is worth
> spelling out.


This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.

In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.

Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/

Re: [virtio-comment] RE: [PATCH v3 2/2] content: Support enabling virtqueue after DRIVER_OK stage

[Cornelia Huck]

On Wed, Oct 18 2023, "Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Wed, Oct 18, 2023 at 12:25:23PM +0200, Cornelia Huck wrote:
>> On Tue, Oct 17 2023, Parav Pandit <parav@nvidia.com> wrote:
>> 
>> >> From: Cornelia Huck <cohuck@redhat.com>
>> >> Sent: Tuesday, October 17, 2023 5:55 PM
>> >> 
>> >> On Mon, Oct 02 2023, Parav Pandit <parav@nvidia.com> wrote:
>> >> > +When VIRTIO_F_RING_DYNAMIC is not negotiated, the driver MUST enable
>> >> > +the required number of virtqueues before setting the DRIVER_OK status bit.
>> >> 
>> >> What does "required" mean here? It just chooses to enable the queues it wants
>> >> to use, right?
>> > Right.
>> > Required meaning, whatever number of queues that driver choose to enable, those must be enabled before driver_ok.
>> > So it is "required by the driver".
>> > Would that be ok?
>> 
>> I'd write it as "the driver MUST enable any virtqueue it plans to use"
>> or something like that.
>> 
>> (...)
>
> It would have to be SHOULD - we can't add new MUST requirements not
> contingent on a feature bit, we can give recommendation based on
> existing installed base.

Then I wonder whether we need to add any normative statement at all --
prior to the introduction of this new feature, the driver had to enable
anything it wanted to use before DRIVER_OK, and if it does not negotiate
the new feature, nothing changes. Spelling that out in non-normative
sections should be enough?


This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.

In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.

Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/

Re: [virtio-comment] RE: [PATCH v3 2/2] content: Support enabling virtqueue after DRIVER_OK stage

[Michael S. Tsirkin]

On Wed, Oct 18, 2023 at 01:03:57PM +0200, Cornelia Huck wrote:
> On Wed, Oct 18 2023, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> 
> > On Wed, Oct 18, 2023 at 12:25:23PM +0200, Cornelia Huck wrote:
> >> On Tue, Oct 17 2023, Parav Pandit <parav@nvidia.com> wrote:
> >> 
> >> >> From: Cornelia Huck <cohuck@redhat.com>
> >> >> Sent: Tuesday, October 17, 2023 5:55 PM
> >> >> 
> >> >> On Mon, Oct 02 2023, Parav Pandit <parav@nvidia.com> wrote:
> >> >> > +When VIRTIO_F_RING_DYNAMIC is not negotiated, the driver MUST enable
> >> >> > +the required number of virtqueues before setting the DRIVER_OK status bit.
> >> >> 
> >> >> What does "required" mean here? It just chooses to enable the queues it wants
> >> >> to use, right?
> >> > Right.
> >> > Required meaning, whatever number of queues that driver choose to enable, those must be enabled before driver_ok.
> >> > So it is "required by the driver".
> >> > Would that be ok?
> >> 
> >> I'd write it as "the driver MUST enable any virtqueue it plans to use"
> >> or something like that.
> >> 
> >> (...)
> >
> > It would have to be SHOULD - we can't add new MUST requirements not
> > contingent on a feature bit, we can give recommendation based on
> > existing installed base.
> 
> Then I wonder whether we need to add any normative statement at all --
> prior to the introduction of this new feature, the driver had to enable
> anything it wanted to use before DRIVER_OK, and if it does not negotiate
> the new feature, nothing changes. Spelling that out in non-normative
> sections should be enough?

I think it's not a bad thing to be clearer, but I don't care much.

-- 
MST


This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.

In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.

Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/

RE: [virtio-comment] RE: [PATCH v3 2/2] content: Support enabling virtqueue after DRIVER_OK stage

[Parav Pandit]

> From: Cornelia Huck <cohuck@redhat.com>
> Sent: Wednesday, October 18, 2023 4:34 PM
 
> On Wed, Oct 18 2023, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> 
> > On Wed, Oct 18, 2023 at 12:25:23PM +0200, Cornelia Huck wrote:
> >> On Tue, Oct 17 2023, Parav Pandit <parav@nvidia.com> wrote:
> >>
> >> >> From: Cornelia Huck <cohuck@redhat.com>
> >> >> Sent: Tuesday, October 17, 2023 5:55 PM
> >> >>
> >> >> On Mon, Oct 02 2023, Parav Pandit <parav@nvidia.com> wrote:
> >> >> > +When VIRTIO_F_RING_DYNAMIC is not negotiated, the driver MUST
> >> >> > +enable the required number of virtqueues before setting the
> DRIVER_OK status bit.
> >> >>
> >> >> What does "required" mean here? It just chooses to enable the
> >> >> queues it wants to use, right?
> >> > Right.
> >> > Required meaning, whatever number of queues that driver choose to
> enable, those must be enabled before driver_ok.
> >> > So it is "required by the driver".
> >> > Would that be ok?
> >>
> >> I'd write it as "the driver MUST enable any virtqueue it plans to use"
> >> or something like that.
> >>
> >> (...)
> >
> > It would have to be SHOULD - we can't add new MUST requirements not
> > contingent on a feature bit, we can give recommendation based on
> > existing installed base.
> 
It is not just installed base.
Spec clearly says that:
"Perform device-specific setup, including discovery of virtqueues for the device, optional per-bus setup,
reading and possibly writing the device's virtio configuration space, and population of virtqueues."

There is no feature bit or any text that says "population of virtqueues" can be done later outside of the device initialization phase.
Only reference is _F_RESET bit which is only for re-enabling. (not enabling).

> Then I wonder whether we need to add any normative statement at all -- prior
> to the introduction of this new feature, the driver had to enable anything it
> wanted to use before DRIVER_OK, and if it does not negotiate the new feature,
> nothing changes. Spelling that out in non-normative sections should be
> enough?
It is already implied in the spec that queues must be enabled before DRIVER_OK.
So what is written in normative is not changing any behavior.
It is only making the implied behavior explicit.

If it says SHOULD, that means, one can enable the queue for the first time after DRIVER_OK stage too.
And that would violate the "Driver Requirements: Device Initialization".


This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.

In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.

Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/

RE: [virtio-comment] RE: [PATCH v3 2/2] content: Support enabling virtqueue after DRIVER_OK stage

[Parav Pandit]

Hi Cornelia, Michael,

> From: virtio-comment@lists.oasis-open.org <virtio-comment@lists.oasis-
> open.org> On Behalf Of Parav Pandit
> Sent: Wednesday, October 18, 2023 4:43 PM
> 
> > From: Cornelia Huck <cohuck@redhat.com>
> > Sent: Wednesday, October 18, 2023 4:34 PM
> 
> > On Wed, Oct 18 2023, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> >
> > > On Wed, Oct 18, 2023 at 12:25:23PM +0200, Cornelia Huck wrote:
> > >> On Tue, Oct 17 2023, Parav Pandit <parav@nvidia.com> wrote:
> > >>
> > >> >> From: Cornelia Huck <cohuck@redhat.com>
> > >> >> Sent: Tuesday, October 17, 2023 5:55 PM
> > >> >>
> > >> >> On Mon, Oct 02 2023, Parav Pandit <parav@nvidia.com> wrote:
> > >> >> > +When VIRTIO_F_RING_DYNAMIC is not negotiated, the driver MUST
> > >> >> > +enable the required number of virtqueues before setting the
> > DRIVER_OK status bit.
> > >> >>
> > >> >> What does "required" mean here? It just chooses to enable the
> > >> >> queues it wants to use, right?
> > >> > Right.
> > >> > Required meaning, whatever number of queues that driver choose to
> > enable, those must be enabled before driver_ok.
> > >> > So it is "required by the driver".
> > >> > Would that be ok?
> > >>
> > >> I'd write it as "the driver MUST enable any virtqueue it plans to use"
> > >> or something like that.
> > >>
> > >> (...)
> > >
> > > It would have to be SHOULD - we can't add new MUST requirements not
> > > contingent on a feature bit, we can give recommendation based on
> > > existing installed base.
> >
> It is not just installed base.
> Spec clearly says that:
> "Perform device-specific setup, including discovery of virtqueues for the device,
> optional per-bus setup, reading and possibly writing the device's virtio
> configuration space, and population of virtqueues."
> 
> There is no feature bit or any text that says "population of virtqueues" can be
> done later outside of the device initialization phase.
> Only reference is _F_RESET bit which is only for re-enabling. (not enabling).
> 
> > Then I wonder whether we need to add any normative statement at all --
> > prior to the introduction of this new feature, the driver had to
> > enable anything it wanted to use before DRIVER_OK, and if it does not
> > negotiate the new feature, nothing changes. Spelling that out in
> > non-normative sections should be enough?
> It is already implied in the spec that queues must be enabled before
> DRIVER_OK.
> So what is written in normative is not changing any behavior.
> It is only making the implied behavior explicit.
> 
> If it says SHOULD, that means, one can enable the queue for the first time after
> DRIVER_OK stage too.
> And that would violate the "Driver Requirements: Device Initialization".
> 

I have addressed the comments of v3 in v4 at [1].
The device requirement is kept as MUST to keep it aligned with the "driver requirements section".'

[1] https://lore.kernel.org/virtio-comment/20231017143135.758523-1-parav@nvidia.com/T/#mc49b6e54186c5f1b1e8c743bc440ca0cc0938212

Please review.

This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.

In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.

Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/

RE: [virtio-comment] RE: [PATCH v3 2/2] content: Support enabling virtqueue after DRIVER_OK stage

[Parav Pandit]

Hi Michael, Cornelia,

> From: Parav Pandit
> Sent: Thursday, October 19, 2023 7:27 PM
> 
> Hi Cornelia, Michael,
> 
> > From: virtio-comment@lists.oasis-open.org <virtio-comment@lists.oasis-
> > open.org> On Behalf Of Parav Pandit
> > Sent: Wednesday, October 18, 2023 4:43 PM
> >
> > > From: Cornelia Huck <cohuck@redhat.com>
> > > Sent: Wednesday, October 18, 2023 4:34 PM
> >
> > > On Wed, Oct 18 2023, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > >
> > > > On Wed, Oct 18, 2023 at 12:25:23PM +0200, Cornelia Huck wrote:
> > > >> On Tue, Oct 17 2023, Parav Pandit <parav@nvidia.com> wrote:
> > > >>
> > > >> >> From: Cornelia Huck <cohuck@redhat.com>
> > > >> >> Sent: Tuesday, October 17, 2023 5:55 PM
> > > >> >>
> > > >> >> On Mon, Oct 02 2023, Parav Pandit <parav@nvidia.com> wrote:
> > > >> >> > +When VIRTIO_F_RING_DYNAMIC is not negotiated, the driver
> > > >> >> > +MUST enable the required number of virtqueues before
> > > >> >> > +setting the
> > > DRIVER_OK status bit.
> > > >> >>
> > > >> >> What does "required" mean here? It just chooses to enable the
> > > >> >> queues it wants to use, right?
> > > >> > Right.
> > > >> > Required meaning, whatever number of queues that driver choose
> > > >> > to
> > > enable, those must be enabled before driver_ok.
> > > >> > So it is "required by the driver".
> > > >> > Would that be ok?
> > > >>
> > > >> I'd write it as "the driver MUST enable any virtqueue it plans to use"
> > > >> or something like that.
> > > >>
> > > >> (...)
> > > >
> > > > It would have to be SHOULD - we can't add new MUST requirements
> > > > not contingent on a feature bit, we can give recommendation based
> > > > on existing installed base.
> > >
> > It is not just installed base.
> > Spec clearly says that:
> > "Perform device-specific setup, including discovery of virtqueues for
> > the device, optional per-bus setup, reading and possibly writing the
> > device's virtio configuration space, and population of virtqueues."
> >
> > There is no feature bit or any text that says "population of
> > virtqueues" can be done later outside of the device initialization phase.
> > Only reference is _F_RESET bit which is only for re-enabling. (not enabling).
> >
> > > Then I wonder whether we need to add any normative statement at all
> > > -- prior to the introduction of this new feature, the driver had to
> > > enable anything it wanted to use before DRIVER_OK, and if it does
> > > not negotiate the new feature, nothing changes. Spelling that out in
> > > non-normative sections should be enough?
> > It is already implied in the spec that queues must be enabled before
> > DRIVER_OK.
> > So what is written in normative is not changing any behavior.
> > It is only making the implied behavior explicit.
> >
> > If it says SHOULD, that means, one can enable the queue for the first
> > time after DRIVER_OK stage too.
> > And that would violate the "Driver Requirements: Device Initialization".
> >
> 
> I have addressed the comments of v3 in v4 at [1].
> The device requirement is kept as MUST to keep it aligned with the "driver
> requirements section".'
> 
> [1] https://lore.kernel.org/virtio-comment/20231017143135.758523-1-
> parav@nvidia.com/T/#mc49b6e54186c5f1b1e8c743bc440ca0cc0938212
> 
> Please review.

If no comments in v4 which was a week ago, please raise the voting ballot.

[1] https://lists.oasis-open.org/archives/virtio-comment/202310/msg00183.html

Parav

This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.

In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.

Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/

RE: [virtio-comment] RE: [PATCH v3 2/2] content: Support enabling virtqueue after DRIVER_OK stage

[Cornelia Huck]

On Mon, Oct 23 2023, Parav Pandit <parav@nvidia.com> wrote:

> Hi Michael, Cornelia,
>
>> From: Parav Pandit
>> Sent: Thursday, October 19, 2023 7:27 PM
>> 
>> Hi Cornelia, Michael,
>> 
>> > From: virtio-comment@lists.oasis-open.org <virtio-comment@lists.oasis-
>> > open.org> On Behalf Of Parav Pandit
>> > Sent: Wednesday, October 18, 2023 4:43 PM
>> >
>> > > From: Cornelia Huck <cohuck@redhat.com>
>> > > Sent: Wednesday, October 18, 2023 4:34 PM
>> >
>> > > On Wed, Oct 18 2023, "Michael S. Tsirkin" <mst@redhat.com> wrote:
>> > >
>> > > > On Wed, Oct 18, 2023 at 12:25:23PM +0200, Cornelia Huck wrote:
>> > > >> On Tue, Oct 17 2023, Parav Pandit <parav@nvidia.com> wrote:
>> > > >>
>> > > >> >> From: Cornelia Huck <cohuck@redhat.com>
>> > > >> >> Sent: Tuesday, October 17, 2023 5:55 PM
>> > > >> >>
>> > > >> >> On Mon, Oct 02 2023, Parav Pandit <parav@nvidia.com> wrote:
>> > > >> >> > +When VIRTIO_F_RING_DYNAMIC is not negotiated, the driver
>> > > >> >> > +MUST enable the required number of virtqueues before
>> > > >> >> > +setting the
>> > > DRIVER_OK status bit.
>> > > >> >>
>> > > >> >> What does "required" mean here? It just chooses to enable the
>> > > >> >> queues it wants to use, right?
>> > > >> > Right.
>> > > >> > Required meaning, whatever number of queues that driver choose
>> > > >> > to
>> > > enable, those must be enabled before driver_ok.
>> > > >> > So it is "required by the driver".
>> > > >> > Would that be ok?
>> > > >>
>> > > >> I'd write it as "the driver MUST enable any virtqueue it plans to use"
>> > > >> or something like that.
>> > > >>
>> > > >> (...)
>> > > >
>> > > > It would have to be SHOULD - we can't add new MUST requirements
>> > > > not contingent on a feature bit, we can give recommendation based
>> > > > on existing installed base.
>> > >
>> > It is not just installed base.
>> > Spec clearly says that:
>> > "Perform device-specific setup, including discovery of virtqueues for
>> > the device, optional per-bus setup, reading and possibly writing the
>> > device's virtio configuration space, and population of virtqueues."
>> >
>> > There is no feature bit or any text that says "population of
>> > virtqueues" can be done later outside of the device initialization phase.
>> > Only reference is _F_RESET bit which is only for re-enabling. (not enabling).
>> >
>> > > Then I wonder whether we need to add any normative statement at all
>> > > -- prior to the introduction of this new feature, the driver had to
>> > > enable anything it wanted to use before DRIVER_OK, and if it does
>> > > not negotiate the new feature, nothing changes. Spelling that out in
>> > > non-normative sections should be enough?
>> > It is already implied in the spec that queues must be enabled before
>> > DRIVER_OK.
>> > So what is written in normative is not changing any behavior.
>> > It is only making the implied behavior explicit.
>> >
>> > If it says SHOULD, that means, one can enable the queue for the first
>> > time after DRIVER_OK stage too.
>> > And that would violate the "Driver Requirements: Device Initialization".
>> >
>> 
>> I have addressed the comments of v3 in v4 at [1].
>> The device requirement is kept as MUST to keep it aligned with the "driver
>> requirements section".'
>> 
>> [1] https://lore.kernel.org/virtio-comment/20231017143135.758523-1-
>> parav@nvidia.com/T/#mc49b6e54186c5f1b1e8c743bc440ca0cc0938212
>> 
>> Please review.
>
> If no comments in v4 which was a week ago, please raise the voting ballot.

Well, there are  still comments from me here that post-date v4 (please
wait for a bit before posting another version!), so I'll continue
waiting for them to be addressed first.

>
> [1] https://lists.oasis-open.org/archives/virtio-comment/202310/msg00183.html
>
> Parav


This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.

In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.

Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/

RE: [virtio-comment] RE: [PATCH v3 2/2] content: Support enabling virtqueue after DRIVER_OK stage

[Parav Pandit]

> From: virtio-comment@lists.oasis-open.org <virtio-comment@lists.oasis-
> open.org> On Behalf Of Cornelia Huck
> Sent: Monday, October 23, 2023 7:27 PM

> Well, there are  still comments from me here that post-date v4 (please wait for
> a bit before posting another version!), so I'll continue waiting for them to be
> addressed first.
The last one was [1].
Which I replied few days ago at [2].

[1] https://lists.oasis-open.org/archives/virtio-comment/202310/msg00240.html
[2] https://lists.oasis-open.org/archives/virtio-comment/202310/msg00242.html

This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.

In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.

Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/

RE: [virtio-comment] RE: [PATCH v3 2/2] content: Support enabling virtqueue after DRIVER_OK stage

[Cornelia Huck]

On Mon, Oct 23 2023, Parav Pandit <parav@nvidia.com> wrote:

>> From: virtio-comment@lists.oasis-open.org <virtio-comment@lists.oasis-
>> open.org> On Behalf Of Cornelia Huck
>> Sent: Monday, October 23, 2023 7:27 PM
>
>> Well, there are  still comments from me here that post-date v4 (please wait for
>> a bit before posting another version!), so I'll continue waiting for them to be
>> addressed first.
> The last one was [1].
> Which I replied few days ago at [2].
>
> [1] https://lists.oasis-open.org/archives/virtio-comment/202310/msg00240.html
> [2] https://lists.oasis-open.org/archives/virtio-comment/202310/msg00242.html

I still have some open questions from <87sf68cjn0.fsf@redhat.com> (first
and last paragraph.)


This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.

In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.

Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/

RE: [virtio-comment] RE: [PATCH v3 2/2] content: Support enabling virtqueue after DRIVER_OK stage

[Parav Pandit]

> From: Cornelia Huck <cohuck@redhat.com>
> Sent: Monday, October 23, 2023 7:58 PM
 
> On Mon, Oct 23 2023, Parav Pandit <parav@nvidia.com> wrote:
> 
> >> From: virtio-comment@lists.oasis-open.org
> >> <virtio-comment@lists.oasis- open.org> On Behalf Of Cornelia Huck
> >> Sent: Monday, October 23, 2023 7:27 PM
> >
> >> Well, there are  still comments from me here that post-date v4
> >> (please wait for a bit before posting another version!), so I'll
> >> continue waiting for them to be addressed first.
> > The last one was [1].
> > Which I replied few days ago at [2].
> >
> > [1]
> > https://lists.oasis-open.org/archives/virtio-comment/202310/msg00240.h
> > tml [2]
> > https://lists.oasis-open.org/archives/virtio-comment/202310/msg00242.h
> > tml
> 
> I still have some open questions from <87sf68cjn0.fsf@redhat.com> (first and
> last paragraph.)

Above link is not accessible to me. ☹
I addressed your comments/questions in v4.
Can you please check v4 if they are addressed? I rewrote as you suggested in v3.

RE: [virtio-comment] RE: [PATCH v3 2/2] content: Support enabling virtqueue after DRIVER_OK stage

[Cornelia Huck]

On Mon, Oct 23 2023, Parav Pandit <parav@nvidia.com> wrote:

>> From: Cornelia Huck <cohuck@redhat.com>
>> Sent: Monday, October 23, 2023 7:58 PM
>  
>> On Mon, Oct 23 2023, Parav Pandit <parav@nvidia.com> wrote:
>> 
>> >> From: virtio-comment@lists.oasis-open.org
>> >> <virtio-comment@lists.oasis- open.org> On Behalf Of Cornelia Huck
>> >> Sent: Monday, October 23, 2023 7:27 PM
>> >
>> >> Well, there are  still comments from me here that post-date v4
>> >> (please wait for a bit before posting another version!), so I'll
>> >> continue waiting for them to be addressed first.
>> > The last one was [1].
>> > Which I replied few days ago at [2].
>> >
>> > [1]
>> > https://lists.oasis-open.org/archives/virtio-comment/202310/msg00240.h
>> > tml [2]
>> > https://lists.oasis-open.org/archives/virtio-comment/202310/msg00242.h
>> > tml
>> 
>> I still have some open questions from <87sf68cjn0.fsf@redhat.com> (first and
>> last paragraph.)
>
> Above link is not accessible to me. ☹

That's not a link, but a message id... many mail clients support
searching by it, and you can also get it from lore:

https://lore.kernel.org/all/87sf68cjn0.fsf@redhat.com/

> I addressed your comments/questions in v4.
> Can you please check v4 if they are addressed? I rewrote as you suggested in v3.

This was for points you did not agree with. (Again, it might be helpful
to wait with posting a new version until discussion has somewhat
concluded.)


This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.

In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.

Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/

RE: [virtio-comment] RE: [PATCH v3 2/2] content: Support enabling virtqueue after DRIVER_OK stage

[Parav Pandit]

> From: virtio-comment@lists.oasis-open.org <virtio-comment@lists.oasis-
> open.org> On Behalf Of Cornelia Huck
> Sent: Monday, October 23, 2023 8:31 PM
> 
> On Mon, Oct 23 2023, Parav Pandit <parav@nvidia.com> wrote:
> 
> >> From: Cornelia Huck <cohuck@redhat.com>
> >> Sent: Monday, October 23, 2023 7:58 PM
> >
> >> On Mon, Oct 23 2023, Parav Pandit <parav@nvidia.com> wrote:
> >>
> >> >> From: virtio-comment@lists.oasis-open.org
> >> >> <virtio-comment@lists.oasis- open.org> On Behalf Of Cornelia Huck
> >> >> Sent: Monday, October 23, 2023 7:27 PM
> >> >
> >> >> Well, there are  still comments from me here that post-date v4
> >> >> (please wait for a bit before posting another version!), so I'll
> >> >> continue waiting for them to be addressed first.
> >> > The last one was [1].
> >> > Which I replied few days ago at [2].
> >> >
> >> > [1]
> >> > https://lists.oasis-open.org/archives/virtio-comment/202310/msg0024
> >> > 0.h
> >> > tml [2]
> >> > https://lists.oasis-open.org/archives/virtio-comment/202310/msg0024
> >> > 2.h
> >> > tml
> >>
> >> I still have some open questions from <87sf68cjn0.fsf@redhat.com>
> >> (first and last paragraph.)
> >
> > Above link is not accessible to me. ☹
> 
> That's not a link, but a message id... many mail clients support searching by it,
> and you can also get it from lore:
> 
> https://lore.kernel.org/all/87sf68cjn0.fsf@redhat.com/
> 
> > I addressed your comments/questions in v4.
> > Can you please check v4 if they are addressed? I rewrote as you suggested in
> v3.
> 
> This was for points you did not agree with. (Again, it might be helpful to wait
> with posting a new version until discussion has somewhat
> concluded.)
Sure, will wait next time. 

Your comment was,
=====
This is not really clear to me just from this text, especially if you
just wrote above that enabling or re-enabling is something
different... my understanding would be:

- if neither dynamic vqs nor queue reset are supported or negotiated,
  the only way to enable a vq is before DRIVER_OK, during setup
- both of these features rely on the transport supporting enabling
  individual queues (either a queue that has not been enabled before, or
  a queue that has been reset)
- the transport is supposed to use the same mechanism for either

Did I get it right? If so, I think we should make it a bit more clear.
=====

Above is clarified in below wording without complicating the queue_reset here.

Does that look ok to you?

+When VIRTIO_F_RING_DYNAMIC is not negotiated, the driver enables the virtqueues
+during the device initialization sequence, i.e. after the device sets the
+FEATURES_OK status bit and before the driver setting the DRIVER_OK status bit.
+
+When VIRTIO_F_RING_DYNAMIC is negotiated, the driver is not required to enable
+every virtqueue it wants to use before setting the DRIVER_OK status bit; the
+driver can choose to enable a virtqueue even after the driver has set the
+DRIVER_OK status bit. The virtqueue enable mechanism is transport specific.
+

RE: [virtio-comment] RE: [PATCH v3 2/2] content: Support enabling virtqueue after DRIVER_OK stage

[Parav Pandit]

Hi Cornelia,

> From: Parav Pandit <parav@nvidia.com>
> Sent: Monday, October 23, 2023 8:52 PM
> To: Cornelia Huck <cohuck@redhat.com>; Michael S. Tsirkin
> <mst@redhat.com>
> Cc: virtio-comment@lists.oasis-open.org; hengqi@linux.alibaba.com;
> xuanzhuo@linux.alibaba.com; Shahaf Shuler <shahafs@nvidia.com>
> Subject: RE: [virtio-comment] RE: [PATCH v3 2/2] content: Support enabling
> virtqueue after DRIVER_OK stage
> 
> 
> > From: virtio-comment@lists.oasis-open.org <virtio-comment@lists.oasis-
> > open.org> On Behalf Of Cornelia Huck
> > Sent: Monday, October 23, 2023 8:31 PM
> >
> > On Mon, Oct 23 2023, Parav Pandit <parav@nvidia.com> wrote:
> >
> > >> From: Cornelia Huck <cohuck@redhat.com>
> > >> Sent: Monday, October 23, 2023 7:58 PM
> > >
> > >> On Mon, Oct 23 2023, Parav Pandit <parav@nvidia.com> wrote:
> > >>
> > >> >> From: virtio-comment@lists.oasis-open.org
> > >> >> <virtio-comment@lists.oasis- open.org> On Behalf Of Cornelia
> > >> >> Huck
> > >> >> Sent: Monday, October 23, 2023 7:27 PM
> > >> >
> > >> >> Well, there are  still comments from me here that post-date v4
> > >> >> (please wait for a bit before posting another version!), so I'll
> > >> >> continue waiting for them to be addressed first.
> > >> > The last one was [1].
> > >> > Which I replied few days ago at [2].
> > >> >
> > >> > [1]
> > >> > https://lists.oasis-open.org/archives/virtio-comment/202310/msg00
> > >> > 24
> > >> > 0.h
> > >> > tml [2]
> > >> > https://lists.oasis-open.org/archives/virtio-comment/202310/msg00
> > >> > 24
> > >> > 2.h
> > >> > tml
> > >>
> > >> I still have some open questions from <87sf68cjn0.fsf@redhat.com>
> > >> (first and last paragraph.)
> > >
> > > Above link is not accessible to me. ☹
> >
> > That's not a link, but a message id... many mail clients support
> > searching by it, and you can also get it from lore:
> >
> > https://lore.kernel.org/all/87sf68cjn0.fsf@redhat.com/
> >
> > > I addressed your comments/questions in v4.
> > > Can you please check v4 if they are addressed? I rewrote as you
> > > suggested in
> > v3.
> >
> > This was for points you did not agree with. (Again, it might be
> > helpful to wait with posting a new version until discussion has
> > somewhat
> > concluded.)
> Sure, will wait next time.
> 
> Your comment was,
> =====
> This is not really clear to me just from this text, especially if you just wrote
> above that enabling or re-enabling is something different... my understanding
> would be:
> 
> - if neither dynamic vqs nor queue reset are supported or negotiated,
>   the only way to enable a vq is before DRIVER_OK, during setup
> - both of these features rely on the transport supporting enabling
>   individual queues (either a queue that has not been enabled before, or
>   a queue that has been reset)
> - the transport is supposed to use the same mechanism for either
> 
> Did I get it right? If so, I think we should make it a bit more clear.
> =====
> 
> Above is clarified in below wording without complicating the queue_reset here.
> 
> Does that look ok to you?
> 
> +When VIRTIO_F_RING_DYNAMIC is not negotiated, the driver enables the
> +virtqueues during the device initialization sequence, i.e. after the
> +device sets the FEATURES_OK status bit and before the driver setting the
> DRIVER_OK status bit.
> +
> +When VIRTIO_F_RING_DYNAMIC is negotiated, the driver is not required to
> +enable every virtqueue it wants to use before setting the DRIVER_OK
> +status bit; the driver can choose to enable a virtqueue even after the
> +driver has set the DRIVER_OK status bit. The virtqueue enable mechanism is
> transport specific.
> +

Can you please respond?
We must make progress with this and the flow filters which are depending on this for a long time now.

RE: [virtio-comment] RE: [PATCH v3 2/2] content: Support enabling virtqueue after DRIVER_OK stage

[Cornelia Huck]

On Wed, Oct 25 2023, Parav Pandit <parav@nvidia.com> wrote:

> Hi Cornelia,
>
>> From: Parav Pandit <parav@nvidia.com>
>> Sent: Monday, October 23, 2023 8:52 PM
>> To: Cornelia Huck <cohuck@redhat.com>; Michael S. Tsirkin
>> <mst@redhat.com>
>> Cc: virtio-comment@lists.oasis-open.org; hengqi@linux.alibaba.com;
>> xuanzhuo@linux.alibaba.com; Shahaf Shuler <shahafs@nvidia.com>
>> Subject: RE: [virtio-comment] RE: [PATCH v3 2/2] content: Support enabling
>> virtqueue after DRIVER_OK stage
>> 
>> 
>> > From: virtio-comment@lists.oasis-open.org <virtio-comment@lists.oasis-
>> > open.org> On Behalf Of Cornelia Huck
>> > Sent: Monday, October 23, 2023 8:31 PM
>> >
>> > On Mon, Oct 23 2023, Parav Pandit <parav@nvidia.com> wrote:
>> >
>> > >> From: Cornelia Huck <cohuck@redhat.com>
>> > >> Sent: Monday, October 23, 2023 7:58 PM
>> > >
>> > >> On Mon, Oct 23 2023, Parav Pandit <parav@nvidia.com> wrote:
>> > >>
>> > >> >> From: virtio-comment@lists.oasis-open.org
>> > >> >> <virtio-comment@lists.oasis- open.org> On Behalf Of Cornelia
>> > >> >> Huck
>> > >> >> Sent: Monday, October 23, 2023 7:27 PM
>> > >> >
>> > >> >> Well, there are  still comments from me here that post-date v4
>> > >> >> (please wait for a bit before posting another version!), so I'll
>> > >> >> continue waiting for them to be addressed first.
>> > >> > The last one was [1].
>> > >> > Which I replied few days ago at [2].
>> > >> >
>> > >> > [1]
>> > >> > https://lists.oasis-open.org/archives/virtio-comment/202310/msg00
>> > >> > 24
>> > >> > 0.h
>> > >> > tml [2]
>> > >> > https://lists.oasis-open.org/archives/virtio-comment/202310/msg00
>> > >> > 24
>> > >> > 2.h
>> > >> > tml
>> > >>
>> > >> I still have some open questions from <87sf68cjn0.fsf@redhat.com>
>> > >> (first and last paragraph.)
>> > >
>> > > Above link is not accessible to me. ☹
>> >
>> > That's not a link, but a message id... many mail clients support
>> > searching by it, and you can also get it from lore:
>> >
>> > https://lore.kernel.org/all/87sf68cjn0.fsf@redhat.com/
>> >
>> > > I addressed your comments/questions in v4.
>> > > Can you please check v4 if they are addressed? I rewrote as you
>> > > suggested in
>> > v3.
>> >
>> > This was for points you did not agree with. (Again, it might be
>> > helpful to wait with posting a new version until discussion has
>> > somewhat
>> > concluded.)
>> Sure, will wait next time.
>> 
>> Your comment was,
>> =====
>> This is not really clear to me just from this text, especially if you just wrote
>> above that enabling or re-enabling is something different... my understanding
>> would be:
>> 
>> - if neither dynamic vqs nor queue reset are supported or negotiated,
>>   the only way to enable a vq is before DRIVER_OK, during setup
>> - both of these features rely on the transport supporting enabling
>>   individual queues (either a queue that has not been enabled before, or
>>   a queue that has been reset)
>> - the transport is supposed to use the same mechanism for either
>> 
>> Did I get it right? If so, I think we should make it a bit more clear.
>> =====
>> 
>> Above is clarified in below wording without complicating the queue_reset here.
>> 
>> Does that look ok to you?
>> 
>> +When VIRTIO_F_RING_DYNAMIC is not negotiated, the driver enables the
>> +virtqueues during the device initialization sequence, i.e. after the
>> +device sets the FEATURES_OK status bit and before the driver setting the
>> DRIVER_OK status bit.
>> +
>> +When VIRTIO_F_RING_DYNAMIC is negotiated, the driver is not required to
>> +enable every virtqueue it wants to use before setting the DRIVER_OK
>> +status bit; the driver can choose to enable a virtqueue even after the
>> +driver has set the DRIVER_OK status bit. The virtqueue enable mechanism is
>> transport specific.
>> +
>
> Can you please respond?
> We must make progress with this and the flow filters which are depending on this for a long time now.

I will not be able to look at this before Friday.


This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.

In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.

Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/

RE: [virtio-comment] RE: [PATCH v3 2/2] content: Support enabling virtqueue after DRIVER_OK stage

[Parav Pandit]

> From: Cornelia Huck <cohuck@redhat.com>
> Sent: Wednesday, October 25, 2023 3:48 PM
> 
> On Wed, Oct 25 2023, Parav Pandit <parav@nvidia.com> wrote:
> 
> > Hi Cornelia,
> >
> >> From: Parav Pandit <parav@nvidia.com>
> >> Sent: Monday, October 23, 2023 8:52 PM
> >> To: Cornelia Huck <cohuck@redhat.com>; Michael S. Tsirkin
> >> <mst@redhat.com>
> >> Cc: virtio-comment@lists.oasis-open.org; hengqi@linux.alibaba.com;
> >> xuanzhuo@linux.alibaba.com; Shahaf Shuler <shahafs@nvidia.com>
> >> Subject: RE: [virtio-comment] RE: [PATCH v3 2/2] content: Support
> >> enabling virtqueue after DRIVER_OK stage
> >>
> >>
> >> > From: virtio-comment@lists.oasis-open.org
> >> > <virtio-comment@lists.oasis- open.org> On Behalf Of Cornelia Huck
> >> > Sent: Monday, October 23, 2023 8:31 PM
> >> >
> >> > On Mon, Oct 23 2023, Parav Pandit <parav@nvidia.com> wrote:
> >> >
> >> > >> From: Cornelia Huck <cohuck@redhat.com>
> >> > >> Sent: Monday, October 23, 2023 7:58 PM
> >> > >
> >> > >> On Mon, Oct 23 2023, Parav Pandit <parav@nvidia.com> wrote:
> >> > >>
> >> > >> >> From: virtio-comment@lists.oasis-open.org
> >> > >> >> <virtio-comment@lists.oasis- open.org> On Behalf Of Cornelia
> >> > >> >> Huck
> >> > >> >> Sent: Monday, October 23, 2023 7:27 PM
> >> > >> >
> >> > >> >> Well, there are  still comments from me here that post-date
> >> > >> >> v4 (please wait for a bit before posting another version!),
> >> > >> >> so I'll continue waiting for them to be addressed first.
> >> > >> > The last one was [1].
> >> > >> > Which I replied few days ago at [2].
> >> > >> >
> >> > >> > [1]
> >> > >> > https://lists.oasis-open.org/archives/virtio-comment/202310/ms
> >> > >> > g00
> >> > >> > 24
> >> > >> > 0.h
> >> > >> > tml [2]
> >> > >> > https://lists.oasis-open.org/archives/virtio-comment/202310/ms
> >> > >> > g00
> >> > >> > 24
> >> > >> > 2.h
> >> > >> > tml
> >> > >>
> >> > >> I still have some open questions from
> >> > >> <87sf68cjn0.fsf@redhat.com> (first and last paragraph.)
> >> > >
> >> > > Above link is not accessible to me. ☹
> >> >
> >> > That's not a link, but a message id... many mail clients support
> >> > searching by it, and you can also get it from lore:
> >> >
> >> > https://lore.kernel.org/all/87sf68cjn0.fsf@redhat.com/
> >> >
> >> > > I addressed your comments/questions in v4.
> >> > > Can you please check v4 if they are addressed? I rewrote as you
> >> > > suggested in
> >> > v3.
> >> >
> >> > This was for points you did not agree with. (Again, it might be
> >> > helpful to wait with posting a new version until discussion has
> >> > somewhat
> >> > concluded.)
> >> Sure, will wait next time.
> >>
> >> Your comment was,
> >> =====
> >> This is not really clear to me just from this text, especially if you
> >> just wrote above that enabling or re-enabling is something
> >> different... my understanding would be:
> >>
> >> - if neither dynamic vqs nor queue reset are supported or negotiated,
> >>   the only way to enable a vq is before DRIVER_OK, during setup
> >> - both of these features rely on the transport supporting enabling
> >>   individual queues (either a queue that has not been enabled before, or
> >>   a queue that has been reset)
> >> - the transport is supposed to use the same mechanism for either
> >>
> >> Did I get it right? If so, I think we should make it a bit more clear.
> >> =====
> >>
> >> Above is clarified in below wording without complicating the queue_reset
> here.
> >>
> >> Does that look ok to you?
> >>
> >> +When VIRTIO_F_RING_DYNAMIC is not negotiated, the driver enables the
> >> +virtqueues during the device initialization sequence, i.e. after the
> >> +device sets the FEATURES_OK status bit and before the driver setting
> >> +the
> >> DRIVER_OK status bit.
> >> +
> >> +When VIRTIO_F_RING_DYNAMIC is negotiated, the driver is not required
> >> +to enable every virtqueue it wants to use before setting the
> >> +DRIVER_OK status bit; the driver can choose to enable a virtqueue
> >> +even after the driver has set the DRIVER_OK status bit. The
> >> +virtqueue enable mechanism is
> >> transport specific.
> >> +
> >
> > Can you please respond?
> > We must make progress with this and the flow filters which are depending on
> this for a long time now.
> 
> I will not be able to look at this before Friday.
Ok. Thanks for the update.
Will seek for your response on Friday.

Re: [virtio-comment] RE: [PATCH v3 2/2] content: Support enabling virtqueue after DRIVER_OK stage

[Michael S. Tsirkin]

On Wed, Oct 25, 2023 at 09:55:20AM +0000, Parav Pandit wrote:
> Hi Cornelia,
> 
> > From: Parav Pandit <parav@nvidia.com>
> > Sent: Monday, October 23, 2023 8:52 PM
> > To: Cornelia Huck <cohuck@redhat.com>; Michael S. Tsirkin
> > <mst@redhat.com>
> > Cc: virtio-comment@lists.oasis-open.org; hengqi@linux.alibaba.com;
> > xuanzhuo@linux.alibaba.com; Shahaf Shuler <shahafs@nvidia.com>
> > Subject: RE: [virtio-comment] RE: [PATCH v3 2/2] content: Support enabling
> > virtqueue after DRIVER_OK stage
> > 
> > 
> > > From: virtio-comment@lists.oasis-open.org <virtio-comment@lists.oasis-
> > > open.org> On Behalf Of Cornelia Huck
> > > Sent: Monday, October 23, 2023 8:31 PM
> > >
> > > On Mon, Oct 23 2023, Parav Pandit <parav@nvidia.com> wrote:
> > >
> > > >> From: Cornelia Huck <cohuck@redhat.com>
> > > >> Sent: Monday, October 23, 2023 7:58 PM
> > > >
> > > >> On Mon, Oct 23 2023, Parav Pandit <parav@nvidia.com> wrote:
> > > >>
> > > >> >> From: virtio-comment@lists.oasis-open.org
> > > >> >> <virtio-comment@lists.oasis- open.org> On Behalf Of Cornelia
> > > >> >> Huck
> > > >> >> Sent: Monday, October 23, 2023 7:27 PM
> > > >> >
> > > >> >> Well, there are  still comments from me here that post-date v4
> > > >> >> (please wait for a bit before posting another version!), so I'll
> > > >> >> continue waiting for them to be addressed first.
> > > >> > The last one was [1].
> > > >> > Which I replied few days ago at [2].
> > > >> >
> > > >> > [1]
> > > >> > https://lists.oasis-open.org/archives/virtio-comment/202310/msg00
> > > >> > 24
> > > >> > 0.h
> > > >> > tml [2]
> > > >> > https://lists.oasis-open.org/archives/virtio-comment/202310/msg00
> > > >> > 24
> > > >> > 2.h
> > > >> > tml
> > > >>
> > > >> I still have some open questions from <87sf68cjn0.fsf@redhat.com>
> > > >> (first and last paragraph.)
> > > >
> > > > Above link is not accessible to me. ☹
> > >
> > > That's not a link, but a message id... many mail clients support
> > > searching by it, and you can also get it from lore:
> > >
> > > https://lore.kernel.org/all/87sf68cjn0.fsf@redhat.com/
> > >
> > > > I addressed your comments/questions in v4.
> > > > Can you please check v4 if they are addressed? I rewrote as you
> > > > suggested in
> > > v3.
> > >
> > > This was for points you did not agree with. (Again, it might be
> > > helpful to wait with posting a new version until discussion has
> > > somewhat
> > > concluded.)
> > Sure, will wait next time.
> > 
> > Your comment was,
> > =====
> > This is not really clear to me just from this text, especially if you just wrote
> > above that enabling or re-enabling is something different... my understanding
> > would be:
> > 
> > - if neither dynamic vqs nor queue reset are supported or negotiated,
> >   the only way to enable a vq is before DRIVER_OK, during setup
> > - both of these features rely on the transport supporting enabling
> >   individual queues (either a queue that has not been enabled before, or
> >   a queue that has been reset)
> > - the transport is supposed to use the same mechanism for either
> > 
> > Did I get it right? If so, I think we should make it a bit more clear.
> > =====
> > 
> > Above is clarified in below wording without complicating the queue_reset here.
> > 
> > Does that look ok to you?
> > 
> > +When VIRTIO_F_RING_DYNAMIC is not negotiated, the driver enables the
> > +virtqueues during the device initialization sequence, i.e. after the
> > +device sets the FEATURES_OK status bit

all status bits are set by driver

> and before the driver setting the


setting -> sets

> > DRIVER_OK status bit.
> > +
> > +When VIRTIO_F_RING_DYNAMIC is negotiated, the driver is not required to

try to avoid "required" outside conformance sections though pls.
And when used, we upper-case it.

> > +enable every virtqueue it wants to use before setting the DRIVER_OK
> > +status bit; the driver can choose to enable a virtqueue even after the
> > +driver has set the DRIVER_OK status bit. The virtqueue enable mechanism is
> > transport specific.
> > +
> 
> Can you please respond?
> We must make progress with this and the flow filters which are depending on this for a long time now.


This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.

In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.

Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/

RE: [virtio-comment] RE: [PATCH v3 2/2] content: Support enabling virtqueue after DRIVER_OK stage

[Parav Pandit]

> From: virtio-comment@lists.oasis-open.org <virtio-comment@lists.oasis-
> open.org> On Behalf Of Michael S. Tsirkin
> Sent: Wednesday, October 25, 2023 3:54 PM
> 
> On Wed, Oct 25, 2023 at 09:55:20AM +0000, Parav Pandit wrote:
> > Hi Cornelia,
> >
> > > From: Parav Pandit <parav@nvidia.com>
> > > Sent: Monday, October 23, 2023 8:52 PM
> > > To: Cornelia Huck <cohuck@redhat.com>; Michael S. Tsirkin
> > > <mst@redhat.com>
> > > Cc: virtio-comment@lists.oasis-open.org; hengqi@linux.alibaba.com;
> > > xuanzhuo@linux.alibaba.com; Shahaf Shuler <shahafs@nvidia.com>
> > > Subject: RE: [virtio-comment] RE: [PATCH v3 2/2] content: Support
> > > enabling virtqueue after DRIVER_OK stage
> > >
> > >
> > > > From: virtio-comment@lists.oasis-open.org
> > > > <virtio-comment@lists.oasis- open.org> On Behalf Of Cornelia Huck
> > > > Sent: Monday, October 23, 2023 8:31 PM
> > > >
> > > > On Mon, Oct 23 2023, Parav Pandit <parav@nvidia.com> wrote:
> > > >
> > > > >> From: Cornelia Huck <cohuck@redhat.com>
> > > > >> Sent: Monday, October 23, 2023 7:58 PM
> > > > >
> > > > >> On Mon, Oct 23 2023, Parav Pandit <parav@nvidia.com> wrote:
> > > > >>
> > > > >> >> From: virtio-comment@lists.oasis-open.org
> > > > >> >> <virtio-comment@lists.oasis- open.org> On Behalf Of Cornelia
> > > > >> >> Huck
> > > > >> >> Sent: Monday, October 23, 2023 7:27 PM
> > > > >> >
> > > > >> >> Well, there are  still comments from me here that post-date
> > > > >> >> v4 (please wait for a bit before posting another version!),
> > > > >> >> so I'll continue waiting for them to be addressed first.
> > > > >> > The last one was [1].
> > > > >> > Which I replied few days ago at [2].
> > > > >> >
> > > > >> > [1]
> > > > >> > https://lists.oasis-open.org/archives/virtio-comment/202310/m
> > > > >> > sg00
> > > > >> > 24
> > > > >> > 0.h
> > > > >> > tml [2]
> > > > >> > https://lists.oasis-open.org/archives/virtio-comment/202310/m
> > > > >> > sg00
> > > > >> > 24
> > > > >> > 2.h
> > > > >> > tml
> > > > >>
> > > > >> I still have some open questions from
> > > > >> <87sf68cjn0.fsf@redhat.com> (first and last paragraph.)
> > > > >
> > > > > Above link is not accessible to me. ☹
> > > >
> > > > That's not a link, but a message id... many mail clients support
> > > > searching by it, and you can also get it from lore:
> > > >
> > > > https://lore.kernel.org/all/87sf68cjn0.fsf@redhat.com/
> > > >
> > > > > I addressed your comments/questions in v4.
> > > > > Can you please check v4 if they are addressed? I rewrote as you
> > > > > suggested in
> > > > v3.
> > > >
> > > > This was for points you did not agree with. (Again, it might be
> > > > helpful to wait with posting a new version until discussion has
> > > > somewhat
> > > > concluded.)
> > > Sure, will wait next time.
> > >
> > > Your comment was,
> > > =====
> > > This is not really clear to me just from this text, especially if
> > > you just wrote above that enabling or re-enabling is something
> > > different... my understanding would be:
> > >
> > > - if neither dynamic vqs nor queue reset are supported or negotiated,
> > >   the only way to enable a vq is before DRIVER_OK, during setup
> > > - both of these features rely on the transport supporting enabling
> > >   individual queues (either a queue that has not been enabled before, or
> > >   a queue that has been reset)
> > > - the transport is supposed to use the same mechanism for either
> > >
> > > Did I get it right? If so, I think we should make it a bit more clear.
> > > =====
> > >
> > > Above is clarified in below wording without complicating the queue_reset
> here.
> > >
> > > Does that look ok to you?
> > >
> > > +When VIRTIO_F_RING_DYNAMIC is not negotiated, the driver enables
> > > +the virtqueues during the device initialization sequence, i.e.
> > > +after the device sets the FEATURES_OK status bit
> 
> all status bits are set by driver
>
Well device clears it if it does not like the features.

Yes. I will rewrite it as, 

i.e. after the driver has verified that FEATURES_OK bit is set and before the driver sets DRIVER_OK bit.
 
> > and before the driver setting the
> 
> 
> setting -> sets
> 
> > > DRIVER_OK status bit.
> > > +
> > > +When VIRTIO_F_RING_DYNAMIC is negotiated, the driver is not
> > > +required to
> 
> try to avoid "required" outside conformance sections though pls.
> And when used, we upper-case it.
> 
Ok.
How about a rewrite as,

When VIRTIO_F_RING_DYNAMIC is negotiated, the driver may not enable every virtqueue, which the driver wants to use before setting the DRIVER_OK...

> > > +enable every virtqueue it wants to use before setting the DRIVER_OK
> > > +status bit; the driver can choose to enable a virtqueue even after
> > > +the driver has set the DRIVER_OK status bit. The virtqueue enable
> > > +mechanism is
> > > transport specific.
> > > +

Re: [virtio-comment] RE: [PATCH v3 2/2] content: Support enabling virtqueue after DRIVER_OK stage

[Michael S. Tsirkin]

On Thu, Oct 26, 2023 at 03:30:56AM +0000, Parav Pandit wrote:
> 
> > From: virtio-comment@lists.oasis-open.org <virtio-comment@lists.oasis-
> > open.org> On Behalf Of Michael S. Tsirkin
> > Sent: Wednesday, October 25, 2023 3:54 PM
> > 
> > On Wed, Oct 25, 2023 at 09:55:20AM +0000, Parav Pandit wrote:
> > > Hi Cornelia,
> > >
> > > > From: Parav Pandit <parav@nvidia.com>
> > > > Sent: Monday, October 23, 2023 8:52 PM
> > > > To: Cornelia Huck <cohuck@redhat.com>; Michael S. Tsirkin
> > > > <mst@redhat.com>
> > > > Cc: virtio-comment@lists.oasis-open.org; hengqi@linux.alibaba.com;
> > > > xuanzhuo@linux.alibaba.com; Shahaf Shuler <shahafs@nvidia.com>
> > > > Subject: RE: [virtio-comment] RE: [PATCH v3 2/2] content: Support
> > > > enabling virtqueue after DRIVER_OK stage
> > > >
> > > >
> > > > > From: virtio-comment@lists.oasis-open.org
> > > > > <virtio-comment@lists.oasis- open.org> On Behalf Of Cornelia Huck
> > > > > Sent: Monday, October 23, 2023 8:31 PM
> > > > >
> > > > > On Mon, Oct 23 2023, Parav Pandit <parav@nvidia.com> wrote:
> > > > >
> > > > > >> From: Cornelia Huck <cohuck@redhat.com>
> > > > > >> Sent: Monday, October 23, 2023 7:58 PM
> > > > > >
> > > > > >> On Mon, Oct 23 2023, Parav Pandit <parav@nvidia.com> wrote:
> > > > > >>
> > > > > >> >> From: virtio-comment@lists.oasis-open.org
> > > > > >> >> <virtio-comment@lists.oasis- open.org> On Behalf Of Cornelia
> > > > > >> >> Huck
> > > > > >> >> Sent: Monday, October 23, 2023 7:27 PM
> > > > > >> >
> > > > > >> >> Well, there are  still comments from me here that post-date
> > > > > >> >> v4 (please wait for a bit before posting another version!),
> > > > > >> >> so I'll continue waiting for them to be addressed first.
> > > > > >> > The last one was [1].
> > > > > >> > Which I replied few days ago at [2].
> > > > > >> >
> > > > > >> > [1]
> > > > > >> > https://lists.oasis-open.org/archives/virtio-comment/202310/m
> > > > > >> > sg00
> > > > > >> > 24
> > > > > >> > 0.h
> > > > > >> > tml [2]
> > > > > >> > https://lists.oasis-open.org/archives/virtio-comment/202310/m
> > > > > >> > sg00
> > > > > >> > 24
> > > > > >> > 2.h
> > > > > >> > tml
> > > > > >>
> > > > > >> I still have some open questions from
> > > > > >> <87sf68cjn0.fsf@redhat.com> (first and last paragraph.)
> > > > > >
> > > > > > Above link is not accessible to me. ☹
> > > > >
> > > > > That's not a link, but a message id... many mail clients support
> > > > > searching by it, and you can also get it from lore:
> > > > >
> > > > > https://lore.kernel.org/all/87sf68cjn0.fsf@redhat.com/
> > > > >
> > > > > > I addressed your comments/questions in v4.
> > > > > > Can you please check v4 if they are addressed? I rewrote as you
> > > > > > suggested in
> > > > > v3.
> > > > >
> > > > > This was for points you did not agree with. (Again, it might be
> > > > > helpful to wait with posting a new version until discussion has
> > > > > somewhat
> > > > > concluded.)
> > > > Sure, will wait next time.
> > > >
> > > > Your comment was,
> > > > =====
> > > > This is not really clear to me just from this text, especially if
> > > > you just wrote above that enabling or re-enabling is something
> > > > different... my understanding would be:
> > > >
> > > > - if neither dynamic vqs nor queue reset are supported or negotiated,
> > > >   the only way to enable a vq is before DRIVER_OK, during setup
> > > > - both of these features rely on the transport supporting enabling
> > > >   individual queues (either a queue that has not been enabled before, or
> > > >   a queue that has been reset)
> > > > - the transport is supposed to use the same mechanism for either
> > > >
> > > > Did I get it right? If so, I think we should make it a bit more clear.
> > > > =====
> > > >
> > > > Above is clarified in below wording without complicating the queue_reset
> > here.
> > > >
> > > > Does that look ok to you?
> > > >
> > > > +When VIRTIO_F_RING_DYNAMIC is not negotiated, the driver enables
> > > > +the virtqueues during the device initialization sequence, i.e.
> > > > +after the device sets the FEATURES_OK status bit
> > 
> > all status bits are set by driver
> >
> Well device clears it if it does not like the features.
> 
> Yes. I will rewrite it as, 
> 
> i.e. after the driver has verified that FEATURES_OK bit is set and before the driver sets DRIVER_OK bit.

shorter: i.e. after FEATURES_OK is set and before DRIVER_OK is set.

> > > and before the driver setting the
> > 
> > 
> > setting -> sets
> > 
> > > > DRIVER_OK status bit.
> > > > +
> > > > +When VIRTIO_F_RING_DYNAMIC is negotiated, the driver is not
> > > > +required to
> > 
> > try to avoid "required" outside conformance sections though pls.
> > And when used, we upper-case it.
> > 
> Ok.
> How about a rewrite as,
> 
> When VIRTIO_F_RING_DYNAMIC is negotiated, the driver may not enable every virtqueue, which the driver wants to use before setting the DRIVER_OK...

This repetition does not buy us much - verbose and "when" is confusing too, you just mean "if". So simply:

If VIRTIO_F_RING_DYNAMIC is negotiated,
the driver can choose to enable some virtqueues
after DRIVER_OK is set.


> 
> > > > +enable every virtqueue it wants to use before setting the DRIVER_OK
> > > > +status bit; the driver can choose to enable a virtqueue even after
> > > > +the driver has set the DRIVER_OK status bit.




>> The virtqueue enable
> > > > +mechanism is
> > > > transport specific.




I still feel there is no real explanation in a transport agnostic
manner what does it mean to "enable" virtqueue. Do you maybe just mean
"configure virtqueue"?


Can we just avoid talking about "enabling"?
Is it true that you can basically change anything you want about the vq
not just the enable bit? If so talking about enabling is just
confusing I think.

-- 
MST


This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.

In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.

Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/

RE: [virtio-comment] RE: [PATCH v3 2/2] content: Support enabling virtqueue after DRIVER_OK stage

[Parav Pandit]

> From: Michael S. Tsirkin <mst@redhat.com>
> Sent: Thursday, October 26, 2023 11:10 AM

> 
> shorter: i.e. after FEATURES_OK is set and before DRIVER_OK is set.

Ok.

> If VIRTIO_F_RING_DYNAMIC is negotiated,
> the driver can choose to enable some virtqueues after DRIVER_OK is set.
Ok. will take this simpler version.

> 
> 
> >
> > > > > +enable every virtqueue it wants to use before setting the
> > > > > +DRIVER_OK status bit; the driver can choose to enable a
> > > > > +virtqueue even after the driver has set the DRIVER_OK status bit.
> 
> 
> 
> 
> >> The virtqueue enable
> > > > > +mechanism is
> > > > > transport specific.
> 
> 
> 
> 
> I still feel there is no real explanation in a transport agnostic manner what does
> it mean to "enable" virtqueue. Do you maybe just mean "configure virtqueue"?
> 
There is explanation of "re-enabling" a virtqueue in section " Virtqueue Re-enable".

There is no good example of "configured" in generic section either, other than cleanup and mmio section.

> 
> Can we just avoid talking about "enabling"?
> Is it true that you can basically change anything you want about the vq not just
> the enable bit? If so talking about enabling is just confusing I think.
Not really.
Enabling sequence of a virtqueue for pci and mmio transport is:

1. select a q
2. program the addresses
3. set the enable bit

However, as I think more, this one-way interface of driver instructing the device without any error code and without any response is just too limiting interface.
I will be dropping this series that misuses the config registers beyond its intended init time use.

Let me work on more mature interface that solves following challenges.
1. ability to create a virtqueue dynamically after driver_ok
2. ability to create virtqueue with one or multiple physical addresses
3. ability for the device to return error if it cannot create dynamic vq creation

The question is, do we need this generic facility beyond virtio-net?

Re: [virtio-comment] RE: [PATCH v3 2/2] content: Support enabling virtqueue after DRIVER_OK stage

[Michael S. Tsirkin]

On Thu, Oct 26, 2023 at 06:02:28AM +0000, Parav Pandit wrote:
> 
> 
> > From: Michael S. Tsirkin <mst@redhat.com>
> > Sent: Thursday, October 26, 2023 11:10 AM
> 
> > 
> > shorter: i.e. after FEATURES_OK is set and before DRIVER_OK is set.
> 
> Ok.
> 
> > If VIRTIO_F_RING_DYNAMIC is negotiated,
> > the driver can choose to enable some virtqueues after DRIVER_OK is set.
> Ok. will take this simpler version.
> 
> > 
> > 
> > >
> > > > > > +enable every virtqueue it wants to use before setting the
> > > > > > +DRIVER_OK status bit; the driver can choose to enable a
> > > > > > +virtqueue even after the driver has set the DRIVER_OK status bit.
> > 
> > 
> > 
> > 
> > >> The virtqueue enable
> > > > > > +mechanism is
> > > > > > transport specific.
> > 
> > 
> > 
> > 
> > I still feel there is no real explanation in a transport agnostic manner what does
> > it mean to "enable" virtqueue. Do you maybe just mean "configure virtqueue"?
> > 
> There is explanation of "re-enabling" a virtqueue in section " Virtqueue Re-enable".
> 
> There is no good example of "configured" in generic section either, other than cleanup and mmio section.
> 
> > 
> > Can we just avoid talking about "enabling"?
> > Is it true that you can basically change anything you want about the vq not just
> > the enable bit? If so talking about enabling is just confusing I think.
> Not really.
> Enabling sequence of a virtqueue for pci and mmio transport is:
> 
> 1. select a q
> 2. program the addresses
> 3. set the enable bit
> 
> However, as I think more, this one-way interface of driver instructing the device without any error code and without any response is just too limiting interface.
> I will be dropping this series that misuses the config registers beyond its intended init time use.
> 
> Let me work on more mature interface that solves following challenges.
> 1. ability to create a virtqueue dynamically after driver_ok
> 2. ability to create virtqueue with one or multiple physical addresses
> 3. ability for the device to return error if it cannot create dynamic vq creation
> 
> The question is, do we need this generic facility beyond virtio-net?

Wow, sounds rather complex.

-- 
MST


This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.

In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.

Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/

RE: [virtio-comment] RE: [PATCH v3 2/2] content: Support enabling virtqueue after DRIVER_OK stage

[Parav Pandit]

> From: Michael S. Tsirkin <mst@redhat.com>
> Sent: Thursday, October 26, 2023 11:54 AM

> > Let me work on more mature interface that solves following challenges.
> > 1. ability to create a virtqueue dynamically after driver_ok 2.
> > ability to create virtqueue with one or multiple physical addresses 3.
> > ability for the device to return error if it cannot create dynamic vq
> > creation
> >
> > The question is, do we need this generic facility beyond virtio-net?
> 
> Wow, sounds rather complex.

Yes and no.

It is if there is any use case for generic devices.
For all the devices which already has cvq, it is fairly straightforward.
All dynamic vqs can be created and destroyed using a simple cvq commands.

This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.

In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.

Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/

Re: [virtio-comment] RE: [PATCH v3 2/2] content: Support enabling virtqueue after DRIVER_OK stage

[Eugenio Perez Martin]

On Thu, Oct 26, 2023 at 8:02 AM Parav Pandit <parav@nvidia.com> wrote:
>
>
>
> > From: Michael S. Tsirkin <mst@redhat.com>
> > Sent: Thursday, October 26, 2023 11:10 AM
>
> >
> > shorter: i.e. after FEATURES_OK is set and before DRIVER_OK is set.
>
> Ok.
>
> > If VIRTIO_F_RING_DYNAMIC is negotiated,
> > the driver can choose to enable some virtqueues after DRIVER_OK is set.
> Ok. will take this simpler version.
>
> >
> >
> > >
> > > > > > +enable every virtqueue it wants to use before setting the
> > > > > > +DRIVER_OK status bit; the driver can choose to enable a
> > > > > > +virtqueue even after the driver has set the DRIVER_OK status bit.
> >
> >
> >
> >
> > >> The virtqueue enable
> > > > > > +mechanism is
> > > > > > transport specific.
> >
> >
> >
> >
> > I still feel there is no real explanation in a transport agnostic manner what does
> > it mean to "enable" virtqueue. Do you maybe just mean "configure virtqueue"?
> >
> There is explanation of "re-enabling" a virtqueue in section " Virtqueue Re-enable".
>
> There is no good example of "configured" in generic section either, other than cleanup and mmio section.
>
> >
> > Can we just avoid talking about "enabling"?
> > Is it true that you can basically change anything you want about the vq not just
> > the enable bit? If so talking about enabling is just confusing I think.
> Not really.
> Enabling sequence of a virtqueue for pci and mmio transport is:
>
> 1. select a q
> 2. program the addresses
> 3. set the enable bit
>
> However, as I think more, this one-way interface of driver instructing the device without any error code and without any response is just too limiting interface.
> I will be dropping this series that misuses the config registers beyond its intended init time use.
>
> Let me work on more mature interface that solves following challenges.
> 1. ability to create a virtqueue dynamically after driver_ok
> 2. ability to create virtqueue with one or multiple physical addresses
> 3. ability for the device to return error if it cannot create dynamic vq creation
>

I think 1 is very complex for the simpler use case of delaying the
enabling of the dataplane, especially because virtio-net already
assigns "the last queue" for CVQ. It would require changes to it too.

2 can be solved if we allow resetting a queue before DRIVER_OK, as MST
proposed previously IIRC. Would that work for the use case you have in
mind?

3 can be solved by re-reading the queue_enable field? This is the way
we're already using for checking DRIVER_OK in the PCI transport.

> The question is, do we need this generic facility beyond virtio-net?

I would try to avoid solutions specific for virtio-net, as other
devices may need to reuse them and it would be bad to need to add
quirks for that.

Thanks!


This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.

In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.

Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/

RE: [virtio-comment] RE: [PATCH v3 2/2] content: Support enabling virtqueue after DRIVER_OK stage

[Parav Pandit]

> From: Eugenio Perez Martin <eperezma@redhat.com>
> Sent: Thursday, October 26, 2023 1:58 PM

> > Let me work on more mature interface that solves following challenges.
> > 1. ability to create a virtqueue dynamically after driver_ok 2.
> > ability to create virtqueue with one or multiple physical addresses 3.
> > ability for the device to return error if it cannot create dynamic vq
> > creation
> >
> 
> I think 1 is very complex for the simpler use case of delaying the enabling of the
> dataplane, especially because virtio-net already assigns "the last queue" for
> CVQ. It would require changes to it too.
Vq numbers do not change for virtio-net when enabled dynamically after driver_ok.
This is because max_virtqueue_pair is RO parameter.

It is not complex because is it only about defining a command format for create and destroy a vq to run on the cvq.
It is actually the opposite because there is good protocol and handshake.

Many devices has simple array of physical addresses for queue creation.
So command is straightforward.

> 
> 2 can be solved if we allow resetting a queue before DRIVER_OK, 
This is just the hack of enabling and reset for no apparent gain.
Future devices may have many queues and enabling 1000 queues, resetting them is just an ugly interface.

> Would that work for the use case you have in mind?
It works but not spec worthy method to build which just delays the whole device init flow with no gain.

> 
> 3 can be solved by re-reading the queue_enable field? This is the way we're
> already using for checking DRIVER_OK in the PCI transport.
Only polarity is not enough. An error code such as out of resource/busy/success is needed.

The whole point is to not abuse these basic init time registers at run time.
> 
> > The question is, do we need this generic facility beyond virtio-net?
> 
> I would try to avoid solutions specific for virtio-net, as other devices may need
> to reuse them and it would be bad to need to add quirks for that.
No need to add the quirk.
But I would like to listen if there is any concrete requirement.
And if there is, possibly such device can add the cvq at that point for dynamic work.
So, this will still provide the unified approach when/if needed on other devices.

Re: [virtio-comment] RE: [PATCH v3 2/2] content: Support enabling virtqueue after DRIVER_OK stage

[Michael S. Tsirkin]

On Thu, Oct 26, 2023 at 08:38:51AM +0000, Parav Pandit wrote:
> > 
> > > The question is, do we need this generic facility beyond virtio-net?
> > 
> > I would try to avoid solutions specific for virtio-net, as other devices may need
> > to reuse them and it would be bad to need to add quirks for that.
> No need to add the quirk.
> But I would like to listen if there is any concrete requirement.
> And if there is, possibly such device can add the cvq at that point for dynamic work.
> So, this will still provide the unified approach when/if needed on other devices.

What do you want to know? Whether dynamically adding/deleting queues is
useful generally? For example, queue per CPU is not uncommon to avoid
need for locks. In the presence of CPU hotplug, the natural thing to do
is to add/delete them.

-- 
MST


This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.

In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.

Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/

Re: [virtio-comment] RE: [PATCH v3 2/2] content: Support enabling virtqueue after DRIVER_OK stage

[Michael S. Tsirkin]

On Thu, Oct 26, 2023 at 08:38:51AM +0000, Parav Pandit wrote:
> > 2 can be solved if we allow resetting a queue before DRIVER_OK, 
> This is just the hack of enabling and reset for no apparent gain.
> Future devices may have many queues and enabling 1000 queues, resetting them is just an ugly interface.

Yes I can't say I love it either. The main driver is to try and make it
possible for device to know in advance the max # of queues that will
be used. We could alternatively just add this in a config register
maybe?

-- 
MST


This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.

In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.

Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/

RE: [virtio-comment] RE: [PATCH v3 2/2] content: Support enabling virtqueue after DRIVER_OK stage

[Parav Pandit]

> From: Michael S. Tsirkin <mst@redhat.com>
> Sent: Thursday, October 26, 2023 2:48 PM
> 
> On Thu, Oct 26, 2023 at 08:38:51AM +0000, Parav Pandit wrote:
> > > 2 can be solved if we allow resetting a queue before DRIVER_OK,
> > This is just the hack of enabling and reset for no apparent gain.
> > Future devices may have many queues and enabling 1000 queues, resetting
> them is just an ugly interface.
> 
> Yes I can't say I love it either. The main driver is to try and make it possible for
> device to know in advance the max # of queues that will be used. We could
> alternatively just add this in a config register maybe?

I didn't follow your suggestion - what you want to add in the config register?

This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.

In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.

Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/

RE: [virtio-comment] RE: [PATCH v3 2/2] content: Support enabling virtqueue after DRIVER_OK stage

[Parav Pandit]

> From: Michael S. Tsirkin <mst@redhat.com>
> Sent: Thursday, October 26, 2023 2:46 PM

> > So, this will still provide the unified approach when/if needed on other
> devices.
> 
> What do you want to know? Whether dynamically adding/deleting queues is
> useful generally? For example, queue per CPU is not uncommon to avoid need
> for locks. In the presence of CPU hotplug, the natural thing to do is to
> add/delete them.
So for those devices who care for such performance, it is reasonable for them to have the cvq providing channel for complex and dynamic commands.

This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.

In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.

Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/

Re: [virtio-comment] RE: [PATCH v3 2/2] content: Support enabling virtqueue after DRIVER_OK stage

[Michael S. Tsirkin]

On Thu, Oct 26, 2023 at 09:22:25AM +0000, Parav Pandit wrote:
> 
> > From: Michael S. Tsirkin <mst@redhat.com>
> > Sent: Thursday, October 26, 2023 2:48 PM
> > 
> > On Thu, Oct 26, 2023 at 08:38:51AM +0000, Parav Pandit wrote:
> > > > 2 can be solved if we allow resetting a queue before DRIVER_OK,
> > > This is just the hack of enabling and reset for no apparent gain.
> > > Future devices may have many queues and enabling 1000 queues, resetting
> > them is just an ugly interface.
> > 
> > Yes I can't say I love it either. The main driver is to try and make it possible for
> > device to know in advance the max # of queues that will be used. We could
> > alternatively just add this in a config register maybe?
> 
> I didn't follow your suggestion - what you want to add in the config register?

The reason devices want vqs to be enabled before DRIVER_OK is so that
they can preallocate device resources. Problems with this:
- no way to fail
- wasting driver resources too

Idea: a new writeable common config space register max_vqs +
a way for device to fail. Maybe set it before FEATURES_OK?

-- 
MST


This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.

In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.

Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/

Re: [virtio-comment] RE: [PATCH v3 2/2] content: Support enabling virtqueue after DRIVER_OK stage

[Michael S. Tsirkin]

On Thu, Oct 26, 2023 at 09:24:06AM +0000, Parav Pandit wrote:
> 
> > From: Michael S. Tsirkin <mst@redhat.com>
> > Sent: Thursday, October 26, 2023 2:46 PM
> 
> > > So, this will still provide the unified approach when/if needed on other
> > devices.
> > 
> > What do you want to know? Whether dynamically adding/deleting queues is
> > useful generally? For example, queue per CPU is not uncommon to avoid need
> > for locks. In the presence of CPU hotplug, the natural thing to do is to
> > add/delete them.
> So for those devices who care for such performance, it is reasonable for them to have the cvq providing channel for complex and dynamic commands.

Again, let's please not waste time adding this piecemeal to each device
type, this is clearly a generic thing that belongs in the core.


This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.

In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.

Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/

RE: [virtio-comment] RE: [PATCH v3 2/2] content: Support enabling virtqueue after DRIVER_OK stage

[Parav Pandit]

> From: Michael S. Tsirkin <mst@redhat.com>
> Sent: Thursday, October 26, 2023 2:58 PM
> 
> On Thu, Oct 26, 2023 at 09:24:06AM +0000, Parav Pandit wrote:
> >
> > > From: Michael S. Tsirkin <mst@redhat.com>
> > > Sent: Thursday, October 26, 2023 2:46 PM
> >
> > > > So, this will still provide the unified approach when/if needed on
> > > > other
> > > devices.
> > >
> > > What do you want to know? Whether dynamically adding/deleting queues
> > > is useful generally? For example, queue per CPU is not uncommon to
> > > avoid need for locks. In the presence of CPU hotplug, the natural
> > > thing to do is to add/delete them.
> > So for those devices who care for such performance, it is reasonable for them
> to have the cvq providing channel for complex and dynamic commands.
> 
> Again, let's please not waste time adding this piecemeal to each device type,
> this is clearly a generic thing that belongs in the core.

Maybe I didn't explain well.

I am proposing a generic way for all device types as following.
1. create cvq as main communication channel for resource create/destroy
2. create resource dynamically using this cvq

These are common commands defined across all device types in basic facilities section.
Those devices which has cvq, they define how to transport these generic defined commands.
Those devices which do not have cvq, but when they need dynamic resources, they will define and create one cvq.
As such devices will likely need more than just dynamic vq creation, for which it will need cvq anyway.


This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.

In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.

Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/

RE: [virtio-comment] RE: [PATCH v3 2/2] content: Support enabling virtqueue after DRIVER_OK stage

[Cornelia Huck]

On Thu, Oct 26 2023, Parav Pandit <parav@nvidia.com> wrote:

>> From: Michael S. Tsirkin <mst@redhat.com>
>> Sent: Thursday, October 26, 2023 2:58 PM
>> 
>> On Thu, Oct 26, 2023 at 09:24:06AM +0000, Parav Pandit wrote:
>> >
>> > > From: Michael S. Tsirkin <mst@redhat.com>
>> > > Sent: Thursday, October 26, 2023 2:46 PM
>> >
>> > > > So, this will still provide the unified approach when/if needed on
>> > > > other
>> > > devices.
>> > >
>> > > What do you want to know? Whether dynamically adding/deleting queues
>> > > is useful generally? For example, queue per CPU is not uncommon to
>> > > avoid need for locks. In the presence of CPU hotplug, the natural
>> > > thing to do is to add/delete them.
>> > So for those devices who care for such performance, it is reasonable for them
>> to have the cvq providing channel for complex and dynamic commands.
>> 
>> Again, let's please not waste time adding this piecemeal to each device type,
>> this is clearly a generic thing that belongs in the core.
>
> Maybe I didn't explain well.
>
> I am proposing a generic way for all device types as following.
> 1. create cvq as main communication channel for resource create/destroy
> 2. create resource dynamically using this cvq
>
> These are common commands defined across all device types in basic facilities section.
> Those devices which has cvq, they define how to transport these generic defined commands.
> Those devices which do not have cvq, but when they need dynamic resources, they will define and create one cvq.
> As such devices will likely need more than just dynamic vq creation, for which it will need cvq anyway.

I think that is too complicated: not all devices will want to add a
control vq for resource management, especially if other mechanisms are
already in use. Just using a generic mechanism guarded by a feature bit
seems much simpler.


This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.

In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.

Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/

RE: [virtio-comment] RE: [PATCH v3 2/2] content: Support enabling virtqueue after DRIVER_OK stage

[Parav Pandit]

> From: Cornelia Huck <cohuck@redhat.com>
> Sent: Friday, October 27, 2023 3:43 PM

> > These are common commands defined across all device types in basic
> facilities section.
> > Those devices which has cvq, they define how to transport these generic
> defined commands.
> > Those devices which do not have cvq, but when they need dynamic resources,
> they will define and create one cvq.
> > As such devices will likely need more than just dynamic vq creation, for which
> it will need cvq anyway.
> 
> I think that is too complicated: not all devices will want to add a control vq for
> resource management, especially if other mechanisms are already in use. Just
> using a generic mechanism guarded by a feature bit seems much simpler.
Not really, if a complex device has dynamic resource management, it is likely to have more functionality than just q creation.
And for some reason, even if it does not, having efficient control channel for non_init is needed anyway.

It not about device wants to add/not_add a control vq. It is about how device a should offer dynamic resource management.
From device point of view, it is equally complex for the hw device to build dynamic config registers for N VQs which it never knows whether it will be used or not.

Out of 19 devices, 6 devices already has cvq.
Out of remaining 13 devices, 12 devices have fixed number of queues as they small enough in functionality that won't need a scale and dynamism either.
So for dynamic resource management reusing the existing cvq is more efficient for the devices.

This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.

In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.

Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/

RE: [virtio-comment] RE: [PATCH v3 2/2] content: Support enabling virtqueue after DRIVER_OK stage

[Cornelia Huck]

On Fri, Oct 27 2023, Parav Pandit <parav@nvidia.com> wrote:

>> From: Cornelia Huck <cohuck@redhat.com>
>> Sent: Friday, October 27, 2023 3:43 PM
>
>> > These are common commands defined across all device types in basic
>> facilities section.
>> > Those devices which has cvq, they define how to transport these generic
>> defined commands.
>> > Those devices which do not have cvq, but when they need dynamic resources,
>> they will define and create one cvq.
>> > As such devices will likely need more than just dynamic vq creation, for which
>> it will need cvq anyway.
>> 
>> I think that is too complicated: not all devices will want to add a control vq for
>> resource management, especially if other mechanisms are already in use. Just
>> using a generic mechanism guarded by a feature bit seems much simpler.
> Not really, if a complex device has dynamic resource management, it is likely to have more functionality than just q creation.
> And for some reason, even if it does not, having efficient control channel for non_init is needed anyway.
>
> It not about device wants to add/not_add a control vq. It is about how device a should offer dynamic resource management.
> From device point of view, it is equally complex for the hw device to build dynamic config registers for N VQs which it never knows whether it will be used or not.

"config registers" are transport specific. If they aren't an efficient
method, then the transport needs to come up with something else -- but
it's the task of the *transport* to provide something useful. The device
should not need to do anything beyond specifying that it can support
adding vqs dynamically, and exposing the upper boundary to the number of
queues.

>
> Out of 19 devices, 6 devices already has cvq.
> Out of remaining 13 devices, 12 devices have fixed number of queues as they small enough in functionality that won't need a scale and dynamism either.
> So for dynamic resource management reusing the existing cvq is more efficient for the devices.

They have a device-specific cvq, which is used in a device-specific way
for device-specific purposes. I don't see how turning them into
frankencvqs is "efficient".


This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.

In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.

Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/

RE: [virtio-comment] RE: [PATCH v3 2/2] content: Support enabling virtqueue after DRIVER_OK stage

[Parav Pandit]

> From: Cornelia Huck <cohuck@redhat.com>
> Sent: Friday, October 27, 2023 5:16 PM

> "config registers" are transport specific. If they aren't an efficient method, then
> the transport needs to come up with something else -- but it's the task of the
> *transport* to provide something useful. The device should not need to do
> anything beyond specifying that it can support adding vqs dynamically, and
> exposing the upper boundary to the number of queues.
>
You are welcome to propose to PCI-SIG such enhancements. :)
We cannot keep blind eye to transport and say, virtio will expand infinity registers without caring about underlying transport limitations and efficiencies.

There have been many discussions in past across many device + cpu vendors in various virtio and non virtio forums.
I would like to share that the industry does not prefer to put things as infinite set of registers unless its most fundamental thing needed to bootstrap the device.

> >
> > Out of 19 devices, 6 devices already has cvq.
> > Out of remaining 13 devices, 12 devices have fixed number of queues as they
> small enough in functionality that won't need a scale and dynamism either.
> > So for dynamic resource management reusing the existing cvq is more
> efficient for the devices.
> 
> They have a device-specific cvq, which is used in a device-specific way for
> device-specific purposes. I don't see how turning them into frankencvqs is
> "efficient".
Sure, they are device specific, but we can define generic set of commands in basic facilities, which each device type can send via their cvq.
It is only matter of drafting it.

This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.

In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.

Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/