Re: [PATCH 07/10] vfio: Move vfio_external_check_extension() to vfio_file_ops

From: Jason Gunthorpe <jgg@nvidia.com>
To: "Tian, Kevin" <kevin.tian@intel.com>
Cc: Alex Williamson <alex.williamson@redhat.com>,
	Cornelia Huck <cohuck@redhat.com>,
	"kvm@vger.kernel.org" <kvm@vger.kernel.org>,
	Paolo Bonzini <pbonzini@redhat.com>,
	Eric Auger <eric.auger@redhat.com>,
	Christoph Hellwig <hch@lst.de>, "Liu, Yi L" <yi.l.liu@intel.com>
Subject: Re: [PATCH 07/10] vfio: Move vfio_external_check_extension() to vfio_file_ops
Date: Tue, 19 Apr 2022 16:23:40 -0300	[thread overview]
Message-ID: <20220419192340.GB1251821@nvidia.com> (raw)
In-Reply-To: <BN9PR11MB52761E7F5FB90976888A24088CEE9@BN9PR11MB5276.namprd11.prod.outlook.com>

On Fri, Apr 15, 2022 at 04:07:51AM +0000, Tian, Kevin wrote:

> > +	struct vfio_group *group = filep->private_data;
> > +	bool ret;
> > +
> > +	/*
> > +	 * Since the coherency state is determined only once a container is
> > +	 * attached the user must do so before they can prove they have
> > +	 * permission.
> > +	 */
> > +	if (vfio_group_add_container_user(group))
> > +		return true;
> 
> I wonder whether it's better to return error here and let KVM to
> decide whether it wants to allow wbinvd in such case (though
> likely the conclusion is same) or simply rejects adding the group.

Since the new model is to present proof at add it is OK - it just
means the user doesn't have a proof to enable wbinvd.

The thing that is missing here is a notifier so kvm can track changes
in the group's assigned iommu_domain, but I think it is not necessary..

Jason