Messages in this thread | | | Subject | Re: [PATCH v4 05/22] iommu: introduce iommu invalidate API function | From | Jean-Philippe Brucker <> | Date | Tue, 8 May 2018 11:35:00 +0100 |
| |
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"?
> * 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.
> * > * 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.
> * 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
Thanks, Jean
| |