[PATCH 4/5] iommu/arm-smmu-v3: Add set_dirty_tracking() support

[Joao Martins]

On 30/11/2023 12:54, Jason Gunthorpe wrote:
> On Thu, Nov 30, 2023 at 08:56:32AM +0000, Shameerali Kolothum Thodi wrote:
>>> On Tue, Nov 28, 2023 at 09:49:39AM +0000, Shameer Kolothum wrote:
>>>> From: Joao Martins <joao.m.martins at oracle.com>
>>>>
>>>> Dirty tracking will always be enabled with DBM=1 modifier enabled
>>>> by default when HD is supported.
>>>
>>> Is this trying to say that ARM doesn't have a per-table global enable
>>> for dirty tracking but instead pre-sets the DBM bit to avoid the cost?
>>>
>>
>> Yes. SMMUv3 has per-PTE DBM control and I think the initial RFC had
>> it walking the PTEs and setting the DBM on set_dirty_tracking().
> 
> set_dirty_tracking doesn't have access to the necessary locking to
> touch the PTEs.
> 
This is done for free by iopt_clear_dirty_data().

set_dirty_tracking() is mostly a nop, under the assumption that dirty
tracking is always enabled.

>>> So on smmuv3 to enable we have to clear everything and disable
>>> continues to pay a penalty since we don't go and mark all things as
>>> dirty again?
>>
>> Yes we clear everything on enable. Sorry I didn't get the second part.
>> We don't mark dirty on disable. How is that different from Intel/AMD?
> 
> Intel/AMD have a global switch so they just turn off the tracking and
> stop paying the cost.
> 
> This approach on ARM means once the tracking is logically turned off
> the HW will continue to generate memory traffic to set dirty bits on
> DMAs. There is no way to back to the at-start state where their is 0
> memory traffic on DMAs. Not sure that it totally matters, but it is
> worth noting someplace.
> 
> If we do want to solve this then ARM would need iommufd to make a pass
> over the page table to set for disable similar to how we have to clear
> for enable.

The firsts attempt at this had to be dynamic[0].

Like set_dirty_tracking_range would pass over the io pagetable and set the DBM
bit to enable dirty tracking. It was suggested that we switch to an always-on
mode to simplify initial bringup of the feature, and if the always-on was
somehow affecting DMA performance, we would re-attempt at this dynamic mode
post-mortem.

With the current code structure, perhaps having set_dirty_tracking() do the
DBM-enable pass we would need to move iorw_sem section to include the call to
set_dirty_tracking() and smmu op set_dirty_tracking() op would walk the whole
pt, without relying on iopt areas. Or we go back to iterating areas similar to
[0] with a new op.

[0] https://lore.kernel.org/kvm/20220428210933.3583-16-joao.m.martins@oracle.com/