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

From: Robin Murphy <robin.murphy@arm.com>
To: Jason Gunthorpe <jgg@nvidia.com>, "Tian, Kevin" <kevin.tian@intel.com>
Cc: "kvm@vger.kernel.org" <kvm@vger.kernel.org>,
	Will Deacon <will@kernel.org>, Cornelia Huck <cohuck@redhat.com>,
	"iommu@lists.linux-foundation.org"
	<iommu@lists.linux-foundation.org>,
	Alex Williamson <alex.williamson@redhat.com>,
	David Woodhouse <dwmw2@infradead.org>,
	Christoph Hellwig <hch@lst.de>
Subject: Re: [PATCH v2 4/4] vfio: Require that devices support DMA cache coherence
Date: Fri, 8 Apr 2022 14:28:02 +0100	[thread overview]
Message-ID: <d6b1c72b-c05e-8bd8-c0cb-38e6c7ccfdb6@arm.com> (raw)
In-Reply-To: <20220408122256.GV2120790@nvidia.com>

On 2022-04-08 13:22, Jason Gunthorpe wrote:
> On Fri, Apr 08, 2022 at 08:26:10AM +0000, Tian, Kevin wrote:
>>> 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
>>> +++ 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?
> 
> For bisectability it has to be after
> 
>      iommu: Redefine IOMMU_CAP_CACHE_COHERENCY as the cap flag for IOMMU_CACHE
> 
> Otherwise Intel iommu will stop working with VFIO
> 
> The ordering is OK as is because no IOMMU that works with VFIO cares
> about IOMMU_CACHE.

The Arm SMMU drivers do (without it even coherent traffic would be 
downgraded to non-cacheable), but then they also handle 
IOMMU_CAP_CACHE_COHERENCY nonsensically, and it happens to work out 
since AFAIK there aren't (yet) any Arm-based systems where you can 
reasonably try to use VFIO that don't also have hardware-coherent PCI. 
Thus I don't think there's any risk of regression for us here.

Robin.
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu