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

Robin Murphy
2022-04-08T13:28:13Z

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.