On 4/29/22 17:11, Jason Gunthorpe wrote: > On Fri, Apr 29, 2022 at 03:45:23PM +0100, Joao Martins wrote: >> On 4/29/22 13:23, Jason Gunthorpe wrote: >>> On Fri, Apr 29, 2022 at 01:06:06PM +0100, Joao Martins wrote: >>> >>>>> 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. >>> >>> It just means that on SMMU the start tracking op clears all the dirty >>> bits. >>> >> Hmm, OK. But aren't really picking a poison here? On ARM it's the difference >> from switching the setting the DBM bit and put the IOPTE as writeable-clean (which >> is clearing another bit) versus read-and-clear-when-dirty-track-start which means >> we need to re-walk the pagetables to clear one bit. > > Yes, I don't think a iopte walk is avoidable? > Correct -- exactly why I am still more learning towards enable DBM bit only at start versus enabling DBM at domain-creation while clearing dirty at start. >> It's walking over ranges regardless. > > Also, keep in mind start should always come up in a 0 dirties state on > all platforms. So all implementations need to do something to wipe the > dirty state, either explicitly during start or restoring all clean > during stop. > > A common use model might be to just destroy the iommu_domain without > doing stop so prefering the clearing io page table at stop might be a > better overall design. If we want to ensure that the IOPTE dirty state is immutable before start and after stop maybe this behaviour could be a new flag in the set-dirty-tracking (or be implicit as you suggest). but ... hmm, at the same time, I wonder if it's better to let userspace fetch the dirties that were there /right after stopping/ (via GET_DIRTY_IOVA) rather than just discarding them implicitly at SET_DIRTY_TRACKING(0|1).
