[virtio-dev] Re: [PATCH v12 00/10] Rename queue number to queue index
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 > >
[virtio-dev] Re: [PATCH v12 03/10] content: Rename confusing queue_notify_data and vqn names
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
>
> ---
> 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
[virtio-dev] Re: [PATCH v12 01/10] content: Add vq index text
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
> Signed-off-by: Parav Pandit
> ---
> 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-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [PATCH v12 03/10] content: Rename confusing queue_notify_data and vqn names
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 > > --- > 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-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [PATCH 08/11] transport-pci: Introduce virtio extended capability
On Tue, Apr 04, 2023 at 09:18:53PM +, Parav Pandit wrote:
>
>
> > From: Michael S. Tsirkin
> > Sent: Tuesday, April 4, 2023 3:35 AM
>
> > > +Virtio extended PCI Express capability structure defines the location
> > > +of certain virtio device configuration related structures using PCI
> > > +Express extended capability. Virtio extended PCI Express capability
> > > +structure uses PCI Express vendor specific extended capability
> > > +(VSEC). It has a below
> >
> > a layout below, or the following layout
> >
> Yes. somehow it got trimmed.
> Will fix it.
>
> > > +layout:
> > > +
> > > +\begin{lstlisting}
> > > +struct pcie_ext_cap {
> > > +le16 cap_vendor_id; /* Generic PCI field: 0xB */
> > > +le16 cap_version : 2; /* Generic PCI field: 0 */
> > > +le16 next_cap_offset : 14; /* Generic PCI field: next cap or
> > > +0 */ };
> > > +
> > > +struct virtio_pcie_ext_cap {
> > > +struct pcie_ext_cap pcie_ecap;
> > > +u8 cfg_type; /* Identifies the structure. */
> > > +u8 bar; /* Index of the BAR where its located */
> > > +u8 id; /* Multiple capabilities of the same type */
> > > +u8 zero_padding[1];
> > > +le64 offset; /* Offset with the bar */
> > > +le64 length; /* Length of the structure, in bytes. */
> > > +u8 data[]; /* Optional variable length data */
> >
> > Maybe le64 data[], for alignment?
> >
> It gets harder to decode (typecasting ..) if its string with le64 data type.
In what language? In C you have to cast anyway, string is char *, often
signed, not u8.
> I will extend the comment,
>
> +u8 data[]; /* Optional variable length data, must be aligned to 8
> bytes */
I'd keep it le64 or u64, it is highly unlikely we'll pass strings through
this interface anyway.
--
MST
-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] [PATCH v12 09/10] virtio-net: Avoid duplicate receive queue example
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
Signed-off-by: Parav Pandit
---
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-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] [PATCH v12 10/10] virtio-net: Describe RSS using rss rq id
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
---
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
[virtio-dev] [PATCH v12 08/10] transport-ccw: Refer to the vq by its index
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
---
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-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] [PATCH v12 07/10] transport-ccw: Rename queue depth/size to other transports
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
---
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-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] [PATCH v12 02/10] content.tex Replace virtqueue number with index
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
---
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-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] [PATCH v12 06/10] transport-mmio: Avoid referring to zero based index
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
---
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-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] [PATCH v12 05/10] transport-mmio: Rename QueueNum register
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
Reviewed-by: Jiri Pirko
Signed-off-by: Parav Pandit
---
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
[virtio-dev] [PATCH v12 04/10] transport-pci: Avoid first vq index reference
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
---
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-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] [PATCH v12 03/10] content: Rename confusing queue_notify_data and vqn names
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
---
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
[virtio-dev] [PATCH v12 01/10] content: Add vq index text
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
Signed-off-by: Parav Pandit
---
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-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] [PATCH v12 00/10] Rename queue number to queue index
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-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] RE: [PATCH 08/11] transport-pci: Introduce virtio extended capability
> From: Michael S. Tsirkin
> Sent: Tuesday, April 4, 2023 3:35 AM
> > +Virtio extended PCI Express capability structure defines the location
> > +of certain virtio device configuration related structures using PCI
> > +Express extended capability. Virtio extended PCI Express capability
> > +structure uses PCI Express vendor specific extended capability
> > +(VSEC). It has a below
>
> a layout below, or the following layout
>
Yes. somehow it got trimmed.
Will fix it.
> > +layout:
> > +
> > +\begin{lstlisting}
> > +struct pcie_ext_cap {
> > +le16 cap_vendor_id; /* Generic PCI field: 0xB */
> > +le16 cap_version : 2; /* Generic PCI field: 0 */
> > +le16 next_cap_offset : 14; /* Generic PCI field: next cap or
> > +0 */ };
> > +
> > +struct virtio_pcie_ext_cap {
> > +struct pcie_ext_cap pcie_ecap;
> > +u8 cfg_type; /* Identifies the structure. */
> > +u8 bar; /* Index of the BAR where its located */
> > +u8 id; /* Multiple capabilities of the same type */
> > +u8 zero_padding[1];
> > +le64 offset; /* Offset with the bar */
> > +le64 length; /* Length of the structure, in bytes. */
> > +u8 data[]; /* Optional variable length data */
>
> Maybe le64 data[], for alignment?
>
It gets harder to decode (typecasting ..) if its string with le64 data type.
I will extend the comment,
+u8 data[]; /* Optional variable length data, must be aligned to 8
bytes */
-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
RE: [virtio-dev] [PATCH v14] virtio-net: support the virtqueue coalescing moderation
> From: Michael S. Tsirkin
> Sent: Tuesday, April 4, 2023 4:14 PM
>
> On Tue, Apr 04, 2023 at 03:39:21PM -0400, Parav Pandit wrote:
> >
> >
> > On 4/4/2023 1:44 PM, Michael S. Tsirkin wrote:
> > > On Tue, Apr 04, 2023 at 04:32:07PM +, Parav Pandit wrote:
> > > >
> > > > > From: Halil Pasic
> > > > > Sent: Tuesday, April 4, 2023 12:29 PM
> > > > >
> > > > > On Thu, 23 Mar 2023 23:24:22 +0800 Heng Qi
> > > > > wrote:
> > > > >
> > > > > > +struct virtio_net_ctrl_coal_vq {
> > > > > > +le16 vqn;
> > > > > > +le16 reserved;
> > > > > > +struct virtio_net_ctrl_coal coal; };
> > > > > > +
> > > > > > #define VIRTIO_NET_CTRL_NOTF_COAL 6
> > > > > >#define VIRTIO_NET_CTRL_NOTF_COAL_TX_SET 0
> > > > > >#define VIRTIO_NET_CTRL_NOTF_COAL_RX_SET 1
> > > > > > + #define VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET 2 #define
> > > > > > + VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET 3
> > > > > > \end{lstlisting}
> > > > > >
> > > > > > Coalescing parameters:
> > > > > > \begin{itemize}
> > > > > > +\item \field{vqn}: The virtqueue number of an enabled
> > > > > > +transmit or receive
> > > > > virtqueue.
> > > > >
> > > > > Just to be on the safe side: VIRTIO_F_NOTIF_CONFIG_DATA has been
> > > > > negotiated, and queue_select != queue_notify_data, is vqn
> > > > > supposed to contain queue_notify_data or the number/index that
> > > > > is used for queue_select (I'm talking about the PCI transport case)?
> > > > Vqn has zero relation to notification config data feature and featue
> > > > bit.
> > > > It is the real vqn enabled via queue_select.
> > > >
> > > > Once the vqn is renamed to vq_notify_id, we won't have this confusion
> anymore.
> > >
> > > vqn here is the index. queue_select is also the index.
> > >
> > Yes to both. No plan to rename them.
> >
> > > Inside notifications-le.c we have:
> > > le32 {
> > > vqn : 16;
> > > next_off : 15;
> > > next_wrap : 1;
> > > };
> > >
> > > vqn here is queue_notify_data.
> > >
> > vqn in above unnamed structure is contain a. either vq index if
> > CONFIG_DATA is not negotiated or b. it contains queue_notifiy_data if
> > CONFIG_DATA is negotiated
> >
> > Therefore, instead of naming it as vqn, renaming it to vq_notify_id
> > crisply describe what it is for.
> >
> > And not some vqn n stands for notification, but "d" of data is dropped
> > somehow.
>
> vqnd then? for virt queue notification data?
>
"id" identifies the queue as opposed to "data" identifying a queue.
>
> > A notification identifier contains depending on negotiated feature bit.
>
> it's not necessarily an identifier. can be e.g. just 0 for all vqs.
> whatever the device needs.
For driver its just an id, content doesn't matter.
-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
Re: [virtio-dev] [PATCH v14] virtio-net: support the virtqueue coalescing moderation
On Tue, Apr 04, 2023 at 03:39:21PM -0400, Parav Pandit wrote:
>
>
> On 4/4/2023 1:44 PM, Michael S. Tsirkin wrote:
> > On Tue, Apr 04, 2023 at 04:32:07PM +, Parav Pandit wrote:
> > >
> > > > From: Halil Pasic
> > > > Sent: Tuesday, April 4, 2023 12:29 PM
> > > >
> > > > On Thu, 23 Mar 2023 23:24:22 +0800
> > > > Heng Qi wrote:
> > > >
> > > > > +struct virtio_net_ctrl_coal_vq {
> > > > > +le16 vqn;
> > > > > +le16 reserved;
> > > > > +struct virtio_net_ctrl_coal coal; };
> > > > > +
> > > > > #define VIRTIO_NET_CTRL_NOTF_COAL 6
> > > > >#define VIRTIO_NET_CTRL_NOTF_COAL_TX_SET 0
> > > > >#define VIRTIO_NET_CTRL_NOTF_COAL_RX_SET 1
> > > > > + #define VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET 2 #define
> > > > > + VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET 3
> > > > > \end{lstlisting}
> > > > >
> > > > > Coalescing parameters:
> > > > > \begin{itemize}
> > > > > +\item \field{vqn}: The virtqueue number of an enabled transmit or
> > > > > receive
> > > > virtqueue.
> > > >
> > > > Just to be on the safe side: VIRTIO_F_NOTIF_CONFIG_DATA has been
> > > > negotiated, and queue_select != queue_notify_data, is vqn supposed to
> > > > contain
> > > > queue_notify_data or the number/index that is used for queue_select (I'm
> > > > talking about the PCI transport case)?
> > > Vqn has zero relation to notification config data feature and featue bit.
> > > It is the real vqn enabled via queue_select.
> > >
> > > Once the vqn is renamed to vq_notify_id, we won't have this confusion
> > > anymore.
> >
> > vqn here is the index. queue_select is also the index.
> >
> Yes to both. No plan to rename them.
>
> > Inside notifications-le.c we have:
> > le32 {
> > vqn : 16;
> > next_off : 15;
> > next_wrap : 1;
> > };
> >
> > vqn here is queue_notify_data.
> >
> vqn in above unnamed structure is contain
> a. either vq index if CONFIG_DATA is not negotiated
> or
> b. it contains queue_notifiy_data if CONFIG_DATA is negotiated
>
> Therefore, instead of naming it as vqn, renaming it to vq_notify_id crisply
> describe what it is for.
>
> And not some vqn n stands for notification, but "d" of data is dropped
> somehow.
vqnd then? for virt queue notification data?
> A notification identifier contains depending on negotiated feature bit.
it's not necessarily an identifier. can be e.g. just 0 for all vqs.
whatever the device needs.
> >
-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
Re: [virtio-dev] [PATCH v14] virtio-net: support the virtqueue coalescing moderation
On 4/4/2023 1:44 PM, Michael S. Tsirkin wrote:
On Tue, Apr 04, 2023 at 04:32:07PM +, Parav Pandit wrote:
From: Halil Pasic
Sent: Tuesday, April 4, 2023 12:29 PM
On Thu, 23 Mar 2023 23:24:22 +0800
Heng Qi wrote:
+struct virtio_net_ctrl_coal_vq {
+le16 vqn;
+le16 reserved;
+struct virtio_net_ctrl_coal coal; };
+
#define VIRTIO_NET_CTRL_NOTF_COAL 6
#define VIRTIO_NET_CTRL_NOTF_COAL_TX_SET 0
#define VIRTIO_NET_CTRL_NOTF_COAL_RX_SET 1
+ #define VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET 2 #define
+ VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET 3
\end{lstlisting}
Coalescing parameters:
\begin{itemize}
+\item \field{vqn}: The virtqueue number of an enabled transmit or receive
virtqueue.
Just to be on the safe side: VIRTIO_F_NOTIF_CONFIG_DATA has been
negotiated, and queue_select != queue_notify_data, is vqn supposed to contain
queue_notify_data or the number/index that is used for queue_select (I'm
talking about the PCI transport case)?
Vqn has zero relation to notification config data feature and featue bit.
It is the real vqn enabled via queue_select.
Once the vqn is renamed to vq_notify_id, we won't have this confusion anymore.
vqn here is the index. queue_select is also the index.
Yes to both. No plan to rename them.
Inside notifications-le.c we have:
le32 {
vqn : 16;
next_off : 15;
next_wrap : 1;
};
vqn here is queue_notify_data.
vqn in above unnamed structure is contain
a. either vq index if CONFIG_DATA is not negotiated
or
b. it contains queue_notifiy_data if CONFIG_DATA is negotiated
Therefore, instead of naming it as vqn, renaming it to vq_notify_id
crisply describe what it is for.
And not some vqn n stands for notification, but "d" of data is dropped
somehow.
A notification identifier contains depending on negotiated feature bit.
-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [PATCH 3/3] transport-pci: Improve queue msix vector register desc
On Tue, Apr 04, 2023 at 06:14:20PM +, Parav Pandit wrote: > > > From: Michael S. Tsirkin > > Sent: Tuesday, April 4, 2023 3:21 AM > > > Ok but "receiving" is confusing here. And the verb writes seems to ask for > > direction, look at queue_desc as an example. > > > What is the confusion in receiving? > Driver some reason is configuring queue's vector even if it doesn't want to > "receive" interrupts? > Do you mean it is more verbose? I mean driver does not even receive interrupts, APIC does. > Word here "here" isn't necessary when describing the register itself, though > it exists at other places. if nothing else let's be consistent. without here it is not immediately clear weather it's the register that is written to or the value specified is written somewhere else. > > Following that example: > > > > The driver writes the MSI-X vector number used for virtqueue > > interrupts here. > > > Since a specific number is used here, article "an" is appropriate one. No because it's the specific value used for interrupts. Again, check other examples. > > would you agree? - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] RE: [PATCH 3/3] transport-pci: Improve queue msix vector register desc
> From: Michael S. Tsirkin > Sent: Tuesday, April 4, 2023 3:21 AM > Ok but "receiving" is confusing here. And the verb writes seems to ask for > direction, look at queue_desc as an example. > What is the confusion in receiving? Driver some reason is configuring queue's vector even if it doesn't want to "receive" interrupts? Do you mean it is more verbose? Word here "here" isn't necessary when describing the register itself, though it exists at other places. > Following that example: > > The driver writes the MSI-X vector number used for virtqueue > interrupts here. > Since a specific number is used here, article "an" is appropriate one. > would you agree? - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
Re: [virtio-dev] [PATCH v14] virtio-net: support the virtqueue coalescing moderation
On Tue, Apr 04, 2023 at 04:32:07PM +, Parav Pandit wrote:
>
> > From: Halil Pasic
> > Sent: Tuesday, April 4, 2023 12:29 PM
> >
> > On Thu, 23 Mar 2023 23:24:22 +0800
> > Heng Qi wrote:
> >
> > > +struct virtio_net_ctrl_coal_vq {
> > > +le16 vqn;
> > > +le16 reserved;
> > > +struct virtio_net_ctrl_coal coal; };
> > > +
> > > #define VIRTIO_NET_CTRL_NOTF_COAL 6
> > > #define VIRTIO_NET_CTRL_NOTF_COAL_TX_SET 0
> > > #define VIRTIO_NET_CTRL_NOTF_COAL_RX_SET 1
> > > + #define VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET 2 #define
> > > + VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET 3
> > > \end{lstlisting}
> > >
> > > Coalescing parameters:
> > > \begin{itemize}
> > > +\item \field{vqn}: The virtqueue number of an enabled transmit or receive
> > virtqueue.
> >
> > Just to be on the safe side: VIRTIO_F_NOTIF_CONFIG_DATA has been
> > negotiated, and queue_select != queue_notify_data, is vqn supposed to
> > contain
> > queue_notify_data or the number/index that is used for queue_select (I'm
> > talking about the PCI transport case)?
> Vqn has zero relation to notification config data feature and featue bit.
> It is the real vqn enabled via queue_select.
>
> Once the vqn is renamed to vq_notify_id, we won't have this confusion anymore.
vqn here is the index. queue_select is also the index.
Inside notifications-le.c we have:
le32 {
vqn : 16;
next_off : 15;
next_wrap : 1;
};
vqn here is queue_notify_data.
--
MST
-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [virtio-comment] Re: [virtio-dev] [PATCH v10 0/8] Rename queue index to queue number
On Tue, 4 Apr 2023 02:33:15 -0400 "Michael S. Tsirkin" wrote: > On Mon, Apr 03, 2023 at 10:57:35PM -0400, Parav Pandit wrote: > > > Both solutions are equally consistent in themselves, but I > > > argue the latter is better because: > > > * it is more consistent with historic usage > > Well index/vqn has mixed up anyway now. > > For historic reason, I agree that index is right. > > But it is too late now. > > Comments have not come on time. > > Why, is there a deadline? Is this blocking some other feature? For > something that is supposedly a cleanup we might as well get this right. > If you guys both agree index is better, and at this point it sounds > convincing, why not do it? It's more or less a machanical replacement. > It's not like there's a lot of word smithing going on here except for > 8/8 which really works with index just as well. I agree. Regards, Halil - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] RE: [PATCH v11 0/8] Rename queue index to queue number
> From: Michael S. Tsirkin > Sent: Tuesday, April 4, 2023 3:04 AM > > Please review. > > This series fixes the issue [1]. > > > > [1] https://github.com/oasis-tcs/virtio-spec/issues/163 > > Could you knock out a version based on Halil's suggestion of using index > throughout? Great. Finally, there is an agreement in choosing a word. :) Will send shortly after interrupt coalescing patches merged to github. Don't see it there yet. - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
RE: [virtio-dev] [PATCH v14] virtio-net: support the virtqueue coalescing moderation
> From: Halil Pasic
> Sent: Tuesday, April 4, 2023 12:29 PM
>
> On Thu, 23 Mar 2023 23:24:22 +0800
> Heng Qi wrote:
>
> > +struct virtio_net_ctrl_coal_vq {
> > +le16 vqn;
> > +le16 reserved;
> > +struct virtio_net_ctrl_coal coal; };
> > +
> > #define VIRTIO_NET_CTRL_NOTF_COAL 6
> > #define VIRTIO_NET_CTRL_NOTF_COAL_TX_SET 0
> > #define VIRTIO_NET_CTRL_NOTF_COAL_RX_SET 1
> > + #define VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET 2 #define
> > + VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET 3
> > \end{lstlisting}
> >
> > Coalescing parameters:
> > \begin{itemize}
> > +\item \field{vqn}: The virtqueue number of an enabled transmit or receive
> virtqueue.
>
> Just to be on the safe side: VIRTIO_F_NOTIF_CONFIG_DATA has been
> negotiated, and queue_select != queue_notify_data, is vqn supposed to contain
> queue_notify_data or the number/index that is used for queue_select (I'm
> talking about the PCI transport case)?
Vqn has zero relation to notification config data feature and featue bit.
It is the real vqn enabled via queue_select.
Once the vqn is renamed to vq_notify_id, we won't have this confusion anymore.
-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
Re: [virtio-dev] [PATCH v14] virtio-net: support the virtqueue coalescing moderation
On Thu, 23 Mar 2023 23:24:22 +0800
Heng Qi wrote:
> +struct virtio_net_ctrl_coal_vq {
> +le16 vqn;
> +le16 reserved;
> +struct virtio_net_ctrl_coal coal;
> +};
> +
> #define VIRTIO_NET_CTRL_NOTF_COAL 6
> #define VIRTIO_NET_CTRL_NOTF_COAL_TX_SET 0
> #define VIRTIO_NET_CTRL_NOTF_COAL_RX_SET 1
> + #define VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET 2
> + #define VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET 3
> \end{lstlisting}
>
> Coalescing parameters:
> \begin{itemize}
> +\item \field{vqn}: The virtqueue number of an enabled transmit or receive
> virtqueue.
Just to be on the safe side: VIRTIO_F_NOTIF_CONFIG_DATA has been
negotiated, and queue_select != queue_notify_data, is vqn supposed
to contain queue_notify_data or the number/index that is used for
queue_select (I'm talking about the PCI transport case)?
Regards,
Halil
-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] RE: [virtio-comment] RE: [PATCH v10 8/8] virtio-net: Describe RSS using rss rq id
> From: Cornelia Huck > Sent: Tuesday, April 4, 2023 4:15 AM > > In this field and other fields, we just refer to the receive virtqueues as > rss_rq_id parent structure itself is describing what it is. > > Hence, we omit re-iterating rss_rq_id at multiple places. > > Personally, I'd prefer to spell it out, as it is a specific way to refer to a > virtqueue. That specific way is already covered without making it more verbose. So I will keep it as is, unless you have strong objection to it. - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [PATCH 07/11] transport-pci: Introduce transitional MMR device id
On 4/4/2023 3:28 AM, Michael S. Tsirkin wrote: On Fri, Mar 31, 2023 at 01:58:30AM +0300, Parav Pandit wrote: Transitional MMR device PCI Device IDs are unique. Hence, any of the existing drivers do not bind to it. This further maintains the backward compatibility with existing drivers. Co-developed-by: Satananda Burla Signed-off-by: Parav Pandit I took a fresh look at it, and I don't get it: what exactly is wrong with just using modern ID? Why do we need new ones? A modern (non transitional device) do not support legacy functionality such as virtio net hdr, device reset sequence. - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [virtio-comment] Re: [virtio-dev] [PATCH v10 0/8] Rename queue index to queue number
On Tue, Apr 04 2023, Halil Pasic wrote: > On Tue, 4 Apr 2023 03:07:26 -0400 > "Michael S. Tsirkin" wrote: > >> > 3. Once interrupt moderation series is merged, rename vqn to vq_index. >> > (really vqn reads better even though history say vq index). >> > Because votes is completed and voting period ended for the important >> > feature. >> >> interrupt moderation voting is still ongoing, isn't it? > > Yes it is. But not for very long. Close date is today. I > won't change my vote because of the index vs number discussion. I believe > we can include that patch in the cleanup if push comes to shove. But > I wouldn't have anything against that ballot getting withdrawn either. > I'm Cc-ing Heng Qi. ...it has actually closed right now. But I don't really see including it as a show stopper. - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
Re: [virtio-dev] [PATCH v10 0/8] Rename queue index to queue number
On Tue, 4 Apr 2023 03:07:26 -0400 "Michael S. Tsirkin" wrote: > > 3. Once interrupt moderation series is merged, rename vqn to vq_index. > > (really vqn reads better even though history say vq index). > > Because votes is completed and voting period ended for the important > > feature. > > interrupt moderation voting is still ongoing, isn't it? Yes it is. But not for very long. Close date is today. I won't change my vote because of the index vs number discussion. I believe we can include that patch in the cleanup if push comes to shove. But I wouldn't have anything against that ballot getting withdrawn either. I'm Cc-ing Heng Qi. Regards, Halil - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [PATCH 08/11] transport-pci: Introduce virtio extended capability
On Tue, Apr 04, 2023 at 03:19:53PM +0200, Cornelia Huck wrote: > >> > What does it "Co-developed-by" mean exactly that Signed-off-by > >> > does not? > >> > >> AIUI, "Co-developed-by:" is more akin to a second "Author:", i.e. to > >> give attribution. (Linux kernel rules actually state that any > >> "Co-developed-by:" must be followed by a "Signed-off-by:" for that > >> author, but we don't really do DCO here, the S-o-b is more of a > >> convention.) > > > > > > Actually, we might want to generally, to signify agreement to the IPR. > > How about adding this to our rules? But in this case Satananda Burla is > > a TC member so yes, no problem. > > Adding a s-o-b requirement is not a bad idea... do you want to propose > an update? OK how about the following: --- The process for forwarding comments from others: Generally, subscribing to the virtio comments mailing list requires agreement to the OASIS feedback license, at https://www.oasis-open.org/who/ipr/feedback_license.pdf When forwarding (in modified or unmodified form) comments from others, please make sure the commenter has read and agreed to the feedback license. If this is the case, please include the following: License-consent: commenter's name where commenter's name is a valid name-addr as specified in RFC 5322, see https://datatracker.ietf.org/doc/html/rfc5322#section-3.4 If the comment is on behalf of an organization, please use the organization's email address. If forwarding a comment from a TC member, please instead get consent to the full Virtio TC IPR, and then, as before, include License-consent: commenter's name If you are reusing a comment that has already been posted to the TC mailing list, the above tags are not required. --- We could reuse Signed-off-by though I am a bit concerned whether people will assume it's a DCO thing which everyone copies. Thoughts? -- MST - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [virtio-comment] [PATCH 00/11] Introduce transitional mmr pci device
On Mon, Apr 03, 2023 at 08:27:28PM +, Parav Pandit wrote: > > > From: Stefan Hajnoczi > > Sent: Monday, April 3, 2023 3:10 PM > > > > Maybe call it "Memory-mapped Transitional"? That name would be easier to > > understand. > > > Sounds fine to me. > > > > > Modern devices were added to Linux in 2014 and support SR-IOV. > > > > > > > Why is it > > > > important to support Transitional (which really means Legacy > > > > devices, otherwise Modern devices would be sufficient)? > > > > > > > To support guest VMs which only understand legacy devices and > > > unfortunately they are still in much wider use by the users. > > > > I wonder which guest software without Modern VIRTIO support will still be > > supported by the time Transitional MMR software and hardware becomes > > available. Are you aiming for particular guest software versions? > > Transitional MMR hardware is available almost now. > Transitional MMR software is also WIP to be available as soon as we ratify > the spec via tvq or via mmr or both. > This will be support the guest sw version such as 2.6.32.754 kernel. That is a RHEL 6 kernel. RHEL 6 entered Extended Life Cycle Support on November 30, 2020 (https://access.redhat.com/articles/4665701). I would use existing software emulation for really old guests, but if you see an opportunity for hardware passthrough, then why not. Stefan signature.asc Description: PGP signature
[virtio-dev] Re: [PATCH 08/11] transport-pci: Introduce virtio extended capability
On Tue, Apr 04 2023, "Michael S. Tsirkin" wrote: > On Tue, Apr 04, 2023 at 09:54:55AM +0200, Cornelia Huck wrote: >> On Tue, Apr 04 2023, "Michael S. Tsirkin" wrote: >> >> > On Fri, Mar 31, 2023 at 01:58:31AM +0300, Parav Pandit wrote: >> >> PCI device configuration space for capabilities is limited to only 192 >> >> bytes shared by many PCI capabilities of generic PCI device and virtio >> >> specific. >> >> >> >> Hence, introduce virtio extended capability that uses PCI Express >> >> extended capability. >> >> Subsequent patch uses this virtio extended capability. >> >> >> >> Co-developed-by: Satananda Burla >> > >> > What does it "Co-developed-by" mean exactly that Signed-off-by >> > does not? >> >> AIUI, "Co-developed-by:" is more akin to a second "Author:", i.e. to >> give attribution. (Linux kernel rules actually state that any >> "Co-developed-by:" must be followed by a "Signed-off-by:" for that >> author, but we don't really do DCO here, the S-o-b is more of a >> convention.) > > > Actually, we might want to generally, to signify agreement to the IPR. > How about adding this to our rules? But in this case Satananda Burla is > a TC member so yes, no problem. Adding a s-o-b requirement is not a bad idea... do you want to propose an update? - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [PATCH v11 09/10] admin: conformance clauses
On Mon, Apr 03, 2023 at 11:03:56AM -0400, Michael S. Tsirkin wrote: > Add conformance clauses for admin commands and admin virtqueues. > > Signed-off-by: Michael S. Tsirkin > > --- > typo and wording fixes reported by David > clarify cmd list use can be repeated but not in parallel with other > commands, and returning same value across uses - comment by Lingshan > add conformance clauses for new error codes > --- > admin.tex | 227 ++ > 1 file changed, 227 insertions(+) Reviewed-by: Stefan Hajnoczi signature.asc Description: PGP signature
[virtio-dev] Re: [PATCH v11 08/10] admin: command list discovery
On Mon, Apr 03, 2023 at 11:03:49AM -0400, Michael S. Tsirkin wrote: > Add commands to find out which commands does each group support, > as well as enable their use by driver. > This will be especially useful once we have multiple group types. > > An alternative is per-type VQs. This is possible but will > require more per-transport work. Discovery through the vq > helps keep things contained. > > e.g. lack of support for some command can switch to a legacy mode > > note that commands are expected to be avolved by adding new > fields to command specific data at the tail, so > we generally do not need feature bits for compatibility. > > Signed-off-by: Michael S. Tsirkin > > --- > dropped S.O.B by Max > explain in commit log how commands will evolve, comment by Stefan > replace will use with is capable of use, comment by Stefan > typo fix reported by David and Lingshan > rename cmds to cmd_opcodes suggested by Jiri > list group_type explicitly in command description suggested by Jiri > clarify how can device not support some commands > always say group administration commands, comment by Jiri > --- > admin.tex | 102 +- > 1 file changed, 101 insertions(+), 1 deletion(-) Reviewed-by: Stefan Hajnoczi signature.asc Description: PGP signature
[virtio-dev] Re: [PATCH v11 05/10] pci: add admin vq registers to virtio over pci
On Mon, Apr 03, 2023 at 11:03:29AM -0400, Michael S. Tsirkin wrote:
> @@ -1033,6 +1037,19 @@ \subsubsection{Common configuration structure
> layout}\label{sec:Virtio Transport
> This field exists only if VIRTIO_F_RING_RESET has been
> negotiated. (see \ref{sec:Basic Facilities of a Virtio Device /
> Virtqueues / Virtqueue Reset}).
>
> +\item[\field{admin_queue_index}]
> +The device uses this to report the index of the first administration
> virtqueue.
> +This field is valid only if VIRTIO_F_ADMIN_VQ has been negotiated.
> +\item[\field{admin_queue_num}]
> + The device uses this to report the number of the
> + supported administration virtqueues.
> + Virtqueues with index
> + between \field{admin_queue_index} and (\field{admin_queue_index} +
> + \field{admin_queue_num} - 1) inclusive serve as administration
> + virtqueues.
> + The value 0 indicates no supported administration virtqueues.
> + This field is valid only if VIRTIO_F_ADMIN_VQ has been
> + negotiated.
> \end{description}
Maybe add a device-normative statement that [admin_queue_index,
admin_queue_index + admin_queue_num) must be located after
device-specific virtqueues?
That would remind implementers that the device-specific virtqueue layout
needs to be followed and cannot be modified by the presence of
Administration Virtqueues.
> \devicenormative{\paragraph}{Common configuration structure layout}{Virtio
> Transport Options / Virtio Over PCI Bus / PCI Device Layout / Common
> configuration structure layout}
> @@ -1119,6 +1136,14 @@ \subsubsection{Common configuration structure
> layout}\label{sec:Virtio Transport
> were used before the queue reset.
> (see \ref{sec:Basic Facilities of a Virtio Device / Virtqueues / Virtqueue
> Reset}).
>
> +If VIRTIO_F_ADMIN_VQ has been negotiated, and if the driver
> +configures any administration virtqueues, the driver MUST
> +configure the administration virtqueues using the index
> +in the range \field{admin_queue_index} to
> +\field{admin_queue_index} + \field{admin_queue_num} - 1 inclusive.
> +The driver MAY configure less administration virtqueues than
s/less/fewer/
signature.asc
Description: PGP signature
[virtio-dev] Re: [PATCH 08/11] transport-pci: Introduce virtio extended capability
On Tue, Apr 04, 2023 at 09:54:55AM +0200, Cornelia Huck wrote: > On Tue, Apr 04 2023, "Michael S. Tsirkin" wrote: > > > On Fri, Mar 31, 2023 at 01:58:31AM +0300, Parav Pandit wrote: > >> PCI device configuration space for capabilities is limited to only 192 > >> bytes shared by many PCI capabilities of generic PCI device and virtio > >> specific. > >> > >> Hence, introduce virtio extended capability that uses PCI Express > >> extended capability. > >> Subsequent patch uses this virtio extended capability. > >> > >> Co-developed-by: Satananda Burla > > > > What does it "Co-developed-by" mean exactly that Signed-off-by > > does not? > > AIUI, "Co-developed-by:" is more akin to a second "Author:", i.e. to > give attribution. (Linux kernel rules actually state that any > "Co-developed-by:" must be followed by a "Signed-off-by:" for that > author, but we don't really do DCO here, the S-o-b is more of a > convention.) Actually, we might want to generally, to signify agreement to the IPR. How about adding this to our rules? But in this case Satananda Burla is a TC member so yes, no problem. > > > > > >> Signed-off-by: Parav Pandit - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [PATCH v11 04/10] admin: introduce virtio admin virtqueues
On Mon, Apr 03, 2023 at 11:03:23AM -0400, Michael S. Tsirkin wrote: > The admin virtqueues will be the first interface used to issue admin commands. > > Currently the virtio specification defines control virtqueue to manipulate > features and configuration of the device it operates on: > virtio-net, virtio-scsi, etc all have existing control virtqueues. However, > control virtqueue commands are device type specific, which makes it very > difficult to extend for device agnostic commands. > > Keeping the device-specific virtqueue separate from the admin virtqueue > is simpler and has fewer potential problems. I don't think creating > common infrastructure for device-specific control virtqueues across > device types worthwhile or within the scope of this patch series. > > To support this requirement in a more generic way, this patch introduces > a new admin virtqueue interface. > The admin virtqueue can be seen as the virtqueue analog to a transport. > The admin queue thus does nothing device type-specific (net, scsi, etc) > and instead focuses on transporting the admin commands. > > We also support more than one admin virtqueue, for QoS and > scalability requirements. > > Signed-off-by: Michael S. Tsirkin > > --- > explain ordering of commands as suggested by Stefan > dropped Max's S.O.B > reword commit log as suggested by David > minor wording fixes suggested by David > --- > admin.tex | 75 + > content.tex | 7 +++-- > 2 files changed, 80 insertions(+), 2 deletions(-) Reviewed-by: Stefan Hajnoczi signature.asc Description: PGP signature
[virtio-dev] Re: [PATCH v11 03/10] admin: introduce group administration commands
On Mon, Apr 03, 2023 at 11:03:16AM -0400, Michael S. Tsirkin wrote: > This introduces a general structure for group administration commands, > used to control device groups through their owner. > > Following patches will introduce specific commands and an interface for > submitting these commands to the owner. > > Note that the commands are focused on controlling device groups: > this is why group related fields are in the generic part of > the structure. > Without this the admin vq would become a "whatever" vq which does not do > anything specific at all, just a general transport like thing. > I feel going this way opens the design space to the point where > we no longer know what belongs in e.g. config space > what in the control q and what in the admin q. > As it is, whatever deals with groups is in the admin q; other > things not in the admin q. > > There are specific exceptions such as query but that's an exception that > proves the rule ;) > > Signed-off-by: Michael S. Tsirkin > --- > > changes since v10: > explain the role of status vs status_qualifier, addresses > multiple comments > add more status values and appropriate qualifiers, > as suggested by Parav > clarify reserved command opcodes as suggested by Stefan > better formatting for ranges, comment by Jiri > make sure command-specific-data is a multiple of qword, > so things are aligned, suggested by Jiri > add Q_OK qualifier for success > --- > admin.tex| 121 +++ > introduction.tex | 3 ++ > 2 files changed, 124 insertions(+) My thoughts on status/status_qualifier haven't changed, but it's a question of taste: Reviewed-by: Stefan Hajnoczi signature.asc Description: PGP signature
[virtio-dev] Re: [PATCH v11 02/10] admin: introduce device group and related concepts
On Mon, Apr 03, 2023 at 11:03:09AM -0400, Michael S. Tsirkin wrote: > Each device group has a type. For now, define one initial group type: > > SR-IOV type - PCI SR-IOV virtual functions (VFs) of a given > PCI SR-IOV physical function (PF). This group may contain zero or more > virtio devices according to NumVFs configured. > > Each device within a group has a unique identifier. This identifier > is the group member identifier. > > Note: one can argue both ways whether the new device group handling > functionality (this and following patches) is closer > to a new device type or a new transport type. > > However, I expect that we will add more features in the near future. To > facilitate this as much as possible of the text is located in the new > admin chapter. > > I did my best to minimize transport-specific text. > > There's a bit of duplication with 0x1 repeated twice and > no special section for group type identifiers. > I think it's ok to defer adding these until we have more group > types. > > Signed-off-by: Michael S. Tsirkin > --- > changes in v11: > dropped Max's S.O.B. > dropped an unmatched ) as reported by Parav > document that member id is 1 to numvfs > document that vf enable must be set (will also be reflected in > the conformance section) > document that we deferred generalizing text as requested by Parav - > I think we can do it later > minor wording fixes to address comments by David > add concepts of owner and member driver. unused currently > but will come in handy later, as suggested by Stefan > --- > admin.tex | 63 + > content.tex | 2 ++ > 2 files changed, 65 insertions(+) > create mode 100644 admin.tex Reviewed-by: Stefan Hajnoczi signature.asc Description: PGP signature
[virtio-dev] RE: [virtio-comment] [PATCH 0/3] transport-pci: msix summary update
On Mon, Apr 03 2023, Parav Pandit wrote: > Hi Cornelia, > >> From: Xuan Zhuo >> Sent: Monday, March 27, 2023 2:09 AM >> Series: >> >> Reviewed-by: Xuan Zhuo >> > > It is editorial change rewording the text. > Can you please take this short series forward? Patch 1 (which Michael has already pushed) is editorial; for the other two patches, I'm not that sure, especially as there still seems to be some discussion? - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [virtio-comment] RE: [PATCH v10 8/8] virtio-net: Describe RSS using rss rq id
On Mon, Apr 03 2023, Parav Pandit wrote:
>> From: Cornelia Huck
>> Sent: Thursday, March 30, 2023 5:17 AM
>> > +Field \field{indirection_table} is an array of receive virtqueues.
>>
>> "an array of receive virtqueues identified via their rss_rq_id" ?
> No need to make it this verbose. It is evident from the definition itself.
>
>>
>> >
>> > 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.
>>
>> "only with rss_rq_id references to enabled receive virtqueues" ?
>>
> In this field and other fields, we just refer to the receive virtqueues as
> rss_rq_id parent structure itself is describing what it is.
> Hence, we omit re-iterating rss_rq_id at multiple places.
Personally, I'd prefer to spell it out, as it is a specific way to refer
to a virtqueue.
-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [PATCH v11 00/10] Introduce device group and device management
On Mon, Apr 03 2023, "Michael S. Tsirkin" wrote: > On Mon, Apr 03, 2023 at 06:39:12PM +0200, Cornelia Huck wrote: >> On Mon, Apr 03 2023, "Michael S. Tsirkin" wrote: >> >> > Change log: >> > >> > since v10: >> >addressed lots of comments by Jiri, Stefan. Cornelia, Lngshan, Parav, >> > Max >> >Cornelia, do you want to pick up 10/10 separately? >> >> I'm not sure that makes sense -- if we refactor to introduce a patch >> that deals with all of the currently unimplemented features (and respin >> this series with the patch that lists the admin vq as unsupported on >> top) we could do that, but I don't think it's worth the effort. > > My concern is that people are already kind of familiar with the > patchset, reshuffling for no end result will cause everyone to re-read. Yeah, let's just have 10/10 walk along with the rest. - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [PATCH 08/11] transport-pci: Introduce virtio extended capability
On Tue, Apr 04 2023, "Michael S. Tsirkin" wrote: > On Fri, Mar 31, 2023 at 01:58:31AM +0300, Parav Pandit wrote: >> PCI device configuration space for capabilities is limited to only 192 >> bytes shared by many PCI capabilities of generic PCI device and virtio >> specific. >> >> Hence, introduce virtio extended capability that uses PCI Express >> extended capability. >> Subsequent patch uses this virtio extended capability. >> >> Co-developed-by: Satananda Burla > > What does it "Co-developed-by" mean exactly that Signed-off-by > does not? AIUI, "Co-developed-by:" is more akin to a second "Author:", i.e. to give attribution. (Linux kernel rules actually state that any "Co-developed-by:" must be followed by a "Signed-off-by:" for that author, but we don't really do DCO here, the S-o-b is more of a convention.) > > >> Signed-off-by: Parav Pandit - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [PATCH 08/11] transport-pci: Introduce virtio extended capability
On Fri, Mar 31, 2023 at 01:58:31AM +0300, Parav Pandit wrote:
> PCI device configuration space for capabilities is limited to only 192
> bytes shared by many PCI capabilities of generic PCI device and virtio
> specific.
>
> Hence, introduce virtio extended capability that uses PCI Express
> extended capability.
> Subsequent patch uses this virtio extended capability.
>
> Co-developed-by: Satananda Burla
What does it "Co-developed-by" mean exactly that Signed-off-by
does not?
> Signed-off-by: Parav Pandit
> ---
> transport-pci.tex | 69 ++-
> 1 file changed, 68 insertions(+), 1 deletion(-)
>
> diff --git a/transport-pci.tex b/transport-pci.tex
> index 665448e..aeda4a1 100644
> --- a/transport-pci.tex
> +++ b/transport-pci.tex
> @@ -174,7 +174,8 @@ \subsection{Virtio Structure PCI
> Capabilities}\label{sec:Virtio Transport Option
> the function, or accessed via the special VIRTIO_PCI_CAP_PCI_CFG field in
> the PCI configuration space.
>
> The location of each structure is specified using a vendor-specific PCI
> capability located
> -on the capability list in PCI configuration space of the device.
> +on the capability list in PCI configuration space of the device
> +unless stated otherwise.
> This virtio structure capability uses little-endian format; all fields are
> read-only for the driver unless stated otherwise:
>
> @@ -301,6 +302,72 @@ \subsection{Virtio Structure PCI
> Capabilities}\label{sec:Virtio Transport Option
> fields provide the most significant 32 bits of a total 64 bit offset and
> length within the BAR specified by \field{cap.bar}.
>
> +Virtio extended PCI Express capability structure defines
> +the location of certain virtio device configuration related
> +structures using PCI Express extended capability. Virtio
> +extended PCI Express capability structure uses PCI Express
> +vendor specific extended capability (VSEC). It has a below
a layout below, or the following layout
> +layout:
> +
> +\begin{lstlisting}
> +struct pcie_ext_cap {
> +le16 cap_vendor_id; /* Generic PCI field: 0xB */
> +le16 cap_version : 2; /* Generic PCI field: 0 */
> +le16 next_cap_offset : 14; /* Generic PCI field: next cap or 0 */
> +};
> +
> +struct virtio_pcie_ext_cap {
> +struct pcie_ext_cap pcie_ecap;
> +u8 cfg_type; /* Identifies the structure. */
> +u8 bar; /* Index of the BAR where its located */
> +u8 id; /* Multiple capabilities of the same type */
> +u8 zero_padding[1];
> +le64 offset; /* Offset with the bar */
> +le64 length; /* Length of the structure, in bytes. */
> +u8 data[]; /* Optional variable length data */
Maybe le64 data[], for alignment?
> +};
> +\end{lstlisting}
> +
> +This structure contains optional data, depending on
> +\field{cfg_type}. The fields are interpreted as follows:
> +
> +\begin{description}
> +\item[\field{cap_vendor_id}]
> + 0x0B; identifies a vendor-specific extended capability.
> +
> +\item[\field{cap_version}]
> + contains a value of 0.
> +
> +\item[\field{next_cap_offset}]
> +Offset to the next capability.
> +
> +\item[\field{cfg_type}]
> +follows the same definition as \field{cfg_type}
> +from the \field{struct virtio_pci_cap}.
> +
> +\item[\field{bar}]
> +follows the same same definition as \field{bar}
> +from the \field{struct virtio_pci_cap}.
> +
> +\item[\field{id}]
> +follows the same same definition as \field{id}
> +from the \field{struct virtio_pci_cap}.
> +
> +\item[\field{offset}]
> +indicates where the structure begins relative to the
> +base address associated with the BAR. The alignment
> +requirements of offset are indicated in each
> +structure-specific section that uses
> +\field{struct virtio_pcie_ext_cap}.
> +
> +\item[\field{length}]
> +indicates the length of the structure indicated by this
> +capability.
> +
> +\item[\field{data}]
> +optional data of this capability.
> +\end{description}
> +
> \drivernormative{\subsubsection}{Virtio Structure PCI Capabilities}{Virtio
> Transport Options / Virtio Over PCI Bus / Virtio Structure PCI Capabilities}
>
> The driver MUST ignore any vendor-specific capability structure which has
> --
> 2.26.2
-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [PATCH 07/11] transport-pci: Introduce transitional MMR device id
On Fri, Mar 31, 2023 at 01:58:30AM +0300, Parav Pandit wrote:
> Transitional MMR device PCI Device IDs are unique. Hence,
> any of the existing drivers do not bind to it.
> This further maintains the backward compatibility with
> existing drivers.
>
> Co-developed-by: Satananda Burla
> Signed-off-by: Parav Pandit
I took a fresh look at it, and I don't get it: what exactly is wrong
with just using modern ID? Why do we need new ones?
> ---
> transport-pci.tex | 45 +
> 1 file changed, 41 insertions(+), 4 deletions(-)
>
> diff --git a/transport-pci.tex b/transport-pci.tex
> index ee11ba5..665448e 100644
> --- a/transport-pci.tex
> +++ b/transport-pci.tex
> @@ -19,12 +19,14 @@ \section{Virtio Over PCI Bus}\label{sec:Virtio Transport
> Options / Virtio Over P
> \subsection{PCI Device Discovery}\label{sec:Virtio Transport Options /
> Virtio Over PCI Bus / PCI Device Discovery}
>
> Any PCI device with PCI Vendor ID 0x1af4, and PCI Device ID 0x1000 through
> -0x107f inclusive is a virtio device. The actual value within this range
> -indicates which virtio device is supported by the device.
> +0x107f inclusive and DeviceID 0x10f9 through 0x10ff is a virtio device.
> +The actual value within this range indicates which virtio device
> +type it is.
> The PCI Device ID is calculated by adding 0x1040 to the Virtio Device ID,
> as indicated in section \ref{sec:Device Types}.
> -Additionally, devices MAY utilize a Transitional PCI Device ID range,
> -0x1000 to 0x103f depending on the device type.
> +Additionally, devices MAY utilize a Transitional PCI Device ID range
> +0x1000 to 0x103f inclusive or a Transitional MMR PCI Device ID range
> +0x10f9 to 0x10ff inclusive, depending on the device type.
>
> \devicenormative{\subsubsection}{PCI Device Discovery}{Virtio Transport
> Options / Virtio Over PCI Bus / PCI Device Discovery}
>
> @@ -95,6 +97,41 @@ \subsubsection{Legacy Interfaces: A Note on PCI Device
> Discovery}\label{sec:Virt
>
> This is to match legacy drivers.
>
> +\subsubsection{Transitional MMR Interface: A Note on PCI Device
> +Discovery}\label{sec:Virtio Transport Options / Virtio Over PCI
> +Bus / PCI Device Discovery / Transitional MMR Interface: A Note on PCI
> Device Discovery}
> +
> +The transitional MMR device has one of the following PCI Device ID
> +depending on the device type:
> +
> +\begin{tabular}{|l|c|}
> +\hline
> +Transitional PCI Device ID & Virtio Device\\
> +\hline \hline
> +0x10f9 & network device \\
> +\hline
> +0x10fa & block device \\
> +\hline
> +0x10fb & memory ballooning (traditional) \\
> +\hline
> +0x10fc & console \\
> +\hline
> +0x10fd & SCSI host \\
> +\hline
> +0x10fe & entropy source\\
> +\hline
> +0x10ff & 9P transport \\
> +\hline
> +\end{tabular}
> +
> +The PCI Subsystem Vendor ID and the PCI Subsystem Device ID MAY
> +reflect the PCI Vendor and Device ID of the environment.
> +
> +The transitional MMR driver MUST match any PCI Revision ID value.
> +
> +The transitional MMR driver MAY match any PCI Subsystem Vendor ID and
> +any PCI Subsystem Device ID value.
> +
> \subsection{PCI Device Layout}\label{sec:Virtio Transport Options / Virtio
> Over PCI Bus / PCI Device Layout}
>
> The device is configured via I/O and/or memory regions (though see
> --
> 2.26.2
-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [PATCH 1/3] transport-pci: Remove empty line at end of file
On Fri, Mar 24, 2023 at 04:35:32AM +0300, Parav Pandit wrote:
> Remove empty line at end of file.
>
> Signed-off-by: Parav Pandit
Applied this one.
> ---
> transport-pci.tex | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/transport-pci.tex b/transport-pci.tex
> index 044c085..6e0cb45 100644
> --- a/transport-pci.tex
> +++ b/transport-pci.tex
> @@ -1148,4 +1148,3 @@ \subsubsection{Driver Handling
> Interrupts}\label{sec:Virtio Transport Options /
> re-examine the configuration space to see what changed.
> \end{itemize}
> \end{itemize}
> -
> --
> 2.26.2
-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [PATCH 3/3] transport-pci: Improve queue msix vector register desc
On Fri, Mar 24, 2023 at 04:35:34AM +0300, Parav Pandit wrote:
> queue_msix_vector register is for receiving interrupts from the device
> for the virtqueue.
>
> "for MSI-X" is confusing term.
>
> Also it is the register that driver "writes" to, similar to
> many other registers such as queue_desc, queue_driver etc.
>
> Hence, replace the verb from use to write.
>
> Signed-off-by: Parav Pandit
> Reviewed-by: Max Gurtovoy
> ---
> transport-pci.tex | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/transport-pci.tex b/transport-pci.tex
> index 1bc89b4..9d492d5 100644
> --- a/transport-pci.tex
> +++ b/transport-pci.tex
> @@ -367,7 +367,8 @@ \subsubsection{Common configuration structure
> layout}\label{sec:Virtio Transport
> A 0 means the queue is unavailable.
>
> \item[\field{queue_msix_vector}]
> -The driver uses this to specify the queue vector for MSI-X.
> +The driver writes an MSI-X vector number for receiving
> +virtqueue interrupts.
Ok but "receiving" is confusing here. And the verb writes seems
to ask for direction, look at queue_desc as an example.
Following that example:
The driver writes the MSI-X vector number used for virtqueue interrupts
here.
would you agree?
> \item[\field{queue_enable}]
> The driver uses this to selectively prevent the device from
> executing requests from this virtqueue.
> --
> 2.26.2
-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
Re: [virtio-dev] [PATCH v10 0/8] Rename queue index to queue number
On Mon, Apr 03, 2023 at 10:57:35PM -0400, Parav Pandit wrote: > I see few options. > > Option_1: > 1. I will send v12 of this series to use index all the places instead of > number. > 2. driver notification structure vqn to rename to vq_notify_id. > (after merging this series as follow up cleanup). We can pretend that vqn means "VQ Notification". Backronims are fun. In fact it is not always a vq index, sometimes it's a different value. So maybe keeping vqn there makes sense. > 3. Once interrupt moderation series is merged, rename vqn to vq_index. > (really vqn reads better even though history say vq index). > Because votes is completed and voting period ended for the important > feature. It's ongoing but sure, we can do a fix up on top. > 4. Michael send v12 with index. Of AQ? Sure. > Option_2: > 1. Continue with the v11 of this series as vq number. > 2. Continue with interrupt moderation series voted > 3. Continue with Michael's work of AQ > 4. Will do #2 of option_1. > > Please decide at earliest. > It looks like the sentiment is now going more option 1. If you like post v12 and we'll see which gets more acks. -- MST - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
Re: [virtio-dev] [PATCH v10 0/8] Rename queue index to queue number
On Mon, Apr 03, 2023 at 10:57:35PM -0400, Parav Pandit wrote: > 3. Once interrupt moderation series is merged, rename vqn to vq_index. > (really vqn reads better even though history say vq index). > Because votes is completed and voting period ended for the important > feature. interrupt moderation voting is still ongoing, isn't it? -- MST - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [PATCH v11 0/8] Rename queue index to queue number
On Tue, Apr 04, 2023 at 01:50:44AM +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 Number. > > Solution: > a. Use virtqueue number description, and rename MMIO register as QueueSize. > b. Replace virtqueue index with virtqueue number > c. RSS area of virtio net has inherited some logic, describe it > using abstract rss_rq_id. > > Patch summary: > patch-1 introduce vq number as generic term > patch-2 renames index to number for pci transport > patch-3 renames mmio register from Num to Size > patch-4 renames index to number for mmio transport > patch-5 renames num field to size for ccw transport > patch-6 renames index field to vqn for ccw transport > patch-7 for virtio-net removes duplicate example from requirements > patch-8 for virtio-net updates rss description to use vq number > > 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 Could you knock out a version based on Halil's suggestion of using index throughout? > --- > changelog: > 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 (8): > content: Add vq number text > transport-pci: Refer to the vq by its number > transport-mmio: Rename QueueNum register > transport-mmio: Refer to the vq by its number > transport-ccw: Rename queue depth/size to other transports > transport-ccw: Refer to the vq by its number > virtio-net: Avoid duplicate receive queue example > virtio-net: Describe RSS using rss rq id > > content.tex | 4 ++ > device-types/net/description.tex | 29 ++ > transport-ccw.tex| 26 +++-- > transport-mmio.tex | 65 ++-- > transport-pci.tex| 13 --- > 5 files changed, 84 insertions(+), 53 deletions(-) > > -- > 2.26.2 - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
Re: [virtio-dev] [PATCH v10 0/8] Rename queue index to queue number
On Fri, Mar 31, 2023 at 10:13:51AM +0200, Cornelia Huck wrote:
> On Thu, Mar 30 2023, Halil Pasic wrote:
>
> > On Thu, 30 Mar 2023 00:23:33 +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 Number.
> >>
> >> Solution:
> >> a. Use virtqueue number description, and rename MMIO register as QueueSize.
> >
> > I'm in favor of replacing number with size where appropriate.
> >
> >> b. Replace virtqueue index with virtqueue number
> >
> > I don't see the benefit of replacing virtqueue index with virtqueue
> > number.
> >
> > Currently virtqueue number is only used in the parts that describe
> > notifications (Guest->Host), the rest of the spec uses virtqueue index.
> >
> > I argue that using a different term in that context than in the rest
> > of the specification makes sense, because in the context of notifications
> > the virtqueue isn't always identified by its index.
> >
> > More precisely: if VIRTIO_F_NOTIF_CONFIG_DATA has been negotiated in the
> > context of notifications the virtqueue is identified by the
> > so called "queue_notify_data"; if VIRTIO_F_NOTIF_CONFIG_DATA has been
> > negotiated in the context of notifications the virtqueue is identified by
> > the virtqueue index (as usual, for example in queue_select, or in
> > the ccws).
> >
> > As I've pointed out in my comment to patch 2, I believe replacing
> > virtqueue index with virtqueue number is detrimental to clarity.
> >
> > Thus please find a counter-proposal below. If there is interest
> > I can make a series out of it, and prettify it. If I can't convince
> > you guys, then I will have to get used to vqn and virtqueue number.
>
> I would generally prefer "index" as well, but there seemed to be a
> strong sentiment that we should go with "number"... so, what *is* the
> actual general sentiment? It's hard to say, but maybe most people are
> fine with either?
If we really can't decide one way or another then I can run a ballot,
it's not hard.
> >
> > AFAIR the other problem with index was the RSS for virtio-net. But there
> > we are currently heading down a direction of introducing a new
> > abstraction. This approach avoids confusion around the term 'virtqueue
> > index' as much as it avoids confusion around the term 'virtqueue nuber'.
> >
> >
> >> c. RSS area of virtio net has inherited some logic, describe it
> >> using abstract rss_rq_id.
> >
> > -8<--
> > From: Halil Pasic
> > Date: Thu, 30 Mar 2023 17:57:53 +0200
> > Subject: [PATCH 1/1] content: clarify how virtques are identified
> >
> > Clarify how virtqueues are identified in the context of
> > available notifications and in the context of RSS for
> > virtio-net .
> >
> > Signed-off-by: Halil Pasic
> > ---
> > content.tex | 15 ++-
> > device-types/net/description.tex | 30 ++
> > transport-ccw.tex| 2 +-
> > transport-pci.tex| 7 ---
> > 4 files changed, 37 insertions(+), 17 deletions(-)
>
> (...)
>
> > +struct rss_rq_id {
> > + le16 value; /* virtqueue index divided by two */
> > +};
> > +
> > 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];
> > };
> > \end{lstlisting}
> > +
> > +The type struct rss\_rq\_id is introduced to better distinguish receive
> > queue
> > +ids form other integral fields.
> > +
> > +A receive queue id is only defined for receive queues, as the virtqueue
> > index
> > +of the receive virtqueue divided by two (the virtqueue index of a receive
> > +queue is always even). For example receiveq4 is identified by the virtqueue
> > +index 6 and the receive queue id 3.
>
> FWIW, I think this is much easier to understand.
-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
Re: [virtio-dev] [PATCH v10 0/8] Rename queue index to queue number
On Mon, Apr 03, 2023 at 10:57:35PM -0400, Parav Pandit wrote: > > Both solutions are equally consistent in themselves, but I > > argue the latter is better because: > > * it is more consistent with historic usage > Well index/vqn has mixed up anyway now. > For historic reason, I agree that index is right. > But it is too late now. > Comments have not come on time. Why, is there a deadline? Is this blocking some other feature? For something that is supposedly a cleanup we might as well get this right. If you guys both agree index is better, and at this point it sounds convincing, why not do it? It's more or less a machanical replacement. It's not like there's a lot of word smithing going on here except for 8/8 which really works with index just as well. -- MST - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
