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@xxxxxxxxxx> >>>> 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.