Re: [virtio-comment] [PATCH v9] virtio-net: Add support for the flexible driver notification structure.

["Michael S. Tsirkin" <mst@redhat.com>]

On Tue, Nov 24, 2020 at 11:19:07AM +0100, Cornelia Huck wrote:
> On Mon, 23 Nov 2020 17:16:42 +0100
> Halil Pasic <pasic@linux.ibm.com> wrote:
> 
> > On Thu, 5 Nov 2020 22:41:42 +0000
> > Vitaly Mireyno <vmireyno@marvell.com> 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 v8:
> > >  * Incorporated comments for v8:
> > >      - moved the feature from a network device to a global section
> > >      - few minor changes
> > > 
> > > Signed-off-by: Vitaly Mireyno <vmireyno@marvell.com>
> > > ---
> > >  content.tex | 46 +++++++++++++++++++++++++++++++++++++++++++++-
> > >  1 file changed, 45 insertions(+), 1 deletion(-)
> 
> > 
> > Whith your patch applied on top of current master I get something like this
> > 
> > """
> > \subsubsection{Available Buffer Notifications}\label{sec:Virtio Transport Options / Virtio Over PCI Bus / PCI-specific Initialization And Device Operation / Available Buffer Notifications}
> > 
> > When VIRTIO_F_NOTIFICATION_DATA has not been negotiated,
> > the driver sends an available buffer notification to the device by writing
> > the 16-bit virtqueue index
> > of this virtqueue to the Queue Notify address.
> > 
> > When VIRTIO_F_NOTIFICATION_DATA has been negotiated,
> > the driver sends an available buffer notification to the device by writing
> > the following 32-bit value to the Queue Notify address:
> > \lstinputlisting{notifications-le.c}
> > 
> > See \ref{sec:Virtqueues / Driver notifications}~\nameref{sec:Virtqueues / Driver notifications}
> > for the definition of the components.
> > 
> > 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_F_NOTIF_CONFIG_DATA has been negotiated:
> > \begin{itemize}
> > \item If VIRTIO_F_NOTIFICATION_DATA has not been negotiated, the driver MUST use the
> > \field{queue_notify_data} value instead of the virtqueue index.
> > \item If VIRTIO_F_NOTIFICATION_DATA has been negotiated, the driver MUST set the
> > \field{vqn} field to the \field{queue_notify_data} value.
> > \end{itemize}
> > 
> > \subsubsection{Used Buffer Notifications}\label{sec:Virtio Transport Options / Virtio Over PCI Bus / PCI-specific Initialization And Device Operation / Used Buffer Notifications}
> > """
> > 
> > The device normative section and the preceding text are IMHO in
> > contradiction. The former says  the driver sends a 16-bit virtqueue
> > index unconditionally, the later says if if VIRTIO_F_NOTIFICATION_DATA
> > has been negotiated then use queue_notify_data, instead (thus
> > referencing the non-normative content).
> > 
> 
> I'd probably reword the paragraph above to
> 
> "When VIRTIO_F_NOTIFICATION_DATA has not been negotiated, the driver
> sends an available buffer notification to the device by writing either
> the 16-bit virtqueue index of this virtqueue or, if
> VIRTIO_F_NOTIF_CONFIG_DATA has been negotiated, the 16-bit queue
> notification data to the Queue Notify address."
> 
> > IMHO we need a normative section that fully describes all the available
> > cases.
> > 
> > I think we can do this on top.
> 
> Agreed.

Vitaly can you please propose a patch addressing all these comments?


> > 
> > Regards,
> > Halil
> > 
> > This publicly archived list offers a means to provide input to the
> > OASIS Virtual I/O Device (VIRTIO) TC.
> > 
> > In order to verify user consent to the Feedback License terms and
> > to minimize spam in the list archive, subscription is required
> > before posting.
> > 
> > Subscribe: virtio-comment-subscribe@lists.oasis-open.org
> > Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
> > List help: virtio-comment-help@lists.oasis-open.org
> > List archive: https://lists.oasis-open.org/archives/virtio-comment/
> > Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
> > List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
> > Committee: https://www.oasis-open.org/committees/virtio/
> > Join OASIS: https://www.oasis-open.org/join/
> > 


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/