RE: [PATCH v2 4/4] vfio: Require that devices support DMA cache coherence

From: "Tian, Kevin" <kevin.tian@intel.com>
To: Jason Gunthorpe <jgg@nvidia.com>,
	Alex Williamson <alex.williamson@redhat.com>,
	Lu Baolu <baolu.lu@linux.intel.com>,
	"Cornelia Huck" <cohuck@redhat.com>,
	David Woodhouse <dwmw2@infradead.org>,
	"iommu@lists.linux-foundation.org"
	<iommu@lists.linux-foundation.org>,
	"Joerg Roedel" <joro@8bytes.org>,
	"kvm@vger.kernel.org" <kvm@vger.kernel.org>,
	Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>,
	Will Deacon <will@kernel.org>
Cc: Christoph Hellwig <hch@lst.de>, Robin Murphy <robin.murphy@arm.com>
Subject: RE: [PATCH v2 4/4] vfio: Require that devices support DMA cache coherence
Date: Fri, 8 Apr 2022 08:26:10 +0000	[thread overview]
Message-ID: <BN9PR11MB52764BF8AC747D4B2E2B8BAA8CE99@BN9PR11MB5276.namprd11.prod.outlook.com> (raw)
In-Reply-To: <4-v2-f090ae795824+6ad-intel_no_snoop_jgg@nvidia.com>

> From: Jason Gunthorpe <jgg@nvidia.com>
> Sent: Thursday, April 7, 2022 11:24 PM
> 
> IOMMU_CACHE means that normal DMAs do not require any additional
> coherency
> mechanism and is the basic uAPI that VFIO exposes to userspace. For
> instance VFIO applications like DPDK will not work if additional coherency
> operations are required.
> 
> Therefore check IOMMU_CAP_CACHE_COHERENCY like vdpa & usnic do
> before
> allowing an IOMMU backed VFIO device to be created.
> 
> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
> ---
>  drivers/vfio/vfio.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
> index a4555014bd1e72..9edad767cfdad3 100644
> --- a/drivers/vfio/vfio.c
> +++ b/drivers/vfio/vfio.c
> @@ -815,6 +815,13 @@ static int __vfio_register_dev(struct vfio_device
> *device,
> 
>  int vfio_register_group_dev(struct vfio_device *device)
>  {
> +	/*
> +	 * VFIO always sets IOMMU_CACHE because we offer no way for
> userspace to
> +	 * restore cache coherency.
> +	 */
> +	if (!iommu_capable(device->dev->bus,
> IOMMU_CAP_CACHE_COHERENCY))
> +		return -EINVAL;
> +

One nit. Is it logistically more reasonable to put this patch before
changing VFIO to always set IOMMU_CACHE?

otherwise,

Reviewed-by: Kevin Tian <kevin.tian@intel.com>