RE: [virtio-comment] RE: [PATCH v3 2/2] content: Support enabling virtqueue after DRIVER_OK stage

[Parav Pandit <parav@nvidia.com>]

> From: virtio-comment@lists.oasis-open.org <virtio-comment@lists.oasis-
> open.org> On Behalf Of Michael S. Tsirkin
> Sent: Wednesday, October 25, 2023 3:54 PM
> 
> On Wed, Oct 25, 2023 at 09:55:20AM +0000, Parav Pandit wrote:
> > Hi Cornelia,
> >
> > > From: Parav Pandit <parav@nvidia.com>
> > > Sent: Monday, October 23, 2023 8:52 PM
> > > To: Cornelia Huck <cohuck@redhat.com>; Michael S. Tsirkin
> > > <mst@redhat.com>
> > > Cc: virtio-comment@lists.oasis-open.org; hengqi@linux.alibaba.com;
> > > xuanzhuo@linux.alibaba.com; Shahaf Shuler <shahafs@nvidia.com>
> > > Subject: RE: [virtio-comment] RE: [PATCH v3 2/2] content: Support
> > > enabling virtqueue after DRIVER_OK stage
> > >
> > >
> > > > From: virtio-comment@lists.oasis-open.org
> > > > <virtio-comment@lists.oasis- open.org> On Behalf Of Cornelia Huck
> > > > Sent: Monday, October 23, 2023 8:31 PM
> > > >
> > > > On Mon, Oct 23 2023, Parav Pandit <parav@nvidia.com> wrote:
> > > >
> > > > >> From: Cornelia Huck <cohuck@redhat.com>
> > > > >> Sent: Monday, October 23, 2023 7:58 PM
> > > > >
> > > > >> On Mon, Oct 23 2023, Parav Pandit <parav@nvidia.com> wrote:
> > > > >>
> > > > >> >> From: virtio-comment@lists.oasis-open.org
> > > > >> >> <virtio-comment@lists.oasis- open.org> On Behalf Of Cornelia
> > > > >> >> Huck
> > > > >> >> Sent: Monday, October 23, 2023 7:27 PM
> > > > >> >
> > > > >> >> Well, there are  still comments from me here that post-date
> > > > >> >> v4 (please wait for a bit before posting another version!),
> > > > >> >> so I'll continue waiting for them to be addressed first.
> > > > >> > The last one was [1].
> > > > >> > Which I replied few days ago at [2].
> > > > >> >
> > > > >> > [1]
> > > > >> > https://lists.oasis-open.org/archives/virtio-comment/202310/m
> > > > >> > sg00
> > > > >> > 24
> > > > >> > 0.h
> > > > >> > tml [2]
> > > > >> > https://lists.oasis-open.org/archives/virtio-comment/202310/m
> > > > >> > sg00
> > > > >> > 24
> > > > >> > 2.h
> > > > >> > tml
> > > > >>
> > > > >> I still have some open questions from
> > > > >> <87sf68cjn0.fsf@redhat.com> (first and last paragraph.)
> > > > >
> > > > > Above link is not accessible to me. â
> > > >
> > > > That's not a link, but a message id... many mail clients support
> > > > searching by it, and you can also get it from lore:
> > > >
> > > > 87sf68cjn0.fsf@redhat.com/">https://lore.kernel.org/all/87sf68cjn0.fsf@redhat.com/
> > > >
> > > > > I addressed your comments/questions in v4.
> > > > > Can you please check v4 if they are addressed? I rewrote as you
> > > > > suggested in
> > > > v3.
> > > >
> > > > This was for points you did not agree with. (Again, it might be
> > > > helpful to wait with posting a new version until discussion has
> > > > somewhat
> > > > concluded.)
> > > Sure, will wait next time.
> > >
> > > Your comment was,
> > > =====
> > > This is not really clear to me just from this text, especially if
> > > you just wrote above that enabling or re-enabling is something
> > > different... my understanding would be:
> > >
> > > - if neither dynamic vqs nor queue reset are supported or negotiated,
> > >   the only way to enable a vq is before DRIVER_OK, during setup
> > > - both of these features rely on the transport supporting enabling
> > >   individual queues (either a queue that has not been enabled before, or
> > >   a queue that has been reset)
> > > - the transport is supposed to use the same mechanism for either
> > >
> > > Did I get it right? If so, I think we should make it a bit more clear.
> > > =====
> > >
> > > Above is clarified in below wording without complicating the queue_reset
> here.
> > >
> > > Does that look ok to you?
> > >
> > > +When VIRTIO_F_RING_DYNAMIC is not negotiated, the driver enables
> > > +the virtqueues during the device initialization sequence, i.e.
> > > +after the device sets the FEATURES_OK status bit
> 
> all status bits are set by driver
>
Well device clears it if it does not like the features.

Yes. I will rewrite it as, 

i.e. after the driver has verified that FEATURES_OK bit is set and before the driver sets DRIVER_OK bit.
 
> > and before the driver setting the
> 
> 
> setting -> sets
> 
> > > DRIVER_OK status bit.
> > > +
> > > +When VIRTIO_F_RING_DYNAMIC is negotiated, the driver is not
> > > +required to
> 
> try to avoid "required" outside conformance sections though pls.
> And when used, we upper-case it.
> 
Ok.
How about a rewrite as,

When VIRTIO_F_RING_DYNAMIC is negotiated, the driver may not enable every virtqueue, which the driver wants to use before setting the DRIVER_OK...

> > > +enable every virtqueue it wants to use before setting the DRIVER_OK
> > > +status bit; the driver can choose to enable a virtqueue even after
> > > +the driver has set the DRIVER_OK status bit. The virtqueue enable
> > > +mechanism is
> > > transport specific.
> > > +