[PATCH v3 2/4] iommu/io-pgtable-arm: Add read_and_clear_dirty() support

[Joao Martins]

On 27/05/2024 02:21, Tian, Kevin wrote:
>> From: Jason Gunthorpe <jgg at nvidia.com>
>> Sent: Friday, May 24, 2024 10:08 PM
>>
>> On Fri, May 24, 2024 at 12:30:22PM +0100, Joao Martins wrote:
>>> On 23/05/2024 04:30, Tian, Kevin wrote:
>>>>> From: Jason Gunthorpe <jgg at nvidia.com>
>>>>> Sent: Thursday, May 23, 2024 1:51 AM
>>>>>
>>>>> diff --git a/drivers/iommu/iommufd/iommufd_private.h
>>>>> b/drivers/iommu/iommufd/iommufd_private.h
>>>>> index 991f864d1f9bc1..de3761e15cab54 100644
>>>>> --- a/drivers/iommu/iommufd/iommufd_private.h
>>>>> +++ b/drivers/iommu/iommufd/iommufd_private.h
>>>>> @@ -52,6 +52,7 @@ struct io_pagetable {
>>>>>  	/* IOVA that cannot be allocated, struct iopt_reserved */
>>>>>  	struct rb_root_cached reserved_itree;
>>>>>  	u8 disable_large_pages;
>>>>> +	u8 dirty_tracking_enabled;
>>>>>  	unsigned long iova_alignment;
>>>>>  };
>>>>>
>>>>
>>>> should it be a hwpt flag instead?
>>>>
>>>
>>> Most of this deals with iopt locks and walking iopt areas to clear dirty. So
>>> this being a iopt attribute looks cleaner in implementation. But I think I see
>>> your point suggestion considering it represents a iommu domain property.
>>
>> Yeah, the original idea of the hwpt/iopt split was to keep code that
>> was principly iommu_domain related away from the code which was
>> principally attachment and lifetime related. Since this value is
>> covered by iopt locks it makes sense in this struct.
>>
>> Not sure the split will stand the test of time as we keep finding
>> reasons to muddle the boundary :\
>>
> 
> emmm there could be multiple domains under a iopt while the dirty
> tracking toggling must be done per domain.
> 
> with a iopt flag like this only the 1st domain under the iopt will be
> properly configured?
> 

Using intel as the reference given that it's the only one that has that
implemented: so far seems to be the other way around i.e. a parent domain sets
its childs' DPT in the iommu driver?

	Joao