Messages in this thread | | | Date | Wed, 9 May 2018 05:55:30 -0700 | From | Jacob Pan <> | Subject | Re: [PATCH v4 05/22] iommu: introduce iommu invalidate API function |
| |
On Tue, 8 May 2018 11:35:00 +0100 Jean-Philippe Brucker <jean-philippe.brucker@arm.com> wrote:
> Hi Jacob, > > Looks mostly good to me, I just have a couple more comments > > On 04/05/18 19:07, Jacob Pan wrote: > > Now the passdown invalidation granularities look like: > > (sorted by coarseness), will send out in v5 patchset soon if no > > issues. > > > > /** > > * enum iommu_inv_granularity - Generic invalidation granularity > > * > > * @IOMMU_INV_GRANU_DOMAIN: Device context cache > > associated with a > > * domain ID. > > * @IOMMU_INV_GRANU_DEVICE: Device context cache > > associated with a > > * device ID > > * @IOMMU_INV_GRANU_DOMAIN_ALL_PASID: TLB entries or PASID > > caches of all > > * PASIDs associated with a > > domain ID > > * @IOMMU_INV_GRANU_PASID_SEL: TLB entries or PASID > > cache associated > > * with a PASID and a domain > > * @IOMMU_INV_GRANU_PAGE_PASID: TLB entries of > > selected page range > > * within a PASID > > * > > * When an invalidation request is passed down to IOMMU to flush > > translation > > * caches, it may carry different granularity levels, which can be > > specific > > * to certain types of translation caches. For an example, PASID > > selective > > * granularity is only applicable PASID cache and IOTLB > > invalidation but for > > * device context caches. > > Should it be "PASID selective granularity is only applicable to PASID > cache and IOTLB but not device context caches"? > right, thanks! > > * This enum is a collection of granularities for all types of > > translation > > * caches. The idea is to make it easy for IOMMU model specific > > driver to > > * convert from generic to model specific value. Not all > > combinations between > > * translation caches and granularity levels are valid. Each IOMMU > > driver > > * can enforce check based on its own conversion table. The > > conversion is > > * based on 2D look-up with inputs as follows: > > * - translation cache types > > * - granularity > > * No global granularity is allowed in that passdown invalidation > > for an > > * assigned device should only impact the device or domain itself. > > That last sentence is a bit confusing, because "global granularity" > might also refer to the "global" TLB flag which is allowed. In my > opinion you can leave this rationale out, I doubt userspace will ever > demand a mechanism for global invalidation. > yes, i can leave the last sentence out. > > * > > * type | DTLB | TLB | PASID | CONTEXT > > * granule | | | | > > * -----------------+-----------+-----------+-----------+----------- > > * DOMAIN | | | | Y > > * DEVICE | | | | Y > > I can't really see a use-case for DOMAIN and DEVICE. It might make > more sense to keep only DN_ALL_PASID, which would then also > invalidate the device context cache. But since they will be very rare > events, factoring them doesn't seem important. > ok. we have no use for now either, was there for completeness. i will remove for now. > > * DN_ALL_PASID | Y | Y | Y | > > * PASID_SEL | Y | Y | Y | > > * PAGE_PASID | | Y | | > > Why not allow PAGE_PASID+DTLB? We need a way to invalidate individual > DTLB entries > I was thinking PAGE_PASID+TLB includes PAGE_PASID+DTLB, but you are right, DTLB should be a 'Y' here. > Thanks, > Jean
[Jacob Pan]
| |