Re: [virtio-comment] Re: [PATCH V2 4/6] virtio-pci: implement VIRTIO_F_QUEUE_STATE

["Zhu, Lingshan" <lingshan.zhu@intel.com>]

On 11/3/2023 11:50 PM, Parav Pandit wrote:
> > From: virtio-comment@lists.oasis-open.org <virtio-comment@lists.oasis-
> > open.org> On Behalf Of Zhu, Lingshan
> > Sent: Friday, November 3, 2023 8:27 PM
> >
> > On 11/3/2023 7:35 PM, Parav Pandit wrote:
> > > > From: Zhu Lingshan <lingshan.zhu@intel.com>
> > > > Sent: Friday, November 3, 2023 4:05 PM
> > > >
> > > > This patch adds two new le16 fields to common configuration structure
> > > > to support VIRTIO_F_QUEUE_STATE in PCI transport layer.
> > > >
> > > > Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com>
> > > > ---
> > > >    transport-pci.tex | 18 ++++++++++++++++++
> > > >    1 file changed, 18 insertions(+)
> > > >
> > > > diff --git a/transport-pci.tex b/transport-pci.tex index
> > > > a5c6719..3161519 100644
> > > > --- a/transport-pci.tex
> > > > +++ b/transport-pci.tex
> > > > @@ -325,6 +325,10 @@ \subsubsection{Common configuration structure
> > > > layout}\label{sec:Virtio Transport
> > > >            /* About the administration virtqueue. */
> > > >            le16 admin_queue_index;         /* read-only for driver */
> > > >            le16 admin_queue_num;         /* read-only for driver */
> > > > +
> > > > +	/* Virtqueue state */
> > > > +        le16 queue_avail_state;         /* read-write */
> > > > +        le16 queue_used_state;          /* read-write */
> > > This tiny interface for 128 virtio net queues through register read writes, does
> > not work effectively.
> > > There are inflight out of order descriptors for block also.
> > > Hence toy registers like this do not work.
> > Do you know there is a queue_select? Why this does not work? Do you know
> > how other queue related fields work?
> :)
> Yes. If you notice queue_reset related critical spec bug fix was done when it was introduced so that live migration can _actually_ work.
>
> When queue_select is done for 128 queues serially, it take a lot of time to read those slow register interface for this + inflight descriptors + more.
interesting, virtio work in this pattern for many years, right?
> > Like how to set a queue size and enable it?
> Those are meant to be used before DRIVER_OK stage as they are init time registers.
> Not to keep abusing them..
don't you need to set queue_size at the destination side?
> > > Series [1] is comprehensive that covers it even if you consider non-
> > passtrhough device migration model.
> > > Where you can suspend individual queues using new admin command and get
> > them in the device context state.
> > > [1]
> > > https://lists.oasis-open.org/archives/virtio-comment/202310/msg00472.h
> > > tml
> > I suggest you read QEMU migration code. If you don't want to, not my fault.
> > > >    };
> > > >    \end{lstlisting}
> > > >
> > > > @@ -428,6 +432,17 @@ \subsubsection{Common configuration structure
> > > > layout}\label{sec:Virtio Transport
> > > >    	The value 0 indicates no supported administration virtqueues.
> > > >    	This field is valid only if VIRTIO_F_ADMIN_VQ has been
> > > >    	negotiated.
> > > > +
> > > > +\item[\field{queue_avail_state}]
> > > > +        This field is valid only if VIRTIO_F_QUEUE_STATE has been
> > > > +        negotiated. The driver sets and gets the available state of
> > > > +        the virtqueue here (see \ref{sec:Virtqueues / Virtqueue State}).
> > > > +
> > > > +\item[\field{queue_used_state}]
> > > > +        This field is valid only if VIRTIO_F_QUEUE_STATE has been
> > > > +        negotiated. The driver sets and gets the used state of the
> > > > +        virtqueue here (see \ref{sec:Virtqueues / Virtqueue State}).
> > > > +
> > > >    \end{description}
> > > >
> > > >    \devicenormative{\paragraph}{Common configuration structure
> > > > layout}{Virtio Transport Options / Virtio Over PCI Bus / PCI Device
> > > > Layout / Common configuration structure layout} @@ -488,6 +503,9 @@
> > > > \subsubsection{Common configuration structure
> > > > layout}\label{sec:Virtio Transport  present either a value of 0 or a power of 2
> > in  \field{queue_size}.
> > > > +If VIRTIO_F_QUEUE_STATE has not been negotiated, the device MUST
> > > > +ignore any accesses to \field{queue_avail_state} and
> > \field{queue_used_state}.
> > > > +
> > > >    If VIRTIO_F_ADMIN_VQ has been negotiated, the value
> > > > \field{admin_queue_index} MUST be equal to, or bigger than
> > > > \field{num_queues}; also, \field{admin_queue_num} MUST be
> > > > --
> > > > 2.35.3
> >
> > 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/