Re: [PATCH v4 2/9] vfio-iommufd: Create iommufd_access for noiommu devices

Yi Liu
2023-04-28T07:03:02Z

On 2023/4/28 15:00, Tian, Kevin wrote:
>> From: Liu, Yi L <yi.l.liu@intel.com>
>> Sent: Friday, April 28, 2023 2:21 PM
>>
>> On 2023/4/28 02:32, Alex Williamson wrote:
>>> On Thu, 27 Apr 2023 06:59:17 +0000
>>> "Liu, Yi L" <yi.l.liu@intel.com> wrote:
>>>
>>>>> From: Tian, Kevin <kevin.tian@intel.com>
>>>>> Sent: Thursday, April 27, 2023 2:39 PM
>>>>>
>>>>>> From: Liu, Yi L <yi.l.liu@intel.com>
>>>>>> Sent: Wednesday, April 26, 2023 10:54 PM
>>>>>> @@ -121,7 +128,8 @@ static void vfio_emulated_unmap(void *data,
>>>>>> unsigned long iova,
>>>>>> {
>>>>>> struct vfio_device *vdev = data;
>>>>>>
>>>>>> - if (vdev->ops->dma_unmap)
>>>>>> + /* noiommu devices cannot do map/unmap */
>>>>>> + if (vdev->noiommu && vdev->ops->dma_unmap)
>>>>>> vdev->ops->dma_unmap(vdev, iova, length);
>>>>>
>>>>> Is it necessary? All mdev devices implementing @dma_unmap won't
>>>>> set noiommu flag.
>>>>
>>>> Hmmm. Yes, and all the devices set noiommu is not implementing
>> @dma_unmap
>>>> as far as I see. Maybe this noiommu check can be removed.
>>>
>>> Not to mention that the polarity of the noiommu test is backwards here!
>>> This also seems to be the only performance path where noiommu is tested
>>> and therefore I believe the only actual justification of the previous
>>> patch.
>>
>> but this patch needs to use vfio_iommufd_emulated_bind() and
>> vfio_iommufd_emulated_unbind() for the noiommu devices when binding
>> to iommufd. So needs to check noiommu in the vfio_iommufd_bind()
>> and vfio_iommu_unbind() as well.
>>
>
> You can continue to use vfio_device_is_noiommu() in this patch. It's not
> big deal to drop it from this series and then add back in cdev series.

yes.:-) patch 01 of this series was added more per the cdev series reviews.

--
Regards,
Yi Liu