Re: [PATCH v3 07/19] iommufd: Dirty tracking data support

Joao Martins
2023-10-17T15:52:02Z

On 17/10/2023 16:29, Jason Gunthorpe wrote:
> On Tue, Oct 17, 2023 at 01:06:12PM +0100, Joao Martins wrote:
>> On 23/09/2023 02:40, Joao Martins wrote:
>>> On 23/09/2023 02:24, Joao Martins wrote:
>>>> +int iopt_read_and_clear_dirty_data(struct io_pagetable *iopt,
>>>> + struct iommu_domain *domain,
>>>> + unsigned long flags,
>>>> + struct iommufd_dirty_data *bitmap)
>>>> +{
>>>> + unsigned long last_iova, iova = bitmap->iova;
>>>> + unsigned long length = bitmap->length;
>>>> + int ret = -EOPNOTSUPP;
>>>> +
>>>> + if ((iova & (iopt->iova_alignment - 1)))
>>>> + return -EINVAL;
>>>> +
>>>> + if (check_add_overflow(iova, length - 1, &last_iova))
>>>> + return -EOVERFLOW;
>>>> +
>>>> + down_read(&iopt->iova_rwsem);
>>>> + ret = iommu_read_and_clear_dirty(domain, flags, bitmap);
>>>> + up_read(&iopt->iova_rwsem);
>>>> + return ret;
>>>> +}
>>>
>>> I need to call out that a mistake I made, noticed while submitting. I should be
>>> walk over iopt_areas here (or in iommu_read_and_clear_dirty()) to check
>>> area::pages. So this is a comment I have to fix for next version.
>>
>> Below is how I fixed it.
>>
>> Essentially the thinking being that the user passes either an mapped IOVA area
>> it mapped *or* a subset of a mapped IOVA area. This should also allow the
>> possibility of having multiple threads read dirties from huge IOVA area splitted
>> in different chunks (in the case it gets splitted into lowest level).
>
> What happens if the iommu_read_and_clear_dirty is done on unmapped
> PTEs? It fails?

If there's no IOPTE or the IOPTE is non-present, it keeps walking to the next
base page (or level-0 IOVA range). For both drivers in this series.

Joao