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

[Eugenio Perez Martin <eperezma@redhat.com>]

On Thu, Oct 26, 2023 at 8:02âAM Parav Pandit <parav@nvidia.com> wrote:
>
>
>
> > From: Michael S. Tsirkin <mst@redhat.com>
> > Sent: Thursday, October 26, 2023 11:10 AM
>
> >
> > shorter: i.e. after FEATURES_OK is set and before DRIVER_OK is set.
>
> Ok.
>
> > If VIRTIO_F_RING_DYNAMIC is negotiated,
> > the driver can choose to enable some virtqueues after DRIVER_OK is set.
> Ok. will take this simpler version.
>
> >
> >
> > >
> > > > > > +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.
> >
> >
> >
> >
> > I still feel there is no real explanation in a transport agnostic manner what does
> > it mean to "enable" virtqueue. Do you maybe just mean "configure virtqueue"?
> >
> There is explanation of "re-enabling" a virtqueue in section " Virtqueue Re-enable".
>
> There is no good example of "configured" in generic section either, other than cleanup and mmio section.
>
> >
> > Can we just avoid talking about "enabling"?
> > Is it true that you can basically change anything you want about the vq not just
> > the enable bit? If so talking about enabling is just confusing I think.
> Not really.
> Enabling sequence of a virtqueue for pci and mmio transport is:
>
> 1. select a q
> 2. program the addresses
> 3. set the enable bit
>
> However, as I think more, this one-way interface of driver instructing the device without any error code and without any response is just too limiting interface.
> I will be dropping this series that misuses the config registers beyond its intended init time use.
>
> Let me work on more mature interface that solves following challenges.
> 1. ability to create a virtqueue dynamically after driver_ok
> 2. ability to create virtqueue with one or multiple physical addresses
> 3. ability for the device to return error if it cannot create dynamic vq creation
>

I think 1 is very complex for the simpler use case of delaying the
enabling of the dataplane, especially because virtio-net already
assigns "the last queue" for CVQ. It would require changes to it too.

2 can be solved if we allow resetting a queue before DRIVER_OK, as MST
proposed previously IIRC. Would that work for the use case you have in
mind?

3 can be solved by re-reading the queue_enable field? This is the way
we're already using for checking DRIVER_OK in the PCI transport.

> The question is, do we need this generic facility beyond virtio-net?

I would try to avoid solutions specific for virtio-net, as other
devices may need to reuse them and it would be bad to need to add
quirks for that.

Thanks!