On 21-03-16 19:39:47, Keqian Zhu wrote: > 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? > Per current design, start/stop interfaces have similar arguments. So I think it is ok for now. For future extension, we may think to define a structure to pass these arguments.
> > > > Same comments to patch 5. > OK. Thanks. > > > > > BRs, > > Yi Sun > > > >> -- > >> 2.19.1 > > . > Thanks, > Keqian BRs, Yi Sun _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
