Re: [PATCH RFC 15/19] iommu/arm-smmu-v3: Add set_dirty_tracking_range() support

Joao Martins
2022-04-29T12:06:56Z

On 4/29/22 12:19, Robin Murphy wrote:
> On 2022-04-29 12:05, Joao Martins wrote:
>> On 4/29/22 09:28, Tian, Kevin wrote:
>>>> From: Joao Martins <joao.m.martins@oracle.com>
>>>> Sent: Friday, April 29, 2022 5:09 AM
>>>>
>>>> Similar to .read_and_clear_dirty() use the page table
>>>> walker helper functions and set DBM|RDONLY bit, thus
>>>> switching the IOPTE to writeable-clean.
>>>
>>> this should not be one-off if the operation needs to be
>>> applied to IOPTE. Say a map request comes right after
>>> set_dirty_tracking() is called. If it's agreed to remove
>>> the range op then smmu driver should record the tracking
>>> status internally and then apply the modifier to all the new
>>> mappings automatically before dirty tracking is disabled.
>>> Otherwise the same logic needs to be kept in iommufd to
>>> call set_dirty_tracking_range() explicitly for every new
>>> iopt_area created within the tracking window.
>>
>> Gah, I totally missed that by mistake. New mappings aren't
>> carrying over the "DBM is set". This needs a new io-pgtable
>> quirk added post dirty-tracking toggling.
>>
>> I can adjust, but I am at odds on including this in a future
>> iteration given that I can't really test any of this stuff.
>> Might drop the driver until I have hardware/emulation I can
>> use (or maybe others can take over this). It was included
>> for revising the iommu core ops and whether iommufd was
>> affected by it.
>>
>> I'll delete the range op, and let smmu v3 driver walk its
>> own IO pgtables.
>
> TBH I'd be inclined to just enable DBM unconditionally in
> arm_smmu_domain_finalise() if the SMMU supports it. Trying to toggle it
> dynamically (especially on a live domain) seems more trouble that it's
> worth.

Hmmm, but then it would strip userland/VMM from any sort of control (contrary
to what we can do on the CPU/KVM side). e.g. the first time you do
GET_DIRTY_IOVA it would return all dirtied IOVAs since the beginning
of guest time, as opposed to those only after you enabled dirty-tracking.

We do add the TCR values unconditionally if supported, but not
the actual dirty tracking.