Hi Zenghui, On 4/1/21 8:11 AM, Zenghui Yu wrote: > Hi Eric, > > On 2021/2/24 4:56, Eric Auger wrote: >> +static int >> +arm_smmu_cache_invalidate(struct iommu_domain *domain, struct device >> *dev, >> + struct iommu_cache_invalidate_info *inv_info) >> +{ >> + struct arm_smmu_cmdq_ent cmd = {.opcode = CMDQ_OP_TLBI_NSNH_ALL}; >> + struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain); >> + struct arm_smmu_device *smmu = smmu_domain->smmu; >> + >> + if (smmu_domain->stage != ARM_SMMU_DOMAIN_NESTED) >> + return -EINVAL; >> + >> + if (!smmu) >> + return -EINVAL; >> + >> + if (inv_info->version != IOMMU_CACHE_INVALIDATE_INFO_VERSION_1) >> + return -EINVAL; >> + >> + if (inv_info->cache & IOMMU_CACHE_INV_TYPE_PASID || > > I didn't find any code where we would emulate the CFGI_CD{_ALL} commands > for guest and invalidate the stale CD entries on the physical side. Is > PASID-cache type designed for that effect? Yes it is. PASID-cache matches the CD table. > >> + inv_info->cache & IOMMU_CACHE_INV_TYPE_DEV_IOTLB) { >> + return -ENOENT; >> + } >> + >> + if (!(inv_info->cache & IOMMU_CACHE_INV_TYPE_IOTLB)) >> + return -EINVAL; >> + >> + /* IOTLB invalidation */ >> + >> + switch (inv_info->granularity) { >> + case IOMMU_INV_GRANU_PASID: >> + { >> + struct iommu_inv_pasid_info *info = >> + &inv_info->granu.pasid_info; >> + >> + if (info->flags & IOMMU_INV_ADDR_FLAGS_PASID) >> + return -ENOENT; >> + if (!(info->flags & IOMMU_INV_PASID_FLAGS_ARCHID)) >> + return -EINVAL; >> + >> + __arm_smmu_tlb_inv_context(smmu_domain, info->archid); >> + return 0; >> + } >> + case IOMMU_INV_GRANU_ADDR: >> + { >> + struct iommu_inv_addr_info *info = &inv_info->granu.addr_info; >> + size_t size = info->nb_granules * info->granule_size; >> + bool leaf = info->flags & IOMMU_INV_ADDR_FLAGS_LEAF; >> + >> + if (info->flags & IOMMU_INV_ADDR_FLAGS_PASID) >> + return -ENOENT; >> + >> + if (!(info->flags & IOMMU_INV_ADDR_FLAGS_ARCHID)) >> + break; >> + >> + arm_smmu_tlb_inv_range_domain(info->addr, size, >> + info->granule_size, leaf, >> + info->archid, smmu_domain); >> + >> + arm_smmu_cmdq_issue_sync(smmu); > > There is no need to issue one more SYNC. Hum yes I did not notice it was made by the arm_smmu_cmdq_issue_cmdlist() Thanks! Eric >