On Thu, 30 Mar 2023 00:23:35 +0300 Parav Pandit <pa...@nvidia.com> wrote:
> Currently specification uses virtqueue index and > number interchangeably to refer to the virtqueue. > > Instead refer to it by its number. > > Fixes: https://github.com/oasis-tcs/virtio-spec/issues/163 > Reviewed-by: Cornelia Huck <coh...@redhat.com> > Reviewed-by: Max Gurtovoy <mgurto...@nvidia.com> > Reviewed-by: Jiri Pirko <j...@nvidia.com> > Signed-off-by: Parav Pandit <pa...@nvidia.com> > --- [..] > transport-pci.tex | 13 +++++++------ > 1 file changed, 7 insertions(+), 6 deletions(-) > > diff --git a/transport-pci.tex b/transport-pci.tex > index b07a822..0f3a48b 100644 > --- a/transport-pci.tex > +++ b/transport-pci.tex > @@ -390,13 +390,14 @@ \subsubsection{Common configuration structure > layout}\label{sec:Virtio Transport > > \item[\field{queue_notify_data}] > This field exists only if VIRTIO_F_NOTIF_CONFIG_DATA has been > negotiated. > - The driver will use this value to put it in the 'virtqueue number' > field > + The driver uses this value in the field \field{vqn} This is one of the problems with this approach... > 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 > + In a trivial case the device can set > + \field{queue_notify_data} to the vq number. Some devices ... the 'vq' number here is not the same vqn above which renders the usage of vqn ambiguous. > @@ -1053,7 +1054,7 @@ \subsubsection{Available Buffer > Notifications}\label{sec:Virtio Transport Option > If VIRTIO_F_NOTIF_CONFIG_DATA has been negotiated: > \begin{itemize} > \item If VIRTIO_F_NOTIFICATION_DATA has not been negotiated, the driver MUST > use the > -\field{queue_notify_data} value instead of the virtqueue index. > +\field{queue_notify_data} value instead of the vq number. > \item If VIRTIO_F_NOTIFICATION_DATA has been negotiated, the driver MUST set > the > \field{vqn} field to the \field{queue_notify_data} value. And here it is really obvious: if VIRTIO_F_NOTIF_CONFIG_DATA has been negotiated the field \field{vqn} does not hold a virtqueue number/vq nuber/vqn but a device supplied identifier which may or may not be same as the vqn. So we went from: if !VIRTIO_F_NOTIF_CONFIG_DATA then vqn is the virtqueue index if !!VIRTIO_F_NOTIF_CONFIG_DATA then vqn is queue_notify_data which may or may not be the same as the virtqueue index to if !VIRTIO_F_NOTIF_CONFIG_DATA then vqn is the vq number if !!VIRTIO_F_NOTIF_CONFIG_DATA then vqn is queue_notify_data which may or may not be the same as the vq number. Which is my opinion less clear that what we had before. And in my opinion calling the very same thing virtqueue und vq number interchangeably is not helpful either -- makes grepping harder. I don't see the benefit of replacing virtqueue index with virtqueue number. AFAIR the supposed benefit was to: a) harmonize the terminology in the notifications part with the rest of the spec b) to resolve the RSS problematic with its receive virtqueue indexes. For b) we are going down a different route calling those receive queue ids (AFAIR), and for the notifications part see my comments above. I'm about to reply to the cover letter, and make my argument against standardizing virtqueue nuber (and in favor of standardizing on the on virtqueue index) along with a diff that is supposed to act as a counter proposal. If that doesn't work I will give up and make peace with vq number and vqn. Regards, Halil > \end{itemize} --------------------------------------------------------------------- To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org