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

Alexey Kardashevskiy
2022-07-01T06:24:47Z

On 7/1/22 16:07, Tian, Kevin wrote:
>> From: Alexey Kardashevskiy <aik@ozlabs.ru>
>> Sent: Friday, July 1, 2022 12:58 PM
>>
>> On 4/8/22 01:23, Jason Gunthorpe via iommu wrote:
>>> 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.
>>
>>
>> This just broke VFIO on POWER which does not use iommu_ops.
>
> In this case below check is more reasonable to be put in type1
> attach_group(). Do a iommu_group_for_each_dev() to verify
> CACHE_COHERENCY similar to what Robin did for INTR_REMAP.

ah makes sense. I've posted an RFC with another problem - "[RFC PATCH
kernel] vfio: Skip checking for IOMMU_CAP_CACHE_COHERENCY on POWER and
more", would be great if both addressed, or I try moving those next week
:) Thanks,

>
> (sorry no access to my build machine now but I suppose Jason
> can soon work out a fix once he sees this. 😊)
>
>>
>>
>>>
>>> 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;
>>> +
>>> return __vfio_register_dev(device,
>>> vfio_group_find_or_alloc(device->dev));
>>> }
>>
>> --
>> Alexey

--
Alexey