All of lore.kernel.org
 help / color / mirror / Atom feed
* [virtio-comment] [PATCH v8] virtio-net: Add support for the flexible driver notification structure.
@ 2020-08-30 21:16 Vitaly Mireyno
  2020-09-04  4:19 ` [virtio-comment] " Jason Wang
  0 siblings, 1 reply; 4+ messages in thread
From: Vitaly Mireyno @ 2020-08-30 21:16 UTC (permalink / raw)
  To: virtio-comment; +Cc: Michael S. Tsirkin, Jason Wang, Ariel Elior

When the driver is required to send an available buffer notification to the device, it sends the virtqueue number to be notified.
With this new feature, the device can optionally provide a per-virtqueue value for the driver to use in driver notifications, instead of the virtqueue number.
Some devices may benefit from this flexibility by providing, for example, an internal virtqueue identifier, or an internal offset related to the virtqueue number.

Changes from v6:
 * Changed the feature definition such that the value, that the driver sets in the vqn field, is per-virtqueue

Signed-off-by: Vitaly Mireyno <vmireyno@marvell.com>
---
 content.tex | 45 ++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 44 insertions(+), 1 deletion(-)

diff --git a/content.tex b/content.tex
index 91735e3..09043e6 100644
--- a/content.tex
+++ b/content.tex
@@ -824,6 +824,8 @@ \subsubsection{Common configuration structure layout}\label{sec:Virtio Transport
         le64 queue_desc;                /* read-write */
         le64 queue_driver;              /* read-write */
         le64 queue_device;              /* read-write */
+        le16 queue_notify_data;         /* read-only for driver */
+        le16 padding;
 };
 \end{lstlisting}
 
@@ -890,6 +892,18 @@ \subsubsection{Common configuration structure layout}\label{sec:Virtio Transport
 
 \item[\field{queue_device}]
         The driver writes the physical address of Device Area here.  See section \ref{sec:Basic Facilities of a Virtio Device / Virtqueues}.
+
+\item[\field{queue_notify_data}]
+        If VIRTIO_NET_F_NOTIF_CONFIG_DATA has been negotiated, the driver will use this value
+        to put it in the 'virtqueue number' field in the available buffer notification structure.
+        See section \ref{sec:Virtio Transport Options / Virtio Over PCI Bus / PCI-specific Initialization And Device Operation / Available Buffer Notifications}.
+        \begin{note}
+        This field provides the device with flexibility to determine how virtqueues
+        will be referred to in available buffer notifications.
+        In a trivial case the device can set \field{queue_notify_data}=vqn. Some devices
+        may benefit from providing another value, for example an internal virtqueue
+        identifier, or an internal offset related to the virtqueue number.
+        \end{note}
 \end{description}
 
 \devicenormative{\paragraph}{Common configuration structure layout}{Virtio Transport Options / Virtio Over PCI Bus / PCI Device Layout / Common configuration structure layout}
@@ -940,7 +954,7 @@ \subsubsection{Common configuration structure layout}\label{sec:Virtio Transport
 
 \drivernormative{\paragraph}{Common configuration structure layout}{Virtio Transport Options / Virtio Over PCI Bus / PCI Device Layout / Common configuration structure layout}
 
-The driver MUST NOT write to \field{device_feature}, \field{num_queues}, \field{config_generation} or \field{queue_notify_off}.
+The driver MUST NOT write to \field{device_feature}, \field{num_queues}, \field{config_generation}, \field{queue_notify_off} or \field{queue_notify_data}.
 
 If VIRTIO_F_RING_PACKED has been negotiated,
 the driver MUST NOT write the value 0 to \field{queue_size}.
@@ -1519,6 +1533,10 @@ \subsubsection{Available Buffer Notifications}\label{sec:Virtio Transport Option
 See \ref{sec:Virtio Transport Options / Virtio Over PCI Bus / PCI Device Layout / Notification capability}
 for how to calculate the Queue Notify address.
 
+\drivernormative{\paragraph}{Available Buffer Notifications}{Virtio Transport Options / Virtio Over PCI Bus / PCI-specific Initialization And Device Operation / Available Buffer Notifications}
+If VIRTIO_NET_F_NOTIF_CONFIG_DATA has been negotiated, the driver MUST set the 'virtqueue number'
+field of the available buffer notification structure to the \field{queue_notify_data} value.
+
 \subsubsection{Used Buffer Notifications}\label{sec:Virtio Transport Options / Virtio Over PCI Bus / PCI-specific Initialization And Device Operation / Used Buffer Notifications}
 
 If a used buffer notification is necessary for a virtqueue, the device would typically act as follows:
@@ -2895,6 +2913,9 @@ \subsection{Feature bits}\label{sec:Device Types / Network Device / Feature bits
 \item[VIRTIO_NET_F_CTRL_MAC_ADDR(23)] Set MAC address through control
     channel.
 
+\item[VIRTIO_NET_F_NOTIF_CONFIG_DATA(57)] Driver uses the data provided by the
+    device as a virtqueue identifier in available buffer notifications.
+
 \item[VIRTIO_NET_F_GUEST_HDRLEN(59)] Driver can provide the exact \field{hdr_len}
     value. Device benefits from knowing the exact header length.
 
@@ -4180,6 +4201,28 @@ \subsubsection{Legacy Interface: Framing Requirements}\label{sec:Device
 See \ref{sec:Basic
 Facilities of a Virtio Device / Virtqueues / Message Framing}.
 
+\subsubsection{Driver Notifications}\label{sec:Device Types / Network Device / Device Operation / Driver Notifications}
+As mentioned in section \ref{sec:Virtqueues / 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 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.
+Some devices may benefit from this flexibility by providing, for example,
+an internal virtqueue identifier, or an internal offset related to the virtqueue number.
+
+The VIRTIO_NET_F_NOTIF_CONFIG_DATA feature indicates the availability of such value.
+The definition of the data to be provided by the driver in driver notification and
+the method for delivering it is transport specific.
+
+For more details about driver notifications over PCI see \ref{sec:Virtio Transport Options / Virtio Over PCI Bus / PCI-specific Initialization And Device Operation / Available Buffer Notifications}.
+
+\drivernormative{\paragraph}{Driver Notifications}{Device Types / Network Device / Device Operation / Driver Notifications}
+The driver SHOULD accept the VIRTIO_NET_F_NOTIF_CONFIG_DATA feature if it has
+been offered.
+
+If the VIRTIO_NET_F_NOTIF_CONFIG_DATA feature has been negotiated, the driver MUST provide the transport-specific value for the virtqueue number with driver notifications.
+
 \section{Block Device}\label{sec:Device Types / Block Device}
 
 The virtio block device is a simple virtual block device (ie.
--

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

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

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


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [virtio-comment] Re: [PATCH v8] virtio-net: Add support for the flexible driver notification structure.
  2020-08-30 21:16 [virtio-comment] [PATCH v8] virtio-net: Add support for the flexible driver notification structure Vitaly Mireyno
@ 2020-09-04  4:19 ` Jason Wang
  2020-09-14 15:45   ` [virtio-comment] RE: [EXT] " Vitaly Mireyno
  0 siblings, 1 reply; 4+ messages in thread
From: Jason Wang @ 2020-09-04  4:19 UTC (permalink / raw)
  To: Vitaly Mireyno, virtio-comment; +Cc: Michael S. Tsirkin, Ariel Elior


On 2020/8/31 上午5:16, Vitaly Mireyno wrote:
> When the driver is required to send an available buffer notification to the device, it sends the virtqueue number to be notified.
> With this new feature, the device can optionally provide a per-virtqueue value for the driver to use in driver notifications, instead of the virtqueue number.
> Some devices may benefit from this flexibility by providing, for example, an internal virtqueue identifier, or an internal offset related to the virtqueue number.
>
> Changes from v6:
>   * Changed the feature definition such that the value, that the driver sets in the vqn field, is per-virtqueue
>
> Signed-off-by: Vitaly Mireyno <vmireyno@marvell.com>
> ---
>   content.tex | 45 ++++++++++++++++++++++++++++++++++++++++++++-
>   1 file changed, 44 insertions(+), 1 deletion(-)
>
> diff --git a/content.tex b/content.tex
> index 91735e3..09043e6 100644
> --- a/content.tex
> +++ b/content.tex
> @@ -824,6 +824,8 @@ \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 */


Do we need to mention this field only exists when 
VIRTIO_NET_F_NOTIF_CONFIG_DATA is negotiated?


> +        le16 padding;


Any reason for intruding the padding here?


>   };
>   \end{lstlisting}
>   
> @@ -890,6 +892,18 @@ \subsubsection{Common configuration structure layout}\label{sec:Virtio Transport
>   
>   \item[\field{queue_device}]
>           The driver writes the physical address of Device Area here.  See section \ref{sec:Basic Facilities of a Virtio Device / Virtqueues}.
> +
> +\item[\field{queue_notify_data}]
> +        If VIRTIO_NET_F_NOTIF_CONFIG_DATA has been negotiated, the driver will use this value


The feature should work for other types of devices, so there's probably 
no need for "NET" prefix.


> +        to put it in the 'virtqueue number' field in the available buffer notification structure.
> +        See section \ref{sec:Virtio Transport Options / Virtio Over PCI Bus / PCI-specific Initialization And Device Operation / Available Buffer Notifications}.
> +        \begin{note}
> +        This field provides the device with flexibility to determine how virtqueues
> +        will be referred to in available buffer notifications.
> +        In a trivial case the device can set \field{queue_notify_data}=vqn. Some devices
> +        may benefit from providing another value, for example an internal virtqueue
> +        identifier, or an internal offset related to the virtqueue number.
> +        \end{note}
>   \end{description}
>   
>   \devicenormative{\paragraph}{Common configuration structure layout}{Virtio Transport Options / Virtio Over PCI Bus / PCI Device Layout / Common configuration structure layout}
> @@ -940,7 +954,7 @@ \subsubsection{Common configuration structure layout}\label{sec:Virtio Transport
>   
>   \drivernormative{\paragraph}{Common configuration structure layout}{Virtio Transport Options / Virtio Over PCI Bus / PCI Device Layout / Common configuration structure layout}
>   
> -The driver MUST NOT write to \field{device_feature}, \field{num_queues}, \field{config_generation} or \field{queue_notify_off}.
> +The driver MUST NOT write to \field{device_feature}, \field{num_queues}, \field{config_generation}, \field{queue_notify_off} or \field{queue_notify_data}.
>   
>   If VIRTIO_F_RING_PACKED has been negotiated,
>   the driver MUST NOT write the value 0 to \field{queue_size}.
> @@ -1519,6 +1533,10 @@ \subsubsection{Available Buffer Notifications}\label{sec:Virtio Transport Option
>   See \ref{sec:Virtio Transport Options / Virtio Over PCI Bus / PCI Device Layout / Notification capability}
>   for how to calculate the Queue Notify address.
>   
> +\drivernormative{\paragraph}{Available Buffer Notifications}{Virtio Transport Options / Virtio Over PCI Bus / PCI-specific Initialization And Device Operation / Available Buffer Notifications}
> +If VIRTIO_NET_F_NOTIF_CONFIG_DATA has been negotiated, the driver MUST set the 'virtqueue number'
> +field of the available buffer notification structure to the \field{queue_notify_data} value.
> +
>   \subsubsection{Used Buffer Notifications}\label{sec:Virtio Transport Options / Virtio Over PCI Bus / PCI-specific Initialization And Device Operation / Used Buffer Notifications}
>   
>   If a used buffer notification is necessary for a virtqueue, the device would typically act as follows:
> @@ -2895,6 +2913,9 @@ \subsection{Feature bits}\label{sec:Device Types / Network Device / Feature bits
>   \item[VIRTIO_NET_F_CTRL_MAC_ADDR(23)] Set MAC address through control
>       channel.
>   
> +\item[VIRTIO_NET_F_NOTIF_CONFIG_DATA(57)] Driver uses the data provided by the
> +    device as a virtqueue identifier in available buffer notifications.
> +
>   \item[VIRTIO_NET_F_GUEST_HDRLEN(59)] Driver can provide the exact \field{hdr_len}
>       value. Device benefits from knowing the exact header length.
>   
> @@ -4180,6 +4201,28 @@ \subsubsection{Legacy Interface: Framing Requirements}\label{sec:Device
>   See \ref{sec:Basic
>   Facilities of a Virtio Device / Virtqueues / Message Framing}.
>   
> +\subsubsection{Driver Notifications}\label{sec:Device Types / Network Device / Device Operation / Driver Notifications}
> +As mentioned in section \ref{sec:Virtqueues / 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 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.
> +Some devices may benefit from this flexibility by providing, for example,
> +an internal virtqueue identifier, or an internal offset related to the virtqueue number.
> +
> +The VIRTIO_NET_F_NOTIF_CONFIG_DATA feature indicates the availability of such value.
> +The definition of the data to be provided by the driver in driver notification and
> +the method for delivering it is transport specific.
> +
> +For more details about driver notifications over PCI see \ref{sec:Virtio Transport Options / Virtio Over PCI Bus / PCI-specific Initialization And Device Operation / Available Buffer Notifications}.
> +
> +\drivernormative{\paragraph}{Driver Notifications}{Device Types / Network Device / Device Operation / Driver Notifications}
> +The driver SHOULD accept the VIRTIO_NET_F_NOTIF_CONFIG_DATA feature if it has
> +been offered.
> +
> +If the VIRTIO_NET_F_NOTIF_CONFIG_DATA feature has been negotiated, the driver MUST provide the transport-specific value for the virtqueue number with driver notifications.
> +


Do we need to clarify that the NOTIFICATION_DATA and NOTIF_CONFIG_DATA 
is mutually exclusive?

Thanks


>   \section{Block Device}\label{sec:Device Types / Block Device}
>   
>   The virtio block device is a simple virtual block device (ie.
> --
>


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

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

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


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [virtio-comment] RE: [EXT] Re: [PATCH v8] virtio-net: Add support for the flexible driver notification structure.
  2020-09-04  4:19 ` [virtio-comment] " Jason Wang
@ 2020-09-14 15:45   ` Vitaly Mireyno
  2020-09-15  8:26     ` [virtio-comment] " Jason Wang
  0 siblings, 1 reply; 4+ messages in thread
From: Vitaly Mireyno @ 2020-09-14 15:45 UTC (permalink / raw)
  To: Jason Wang, virtio-comment; +Cc: Michael S. Tsirkin, Ariel Elior

>
>On 2020/8/31 上午5:16, Vitaly Mireyno wrote:
>> When the driver is required to send an available buffer notification to the device, it sends the
>virtqueue number to be notified.
>> With this new feature, the device can optionally provide a per-virtqueue value for the driver to use in
>driver notifications, instead of the virtqueue number.
>> Some devices may benefit from this flexibility by providing, for example, an internal virtqueue
>identifier, or an internal offset related to the virtqueue number.
>>
>> Changes from v6:
>>   * Changed the feature definition such that the value, that the
>> driver sets in the vqn field, is per-virtqueue
>>
>> Signed-off-by: Vitaly Mireyno <vmireyno@marvell.com>
>> ---
>>   content.tex | 45 ++++++++++++++++++++++++++++++++++++++++++++-
>>   1 file changed, 44 insertions(+), 1 deletion(-)
>>
>> diff --git a/content.tex b/content.tex index 91735e3..09043e6 100644
>> --- a/content.tex
>> +++ b/content.tex
>> @@ -824,6 +824,8 @@ \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 */
>
>
>Do we need to mention this field only exists when VIRTIO_NET_F_NOTIF_CONFIG_DATA is
>negotiated?
>

Will add.

>
>> +        le16 padding;
>
>
>Any reason for intruding the padding here?
>

I thought of a dword alignment, but reading again the spec seems that it's not required. Will remove.

>
>>   };
>>   \end{lstlisting}
>>
>> @@ -890,6 +892,18 @@ \subsubsection{Common configuration structure layout}\label{sec:Virtio
>Transport
>>
>>   \item[\field{queue_device}]
>>           The driver writes the physical address of Device Area here.  See section \ref{sec:Basic Facilities
>of a Virtio Device / Virtqueues}.
>> +
>> +\item[\field{queue_notify_data}]
>> +        If VIRTIO_NET_F_NOTIF_CONFIG_DATA has been negotiated, the driver will use this value
>
>
>The feature should work for other types of devices, so there's probably
>no need for "NET" prefix.
>

Ok. Will move it to the device independent features section ("6 Reserved Feature Bits")

>
>> +        to put it in the 'virtqueue number' field in the available buffer notification structure.
>> +        See section \ref{sec:Virtio Transport Options / Virtio Over PCI Bus / PCI-specific Initialization And
>Device Operation / Available Buffer Notifications}.
>> +        \begin{note}
>> +        This field provides the device with flexibility to determine how virtqueues
>> +        will be referred to in available buffer notifications.
>> +        In a trivial case the device can set \field{queue_notify_data}=vqn. Some devices
>> +        may benefit from providing another value, for example an internal virtqueue
>> +        identifier, or an internal offset related to the virtqueue number.
>> +        \end{note}
>>   \end{description}
>>
>>   \devicenormative{\paragraph}{Common configuration structure layout}{Virtio Transport Options /
>Virtio Over PCI Bus / PCI Device Layout / Common configuration structure layout}
>> @@ -940,7 +954,7 @@ \subsubsection{Common configuration structure layout}\label{sec:Virtio
>Transport
>>
>>   \drivernormative{\paragraph}{Common configuration structure layout}{Virtio Transport Options /
>Virtio Over PCI Bus / PCI Device Layout / Common configuration structure layout}
>>
>> -The driver MUST NOT write to \field{device_feature}, \field{num_queues},
>\field{config_generation} or \field{queue_notify_off}.
>> +The driver MUST NOT write to \field{device_feature}, \field{num_queues},
>\field{config_generation}, \field{queue_notify_off} or \field{queue_notify_data}.
>>
>>   If VIRTIO_F_RING_PACKED has been negotiated,
>>   the driver MUST NOT write the value 0 to \field{queue_size}.
>> @@ -1519,6 +1533,10 @@ \subsubsection{Available Buffer Notifications}\label{sec:Virtio Transport
>Option
>>   See \ref{sec:Virtio Transport Options / Virtio Over PCI Bus / PCI Device Layout / Notification
>capability}
>>   for how to calculate the Queue Notify address.
>>
>> +\drivernormative{\paragraph}{Available Buffer Notifications}{Virtio Transport Options / Virtio Over
>PCI Bus / PCI-specific Initialization And Device Operation / Available Buffer Notifications}
>> +If VIRTIO_NET_F_NOTIF_CONFIG_DATA has been negotiated, the driver MUST set the 'virtqueue
>number'
>> +field of the available buffer notification structure to the \field{queue_notify_data} value.
>> +
>>   \subsubsection{Used Buffer Notifications}\label{sec:Virtio Transport Options / Virtio Over PCI Bus /
>PCI-specific Initialization And Device Operation / Used Buffer Notifications}
>>
>>   If a used buffer notification is necessary for a virtqueue, the device would typically act as follows:
>> @@ -2895,6 +2913,9 @@ \subsection{Feature bits}\label{sec:Device Types / Network Device /
>Feature bits
>>   \item[VIRTIO_NET_F_CTRL_MAC_ADDR(23)] Set MAC address through control
>>       channel.
>>
>> +\item[VIRTIO_NET_F_NOTIF_CONFIG_DATA(57)] Driver uses the data provided by the
>> +    device as a virtqueue identifier in available buffer notifications.
>> +
>>   \item[VIRTIO_NET_F_GUEST_HDRLEN(59)] Driver can provide the exact \field{hdr_len}
>>       value. Device benefits from knowing the exact header length.
>>
>> @@ -4180,6 +4201,28 @@ \subsubsection{Legacy Interface: Framing Requirements}\label{sec:Device
>>   See \ref{sec:Basic
>>   Facilities of a Virtio Device / Virtqueues / Message Framing}.
>>
>> +\subsubsection{Driver Notifications}\label{sec:Device Types / Network Device / Device Operation /
>Driver Notifications}
>> +As mentioned in section \ref{sec:Virtqueues / 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 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.
>> +Some devices may benefit from this flexibility by providing, for example,
>> +an internal virtqueue identifier, or an internal offset related to the virtqueue number.
>> +
>> +The VIRTIO_NET_F_NOTIF_CONFIG_DATA feature indicates the availability of such value.
>> +The definition of the data to be provided by the driver in driver notification and
>> +the method for delivering it is transport specific.
>> +
>> +For more details about driver notifications over PCI see \ref{sec:Virtio Transport Options / Virtio
>Over PCI Bus / PCI-specific Initialization And Device Operation / Available Buffer Notifications}.
>> +
>> +\drivernormative{\paragraph}{Driver Notifications}{Device Types / Network Device / Device
>Operation / Driver Notifications}
>> +The driver SHOULD accept the VIRTIO_NET_F_NOTIF_CONFIG_DATA feature if it has
>> +been offered.
>> +
>> +If the VIRTIO_NET_F_NOTIF_CONFIG_DATA feature has been negotiated, the driver MUST provide
>the transport-specific value for the virtqueue number with driver notifications.
>> +
>
>
>Do we need to clarify that the NOTIFICATION_DATA and NOTIF_CONFIG_DATA
>is mutually exclusive?
>

I don’t see them as mutually exclusive.
NOTIF_CONFIG_DATA means that instead of the vq number, the driver will use other per-vq value in driver notifications.
If NOTIFICATION_DATA has not been negotiated,  it will be just that value.
If NOTIFICATION_DATA has been negotiated, it will be vqn field in the notification struct.
(according to "4.1.5.2 Available Buffer Notifications")
Does this make sense?

>Thanks
>
>
>>   \section{Block Device}\label{sec:Device Types / Block Device}
>>
>>   The virtio block device is a simple virtual block device (ie.
>> --
>>


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [virtio-comment] Re: [EXT] Re: [PATCH v8] virtio-net: Add support for the flexible driver notification structure.
  2020-09-14 15:45   ` [virtio-comment] RE: [EXT] " Vitaly Mireyno
@ 2020-09-15  8:26     ` Jason Wang
  0 siblings, 0 replies; 4+ messages in thread
From: Jason Wang @ 2020-09-15  8:26 UTC (permalink / raw)
  To: Vitaly Mireyno, virtio-comment; +Cc: Michael S. Tsirkin, Ariel Elior


On 2020/9/14 下午11:45, Vitaly Mireyno wrote:
>> On 2020/8/31 上午5:16, Vitaly Mireyno wrote:
>>> When the driver is required to send an available buffer notification to the device, it sends the
>> virtqueue number to be notified.
>>> With this new feature, the device can optionally provide a per-virtqueue value for the driver to use in
>> driver notifications, instead of the virtqueue number.
>>> Some devices may benefit from this flexibility by providing, for example, an internal virtqueue
>> identifier, or an internal offset related to the virtqueue number.
>>> Changes from v6:
>>>    * Changed the feature definition such that the value, that the
>>> driver sets in the vqn field, is per-virtqueue
>>>
>>> Signed-off-by: Vitaly Mireyno <vmireyno@marvell.com>
>>> ---
>>>    content.tex | 45 ++++++++++++++++++++++++++++++++++++++++++++-
>>>    1 file changed, 44 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/content.tex b/content.tex index 91735e3..09043e6 100644
>>> --- a/content.tex
>>> +++ b/content.tex
>>> @@ -824,6 +824,8 @@ \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 */
>>
>> Do we need to mention this field only exists when VIRTIO_NET_F_NOTIF_CONFIG_DATA is
>> negotiated?
>>
> Will add.
>
>>> +        le16 padding;
>>
>> Any reason for intruding the padding here?
>>
> I thought of a dword alignment, but reading again the spec seems that it's not required. Will remove.
>
>>>    };
>>>    \end{lstlisting}
>>>
>>> @@ -890,6 +892,18 @@ \subsubsection{Common configuration structure layout}\label{sec:Virtio
>> Transport
>>>    \item[\field{queue_device}]
>>>            The driver writes the physical address of Device Area here.  See section \ref{sec:Basic Facilities
>> of a Virtio Device / Virtqueues}.
>>> +
>>> +\item[\field{queue_notify_data}]
>>> +        If VIRTIO_NET_F_NOTIF_CONFIG_DATA has been negotiated, the driver will use this value
>>
>> The feature should work for other types of devices, so there's probably
>> no need for "NET" prefix.
>>
> Ok. Will move it to the device independent features section ("6 Reserved Feature Bits")
>
>>> +        to put it in the 'virtqueue number' field in the available buffer notification structure.
>>> +        See section \ref{sec:Virtio Transport Options / Virtio Over PCI Bus / PCI-specific Initialization And
>> Device Operation / Available Buffer Notifications}.
>>> +        \begin{note}
>>> +        This field provides the device with flexibility to determine how virtqueues
>>> +        will be referred to in available buffer notifications.
>>> +        In a trivial case the device can set \field{queue_notify_data}=vqn. Some devices
>>> +        may benefit from providing another value, for example an internal virtqueue
>>> +        identifier, or an internal offset related to the virtqueue number.
>>> +        \end{note}
>>>    \end{description}
>>>
>>>    \devicenormative{\paragraph}{Common configuration structure layout}{Virtio Transport Options /
>> Virtio Over PCI Bus / PCI Device Layout / Common configuration structure layout}
>>> @@ -940,7 +954,7 @@ \subsubsection{Common configuration structure layout}\label{sec:Virtio
>> Transport
>>>    \drivernormative{\paragraph}{Common configuration structure layout}{Virtio Transport Options /
>> Virtio Over PCI Bus / PCI Device Layout / Common configuration structure layout}
>>> -The driver MUST NOT write to \field{device_feature}, \field{num_queues},
>> \field{config_generation} or \field{queue_notify_off}.
>>> +The driver MUST NOT write to \field{device_feature}, \field{num_queues},
>> \field{config_generation}, \field{queue_notify_off} or \field{queue_notify_data}.
>>>    If VIRTIO_F_RING_PACKED has been negotiated,
>>>    the driver MUST NOT write the value 0 to \field{queue_size}.
>>> @@ -1519,6 +1533,10 @@ \subsubsection{Available Buffer Notifications}\label{sec:Virtio Transport
>> Option
>>>    See \ref{sec:Virtio Transport Options / Virtio Over PCI Bus / PCI Device Layout / Notification
>> capability}
>>>    for how to calculate the Queue Notify address.
>>>
>>> +\drivernormative{\paragraph}{Available Buffer Notifications}{Virtio Transport Options / Virtio Over
>> PCI Bus / PCI-specific Initialization And Device Operation / Available Buffer Notifications}
>>> +If VIRTIO_NET_F_NOTIF_CONFIG_DATA has been negotiated, the driver MUST set the 'virtqueue
>> number'
>>> +field of the available buffer notification structure to the \field{queue_notify_data} value.
>>> +
>>>    \subsubsection{Used Buffer Notifications}\label{sec:Virtio Transport Options / Virtio Over PCI Bus /
>> PCI-specific Initialization And Device Operation / Used Buffer Notifications}
>>>    If a used buffer notification is necessary for a virtqueue, the device would typically act as follows:
>>> @@ -2895,6 +2913,9 @@ \subsection{Feature bits}\label{sec:Device Types / Network Device /
>> Feature bits
>>>    \item[VIRTIO_NET_F_CTRL_MAC_ADDR(23)] Set MAC address through control
>>>        channel.
>>>
>>> +\item[VIRTIO_NET_F_NOTIF_CONFIG_DATA(57)] Driver uses the data provided by the
>>> +    device as a virtqueue identifier in available buffer notifications.
>>> +
>>>    \item[VIRTIO_NET_F_GUEST_HDRLEN(59)] Driver can provide the exact \field{hdr_len}
>>>        value. Device benefits from knowing the exact header length.
>>>
>>> @@ -4180,6 +4201,28 @@ \subsubsection{Legacy Interface: Framing Requirements}\label{sec:Device
>>>    See \ref{sec:Basic
>>>    Facilities of a Virtio Device / Virtqueues / Message Framing}.
>>>
>>> +\subsubsection{Driver Notifications}\label{sec:Device Types / Network Device / Device Operation /
>> Driver Notifications}
>>> +As mentioned in section \ref{sec:Virtqueues / 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 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.
>>> +Some devices may benefit from this flexibility by providing, for example,
>>> +an internal virtqueue identifier, or an internal offset related to the virtqueue number.
>>> +
>>> +The VIRTIO_NET_F_NOTIF_CONFIG_DATA feature indicates the availability of such value.
>>> +The definition of the data to be provided by the driver in driver notification and
>>> +the method for delivering it is transport specific.
>>> +
>>> +For more details about driver notifications over PCI see \ref{sec:Virtio Transport Options / Virtio
>> Over PCI Bus / PCI-specific Initialization And Device Operation / Available Buffer Notifications}.
>>> +
>>> +\drivernormative{\paragraph}{Driver Notifications}{Device Types / Network Device / Device
>> Operation / Driver Notifications}
>>> +The driver SHOULD accept the VIRTIO_NET_F_NOTIF_CONFIG_DATA feature if it has
>>> +been offered.
>>> +
>>> +If the VIRTIO_NET_F_NOTIF_CONFIG_DATA feature has been negotiated, the driver MUST provide
>> the transport-specific value for the virtqueue number with driver notifications.
>>> +
>>
>> Do we need to clarify that the NOTIFICATION_DATA and NOTIF_CONFIG_DATA
>> is mutually exclusive?
>>
> I don’t see them as mutually exclusive.
> NOTIF_CONFIG_DATA means that instead of the vq number, the driver will use other per-vq value in driver notifications.
> If NOTIFICATION_DATA has not been negotiated,  it will be just that value.
> If NOTIFICATION_DATA has been negotiated, it will be vqn field in the notification struct.
> (according to "4.1.5.2 Available Buffer Notifications")
> Does this make sense?


I think is works.

Btw, the above description:

"
If the VIRTIO_NET_F_NOTIF_CONFIG_DATA feature has been negotiated, the 
driver MUST provide
the transport-specific value for the virtqueue number with driver 
notifications.
"

The "transport-specific" value seems not accurate, I think it should be 
"device specific"?

Thanks


>
>> Thanks
>>
>>
>>>    \section{Block Device}\label{sec:Device Types / Block Device}
>>>
>>>    The virtio block device is a simple virtual block device (ie.
>>> --
>>>


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

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

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


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2020-09-15  8:26 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-30 21:16 [virtio-comment] [PATCH v8] virtio-net: Add support for the flexible driver notification structure Vitaly Mireyno
2020-09-04  4:19 ` [virtio-comment] " Jason Wang
2020-09-14 15:45   ` [virtio-comment] RE: [EXT] " Vitaly Mireyno
2020-09-15  8:26     ` [virtio-comment] " Jason Wang

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.