RE: [PATCH V2 vfio 2/9] virtio-pci: Introduce admin virtqueue

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




> From: Michael S. Tsirkin <mst@xxxxxxxxxx>
> Sent: Monday, October 30, 2023 9:29 PM
> On Mon, Oct 30, 2023 at 03:51:40PM +0000, Parav Pandit wrote:
> >
> >
> > > From: Michael S. Tsirkin <mst@xxxxxxxxxx>
> > > Sent: Monday, October 30, 2023 1:53 AM
> > >
> > > On Sun, Oct 29, 2023 at 05:59:45PM +0200, Yishai Hadas wrote:
> > > > From: Feng Liu <feliu@xxxxxxxxxx>
> > > >
> > > > Introduce support for the admin virtqueue. By negotiating
> > > > VIRTIO_F_ADMIN_VQ feature, driver detects capability and creates
> > > > one administration virtqueue. Administration virtqueue
> > > > implementation in virtio pci generic layer, enables multiple types
> > > > of upper layer drivers such as vfio, net, blk to utilize it.
> > > >
> > > > Signed-off-by: Feng Liu <feliu@xxxxxxxxxx>
> > > > Reviewed-by: Parav Pandit <parav@xxxxxxxxxx>
> > > > Reviewed-by: Jiri Pirko <jiri@xxxxxxxxxx>
> > > > Signed-off-by: Yishai Hadas <yishaih@xxxxxxxxxx>
> > > > ---
> > > >  drivers/virtio/virtio.c                | 37 ++++++++++++++--
> > > >  drivers/virtio/virtio_pci_common.c     |  3 ++
> > > >  drivers/virtio/virtio_pci_common.h     | 15 ++++++-
> > > >  drivers/virtio/virtio_pci_modern.c     | 61 +++++++++++++++++++++++++-
> > > >  drivers/virtio/virtio_pci_modern_dev.c | 18 ++++++++
> > > >  include/linux/virtio_config.h          |  4 ++
> > > >  include/linux/virtio_pci_modern.h      |  5 +++
> > > >  7 files changed, 137 insertions(+), 6 deletions(-)
> > > >
> > > > diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c
> > > > index
> > > > 3893dc29eb26..f4080692b351 100644
> > > > --- a/drivers/virtio/virtio.c
> > > > +++ b/drivers/virtio/virtio.c
> > > > @@ -302,9 +302,15 @@ static int virtio_dev_probe(struct device *_d)
> > > >  	if (err)
> > > >  		goto err;
> > > >
> > > > +	if (dev->config->create_avq) {
> > > > +		err = dev->config->create_avq(dev);
> > > > +		if (err)
> > > > +			goto err;
> > > > +	}
> > > > +
> > > >  	err = drv->probe(dev);
> > > >  	if (err)
> > > > -		goto err;
> > > > +		goto err_probe;
> > > >
> > > >  	/* If probe didn't do it, mark device DRIVER_OK ourselves. */
> > > >  	if (!(dev->config->get_status(dev) & VIRTIO_CONFIG_S_DRIVER_OK))
> > >
> > > Hmm I am not all that happy that we are just creating avq unconditionally.
> > > Can't we do it on demand to avoid wasting resources if no one uses it?
> > >
> > Virtio queues must be enabled before driver_ok as we discussed in
> F_DYNAMIC bit exercise.
> > So creating AQ when first legacy command is invoked, would be too late.
> 
> Well we didn't release the spec with AQ so I am pretty sure there are no devices
> using the feature. Do we want to already make an exception for AQ and allow
> creating AQs after DRIVER_OK even without F_DYNAMIC?
> 
No. it would abuse the init time config registers for the dynamic things like this.
For flow filters and others there is need for dynamic q creation with multiple physical address anyway.
So creating virtqueues dynamically using a generic scheme is desired with new feature bit.





[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux