Hi Yi, On 2021/3/16 17:17, Yi Sun wrote: > On 21-03-10 17:06:07, Keqian Zhu wrote: >> From: jiangkunkun <jiangkun...@huawei.com> >> >> Block descriptor is not a proper granule for dirty log tracking. >> Take an extreme example, if DMA writes one byte, under 1G mapping, >> the dirty amount reported to userspace is 1G, but under 4K mapping, >> the dirty amount is just 4K. >> >> This adds a new interface named start_dirty_log in iommu layer and >> arm smmuv3 implements it, which splits block descriptor to an span >> of page descriptors. Other types of IOMMU will perform architecture >> specific actions to start dirty log. >> >> To allow code reuse, the split_block operation is realized as an >> iommu_ops too. We flush all iotlbs after the whole procedure is >> completed to ease the pressure of iommu, as we will hanle a huge >> range of mapping in general. >> >> Spliting block does not simultaneously work with other pgtable ops, >> as the only designed user is vfio, which always hold a lock, so race >> condition is not considered in the pgtable ops. >> >> Co-developed-by: Keqian Zhu <zhukeqi...@huawei.com> >> Signed-off-by: Kunkun Jiang <jiangkun...@huawei.com> >> --- >> >> changelog: >> >> v2: >> - Change the return type of split_block(). size_t -> int. >> - Change commit message to properly describe race condition. (Robin) >> - Change commit message to properly describe the need of split block. >> - Add a new interface named start_dirty_log(). (Sun Yi) >> - Change commit message to explain the realtionship of split_block() and >> start_dirty_log(). >> >> --- >> drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 52 +++++++++ >> drivers/iommu/io-pgtable-arm.c | 122 ++++++++++++++++++++ >> drivers/iommu/iommu.c | 48 ++++++++ >> include/linux/io-pgtable.h | 2 + >> include/linux/iommu.h | 24 ++++ >> 5 files changed, 248 insertions(+) >> > Could you please split iommu common interface to a separate patch? > This may make review and comments easier. Yup, good suggestion.
> > IMHO, I think the start/stop interfaces could be merged into one, e.g: > int iommu_domain_set_hwdbm(struct iommu_domain *domain, bool enable, > unsigned long iova, size_t size, > int prot); Looks good, this reduces some code. but I have a concern that this causes loss of flexibility, as we must pass same arguments when start|stop dirty log. What's your opinion about this? > > Same comments to patch 5. OK. Thanks. > > BRs, > Yi Sun > >> -- >> 2.19.1 > . Thanks, Keqian