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

