Re: [PATCH v4 1/1] Introduce MGMT Admin commands

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

On Tue, Apr 05, 2022 at 11:44:34AM +0300, Max Gurtovoy wrote:
> 
> On 4/5/2022 1:29 AM, Michael S. Tsirkin wrote:
> > On Wed, Mar 02, 2022 at 06:02:37PM +0200, Max Gurtovoy wrote:
> > > +It is beyond the scope of the virtio specification to define necessary synchronization in system software to ensure that a virtio PCI VF device
> > > +interrupt configuration modification is reflected in the PCI device. However, it is expected that any modern system software implementing virtio
> > > +drivers and PCI subsystem will ensure that any changes occurring in the VF interrupt configuration is either updated in the PCI VF device or
> > > +such configuration fails.
> > I am no longer sure this assertion holds. For example, how would the PF
> > driver ensure that e.g. VFIO is not bound to a VF for passthrough to
> > a VM and is not configuring the MSI-X configuration of the VF?
> > I don't really see a way to do that cleanly.
> > 
> > Would you care to post a proof of concept or even a pseudo-code patch?
> 
> I don't think we a POC for now.
> 
> We have an example of MSI-X configuration in mlx5 driver:
> https://lore.kernel.org/linux-pci/20210314124256.70253-1-leon@kernel.org/
> 
> The infrastructure in Linux already exist.

I don't see where does it block binding VFIO to VFs?

> > 
> > It appears even harder to support in an (admittedly, uncommon,
> > but apparently available due to virtio subsystem not necessarily
> > matching the PF boundary) case where the admin queue is
> > in a VF and so the admin driver is running within guest.
> 
> I'm not sure I follow.
> 
> If the VF will support AQ it doesn't mean it will have all the optional
> functionality we're adding to the PF.

I am saying I don't see how can software enforce the requirements
you are making of it.

> > 
> > I thus have been thinking of an alternate approach, where the # of MSIX
> > vectors does not change, but the # of VQs does.  Since guests do not
> > currently request more vectors than VQs, this will address the
> > requirement in a cleaner way that guests should be able to universally
> > support.  Synchronization then can be achieved by failing the command if
> > any status bits (or just VIRTIO_CONFIG_S_DRIVER?  did not think too
> > deeply about the difference ...) in the affected VFs are set indicating
> > an attached driver. A new feature bit might be required for this and
> > maybe a new field indicating the actual # of vectors.
> 
> We discussed about the VQs settings in the past. It is more challenging
> thing to do since each device type has it's own VQ types and configurations.
> 
> MSI-X is a common feature that we can apply on each virtio device.

I am not sure the feature works robustly though. Yes controlling
VQs is more work but maybe we just have to bite the bullet.
Yes we discussed it and I was not happy then either, but I
did not notice the VFIO issue then.

-- 
MST