Re: [PATCH 1/2] virtio: introduce selective queue enabling

[Xuan Zhuo <xuanzhuo@linux.alibaba.com>]

On Tue, 6 Jun 2023 15:19:53 -0400, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> On Tue, Jun 06, 2023 at 06:04:32PM +0000, Parav Pandit wrote:
> > Hi Eugenio,
> >
> > > From: Eugenio Péez <eperezma@redhat.com>
> > > Sent: Tuesday, June 6, 2023 1:55 PM
> > >
> > > This patch allows the driver to start the device (as set DRIVER_OK) with only
> > > some queues enabled, and then enable another queues later.
> > >
> > > This is the current way to migrate net device state through control virtqueue, in
> > > a software assisted framework with vDPA:
> > > * First, only net CVQ is enabled at DRIVER_OK
> > > * All the control commands (mac address, mq, etc) needed for the device to
> > > behave the same as the source of migration are sent
> > > * Finally all the dataplane queues are enabled.
> > >
> > > Signed-off-by: Eugenio Péez <eperezma@redhat.com>
> > > ---
> > >  content.tex | 15 +++++++++++++--
> > >  1 file changed, 13 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/content.tex b/content.tex
> > > index d2ab9eb..ea4a6d8 100644
> > > --- a/content.tex
> > > +++ b/content.tex
> > > @@ -99,10 +99,10 @@ \section{Feature Bits}\label{sec:Basic Facilities of a
> > > Virtio Device / Feature B  \begin{description}
> > >  \item[0 to 23, and 50 to 127] Feature bits for the specific device type
> > >
> > > -\item[24 to 41] Feature bits reserved for extensions to the queue and
> > > +\item[24 to 42] Feature bits reserved for extensions to the queue and
> > >    feature negotiation mechanisms
> > >
> > > -\item[42 to 49, and 128 and above] Feature bits reserved for future extensions.
> > > +\item[43 to 49, and 128 and above] Feature bits reserved for future
> > > extensions.
> > >  \end{description}
> > >
> > >  \begin{note}
> > > @@ -398,6 +398,13 @@ \section{Virtqueues}\label{sec:Basic Facilities of a
> > > Virtio Device / Virtqueues}  Every driver and device supports either the Packed
> > > or the Split  Virtqueue format, or both.
> > >
> > > +\subsection{Virtqueue selective enabling}\label{sec:Basic Facilities of
> > > +a Virtio Device / Virtqueues / Selective Virtqueue Enable}
> > > +
> > > +If the driver negotiates VIRTIO\_F\_RING\_ENABLE\_ANYTIME, it can
> > > +enable individual virtqueues both before and after the DRIVER\_OK. The
> > > +way to enable a virtqueue is transport specific, but it mimics the
> > > +re-enabling after an hypothetical reset.
> > > +
> > >  \subsection{Virtqueue Reset}\label{sec:Basic Facilities of a Virtio Device /
> > > Virtqueues / Virtqueue Reset}
> > >
> > >  When VIRTIO_F_RING_RESET is negotiated, the driver can reset a virtqueue
> > > @@ -870,6 +877,10 @@ \chapter{Reserved Feature Bits}\label{sec:Reserved
> > > Feature Bits}
> > >  	\ref{devicenormative:Basic Facilities of a Virtio Device / Feature Bits} for
> > >  	handling features reserved for future use.
> > >
> > > +  \item[VIRTIO_F_RING_ENABLE_ANYTIME(42)] This feature indicates  that
> > > + the driver can enable a queue both before and after DRIVER\_OK.
> > > +  See \ref{sec:Basic Facilities of a Virtio Device / Virtqueues / Selective
> > > Virtqueue Enable}.
> > > +
> >
> > Can you please highlight, current spec normative which indicates that
> > all queues must be enabled before DRIVER_OK and more queues cannot be
> > enabled after DRIVER_OK?
>
>
> This one:
> \drivernormative{\subsection}{Device Initialization}{General Initialization And Device Operation / Device Initialization}
>
> it does not say "all queues" if you want to split hairs but I think
> assuming it's safe to enable queues later is dangerous.

Although there is no "all queues" here, if the queue is not ready, the device
may use all the queues after "Device OK". So I also think it might be dangerous.

But I also ran into this problem. We want the device to support 100 queues, but
we also want to use only 16 queues by default. The user can then manually scale
up to 100, if the user has a large memory.

So I think, the point is that "num_queues" is the max and default value. Can we
change "num_queues" to default, then we introduced a new value as max.

Does this way solve your problem? @Eugenio

Thanks.



>
> E.g. vhost user backends tend to assume no new queues appear
> after DRIVER_OK.
>
> --
> MST
>