On Tue, Mar 30, 2021 at 10:04:53AM +0100, Robin Murphy wrote: > On 2021-03-30 02:22, chenxiang (M) wrote: > > Hi Will, > > > > > > 在 2021/3/29 22:45, Will Deacon 写道: > > > On Sat, Mar 27, 2021 at 02:23:10PM +0800, chenxiang wrote: > > > > From: Xiang Chen <[email protected]> > > > > > > > > Currently it will send a iotlb sync at end of iommu unmap even if > > > > iotlb_gather is not valid (iotlb_gather->pgsize = 0). Actually it is not > > > > necessary, so add a check to avoid invalid iotlb sync. > > > > > > > > Signed-off-by: Xiang Chen <[email protected]> > > > > --- > > > > include/linux/iommu.h | 3 +++ > > > > 1 file changed, 3 insertions(+) > > > > > > > > diff --git a/include/linux/iommu.h b/include/linux/iommu.h > > > > index 9ca6e6b..6afa61b 100644 > > > > --- a/include/linux/iommu.h > > > > +++ b/include/linux/iommu.h > > > > @@ -529,6 +529,9 @@ static inline void > > > > iommu_flush_iotlb_all(struct iommu_domain *domain) > > > > static inline void iommu_iotlb_sync(struct iommu_domain *domain, > > > > struct iommu_iotlb_gather *iotlb_gather) > > > > { > > > > + if (!iotlb_gather->pgsize) > > > > + return; > > > In which circumstances does this occur? > > > > > > Will > > > > When iommu_unmap, it initializes iotlb_gather (then iotlb_gather->pgsize > > = 0). > > If the unmap size = ARM_LPAE_BLOCK_SIZE(lvl) && iopte_leaf(), it will > > fill the iotlb_gather (set iotlb_gather->pgsize = granule); > > but if the unmap size = ARM_LPAE_BLOCK_SIZE(lvl) && !iopte_leaf() (tlb > > flush walk situation), iotlb_gather is not filled (iotlb_gather->pgsize > > = 0), > > it will still send iommu_iotlb_sync(domain, &iotlb_gather) which is > > actually invalid iotlb sync at last. > > I guess this can happen in DMA API usage if we've previously mapped a > block's worth of scatterlist pages into a block-=sized IOVA region, but this > is not the place to do anything about it. The main thing this patch will do > is break all the other drivers that implement .iotlb_sync but do not use > iotlb_gather. > > By all means optimise SMMUv3's sync behaviour, but the only valid place to > do that is in SMMUv3's own sync callback. These particular details are > beyond what the IOMMU core knows about.
Yes, that's where I was heading. Chenxiang, please could you send a v2 with the check inside arm_smmu_iotlb_sync() instead? Thanks, Will _______________________________________________ iommu mailing list [email protected] https://lists.linuxfoundation.org/mailman/listinfo/iommu
