[virtio-dev] [PATCH v12 00/10] Rename queue number to queue index

[virtio-dev] [PATCH v12 00/10] Rename queue number to queue index

[Parav Pandit]

1. Currently, virtqueue is identified between driver and device
interchangeably using either number or index terminology.

2. Between PCI and MMIO transport the queue size (depth) is
defined as queue_size and QueueNum respectively.

To avoid confusion and to have consistency, unify them to use
index.

3. Filed name vqn in the driver notification structure is
ambiguous as it is supposed to hold either vq index or device
supplied vq identifier.

4. Device is really supplying queue identifier in the 
queue_notify_data register, and this often get confused with
very similar looking feature bit NOTIFICATION_DATA.

Solution:
a. Use virtqueue index description, and rename MMIO register as QueueSize.
b. Replace virtqueue number with virtqueue index
c. RSS area of virtio net has inherited some logic, describe it
using abstract rss_rq_id.
d. rename queue_notifify_data to queue_notify_id
e. rename vqn to vq_notify_id to reflect it can hold either vq
index of device supplied some id.

Patch summary:
patch-1 introduce vq number as generic term
patch-2 renames index to number for pci transport
patch-3 rename queue_notify_data to queue_notify_id
patch-4 remove first vq index reference
patch-5 renames mmio register from Num to Size
patch-6 renames index to number for mmio transport
patch-7 renames num field to size for ccw transport
patch-8 renames vq by its index for ccw transport
patch-9 for virtio-net removes duplicate example from requirements
patch-10 for virtio-net updates rss description to use vq index

This series only improves the documentation, it does not change any
transport or device functionality.

Please review.
This series fixes the issue [1].

[1] https://github.com/oasis-tcs/virtio-spec/issues/163

---
changelog:
v11->v12:
- replace number to index
- avoid confusion around vqn field and rename to vq_notify_id
- rename queue_notify_data to avoid confusing with NOTIFY_DATA
v10->v11:
- added Reviewed-by for all the reviewed patches
- updated commit log of patch-8 to drop rq_handle reference
- skipped comment to further use rss_rq_id, as rss_rq_id usage
  and structure are self describing
v9->v10:
- added virtqueue number part in content in braces
- replaced queue_select to vqn in ccw
- avoided aggrasive alignment of 65 chars
- updated commit log to drop reference to already merged patches
- added review-by tag for already reviewed patches
v8->v9:
- addressed comments from David
- few corrections with article
- renaming 'virtqueue number' to 'vq number'
- improving text and wording for rss_rq_id, avail notification
- commit log of specific change in individual patches
v7->v8:
- remove note about first virtqueue number
- skipped Max's comment to put word 'structure' in same line as its
  crosses 65 chars limit per line
- reworded queue_notification data set line, as '=' and vq number
  wording was odd
v6->v7:
- remove text around first vq as it is already covered in the basic
  virtqueues facility section
v5->v6:
- moved the vq number description from middle of vq operation
  to beginning of vq introduction
v4->v5:
- fixed accidental removal of "unclassifed packets".
- simplfied text around indirection_table mask
- removed rss_rq_id references as indirection table and
  unclassified_queue data type is self explanatory
v3->v4:
- moved note to comment for ccw
- renamed rq_handle to rss_rq_id
- moved rss_rq_id next to rss_config structure
- define rss_config structure using rss_rq_id
v2->v3:
- addressed comments from Michael
- added previous definitions for ccw fields
- moved rq_handle definition before using it
- added first patch to describe vq number
- updated pci for available buffer notification section
v1->v2:
- added patches for virtio net for rss area
- added patches for covering ccw transport
- added missing entries to refer in mmio transport


Parav Pandit (10):
  content: Add vq index text
  content.tex Replace virtqueue number with index
  content: Rename confusing queue_notify_data and vqn names
  transport-pci: Avoid first vq index reference
  transport-mmio: Rename QueueNum register
  transport-mmio: Avoid referring to zero based index
  transport-ccw: Rename queue depth/size to other transports
  transport-ccw: Refer to the vq by its index
  virtio-net: Avoid duplicate receive queue example
  virtio-net: Describe RSS using rss rq id

 content.tex                      | 27 ++++++++++++----
 device-types/net/description.tex | 29 ++++++++++++-----
 notifications-be.c               |  2 +-
 notifications-le.c               |  2 +-
 transport-ccw.tex                | 15 +++++----
 transport-mmio.tex               | 55 +++++++++++++++++++-------------
 transport-pci.tex                | 26 ++++++++-------
 7 files changed, 99 insertions(+), 57 deletions(-)

-- 
2.26.2


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] [PATCH v12 01/10] content: Add vq index text

[Parav Pandit]

Introduce vq index and its range so that subsequent patches can refer
to it.

Fixes: https://github.com/oasis-tcs/virtio-spec/issues/163
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
Signed-off-by: Parav Pandit <parav@nvidia.com>
---
changelog:
v11->v12:
- renamed 'number' to 'index'
v9->v10:
- added braces around vq number wording
- added vqn as another term for vq number
v8->v9:
- added inclusive when describing the vq number range
- skipped comment to put virtqueue number wording first because we
  prefer to use shorter vq number as much as possible
v5->v6:
- moved description close to introduction, it was in middle of
  queue data transfer description
v2->v3:
- new patch
---
 content.tex | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/content.tex b/content.tex
index cff548a..e64115c 100644
--- a/content.tex
+++ b/content.tex
@@ -298,6 +298,10 @@ \section{Virtqueues}\label{sec:Basic Facilities of a Virtio Device / Virtqueues}
 virtqueues\footnote{For example, the simplest network device has one virtqueue for
 transmit and one for receive.}.
 
+Each virtqueue is identified by a vq index (also referred
+to as a virtqueue index); vq index range is from 0 to 65535
+inclusive.
+
 Driver makes requests available to device by adding
 an available buffer to the queue, i.e., adding a buffer
 describing the request to a virtqueue, and optionally triggering
-- 
2.26.2


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] [PATCH v12 02/10] content.tex Replace virtqueue number with index

[Parav Pandit]

Replace virtqueue number with index to align to rest of the
specification.

Fixes: https://github.com/oasis-tcs/virtio-spec/issues/163
Signed-off-by: Parav Pandit <parav@nvidia.com>

---
changelog:
v11->v12:
- new patch
---
 content.tex | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/content.tex b/content.tex
index e64115c..cd93db2 100644
--- a/content.tex
+++ b/content.tex
@@ -406,7 +406,7 @@ \section{Driver Notifications} \label{sec:Basic Facilities of a Virtio Device /
 
 When VIRTIO_F_NOTIFICATION_DATA has not been negotiated,
 this notification involves sending the
-virtqueue number to the device (method depending on the transport).
+virtqueue index to the device (method depending on the transport).
 
 However, some devices benefit from the ability to find out the
 amount of available data in the queue without accessing the virtqueue in memory:
@@ -790,13 +790,13 @@ \chapter{Reserved Feature Bits}\label{sec:Reserved Feature Bits}
   buffer notifications.
   As mentioned in section \ref{sec:Basic Facilities of a Virtio Device / Driver notifications}, when the
   driver is required to send an available buffer notification to the device, it
-  sends the virtqueue number to be notified. The method of delivering
+  sends the virtqueue index to be notified. The method of delivering
   notifications is transport specific.
   With the PCI transport, the device can optionally provide a per-virtqueue value
-  for the driver to use in driver notifications, instead of the virtqueue number.
+  for the driver to use in driver notifications, instead of the virtqueue index.
   Some devices may benefit from this flexibility by providing, for example,
   an internal virtqueue identifier, or an internal offset related to the
-  virtqueue number.
+  virtqueue index.
 
   This feature indicates the availability of such value. The definition of the
   data to be provided in driver notification and the delivery method is
-- 
2.26.2


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] [PATCH v12 03/10] content: Rename confusing queue_notify_data and vqn names

[Parav Pandit]

Currently queue_notify_data register indicates the device
internal queue notification identifier. This register is
meaningful only when feature bit VIRTIO_F_NOTIF_CONFIG_DATA is
negotiated.

However, above register name often get confusing association with
very similar feature bit VIRTIO_F_NOTIFICATION_DATA.

When VIRTIO_F_NOTIFICATION_DATA feature bit is negotiated,
notification really involves sending additional queue progress
related information (not queue identifier).

Hence
1. to avoid any misunderstanding and association of
queue_notify_data with similar name VIRTIO_F_NOTIFICATION_DATA,

and
2. to reflect that queue_notify_data is the actual device
internal vq identifier, rename it to queue_notify_id.

Reflect vq identifier in the driver notification structure by renaming
ambiguous vqn to vq_notify_id.

The driver notification section assumes that queue notification contains
vq index always. CONFIG_DATA feature bit introduction missed to
update the driver notification section. Hence, correct it.

Signed-off-by: Parav Pandit <parav@nvidia.com>

---
Some side notes:
renaming vqn to vqnd is even more confusing because data is really the
queue identifier.

And NOTIFICATION_DATA really contains queue progress info (data).

(vqn - n is number or notification?, notification word in the
notification structure does not make sense).

Hence above renaming.

---
changelog:
v11->v12:
- new patch
---
 content.tex        | 17 ++++++++++++++---
 notifications-be.c |  2 +-
 notifications-le.c |  2 +-
 transport-pci.tex  | 24 ++++++++++++++----------
 4 files changed, 30 insertions(+), 15 deletions(-)

diff --git a/content.tex b/content.tex
index cd93db2..d5f8026 100644
--- a/content.tex
+++ b/content.tex
@@ -405,8 +405,18 @@ \section{Driver Notifications} \label{sec:Basic Facilities of a Virtio Device /
 notification to the device.
 
 When VIRTIO_F_NOTIFICATION_DATA has not been negotiated,
-this notification involves sending the
-virtqueue index to the device (method depending on the transport).
+this notification involves sending only the 16-bit virtqueue notification
+identifier (notification method depends on the transport).
+
+\begin{itemize}
+\item When VIRTIO_F_NOTIF_CONFIG_DATA is not negotiated, virtqueue
+notification identifier is a 16-bit vq index.
+
+\item When VIRTIO_F_NOTIF_CONFIG_DATA is negotiated, virtqueue
+notification identifier is a device supplied virtqueue identifier. A method
+to supply such virtqueue notification identifier is transport
+specific.
+\end{itemize}
 
 However, some devices benefit from the ability to find out the
 amount of available data in the queue without accessing the virtqueue in memory:
@@ -417,7 +427,8 @@ \section{Driver Notifications} \label{sec:Basic Facilities of a Virtio Device /
 the following information:
 
 \begin{description}
-\item [vqn] VQ number to be notified.
+\item [vq_notify_id] VQ notification identifier of a
+      corresponding virtqueue.
 \item [next_off] Offset
       within the ring where the next available ring entry
       will be written.
diff --git a/notifications-be.c b/notifications-be.c
index 5be947e..0a7cbf1 100644
--- a/notifications-be.c
+++ b/notifications-be.c
@@ -1,5 +1,5 @@
 be32 {
-	vqn : 16;
+	vq_notify_id : 16; /* previously known as vqn */
 	next_off : 15;
 	next_wrap : 1;
 };
diff --git a/notifications-le.c b/notifications-le.c
index fe51267..6cca8fb 100644
--- a/notifications-le.c
+++ b/notifications-le.c
@@ -1,5 +1,5 @@
 le32 {
-	vqn : 16;
+	vq_notify_id : 16; /* previously known as vqn */
 	next_off : 15;
 	next_wrap : 1;
 };
diff --git a/transport-pci.tex b/transport-pci.tex
index 5d98467..6bbf61c 100644
--- a/transport-pci.tex
+++ b/transport-pci.tex
@@ -319,7 +319,7 @@ \subsubsection{Common configuration structure layout}\label{sec:Virtio Transport
         le64 queue_desc;                /* read-write */
         le64 queue_driver;              /* read-write */
         le64 queue_device;              /* read-write */
-        le16 queue_notify_data;         /* read-only for driver */
+        le16 queue_notify_id;           /* read-only for driver */
         le16 queue_reset;               /* read-write */
 };
 \end{lstlisting}
@@ -388,17 +388,21 @@ \subsubsection{Common configuration structure layout}\label{sec:Virtio Transport
 \item[\field{queue_device}]
         The driver writes the physical address of Device Area here.  See section \ref{sec:Basic Facilities of a Virtio Device / Virtqueues}.
 
-\item[\field{queue_notify_data}]
+\item[\field{queue_notify_id}]
         This field exists only if VIRTIO_F_NOTIF_CONFIG_DATA has been negotiated.
-        The driver will use this value to put it in the 'virtqueue number' field
-        in the available buffer notification structure.
+        The driver will use this value when driver sends available buffer
+        notification to the device.
         See section \ref{sec:Virtio Transport Options / Virtio Over PCI Bus / PCI-specific Initialization And Device Operation / Available Buffer Notifications}.
         \begin{note}
         This field provides the device with flexibility to determine how virtqueues
         will be referred to in available buffer notifications.
-        In a trivial case the device can set \field{queue_notify_data}=vqn. Some devices
-        may benefit from providing another value, for example an internal virtqueue
-        identifier, or an internal offset related to the virtqueue number.
+        In a trivial case the device can set \field{queue_notify_id} to
+        vq index. Some devices may benefit from providing another value,
+        for example an internal virtqueue identifier, or an internal offset
+        related to the virtqueue index.
+        \end{note}
+        \begin{note}
+        This field is previously known as queue_notify_id.
         \end{note}
 
 \item[\field{queue_reset}]
@@ -468,7 +472,7 @@ \subsubsection{Common configuration structure layout}\label{sec:Virtio Transport
 
 \drivernormative{\paragraph}{Common configuration structure layout}{Virtio Transport Options / Virtio Over PCI Bus / PCI Device Layout / Common configuration structure layout}
 
-The driver MUST NOT write to \field{device_feature}, \field{num_queues}, \field{config_generation}, \field{queue_notify_off} or \field{queue_notify_data}.
+The driver MUST NOT write to \field{device_feature}, \field{num_queues}, \field{config_generation}, \field{queue_notify_off} or \field{queue_notify_id}.
 
 If VIRTIO_F_RING_PACKED has been negotiated,
 the driver MUST NOT write the value 0 to \field{queue_size}.
@@ -1053,9 +1057,9 @@ \subsubsection{Available Buffer Notifications}\label{sec:Virtio Transport Option
 If VIRTIO_F_NOTIF_CONFIG_DATA has been negotiated:
 \begin{itemize}
 \item If VIRTIO_F_NOTIFICATION_DATA has not been negotiated, the driver MUST use the
-\field{queue_notify_data} value instead of the virtqueue index.
+\field{queue_notify_id} value instead of the virtqueue index.
 \item If VIRTIO_F_NOTIFICATION_DATA has been negotiated, the driver MUST set the
-\field{vqn} field to the \field{queue_notify_data} value.
+\field{vq_notify_id} field to the \field{queue_notify_id} value.
 \end{itemize}
 
 \subsubsection{Used Buffer Notifications}\label{sec:Virtio Transport Options / Virtio Over PCI Bus / PCI-specific Initialization And Device Operation / Used Buffer Notifications}
-- 
2.26.2


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] [PATCH v12 04/10] transport-pci: Avoid first vq index reference

[Parav Pandit]

Drop reference to first virtqueue as it is already
covered now by the generic section in first patch.

Fixes: https://github.com/oasis-tcs/virtio-spec/issues/163
Signed-off-by: Parav Pandit <parav@nvidia.com>
---
changelog:
v11->v12:
- drop changes related to vq number
v9->v10:
- updated commit log to drop reference to old patch
v8->v9:
- reword the sentence to avoid future tense, like rest of the other
  fields description
- reword the sentence to avoid multiple verbs use and put -> uses
- use shorter name 'vq number' instead of 'virtqueue number'
v7->v8:
- remove note about first virtqueue number
- skipped Max's comment to put word 'structure' in same line as its
  crosses 65 chars limit per line
- reworded queue_notification data set line, as '=' and vq number
  wording was odd
v2->v3:
- addressed comments from Michael
- changed vqn to virtqueue number in the Note
- refer to vqn field instead of virtqueue number
---
 transport-pci.tex | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/transport-pci.tex b/transport-pci.tex
index 6bbf61c..1e6604d 100644
--- a/transport-pci.tex
+++ b/transport-pci.tex
@@ -1009,7 +1009,7 @@ \subsubsection{Device Initialization}\label{sec:Virtio Transport Options / Virti
 The driver typically does this as follows, for each virtqueue a device has:
 
 \begin{enumerate}
-\item Write the virtqueue index (first queue is 0) to \field{queue_select}.
+\item Write the virtqueue index to \field{queue_select}.
 
 \item Read the virtqueue size from \field{queue_size}. This controls how big the virtqueue is
   (see \ref{sec:Basic Facilities of a Virtio Device / Virtqueues}~\nameref{sec:Basic Facilities of a Virtio Device / Virtqueues}). If this field is 0, the virtqueue does not exist.
-- 
2.26.2


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] [PATCH v12 05/10] transport-mmio: Rename QueueNum register

[Parav Pandit]

These are further named differently between pci and mmio transport.
PCI transport indicates queue size as queue_size.

To bring consistency between pci and mmio transport,
rename the QueueNumMax and QueueNum
registers to QueueSizeMax and QueueSize respectively.

Fixes: https://github.com/oasis-tcs/virtio-spec/issues/163
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
Reviewed-by: Jiri Pirko <jiri@nvidia.com>
Signed-off-by: Parav Pandit <parav@nvidia.com>

---
changelog:
v8->v9:
- added field tag to indicate field name instead of English word
v0->v1:
- replaced references of QueueNumMax to QueueSizeMax
- replaced references of QueueNum to QueueSize
- added note for renamed fields old name suggested by @Michael Tsirkin
---
 transport-mmio.tex | 42 ++++++++++++++++++++++++++++--------------
 1 file changed, 28 insertions(+), 14 deletions(-)

diff --git a/transport-mmio.tex b/transport-mmio.tex
index f884a2c..164e640 100644
--- a/transport-mmio.tex
+++ b/transport-mmio.tex
@@ -110,24 +110,31 @@ \subsection{MMIO Device Register Layout}\label{sec:Virtio Transport Options / Vi
   \hline
   \mmioreg{QueueSel}{Virtual queue index}{0x030}{W}{%
     Writing to this register selects the virtual queue that the
-    following operations on \field{QueueNumMax}, \field{QueueNum}, \field{QueueReady},
+    following operations on \field{QueueSizeMax},
+    \field{QueueSize}, \field{QueueReady},
     \field{QueueDescLow}, \field{QueueDescHigh}, \field{QueueDriverlLow}, \field{QueueDriverHigh},
     \field{QueueDeviceLow}, \field{QueueDeviceHigh} and \field{QueueReset} apply to. The index
     number of the first queue is zero (0x0).
   }
   \hline
-  \mmioreg{QueueNumMax}{Maximum virtual queue size}{0x034}{R}{%
+  \mmioreg{QueueSizeMax}{Maximum virtual queue size}{0x034}{R}{%
     Reading from the register returns the maximum size (number of
     elements) of the queue the device is ready to process or
     zero (0x0) if the queue is not available. This applies to the
     queue selected by writing to \field{QueueSel}.
+    \begin{note}
+    \field{QueueSizeMax} was previously known as \field{QueueNumMax}.
+    \end{note}
   }
   \hline
-  \mmioreg{QueueNum}{Virtual queue size}{0x038}{W}{%
+  \mmioreg{QueueSize}{Virtual queue size}{0x038}{W}{%
     Queue size is the number of elements in the queue.
     Writing to this register notifies the device what size of the
     queue the driver will use. This applies to the queue selected by
     writing to \field{QueueSel}.
+    \begin{note}
+    \field{QueueSize} was previously known as \field{QueueNum}.
+    \end{note}
   }
   \hline
   \mmioreg{QueueReady}{Virtual queue ready bit}{0x044}{RW}{%
@@ -308,11 +315,11 @@ \subsection{MMIO Device Register Layout}\label{sec:Virtio Transport Options / Vi
 
 Before writing to the \field{DriverFeatures} register, the driver MUST write a value to the \field{DriverFeaturesSel} register.
 
-The driver MUST write a value to \field{QueueNum} which is less than
-or equal to the value presented by the device in \field{QueueNumMax}.
+The driver MUST write a value to \field{QueueSize} which is less than
+or equal to the value presented by the device in \field{QueueSizeMax}.
 
 When \field{QueueReady} is not zero, the driver MUST NOT access
-\field{QueueNum}, \field{QueueDescLow}, \field{QueueDescHigh},
+\field{QueueSize}, \field{QueueDescLow}, \field{QueueDescHigh},
 \field{QueueDriverLow}, \field{QueueDriverHigh}, \field{QueueDeviceLow}, \field{QueueDeviceHigh}.
 
 To stop using the queue the driver MUST write zero (0x0) to this
@@ -363,14 +370,14 @@ \subsubsection{Virtqueue Configuration}\label{sec:Virtio Transport Options / Vir
    and expect a returned value of zero (0x0).
 
 \item Read maximum queue size (number of elements) from
-   \field{QueueNumMax}. If the returned value is zero (0x0) the
+   \field{QueueSizeMax}. If the returned value is zero (0x0) the
    queue is not available.
 
 \item Allocate and zero the queue memory, making sure the memory
    is physically contiguous.
 
 \item Notify the device about the queue size by writing the size to
-   \field{QueueNum}.
+   \field{QueueSize}.
 
 \item Write physical addresses of the queue's Descriptor Area,
    Driver Area and Device Area to (respectively) the
@@ -465,25 +472,32 @@ \subsection{Legacy interface}\label{sec:Virtio Transport Options / Virtio Over M
   \hline
   \mmioreg{QueueSel}{Virtual queue index}{0x030}{W}{%
     Writing to this register selects the virtual queue that the
-    following operations on the \field{QueueNumMax}, \field{QueueNum}, \field{QueueAlign}
+    following operations on the \field{QueueSizeMax},
+    \field{QueueSize}, \field{QueueAlign}
     and \field{QueuePFN} registers apply to. The index
     number of the first queue is zero (0x0).
 .
   }
   \hline
-  \mmioreg{QueueNumMax}{Maximum virtual queue size}{0x034}{R}{%
+  \mmioreg{QueueSizeMax}{Maximum virtual queue size}{0x034}{R}{%
     Reading from the register returns the maximum size of the queue
     the device is ready to process or zero (0x0) if the queue is not
     available. This applies to the queue selected by writing to
     \field{QueueSel} and is allowed only when \field{QueuePFN} is set to zero
     (0x0), so when the queue is not actively used.
+    \begin{note}
+    \field{QueueSizeMax} was previously known as \field{QueueNumMax}.
+    \end{note}
   }
   \hline
-  \mmioreg{QueueNum}{Virtual queue size}{0x038}{W}{%
+  \mmioreg{QueueSize}{Virtual queue size}{0x038}{W}{%
     Queue size is the number of elements in the queue.
     Writing to this register notifies the device what size of the
     queue the driver will use. This applies to the queue selected by
     writing to \field{QueueSel}.
+    \begin{note}
+    \field{QueueSize} was previously known as \field{QueueNum}.
+    \end{note}
   }
   \hline
   \mmioreg{QueueAlign}{Used Ring alignment in the virtual queue}{0x03c}{W}{%
@@ -543,16 +557,16 @@ \subsection{Legacy interface}\label{sec:Virtio Transport Options / Virtio Over M
    expecting a returned value of zero (0x0).
 
 \item Read maximum queue size (number of elements) from
-   \field{QueueNumMax}. If the returned value is zero (0x0) the
+   \field{QueueSizeMax}. If the returned value is zero (0x0) the
    queue is not available.
 
 \item Allocate and zero the queue pages in contiguous virtual
    memory, aligning the Used Ring to an optimal boundary (usually
    page size). The driver should choose a queue size smaller than or
-   equal to \field{QueueNumMax}.
+   equal to \field{QueueSizeMax}.
 
 \item Notify the device about the queue size by writing the size to
-   \field{QueueNum}.
+   \field{QueueSize}.
 
 \item Notify the device about the used alignment by writing its value
    in bytes to \field{QueueAlign}.
-- 
2.26.2


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] [PATCH v12 06/10] transport-mmio: Avoid referring to zero based index

[Parav Pandit]

VQ range is already described in the first patch in basic virtqueue
section. Hence remove the duplicate reference to it.

Fixes: https://github.com/oasis-tcs/virtio-spec/issues/163
Signed-off-by: Parav Pandit <parav@nvidia.com>

---
changelog:
v11->v12:
- remove changes related to 'vq number'
v8->v9:
- added 'by' at two places
- replaced 'queue number' with 'vq number'

v6->v7:
- remove text around first vq as it is already covered in the basic
  virtqueues facility section
---
 transport-mmio.tex | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/transport-mmio.tex b/transport-mmio.tex
index 164e640..da58612 100644
--- a/transport-mmio.tex
+++ b/transport-mmio.tex
@@ -113,8 +113,7 @@ \subsection{MMIO Device Register Layout}\label{sec:Virtio Transport Options / Vi
     following operations on \field{QueueSizeMax},
     \field{QueueSize}, \field{QueueReady},
     \field{QueueDescLow}, \field{QueueDescHigh}, \field{QueueDriverlLow}, \field{QueueDriverHigh},
-    \field{QueueDeviceLow}, \field{QueueDeviceHigh} and \field{QueueReset} apply to. The index
-    number of the first queue is zero (0x0).
+    \field{QueueDeviceLow}, \field{QueueDeviceHigh} and \field{QueueReset} apply to.
   }
   \hline
   \mmioreg{QueueSizeMax}{Maximum virtual queue size}{0x034}{R}{%
@@ -363,8 +362,7 @@ \subsubsection{Virtqueue Configuration}\label{sec:Virtio Transport Options / Vir
 The driver will typically initialize the virtual queue in the following way:
 
 \begin{enumerate}
-\item Select the queue writing its index (first queue is 0) to
-   \field{QueueSel}.
+\item Select the queue by writing its index to \field{QueueSel}.
 
 \item Check if the queue is not already in use: read \field{QueueReady},
    and expect a returned value of zero (0x0).
@@ -474,9 +472,7 @@ \subsection{Legacy interface}\label{sec:Virtio Transport Options / Virtio Over M
     Writing to this register selects the virtual queue that the
     following operations on the \field{QueueSizeMax},
     \field{QueueSize}, \field{QueueAlign}
-    and \field{QueuePFN} registers apply to. The index
-    number of the first queue is zero (0x0).
-.
+    and \field{QueuePFN} registers apply to.
   }
   \hline
   \mmioreg{QueueSizeMax}{Maximum virtual queue size}{0x034}{R}{%
@@ -550,8 +546,7 @@ \subsection{Legacy interface}\label{sec:Virtio Transport Options / Virtio Over M
 
 The virtual queue is configured as follows:
 \begin{enumerate}
-\item Select the queue writing its index (first queue is 0) to
-   \field{QueueSel}.
+\item Select the queue by writing its number to \field{QueueSel}.
 
 \item Check if the queue is not already in use: read \field{QueuePFN},
    expecting a returned value of zero (0x0).
-- 
2.26.2


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] [PATCH v12 07/10] transport-ccw: Rename queue depth/size to other transports

[Parav Pandit]

max_num field reflects the maximum queue size/depth. Hence align name of
this field with similar field in PCI and MMIO transport to
max_queue_size.
Similarly rename 'num' to 'size'.

Fixes: https://github.com/oasis-tcs/virtio-spec/issues/163
Signed-off-by: Parav Pandit <parav@nvidia.com>

---
changelog:
v9>v10:
- used lower case letter for comment first word
v8->v9:
- replaced 'named' as 'known'
v3->v4:
- moved note to comment
---
 transport-ccw.tex | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/transport-ccw.tex b/transport-ccw.tex
index c492cb9..cb476c7 100644
--- a/transport-ccw.tex
+++ b/transport-ccw.tex
@@ -237,12 +237,12 @@ \subsubsection{Configuring a Virtqueue}\label{sec:Virtio Transport Options / Vir
 \begin{lstlisting}
 struct vq_config_block {
         be16 index;
-        be16 max_num;
+        be16 max_queue_size; /* previously known as max_num */
 };
 \end{lstlisting}
 
 The requested number of buffers for queue \field{index} is returned in
-\field{max_num}.
+\field{max_queue_size}.
 
 Afterwards, CCW_CMD_SET_VQ is issued by the driver to inform the
 device about the location used for its queue. The transmitted
@@ -253,7 +253,7 @@ \subsubsection{Configuring a Virtqueue}\label{sec:Virtio Transport Options / Vir
         be64 desc;
         be32 res0;
         be16 index;
-        be16 num;
+        be16 size; /* previously known as num */
         be64 driver;
         be64 device;
 };
@@ -262,7 +262,7 @@ \subsubsection{Configuring a Virtqueue}\label{sec:Virtio Transport Options / Vir
 \field{desc}, \field{driver} and \field{device} contain the guest
 addresses for the descriptor area,
 available area and used area for queue \field{index}, respectively. The actual
-virtqueue size (number of allocated buffers) is transmitted in \field{num}.
+virtqueue size (number of allocated buffers) is transmitted in \field{size}.
 
 \devicenormative{\paragraph}{Configuring a Virtqueue}{Virtio Transport Options / Virtio over channel I/O / Device Initialization / Configuring a Virtqueue}
 
@@ -278,11 +278,12 @@ \subsubsection{Configuring a Virtqueue}\label{sec:Virtio Transport Options / Vir
         be64 queue;
         be32 align;
         be16 index;
-        be16 num;
+        be16 size; /* previously known as num */
 };
 \end{lstlisting}
 
-\field{queue} contains the guest address for queue \field{index}, \field{num} the number of buffers
+\field{queue} contains the guest address for queue \field{index},
+\field{size} the number of buffers
 and \field{align} the alignment. The queue layout follows \ref{sec:Basic Facilities of a Virtio Device / Virtqueues / Legacy Interfaces: A Note on Virtqueue Layout}~\nameref{sec:Basic Facilities of a Virtio Device / Virtqueues / Legacy Interfaces: A Note on Virtqueue Layout}.
 
 \subsubsection{Communicating Status Information}\label{sec:Virtio Transport Options / Virtio over channel I/O / Device Initialization / Communicating Status Information}
-- 
2.26.2


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] [PATCH v12 08/10] transport-ccw: Refer to the vq by its index

[Parav Pandit]

Currently specification uses virtqueue index and
number interchangeably to refer to the virtqueue.

Instead refer to it by its index.

Fixes: https://github.com/oasis-tcs/virtio-spec/issues/163
Signed-off-by: Parav Pandit <parav@nvidia.com>
---
changelog:
v11->v12:
- removed changes related to index
- replace number with index
- added 'only' to make it more clear that
  notification has only vq index
v9->v10:
- replaced queue_select with vqn
- used lower case letter for first word in comment
v8->v9:
- replaced 'named' with 'known'
- replaced 'queue number' with 'vq number'
v3->v4:
- moved note to comment
v2->v3:
- added comment note for queue_select similar to max_queue_size
v0->v1:
- new patch
---
 transport-ccw.tex | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/transport-ccw.tex b/transport-ccw.tex
index cb476c7..86272d1 100644
--- a/transport-ccw.tex
+++ b/transport-ccw.tex
@@ -545,7 +545,7 @@ \subsubsection{Guest->Host Notification}\label{sec:Virtio Transport Options / Vi
 \end{tabular}
 
 When VIRTIO_F_NOTIFICATION_DATA has not been negotiated,
-the \field{Notification data} contains the Virtqueue number.
+the \field{Notification data} contains the virtqueue index.
 
 When VIRTIO_F_NOTIFICATION_DATA has been negotiated,
 the value has the following format:
-- 
2.26.2


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] [PATCH v12 09/10] virtio-net: Avoid duplicate receive queue example

[Parav Pandit]

Receive queue number/index example is duplicate which is already defined
in the Setting RSS parameters section.

Hence, avoid such duplicate example and prepare it for the subsequent
patch to describe using receive queue handle.

Fixes: https://github.com/oasis-tcs/virtio-spec/issues/163
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
Signed-off-by: Parav Pandit <parav@nvidia.com>
---
 device-types/net/description.tex | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/device-types/net/description.tex b/device-types/net/description.tex
index d7c8b1b..435c1fc 100644
--- a/device-types/net/description.tex
+++ b/device-types/net/description.tex
@@ -1467,8 +1467,8 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
 The device MUST determine the destination queue for a network packet as follows:
 \begin{itemize}
 \item Calculate the hash of the packet as defined in \ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets}.
-\item If the device did not calculate the hash for the specific packet, the device directs the packet to the receiveq specified by \field{unclassified_queue} of virtio_net_rss_config structure (value of 0 corresponds to receiveq1).
-\item Apply \field{indirection_table_mask} to the calculated hash and use the result as the index in the indirection table to get 0-based number of destination receiveq (value of 0 corresponds to receiveq1).
+\item If the device did not calculate the hash for the specific packet, the device directs the packet to the receiveq specified by \field{unclassified_queue} of virtio_net_rss_config structure.
+\item Apply \field{indirection_table_mask} to the calculated hash and use the result as the index in the indirection table to get 0-based number of destination receiveq.
 \item If the destination receive queue is being reset (See \ref{sec:Basic Facilities of a Virtio Device / Virtqueues / Virtqueue Reset}), the device MUST drop the packet.
 \end{itemize}
 
-- 
2.26.2


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] [PATCH v12 10/10] virtio-net: Describe RSS using rss rq id

[Parav Pandit]

The content of the indirection table and unclassified_queue were
originally described based on mathematical operations. In order to
make it easier to understand and to avoid intermixing the array
index with the vq index, introduce a structure
rss_rq_id (RSS receive queue
ID) and use it to describe the unclassified_queue and
indirection_table fields.

As part of it, have the example that uses non-zero virtqueue
index which helps to have better mapping between receiveX
object with virtqueue index and the actual value in the
indirection table.

Fixes: https://github.com/oasis-tcs/virtio-spec/issues/163
Signed-off-by: Parav Pandit <parav@nvidia.com>

---
changelog:
v11->v12:
- use 'virtqueue index' instead of 'virtqueue number'
v10->v11:
- commit log updated to drop old reference to rq_handle, replaced with
  rss_rq_id detail
v8->v9:
- reword rss_rq_id and unclassified_packets description
- use article
- use 'vq number' instead of 'virtqueue number'
v4->v5:
- change subject to reflect rss_rq_id
- fixed accidental removal of "unclassifed packets".
- simplfied text around indirection_table mask to use the term
  destination receive virtqueue. This aligns with next line about queue
  reset.
- removed rss_rq_id references as indirection table and
  unclassified_queue data type is self explanatory
v3->v4:
- renamed rq_handle to rss_rq_id
- moved rss_rq_id definition close to its usage in rss_config struct
v2->v3:
- moved rq_handle definition before using it
- removed duplicate example as rq_handle is now described first
v0->v1:
- new patch suggested by Michael Tsirkin
---
 device-types/net/description.tex | 27 ++++++++++++++++++++-------
 1 file changed, 20 insertions(+), 7 deletions(-)

diff --git a/device-types/net/description.tex b/device-types/net/description.tex
index 435c1fc..4eb10d1 100644
--- a/device-types/net/description.tex
+++ b/device-types/net/description.tex
@@ -1423,11 +1423,16 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
 
 Driver sends a VIRTIO_NET_CTRL_MQ_RSS_CONFIG command using the following format for \field{command-specific-data}:
 \begin{lstlisting}
+struct rss_rq_id {
+   le16 vqn_1_16: 15; /* Bits 1 to 16 of the vq index */
+   le16 reserved: 1; /* Set to zero */
+};
+
 struct virtio_net_rss_config {
     le32 hash_types;
     le16 indirection_table_mask;
-    le16 unclassified_queue;
-    le16 indirection_table[indirection_table_length];
+    struct rss_rq_id unclassified_queue;
+    struct rss_rq_id indirection_table[indirection_table_length];
     le16 max_tx_vq;
     u8 hash_key_length;
     u8 hash_key_data[hash_key_length];
@@ -1442,10 +1447,15 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
 \field{indirection_table} array.
 Number of entries in \field{indirection_table} is (\field{indirection_table_mask} + 1).
 
-Field \field{unclassified_queue} contains the 0-based index of
-the receive virtqueue to place unclassified packets in. Index 0 corresponds to receiveq1.
+\field{rss_rq_id} is a receive virtqueue id. \field{vqn_1_16}
+consists of bits 1 to 16 of a vq index. For example, a
+\field{vqn_1_16} value of 3 corresponds to vq index 6,
+which maps to receiveq4.
+
+Field \field{unclassified_queue} contains the receive virtqueue
+in which to place unclassified packets.
 
-Field \field{indirection_table} contains an array of 0-based indices of receive virtqueues. Index 0 corresponds to receiveq1.
+Field \field{indirection_table} is an array of receive virtqueues.
 
 A driver sets \field{max_tx_vq} to inform a device how many transmit virtqueues it may use (transmitq1\ldots transmitq \field{max_tx_vq}).
 
@@ -1455,7 +1465,8 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
 
 A driver MUST NOT send the VIRTIO_NET_CTRL_MQ_RSS_CONFIG command if the feature VIRTIO_NET_F_RSS has not been negotiated.
 
-A driver MUST fill the \field{indirection_table} array only with indices of enabled queues. Index 0 corresponds to receiveq1.
+A driver MUST fill the \field{indirection_table} array only with
+enabled receive virtqueues.
 
 The number of entries in \field{indirection_table} (\field{indirection_table_mask} + 1) MUST be a power of two.
 
@@ -1468,7 +1479,9 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
 \begin{itemize}
 \item Calculate the hash of the packet as defined in \ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets}.
 \item If the device did not calculate the hash for the specific packet, the device directs the packet to the receiveq specified by \field{unclassified_queue} of virtio_net_rss_config structure.
-\item Apply \field{indirection_table_mask} to the calculated hash and use the result as the index in the indirection table to get 0-based number of destination receiveq.
+\item Apply \field{indirection_table_mask} to the calculated hash
+and use the result as the index in the indirection table to get
+the destination receive virtqueue.
 \item If the destination receive queue is being reset (See \ref{sec:Basic Facilities of a Virtio Device / Virtqueues / Virtqueue Reset}), the device MUST drop the packet.
 \end{itemize}
 
-- 
2.26.2


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] Re: [PATCH v12 03/10] content: Rename confusing queue_notify_data and vqn names

[Michael S. Tsirkin]

This is not much of an improvement.

On Wed, Apr 05, 2023 at 04:06:50AM +0300, Parav Pandit wrote:
> Currently queue_notify_data register indicates the device
> internal queue notification identifier. This register is
> meaningful only when feature bit VIRTIO_F_NOTIF_CONFIG_DATA is
> negotiated.
> 
> However, above register name often get confusing association with
> very similar feature bit VIRTIO_F_NOTIFICATION_DATA.
> 
> When VIRTIO_F_NOTIFICATION_DATA feature bit is negotiated,
> notification really involves sending additional queue progress
> related information (not queue identifier).
> 
> Hence
> 1. to avoid any misunderstanding and association of
> queue_notify_data with similar name VIRTIO_F_NOTIFICATION_DATA,
> 
> and
> 2. to reflect that queue_notify_data is the actual device
> internal vq identifier, rename it to queue_notify_id.
> 
> Reflect vq identifier in the driver notification structure by renaming
> ambiguous vqn to vq_notify_id.
> 
> The driver notification section assumes that queue notification contains
> vq index always. CONFIG_DATA feature bit introduction missed to
> update the driver notification section. Hence, correct it.
> 
> Signed-off-by: Parav Pandit <parav@nvidia.com>
> 
> ---
> Some side notes:
> renaming vqn to vqnd is even more confusing because data is really the
> queue identifier.

Clear to whom?  Why do you think so? Marvell who pushed this feature
said they stick some kind of constant value there which matches what
their hardware expects. Sounds like a valid way to use this.
So no, not an identifier, and in any case "vq identifier" is a really
general and useful term, I would rather not burn it up on
a baroque feature that almost no one sets - for almost
everyone else this is simply vq index.

If you really insist on renaming this away from "vqn",
maybe vq_index_config_data will do, and we
can add a comment /* Either vq index or vq config data, previously named vqn */


> And NOTIFICATION_DATA really contains queue progress info (data).
> 
> (vqn - n is number or notification?, notification word in the
> notification structure does not make sense).
> 
> Hence above renaming.


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] Re: [PATCH v12 01/10] content: Add vq index text

[Michael S. Tsirkin]

On Wed, Apr 05, 2023 at 04:06:48AM +0300, Parav Pandit wrote:
> Introduce vq index and its range so that subsequent patches can refer
> to it.
> 
> Fixes: https://github.com/oasis-tcs/virtio-spec/issues/163
> Reviewed-by: Cornelia Huck <cohuck@redhat.com>
> Signed-off-by: Parav Pandit <parav@nvidia.com>
> ---
> changelog:
> v11->v12:
> - renamed 'number' to 'index'
> v9->v10:
> - added braces around vq number wording
> - added vqn as another term for vq number
> v8->v9:
> - added inclusive when describing the vq number range
> - skipped comment to put virtqueue number wording first because we
>   prefer to use shorter vq number as much as possible
> v5->v6:
> - moved description close to introduction, it was in middle of
>   queue data transfer description
> v2->v3:
> - new patch
> ---
>  content.tex | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/content.tex b/content.tex
> index cff548a..e64115c 100644
> --- a/content.tex
> +++ b/content.tex
> @@ -298,6 +298,10 @@ \section{Virtqueues}\label{sec:Basic Facilities of a Virtio Device / Virtqueues}
>  virtqueues\footnote{For example, the simplest network device has one virtqueue for
>  transmit and one for receive.}.
>  
> +Each virtqueue is identified by a vq index (also referred
> +to as a virtqueue index); vq index range is from 0 to 65535
> +inclusive.
> +

It's the other way around: start with full name, then abbreviation.
Each virtqueue is identified by virtqueue
index (sometimes abbreviated to vq index). A virtqueue index range ...

are there more abbreviations we want to mention here? vq_idx?


>  Driver makes requests available to device by adding
>  an available buffer to the queue, i.e., adding a buffer
>  describing the request to a virtqueue, and optionally triggering
> -- 
> 2.26.2


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] Re: [PATCH v12 03/10] content: Rename confusing queue_notify_data and vqn names

[Michael S. Tsirkin]

On Wed, Apr 05, 2023 at 04:06:50AM +0300, Parav Pandit wrote:
> Currently queue_notify_data register indicates the device
> internal queue notification identifier. This register is
> meaningful only when feature bit VIRTIO_F_NOTIF_CONFIG_DATA is
> negotiated.
> 
> However, above register name often get confusing association with
> very similar feature bit VIRTIO_F_NOTIFICATION_DATA.
> 
> When VIRTIO_F_NOTIFICATION_DATA feature bit is negotiated,
> notification really involves sending additional queue progress
> related information (not queue identifier).
> 
> Hence
> 1. to avoid any misunderstanding and association of
> queue_notify_data with similar name VIRTIO_F_NOTIFICATION_DATA,
> 
> and
> 2. to reflect that queue_notify_data is the actual device
> internal vq identifier, rename it to queue_notify_id.
> 
> Reflect vq identifier in the driver notification structure by renaming
> ambiguous vqn to vq_notify_id.
> 
> The driver notification section assumes that queue notification contains
> vq index always. CONFIG_DATA feature bit introduction missed to
> update the driver notification section. Hence, correct it.
> 
> Signed-off-by: Parav Pandit <parav@nvidia.com>
> 
> ---
> Some side notes:
> renaming vqn to vqnd is even more confusing because data is really the
> queue identifier.
> 
> And NOTIFICATION_DATA really contains queue progress info (data).
> 
> (vqn - n is number or notification?, notification word in the
> notification structure does not make sense).
> 
> Hence above renaming.
> 
> ---
> changelog:
> v11->v12:
> - new patch
> ---
>  content.tex        | 17 ++++++++++++++---
>  notifications-be.c |  2 +-
>  notifications-le.c |  2 +-
>  transport-pci.tex  | 24 ++++++++++++++----------
>  4 files changed, 30 insertions(+), 15 deletions(-)
> 
> diff --git a/content.tex b/content.tex
> index cd93db2..d5f8026 100644
> --- a/content.tex
> +++ b/content.tex
> @@ -405,8 +405,18 @@ \section{Driver Notifications} \label{sec:Basic Facilities of a Virtio Device /
>  notification to the device.
>  
>  When VIRTIO_F_NOTIFICATION_DATA has not been negotiated,
> -this notification involves sending the
> -virtqueue index to the device (method depending on the transport).
> +this notification involves sending only the 16-bit virtqueue notification
> +identifier (notification method depends on the transport).
> +
> +\begin{itemize}
> +\item When VIRTIO_F_NOTIF_CONFIG_DATA is not negotiated, virtqueue
> +notification identifier is a 16-bit vq index.
> +
> +\item When VIRTIO_F_NOTIF_CONFIG_DATA is negotiated, virtqueue
> +notification identifier is a device supplied virtqueue identifier. A method
> +to supply such virtqueue notification identifier is transport
> +specific.
> +\end{itemize}
>  
>  However, some devices benefit from the ability to find out the
>  amount of available data in the queue without accessing the virtqueue in memory:


VIRTIO_F_NOTIF_CONFIG_DATA is such a narrow usecase, I don't like
burning "vq identifier" on this. How about we just say something
along the lines of:


When VIRTIO_F_NOTIFICATION_DATA has not been negotiated, this
notification involves sending either the virtqueue index or the
virtqueue config data to the device (method depending on the
transport).

And then "the data sent is a device supplied virtqueue config data".



> @@ -417,7 +427,8 @@ \section{Driver Notifications} \label{sec:Basic Facilities of a Virtio Device /
>  the following information:
>  
>  \begin{description}
> -\item [vqn] VQ number to be notified.
> +\item [vq_notify_id] VQ notification identifier of a
> +      corresponding virtqueue.
>  \item [next_off] Offset
>        within the ring where the next available ring entry
>        will be written.
> diff --git a/notifications-be.c b/notifications-be.c
> index 5be947e..0a7cbf1 100644
> --- a/notifications-be.c
> +++ b/notifications-be.c
> @@ -1,5 +1,5 @@
>  be32 {
> -	vqn : 16;
> +	vq_notify_id : 16; /* previously known as vqn */
>  	next_off : 15;
>  	next_wrap : 1;
>  };
> diff --git a/notifications-le.c b/notifications-le.c
> index fe51267..6cca8fb 100644
> --- a/notifications-le.c
> +++ b/notifications-le.c
> @@ -1,5 +1,5 @@
>  le32 {
> -	vqn : 16;
> +	vq_notify_id : 16; /* previously known as vqn */
>  	next_off : 15;
>  	next_wrap : 1;
>  };
> diff --git a/transport-pci.tex b/transport-pci.tex
> index 5d98467..6bbf61c 100644
> --- a/transport-pci.tex
> +++ b/transport-pci.tex
> @@ -319,7 +319,7 @@ \subsubsection{Common configuration structure layout}\label{sec:Virtio Transport
>          le64 queue_desc;                /* read-write */
>          le64 queue_driver;              /* read-write */
>          le64 queue_device;              /* read-write */
> -        le16 queue_notify_data;         /* read-only for driver */
> +        le16 queue_notify_id;           /* read-only for driver */
>          le16 queue_reset;               /* read-write */
>  };
>  \end{lstlisting}
> @@ -388,17 +388,21 @@ \subsubsection{Common configuration structure layout}\label{sec:Virtio Transport
>  \item[\field{queue_device}]
>          The driver writes the physical address of Device Area here.  See section \ref{sec:Basic Facilities of a Virtio Device / Virtqueues}.
>  
> -\item[\field{queue_notify_data}]
> +\item[\field{queue_notify_id}]
>          This field exists only if VIRTIO_F_NOTIF_CONFIG_DATA has been negotiated.
> -        The driver will use this value to put it in the 'virtqueue number' field
> -        in the available buffer notification structure.
> +        The driver will use this value when driver sends available buffer
> +        notification to the device.
>          See section \ref{sec:Virtio Transport Options / Virtio Over PCI Bus / PCI-specific Initialization And Device Operation / Available Buffer Notifications}.
>          \begin{note}
>          This field provides the device with flexibility to determine how virtqueues
>          will be referred to in available buffer notifications.
> -        In a trivial case the device can set \field{queue_notify_data}=vqn. Some devices
> -        may benefit from providing another value, for example an internal virtqueue
> -        identifier, or an internal offset related to the virtqueue number.
> +        In a trivial case the device can set \field{queue_notify_id} to
> +        vq index. Some devices may benefit from providing another value,
> +        for example an internal virtqueue identifier, or an internal offset
> +        related to the virtqueue index.
> +        \end{note}
> +        \begin{note}
> +        This field is previously known as queue_notify_id.
>          \end{note}
>  
>  \item[\field{queue_reset}]
> @@ -468,7 +472,7 @@ \subsubsection{Common configuration structure layout}\label{sec:Virtio Transport
>  
>  \drivernormative{\paragraph}{Common configuration structure layout}{Virtio Transport Options / Virtio Over PCI Bus / PCI Device Layout / Common configuration structure layout}
>  
> -The driver MUST NOT write to \field{device_feature}, \field{num_queues}, \field{config_generation}, \field{queue_notify_off} or \field{queue_notify_data}.
> +The driver MUST NOT write to \field{device_feature}, \field{num_queues}, \field{config_generation}, \field{queue_notify_off} or \field{queue_notify_id}.
>  
>  If VIRTIO_F_RING_PACKED has been negotiated,
>  the driver MUST NOT write the value 0 to \field{queue_size}.
> @@ -1053,9 +1057,9 @@ \subsubsection{Available Buffer Notifications}\label{sec:Virtio Transport Option
>  If VIRTIO_F_NOTIF_CONFIG_DATA has been negotiated:
>  \begin{itemize}
>  \item If VIRTIO_F_NOTIFICATION_DATA has not been negotiated, the driver MUST use the
> -\field{queue_notify_data} value instead of the virtqueue index.
> +\field{queue_notify_id} value instead of the virtqueue index.
>  \item If VIRTIO_F_NOTIFICATION_DATA has been negotiated, the driver MUST set the
> -\field{vqn} field to the \field{queue_notify_data} value.
> +\field{vq_notify_id} field to the \field{queue_notify_id} value.
>  \end{itemize}
>  
>  \subsubsection{Used Buffer Notifications}\label{sec:Virtio Transport Options / Virtio Over PCI Bus / PCI-specific Initialization And Device Operation / Used Buffer Notifications}
> -- 
> 2.26.2


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] Re: [PATCH v12 00/10] Rename queue number to queue index

[Michael S. Tsirkin]

On Wed, Apr 05, 2023 at 04:06:47AM +0300, Parav Pandit wrote:
> 1. Currently, virtqueue is identified between driver and device
> interchangeably using either number or index terminology.
> 
> 2. Between PCI and MMIO transport the queue size (depth) is
> defined as queue_size and QueueNum respectively.
> 
> To avoid confusion and to have consistency, unify them to use
> index.
> 
> 3. Filed name vqn in the driver notification structure is
> ambiguous as it is supposed to hold either vq index or device
> supplied vq identifier.
> 
> 4. Device is really supplying queue identifier in the 
> queue_notify_data register, and this often get confused with
> very similar looking feature bit NOTIFICATION_DATA.
> 
> Solution:
> a. Use virtqueue index description, and rename MMIO register as QueueSize.
> b. Replace virtqueue number with virtqueue index
> c. RSS area of virtio net has inherited some logic, describe it
> using abstract rss_rq_id.
> d. rename queue_notifify_data to queue_notify_id
> e. rename vqn to vq_notify_id to reflect it can hold either vq
> index of device supplied some id.
> 
> Patch summary:
> patch-1 introduce vq number as generic term
> patch-2 renames index to number for pci transport
> patch-3 rename queue_notify_data to queue_notify_id
> patch-4 remove first vq index reference
> patch-5 renames mmio register from Num to Size
> patch-6 renames index to number for mmio transport
> patch-7 renames num field to size for ccw transport
> patch-8 renames vq by its index for ccw transport
> patch-9 for virtio-net removes duplicate example from requirements
> patch-10 for virtio-net updates rss description to use vq index
> 
> This series only improves the documentation, it does not change any
> transport or device functionality.
> 
> Please review.
> This series fixes the issue [1].
> 
> [1] https://github.com/oasis-tcs/virtio-spec/issues/163


I like where this is going. I will change AQ patches to be like this,
but it's holidays here in israel so not reposting immediately,
pls review assuming I will address that change.
For same reason pls don't expect quick review.
The patchset itself is just a cleanup so even though it's up
to crazy revision numbers already it's ok if it takes a bit more
time to merge.

> ---
> changelog:
> v11->v12:
> - replace number to index
> - avoid confusion around vqn field and rename to vq_notify_id
> - rename queue_notify_data to avoid confusing with NOTIFY_DATA
> v10->v11:
> - added Reviewed-by for all the reviewed patches
> - updated commit log of patch-8 to drop rq_handle reference
> - skipped comment to further use rss_rq_id, as rss_rq_id usage
>   and structure are self describing
> v9->v10:
> - added virtqueue number part in content in braces
> - replaced queue_select to vqn in ccw
> - avoided aggrasive alignment of 65 chars
> - updated commit log to drop reference to already merged patches
> - added review-by tag for already reviewed patches
> v8->v9:
> - addressed comments from David
> - few corrections with article
> - renaming 'virtqueue number' to 'vq number'
> - improving text and wording for rss_rq_id, avail notification
> - commit log of specific change in individual patches
> v7->v8:
> - remove note about first virtqueue number
> - skipped Max's comment to put word 'structure' in same line as its
>   crosses 65 chars limit per line
> - reworded queue_notification data set line, as '=' and vq number
>   wording was odd
> v6->v7:
> - remove text around first vq as it is already covered in the basic
>   virtqueues facility section
> v5->v6:
> - moved the vq number description from middle of vq operation
>   to beginning of vq introduction
> v4->v5:
> - fixed accidental removal of "unclassifed packets".
> - simplfied text around indirection_table mask
> - removed rss_rq_id references as indirection table and
>   unclassified_queue data type is self explanatory
> v3->v4:
> - moved note to comment for ccw
> - renamed rq_handle to rss_rq_id
> - moved rss_rq_id next to rss_config structure
> - define rss_config structure using rss_rq_id
> v2->v3:
> - addressed comments from Michael
> - added previous definitions for ccw fields
> - moved rq_handle definition before using it
> - added first patch to describe vq number
> - updated pci for available buffer notification section
> v1->v2:
> - added patches for virtio net for rss area
> - added patches for covering ccw transport
> - added missing entries to refer in mmio transport
> 
> 
> Parav Pandit (10):
>   content: Add vq index text
>   content.tex Replace virtqueue number with index
>   content: Rename confusing queue_notify_data and vqn names
>   transport-pci: Avoid first vq index reference
>   transport-mmio: Rename QueueNum register
>   transport-mmio: Avoid referring to zero based index
>   transport-ccw: Rename queue depth/size to other transports
>   transport-ccw: Refer to the vq by its index
>   virtio-net: Avoid duplicate receive queue example
>   virtio-net: Describe RSS using rss rq id
> 
>  content.tex                      | 27 ++++++++++++----
>  device-types/net/description.tex | 29 ++++++++++++-----
>  notifications-be.c               |  2 +-
>  notifications-le.c               |  2 +-
>  transport-ccw.tex                | 15 +++++----
>  transport-mmio.tex               | 55 +++++++++++++++++++-------------
>  transport-pci.tex                | 26 ++++++++-------
>  7 files changed, 99 insertions(+), 57 deletions(-)
> 
> -- 
> 2.26.2


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] Re: [PATCH v12 10/10] virtio-net: Describe RSS using rss rq id

[Michael S. Tsirkin]

On Wed, Apr 05, 2023 at 04:06:57AM +0300, Parav Pandit wrote:
> The content of the indirection table and unclassified_queue were
> originally described based on mathematical operations. In order to
> make it easier to understand and to avoid intermixing the array
> index with the vq index, introduce a structure
> rss_rq_id (RSS receive queue
> ID) and use it to describe the unclassified_queue and
> indirection_table fields.
> 
> As part of it, have the example that uses non-zero virtqueue
> index which helps to have better mapping between receiveX
> object with virtqueue index and the actual value in the
> indirection table.
> 
> Fixes: https://github.com/oasis-tcs/virtio-spec/issues/163
> Signed-off-by: Parav Pandit <parav@nvidia.com>
> 
> ---
> changelog:
> v11->v12:
> - use 'virtqueue index' instead of 'virtqueue number'
> v10->v11:
> - commit log updated to drop old reference to rq_handle, replaced with
>   rss_rq_id detail
> v8->v9:
> - reword rss_rq_id and unclassified_packets description
> - use article
> - use 'vq number' instead of 'virtqueue number'
> v4->v5:
> - change subject to reflect rss_rq_id
> - fixed accidental removal of "unclassifed packets".
> - simplfied text around indirection_table mask to use the term
>   destination receive virtqueue. This aligns with next line about queue
>   reset.
> - removed rss_rq_id references as indirection table and
>   unclassified_queue data type is self explanatory
> v3->v4:
> - renamed rq_handle to rss_rq_id
> - moved rss_rq_id definition close to its usage in rss_config struct
> v2->v3:
> - moved rq_handle definition before using it
> - removed duplicate example as rq_handle is now described first
> v0->v1:
> - new patch suggested by Michael Tsirkin
> ---
>  device-types/net/description.tex | 27 ++++++++++++++++++++-------
>  1 file changed, 20 insertions(+), 7 deletions(-)
> 
> diff --git a/device-types/net/description.tex b/device-types/net/description.tex
> index 435c1fc..4eb10d1 100644
> --- a/device-types/net/description.tex
> +++ b/device-types/net/description.tex
> @@ -1423,11 +1423,16 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
>  
>  Driver sends a VIRTIO_NET_CTRL_MQ_RSS_CONFIG command using the following format for \field{command-specific-data}:
>  \begin{lstlisting}
> +struct rss_rq_id {
> +   le16 vqn_1_16: 15; /* Bits 1 to 16 of the vq index */

Do you want to then call this vq_index_1_16 ?
And in the comment I would just write virtqueue index.

> +   le16 reserved: 1; /* Set to zero */
> +};
> +
>  struct virtio_net_rss_config {
>      le32 hash_types;
>      le16 indirection_table_mask;
> -    le16 unclassified_queue;
> -    le16 indirection_table[indirection_table_length];
> +    struct rss_rq_id unclassified_queue;
> +    struct rss_rq_id indirection_table[indirection_table_length];
>      le16 max_tx_vq;
>      u8 hash_key_length;
>      u8 hash_key_data[hash_key_length];
> @@ -1442,10 +1447,15 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
>  \field{indirection_table} array.
>  Number of entries in \field{indirection_table} is (\field{indirection_table_mask} + 1).
>  
> -Field \field{unclassified_queue} contains the 0-based index of
> -the receive virtqueue to place unclassified packets in. Index 0 corresponds to receiveq1.
> +\field{rss_rq_id} is a receive virtqueue id. \field{vqn_1_16}
> +consists of bits 1 to 16 of a vq index. For example, a
> +\field{vqn_1_16} value of 3 corresponds to vq index 6,
> +which maps to receiveq4.
> +
> +Field \field{unclassified_queue} contains the receive virtqueue
> +in which to place unclassified packets.
>  
> -Field \field{indirection_table} contains an array of 0-based indices of receive virtqueues. Index 0 corresponds to receiveq1.
> +Field \field{indirection_table} is an array of receive virtqueues.
>  
>  A driver sets \field{max_tx_vq} to inform a device how many transmit virtqueues it may use (transmitq1\ldots transmitq \field{max_tx_vq}).
>  
> @@ -1455,7 +1465,8 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
>  
>  A driver MUST NOT send the VIRTIO_NET_CTRL_MQ_RSS_CONFIG command if the feature VIRTIO_NET_F_RSS has not been negotiated.
>  
> -A driver MUST fill the \field{indirection_table} array only with indices of enabled queues. Index 0 corresponds to receiveq1.
> +A driver MUST fill the \field{indirection_table} array only with
> +enabled receive virtqueues.
>  
>  The number of entries in \field{indirection_table} (\field{indirection_table_mask} + 1) MUST be a power of two.
>  
> @@ -1468,7 +1479,9 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
>  \begin{itemize}
>  \item Calculate the hash of the packet as defined in \ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets}.
>  \item If the device did not calculate the hash for the specific packet, the device directs the packet to the receiveq specified by \field{unclassified_queue} of virtio_net_rss_config structure.
> -\item Apply \field{indirection_table_mask} to the calculated hash and use the result as the index in the indirection table to get 0-based number of destination receiveq.
> +\item Apply \field{indirection_table_mask} to the calculated hash
> +and use the result as the index in the indirection table to get
> +the destination receive virtqueue.
>  \item If the destination receive queue is being reset (See \ref{sec:Basic Facilities of a Virtio Device / Virtqueues / Virtqueue Reset}), the device MUST drop the packet.
>  \end{itemize}
>  
> -- 
> 2.26.2


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] Re: [PATCH v12 01/10] content: Add vq index text

[Michael S. Tsirkin]

On Wed, Apr 05, 2023 at 04:06:48AM +0300, Parav Pandit wrote:
> Introduce vq index and its range so that subsequent patches can refer
> to it.
> 
> Fixes: https://github.com/oasis-tcs/virtio-spec/issues/163
> Reviewed-by: Cornelia Huck <cohuck@redhat.com>
> Signed-off-by: Parav Pandit <parav@nvidia.com>
> ---
> changelog:
> v11->v12:
> - renamed 'number' to 'index'
> v9->v10:
> - added braces around vq number wording
> - added vqn as another term for vq number
> v8->v9:
> - added inclusive when describing the vq number range
> - skipped comment to put virtqueue number wording first because we
>   prefer to use shorter vq number as much as possible
> v5->v6:
> - moved description close to introduction, it was in middle of
>   queue data transfer description
> v2->v3:
> - new patch
> ---
>  content.tex | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/content.tex b/content.tex
> index cff548a..e64115c 100644
> --- a/content.tex
> +++ b/content.tex
> @@ -298,6 +298,10 @@ \section{Virtqueues}\label{sec:Basic Facilities of a Virtio Device / Virtqueues}
>  virtqueues\footnote{For example, the simplest network device has one virtqueue for
>  transmit and one for receive.}.
>  
> +Each virtqueue is identified by a vq index (also referred
> +to as a virtqueue index); vq index range is from 0 to 65535
> +inclusive.
> +

In fact can we just say "virtqueue index" everywhere? The less slang
reader has to learn, the better.

>  Driver makes requests available to device by adding
>  an available buffer to the queue, i.e., adding a buffer
>  describing the request to a virtqueue, and optionally triggering
> -- 
> 2.26.2


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] Re: [PATCH v12 00/10] Rename queue number to queue index

[Michael S. Tsirkin]

On Wed, Apr 05, 2023 at 04:06:47AM +0300, Parav Pandit wrote:
> 1. Currently, virtqueue is identified between driver and device
> interchangeably using either number or index terminology.
> 
> 2. Between PCI and MMIO transport the queue size (depth) is
> defined as queue_size and QueueNum respectively.
> 
> To avoid confusion and to have consistency, unify them to use
> index.
> 
> 3. Filed name vqn in the driver notification structure is
> ambiguous as it is supposed to hold either vq index or device
> supplied vq identifier.
> 
> 4. Device is really supplying queue identifier in the 
> queue_notify_data register, and this often get confused with
> very similar looking feature bit NOTIFICATION_DATA.
> 
> Solution:
> a. Use virtqueue index description, and rename MMIO register as QueueSize.
> b. Replace virtqueue number with virtqueue index
> c. RSS area of virtio net has inherited some logic, describe it
> using abstract rss_rq_id.
> d. rename queue_notifify_data to queue_notify_id
> e. rename vqn to vq_notify_id to reflect it can hold either vq
> index of device supplied some id.
> 
> Patch summary:
> patch-1 introduce vq number as generic term
> patch-2 renames index to number for pci transport
> patch-3 rename queue_notify_data to queue_notify_id
> patch-4 remove first vq index reference
> patch-5 renames mmio register from Num to Size
> patch-6 renames index to number for mmio transport
> patch-7 renames num field to size for ccw transport
> patch-8 renames vq by its index for ccw transport
> patch-9 for virtio-net removes duplicate example from requirements
> patch-10 for virtio-net updates rss description to use vq index
> 
> This series only improves the documentation, it does not change any
> transport or device functionality.
> 
> Please review.
> This series fixes the issue [1].
> 
> [1] https://github.com/oasis-tcs/virtio-spec/issues/163

I feel there are now two patchsets here:
1. renaming of queue num to queue size
2. virtqueue index and rss clarification depending on it

1 seems mostly ready, for 2 I don't yet know how long it will take.
Split them up, also from POV of github issues?


> ---
> changelog:
> v11->v12:
> - replace number to index
> - avoid confusion around vqn field and rename to vq_notify_id
> - rename queue_notify_data to avoid confusing with NOTIFY_DATA
> v10->v11:
> - added Reviewed-by for all the reviewed patches
> - updated commit log of patch-8 to drop rq_handle reference
> - skipped comment to further use rss_rq_id, as rss_rq_id usage
>   and structure are self describing
> v9->v10:
> - added virtqueue number part in content in braces
> - replaced queue_select to vqn in ccw
> - avoided aggrasive alignment of 65 chars
> - updated commit log to drop reference to already merged patches
> - added review-by tag for already reviewed patches
> v8->v9:
> - addressed comments from David
> - few corrections with article
> - renaming 'virtqueue number' to 'vq number'
> - improving text and wording for rss_rq_id, avail notification
> - commit log of specific change in individual patches
> v7->v8:
> - remove note about first virtqueue number
> - skipped Max's comment to put word 'structure' in same line as its
>   crosses 65 chars limit per line
> - reworded queue_notification data set line, as '=' and vq number
>   wording was odd
> v6->v7:
> - remove text around first vq as it is already covered in the basic
>   virtqueues facility section
> v5->v6:
> - moved the vq number description from middle of vq operation
>   to beginning of vq introduction
> v4->v5:
> - fixed accidental removal of "unclassifed packets".
> - simplfied text around indirection_table mask
> - removed rss_rq_id references as indirection table and
>   unclassified_queue data type is self explanatory
> v3->v4:
> - moved note to comment for ccw
> - renamed rq_handle to rss_rq_id
> - moved rss_rq_id next to rss_config structure
> - define rss_config structure using rss_rq_id
> v2->v3:
> - addressed comments from Michael
> - added previous definitions for ccw fields
> - moved rq_handle definition before using it
> - added first patch to describe vq number
> - updated pci for available buffer notification section
> v1->v2:
> - added patches for virtio net for rss area
> - added patches for covering ccw transport
> - added missing entries to refer in mmio transport
> 
> 
> Parav Pandit (10):
>   content: Add vq index text
>   content.tex Replace virtqueue number with index
>   content: Rename confusing queue_notify_data and vqn names
>   transport-pci: Avoid first vq index reference
>   transport-mmio: Rename QueueNum register
>   transport-mmio: Avoid referring to zero based index
>   transport-ccw: Rename queue depth/size to other transports
>   transport-ccw: Refer to the vq by its index
>   virtio-net: Avoid duplicate receive queue example
>   virtio-net: Describe RSS using rss rq id
> 
>  content.tex                      | 27 ++++++++++++----
>  device-types/net/description.tex | 29 ++++++++++++-----
>  notifications-be.c               |  2 +-
>  notifications-le.c               |  2 +-
>  transport-ccw.tex                | 15 +++++----
>  transport-mmio.tex               | 55 +++++++++++++++++++-------------
>  transport-pci.tex                | 26 ++++++++-------
>  7 files changed, 99 insertions(+), 57 deletions(-)
> 
> -- 
> 2.26.2


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] Re: [PATCH v12 02/10] content.tex Replace virtqueue number with index

[Cornelia Huck]

On Wed, Apr 05 2023, Parav Pandit <parav@nvidia.com> wrote:

> Replace virtqueue number with index to align to rest of the
> specification.
>
> Fixes: https://github.com/oasis-tcs/virtio-spec/issues/163
> Signed-off-by: Parav Pandit <parav@nvidia.com>
>
> ---
> changelog:
> v11->v12:
> - new patch
> ---
>  content.tex | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)

Reviewed-by: Cornelia Huck <cohuck@redhat.com>


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] Re: [PATCH v12 03/10] content: Rename confusing queue_notify_data and vqn names

[Cornelia Huck]

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

> On Wed, Apr 05, 2023 at 04:06:50AM +0300, Parav Pandit wrote:
>> diff --git a/content.tex b/content.tex
>> index cd93db2..d5f8026 100644
>> --- a/content.tex
>> +++ b/content.tex
>> @@ -405,8 +405,18 @@ \section{Driver Notifications} \label{sec:Basic Facilities of a Virtio Device /
>>  notification to the device.
>>  
>>  When VIRTIO_F_NOTIFICATION_DATA has not been negotiated,
>> -this notification involves sending the
>> -virtqueue index to the device (method depending on the transport).
>> +this notification involves sending only the 16-bit virtqueue notification
>> +identifier (notification method depends on the transport).
>> +
>> +\begin{itemize}
>> +\item When VIRTIO_F_NOTIF_CONFIG_DATA is not negotiated, virtqueue
>> +notification identifier is a 16-bit vq index.
>> +
>> +\item When VIRTIO_F_NOTIF_CONFIG_DATA is negotiated, virtqueue
>> +notification identifier is a device supplied virtqueue identifier. A method
>> +to supply such virtqueue notification identifier is transport
>> +specific.
>> +\end{itemize}
>>  
>>  However, some devices benefit from the ability to find out the
>>  amount of available data in the queue without accessing the virtqueue in memory:
>
>
> VIRTIO_F_NOTIF_CONFIG_DATA is such a narrow usecase, I don't like
> burning "vq identifier" on this. How about we just say something
> along the lines of:
>
>
> When VIRTIO_F_NOTIFICATION_DATA has not been negotiated, this
> notification involves sending either the virtqueue index or the
> virtqueue config data to the device (method depending on the
> transport).
>
> And then "the data sent is a device supplied virtqueue config data".

Agreed, referring to "virtqueue configuration data" or somesuch is
better than using up the "vq identifier" name for it.


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] RE: [PATCH v12 01/10] content: Add vq index text

[Parav Pandit]

> From: Michael S. Tsirkin <mst@redhat.com>
> Sent: Wednesday, April 5, 2023 1:26 AM
> 
> On Wed, Apr 05, 2023 at 04:06:48AM +0300, Parav Pandit wrote:
> > Introduce vq index and its range so that subsequent patches can refer
> > to it.
> >
> > Fixes: https://github.com/oasis-tcs/virtio-spec/issues/163
> > Reviewed-by: Cornelia Huck <cohuck@redhat.com>
> > Signed-off-by: Parav Pandit <parav@nvidia.com>
> > ---
> > changelog:
> > v11->v12:
> > - renamed 'number' to 'index'
> > v9->v10:
> > - added braces around vq number wording
> > - added vqn as another term for vq number
> > v8->v9:
> > - added inclusive when describing the vq number range
> > - skipped comment to put virtqueue number wording first because we
> >   prefer to use shorter vq number as much as possible
> > v5->v6:
> > - moved description close to introduction, it was in middle of
> >   queue data transfer description
> > v2->v3:
> > - new patch
> > ---
> >  content.tex | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/content.tex b/content.tex index cff548a..e64115c 100644
> > --- a/content.tex
> > +++ b/content.tex
> > @@ -298,6 +298,10 @@ \section{Virtqueues}\label{sec:Basic Facilities
> > of a Virtio Device / Virtqueues}  virtqueues\footnote{For example, the
> > simplest network device has one virtqueue for  transmit and one for receive.}.
> >
> > +Each virtqueue is identified by a vq index (also referred to as a
> > +virtqueue index); vq index range is from 0 to 65535 inclusive.
> > +
> 
> It's the other way around: start with full name, then abbreviation.
> Each virtqueue is identified by virtqueue index (sometimes abbreviated to vq
> index). A virtqueue index range ...
>
Ok. developers end up using "virtqueue index" because of addition to "sometimes".
But fine.. will change.
 
> are there more abbreviations we want to mention here? vq_idx?
> 
We don't have any user of vq_idx, so will drop for now.

> 
> >  Driver makes requests available to device by adding  an available
> > buffer to the queue, i.e., adding a buffer  describing the request to
> > a virtqueue, and optionally triggering
> > --
> > 2.26.2


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] RE: [PATCH v12 10/10] virtio-net: Describe RSS using rss rq id

[Parav Pandit]

> From: Michael S. Tsirkin <mst@redhat.com>
> Sent: Wednesday, April 5, 2023 5:18 AM

> following format for \field{command-specific-data}:
> >  \begin{lstlisting}
> > +struct rss_rq_id {
> > +   le16 vqn_1_16: 15; /* Bits 1 to 16 of the vq index */
> 
> Do you want to then call this vq_index_1_16 ?
Yes. will change.

> And in the comment I would just write virtqueue index.
> 
Why do you prefer longer to name here instead of abbreviation?

---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] RE: [PATCH v12 01/10] content: Add vq index text

[Parav Pandit]

> From: Michael S. Tsirkin <mst@redhat.com>
> Sent: Wednesday, April 5, 2023 5:19 AM
> 
> In fact can we just say "virtqueue index" everywhere? The less slang reader has
> to learn, the better.

Wow, didn't know that "vq" is a slang. :)

---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] RE: [PATCH v12 00/10] Rename queue number to queue index

[Parav Pandit]

> From: Michael S. Tsirkin <mst@redhat.com>
> Sent: Wednesday, April 5, 2023 5:21 AM

> > [1] https://github.com/oasis-tcs/virtio-spec/issues/163
> 
> I feel there are now two patchsets here:
> 1. renaming of queue num to queue size
> 2. virtqueue index and rss clarification depending on it
> 
> 1 seems mostly ready, for 2 I don't yet know how long it will take.
> Split them up, also from POV of github issues?
>
Num, index all related..
If v13 cannot converge, I will split.



---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] RE: [PATCH v12 00/10] Rename queue number to queue index

[Parav Pandit]

> From: Michael S. Tsirkin <mst@redhat.com>
> Sent: Wednesday, April 5, 2023 1:33 AM

> 
> I like where this is going. I will change AQ patches to be like this, but it's holidays
> here in israel so not reposting immediately, pls review assuming I will address
> that change.
Yes, will review AQ shortly.

> For same reason pls don't expect quick review.

o.k. we need to wrap up AQ, transitional device soon to be part of 1.3 spec as users needs to start using it soon.
So once holidays are over, lets close the open items for both of them.

---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] RE: [PATCH v12 03/10] content: Rename confusing queue_notify_data and vqn names

[Parav Pandit]

> From: Michael S. Tsirkin <mst@redhat.com>
> Sent: Wednesday, April 5, 2023 1:22 AM
> 
> This is not much of an improvement.
> 
> On Wed, Apr 05, 2023 at 04:06:50AM +0300, Parav Pandit wrote:
> > Currently queue_notify_data register indicates the device internal
> > queue notification identifier. This register is meaningful only when
> > feature bit VIRTIO_F_NOTIF_CONFIG_DATA is negotiated.
> >
> > However, above register name often get confusing association with very
> > similar feature bit VIRTIO_F_NOTIFICATION_DATA.
> >
> > When VIRTIO_F_NOTIFICATION_DATA feature bit is negotiated,
> > notification really involves sending additional queue progress related
> > information (not queue identifier).
> >
> > Hence
> > 1. to avoid any misunderstanding and association of queue_notify_data
> > with similar name VIRTIO_F_NOTIFICATION_DATA,
> >
> > and
> > 2. to reflect that queue_notify_data is the actual device internal vq
> > identifier, rename it to queue_notify_id.
> >
> > Reflect vq identifier in the driver notification structure by renaming
> > ambiguous vqn to vq_notify_id.
> >
> > The driver notification section assumes that queue notification
> > contains vq index always. CONFIG_DATA feature bit introduction missed
> > to update the driver notification section. Hence, correct it.
> >
> > Signed-off-by: Parav Pandit <parav@nvidia.com>
> >
> > ---
> > Some side notes:
> > renaming vqn to vqnd is even more confusing because data is really the
> > queue identifier.
> 
> Clear to whom?  Why do you think so? Marvell who pushed this feature said
> they stick some kind of constant value there which matches what their
> hardware expects. Sounds like a valid way to use this.
Spec currently has described it as "identifier".
Snippet:
"for example an internal virtqueue identifier,
or an internal offset related to the virtqueue number"

> So no, not an identifier, and in any case "vq identifier" is a really general and
> useful term, I would rather not burn it up on a baroque feature that almost no
> one sets - for almost everyone else this is simply vq index.
> 
> If you really insist on renaming this away from "vqn", maybe
> vq_index_config_data will do, and we can add a comment /* Either vq index or
> vq config data, previously named vqn */
> 
Vq_index_config_data also aligns with the crazy feature bit name.
So I will use that.

---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] RE: [PATCH v12 03/10] content: Rename confusing queue_notify_data and vqn names

[Parav Pandit]

> From: Michael S. Tsirkin <mst@redhat.com>
> Sent: Wednesday, April 5, 2023 1:30 AM


> VIRTIO_F_NOTIF_CONFIG_DATA is such a narrow usecase, I don't like burning
> "vq identifier" on this. How about we just say something along the lines of:
> 
Ok. 
> 
> When VIRTIO_F_NOTIFICATION_DATA has not been negotiated, this
> notification involves sending either the virtqueue index or the virtqueue config
> data to the device (method depending on the transport).
> 
> And then "the data sent is a device supplied virtqueue config data".
> 
Sounds fine. I will reword it.

---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

Re: [virtio-dev] RE: [PATCH v12 03/10] content: Rename confusing queue_notify_data and vqn names

[Halil Pasic]

On Wed, 5 Apr 2023 13:21:40 +0000
Parav Pandit <parav@nvidia.com> wrote:

> > VIRTIO_F_NOTIF_CONFIG_DATA is such a narrow usecase, I don't like burning
> > "vq identifier" on this. How about we just say something along the lines of:
> >   
> Ok. 
> > 
> > When VIRTIO_F_NOTIFICATION_DATA has not been negotiated, this
> > notification involves sending either the virtqueue index or the virtqueue config
> > data to the device (method depending on the transport).
> > 
> > And then "the data sent is a device supplied virtqueue config data".
> >   
> Sounds fine. I will reword it.

FYI in an other thread I proposed calling this a "cookie". Sorry for
being late to the party. Yet again.

Regards,
Halil

---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

RE: [virtio-dev] RE: [PATCH v12 03/10] content: Rename confusing queue_notify_data and vqn names

[Parav Pandit]

> From: Halil Pasic <pasic@linux.ibm.com>
> Sent: Wednesday, April 5, 2023 11:28 AM
> 
> On Wed, 5 Apr 2023 13:21:40 +0000
> Parav Pandit <parav@nvidia.com> wrote:
> 
> > > VIRTIO_F_NOTIF_CONFIG_DATA is such a narrow usecase, I don't like
> > > burning "vq identifier" on this. How about we just say something along the
> lines of:
> > >
> > Ok.
> > >
> > > When VIRTIO_F_NOTIFICATION_DATA has not been negotiated, this
> > > notification involves sending either the virtqueue index or the
> > > virtqueue config data to the device (method depending on the transport).
> > >
> > > And then "the data sent is a device supplied virtqueue config data".
> > >
> > Sounds fine. I will reword it.
> 
> FYI in an other thread I proposed calling this a "cookie". Sorry for being late to
> the party. Yet again.

If we spend (waste) more time, we will find many examples where "identifier" and "cookie" both are used in things associated with computer science.

That too when same set of people has accepted text " internal virtqueue identifier" for same feature of CONFIG_DATA even though somehow it was not "id"!
And when this spec refers to an RFC of UUID, session id (not "session cookie", even though session id is opaque and not meaningful to the recipient as per Wikipedia usage desc that you listed).

For sure "cookie" is better than "config_data" and I don't have objection to "cookie".

But I disagree to the claim that "identifier" is less good than "cookie".

It is pointless debate of "identifier" vs "cookie".

The union format is very useful to describe this crisply, I will use it.

Do you prefer to rename F_CONFIG_DATA To F_CONFIG_COOKIE?

---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

Re: [virtio-dev] RE: [PATCH v12 01/10] content: Add vq index text

[Michael S. Tsirkin]

On Wed, Apr 05, 2023 at 01:03:29PM +0000, Parav Pandit wrote:
> 
> 
> > From: Michael S. Tsirkin <mst@redhat.com>
> > Sent: Wednesday, April 5, 2023 5:19 AM
> > 
> > In fact can we just say "virtqueue index" everywhere? The less slang reader has
> > to learn, the better.
> 
> Wow, didn't know that "vq" is a slang. :)

And so is "virtqueue".  It is certainly not in my dictionary of standard english.
One term to learn is enough I think.

-- 
MST


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] Re: [virtio-comment] RE: [PATCH v12 10/10] virtio-net: Describe RSS using rss rq id

[Michael S. Tsirkin]

On Wed, Apr 05, 2023 at 01:02:24PM +0000, Parav Pandit wrote:
> 
> 
> > From: Michael S. Tsirkin <mst@redhat.com>
> > Sent: Wednesday, April 5, 2023 5:18 AM
> 
> > following format for \field{command-specific-data}:
> > >  \begin{lstlisting}
> > > +struct rss_rq_id {
> > > +   le16 vqn_1_16: 15; /* Bits 1 to 16 of the vq index */
> > 
> > Do you want to then call this vq_index_1_16 ?
> Yes. will change.
> 
> > And in the comment I would just write virtqueue index.
> > 
> Why do you prefer longer to name here instead of abbreviation?

Eschew abbreviation is standard advice for writers.
Full names reduce the cognitive load - reader has less terms to
remember to be able to understand the spec.

-- 
MST


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] Re: [virtio-comment] RE: [virtio-dev] RE: [PATCH v12 03/10] content: Rename confusing queue_notify_data and vqn names

[Michael S. Tsirkin]

On Wed, Apr 05, 2023 at 03:58:55PM +0000, Parav Pandit wrote:
> > From: Halil Pasic <pasic@linux.ibm.com>
> > Sent: Wednesday, April 5, 2023 11:28 AM
> > 
> > On Wed, 5 Apr 2023 13:21:40 +0000
> > Parav Pandit <parav@nvidia.com> wrote:
> > 
> > > > VIRTIO_F_NOTIF_CONFIG_DATA is such a narrow usecase, I don't like
> > > > burning "vq identifier" on this. How about we just say something along the
> > lines of:
> > > >
> > > Ok.
> > > >
> > > > When VIRTIO_F_NOTIFICATION_DATA has not been negotiated, this
> > > > notification involves sending either the virtqueue index or the
> > > > virtqueue config data to the device (method depending on the transport).
> > > >
> > > > And then "the data sent is a device supplied virtqueue config data".
> > > >
> > > Sounds fine. I will reword it.
> > 
> > FYI in an other thread I proposed calling this a "cookie". Sorry for being late to
> > the party. Yet again.
> 
> If we spend (waste) more time, we will find many examples where "identifier" and "cookie" both are used in things associated with computer science.
> 
> That too when same set of people has accepted text " internal virtqueue identifier" for same feature of CONFIG_DATA even though somehow it was not "id"!

because that's just an example:
        In a trivial case the device can set \field{queue_notify_data}=vqn. Some devices
        may benefit from providing another value, for example an internal virtqueue
        identifier, or an internal offset related to the virtqueue number.

so the cookie can either be an identifier or something else.


> And when this spec refers to an RFC of UUID, session id (not "session cookie", even though session id is opaque and not meaningful to the recipient as per Wikipedia usage desc that you listed).
> 
> For sure "cookie" is better than "config_data" and I don't have objection to "cookie".
> 
> But I disagree to the claim that "identifier" is less good than "cookie".
> 
> It is pointless debate of "identifier" vs "cookie".
> 
> The union format is very useful to describe this crisply, I will use it.

I guess I'm fine with "cookie" in that in CS it is by now widely
understood to mean "some opaque data".
identifier comes from "identical", so implies a 1:1 mapping IMO.


The logic behind using a cookie is that it's a bit similar
to host cookie from ccw.
However, with ccw host cookie is used unconditionally, as
opposed to depending on the flag.



> Do you prefer to rename F_CONFIG_DATA To F_CONFIG_COOKIE?

It should all be consistent, but I worry about ccw which uses cookies
unconditionally. I am fine with leaving it as F_CONFIG_DATA for now
unless we see a good way to avoid confusion for ccw.

-- 
MST


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] Re: [virtio-comment] RE: [PATCH v12 03/10] content: Rename confusing queue_notify_data and vqn names

[Michael S. Tsirkin]

On Wed, Apr 05, 2023 at 01:21:40PM +0000, Parav Pandit wrote:
> 
> 
> > From: Michael S. Tsirkin <mst@redhat.com>
> > Sent: Wednesday, April 5, 2023 1:30 AM
> 
> 
> > VIRTIO_F_NOTIF_CONFIG_DATA is such a narrow usecase, I don't like burning
> > "vq identifier" on this. How about we just say something along the lines of:
> > 
> Ok. 
> > 
> > When VIRTIO_F_NOTIFICATION_DATA has not been negotiated, this
> > notification involves sending either the virtqueue index or the virtqueue config
> > data to the device (method depending on the transport).
> > 
> > And then "the data sent is a device supplied virtqueue config data".
> > 
> Sounds fine. I will reword it.

FWIW I like this more than coming up with a cookie term, it is definitely a
more conservative approach.

-- 
MST


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

RE: [virtio-dev] RE: [PATCH v12 01/10] content: Add vq index text

[Parav Pandit]

> From: Michael S. Tsirkin <mst@redhat.com>
> Sent: Friday, April 7, 2023 7:30 AM
> On Wed, Apr 05, 2023 at 01:03:29PM +0000, Parav Pandit wrote:
> >
> >
> > > From: Michael S. Tsirkin <mst@redhat.com>
> > > Sent: Wednesday, April 5, 2023 5:19 AM
> > >
> > > In fact can we just say "virtqueue index" everywhere? The less slang
> > > reader has to learn, the better.
> >
> > Wow, didn't know that "vq" is a slang. :)
> 
> And so is "virtqueue".  It is certainly not in my dictionary of standard english.
> One term to learn is enough I think.

Sound good to me.
With above reasoning please replace all "vq" with "virtqueue" in the commit log and the spec area in aq series v12.


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

Re: [virtio-dev] RE: [PATCH v12 01/10] content: Add vq index text

[Michael S. Tsirkin]

On Fri, Apr 07, 2023 at 03:34:19PM +0000, Parav Pandit wrote:
> 
> 
> > From: Michael S. Tsirkin <mst@redhat.com>
> > Sent: Friday, April 7, 2023 7:30 AM
> > On Wed, Apr 05, 2023 at 01:03:29PM +0000, Parav Pandit wrote:
> > >
> > >
> > > > From: Michael S. Tsirkin <mst@redhat.com>
> > > > Sent: Wednesday, April 5, 2023 5:19 AM
> > > >
> > > > In fact can we just say "virtqueue index" everywhere? The less slang
> > > > reader has to learn, the better.
> > >
> > > Wow, didn't know that "vq" is a slang. :)
> > 
> > And so is "virtqueue".  It is certainly not in my dictionary of standard english.
> > One term to learn is enough I think.
> 
> Sound good to me.
> With above reasoning please replace all "vq" with "virtqueue" in the commit log and the spec area in aq series v12.

commit log does not matter much, it's for us spec experts who know
the jargon :) I did not think aq patches say vq anywhere
but if yes I will change that, sure. Feel free to point it out.
Abbreviation of register names is probably ok, I'll check
whether other registers are abbreviated or not and make this match.

-- 
MST


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] Re: [PATCH v12 03/10] content: Rename confusing queue_notify_data and vqn names

[Michael S. Tsirkin]

On Wed, Apr 05, 2023 at 01:20:04PM +0000, Parav Pandit wrote:
> 
> 
> > From: Michael S. Tsirkin <mst@redhat.com>
> > Sent: Wednesday, April 5, 2023 1:22 AM
> > 
> > This is not much of an improvement.
> > 
> > On Wed, Apr 05, 2023 at 04:06:50AM +0300, Parav Pandit wrote:
> > > Currently queue_notify_data register indicates the device internal
> > > queue notification identifier. This register is meaningful only when
> > > feature bit VIRTIO_F_NOTIF_CONFIG_DATA is negotiated.
> > >
> > > However, above register name often get confusing association with very
> > > similar feature bit VIRTIO_F_NOTIFICATION_DATA.
> > >
> > > When VIRTIO_F_NOTIFICATION_DATA feature bit is negotiated,
> > > notification really involves sending additional queue progress related
> > > information (not queue identifier).
> > >
> > > Hence
> > > 1. to avoid any misunderstanding and association of queue_notify_data
> > > with similar name VIRTIO_F_NOTIFICATION_DATA,
> > >
> > > and
> > > 2. to reflect that queue_notify_data is the actual device internal vq
> > > identifier, rename it to queue_notify_id.
> > >
> > > Reflect vq identifier in the driver notification structure by renaming
> > > ambiguous vqn to vq_notify_id.
> > >
> > > The driver notification section assumes that queue notification
> > > contains vq index always. CONFIG_DATA feature bit introduction missed
> > > to update the driver notification section. Hence, correct it.
> > >
> > > Signed-off-by: Parav Pandit <parav@nvidia.com>
> > >
> > > ---
> > > Some side notes:
> > > renaming vqn to vqnd is even more confusing because data is really the
> > > queue identifier.
> > 
> > Clear to whom?  Why do you think so? Marvell who pushed this feature said
> > they stick some kind of constant value there which matches what their
> > hardware expects. Sounds like a valid way to use this.
> Spec currently has described it as "identifier".

Nope, it gives an identifier as one example of a legal value.


> Snippet:
> "for example an internal virtqueue identifier,
> or an internal offset related to the virtqueue number"
> 
> > So no, not an identifier, and in any case "vq identifier" is a really general and
> > useful term, I would rather not burn it up on a baroque feature that almost no
> > one sets - for almost everyone else this is simply vq index.
> > 
> > If you really insist on renaming this away from "vqn", maybe
> > vq_index_config_data will do, and we can add a comment /* Either vq index or
> > vq config data, previously named vqn */
> > 
> Vq_index_config_data also aligns with the crazy feature bit name.
> So I will use that.

:)

It's just standard thing you get if you overload fields. And this
is data path we could not just add fields easily.

-- 
MST


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] Re: [virtio-comment] RE: [virtio-dev] RE: [PATCH v12 03/10] content: Rename confusing queue_notify_data and vqn names

[Cornelia Huck]

On Fri, Apr 07 2023, "Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Wed, Apr 05, 2023 at 03:58:55PM +0000, Parav Pandit wrote:
>> For sure "cookie" is better than "config_data" and I don't have objection to "cookie".
>> 
>> But I disagree to the claim that "identifier" is less good than "cookie".
>> 
>> It is pointless debate of "identifier" vs "cookie".
>> 
>> The union format is very useful to describe this crisply, I will use it.
>
> I guess I'm fine with "cookie" in that in CS it is by now widely
> understood to mean "some opaque data".
> identifier comes from "identical", so implies a 1:1 mapping IMO.

Agreed, a "cookie" is not the same as an "identifier", although it may
contain one.

>
>
> The logic behind using a cookie is that it's a bit similar
> to host cookie from ccw.
> However, with ccw host cookie is used unconditionally, as
> opposed to depending on the flag.
>
>
>
>> Do you prefer to rename F_CONFIG_DATA To F_CONFIG_COOKIE?
>
> It should all be consistent, but I worry about ccw which uses cookies
> unconditionally. I am fine with leaving it as F_CONFIG_DATA for now
> unless we see a good way to avoid confusion for ccw.

Yes, please leave it as F_CONFIG_DATA, as we're just putting some "data"
there in the end (and F_CONFIG_COOKIE might indeed be confusing for the
ccw case.)


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] RE: [virtio-comment] RE: [virtio-dev] RE: [PATCH v12 03/10] content: Rename confusing queue_notify_data and vqn names

[Parav Pandit]

> From: Cornelia Huck <cohuck@redhat.com>
> Sent: Tuesday, April 11, 2023 4:56 AM

> 
> Yes, please leave it as F_CONFIG_DATA, as we're just putting some "data"
> there in the end (and F_CONFIG_COOKIE might indeed be confusing for the
> ccw case.)

Since Halil didn't respond for 5+ days + Michel and you propose to continue use CONFIG_DATA and this is rare used field, I will rename 

vqn to union of

vq_index
vq_config_data

Thanks. Will roll v13.

---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] Re: [virtio-comment] RE: [virtio-dev] RE: [PATCH v12 03/10] content: Rename confusing queue_notify_data and vqn names

[Halil Pasic]

On Tue, 11 Apr 2023 13:35:09 +0000
Parav Pandit <parav@nvidia.com> wrote:

> > From: Cornelia Huck <cohuck@redhat.com>
> > Sent: Tuesday, April 11, 2023 4:56 AM  
> 
> > 
> > Yes, please leave it as F_CONFIG_DATA, as we're just putting some "data"
> > there in the end (and F_CONFIG_COOKIE might indeed be confusing for the
> > ccw case.)  
> 
> Since Halil didn't respond for 5+ days + Michel and you propose to continue use CONFIG_DATA and this is rare used field, I will rename 
> 

Sorry, this one has fallen through the cracks. For
the preceding ones: I do not have a strong opinion. I do
share Michael's and Connie's assessment regarding a possible
clash with CCW.

Let me just note that the feature ain't called, "F_CONFIG_DATA" (i.e.
with full name VIRTIO_F_CONFIG_DATA) but rather "F_NOTIF_CONFIG_DATA"
(i.e. with full name VIRTIO_F_NOTIF_CONFIG_DATA).

> vqn to union of
> 
> vq_index
> vq_config_data

In that sense vq_config_data is not good in my opinion, because
it misses "notif" which is present in both "F_NOTIF_CONFIG_DATA"
and "queue_notify_data".
> 
> Thanks. Will roll v13.
>

I'm about tho have a look how this panned out in v13. I propose
let us continue the discussion there.

Regards,
Halil
 
> 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/
> 


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] Re: [virtio-comment] RE: [virtio-dev] RE: [PATCH v12 03/10] content: Rename confusing queue_notify_data and vqn names

[Michael S. Tsirkin]

On Mon, Apr 17, 2023 at 05:18:44AM +0200, Halil Pasic wrote:
> On Tue, 11 Apr 2023 13:35:09 +0000
> Parav Pandit <parav@nvidia.com> wrote:
> 
> > > From: Cornelia Huck <cohuck@redhat.com>
> > > Sent: Tuesday, April 11, 2023 4:56 AM  
> > 
> > > 
> > > Yes, please leave it as F_CONFIG_DATA, as we're just putting some "data"
> > > there in the end (and F_CONFIG_COOKIE might indeed be confusing for the
> > > ccw case.)  
> > 
> > Since Halil didn't respond for 5+ days + Michel and you propose to continue use CONFIG_DATA and this is rare used field, I will rename 
> > 
> 
> Sorry, this one has fallen through the cracks.

Well this whole patchset is just a cleanup so it's not holding up other
work at least. But I have to say it's difficult to make progress when
someone comes back from outer space after more than a week of silence
while others finished a discussion and reopens it with some new
feedback.

I am not saying don't give feedback but I'm saying please help us
all be more organized, feedback time really should be within a
day or two, in rare cases up to a week.

And I'd like to remind everyone if you are going away you are supposed
to report a leave of absence.

TC's that have meetings just take away voting rights from someone who
does not attend two meetings in a row.  We do it by ballot so this does
not apply, but I think we should set some limits in group's bylaws,
anyway. Ideas on formalizing this? If not we can just have informal
guidelines.  There's of course a flip side to this. Some patches
seemingly go through two versions a day. Keeping up becomes a full time
job. We'd need a guideline for that, too.

I also feel high latency is one of the reasons people are beginning to
ask to split into subcommitees where they won't have to deal with this
kind of thing. Let's try to keep the latency low, please.

> For
> the preceding ones: I do not have a strong opinion. I do
> share Michael's and Connie's assessment regarding a possible
> clash with CCW.
> 
> Let me just note that the feature ain't called, "F_CONFIG_DATA" (i.e.
> with full name VIRTIO_F_CONFIG_DATA) but rather "F_NOTIF_CONFIG_DATA"
> (i.e. with full name VIRTIO_F_NOTIF_CONFIG_DATA).
> 
> > vqn to union of
> > 
> > vq_index
> > vq_config_data
> 
> In that sense vq_config_data is not good in my opinion, because
> it misses "notif" which is present in both "F_NOTIF_CONFIG_DATA"
> and "queue_notify_data".
> > 
> > Thanks. Will roll v13.
> >
> 
> I'm about tho have a look how this panned out in v13. I propose
> let us continue the discussion there.
> 
> Regards,
> Halil
>  
> > 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/
> > 


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] Participation (was: Re: [virtio-comment] RE: [virtio-dev] RE: [PATCH v12 03/10] content: Rename confusing queue_notify_data and vqn names)

[Cornelia Huck]

On Mon, Apr 17 2023, "Michael S. Tsirkin" <mst@redhat.com> wrote:

> I am not saying don't give feedback but I'm saying please help us
> all be more organized, feedback time really should be within a
> day or two, in rare cases up to a week.

Given that there are things like weekends, public holidays, etc. one day
does not look reasonable; while it certainly makes sense to continue if
no feedback is forthcoming for a few days, not accounting for the fact
that this is not the exclusive job for most/any of us is just a fast
track to either burnout or people dropping out of virtio standardization
altogether.

> And I'd like to remind everyone if you are going away you are supposed
> to report a leave of absence.

Well... "Each request shall be made by sending an email to the TC
mailing list at least 7 days before the Leave is to start." is probably
not going to work for many cases. (Also, in any other community I'm
participating in it is expected that you just might not be there or have
time to work on it every week -- I've always seen that leave of absence
thingy as something for a really long vacation or for something like
parental leave, for which the max 45 days is really too short...) Not to
mention that it applies to voting, not to participation on the lists.

> TC's that have meetings just take away voting rights from someone who
> does not attend two meetings in a row.  We do it by ballot so this does
> not apply, but I think we should set some limits in group's bylaws,
> anyway. Ideas on formalizing this? If not we can just have informal
> guidelines.  There's of course a flip side to this. Some patches
> seemingly go through two versions a day. Keeping up becomes a full time
> job. We'd need a guideline for that, too.

What do we actually expect from TC members? "Reply to emails" is not
part of any formal requirement AFAICS (and not all TC members do
participate on the lists on a regular basis anyway). The requirement is
only to vote on the ballots, and there you're completely free to vote
"abstain", so you can always squeeze in voting even if you're not able
to participate otherwise. I think that's fine.

"If I don't get a reply after $NUMBER_OF_WORKING_DAYS, I'll assume I can
proceed as I think fit" is a reasonable assumption to make, e.g. to
request a vote. Not sure if/how to formalize that. Also, how is this
supposed to work if the original author doesn't reply to comments?
Should the proposal be considered abandoned?

I agree that patch reposting is happening much too fast in some
cases. Not sure how to formalize that, either. Can we please just be
more mindful of that? Reviewing time is not free. If I'm trying to do a
timely review of something and constantly see new versions while I'm not
finished yet, I do not feel like my feedback is actually valued.

> I also feel high latency is one of the reasons people are beginning to
> ask to split into subcommitees where they won't have to deal with this
> kind of thing. Let's try to keep the latency low, please.

I think there's multiple things to unpack here.

- The very common strain of limited reviewer time. This seems to be an
  issue nearly everywhere. Encouraging more review helps; but if review
  and ensuing discussing turns into a time sink, it just cannot be
  handled at a reasonable activity level anymore.
- Latency due to missing feedback. Can be solved by just requesting a
  vote if no feedback is forthcoming in a reasonable time frame.
- Latency due to missing reaction to feedback. This means the proposal
  just doesn't make any progress. The onus is on the submitter here.
- Conflicting approaches favoured by different people. This cannot be
  resolved in a formal way; either people need to be convinced that a
  certain approach will work, a middle ground found, or a way worked out
  that the different approaches can co-exist. In any case, this usually
  means long discussions which can be very frustrating, but unless we
  want to bulldoze over some people this is something we'll have to
  live with. [Personally, I think this is the worst contributor to
  frustration, and not something that can be solved by subcommittees.]
- [I'm also not happy with the tone of some emails I've been seeing. I
  won't point to them in order not to stir up things that have already
  calmed down again.]


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] Re: Participation (was: Re: [virtio-comment] RE: [virtio-dev] RE: [PATCH v12 03/10] content: Rename confusing queue_notify_data and vqn names)

[Michael S. Tsirkin]

On Mon, Apr 17, 2023 at 10:47:47AM +0200, Cornelia Huck wrote:
> I agree that patch reposting is happening much too fast in some
> cases. Not sure how to formalize that, either. Can we please just be
> more mindful of that? Reviewing time is not free. If I'm trying to do a
> timely review of something and constantly see new versions while I'm not
> finished yet, I do not feel like my feedback is actually valued.

Imagine a group of contributors working on spec 100% of time.
What do you want them to do? they need to exchange patches,
slowing them down because part time reviewers are overwhelmed
would be a waste. A separate mailing list would be one
solution. Some tag in the subject would be another. RFC?
Though RFC is used for other things as well...

-- 
MST


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

Re: [virtio-dev] Re: [virtio-comment] RE: [virtio-dev] RE: [PATCH v12 03/10] content: Rename confusing queue_notify_data and vqn names

[Halil Pasic]

On Mon, 17 Apr 2023 03:04:34 -0400
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Mon, Apr 17, 2023 at 05:18:44AM +0200, Halil Pasic wrote:
> > On Tue, 11 Apr 2023 13:35:09 +0000
> > Parav Pandit <parav@nvidia.com> wrote:
> >   
> > > > From: Cornelia Huck <cohuck@redhat.com>
> > > > Sent: Tuesday, April 11, 2023 4:56 AM    
> > >   
> > > > 
> > > > Yes, please leave it as F_CONFIG_DATA, as we're just putting some "data"
> > > > there in the end (and F_CONFIG_COOKIE might indeed be confusing for the
> > > > ccw case.)    
> > > 
> > > Since Halil didn't respond for 5+ days + Michel and you propose to continue use CONFIG_DATA and this is rare used field, I will rename 
> > >   
> > 
> > Sorry, this one has fallen through the cracks.  
> 
> Well this whole patchset is just a cleanup so it's not holding up other
> work at least. But I have to say it's difficult to make progress when
> someone comes back from outer space after more than a week of silence
> while others finished a discussion and reopens it with some new
> feedback.

Sorry, this was after 6 days. I didn't know that qualifies
as 'outer space'. As pointed out below, I was monitoring the preceding
discussion, and since the way things went was and is acceptable for
me, I didn't want to muddy the waters any further.

The issue I ended up addressing got introduced in very last email, which
pre-announced the next version.

My first intention was to explain myself, and apologize, after being
called out.

But then, also looking by looking at v13 I realized that
there might have been a slip up because F_NOTIF_CONFIG_DATA got shortened to
F_CONFIG_DATA in the discussion, which is no big deal for the discussion
itself, but may have leaked in the v13 proposal. Parav has sent out the
announced next version after about 8 hours. And if it weren't for my
hypothesis why we ended up with the proposed name vq_config_data, the
right place to discuss further would have been v13.

In hindsight, I see, replying to the v12 thread wasn't a good move.

[..]

> 
> I also feel high latency is one of the reasons people are beginning to
> ask to split into subcommitees where they won't have to deal with this
> kind of thing. 
> 

I tend to agree. 

> Let's try to keep the latency low, please.

Believe me, it is not like I'm actively trying to introduce extra
latency.

Regards,
Halil

> > For
> > the preceding ones: I do not have a strong opinion. I do
> > share Michael's and Connie's assessment regarding a possible
> > clash with CCW.

---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org