Re: [PATCH 2/2] virtio: pci support virtqueue selective enabling

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

On Wed, Jun 07, 2023 at 09:37:00AM +0200, Eugenio Perez Martin wrote:
> On Tue, Jun 6, 2023 at 9:09âPM Michael S. Tsirkin <mst@redhat.com> wrote:
> >
> > On Tue, Jun 06, 2023 at 07:55:11PM +0200, Eugenio PÃrez wrote:
> > > Reusing virtqueue reset method to enabling a vq.
> > >
> > > Signed-off-by: Eugenio PÃrez <eperezma@redhat.com>
> > > ---
> > >  transport-pci.tex | 4 ++++
> > >  1 file changed, 4 insertions(+)
> > >
> > > diff --git a/transport-pci.tex b/transport-pci.tex
> > > index a5c6719..04c0429 100644
> > > --- a/transport-pci.tex
> > > +++ b/transport-pci.tex
> > > @@ -375,6 +375,10 @@ \subsubsection{Common configuration structure layout}\label{sec:Virtio Transport
> > >
> > >  \item[\field{queue_enable}]
> > >          The driver uses this to selectively prevent the device from executing requests from this virtqueue.
> > > +        If the driver negotiates VIRTIO_F_RING_ENABLE_ANYTIME or
> > > +        VIRTIO_F_RING_RESET, the driver writes 1 to this field to enable a
> > > +        virtqueue.  See \ref{sec:Basic Facilities of a Virtio Device / Virtqueues / Selective Virtqueue Enable}
> > > +        and \ref{sec:Basic Facilities of a Virtio Device / Virtqueues / Virtqueue Reset}.
> > >          1 - enabled; 0 - disabled.
> >
> > No, this is always used to enable queues even with no features.
> >
> 
> Yes. Now I see it is confusing if reading the spec and not the diff,
> but my intention was to not modify the behavior without the feature
> flags. Maybe there is a better way to achieve it.

This is bad:

 Driver always does X.
+If Y then driver does Z.


This is good:

-Driver always does X.
-If Y, then driver does Z, otherwise X.

even though it's a bigger patch, but patch review is
secondary to a readable spec.



> > >  \item[\field{queue_notify_off}]
> > > --
> > > 2.31.1
> >