Re: [PATCH v2 5/8] vfio: Change vfio_external_check_extension() to vfio_file_enforced_coherent()

[Jason Gunthorpe <jgg@nvidia.com>]

On Fri, Apr 22, 2022 at 12:32:58AM +0000, Tian, Kevin wrote:
> > From: Tian, Kevin
> > Sent: Friday, April 22, 2022 8:13 AM
> > 
> > > From: Christoph Hellwig <hch@lst.de>
> > > Sent: Thursday, April 21, 2022 1:41 PM
> > >
> > > I can see why a specific error might be nice for some cases and am
> > > open to that, but as a simple transformation this already looks good:
> > >
> > 
> > There is a slight semantics change together with patch7.
> > 
> > Before patch7 the container must be attached before calling
> > KVM_DEV_VFIO_GROUP_ADD, otherwise vfio_group_get_external_user()
> > will fail. In this case the result of cache coherency for a group is
> > deterministic, either true or false.

No, it isn't. The coherency is a propery of the iommu_domain/container
and it can change when more groups are attached to the same
domain. The best KVM can say is that if it is reporting coherency
enforced it won't stop reporting that - which doesn't change in this
series.

> > After patch7 vfio_group_get_external_user() is not called. It's possible
> > that KVM_DEV_VFIO_GROUP_ADD is called before a container is attached
> > by the group. In this case cache coherency of the group cannot be
> > determined at that point. 

groups don't have cache coherency, only iommu_domains do. In this case
it is correct to report that no non-coherent DMA is possible becuase
it isn't possible at that instant when no domain is attached.

> I prefer to returning an error in this callback > so KVM can still
> fail adding the group, instead of letting the inaccurate > coherency
> info to bit the user in a much latter point...
> 
> More accurately: s/cache coherency/enforced coherency/ in above.

An error here will not cause KVM to fail adding the group without more
changes to how it works.

I'll check, it would be nice to preserve the ABI behavior of rejecting
groups with no container.

Jason