[virtio-dev] [PATCH v16 09/11] 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 875c4e6..f3f9f1d 100644
--- a/device-types/net/description.tex
+++ b/device-types/net/description.tex
@@ -1478,8 +1478,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 v16 10/11] 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:
v15->v16:
- replaced receive virtqueue with receive virtqueue id
v13->v14:
- address next comment from Michael and Halil
- changed description of unclassified_queue to use 'specifies'
v12->v13:
- rename vqn to vq_index
- renamed vq index to virtqueue index
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 f3f9f1d..83bdaef 100644
--- a/device-types/net/description.tex
+++ b/device-types/net/description.tex
@@ -1434,11 +1434,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 vq_index_1_16: 15; /* Bits 1 to 16 of the virtqueue index */
+ le16 reserved: 1; /* Set to zero */
+};
+
struct virtio_net_rss_config {
le32 hash_types;
le16 indirection_table_mask;
-le16 unclassified_queue;
-le16 indirection_table[indirection_table_length];
+struct rss_rq_id unclassified_queue;
+struct rss_rq_id indirection_table[indirection_table_length];
le16 max_tx_vq;
u8 hash_key_length;
u8 hash_key_data[hash_key_length];
@@ -1453,10 +1458,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{vq_index_1_16}
+consists of bits 1 to 16 of a virtqueue index. For example, a
+\field{vq_index_1_16} value of 3 corresponds to virtqueue index 6,
+which maps to receiveq4.
+
+Field \field{unclassified_queue} specifies the receive virtqueue id 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 ids.
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}).
@@ -1466,7 +1476,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 ids.
The number of entries in \field{indirection_table}
(\field{indirection_table_mask} + 1) MUST be a power of two.
@@ -1479,7 +1490,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
[virtio-dev] [PATCH v16 11/11] virtio-net: Update vqn to vq_index for cvq cmds
Replace field name vqn to vq_index for recent virtqueue level commands.
Fixes: https://github.com/oasis-tcs/virtio-spec/issues/163
Signed-off-by: Parav Pandit
---
changelog:
v12->v13:
- new patch
---
device-types/net/description.tex | 24
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/device-types/net/description.tex b/device-types/net/description.tex
index 83bdaef..3030222 100644
--- a/device-types/net/description.tex
+++ b/device-types/net/description.tex
@@ -1560,7 +1560,7 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types
/ Network Device / Devi
};
struct virtio_net_ctrl_coal_vq {
-le16 vqn;
+le16 vq_index;
le16 reserved;
struct virtio_net_ctrl_coal coal;
};
@@ -1574,7 +1574,7 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types
/ Network Device / Devi
Coalescing parameters:
\begin{itemize}
-\item \field{vqn}: The virtqueue number of an enabled transmit or receive
virtqueue.
+\item \field{vq_index}: The virtqueue index of an enabled transmit or receive
virtqueue.
\item \field{max_usecs} for RX: Maximum number of microseconds to delay a RX
notification.
\item \field{max_usecs} for TX: Maximum number of microseconds to delay a TX
notification.
\item \field{max_packets} for RX: Maximum number of packets to receive before
a RX notification.
@@ -1587,7 +1587,7 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types
/ Network Device / Devi
\begin{itemize}
\item For commands VIRTIO_NET_CTRL_NOTF_COAL_TX_SET and
VIRTIO_NET_CTRL_NOTF_COAL_RX_SET, the structure virtio_net_ctrl_coal is
write-only for the driver.
\item For the command VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET, the structure
virtio_net_ctrl_coal_vq is write-only for the driver.
-\item For the command VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET, \field{vqn} and
\field{reserved} are write-only
+\item For the command VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET, \field{vq_index} and
\field{reserved} are write-only
for the driver, and the structure virtio_net_ctrl_coal is read-only for
the driver.
\end{itemize}
@@ -1596,9 +1596,9 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types
/ Network Device / Devi
\item VIRTIO_NET_CTRL_NOTF_COAL_TX_SET: use the structure virtio_net_ctrl_coal
to set the \field{max_usecs} and \field{max_packets} parameters for all
transmit virtqueues.
\item VIRTIO_NET_CTRL_NOTF_COAL_RX_SET: use the structure virtio_net_ctrl_coal
to set the \field{max_usecs} and \field{max_packets} parameters for all receive
virtqueues.
\item VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET: use the structure
virtio_net_ctrl_coal_vq to set the \field{max_usecs} and \field{max_packets}
parameters
-for an enabled transmit/receive
virtqueue whose number is \field{vqn}.
+for an enabled transmit/receive
virtqueue whose index is \field{vq_index}.
\item VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET: use the structure
virtio_net_ctrl_coal_vq to get the \field{max_usecs} and \field{max_packets}
parameters
-for an enabled transmit/receive
virtqueue whose number is \field{vqn}.
+for an enabled transmit/receive
virtqueue whose index is \field{vq_index}.
\end{enumerate}
The device may generate notifications more or less frequently than specified
by set commands of the VIRTIO_NET_CTRL_NOTF_COAL class.
@@ -1608,12 +1608,12 @@ \subsubsection{Control Virtqueue}\label{sec:Device
Types / Network Device / Devi
with two pairs of virtqueues as an example:
Each of the following commands sets \field{max_usecs} and \field{max_packets}
parameters for virtqueues.
\begin{itemize}
-\item Command1: VIRTIO_NET_CTRL_NOTF_COAL_RX_SET sets coalescing parameters
for virtqueues having vqn 0 and vqn 2. Virtqueues having vqn 1 and vqn 3 retain
their previous parameters.
-\item Command2: VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET with \field{vqn} = 0 sets
coalescing parameters for virtqueue having vqn 0. Virtqueue having vqn 2
retains the parameters from command1.
-\item Command3: VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET with \field{vqn} = 0, the
device responds with coalescing parameters of vqn 0 set by command2.
-\item Command4: VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET with \field{vqn} = 1 sets
coalescing parameters for virtqueue having vqn 1. Virtqueue having vqn 3
retains its previous parameters.
-\item Command5: VIRTIO_NET_CTRL_NOTF_COAL_TX_SET sets coalescing parameters
for virtqueues having vqn 1 and vqn 3, and overrides the parameters set by
command4.
-\item Command6: VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET with \field{vqn} = 1, the
device responds with coalescing parameters of vqn 1 set by command5.
+\item Command1: VIRTIO_NET_CTRL_NOTF_COAL_RX_SET sets coalescing parameters
for virtqueues having index 0 and index 2. Virtqueues having index 1 and index
3 retain their previous parameters.
+\item Command2: VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET with
[virtio-dev] [PATCH v16 06/11] 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
Acked-by: Halil Pasic
Signed-off-by: Parav Pandit
---
changelog:
v12->v13:
- corrected number to index
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..2d24b4c 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 index 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 v16 08/11] 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
Reviewed-by: Halil Pasic
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 v16 05/11] 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
Reviewed-by: Halil Pasic
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
[virtio-dev] [PATCH v16 07/11] 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
Reviewed-by: Halil Pasic
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 v16 04/11] 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
Acked-by: Halil Pasic
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 aa9842f..cfe3fe2 100644
--- a/transport-pci.tex
+++ b/transport-pci.tex
@@ -1011,7 +1011,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 v16 03/11] content: Rename confusing queue_notify_data and vqn names
Currently queue_notify_data register indicates the device
internal queue notification content. 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 or index).
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 virtqueue identifier/index/data/cookie,
a. rename queue_notify_data to queue_notif_config_data.
b. rename ambiguous vqn to a union of vq_index and vq_config_data
c. 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.
Fixes: https://github.com/oasis-tcs/virtio-spec/issues/163
Acked-by: Halil Pasic
Signed-off-by: Parav Pandit
---
changelog:
v15->v16:
- fixed adding article
- replaced is with was
- fixed incorrect type of 'not'
v14->v15:
- fixed white spaces at the end of lines
- fixed next listed comments from Michael and Halil
- renamed notifications-pci-le.c to notifications-data-le.c
- removed unwanted \item tag
v13->v14:
- added pci transport specific union structure
- added normative lines for case when VIRTIO_F_NOTIF_CONFIG_DATA
is not negotiated.
- added normataive lines for clarify bit-width for driver notification
- replace left over _id with _config_data
- use _notif_config_data name to align to feature name
v12->v13:
- replace _id with _config_data
- dropped vq identifier
- dropped the idea of union as description is for config data feature
v11->v12:
- new patch
---
content.tex | 11 +--
notifications-be.c | 2 +-
notifications-data-le.c | 8 +
notifications-le.c | 2 +-
transport-pci.tex | 71 ++---
5 files changed, 70 insertions(+), 24 deletions(-)
create mode 100644 notifications-data-le.c
diff --git a/content.tex b/content.tex
index 9cf938c..8c5b07a 100644
--- a/content.tex
+++ b/content.tex
@@ -405,8 +405,12 @@ \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 contains either a virtqueue index if
+VIRTIO_F_NOTIF_CONFIG_DATA is not negotiated or device supplied virtqueue
+notification config data if VIRTIO_F_NOTIF_CONFIG_DATA is negotiated.
+
+The notification method and suppling any such virtqueue notification config
data
+is transport specific.
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 +421,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_index_config_data] Either virtqueue index or device supplied
+ queue notification config data corresponding to a 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..bf6d1cd 100644
--- a/notifications-be.c
+++ b/notifications-be.c
@@ -1,5 +1,5 @@
be32 {
- vqn : 16;
+ vq_index: 16; /* previously known as vqn */
next_off : 15;
next_wrap : 1;
};
diff --git a/notifications-data-le.c b/notifications-data-le.c
new file mode 100644
index 000..3ee80e4
--- /dev/null
+++ b/notifications-data-le.c
@@ -0,0 +1,8 @@
+le32 {
+ union {
+ vq_index: 16; /* Used if VIRTIO_F_NOTIF_CONFIG_DATA not
negotiated */
+ vq_notif_config_data: 16; /* Used if VIRTIO_F_NOTIF_CONFIG_DATA
negotiated */
+ };
+ next_off : 15;
+ next_wrap : 1;
+};
diff --git a/notifications-le.c b/notifications-le.c
index fe51267..8a19389 100644
--- a/notifications-le.c
+++ b/notifications-le.c
@@ -1,5 +1,5 @@
le32 {
- vqn : 16;
+ vq_index: 16; /* previously known as vqn */
next_off : 15;
next_wrap : 1;
};
diff --git a/transport-pci.tex b/transport-pci.tex
index 524bfff..aa9842f 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 */
+
[virtio-dev] [PATCH v16 00/11] 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. Field name vqn in the driver notification structure is ambiguous as it is supposed to hold either vq index or device supplied vq config data. 4. Device is really supplying queue identifier or a opaque data 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_config_data. e. rename vqn to vq_notify_config_data 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_config_data 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 patch-11 for virtio-net to update cvq notifications coalescing commands 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: v15->v16: - rewrote vq index range for better reading - fixed adding article - replaced is with was - fixed incorrect type of 'not' - replaced receive virtqueue with receive virtqueue id v14->v15: - added RB and ack tag from Halil to several patches - fixed next listed comments from Michael and Halil - renamed notifications-pci-le.c to notifications-data-le.c - removed unwanted \item tag - fixed white spaces at the end of lines - address next comment from Michael and Halil - changed description of unclassified_queue to use 'specifies' v13->v14: - added Halil's RB tag - added pci transport specific union structure - added normative lines for case when VIRTIO_F_NOTIF_CONFIG_DATA is not negotiated. - added normataive lines for clarify bit-width for driver notification - replace left over _id with _config_data - use _notif_config_data name to align to feature name v12->v13: - renamed queue_notify_id to queue_notify_config_data - added patch to cover notifications coalescing commands after rebase - fixed left out virtqueue number to virtqueue index - dropped abbreviation of virtqueue to vq 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
[virtio-dev] [PATCH v16 01/11] 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: Halil Pasic
Signed-off-by: Parav Pandit
---
changelog:
v15->v16:
- rewrote for better reading
v12->v13:
- avoid virtqueue -> vq abbreviation
- removed Cornelia's reviewed-by due to vq abbreviation change
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..9b694f2 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.}.
+A virtio device can have maximum of 65536 virtqueues. Each virtqueue is
+identified by a virtqueue index. A virtqueue index has a value in the
+range of 0 to 65535.
+
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 v16 02/11] 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
Reviewed-by: Halil Pasic
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 9b694f2..9cf938c 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
RE: [virtio-dev] [PATCH v15 01/11] content: Add vq index text
Parav Pandit writes: >> From: David Edmondson >> Sent: Wednesday, May 3, 2023 6:35 AM > >> > >> > +Each virtqueue is identified by a virtqueue index; virtqueue index >> > +range is from 0 to 65535 inclusive. >> > + >> >> The range is a property of the index, so generally we would say "a virtqueue >> index's range", but that feels a bit odd. >> > The range applies to the virtqueue too. Index is only to identify it. > > So how about below, > A device can have maximum of 65536 virtqueues. Each virtqueue is identified > by a virtqueue index. > A virtqueue index can have a value in range of 0 to 65535. Seems good. -- When you were the brightest star, who were the shadows? - 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 v15 01/11] content: Add vq index text
> From: David Edmondson > Sent: Wednesday, May 3, 2023 6:35 AM > > > > +Each virtqueue is identified by a virtqueue index; virtqueue index > > +range is from 0 to 65535 inclusive. > > + > > The range is a property of the index, so generally we would say "a virtqueue > index's range", but that feels a bit odd. > The range applies to the virtqueue too. Index is only to identify it. So how about below, A device can have maximum of 65536 virtqueues. Each virtqueue is identified by a virtqueue index. A virtqueue index can have a value in range of 0 to 65535. - 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] Re: [PATCH v1 1/2] transport-pci: Introduce legacy registers access commands
> From: Michael S. Tsirkin
> Sent: Wednesday, May 3, 2023 4:09 PM
>
> On Wed, May 03, 2023 at 03:38:31PM -0400, Parav Pandit wrote:
> >
> >
> > On 5/3/2023 3:21 PM, Michael S. Tsirkin wrote:
> > > On Wed, May 03, 2023 at 06:26:58AM +0300, Parav Pandit wrote:
> > > > +\begin{lstlisting}
> > > > +struct virtio_admin_cmd_lreg_wr_data {
> > > > + u8 offset; /* Starting byte offset of the register(s) to write
> > > > */
> > > > + u8 size; /* Number of bytes to write into the register. */
> > > > + u8 register[];
> > > > +};
> > >
> > > BTW if we don't have padding we could reuse buffer size and won't
> > > need size here.
> > Yes. sounds good.
> >
> > > Do we want to tweak admin command structure generally to use
> > > u8 and not le64 for command data then? WDYT?
> > >
> > yeah, cmd and result be u8 is fine.
> > If padding is needed for perf, may be that command can define the padding.
>
> The commands that I defined do need u64 (much easier to work on bitmaps in
> fixed size chunks), so they will use that.
You have already defined it as le64 field. See it below.
So it is not padding, the field itself is le64.
So no need to typecast it differently based on the descriptor size because it
is always array of le64.
It behaves as padding when number of bits are < 64, but it is really not a
(dynamic) padding.
le64 device_admin_cmd_opcodes[];
-
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] Re: [PATCH v1 1/2] transport-pci: Introduce legacy registers access commands
On Wed, May 03, 2023 at 03:38:31PM -0400, Parav Pandit wrote:
>
>
> On 5/3/2023 3:21 PM, Michael S. Tsirkin wrote:
> > On Wed, May 03, 2023 at 06:26:58AM +0300, Parav Pandit wrote:
> > > +\begin{lstlisting}
> > > +struct virtio_admin_cmd_lreg_wr_data {
> > > + u8 offset; /* Starting byte offset of the register(s) to write */
> > > + u8 size; /* Number of bytes to write into the register. */
> > > + u8 register[];
> > > +};
> >
> > BTW if we don't have padding we could reuse buffer size and won't need
> > size here.
> Yes. sounds good.
>
> > Do we want to tweak admin command structure generally to use
> > u8 and not le64 for command data then? WDYT?
> >
> yeah, cmd and result be u8 is fine.
> If padding is needed for perf, may be that command can define the padding.
The commands that I defined do need u64 (much easier to
work on bitmaps in fixed size chunks), so they will use that.
--
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 v15 10/11] virtio-net: Describe RSS using rss rq id
> From: David Edmondson
> Sent: Wednesday, May 3, 2023 11:58 AM
> > -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{vq_index_1_16}
> > +consists of bits 1 to 16 of a virtqueue index. For example, a
> > +\field{vq_index_1_16} value of 3 corresponds to virtqueue index 6,
> > +which maps to receiveq4.
> > +
> > +Field \field{unclassified_queue} specifies 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.
>
> ...an array of receive virtqueue ids.
Well I preferred this way, but ok, I remember probably Halil also had
same/similar comment.
Will fix 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: [virtio-comment] [PATCH v15 03/11] content: Rename confusing queue_notify_data and vqn names
> From: David Edmondson > Sent: Wednesday, May 3, 2023 6:48 AM > Parav Pandit writes: > [..] Will fix all listed comments in the patch. - 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 v1 2/2] transport-pci: Add legacy register access conformance section
> From: Michael S. Tsirkin
> Sent: Wednesday, May 3, 2023 3:19 PM
[..]
> > > just say all these must be supported.
> > > In fact what are you saying here? That all 3 are supported or none
> > > at all? What about just
> > > VIRTIO_ADMIN_CMD_LREG_READ/VIRTIO_ADMIN_CMD_LREG_WRITE?
> > > Looks like a slower but working way to do notifications is through a
> > > common config write, no?
> > Notifications to be done using the notification region exposed and
> > queried using the 3rd QUERY command.
> > >
> > > > +
> > > > +The device MUST support legacy configuration registers to its defined
> width.
> > >
> > > what is this?
> > >
> > Each register has its defined width as 8-bit, 16-bit, 32-bit etc.
> > So device support the access to its defined width.
> > May be to rewrite it differently?
>
> Again you are duplicating the existing text, no?
> And in this case, contradicting it?
>
Yes, to both the questions.
I started with slightly sane version that contradicts below, but since below is
written it what is written so it is the current version.
I will drop above normative.
> When using the legacy interface the driver MAY access
> the device-specific configuration region using any width accesses, and
> a transitional device MUST present driver with the same results as
> when accessed using the ``natural'' access method (i.e.
> 32-bit accesses for 32-bit fields, etc).
>
>
> > > > +
> > > > +The device MAY fail legacy configuration registers access when
> > > > +either the access is for an incorrct register width or if the register
> > > > offset
> is incorrect.
> > >
> > Spell checkers didnt capture the error of incorrct. Need to find
> > better tool.
> >
> > > with which error code?
> > EINVAL
> > but should we repeat the general section here?
>
> this is a command specific case, isn't it?
>
In this case the generic define error codes are covering it for command.
In this specific example, if the register offset provided is outside the
register range,
it returns VIRTIO_ADMIN_STATUS_Q_INVALID_FIELD.
This is already defined in general section.
>
> > >
> > > > +
> > > > +The device MUST allow access of one or multiple bytes of the
> > > > +registers when such register is defined as byte array, for
> > > > +example \field{mac} of \field{struct virtio_net_config} of the Network
> Device.
> > >
> > > so which accesses need to be supported then?
> > >
> > Not sure I follow the question.
> > Can you please explain?
>
> Consider mac, do you allow access to any length at all, from 1 to 6 bytes?
Yes, maybe you missed the text.
I wrote it as " The device MUST allow access of one or multiple bytes of the
registers when its byte array".
The other paragraph you wrote above about " When using the legacy interface,
the driver", it eliminates most of the normative here.
I tried to keep it little sane, but its fine to keep it relaxed too. Sw will be
able to make it strict as it finds it suitable.
So I will drop these normatives.
-
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] Re: [PATCH v1 1/2] transport-pci: Introduce legacy registers access commands
On 5/3/2023 3:21 PM, Michael S. Tsirkin wrote:
On Wed, May 03, 2023 at 06:26:58AM +0300, Parav Pandit wrote:
+\begin{lstlisting}
+struct virtio_admin_cmd_lreg_wr_data {
+ u8 offset; /* Starting byte offset of the register(s) to write */
+ u8 size; /* Number of bytes to write into the register. */
+ u8 register[];
+};
BTW if we don't have padding we could reuse buffer size and won't need
size here.
Yes. sounds good.
Do we want to tweak admin command structure generally to use
u8 and not le64 for command data then? WDYT?
yeah, cmd and result be u8 is fine.
If padding is needed for perf, may be that command can define the padding.
-
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 v1 1/2] transport-pci: Introduce legacy registers access commands
On Wed, May 03, 2023 at 06:26:58AM +0300, Parav Pandit wrote:
> +\begin{lstlisting}
> +struct virtio_admin_cmd_lreg_wr_data {
> + u8 offset; /* Starting byte offset of the register(s) to write */
> + u8 size; /* Number of bytes to write into the register. */
> + u8 register[];
> +};
BTW if we don't have padding we could reuse buffer size and won't need
size here. Do we want to tweak admin command structure generally to use
u8 and not le64 for command data then? WDYT?
--
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: [PATCH v1 2/2] transport-pci: Add legacy register access conformance section
On Wed, May 03, 2023 at 10:53:56AM -0400, Parav Pandit wrote:
>
>
> On 5/3/2023 1:48 AM, Michael S. Tsirkin wrote:
> > On Wed, May 03, 2023 at 06:26:59AM +0300, Parav Pandit wrote:
> > > Add device and driver conformanace section for legacy registers access
> > > commands interface.
> > >
> > > Fixes: https://github.com/oasis-tcs/virtio-spec/issues/167
> > > Signed-off-by: Parav Pandit
> > > ---
> > > conformance.tex | 2 ++
> > > transport-pci-vf-regs.tex | 31 +++
> > > 2 files changed, 33 insertions(+)
> > >
> > > diff --git a/conformance.tex b/conformance.tex
> > > index 01ccd69..dbd8cd6 100644
> > > --- a/conformance.tex
> > > +++ b/conformance.tex
> > > @@ -109,6 +109,7 @@ \section{Conformance Targets}\label{sec:Conformance /
> > > Conformance Targets}
> > > \item \ref{drivernormative:Virtio Transport Options / Virtio Over PCI
> > > Bus / PCI Device Layout / PCI configuration access capability}
> > > \item \ref{drivernormative:Virtio Transport Options / Virtio Over PCI
> > > Bus / PCI-specific Initialization And Device Operation / Device
> > > Initialization / MSI-X Vector Configuration}
> > > \item \ref{drivernormative:Virtio Transport Options / Virtio Over PCI
> > > Bus / PCI-specific Initialization And Device Operation / Notification of
> > > Device Configuration Changes}
> > > +\item \ref{drivernormative:Virtio Transport Options / Virtio Over PCI
> > > Bus / SR-IOV Legacy Registers Access}
> > > \end{itemize}
> > > \conformance{\subsection}{MMIO Driver
> > > Conformance}\label{sec:Conformance / Driver Conformance / MMIO Driver
> > > Conformance}
> > > @@ -194,6 +195,7 @@ \section{Conformance Targets}\label{sec:Conformance /
> > > Conformance Targets}
> > > \item \ref{devicenormative:Virtio Transport Options / Virtio Over PCI
> > > Bus / PCI-specific Initialization And Device Operation / Device
> > > Initialization / MSI-X Vector Configuration}
> > > \item \ref{devicenormative:Virtio Transport Options / Virtio Over PCI
> > > Bus / PCI-specific Initialization And Device Operation / Used Buffer
> > > Notifications}
> > > \item \ref{devicenormative:Virtio Transport Options / Virtio Over PCI
> > > Bus / PCI-specific Initialization And Device Operation / Notification of
> > > Device Configuration Changes}
> > > +\item \ref{devicenormative:Virtio Transport Options / Virtio Over PCI
> > > Bus / SR-IOV Legacy Registers Access}
> > > \end{itemize}
> > > \conformance{\subsection}{MMIO Device
> > > Conformance}\label{sec:Conformance / Device Conformance / MMIO Device
> > > Conformance}
> > > diff --git a/transport-pci-vf-regs.tex b/transport-pci-vf-regs.tex
> > > index 16ced32..7d0574b 100644
> > > --- a/transport-pci-vf-regs.tex
> > > +++ b/transport-pci-vf-regs.tex
> > > @@ -82,3 +82,34 @@ \subsubsection{Legacy Queue Notify Offset
> > > Query}\label{sec:Virtio Transport Opti
> > > le64 offset; /* Byte offset within the BAR */
> > > };
> > > \end{lstlisting}
> > > +
> > > +\devicenormative{\paragraph}{SR-IOV VFs Legacy Registers Access}{Virtio
> > > Transport Options / Virtio Over PCI Bus / SR-IOV Legacy Registers Access}
> > > +
> > > +If the PCI PF device supports legacy registers access, it SHOULD set
> > > +corresponding bits for commands VIRTIO_ADMIN_CMD_LREG_WRITE,
> > > +VIRTIO_ADMIN_CMD_LREG_READ and VIRTIO_ADMIN_CMD_LQ_NOTIFY_QUERY in
> > > +command result of VIRTIO_ADMIN_CMD_LIST_QUERY in
> > > +\field{device_admin_cmd_opcodes}.
> >
> > Don't repeat documentation of VIRTIO_ADMIN_CMD_LIST_QUERY please.
> yeah, I had dual thoughts, I am fine if this looks duplicate.
> Will remove.
>
> > just say all these must be supported.
> > In fact what are you saying here? That all 3 are supported
> > or none at all? What about just
> > VIRTIO_ADMIN_CMD_LREG_READ/VIRTIO_ADMIN_CMD_LREG_WRITE?
> > Looks like a slower but working way to do notifications
> > is through a common config write, no?
> Notifications to be done using the notification region exposed and queried
> using the 3rd QUERY command.
> >
> > > +
> > > +The device MUST support legacy configuration registers to its defined
> > > width.
> >
> > what is this?
> >
> Each register has its defined width as 8-bit, 16-bit, 32-bit etc.
> So device support the access to its defined width.
> May be to rewrite it differently?
Again you are duplicating the existing text, no?
And in this case, contradicting it?
When using the legacy interface the driver MAY access
the device-specific configuration region using any width accesses, and
a transitional device MUST present driver with the same results as
when accessed using the ``natural'' access method (i.e.
32-bit accesses for 32-bit fields, etc).
> > > +
> > > +The device MAY fail legacy configuration registers access when either the
> > > +access is for an incorrct register width or if the register offset is
> > > incorrect.
> >
> Spell checkers didnt capture
[virtio-dev] Re: [RFC PATCH v2] can: virtio: Initial virtio CAN driver.
On 24.04.23 21:02, Marc Kleine-Budde wrote:
Please don't use unicode chars:
| WARNING: Message contains suspicious unicode control characters!
| Subject: [RFC PATCH v2] can: virtio: Initial virtio CAN driver.
| Line: + __le16 length; /* 0..8 CC, 0..64 CANFD, 0..2048
CANXL, 12 bits */
| ^
| Char: SOFT HYPHEN (0xad)
Patch already provided by you.
Please fix this warning:
| drivers/net/can/virtio_can.c:350:35: warning: incorrect type in assignment
(different base types)
| drivers/net/can/virtio_can.c:350:35:expected restricted __le16 [usertype]
length
| drivers/net/can/virtio_can.c:350:35:
| got unsigned int [assigned] [usertype] len
Patch already provided by you.
For now I've only looked at the xmit function.
---
V2:
* Remove the event indication queue and use the config space instead, to
indicate a bus off condition
* Rework RX and TX messages having a length field and some more fields for CAN
EXT
* Fix CAN_EFF_MASK comparison
* Remove MISRA style code (e.g. '! = 0u')
* Remove priorities leftovers
* Remove BUGONs
* Based on virtio can spec RFCv3
Can you post a link to the RFC?
When we have it ready for sending we will do and this should happen ASAP.
Please sort by CONFIG symbols in the Makefile, see below.
Patch already provided by you.
source "drivers/net/can/c_can/Kconfig"
source "drivers/net/can/cc770/Kconfig"
source "drivers/net/can/ctucanfd/Kconfig"
diff --git a/drivers/net/can/Makefile b/drivers/net/can/Makefile
index 52b0f6e10668..d31949052acf 100644
--- a/drivers/net/can/Makefile
+++ b/drivers/net/can/Makefile
@@ -30,5 +30,6 @@ obj-$(CONFIG_CAN_SJA1000) += sja1000/
obj-$(CONFIG_CAN_SUN4I) += sun4i_can.o
obj-$(CONFIG_CAN_TI_HECC) += ti_hecc.o
obj-$(CONFIG_CAN_XILINXCAN) += xilinx_can.o
+obj-$(CONFIG_CAN_VIRTIO_CAN) += virtio_can.o
Please keep this files sorted alphabetically.
Patch already provided by you.
subdir-ccflags-$(CONFIG_CAN_DEBUG_DEVICES) += -DDEBUG
diff --git a/drivers/net/can/virtio_can.c b/drivers/net/can/virtio_can.c
new file mode 100644
index ..23f9c1b6446d
--- /dev/null
+++ b/drivers/net/can/virtio_can.c
@@ -0,0 +1,1060 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * CAN bus driver for the Virtio CAN controller
+ * Copyright (C) 2021-2023 OpenSynergy GmbH
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+/* CAN device queues */
+#define VIRTIO_CAN_QUEUE_TX 0 /* Driver side view! The device receives here */
+#define VIRTIO_CAN_QUEUE_RX 1 /* Driver side view! The device transmits here */
+#define VIRTIO_CAN_QUEUE_CONTROL 2
+#define VIRTIO_CAN_QUEUE_COUNT 3
+
+#define CAN_KNOWN_FLAGS \
+ (VIRTIO_CAN_FLAGS_EXTENDED |\
+VIRTIO_CAN_FLAGS_FD |\
+VIRTIO_CAN_FLAGS_RTR)
+
+/* Max. number of in flight TX messages */
+#define VIRTIO_CAN_ECHO_SKB_MAX 128
+
+struct virtio_can_tx {
+ struct list_head list;
+ int putidx;
+ struct virtio_can_tx_out tx_out;
+ struct virtio_can_tx_in tx_in;
+};
+
+/* virtio_can private data structure */
+struct virtio_can_priv {
+ struct can_priv can;/* must be the first member */
+ /* NAPI for RX messages */
+ struct napi_struct napi;
+ /* NAPI for TX messages */
+ struct napi_struct napi_tx;
+ /* The network device we're associated with */
+ struct net_device *dev;
+ /* The virtio device we're associated with */
+ struct virtio_device *vdev;
+ /* The virtqueues */
+ struct virtqueue *vqs[VIRTIO_CAN_QUEUE_COUNT];
+ /* I/O callback function pointers for the virtqueues */
+ vq_callback_t *io_callbacks[VIRTIO_CAN_QUEUE_COUNT];
+ /* Lock for TX operations */
+ spinlock_t tx_lock;
+ /* Control queue lock. Defensive programming, may be not needed */
+ struct mutex ctrl_lock;
+ /* Wait for control queue processing without polling */
+ struct completion ctrl_done;
+ /* List of virtio CAN TX message */
+ struct list_head tx_list;
+ /* Array of receive queue messages */
+ struct virtio_can_rx rpkt[128];
+ /* Those control queue messages cannot live on the stack! */
+ struct virtio_can_control_out cpkt_out;
+ struct virtio_can_control_in cpkt_in;
+ /* Data to get and maintain the putidx for local TX echo */
+ struct list_head tx_putidx_free;
+ struct list_head *tx_putidx_list;
Can you please explain the big picture on tx_putidx_list. The "struct
list_head" is supposed to be embedded in some kind of struct, it makes
little sense to use them on their own.
This is used to generate efficiently a tx index between 0 and
VIRTIO_CAN_ECHO_SKB_MAX - 1 in function virtio_can_alloc_tx_idx(). Get
next element and determine the position in the array to get an index.
The index is put back in put
[virtio-dev] Re: [PATCH v1 1/2] transport-pci: Introduce legacy registers access commands
On 5/3/2023 12:49 PM, Michael S. Tsirkin wrote:
On Wed, May 03, 2023 at 10:49:14AM -0400, Parav Pandit wrote:
On 5/3/2023 1:50 AM, Michael S. Tsirkin wrote:
On Wed, May 03, 2023 at 06:26:58AM +0300, Parav Pandit wrote:
+Legacy registers write admin command follows \field{struct virtio_admin_cmd}.
+This command writes legacy registers of a member VF device. Driver should write
+appropriate register \field{size} depending on the width of the legacy
+common registers or device specific registers.
+Driver sets command \field{opcode} to VIRTIO_ADMIN_CMD_LREG_WRITE.
+Driver sets \field{group_type} to 1 for VFs.
+Driver sets \field{group_member_id} to a valid VF number.
+
+The \field{command_specific_data} has following listed structure format:
+
+\begin{lstlisting}
+struct virtio_admin_cmd_lreg_wr_data {
+ u8 offset; /* Starting byte offset of the register(s) to write */
+ u8 size; /* Number of bytes to write into the register. */
+ u8 register[];
And maybe add
u8 reserved[]; /* structure padding to multiple of 8 bytes */
+};
Thinking, what do we miss without the padding here?
it's not 100% clear where the padding is.
It is same as rest of the other virtio structures as described in
"Structure Specifications" section.
"Many device and driver in-memory structure layouts are documented using
the C struct syntax. All structures
are assumed to be without additional padding."
-
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 v1 1/2] transport-pci: Introduce legacy registers access commands
On 5/3/2023 12:48 PM, Michael S. Tsirkin wrote: As simple as it is, I feel this falls far short of describing how a device should operate. Some issues: - legacy device config offset changes as msi is enabled/disabled suggest separate commands for device/common config This is fine and covered here. The one who is making msix enable/disable knows which registers to access before/after disable/enable and device also knows it as it is supplying the register. So they just follow the standard legacy register access behavior. But do VFs support INT#x? I will have to re-read the spec. VFs do not support INTx. When hypervisor knows that it cannot support msix for the guest, it can avoid using the VF for the guest. - legacy device endian-ness changes with guest suggest commands to enable LE and BE mode guest endianeness is not known to the device. But is known to hypervisor. It can be an extension command in future as part of the VF administration command to set it by the hypervisor PF. Currently it is only for the LE guests who had some legacy requirement. I don't like tying this to LE implicitly some devices might be BE only. With my idea device can support command to set LE, command to set BE or both. It can be an addition in future if needed. and PCIe is LE anyway. PCIE config endian-ness does not matter heere. - legacy guests often assume INT#x support suggest a way to tunnel that too; INT#x is present on the PCI device itself. So no need to tunnel it. Also INT#x is very narrow case. When MSI-X is not supported, a hypervisor can choose not to even connect such device to the guest VM. devices will support MSI. But *guest* might not support MSIX you only find out late when it is driving the device. It is a pathological case that may not exist because legacy drivers and linux kernels all the way upto 2.6.32 have msix support. And in case if hypervisor sw wants to support for unknown scenario, it can use the one msix based interrupt to emulate intx. I expected to see more statements along the lines of command ABC has the same effect as access to register DEF of the member through the legacy pci interface Yes, good point. I will add it in the theory of operation section for this mapping detail. OK, and overall if you see an existing statement about legacy do not copy it, just explain how it is mapped. Yes, will not copy, only the mapping part to add. - 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 v1 1/2] transport-pci: Introduce legacy registers access commands
On Wed, May 03, 2023 at 10:49:14AM -0400, Parav Pandit wrote:
>
>
> On 5/3/2023 1:50 AM, Michael S. Tsirkin wrote:
> > On Wed, May 03, 2023 at 06:26:58AM +0300, Parav Pandit wrote:
>
> > > +Legacy registers write admin command follows \field{struct
> > > virtio_admin_cmd}.
> > > +This command writes legacy registers of a member VF device. Driver
> > > should write
> > > +appropriate register \field{size} depending on the width of the legacy
> > > +common registers or device specific registers.
> > > +Driver sets command \field{opcode} to VIRTIO_ADMIN_CMD_LREG_WRITE.
> > > +Driver sets \field{group_type} to 1 for VFs.
> > > +Driver sets \field{group_member_id} to a valid VF number.
> > > +
> > > +The \field{command_specific_data} has following listed structure format:
> > > +
> > > +\begin{lstlisting}
> > > +struct virtio_admin_cmd_lreg_wr_data {
> > > + u8 offset; /* Starting byte offset of the register(s) to write */
> > > + u8 size; /* Number of bytes to write into the register. */
> > > + u8 register[];
> >
> > And maybe add
> > u8 reserved[]; /* structure padding to multiple of 8 bytes */
> >
> > > +};
> Thinking, what do we miss without the padding here?
it's not 100% clear where the padding is.
-
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 v1 1/2] transport-pci: Introduce legacy registers access commands
On Wed, May 03, 2023 at 10:47:26AM -0400, Parav Pandit wrote:
>
>
> On 5/3/2023 1:42 AM, Michael S. Tsirkin wrote:
> > On Wed, May 03, 2023 at 06:26:58AM +0300, Parav Pandit wrote:
>
> > > diff --git a/transport-pci-vf-regs.tex b/transport-pci-vf-regs.tex
> > > new file mode 100644
> > > index 000..16ced32
> > > --- /dev/null
> > > +++ b/transport-pci-vf-regs.tex
> >
> > I'd like the name to reflect "legacy". Also I don't think this has
> > to be SRIOV generally. It's just legacy PCI over admin commands.
> > Except for virtio_admin_cmd_lq_notify_query_result
> > which refers to PCI? But that
> > one I can't say for sure what it does.
> >
> It is for legacy now, in future it can be renamed if this is supported.
> We already discussed in v0 that non legacy should not involve hypervisor
> mediation. May be you still it should be. In such case lets park that
> discussion for future. This proposal is not limiting it.
>
> >
> > > @@ -0,0 +1,84 @@
> > > +\subsection{SR-IOV VFs Legacy Registers Access}\label{sec:Virtio
> > > Transport Options / Virtio Over PCI Bus / SR-IOV Legacy Registers Access}
> > > +
> > > +As described in PCIe base specification \hyperref[intro:PCIe]{[PCIe]}
> > > PCI VFs
> > > +do not support IOBAR. A PCI PF device can optionally enable driver to
> > > access
> > > +its member PCI VFs devices legacy common configuration and device
> > > configuration
> > > +registers using an administration virtqueue. A PCI PF group owner device
> > > that
> > > +supports its member VFs legacy registers access via the administration
> > > +virtqueue should supports following commands.
> >
> > As above. It actually can work for any group if we want to.
> True. As defined by the PCIe spec, for virtualized VFs and SFs devices, VI
> is not necessary, and many devices in PCI space are avoiding hypervisor
> mediation, so whether to tunnel or not is really a question for future for
> non legacy registers.
>
> >
> >
> > > +
> > > +\begin{enumerate}
> > > +\item Legacy Registers Write
> > > +\item Legacy Registers Read
> > > +\item Legacy Queue Notify Offset Query
> > > +\end{enumerate}
> > > +
> >
> > Pls add some theory of operation. How can all this be used?
> >
> ok. I will add in this section.
>
> > > +\begin{lstlisting}
> > > +struct virtio_admin_cmd_lreg_rd_result {
> > > + u8 registers[];
> > > +};
> > > +\end{lstlisting}
> > > +
> > > +\subsubsection{Legacy Queue Notify Offset Query}\label{sec:Virtio
> > > Transport Options / Virtio Over PCI Bus / SR-IOV Legacy Registers Access
> > > / Legacy Queue Notify Offset Query}
> > > +
> > > +This command returns the notify offset of the member VF for queue
> > > +notifications.
> >
> > What is this notify offset? It's never explained.
> >
> ok. will add it.
> It is the notification offset where a hypervisor driver can perform driver
> notifications.
>
> > > This command follows \field{struct virtio_admin_cmd}.
> > > +Driver sets command opcode \field{opcode} to
> > > VIRTIO_ADMIN_CMD_LQ_NOTIFY_QUERY.
> > > +There is no command specific data for this command.
> > > +Driver sets \field{group_type} to 1.
> > > +Driver sets \field{group_member_id} to a valid VF number.
> >
> > I think ATM the limitation for this is that the member must be a pci
> > device, otherwise BAR is not well defined. We will have to
> > find a way to extend this for SIOV.
>
> SIOV device will also have the BAR and offset of the parent PF.
> The limitation of current AQ is currently is it indicates the BAR of the
> member device (and does not allow group owner for SIOV), but we can craft it
> via SIOV device BAR and it will be fine. SIOV spec is not yet released for
> this details at all. So we can wait.
>
> > But that is all, please do
> > not repeat documentation about virtio_admin_cmd header, we have that in
> > a central place.
> >
> Make sense. I will omit it here.
>
> > > +
> > > +When command completes successfully, command result contains the queue
> > > +notification address in the listed format:
> > > +
> > > +\begin{lstlisting}
> > > +struct virtio_admin_cmd_lq_notify_query_result {
> > > + u8 bar; /* PCI BAR number of the member VF */
> > > + u8 reserved[7];
> > > + le64 offset; /* Byte offset within the BAR */
> > > +};
> > > +\end{lstlisting}
> > > diff --git a/transport-pci.tex b/transport-pci.tex
> > > index ff889d3..b187576 100644
> > > --- a/transport-pci.tex
> > > +++ b/transport-pci.tex
> > > @@ -1179,3 +1179,5 @@ \subsubsection{Driver Handling
> > > Interrupts}\label{sec:Virtio Transport Options /
> > > re-examine the configuration space to see what changed.
> > > \end{itemize}
> > > \end{itemize}
> > > +
> > > +\input{transport-pci-vf-regs.tex}
> >
> > As simple as it is, I feel this falls far short of describing how
> > a device should operate.
> > Some issues:
> > - legacy device config offset changes as msi is enabled/disabled
> > suggest separate commands for device/common config
> This is fine and covered
[virtio-dev] Re: [virtio-comment] [PATCH v15 10/11] virtio-net: Describe RSS using rss rq id
Parav Pandit writes:
> 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:
> v14->v15:
> - address next comment from Michael and Halil
> - changed description of unclassified_queue to use 'specifies'
> v12->v13:
> - rename vqn to vq_index
> - renamed vq index to virtqueue index
> 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 f3f9f1d..e25a9d0 100644
> --- a/device-types/net/description.tex
> +++ b/device-types/net/description.tex
> @@ -1434,11 +1434,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 vq_index_1_16: 15; /* Bits 1 to 16 of the virtqueue index */
> + le16 reserved: 1; /* Set to zero */
> +};
> +
> struct virtio_net_rss_config {
> le32 hash_types;
> le16 indirection_table_mask;
> -le16 unclassified_queue;
> -le16 indirection_table[indirection_table_length];
> +struct rss_rq_id unclassified_queue;
> +struct rss_rq_id indirection_table[indirection_table_length];
> le16 max_tx_vq;
> u8 hash_key_length;
> u8 hash_key_data[hash_key_length];
> @@ -1453,10 +1458,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{vq_index_1_16}
> +consists of bits 1 to 16 of a virtqueue index. For example, a
> +\field{vq_index_1_16} value of 3 corresponds to virtqueue index 6,
> +which maps to receiveq4.
> +
> +Field \field{unclassified_queue} specifies 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.
...an array of receive virtqueue ids.
>
> 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}).
>
> @@ -1466,7 +1476,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.
>
> @@ -1479,7 +1490,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 /
Re: [virtio-dev] Re: [RFC PATCH v6] virtio-video: Add virtio video device specification
On Wed, May 03 2023, Alex Bennée wrote: > Cornelia Huck writes: > >> On Fri, Apr 28 2023, Alexander Gordeev >> wrote: >> >>> On 27.04.23 15:16, Alexandre Courbot wrote: But in any case, that's irrelevant to the guest-host interface, and I think a big part of the disagreement stems from the misconception that V4L2 absolutely needs to be used on either the guest or the host, which is absolutely not the case. >>> >>> I understand this, of course. I'm arguing, that it is harder to >>> implement it, get it straight and then maintain it over years. Also it >>> brings limitations, that sometimes can be workarounded in the virtio >>> spec, but this always comes at a cost of decreased readability and >>> increased complexity. Overall it looks clearly as a downgrade compared >>> to virtio-video for our use-case. And I believe it would be the same for >>> every developer, that has to actually implement the spec, not just do >>> the pass through. So if we think of V4L2 UAPI pass through as a >>> compatibility device (which I believe it is), then it is fine to have >>> both and keep improving the virtio-video, including taking the best >>> ideas from the V4L2 and overall using it as a reference to make writing >>> the driver simpler. >> >> Let me jump in here and ask another question: >> >> Imagine that, some years in the future, somebody wants to add a virtio >> device for handling video encoding/decoding to their hypervisor. >> >> Option 1: There are different devices to chose from. How is the person >> implementing this supposed to pick a device? They might have a narrow >> use case, where it is clear which of the devices is the one that needs to >> be supported; but they also might have multiple, diverse use cases, and >> end up needing to implement all of the devices. >> >> Option 2: There is one device with various optional features. The person >> implementing this can start off with a certain subset of features >> depending on their expected use cases, and add to it later, if needed; >> but the upfront complexity might be too high for specialized use cases. >> >> Leaving concrete references to V4L2 out of the picture, we're currently >> trying to decide whether our future will be more like Option 1 or Option >> 2, with their respective trade-offs. >> >> I'm slightly biased towards Option 2; does it look feasible at all, or >> am I missing something essential here? (I had the impression that some >> previous confusion had been cleared up; apologies in advance if I'm >> misrepresenting things.) >> >> I'd really love to see some kind of consensus for 1.3, if at all >> possible :) > > I think feature discovery and extensibility is a key part of the VirtIO > paradigm which is why I find the virtio-v4l approach limiting. By > pegging the device to a Linux API we effectively limit the growth of the > device specification to as fast as the Linux API changes. I'm not fully > immersed in v4l but I don't think it is seeing any additional features > developed for it and its limitations for camera are one of the reasons > stuff is being pushed to userspace in solutions like libcamera: > > How is libcamera different from V4L2? > > We see libcamera as a continuation of V4L2. One that can more easily > handle the recent advances in hardware design. As embedded cameras have > developed, all of the complexity has been pushed on to the developers. > With libcamera, all of that complexity is simplified and a single model > is presented to application developers. Ok, that is interesting; thanks for the information. > > That said its not totally our experience to have virtio devices act as > simple pipes for some higher level protocol. The virtio-gpu spec says > very little about the details of how 3D devices work and simply offers > an opaque pipe to push a (potentially propriety) command stream to the > back end. As far as I'm aware the proposals for Vulkan and Wayland > device support doesn't even offer a feature bit but simply changes the > graphics stream type in the command packets. > > We could just offer a VIRTIO_VIDEO_F_V4L feature bit, document it as > incompatible with other feature bits and make that the baseline > implementation but it's not really in the spirit of what VirtIO is > trying to achieve. I'd not be in favour of an incompatible feature flag, either... extensions are good, but conflicting features is something that I'd like to avoid. So, given that I'd still prefer to have a single device: How well does the proposed virtio-video device map to a Linux driver implementation that hooks into V4L2? If the general process flow is compatible and it is mostly a question of wiring the parts together, I think pushing that part of the complexity into the Linux driver is a reasonable trade-off. Being able to use an existing protocol is nice, but if that protocol is not perceived as flexible enough, it is probably not worth encoding it into a spec. (Similar considerations apply to
Re: [virtio-dev] Re: [RFC PATCH v6] virtio-video: Add virtio video device specification
Cornelia Huck writes: > On Fri, Apr 28 2023, Alexander Gordeev > wrote: > >> On 27.04.23 15:16, Alexandre Courbot wrote: >>> But in any case, that's irrelevant to the guest-host interface, and I >>> think a big part of the disagreement stems from the misconception that >>> V4L2 absolutely needs to be used on either the guest or the host, >>> which is absolutely not the case. >> >> I understand this, of course. I'm arguing, that it is harder to >> implement it, get it straight and then maintain it over years. Also it >> brings limitations, that sometimes can be workarounded in the virtio >> spec, but this always comes at a cost of decreased readability and >> increased complexity. Overall it looks clearly as a downgrade compared >> to virtio-video for our use-case. And I believe it would be the same for >> every developer, that has to actually implement the spec, not just do >> the pass through. So if we think of V4L2 UAPI pass through as a >> compatibility device (which I believe it is), then it is fine to have >> both and keep improving the virtio-video, including taking the best >> ideas from the V4L2 and overall using it as a reference to make writing >> the driver simpler. > > Let me jump in here and ask another question: > > Imagine that, some years in the future, somebody wants to add a virtio > device for handling video encoding/decoding to their hypervisor. > > Option 1: There are different devices to chose from. How is the person > implementing this supposed to pick a device? They might have a narrow > use case, where it is clear which of the devices is the one that needs to > be supported; but they also might have multiple, diverse use cases, and > end up needing to implement all of the devices. > > Option 2: There is one device with various optional features. The person > implementing this can start off with a certain subset of features > depending on their expected use cases, and add to it later, if needed; > but the upfront complexity might be too high for specialized use cases. > > Leaving concrete references to V4L2 out of the picture, we're currently > trying to decide whether our future will be more like Option 1 or Option > 2, with their respective trade-offs. > > I'm slightly biased towards Option 2; does it look feasible at all, or > am I missing something essential here? (I had the impression that some > previous confusion had been cleared up; apologies in advance if I'm > misrepresenting things.) > > I'd really love to see some kind of consensus for 1.3, if at all > possible :) I think feature discovery and extensibility is a key part of the VirtIO paradigm which is why I find the virtio-v4l approach limiting. By pegging the device to a Linux API we effectively limit the growth of the device specification to as fast as the Linux API changes. I'm not fully immersed in v4l but I don't think it is seeing any additional features developed for it and its limitations for camera are one of the reasons stuff is being pushed to userspace in solutions like libcamera: How is libcamera different from V4L2? We see libcamera as a continuation of V4L2. One that can more easily handle the recent advances in hardware design. As embedded cameras have developed, all of the complexity has been pushed on to the developers. With libcamera, all of that complexity is simplified and a single model is presented to application developers. That said its not totally our experience to have virtio devices act as simple pipes for some higher level protocol. The virtio-gpu spec says very little about the details of how 3D devices work and simply offers an opaque pipe to push a (potentially propriety) command stream to the back end. As far as I'm aware the proposals for Vulkan and Wayland device support doesn't even offer a feature bit but simply changes the graphics stream type in the command packets. We could just offer a VIRTIO_VIDEO_F_V4L feature bit, document it as incompatible with other feature bits and make that the baseline implementation but it's not really in the spirit of what VirtIO is trying to achieve. -- Alex Bennée Virtualisation Tech Lead @ Linaro - 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 v1 0/2] transport-pci: Introduce legacy registers access using AQ
On 5/3/2023 6:16 AM, Michael S. Tsirkin wrote: On Wed, May 03, 2023 at 06:26:57AM +0300, Parav Pandit wrote: This patch introduces legacy registers access commands for the owner group member PCI PF to access the legacy registers of the member VFs. If in future any SIOV devices to support legacy registers, they can be easily supported using same commands by using the group member identifiers of the future SIOV devices. Absolutely, but maybe we should not create work for this case by repeating PF/VF terminology everywhere? If we omit the PF, VF it is kind of hard to explain things without any tangible objects. For example in your series lot of documentation of AQ is around SR-IOV and VFs. If you take out that notion of VFs for inclusion of undefined SIOV objects its hard as well. So I think we can continue to talk about PF and VFs as is. When SIOV enters, it will be easier to talk about it as VF or SIOV VDEV or something similar. More details as overview, motivation, use case are further described below. Patch summary: -- patch-1 adds administrative virtuqueue commands patch-2 adds its conformance section This short series is on top of [1]. It uses the newly introduced administrative virtqueue facility with 3 new opcodes and uses the existing virtio_admin_cmd. It is expected to go another rebase once v13 for [1] is rolled out and merged. [1] https://lore.kernel.org/virtio-comment/cover.1682354275.git@redhat.com/T/#t Usecase: 1. A hypervisor/system needs to provide transitional virtio devices to the guest VM at scale of thousands, typically, one to eight devices per VM. 2. A hypervisor/system needs to provide such devices using a vendor agnostic driver in the hypervisor system. 3. A hypervisor system prefers to have single stack regardless of virtio device type (net/blk) and be future compatible with a single vfio stack using SR-IOV or other scalable device virtualization technology to map PCI devices to the guest VM. (as transitional or otherwise) Motivation/Background: -- The existing virtio transitional PCI device is missing support for PCI SR-IOV based devices. Currently it does not work beyond PCI PF, or as software emulated device in reality. Currently it has below cited system level limitations: [a] PCIe spec citation: VFs do not support I/O Space and thus VF BARs shall not indicate I/O Space. [b] cpu arch citiation: Intel 64 and IA-32 Architectures Software Developer’s Manual: The processor’s I/O address space is separate and distinct from the physical-memory address space. The I/O address space consists of 64K individually addressable 8-bit I/O ports, numbered 0 through H. [c] PCIe spec citation: If a bridge implements an I/O address range,...I/O address range will be aligned to a 4 KB boundary. Above usecase requirements can be solved by PCI PF group owner enabling the access to its group member PCI VFs legacy registers using an admin virtqueue of the group owner PCI PF. Software usage example: --- The most common way to use and map to the guest VM is by using vfio driver framework in Linux kernel. +--+ |pci_dev_id = 0x100X | +---|pci_rev_id = 0x0 |-+ |vfio device|BAR0 = I/O region | | | |Other attributes | | | +--+ | || + +--+ +-+ | | |I/O BAR to AQ | | Other vfio | | | |rd/wr mapper | | functionalities | | | +--+ +-+ | || +--+-+---+ | | +++ +++ | +-+ | | PCI VF device A | | | AQ |-+>+-+ | | +-+ | | | | legacy regs | | | PCI PF device | | | +-+ | +-+ | +-+ | | +++ | | PCI VF device N | +>+-+ | | | legacy regs | | | +-+ | +-+ 2. Virtio pci driver to bind to the listed device id and use it as native device in the host. 3. Use it in a light weight hypervisor to run bare-metal OS. Please review. Fixes: https://github.com/oasis-tcs/virtio-spec/issues/167 Signed-off-by: Parav Pandit I don't see an overview here though. I will add it in v2. - 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 v1 2/2] transport-pci: Add legacy register access conformance section
On 5/3/2023 1:48 AM, Michael S. Tsirkin wrote:
On Wed, May 03, 2023 at 06:26:59AM +0300, Parav Pandit wrote:
Add device and driver conformanace section for legacy registers access
commands interface.
Fixes: https://github.com/oasis-tcs/virtio-spec/issues/167
Signed-off-by: Parav Pandit
---
conformance.tex | 2 ++
transport-pci-vf-regs.tex | 31 +++
2 files changed, 33 insertions(+)
diff --git a/conformance.tex b/conformance.tex
index 01ccd69..dbd8cd6 100644
--- a/conformance.tex
+++ b/conformance.tex
@@ -109,6 +109,7 @@ \section{Conformance Targets}\label{sec:Conformance /
Conformance Targets}
\item \ref{drivernormative:Virtio Transport Options / Virtio Over PCI Bus /
PCI Device Layout / PCI configuration access capability}
\item \ref{drivernormative:Virtio Transport Options / Virtio Over PCI Bus /
PCI-specific Initialization And Device Operation / Device Initialization /
MSI-X Vector Configuration}
\item \ref{drivernormative:Virtio Transport Options / Virtio Over PCI Bus /
PCI-specific Initialization And Device Operation / Notification of Device
Configuration Changes}
+\item \ref{drivernormative:Virtio Transport Options / Virtio Over PCI Bus /
SR-IOV Legacy Registers Access}
\end{itemize}
\conformance{\subsection}{MMIO Driver Conformance}\label{sec:Conformance / Driver Conformance / MMIO Driver Conformance}
@@ -194,6 +195,7 @@ \section{Conformance Targets}\label{sec:Conformance /
Conformance Targets}
\item \ref{devicenormative:Virtio Transport Options / Virtio Over PCI Bus /
PCI-specific Initialization And Device Operation / Device Initialization /
MSI-X Vector Configuration}
\item \ref{devicenormative:Virtio Transport Options / Virtio Over PCI Bus /
PCI-specific Initialization And Device Operation / Used Buffer Notifications}
\item \ref{devicenormative:Virtio Transport Options / Virtio Over PCI Bus /
PCI-specific Initialization And Device Operation / Notification of Device
Configuration Changes}
+\item \ref{devicenormative:Virtio Transport Options / Virtio Over PCI Bus /
SR-IOV Legacy Registers Access}
\end{itemize}
\conformance{\subsection}{MMIO Device Conformance}\label{sec:Conformance / Device Conformance / MMIO Device Conformance}
diff --git a/transport-pci-vf-regs.tex b/transport-pci-vf-regs.tex
index 16ced32..7d0574b 100644
--- a/transport-pci-vf-regs.tex
+++ b/transport-pci-vf-regs.tex
@@ -82,3 +82,34 @@ \subsubsection{Legacy Queue Notify Offset
Query}\label{sec:Virtio Transport Opti
le64 offset; /* Byte offset within the BAR */
};
\end{lstlisting}
+
+\devicenormative{\paragraph}{SR-IOV VFs Legacy Registers Access}{Virtio
Transport Options / Virtio Over PCI Bus / SR-IOV Legacy Registers Access}
+
+If the PCI PF device supports legacy registers access, it SHOULD set
+corresponding bits for commands VIRTIO_ADMIN_CMD_LREG_WRITE,
+VIRTIO_ADMIN_CMD_LREG_READ and VIRTIO_ADMIN_CMD_LQ_NOTIFY_QUERY in
+command result of VIRTIO_ADMIN_CMD_LIST_QUERY in
+\field{device_admin_cmd_opcodes}.
Don't repeat documentation of VIRTIO_ADMIN_CMD_LIST_QUERY please.
yeah, I had dual thoughts, I am fine if this looks duplicate.
Will remove.
just say all these must be supported.
In fact what are you saying here? That all 3 are supported
or none at all? What about just
VIRTIO_ADMIN_CMD_LREG_READ/VIRTIO_ADMIN_CMD_LREG_WRITE?
Looks like a slower but working way to do notifications
is through a common config write, no?
Notifications to be done using the notification region exposed and
queried using the 3rd QUERY command.
+
+The device MUST support legacy configuration registers to its defined width.
what is this?
Each register has its defined width as 8-bit, 16-bit, 32-bit etc.
So device support the access to its defined width.
May be to rewrite it differently?
+
+The device MAY fail legacy configuration registers access when either the
+access is for an incorrct register width or if the register offset is
incorrect.
Spell checkers didnt capture the error of incorrct. Need to find better
tool.
with which error code?
EINVAL
but should we repeat the general section here?
+
+The device MUST allow access of one or multiple bytes of the registers when
+such register is defined as byte array, for example \field{mac} of
\field{struct
+virtio_net_config} of the Network Device.
so which accesses need to be supported then?
Not sure I follow the question.
Can you please explain?
-
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 v1 1/2] transport-pci: Introduce legacy registers access commands
On 5/3/2023 1:50 AM, Michael S. Tsirkin wrote:
On Wed, May 03, 2023 at 06:26:58AM +0300, Parav Pandit wrote:
+Legacy registers write admin command follows \field{struct virtio_admin_cmd}.
+This command writes legacy registers of a member VF device. Driver should write
+appropriate register \field{size} depending on the width of the legacy
+common registers or device specific registers.
+Driver sets command \field{opcode} to VIRTIO_ADMIN_CMD_LREG_WRITE.
+Driver sets \field{group_type} to 1 for VFs.
+Driver sets \field{group_member_id} to a valid VF number.
+
+The \field{command_specific_data} has following listed structure format:
+
+\begin{lstlisting}
+struct virtio_admin_cmd_lreg_wr_data {
+ u8 offset; /* Starting byte offset of the register(s) to write */
+ u8 size; /* Number of bytes to write into the register. */
+ u8 register[];
And maybe add
u8 reserved[]; /* structure padding to multiple of 8 bytes */
+};
Thinking, what do we miss without the padding here?
-
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 v1 1/2] transport-pci: Introduce legacy registers access commands
On 5/3/2023 1:42 AM, Michael S. Tsirkin wrote:
On Wed, May 03, 2023 at 06:26:58AM +0300, Parav Pandit wrote:
diff --git a/transport-pci-vf-regs.tex b/transport-pci-vf-regs.tex
new file mode 100644
index 000..16ced32
--- /dev/null
+++ b/transport-pci-vf-regs.tex
I'd like the name to reflect "legacy". Also I don't think this has
to be SRIOV generally. It's just legacy PCI over admin commands.
Except for virtio_admin_cmd_lq_notify_query_result
which refers to PCI? But that
one I can't say for sure what it does.
It is for legacy now, in future it can be renamed if this is supported.
We already discussed in v0 that non legacy should not involve hypervisor
mediation. May be you still it should be. In such case lets park that
discussion for future. This proposal is not limiting it.
@@ -0,0 +1,84 @@
+\subsection{SR-IOV VFs Legacy Registers Access}\label{sec:Virtio Transport
Options / Virtio Over PCI Bus / SR-IOV Legacy Registers Access}
+
+As described in PCIe base specification \hyperref[intro:PCIe]{[PCIe]} PCI VFs
+do not support IOBAR. A PCI PF device can optionally enable driver to access
+its member PCI VFs devices legacy common configuration and device configuration
+registers using an administration virtqueue. A PCI PF group owner device that
+supports its member VFs legacy registers access via the administration
+virtqueue should supports following commands.
As above. It actually can work for any group if we want to.
True. As defined by the PCIe spec, for virtualized VFs and SFs devices,
VI is not necessary, and many devices in PCI space are avoiding
hypervisor mediation, so whether to tunnel or not is really a question
for future for non legacy registers.
+
+\begin{enumerate}
+\item Legacy Registers Write
+\item Legacy Registers Read
+\item Legacy Queue Notify Offset Query
+\end{enumerate}
+
Pls add some theory of operation. How can all this be used?
ok. I will add in this section.
+\begin{lstlisting}
+struct virtio_admin_cmd_lreg_rd_result {
+ u8 registers[];
+};
+\end{lstlisting}
+
+\subsubsection{Legacy Queue Notify Offset Query}\label{sec:Virtio Transport
Options / Virtio Over PCI Bus / SR-IOV Legacy Registers Access / Legacy Queue
Notify Offset Query}
+
+This command returns the notify offset of the member VF for queue
+notifications.
What is this notify offset? It's never explained.
ok. will add it.
It is the notification offset where a hypervisor driver can perform
driver notifications.
This command follows \field{struct virtio_admin_cmd}.
+Driver sets command opcode \field{opcode} to VIRTIO_ADMIN_CMD_LQ_NOTIFY_QUERY.
+There is no command specific data for this command.
+Driver sets \field{group_type} to 1.
+Driver sets \field{group_member_id} to a valid VF number.
I think ATM the limitation for this is that the member must be a pci
device, otherwise BAR is not well defined. We will have to
find a way to extend this for SIOV.
SIOV device will also have the BAR and offset of the parent PF.
The limitation of current AQ is currently is it indicates the BAR of the
member device (and does not allow group owner for SIOV), but we can
craft it via SIOV device BAR and it will be fine. SIOV spec is not yet
released for this details at all. So we can wait.
But that is all, please do
not repeat documentation about virtio_admin_cmd header, we have that in
a central place.
Make sense. I will omit it here.
+
+When command completes successfully, command result contains the queue
+notification address in the listed format:
+
+\begin{lstlisting}
+struct virtio_admin_cmd_lq_notify_query_result {
+ u8 bar; /* PCI BAR number of the member VF */
+ u8 reserved[7];
+ le64 offset; /* Byte offset within the BAR */
+};
+\end{lstlisting}
diff --git a/transport-pci.tex b/transport-pci.tex
index ff889d3..b187576 100644
--- a/transport-pci.tex
+++ b/transport-pci.tex
@@ -1179,3 +1179,5 @@ \subsubsection{Driver Handling
Interrupts}\label{sec:Virtio Transport Options /
re-examine the configuration space to see what changed.
\end{itemize}
\end{itemize}
+
+\input{transport-pci-vf-regs.tex}
As simple as it is, I feel this falls far short of describing how
a device should operate.
Some issues:
- legacy device config offset changes as msi is enabled/disabled
suggest separate commands for device/common config
This is fine and covered here. The one who is making msix enable/disable
knows which registers to access before/after disable/enable and device
also knows it as it is supplying the register.
So they just follow the standard legacy register access behavior.
- legacy device endian-ness changes with guest
suggest commands to enable LE and BE mode
guest endianeness is not known to the device. Currently it is only for
the LE guests who had some legacy requirement.
and PCIe is LE anyway.
- legacy guests often assume INT#x support
suggest a way to tunnel that too;
Re: [virtio-dev] Re: [RFC PATCH v6] virtio-video: Add virtio video device specification
On Fri, Apr 28 2023, Alexander Gordeev wrote: > On 27.04.23 15:16, Alexandre Courbot wrote: >> But in any case, that's irrelevant to the guest-host interface, and I >> think a big part of the disagreement stems from the misconception that >> V4L2 absolutely needs to be used on either the guest or the host, >> which is absolutely not the case. > > I understand this, of course. I'm arguing, that it is harder to > implement it, get it straight and then maintain it over years. Also it > brings limitations, that sometimes can be workarounded in the virtio > spec, but this always comes at a cost of decreased readability and > increased complexity. Overall it looks clearly as a downgrade compared > to virtio-video for our use-case. And I believe it would be the same for > every developer, that has to actually implement the spec, not just do > the pass through. So if we think of V4L2 UAPI pass through as a > compatibility device (which I believe it is), then it is fine to have > both and keep improving the virtio-video, including taking the best > ideas from the V4L2 and overall using it as a reference to make writing > the driver simpler. Let me jump in here and ask another question: Imagine that, some years in the future, somebody wants to add a virtio device for handling video encoding/decoding to their hypervisor. Option 1: There are different devices to chose from. How is the person implementing this supposed to pick a device? They might have a narrow use case, where it is clear which of the devices is the one that needs to be supported; but they also might have multiple, diverse use cases, and end up needing to implement all of the devices. Option 2: There is one device with various optional features. The person implementing this can start off with a certain subset of features depending on their expected use cases, and add to it later, if needed; but the upfront complexity might be too high for specialized use cases. Leaving concrete references to V4L2 out of the picture, we're currently trying to decide whether our future will be more like Option 1 or Option 2, with their respective trade-offs. I'm slightly biased towards Option 2; does it look feasible at all, or am I missing something essential here? (I had the impression that some previous confusion had been cleared up; apologies in advance if I'm misrepresenting things.) I'd really love to see some kind of consensus for 1.3, if at all possible :) - 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 RFC net-next v2 0/4] virtio/vsock: support datagrams
On Sat, Apr 15, 2023 at 03:55:05PM +, Bobby Eshleman wrote: On Fri, Apr 28, 2023 at 12:43:09PM +0200, Stefano Garzarella wrote: On Sat, Apr 15, 2023 at 07:13:47AM +, Bobby Eshleman wrote: > CC'ing virtio-dev@lists.oasis-open.org because this thread is starting > to touch the spec. > > On Wed, Apr 19, 2023 at 12:00:17PM +0200, Stefano Garzarella wrote: > > Hi Bobby, > > > > On Fri, Apr 14, 2023 at 11:18:40AM +, Bobby Eshleman wrote: > > > CC'ing Cong. > > > > > > On Fri, Apr 14, 2023 at 12:25:56AM +, Bobby Eshleman wrote: > > > > Hey all! > > > > > > > > This series introduces support for datagrams to virtio/vsock. > > > > Great! Thanks for restarting this work! > > > > No problem! > > > > > > > > > It is a spin-off (and smaller version) of this series from the summer: > > > > https://lore.kernel.org/all/cover.1660362668.git.bobby.eshle...@bytedance.com/ > > > > > > > > Please note that this is an RFC and should not be merged until > > > > associated changes are made to the virtio specification, which will > > > > follow after discussion from this series. > > > > > > > > This series first supports datagrams in a basic form for virtio, and > > > > then optimizes the sendpath for all transports. > > > > > > > > The result is a very fast datagram communication protocol that > > > > outperforms even UDP on multi-queue virtio-net w/ vhost on a variety > > > > of multi-threaded workload samples. > > > > > > > > For those that are curious, some summary data comparing UDP and VSOCK > > > > DGRAM (N=5): > > > > > > > > vCPUS: 16 > > > > virtio-net queues: 16 > > > > payload size: 4KB > > > > Setup: bare metal + vm (non-nested) > > > > > > > > UDP: 287.59 MB/s > > > > VSOCK DGRAM: 509.2 MB/s > > > > > > > > Some notes about the implementation... > > > > > > > > This datagram implementation forces datagrams to self-throttle according > > > > to the threshold set by sk_sndbuf. It behaves similar to the credits > > > > used by streams in its effect on throughput and memory consumption, but > > > > it is not influenced by the receiving socket as credits are. > > > > So, sk_sndbuf influece the sender and sk_rcvbuf the receiver, right? > > > > Correct. > > > We should check if VMCI behaves the same. > > > > > > > > > > The device drops packets silently. There is room for improvement by > > > > building into the device and driver some intelligence around how to > > > > reduce frequency of kicking the virtqueue when packet loss is high. I > > > > think there is a good discussion to be had on this. > > > > Can you elaborate a bit here? > > > > Do you mean some mechanism to report to the sender that a destination > > (cid, port) is full so the packet will be dropped? > > > > Correct. There is also the case of there being no receiver at all for > this address since this case isn't rejected upon connect(). Ideally, > such a socket (which will have 100% packet loss) will be throttled > aggressively. > > Before we go down too far on this path, I also want to clarify that > using UDP over vhost/virtio-net also has this property... this can be > observed by using tcpdump to dump the UDP packets on the bridge network > your VM is using. UDP packets sent to a garbage address can be seen on > the host bridge (this is the nature of UDP, how is the host supposed to > know the address eventually goes nowhere). I mention the above because I > think it is possible for vsock to avoid this cost, given that it > benefits from being point-to-point and g2h/h2g. > > If we're okay with vsock being on par, then the current series does > that. I propose something below that can be added later and maybe > negotiated as a feature bit too. I see and I agree on that, let's do it step by step. If we can do it in the first phase is great, but I think is fine to add this feature later. > > > Can we adapt the credit mechanism? > > > > I've thought about this a lot because the attraction of the approach for > me would be that we could get the wait/buffer-limiting logic for free > and without big changes to the protocol, but the problem is that the > unreliable nature of datagrams means that the source's free-running > tx_cnt will become out-of-sync with the destination's fwd_cnt upon > packet loss. We need to understand where the packet can be lost. If the packet always reaches the destination (vsock driver or device), we can discard it, but also update the counters. > > Imagine a source that initializes and starts sending packets before a > destination socket even is created, the source's self-throttling will be > dysfunctional because its tx_cnt will always far exceed the > destination's fwd_cnt. Right, the other problem I see is that the socket aren't connected, so we have 1-N relationship. Oh yeah, good point. > > We could play tricks with the meaning of the CREDIT_UPDATE message and > fwd_cnt/buf_alloc fields, but I don't think we want to go down that > path. > > I think that the best and simplest
[virtio-dev] Re: [virtio-comment] [PATCH v15 03/11] content: Rename confusing queue_notify_data and vqn names
Parav Pandit writes:
> Currently queue_notify_data register indicates the device
> internal queue notification content. 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 or index).
>
> 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 virtqueue identifier/index/data/cookie,
>
> a. rename queue_notify_data to queue_notif_config_data.
>
> b. rename ambiguous vqn to a union of vq_index and vq_config_data
>
> c. 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.
>
> Fixes: https://github.com/oasis-tcs/virtio-spec/issues/163
> Acked-by: Halil Pasic
> Signed-off-by: Parav Pandit
>
> ---
> changelog:
> v14->v15:
> - fixed white spaces at the end of lines
> - fixed next listed comments from Michael and Halil
> - renamed notifications-pci-le.c to notifications-data-le.c
> - removed unwanted \item tag
> v13->v14:
> - added pci transport specific union structure
> - added normative lines for case when VIRTIO_F_NOTIF_CONFIG_DATA
> is not negotiated.
> - added normataive lines for clarify bit-width for driver notification
> - replace left over _id with _config_data
> - use _notif_config_data name to align to feature name
> v12->v13:
> - replace _id with _config_data
> - dropped vq identifier
> - dropped the idea of union as description is for config data feature
> v11->v12:
> - new patch
> ---
> content.tex | 11 +--
> notifications-be.c | 2 +-
> notifications-data-le.c | 8 +
> notifications-le.c | 2 +-
> transport-pci.tex | 72 ++---
> 5 files changed, 71 insertions(+), 24 deletions(-)
> create mode 100644 notifications-data-le.c
>
> diff --git a/content.tex b/content.tex
> index e79d722..e1345ea 100644
> --- a/content.tex
> +++ b/content.tex
> @@ -404,8 +404,12 @@ \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 contains either virtqueue index if
> VIRTIO_F_NOTIF_CONFIG_DATA
"a" or "the" virtqueue index.
> +is not negotiated or device supplied virtqueue notification config data if
> +VIRTIO_F_NOTIF_CONFIG_DATA is negotiated.
> +
> +A notification method and suppling such virtqueue notification config data is
> +transport specific.
"The" notification method. "supplying any such"
>
> However, some devices benefit from the ability to find out the
> amount of available data in the queue without accessing the virtqueue in
> memory:
> @@ -416,7 +420,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_index_config_data] Either virtqueue index or device supplied
> + queue notification config data corresponding to a 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..bf6d1cd 100644
> --- a/notifications-be.c
> +++ b/notifications-be.c
> @@ -1,5 +1,5 @@
> be32 {
> - vqn : 16;
> + vq_index: 16; /* previously known as vqn */
> next_off : 15;
> next_wrap : 1;
> };
> diff --git a/notifications-data-le.c b/notifications-data-le.c
> new file mode 100644
> index 000..3ee80e4
> --- /dev/null
> +++ b/notifications-data-le.c
> @@ -0,0 +1,8 @@
> +le32 {
> + union {
> + vq_index: 16; /* Used if VIRTIO_F_NOTIF_CONFIG_DATA not
> negotiated */
> + vq_notif_config_data: 16; /* Used if VIRTIO_F_NOTIF_CONFIG_DATA
> negotiated */
> + };
> + next_off : 15;
> + next_wrap : 1;
> +};
> diff --git a/notifications-le.c b/notifications-le.c
> index fe51267..8a19389 100644
> --- a/notifications-le.c
> +++ b/notifications-le.c
> @@ -1,5 +1,5 @@
> le32 {
> - vqn : 16;
> + vq_index: 16; /* previously known as vqn */
> next_off : 15;
> next_wrap : 1;
> };
> diff --git a/transport-pci.tex b/transport-pci.tex
> index 524bfff..b292fc3 100644
> --- a/transport-pci.tex
> +++ b/transport-pci.tex
> @@ -319,7 +319,7 @@ \subsubsection{Common configuration structure
> layout}\label{sec:Virtio
Re: [virtio-dev] [PATCH v15 01/11] content: Add vq index text
Parav Pandit writes:
> 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: Halil Pasic
> Signed-off-by: Parav Pandit
> ---
> changelog:
> v12->v13:
> - avoid virtqueue -> vq abbreviation
> - removed Cornelia's reviewed-by due to vq abbreviation change
> 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 | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/content.tex b/content.tex
> index cff548a..43be58b 100644
> --- a/content.tex
> +++ b/content.tex
> @@ -298,6 +298,9 @@ \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 virtqueue index; virtqueue index range is
> +from 0 to 65535 inclusive.
> +
The range is a property of the index, so generally we would say
"a virtqueue index's range", but that feels a bit odd.
How about avoiding the problem with something like "virtqueue indexes
range 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
--
You know your green from your red.
-
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 v1 0/2] transport-pci: Introduce legacy registers access using AQ
On Wed, May 03, 2023 at 06:26:57AM +0300, Parav Pandit wrote: > This patch introduces legacy registers access commands for the owner > group member PCI PF to access the legacy registers of the member VFs. > > If in future any SIOV devices to support legacy registers, they > can be easily supported using same commands by using the group > member identifiers of the future SIOV devices. Absolutely, but maybe we should not create work for this case by repeating PF/VF terminology everywhere? > More details as overview, motivation, use case are further described > below. > > Patch summary: > -- > patch-1 adds administrative virtuqueue commands > patch-2 adds its conformance section > > This short series is on top of [1]. > It uses the newly introduced administrative virtqueue facility with 3 new > opcodes and uses the existing virtio_admin_cmd. > > It is expected to go another rebase once v13 for [1] is rolled out and merged. > > [1] > https://lore.kernel.org/virtio-comment/cover.1682354275.git@redhat.com/T/#t > > Usecase: > > 1. A hypervisor/system needs to provide transitional >virtio devices to the guest VM at scale of thousands, >typically, one to eight devices per VM. > > 2. A hypervisor/system needs to provide such devices using a >vendor agnostic driver in the hypervisor system. > > 3. A hypervisor system prefers to have single stack regardless of >virtio device type (net/blk) and be future compatible with a >single vfio stack using SR-IOV or other scalable device >virtualization technology to map PCI devices to the guest VM. >(as transitional or otherwise) > > Motivation/Background: > -- > The existing virtio transitional PCI device is missing support for > PCI SR-IOV based devices. Currently it does not work beyond > PCI PF, or as software emulated device in reality. Currently it > has below cited system level limitations: > > [a] PCIe spec citation: > VFs do not support I/O Space and thus VF BARs shall not indicate I/O Space. > > [b] cpu arch citiation: > Intel 64 and IA-32 Architectures Software Developer’s Manual: > The processor’s I/O address space is separate and distinct from > the physical-memory address space. The I/O address space consists > of 64K individually addressable 8-bit I/O ports, numbered 0 through H. > > [c] PCIe spec citation: > If a bridge implements an I/O address range,...I/O address range will be > aligned to a 4 KB boundary. > > Above usecase requirements can be solved by PCI PF group owner enabling > the access to its group member PCI VFs legacy registers using an admin > virtqueue of the group owner PCI PF. > > Software usage example: > --- > The most common way to use and map to the guest VM is by > using vfio driver framework in Linux kernel. > > +--+ > |pci_dev_id = 0x100X | > +---|pci_rev_id = 0x0 |-+ > |vfio device|BAR0 = I/O region | | > | |Other attributes | | > | +--+ | > || > + +--+ +-+ | > | |I/O BAR to AQ | | Other vfio | | > | |rd/wr mapper | | functionalities | | > | +--+ +-+ | > || > +--+-+---+ >| | > +++ +++ > | +-+ | | PCI VF device A | > | | AQ |-+>+-+ | > | +-+ | | | | legacy regs | | > | PCI PF device | | | +-+ | > +-+ | +-+ > | > | +++ > | | PCI VF device N | > +>+-+ | > | | legacy regs | | > | +-+ | > +-+ > > 2. Virtio pci driver to bind to the listed device id and >use it as native device in the host. > > 3. Use it in a light weight hypervisor to run bare-metal OS. > > Please review. > > Fixes: https://github.com/oasis-tcs/virtio-spec/issues/167 > Signed-off-by: Parav Pandit I don't see an overview here though. > --- > changelog: > v0->v1: > - addressed comments, suggesetions and ideas from Michael Tsirkin and Jason > Wang > - far more simpler design than MMR access > - removed complexities of MMR device ids > - removed complexities of MMR registers and extended capabilities > - dropped adding new extended capabilities because if if they are > added, a pci device still needs to have existing capabilities > in the legacy configuration space and hypervisor driver do not > need to access them > > > Parav Pandit (2): > transport-pci:
