> -----Original Message----- > From: Eric Auger [mailto:eric.au...@redhat.com] > Sent: Wednesday, September 01, 2021 9:14 PM > To: Liu, Renwei; Peter Maydell > Cc: qemu-...@nongnu.org; qemu-devel@nongnu.org; Li, Chunming; Wen, > Jianxian > Subject: Re: [PATCH v2] hw/arm/smmuv3: Simplify range invalidation > > Hi, > > On 9/1/21 8:33 AM, Liu, Renwei wrote: > >> -----Original Message----- > >> From: Eric Auger [mailto:eric.au...@redhat.com] > >> Sent: Tuesday, August 31, 2021 10:46 PM > >> To: Liu, Renwei; Peter Maydell > >> Cc: qemu-...@nongnu.org; qemu-devel@nongnu.org; Li, Chunming; Wen, > >> Jianxian > >> Subject: Re: [PATCH v2] hw/arm/smmuv3: Simplify range invalidation > >> > >> Hi Liu, > >> > >> On 8/23/21 9:50 AM, Liu, Renwei wrote: > >>> Simplify range invalidation which can avoid to iterate over all > >>> iotlb entries multi-times. For instance invalidations patterns like > >>> "invalidate 32 4kB pages starting from 0xffacd000" need to iterate > >> over > >>> all iotlb entries 6 times (num_pages: 1, 2, 16, 8, 4, 1). It only > >> needs > >>> to iterate over all iotlb entries once with new implementation. > >> This wouldn't work. This reverts commit > >> 6d9cd115b9df ("hw/arm/smmuv3: Enforce invalidation on a power of two > >> range") > >> which is mandated for VFIO and virtio to work. IOTLB invalidations > must > >> be naturally aligned and with a power of 2 range, hence this > iteration. > >> > >> Thanks > >> > >> Eric > > Hi Eric, > > > > Could you try the patch firstly? I want to know whether it's failed > > in your application scenario with this implementation. > There are many test cases, virtio-pci, vhost, VFIO, ... > > I agree with you that IOTLB entry must be naturally aligned and > > with a power of 2 range. But we can invalidate multi IOTLB entries > > in one iteration. We check the overlap between invalidation range > > and IOTLB range, not check mask. > This smmu_hash_remove_by_asid_iova() change only affects the internal > SMMUv3 IOTLB hash table lookup. However there are also IOTLB > invalidation notifications sent to components who registered notifiers, > ie. smmuv3_notify_iova path. > > The final result is same with > > your implementation (split to multi times with a power of 2 range). > > I wonder why we can't implement it directly when the application can > > send an invalidation command with a non power of 2 range. > > We have tested it in our application scenario and not find any fail. > Assume the driver invalidates 5 * 4kB pages =0x5000 range. Without the > loop you removed > > in smmuv3_notify_iova() event.entry.addr_mask = num_pages * (1 << > granule) - 1 = 0x4FFF. This addr_mask is an invalid mask > this entry is passed to components who registered invalidation > notifiers > such as vhost or vfio. for instance in VFIO you have '&' ops on the > addr_mask. > addr_mask is expected to be a mask of a power of 2 range. > > Does it clarify? > > Thanks > > Eric Hi Eric
Got it, thanks a lot for your clarification. I don't consider the further notifier from vhost or vfio indeed, because they are not registered in our application scenario. Let's keep the previous implementation and ignore this patch. Thanks Renwei Liu > > > > In addition, from the code implementation, smmu_iotlb_inv_iova() > > should be OK. In another call smmuv3_inv_notifiers_iova() -> > > smmuv3_notify_iova() -> memory_region_notify_iommu_one(), > > it also checks range overlap. So it should be OK if the range > > is not a power of 2. > > > > Could you take a look at it again? > > > > Thanks > > Renwei Liu
