On 4/29/22 20:20, Robin Murphy wrote: > On 2022-04-29 17:40, Joao Martins wrote: >> 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. > > I'd say it's largely down to whether you want the bother of > communicating a dynamic behaviour change into io-pgtable. The big > advantage of having it just use DBM all the time is that you don't have > to do that, and the "start tracking" operation is then nothing more than > a normal "read and clear" operation but ignoring the read result. > > At this point I'd much rather opt for simplicity, and leave the fancier > stuff to revisit later if and when somebody does demonstrate a > significant overhead from using DBM when not strictly needed. > OK -- I did get the code simplicity part[*]. Albeit my concern is that last point: if there's anything fundamentally affecting DMA performance then any SMMU user would see it even if they don't care at all about DBM (i.e. regular baremetal/non-vm iommu usage). [*] It was how I had this initially PoC-ed. And really all IOMMU drivers dirty tracking could be simplified to be always-enabled, and start/stop is essentially flushing/clearing dirties. Albeit I like that this is only really used (by hardware) when needed and any other DMA user isn't affected.
