From 5d62bacc059bb4f783e1d2ad88874abb6056f404 Mon Sep 17 00:00:00 2001 From: Zhen Lei Date: Fri, 21 Apr 2023 15:24:21 +0800 Subject: [PATCH 01/50] iommu/iova: Optimize iova_magazine_alloc() Only the member 'size' needs to be initialized to 0. Clearing the array pfns[], which is about 1 KiB in size, not only wastes time, but also causes cache pollution. Acked-by: Robin Murphy Signed-off-by: Zhen Lei Link: https://lore.kernel.org/r/20230421072422.869-1-thunder.leizhen@huawei.com Signed-off-by: Joerg Roedel --- drivers/iommu/iova.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/drivers/iommu/iova.c b/drivers/iommu/iova.c index fe452ce46642..10b964600948 100644 --- a/drivers/iommu/iova.c +++ b/drivers/iommu/iova.c @@ -647,7 +647,13 @@ struct iova_rcache { static struct iova_magazine *iova_magazine_alloc(gfp_t flags) { - return kzalloc(sizeof(struct iova_magazine), flags); + struct iova_magazine *mag; + + mag = kmalloc(sizeof(*mag), flags); + if (mag) + mag->size = 0; + + return mag; } static void iova_magazine_free(struct iova_magazine *mag) From 354440a7618746096cca4a2254594c00c86dc597 Mon Sep 17 00:00:00 2001 From: Jerry Snitselaar Date: Thu, 20 Apr 2023 01:07:18 -0700 Subject: [PATCH 02/50] iommu/amd: Use page mode macros in fetch_pte() Use the page mode macros instead of magic numbers in fetch_pte. Cc: Robin Murphy Cc: Will Deacon Cc: Suravee Suthikulpanit Cc: Joerg Roedel Signed-off-by: Jerry Snitselaar Reviewed-by: Vasant Hegde Link: https://lore.kernel.org/r/20230420080718.523132-1-jsnitsel@redhat.com Signed-off-by: Joerg Roedel --- drivers/iommu/amd/io_pgtable.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/iommu/amd/io_pgtable.c b/drivers/iommu/amd/io_pgtable.c index 1b67116882be..2892aa1b4dc1 100644 --- a/drivers/iommu/amd/io_pgtable.c +++ b/drivers/iommu/amd/io_pgtable.c @@ -310,8 +310,8 @@ static u64 *fetch_pte(struct amd_io_pgtable *pgtable, return NULL; /* Large PTE */ - if (PM_PTE_LEVEL(*pte) == 7 || - PM_PTE_LEVEL(*pte) == 0) + if (PM_PTE_LEVEL(*pte) == PAGE_MODE_7_LEVEL || + PM_PTE_LEVEL(*pte) == PAGE_MODE_NONE) break; /* No level skipping support yet */ From 75a616168b7810c30aa26819153c64df43bc9d9e Mon Sep 17 00:00:00 2001 From: Carlos Bilbao Date: Thu, 20 Apr 2023 17:30:06 +0000 Subject: [PATCH 03/50] iommu/amd: Update copyright notice The most recent changes to AMD'S IOMMU, such as level 5 guest page table support date to the year 2023. Update copyright statement accordingly. Signed-off-by: Carlos Bilbao Link: https://lore.kernel.org/r/20230420173006.3100682-1-carlos.bilbao@amd.com Signed-off-by: Joerg Roedel --- drivers/iommu/amd/io_pgtable_v2.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/iommu/amd/io_pgtable_v2.c b/drivers/iommu/amd/io_pgtable_v2.c index 27c3015947e6..e9ef2e0a62f6 100644 --- a/drivers/iommu/amd/io_pgtable_v2.c +++ b/drivers/iommu/amd/io_pgtable_v2.c @@ -2,7 +2,7 @@ /* * CPU-agnostic AMD IO page table v2 allocator. * - * Copyright (C) 2022 Advanced Micro Devices, Inc. + * Copyright (C) 2022, 2023 Advanced Micro Devices, Inc. * Author: Suravee Suthikulpanit * Author: Vasant Hegde */ From 4a20ce0ff68eb6fc4b1e8f25139c93b312f21229 Mon Sep 17 00:00:00 2001 From: Robin Murphy Date: Thu, 4 May 2023 22:10:55 +0100 Subject: [PATCH 04/50] iommu: Add a capability for flush queue support Passing a special type to domain_alloc to indirectly query whether flush queues are a worthwhile optimisation with the given driver is a bit clunky, and looking increasingly anachronistic. Let's put that into an explicit capability instead. Signed-off-by: Robin Murphy Reviewed-by: Lu Baolu Tested-by: Jerry Snitselaar # amd, intel, smmu-v3 Reviewed-by: Jerry Snitselaar Reviewed-by: Jason Gunthorpe Link: https://lore.kernel.org/r/f0086a93dbccb92622e1ace775846d81c1c4b174.1683233867.git.robin.murphy@arm.com Signed-off-by: Joerg Roedel --- drivers/iommu/amd/iommu.c | 2 ++ drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 1 + drivers/iommu/arm/arm-smmu/arm-smmu.c | 1 + drivers/iommu/intel/iommu.c | 1 + include/linux/iommu.h | 5 +++++ 5 files changed, 10 insertions(+) diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c index 4a314647d1f7..9b7bd6bed664 100644 --- a/drivers/iommu/amd/iommu.c +++ b/drivers/iommu/amd/iommu.c @@ -2293,6 +2293,8 @@ static bool amd_iommu_capable(struct device *dev, enum iommu_cap cap) return amdr_ivrs_remap_support; case IOMMU_CAP_ENFORCE_CACHE_COHERENCY: return true; + case IOMMU_CAP_DEFERRED_FLUSH: + return true; default: break; } diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c index 3fd83fb75722..6d65a7e81df4 100644 --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c @@ -2008,6 +2008,7 @@ static bool arm_smmu_capable(struct device *dev, enum iommu_cap cap) /* Assume that a coherent TCU implies coherent TBUs */ return master->smmu->features & ARM_SMMU_FEAT_COHERENCY; case IOMMU_CAP_NOEXEC: + case IOMMU_CAP_DEFERRED_FLUSH: return true; default: return false; diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c b/drivers/iommu/arm/arm-smmu/arm-smmu.c index 6e0813b26fb6..7f4ee365912c 100644 --- a/drivers/iommu/arm/arm-smmu/arm-smmu.c +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c @@ -1325,6 +1325,7 @@ static bool arm_smmu_capable(struct device *dev, enum iommu_cap cap) return cfg->smmu->features & ARM_SMMU_FEAT_COHERENT_WALK || device_get_dma_attr(dev) == DEV_DMA_COHERENT; case IOMMU_CAP_NOEXEC: + case IOMMU_CAP_DEFERRED_FLUSH: return true; default: return false; diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c index b871a6afd803..ff923298f8ed 100644 --- a/drivers/iommu/intel/iommu.c +++ b/drivers/iommu/intel/iommu.c @@ -4369,6 +4369,7 @@ static bool intel_iommu_capable(struct device *dev, enum iommu_cap cap) switch (cap) { case IOMMU_CAP_CACHE_COHERENCY: + case IOMMU_CAP_DEFERRED_FLUSH: return true; case IOMMU_CAP_PRE_BOOT_PROTECTION: return dmar_platform_optin(); diff --git a/include/linux/iommu.h b/include/linux/iommu.h index e8c9a7da1060..1b7180d6edae 100644 --- a/include/linux/iommu.h +++ b/include/linux/iommu.h @@ -127,6 +127,11 @@ enum iommu_cap { * this device. */ IOMMU_CAP_ENFORCE_CACHE_COHERENCY, + /* + * IOMMU driver does not issue TLB maintenance during .unmap, so can + * usefully support the non-strict DMA flush queue. + */ + IOMMU_CAP_DEFERRED_FLUSH, }; /* These are the possible reserved region types */ From a4fdd976227240b06ced89b5df88a1a1f388f092 Mon Sep 17 00:00:00 2001 From: Robin Murphy Date: Thu, 4 May 2023 22:10:56 +0100 Subject: [PATCH 05/50] iommu: Use flush queue capability It remains really handy to have distinct DMA domain types within core code for the sake of default domain policy selection, but we can now hide that detail from drivers by using the new capability instead. Signed-off-by: Robin Murphy Tested-by: Jerry Snitselaar # amd, intel, smmu-v3 Reviewed-by: Jerry Snitselaar Reviewed-by: Jason Gunthorpe Link: https://lore.kernel.org/r/1c552d99e8ba452bdac48209fa74c0bdd52fd9d9.1683233867.git.robin.murphy@arm.com Signed-off-by: Joerg Roedel --- drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 1 - drivers/iommu/arm/arm-smmu/arm-smmu.c | 3 +-- drivers/iommu/dma-iommu.c | 3 ++- drivers/iommu/intel/iommu.c | 1 - drivers/iommu/iommu.c | 3 ++- include/linux/iommu.h | 1 + 6 files changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c index 6d65a7e81df4..1ed9c4ed5db9 100644 --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c @@ -2024,7 +2024,6 @@ static struct iommu_domain *arm_smmu_domain_alloc(unsigned type) if (type != IOMMU_DOMAIN_UNMANAGED && type != IOMMU_DOMAIN_DMA && - type != IOMMU_DOMAIN_DMA_FQ && type != IOMMU_DOMAIN_IDENTITY) return NULL; diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c b/drivers/iommu/arm/arm-smmu/arm-smmu.c index 7f4ee365912c..a86acd76c1df 100644 --- a/drivers/iommu/arm/arm-smmu/arm-smmu.c +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c @@ -856,8 +856,7 @@ static struct iommu_domain *arm_smmu_domain_alloc(unsigned type) struct arm_smmu_domain *smmu_domain; if (type != IOMMU_DOMAIN_UNMANAGED && type != IOMMU_DOMAIN_IDENTITY) { - if (using_legacy_binding || - (type != IOMMU_DOMAIN_DMA && type != IOMMU_DOMAIN_DMA_FQ)) + if (using_legacy_binding || type != IOMMU_DOMAIN_DMA) return NULL; } /* diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c index 7a9f0b0bddbd..c4bdd2587daf 100644 --- a/drivers/iommu/dma-iommu.c +++ b/drivers/iommu/dma-iommu.c @@ -586,7 +586,8 @@ static int iommu_dma_init_domain(struct iommu_domain *domain, dma_addr_t base, goto done_unlock; /* If the FQ fails we can simply fall back to strict mode */ - if (domain->type == IOMMU_DOMAIN_DMA_FQ && iommu_dma_init_fq(domain)) + if (domain->type == IOMMU_DOMAIN_DMA_FQ && + (!device_iommu_capable(dev, IOMMU_CAP_DEFERRED_FLUSH) || iommu_dma_init_fq(domain))) domain->type = IOMMU_DOMAIN_DMA; ret = iova_reserve_iommu_regions(dev, domain); diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c index ff923298f8ed..8096273b034c 100644 --- a/drivers/iommu/intel/iommu.c +++ b/drivers/iommu/intel/iommu.c @@ -4064,7 +4064,6 @@ static struct iommu_domain *intel_iommu_domain_alloc(unsigned type) case IOMMU_DOMAIN_BLOCKED: return &blocking_domain; case IOMMU_DOMAIN_DMA: - case IOMMU_DOMAIN_DMA_FQ: case IOMMU_DOMAIN_UNMANAGED: dmar_domain = alloc_domain(type); if (!dmar_domain) { diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c index f1dcfa3f1a1b..7078bf4a8ec8 100644 --- a/drivers/iommu/iommu.c +++ b/drivers/iommu/iommu.c @@ -1980,11 +1980,12 @@ static struct iommu_domain *__iommu_domain_alloc(const struct bus_type *bus, unsigned type) { struct iommu_domain *domain; + unsigned int alloc_type = type & IOMMU_DOMAIN_ALLOC_FLAGS; if (bus == NULL || bus->iommu_ops == NULL) return NULL; - domain = bus->iommu_ops->domain_alloc(type); + domain = bus->iommu_ops->domain_alloc(alloc_type); if (!domain) return NULL; diff --git a/include/linux/iommu.h b/include/linux/iommu.h index 1b7180d6edae..d31642596675 100644 --- a/include/linux/iommu.h +++ b/include/linux/iommu.h @@ -65,6 +65,7 @@ struct iommu_domain_geometry { #define __IOMMU_DOMAIN_SVA (1U << 4) /* Shared process address space */ +#define IOMMU_DOMAIN_ALLOC_FLAGS ~__IOMMU_DOMAIN_DMA_FQ /* * This are the possible domain-types * From 32261d10943b7fffa864f9a532e2b40a813df79b Mon Sep 17 00:00:00 2001 From: Florian Fainelli Date: Tue, 9 May 2023 12:10:48 -0700 Subject: [PATCH 06/50] iommu: Suppress empty whitespaces in prints If IOMMU_CMD_LINE_DMA_API or IOMMU_CMD_LINE_STRICT are not set in iommu_cmd_line, we will be emitting a whitespace before the newline. Signed-off-by: Florian Fainelli Reviewed-by: Jason Gunthorpe Link: https://lore.kernel.org/r/20230509191049.1752259-1-f.fainelli@gmail.com Signed-off-by: Joerg Roedel --- drivers/iommu/iommu.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c index 7078bf4a8ec8..10eb24d2e55a 100644 --- a/drivers/iommu/iommu.c +++ b/drivers/iommu/iommu.c @@ -176,16 +176,16 @@ static int __init iommu_subsys_init(void) if (!iommu_default_passthrough() && !iommu_dma_strict) iommu_def_domain_type = IOMMU_DOMAIN_DMA_FQ; - pr_info("Default domain type: %s %s\n", + pr_info("Default domain type: %s%s\n", iommu_domain_type_str(iommu_def_domain_type), (iommu_cmd_line & IOMMU_CMD_LINE_DMA_API) ? - "(set via kernel command line)" : ""); + " (set via kernel command line)" : ""); if (!iommu_default_passthrough()) - pr_info("DMA domain TLB invalidation policy: %s mode %s\n", + pr_info("DMA domain TLB invalidation policy: %s mode%s\n", iommu_dma_strict ? "strict" : "lazy", (iommu_cmd_line & IOMMU_CMD_LINE_STRICT) ? - "(set via kernel command line)" : ""); + " (set via kernel command line)" : ""); nb = kcalloc(ARRAY_SIZE(iommu_buses), sizeof(*nb), GFP_KERNEL); if (!nb) From 4db0e5f8875ef12be6e946770ed7ef0b9c2b80ff Mon Sep 17 00:00:00 2001 From: Jason Gunthorpe Date: Thu, 11 May 2023 01:41:59 -0300 Subject: [PATCH 07/50] iommu: Replace iommu_group_device_count() with list_count_nodes() No reason to wrapper a standard function, just call the library directly. Reviewed-by: Lu Baolu Reviewed-by: Kevin Tian Tested-by: Heiko Stuebner Tested-by: Niklas Schnelle Signed-off-by: Jason Gunthorpe Link: https://lore.kernel.org/r/1-v5-1b99ae392328+44574-iommu_err_unwind_jgg@nvidia.com Signed-off-by: Joerg Roedel --- drivers/iommu/iommu.c | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c index 10eb24d2e55a..aab956f1c3ab 100644 --- a/drivers/iommu/iommu.c +++ b/drivers/iommu/iommu.c @@ -1125,17 +1125,6 @@ void iommu_group_remove_device(struct device *dev) } EXPORT_SYMBOL_GPL(iommu_group_remove_device); -static int iommu_group_device_count(struct iommu_group *group) -{ - struct group_device *entry; - int ret = 0; - - list_for_each_entry(entry, &group->devices, list) - ret++; - - return ret; -} - static int __iommu_group_for_each_dev(struct iommu_group *group, void *data, int (*fn)(struct device *, void *)) { @@ -2083,7 +2072,7 @@ int iommu_attach_device(struct iommu_domain *domain, struct device *dev) */ mutex_lock(&group->mutex); ret = -EINVAL; - if (iommu_group_device_count(group) != 1) + if (list_count_nodes(&group->devices) != 1) goto out_unlock; ret = __iommu_attach_group(domain, group); @@ -2114,7 +2103,7 @@ void iommu_detach_device(struct iommu_domain *domain, struct device *dev) mutex_lock(&group->mutex); if (WARN_ON(domain != group->domain) || - WARN_ON(iommu_group_device_count(group) != 1)) + WARN_ON(list_count_nodes(&group->devices) != 1)) goto out_unlock; __iommu_group_set_core_domain(group); From 3006b15b364a34a2a19b45bb2948dd6a83c5e1fe Mon Sep 17 00:00:00 2001 From: Jason Gunthorpe Date: Thu, 11 May 2023 01:42:00 -0300 Subject: [PATCH 08/50] iommu: Add for_each_group_device() Convenience macro to iterate over every struct group_device in the group. Replace all open coded list_for_each_entry's with this macro. Reviewed-by: Lu Baolu Reviewed-by: Kevin Tian Tested-by: Heiko Stuebner Tested-by: Niklas Schnelle Signed-off-by: Jason Gunthorpe Link: https://lore.kernel.org/r/2-v5-1b99ae392328+44574-iommu_err_unwind_jgg@nvidia.com Signed-off-by: Joerg Roedel --- .clang-format | 1 + drivers/iommu/iommu.c | 16 ++++++++++------ 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/.clang-format b/.clang-format index 0d1ed8776733..0bbb1991defe 100644 --- a/.clang-format +++ b/.clang-format @@ -254,6 +254,7 @@ ForEachMacros: - 'for_each_free_mem_range' - 'for_each_free_mem_range_reverse' - 'for_each_func_rsrc' + - 'for_each_group_device' - 'for_each_group_evsel' - 'for_each_group_member' - 'for_each_hstate' diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c index aab956f1c3ab..e806f4c781df 100644 --- a/drivers/iommu/iommu.c +++ b/drivers/iommu/iommu.c @@ -68,6 +68,10 @@ struct group_device { char *name; }; +/* Iterate over each struct group_device in a struct iommu_group */ +#define for_each_group_device(group, pos) \ + list_for_each_entry(pos, &(group)->devices, list) + struct iommu_group_attribute { struct attribute attr; ssize_t (*show)(struct iommu_group *group, char *buf); @@ -468,7 +472,7 @@ __iommu_group_remove_device(struct iommu_group *group, struct device *dev) struct group_device *device; lockdep_assert_held(&group->mutex); - list_for_each_entry(device, &group->devices, list) { + for_each_group_device(group, device) { if (device->dev == dev) { list_del(&device->list); return device; @@ -707,7 +711,7 @@ int iommu_get_group_resv_regions(struct iommu_group *group, int ret = 0; mutex_lock(&group->mutex); - list_for_each_entry(device, &group->devices, list) { + for_each_group_device(group, device) { struct list_head dev_resv_regions; /* @@ -1131,7 +1135,7 @@ static int __iommu_group_for_each_dev(struct iommu_group *group, void *data, struct group_device *device; int ret = 0; - list_for_each_entry(device, &group->devices, list) { + for_each_group_device(group, device) { ret = fn(device->dev, data); if (ret) break; @@ -1935,7 +1939,7 @@ bool iommu_group_has_isolated_msi(struct iommu_group *group) bool ret = true; mutex_lock(&group->mutex); - list_for_each_entry(group_dev, &group->devices, list) + for_each_group_device(group, group_dev) ret &= msi_device_has_isolated_msi(group_dev->dev); mutex_unlock(&group->mutex); return ret; @@ -3243,7 +3247,7 @@ static int __iommu_set_group_pasid(struct iommu_domain *domain, struct group_device *device; int ret = 0; - list_for_each_entry(device, &group->devices, list) { + for_each_group_device(group, device) { ret = domain->ops->set_dev_pasid(domain, device->dev, pasid); if (ret) break; @@ -3258,7 +3262,7 @@ static void __iommu_remove_group_pasid(struct iommu_group *group, struct group_device *device; const struct iommu_ops *ops; - list_for_each_entry(device, &group->devices, list) { + for_each_group_device(group, device) { ops = dev_iommu_ops(device->dev); ops->remove_dev_pasid(device->dev, pasid); } From dcf40ed3a20d727be054c4a20db47b32cb5036d4 Mon Sep 17 00:00:00 2001 From: Jason Gunthorpe Date: Thu, 11 May 2023 01:42:01 -0300 Subject: [PATCH 09/50] iommu: Make __iommu_group_set_domain() handle error unwind Let's try to have a consistent and clear strategy for error handling during domain attach failures. There are two broad categories, the first is callers doing destruction and trying to set the domain back to a previously good domain. These cases cannot handle failure during destruction flows and must succeed, or at least avoid a UAF on the current group->domain which is likely about to be freed. Many of the drivers are well behaved here and will not hit the WARN_ON's or a UAF, but some are doing hypercalls/etc that can fail unpredictably and don't meet the expectations. The second case is attaching a domain for the first time in a failable context, failure should restore the attachment back to group->domain using the above unfailable operation. Have __iommu_group_set_domain_internal() execute a common algorithm that tries to achieve this, and in the worst case, would leave a device "detached" or assigned to a global blocking domain. This relies on some existing common driver behaviors where attach failure will also do detatch and true IOMMU_DOMAIN_BLOCK implementations that are not allowed to ever fail. Name the first case with __iommu_group_set_domain_nofail() to make it clear. Pull all the error handling and WARN_ON generation into __iommu_group_set_domain_internal(). Avoid the obfuscating use of __iommu_group_for_each_dev() and be more careful about what should happen during failures by only touching devices we've already touched. Reviewed-by: Lu Baolu Reviewed-by: Kevin Tian Tested-by: Heiko Stuebner Tested-by: Niklas Schnelle Signed-off-by: Jason Gunthorpe Link: https://lore.kernel.org/r/3-v5-1b99ae392328+44574-iommu_err_unwind_jgg@nvidia.com Signed-off-by: Joerg Roedel --- drivers/iommu/iommu.c | 137 ++++++++++++++++++++++++++++++++++-------- 1 file changed, 112 insertions(+), 25 deletions(-) diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c index e806f4c781df..74cb162daac3 100644 --- a/drivers/iommu/iommu.c +++ b/drivers/iommu/iommu.c @@ -101,8 +101,26 @@ static int __iommu_attach_device(struct iommu_domain *domain, struct device *dev); static int __iommu_attach_group(struct iommu_domain *domain, struct iommu_group *group); + +enum { + IOMMU_SET_DOMAIN_MUST_SUCCEED = 1 << 0, +}; + +static int __iommu_group_set_domain_internal(struct iommu_group *group, + struct iommu_domain *new_domain, + unsigned int flags); static int __iommu_group_set_domain(struct iommu_group *group, - struct iommu_domain *new_domain); + struct iommu_domain *new_domain) +{ + return __iommu_group_set_domain_internal(group, new_domain, 0); +} +static void __iommu_group_set_domain_nofail(struct iommu_group *group, + struct iommu_domain *new_domain) +{ + WARN_ON(__iommu_group_set_domain_internal( + group, new_domain, IOMMU_SET_DOMAIN_MUST_SUCCEED)); +} + static int iommu_create_device_direct_mappings(struct iommu_group *group, struct device *dev); static struct iommu_group *iommu_group_get_for_dev(struct device *dev); @@ -2022,15 +2040,13 @@ EXPORT_SYMBOL_GPL(iommu_domain_free); static void __iommu_group_set_core_domain(struct iommu_group *group) { struct iommu_domain *new_domain; - int ret; if (group->owner) new_domain = group->blocking_domain; else new_domain = group->default_domain; - ret = __iommu_group_set_domain(group, new_domain); - WARN(ret, "iommu driver failed to attach the default/blocking domain"); + __iommu_group_set_domain_nofail(group, new_domain); } static int __iommu_attach_device(struct iommu_domain *domain, @@ -2215,21 +2231,55 @@ int iommu_attach_group(struct iommu_domain *domain, struct iommu_group *group) } EXPORT_SYMBOL_GPL(iommu_attach_group); -static int iommu_group_do_set_platform_dma(struct device *dev, void *data) +static int __iommu_device_set_domain(struct iommu_group *group, + struct device *dev, + struct iommu_domain *new_domain, + unsigned int flags) { - const struct iommu_ops *ops = dev_iommu_ops(dev); - - if (!WARN_ON(!ops->set_platform_dma_ops)) - ops->set_platform_dma_ops(dev); + int ret; + ret = __iommu_attach_device(new_domain, dev); + if (ret) { + /* + * If we have a blocking domain then try to attach that in hopes + * of avoiding a UAF. Modern drivers should implement blocking + * domains as global statics that cannot fail. + */ + if ((flags & IOMMU_SET_DOMAIN_MUST_SUCCEED) && + group->blocking_domain && + group->blocking_domain != new_domain) + __iommu_attach_device(group->blocking_domain, dev); + return ret; + } return 0; } -static int __iommu_group_set_domain(struct iommu_group *group, - struct iommu_domain *new_domain) +/* + * If 0 is returned the group's domain is new_domain. If an error is returned + * then the group's domain will be set back to the existing domain unless + * IOMMU_SET_DOMAIN_MUST_SUCCEED, otherwise an error is returned and the group's + * domains is left inconsistent. This is a driver bug to fail attach with a + * previously good domain. We try to avoid a kernel UAF because of this. + * + * IOMMU groups are really the natural working unit of the IOMMU, but the IOMMU + * API works on domains and devices. Bridge that gap by iterating over the + * devices in a group. Ideally we'd have a single device which represents the + * requestor ID of the group, but we also allow IOMMU drivers to create policy + * defined minimum sets, where the physical hardware may be able to distiguish + * members, but we wish to group them at a higher level (ex. untrusted + * multi-function PCI devices). Thus we attach each device. + */ +static int __iommu_group_set_domain_internal(struct iommu_group *group, + struct iommu_domain *new_domain, + unsigned int flags) { + struct group_device *last_gdev; + struct group_device *gdev; + int result; int ret; + lockdep_assert_held(&group->mutex); + if (group->domain == new_domain) return 0; @@ -2239,8 +2289,12 @@ static int __iommu_group_set_domain(struct iommu_group *group, * platform specific behavior. */ if (!new_domain) { - __iommu_group_for_each_dev(group, NULL, - iommu_group_do_set_platform_dma); + for_each_group_device(group, gdev) { + const struct iommu_ops *ops = dev_iommu_ops(gdev->dev); + + if (!WARN_ON(!ops->set_platform_dma_ops)) + ops->set_platform_dma_ops(gdev->dev); + } group->domain = NULL; return 0; } @@ -2250,16 +2304,52 @@ static int __iommu_group_set_domain(struct iommu_group *group, * domain. This switch does not have to be atomic and DMA can be * discarded during the transition. DMA must only be able to access * either new_domain or group->domain, never something else. - * - * Note that this is called in error unwind paths, attaching to a - * domain that has already been attached cannot fail. */ - ret = __iommu_group_for_each_dev(group, new_domain, - iommu_group_do_attach_device); - if (ret) - return ret; + result = 0; + for_each_group_device(group, gdev) { + ret = __iommu_device_set_domain(group, gdev->dev, new_domain, + flags); + if (ret) { + result = ret; + /* + * Keep trying the other devices in the group. If a + * driver fails attach to an otherwise good domain, and + * does not support blocking domains, it should at least + * drop its reference on the current domain so we don't + * UAF. + */ + if (flags & IOMMU_SET_DOMAIN_MUST_SUCCEED) + continue; + goto err_revert; + } + } group->domain = new_domain; - return 0; + return result; + +err_revert: + /* + * This is called in error unwind paths. A well behaved driver should + * always allow us to attach to a domain that was already attached. + */ + last_gdev = gdev; + for_each_group_device(group, gdev) { + const struct iommu_ops *ops = dev_iommu_ops(gdev->dev); + + /* + * If set_platform_dma_ops is not present a NULL domain can + * happen only for first probe, in which case we leave + * group->domain as NULL and let release clean everything up. + */ + if (group->domain) + WARN_ON(__iommu_device_set_domain( + group, gdev->dev, group->domain, + IOMMU_SET_DOMAIN_MUST_SUCCEED)); + else if (ops->set_platform_dma_ops) + ops->set_platform_dma_ops(gdev->dev); + if (gdev == last_gdev) + break; + } + return ret; } void iommu_detach_group(struct iommu_domain *domain, struct iommu_group *group) @@ -3176,16 +3266,13 @@ EXPORT_SYMBOL_GPL(iommu_device_claim_dma_owner); static void __iommu_release_dma_ownership(struct iommu_group *group) { - int ret; - if (WARN_ON(!group->owner_cnt || !group->owner || !xa_empty(&group->pasid_array))) return; group->owner_cnt = 0; group->owner = NULL; - ret = __iommu_group_set_domain(group, group->default_domain); - WARN(ret, "iommu driver failed to attach the default domain"); + __iommu_group_set_domain_nofail(group, group->default_domain); } /** From ecd60dc5d22b2ac2a68d9bf84bed0cf96b654cde Mon Sep 17 00:00:00 2001 From: Jason Gunthorpe Date: Thu, 11 May 2023 01:42:02 -0300 Subject: [PATCH 10/50] iommu: Use __iommu_group_set_domain() for __iommu_attach_group() The error recovery here matches the recovery inside __iommu_group_set_domain(), so just use it directly. Reviewed-by: Kevin Tian Reviewed-by: Lu Baolu Tested-by: Heiko Stuebner Tested-by: Niklas Schnelle Signed-off-by: Jason Gunthorpe Link: https://lore.kernel.org/r/4-v5-1b99ae392328+44574-iommu_err_unwind_jgg@nvidia.com Signed-off-by: Joerg Roedel --- drivers/iommu/iommu.c | 40 +--------------------------------------- 1 file changed, 1 insertion(+), 39 deletions(-) diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c index 74cb162daac3..f31ba66ccb2f 100644 --- a/drivers/iommu/iommu.c +++ b/drivers/iommu/iommu.c @@ -2159,52 +2159,14 @@ struct iommu_domain *iommu_get_dma_domain(struct device *dev) return dev->iommu_group->default_domain; } -/* - * IOMMU groups are really the natural working unit of the IOMMU, but - * the IOMMU API works on domains and devices. Bridge that gap by - * iterating over the devices in a group. Ideally we'd have a single - * device which represents the requestor ID of the group, but we also - * allow IOMMU drivers to create policy defined minimum sets, where - * the physical hardware may be able to distiguish members, but we - * wish to group them at a higher level (ex. untrusted multi-function - * PCI devices). Thus we attach each device. - */ -static int iommu_group_do_attach_device(struct device *dev, void *data) -{ - struct iommu_domain *domain = data; - - return __iommu_attach_device(domain, dev); -} - static int __iommu_attach_group(struct iommu_domain *domain, struct iommu_group *group) { - int ret; - if (group->domain && group->domain != group->default_domain && group->domain != group->blocking_domain) return -EBUSY; - ret = __iommu_group_for_each_dev(group, domain, - iommu_group_do_attach_device); - if (ret == 0) { - group->domain = domain; - } else { - /* - * To recover from the case when certain device within the - * group fails to attach to the new domain, we need force - * attaching all devices back to the old domain. The old - * domain is compatible for all devices in the group, - * hence the iommu driver should always return success. - */ - struct iommu_domain *old_domain = group->domain; - - group->domain = NULL; - WARN(__iommu_group_set_domain(group, old_domain), - "iommu driver failed to attach a compatible domain"); - } - - return ret; + return __iommu_group_set_domain(group, domain); } /** From 4c8ad9da05662141928fe4ed001d3775fd95221c Mon Sep 17 00:00:00 2001 From: Jason Gunthorpe Date: Thu, 11 May 2023 01:42:03 -0300 Subject: [PATCH 11/50] iommu: Use __iommu_group_set_domain() in iommu_change_dev_def_domain() This is missing re-attach error handling if the attach fails, use the common code. The ugly "group->domain = prev_domain" will be cleaned in a later patch. Reviewed-by: Lu Baolu Reviewed-by: Kevin Tian Tested-by: Heiko Stuebner Tested-by: Niklas Schnelle Signed-off-by: Jason Gunthorpe Link: https://lore.kernel.org/r/5-v5-1b99ae392328+44574-iommu_err_unwind_jgg@nvidia.com Signed-off-by: Joerg Roedel --- drivers/iommu/iommu.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c index f31ba66ccb2f..e0bfb114d08d 100644 --- a/drivers/iommu/iommu.c +++ b/drivers/iommu/iommu.c @@ -2946,11 +2946,12 @@ static int iommu_change_dev_def_domain(struct iommu_group *group, if (ret) goto restore_old_domain; - ret = iommu_group_create_direct_mappings(group); + group->domain = prev_dom; + ret = iommu_create_device_direct_mappings(group, dev); if (ret) goto free_new_domain; - ret = __iommu_attach_group(group->default_domain, group); + ret = __iommu_group_set_domain(group, group->default_domain); if (ret) goto free_new_domain; @@ -2962,7 +2963,6 @@ free_new_domain: iommu_domain_free(group->default_domain); restore_old_domain: group->default_domain = prev_dom; - group->domain = prev_dom; return ret; } From d257344c661950986e6129407f7169f54e0bb4cf Mon Sep 17 00:00:00 2001 From: Jason Gunthorpe Date: Thu, 11 May 2023 01:42:04 -0300 Subject: [PATCH 12/50] iommu: Replace __iommu_group_dma_first_attach() with set_domain Reorganize the attach_deferred logic to set dev->iommu->attach_deferred immediately during probe and then have __iommu_device_set_domain() check it and not attach the default_domain. This is to prepare for removing the group->domain set from iommu_group_alloc_default_domain() by calling __iommu_group_set_domain() to set the group->domain. Reviewed-by: Lu Baolu Tested-by: Heiko Stuebner Tested-by: Niklas Schnelle Signed-off-by: Jason Gunthorpe Link: https://lore.kernel.org/r/6-v5-1b99ae392328+44574-iommu_err_unwind_jgg@nvidia.com Signed-off-by: Joerg Roedel --- drivers/iommu/iommu.c | 32 +++++++++++--------------------- 1 file changed, 11 insertions(+), 21 deletions(-) diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c index e0bfb114d08d..eaa63fe887f9 100644 --- a/drivers/iommu/iommu.c +++ b/drivers/iommu/iommu.c @@ -365,6 +365,8 @@ static int __iommu_probe_device(struct device *dev, struct list_head *group_list dev->iommu->iommu_dev = iommu_dev; dev->iommu->max_pasids = dev_iommu_get_max_pasids(dev); + if (ops->is_attach_deferred) + dev->iommu->attach_deferred = ops->is_attach_deferred(dev); group = iommu_group_get_for_dev(dev); if (IS_ERR(group)) { @@ -399,27 +401,14 @@ err_unlock: return ret; } -static bool iommu_is_attach_deferred(struct device *dev) -{ - const struct iommu_ops *ops = dev_iommu_ops(dev); - - if (ops->is_attach_deferred) - return ops->is_attach_deferred(dev); - - return false; -} - static int iommu_group_do_dma_first_attach(struct device *dev, void *data) { struct iommu_domain *domain = data; lockdep_assert_held(&dev->iommu_group->mutex); - if (iommu_is_attach_deferred(dev)) { - dev->iommu->attach_deferred = 1; + if (dev->iommu->attach_deferred) return 0; - } - return __iommu_attach_device(domain, dev); } @@ -1831,12 +1820,6 @@ static void probe_alloc_default_domain(const struct bus_type *bus, } -static int __iommu_group_dma_first_attach(struct iommu_group *group) -{ - return __iommu_group_for_each_dev(group, group->default_domain, - iommu_group_do_dma_first_attach); -} - static int iommu_group_do_probe_finalize(struct device *dev, void *data) { const struct iommu_ops *ops = dev_iommu_ops(dev); @@ -1899,7 +1882,8 @@ int bus_iommu_probe(const struct bus_type *bus) iommu_group_create_direct_mappings(group); - ret = __iommu_group_dma_first_attach(group); + group->domain = NULL; + ret = __iommu_group_set_domain(group, group->default_domain); mutex_unlock(&group->mutex); @@ -2200,6 +2184,12 @@ static int __iommu_device_set_domain(struct iommu_group *group, { int ret; + if (dev->iommu->attach_deferred) { + if (new_domain == group->default_domain) + return 0; + dev->iommu->attach_deferred = 0; + } + ret = __iommu_attach_device(new_domain, dev); if (ret) { /* From 0046a4337eae148510173680f82b483f7c3b7ded Mon Sep 17 00:00:00 2001 From: Jason Gunthorpe Date: Thu, 11 May 2023 01:42:05 -0300 Subject: [PATCH 13/50] iommu: Remove iommu_group_do_dma_first_attach() from iommu_group_add_device() This function is only used to construct the groups, it should not be operating the iommu driver. External callers in VFIO and POWER do not have any iommu drivers on the devices so group->domain will be NULL. The only internal caller is from iommu_probe_device() which already calls iommu_group_do_dma_first_attach(), meaning we are calling it twice in the only case it matters. Since iommu_probe_device() is the logical place to sort out the group's domain, remove the call from iommu_group_add_device(). Reviewed-by: Kevin Tian Tested-by: Heiko Stuebner Tested-by: Niklas Schnelle Signed-off-by: Jason Gunthorpe Link: https://lore.kernel.org/r/7-v5-1b99ae392328+44574-iommu_err_unwind_jgg@nvidia.com Signed-off-by: Joerg Roedel --- drivers/iommu/iommu.c | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c index eaa63fe887f9..fa2b669ecf4b 100644 --- a/drivers/iommu/iommu.c +++ b/drivers/iommu/iommu.c @@ -1080,25 +1080,13 @@ rename: mutex_lock(&group->mutex); list_add_tail(&device->list, &group->devices); - if (group->domain) - ret = iommu_group_do_dma_first_attach(dev, group->domain); mutex_unlock(&group->mutex); - if (ret) - goto err_put_group; - trace_add_device_to_group(group->id, dev); dev_info(dev, "Adding to iommu group %d\n", group->id); return 0; -err_put_group: - mutex_lock(&group->mutex); - list_del(&device->list); - mutex_unlock(&group->mutex); - dev->iommu_group = NULL; - kobject_put(group->devices_kobj); - sysfs_remove_link(group->devices_kobj, device->name); err_free_name: kfree(device->name); err_remove_link: From 2f74198ae006c50a4188ae02c10e2c7b0b8142da Mon Sep 17 00:00:00 2001 From: Jason Gunthorpe Date: Thu, 11 May 2023 01:42:06 -0300 Subject: [PATCH 14/50] iommu: Replace iommu_group_do_dma_first_attach with __iommu_device_set_domain Since __iommu_device_set_domain() now knows how to handle deferred attach we can just call it directly from the only call site. Reviewed-by: Kevin Tian Reviewed-by: Lu Baolu Tested-by: Heiko Stuebner Tested-by: Niklas Schnelle Signed-off-by: Jason Gunthorpe Link: https://lore.kernel.org/r/8-v5-1b99ae392328+44574-iommu_err_unwind_jgg@nvidia.com Signed-off-by: Joerg Roedel --- drivers/iommu/iommu.c | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c index fa2b669ecf4b..ea61a81c0006 100644 --- a/drivers/iommu/iommu.c +++ b/drivers/iommu/iommu.c @@ -106,6 +106,10 @@ enum { IOMMU_SET_DOMAIN_MUST_SUCCEED = 1 << 0, }; +static int __iommu_device_set_domain(struct iommu_group *group, + struct device *dev, + struct iommu_domain *new_domain, + unsigned int flags); static int __iommu_group_set_domain_internal(struct iommu_group *group, struct iommu_domain *new_domain, unsigned int flags); @@ -401,17 +405,6 @@ err_unlock: return ret; } -static int iommu_group_do_dma_first_attach(struct device *dev, void *data) -{ - struct iommu_domain *domain = data; - - lockdep_assert_held(&dev->iommu_group->mutex); - - if (dev->iommu->attach_deferred) - return 0; - return __iommu_attach_device(domain, dev); -} - int iommu_probe_device(struct device *dev) { const struct iommu_ops *ops; @@ -442,7 +435,7 @@ int iommu_probe_device(struct device *dev) * attach the default domain. */ if (group->default_domain && !group->owner) { - ret = iommu_group_do_dma_first_attach(dev, group->default_domain); + ret = __iommu_device_set_domain(group, dev, group->domain, 0); if (ret) { mutex_unlock(&group->mutex); iommu_group_put(group); From e7f85dfbbc9cf8660174c45c213571aaa518df85 Mon Sep 17 00:00:00 2001 From: Jason Gunthorpe Date: Thu, 11 May 2023 01:42:07 -0300 Subject: [PATCH 15/50] iommu: Fix iommu_probe_device() to attach the right domain The general invariant is that all devices in an iommu_group are attached to group->domain. We missed some cases here where an owned group would not get the device attached. Rework this logic so it follows the default domain flow of the bus_iommu_probe() - call iommu_alloc_default_domain(), then use __iommu_group_set_domain_internal() to set up all the devices. Finally always attach the device to the current domain if it is already set. This is an unlikely functional issue as iommufd uses iommu_attach_group(). It is possible to hot plug in a new group member, add a vfio driver to it and then hot add it to an existing iommufd. In this case it is required that the core code set the iommu_domain properly since iommufd won't call iommu_attach_group() again. Reviewed-by: Lu Baolu Reviewed-by: Kevin Tian Tested-by: Heiko Stuebner Tested-by: Niklas Schnelle Signed-off-by: Jason Gunthorpe Link: https://lore.kernel.org/r/9-v5-1b99ae392328+44574-iommu_err_unwind_jgg@nvidia.com Signed-off-by: Joerg Roedel --- drivers/iommu/iommu.c | 44 +++++++++++++++++++++++-------------------- 1 file changed, 24 insertions(+), 20 deletions(-) diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c index ea61a81c0006..29ab5d990ef6 100644 --- a/drivers/iommu/iommu.c +++ b/drivers/iommu/iommu.c @@ -421,27 +421,31 @@ int iommu_probe_device(struct device *dev) goto err_release; } - /* - * Try to allocate a default domain - needs support from the - * IOMMU driver. There are still some drivers which don't - * support default domains, so the return value is not yet - * checked. - */ mutex_lock(&group->mutex); - iommu_alloc_default_domain(group, dev); - /* - * If device joined an existing group which has been claimed, don't - * attach the default domain. - */ - if (group->default_domain && !group->owner) { + if (group->domain) { ret = __iommu_device_set_domain(group, dev, group->domain, 0); - if (ret) { - mutex_unlock(&group->mutex); - iommu_group_put(group); - goto err_release; - } + } else if (!group->default_domain) { + /* + * Try to allocate a default domain - needs support from the + * IOMMU driver. There are still some drivers which don't + * support default domains, so the return value is not yet + * checked. + */ + iommu_alloc_default_domain(group, dev); + group->domain = NULL; + if (group->default_domain) + ret = __iommu_group_set_domain(group, + group->default_domain); + + /* + * We assume that the iommu driver starts up the device in + * 'set_platform_dma_ops' mode if it does not support default + * domains. + */ } + if (ret) + goto err_unlock; iommu_create_device_direct_mappings(group, dev); @@ -454,6 +458,9 @@ int iommu_probe_device(struct device *dev) return 0; +err_unlock: + mutex_unlock(&group->mutex); + iommu_group_put(group); err_release: iommu_release_device(dev); @@ -1665,9 +1672,6 @@ static int iommu_alloc_default_domain(struct iommu_group *group, { unsigned int type; - if (group->default_domain) - return 0; - type = iommu_get_def_domain_type(dev) ? : iommu_def_domain_type; return iommu_group_alloc_default_domain(dev->bus, group, type); From 152431e4fe7f1aac8aa6cc57bfe58d2d2596be4d Mon Sep 17 00:00:00 2001 From: Jason Gunthorpe Date: Thu, 11 May 2023 01:42:08 -0300 Subject: [PATCH 16/50] iommu: Do iommu_group_create_direct_mappings() before attach The iommu_probe_device() path calls iommu_create_device_direct_mappings() after attaching the device. IOMMU_RESV_DIRECT maps need to be continually in place, so if a hotplugged device has new ranges the should have been mapped into the default domain before it is attached. Move the iommu_create_device_direct_mappings() call up. Reviewed-by: Lu Baolu Reviewed-by: Kevin Tian Tested-by: Heiko Stuebner Tested-by: Niklas Schnelle Signed-off-by: Jason Gunthorpe Link: https://lore.kernel.org/r/10-v5-1b99ae392328+44574-iommu_err_unwind_jgg@nvidia.com Signed-off-by: Joerg Roedel --- drivers/iommu/iommu.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c index 29ab5d990ef6..6b39f756c020 100644 --- a/drivers/iommu/iommu.c +++ b/drivers/iommu/iommu.c @@ -423,6 +423,8 @@ int iommu_probe_device(struct device *dev) mutex_lock(&group->mutex); + iommu_create_device_direct_mappings(group, dev); + if (group->domain) { ret = __iommu_device_set_domain(group, dev, group->domain, 0); } else if (!group->default_domain) { @@ -434,9 +436,11 @@ int iommu_probe_device(struct device *dev) */ iommu_alloc_default_domain(group, dev); group->domain = NULL; - if (group->default_domain) + if (group->default_domain) { + iommu_create_device_direct_mappings(group, dev); ret = __iommu_group_set_domain(group, group->default_domain); + } /* * We assume that the iommu driver starts up the device in @@ -447,8 +451,6 @@ int iommu_probe_device(struct device *dev) if (ret) goto err_unlock; - iommu_create_device_direct_mappings(group, dev); - mutex_unlock(&group->mutex); iommu_group_put(group); From dfddd54dc77c4519ee3c94e7462b1c035c69a031 Mon Sep 17 00:00:00 2001 From: Jason Gunthorpe Date: Thu, 11 May 2023 01:42:09 -0300 Subject: [PATCH 17/50] iommu: Remove the assignment of group->domain during default domain alloc group->domain should only be set once all the device's drivers have had their ops->attach_dev() called. iommu_group_alloc_default_domain() doesn't do this, so it shouldn't set the value. The previous patches organized things so that each caller of iommu_group_alloc_default_domain() follows up with calling __iommu_group_set_domain_internal() that does set the group->domain. Reviewed-by: Kevin Tian Reviewed-by: Lu Baolu Tested-by: Heiko Stuebner Tested-by: Niklas Schnelle Signed-off-by: Jason Gunthorpe Link: https://lore.kernel.org/r/11-v5-1b99ae392328+44574-iommu_err_unwind_jgg@nvidia.com Signed-off-by: Joerg Roedel --- drivers/iommu/iommu.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c index 6b39f756c020..2041e3e028de 100644 --- a/drivers/iommu/iommu.c +++ b/drivers/iommu/iommu.c @@ -435,7 +435,6 @@ int iommu_probe_device(struct device *dev) * checked. */ iommu_alloc_default_domain(group, dev); - group->domain = NULL; if (group->default_domain) { iommu_create_device_direct_mappings(group, dev); ret = __iommu_group_set_domain(group, @@ -1664,8 +1663,6 @@ static int iommu_group_alloc_default_domain(const struct bus_type *bus, return -ENOMEM; group->default_domain = dom; - if (!group->domain) - group->domain = dom; return 0; } @@ -1869,7 +1866,6 @@ int bus_iommu_probe(const struct bus_type *bus) iommu_group_create_direct_mappings(group); - group->domain = NULL; ret = __iommu_group_set_domain(group, group->default_domain); mutex_unlock(&group->mutex); From 8b4eb75ee50e6f4606f88debf44aeb47937057d4 Mon Sep 17 00:00:00 2001 From: Jason Gunthorpe Date: Thu, 11 May 2023 01:42:10 -0300 Subject: [PATCH 18/50] iommu: Consolidate the code to calculate the target default domain type Put all the code to calculate the default domain type into one function. Make the function able to handle the iommu_change_dev_def_domain() by taking in the target domain type and erroring out if the target type isn't reachable. This makes it really clear that specifying a 0 type during iommu_change_dev_def_domain() will have the same outcome as the normal probe path. Remove the obfuscating use of __iommu_group_for_each_dev() and related struct __group_domain_type. Reviewed-by: Lu Baolu Reviewed-by: Kevin Tian Tested-by: Heiko Stuebner Tested-by: Niklas Schnelle Signed-off-by: Jason Gunthorpe Link: https://lore.kernel.org/r/12-v5-1b99ae392328+44574-iommu_err_unwind_jgg@nvidia.com Signed-off-by: Joerg Roedel --- drivers/iommu/iommu.c | 88 +++++++++++++++++-------------------------- 1 file changed, 35 insertions(+), 53 deletions(-) diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c index 2041e3e028de..9e661cbd3d42 100644 --- a/drivers/iommu/iommu.c +++ b/drivers/iommu/iommu.c @@ -1758,50 +1758,43 @@ static int iommu_bus_notifier(struct notifier_block *nb, return 0; } -struct __group_domain_type { - struct device *dev; - unsigned int type; -}; - -static int probe_get_default_domain_type(struct device *dev, void *data) +/* A target_type of 0 will select the best domain type and cannot fail */ +static int iommu_get_default_domain_type(struct iommu_group *group, + int target_type) { - struct __group_domain_type *gtype = data; - unsigned int type = iommu_get_def_domain_type(dev); + int best_type = target_type; + struct group_device *gdev; + struct device *last_dev; - if (type) { - if (gtype->type && gtype->type != type) { - dev_warn(dev, "Device needs domain type %s, but device %s in the same iommu group requires type %s - using default\n", - iommu_domain_type_str(type), - dev_name(gtype->dev), - iommu_domain_type_str(gtype->type)); - gtype->type = 0; - } + lockdep_assert_held(&group->mutex); - if (!gtype->dev) { - gtype->dev = dev; - gtype->type = type; + for_each_group_device(group, gdev) { + unsigned int type = iommu_get_def_domain_type(gdev->dev); + + if (best_type && type && best_type != type) { + if (target_type) { + dev_err_ratelimited( + gdev->dev, + "Device cannot be in %s domain\n", + iommu_domain_type_str(target_type)); + return -1; + } + + dev_warn( + gdev->dev, + "Device needs domain type %s, but device %s in the same iommu group requires type %s - using default\n", + iommu_domain_type_str(type), dev_name(last_dev), + iommu_domain_type_str(best_type)); + return iommu_def_domain_type; } + if (!best_type) + best_type = type; + last_dev = gdev->dev; } - return 0; -} - -static void probe_alloc_default_domain(const struct bus_type *bus, - struct iommu_group *group) -{ - struct __group_domain_type gtype; - - memset(>ype, 0, sizeof(gtype)); - - /* Ask for default domain requirements of all devices in the group */ - __iommu_group_for_each_dev(group, >ype, - probe_get_default_domain_type); - - if (!gtype.type) - gtype.type = iommu_def_domain_type; - - iommu_group_alloc_default_domain(bus, group, gtype.type); - + if (!best_type) + return iommu_def_domain_type; + return best_type; } static int iommu_group_do_probe_finalize(struct device *dev, void *data) @@ -1857,7 +1850,8 @@ int bus_iommu_probe(const struct bus_type *bus) list_del_init(&group->entry); /* Try to allocate default domain */ - probe_alloc_default_domain(bus, group); + iommu_group_alloc_default_domain( + bus, group, iommu_get_default_domain_type(group, 0)); if (!group->default_domain) { mutex_unlock(&group->mutex); @@ -2882,27 +2876,15 @@ EXPORT_SYMBOL_GPL(iommu_dev_disable_feature); static int iommu_change_dev_def_domain(struct iommu_group *group, struct device *dev, int type) { - struct __group_domain_type gtype = {NULL, 0}; struct iommu_domain *prev_dom; int ret; lockdep_assert_held(&group->mutex); prev_dom = group->default_domain; - __iommu_group_for_each_dev(group, >ype, - probe_get_default_domain_type); - if (!type) { - /* - * If the user hasn't requested any specific type of domain and - * if the device supports both the domains, then default to the - * domain the device was booted with - */ - type = gtype.type ? : iommu_def_domain_type; - } else if (gtype.type && type != gtype.type) { - dev_err_ratelimited(dev, "Device cannot be in %s domain\n", - iommu_domain_type_str(type)); + type = iommu_get_default_domain_type(group, type); + if (type < 0) return -EINVAL; - } /* * Switch to a new domain only if the requested domain type is different From fcbb0a4d738ce3ccc06d2c73ba227cce5094d885 Mon Sep 17 00:00:00 2001 From: Jason Gunthorpe Date: Thu, 11 May 2023 01:42:11 -0300 Subject: [PATCH 19/50] iommu: Revise iommu_group_alloc_default_domain() Robin points out that the fallback to guessing what domains the driver supports should only happen if the driver doesn't return a preference from its ops->def_domain_type(). Re-organize iommu_group_alloc_default_domain() so it internally uses iommu_def_domain_type only during the fallback and makes it clearer how the fallback sequence works. Make iommu_group_alloc_default_domain() return the domain so the return based logic is cleaner and to prepare for the next patch. Remove the iommu_alloc_default_domain() function as it is now trivially just calling iommu_group_alloc_default_domain(). Reviewed-by: Lu Baolu Reviewed-by: Kevin Tian Tested-by: Heiko Stuebner Tested-by: Niklas Schnelle Signed-off-by: Jason Gunthorpe Link: https://lore.kernel.org/r/13-v5-1b99ae392328+44574-iommu_err_unwind_jgg@nvidia.com Signed-off-by: Joerg Roedel --- drivers/iommu/iommu.c | 73 ++++++++++++++++++++++--------------------- 1 file changed, 38 insertions(+), 35 deletions(-) diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c index 9e661cbd3d42..7120f57c8028 100644 --- a/drivers/iommu/iommu.c +++ b/drivers/iommu/iommu.c @@ -93,8 +93,9 @@ static const char * const iommu_group_resv_type_string[] = { static int iommu_bus_notifier(struct notifier_block *nb, unsigned long action, void *data); static void iommu_release_device(struct device *dev); -static int iommu_alloc_default_domain(struct iommu_group *group, - struct device *dev); +static struct iommu_domain * +iommu_group_alloc_default_domain(struct iommu_group *group, int req_type); +static int iommu_get_def_domain_type(struct device *dev); static struct iommu_domain *__iommu_domain_alloc(const struct bus_type *bus, unsigned type); static int __iommu_attach_device(struct iommu_domain *domain, @@ -434,7 +435,8 @@ int iommu_probe_device(struct device *dev) * support default domains, so the return value is not yet * checked. */ - iommu_alloc_default_domain(group, dev); + group->default_domain = iommu_group_alloc_default_domain( + group, iommu_get_def_domain_type(dev)); if (group->default_domain) { iommu_create_device_direct_mappings(group, dev); ret = __iommu_group_set_domain(group, @@ -1645,35 +1647,38 @@ static int iommu_get_def_domain_type(struct device *dev) return 0; } -static int iommu_group_alloc_default_domain(const struct bus_type *bus, - struct iommu_group *group, - unsigned int type) +/* + * req_type of 0 means "auto" which means to select a domain based on + * iommu_def_domain_type or what the driver actually supports. + */ +static struct iommu_domain * +iommu_group_alloc_default_domain(struct iommu_group *group, int req_type) { + const struct bus_type *bus = + list_first_entry(&group->devices, struct group_device, list) + ->dev->bus; struct iommu_domain *dom; - dom = __iommu_domain_alloc(bus, type); - if (!dom && type != IOMMU_DOMAIN_DMA) { - dom = __iommu_domain_alloc(bus, IOMMU_DOMAIN_DMA); - if (dom) - pr_warn("Failed to allocate default IOMMU domain of type %u for group %s - Falling back to IOMMU_DOMAIN_DMA", - type, group->name); - } + lockdep_assert_held(&group->mutex); + if (req_type) + return __iommu_domain_alloc(bus, req_type); + + /* The driver gave no guidance on what type to use, try the default */ + dom = __iommu_domain_alloc(bus, iommu_def_domain_type); + if (dom) + return dom; + + /* Otherwise IDENTITY and DMA_FQ defaults will try DMA */ + if (iommu_def_domain_type == IOMMU_DOMAIN_DMA) + return NULL; + dom = __iommu_domain_alloc(bus, IOMMU_DOMAIN_DMA); if (!dom) - return -ENOMEM; + return NULL; - group->default_domain = dom; - return 0; -} - -static int iommu_alloc_default_domain(struct iommu_group *group, - struct device *dev) -{ - unsigned int type; - - type = iommu_get_def_domain_type(dev) ? : iommu_def_domain_type; - - return iommu_group_alloc_default_domain(dev->bus, group, type); + pr_warn("Failed to allocate default IOMMU domain of type %u for group %s - Falling back to IOMMU_DOMAIN_DMA", + iommu_def_domain_type, group->name); + return dom; } /** @@ -1785,15 +1790,12 @@ static int iommu_get_default_domain_type(struct iommu_group *group, "Device needs domain type %s, but device %s in the same iommu group requires type %s - using default\n", iommu_domain_type_str(type), dev_name(last_dev), iommu_domain_type_str(best_type)); - return iommu_def_domain_type; + return 0; } if (!best_type) best_type = type; last_dev = gdev->dev; } - - if (!best_type) - return iommu_def_domain_type; return best_type; } @@ -1850,9 +1852,8 @@ int bus_iommu_probe(const struct bus_type *bus) list_del_init(&group->entry); /* Try to allocate default domain */ - iommu_group_alloc_default_domain( - bus, group, iommu_get_default_domain_type(group, 0)); - + group->default_domain = iommu_group_alloc_default_domain( + group, iommu_get_default_domain_type(group, 0)); if (!group->default_domain) { mutex_unlock(&group->mutex); continue; @@ -2897,9 +2898,11 @@ static int iommu_change_dev_def_domain(struct iommu_group *group, group->domain = NULL; /* Sets group->default_domain to the newly allocated domain */ - ret = iommu_group_alloc_default_domain(dev->bus, group, type); - if (ret) + group->default_domain = iommu_group_alloc_default_domain(group, type); + if (!group->default_domain) { + ret = -EINVAL; goto restore_old_domain; + } group->domain = prev_dom; ret = iommu_create_device_direct_mappings(group, dev); From d99be00f42eac9fc35a164f3f6c8c7a56b295aa9 Mon Sep 17 00:00:00 2001 From: Jason Gunthorpe Date: Thu, 11 May 2023 01:42:12 -0300 Subject: [PATCH 20/50] iommu: Consolidate the default_domain setup to one function Make iommu_change_dev_def_domain() general enough to setup the initial default_domain or replace it with a new default_domain. Call the new function iommu_setup_default_domain() and make it the only place in the code that stores to group->default_domain. Consolidate the three copies of the default_domain setup sequence. The flow flow requires: - Determining the domain type to use - Checking if the current default domain is the same type - Allocating a domain - Doing iommu_create_device_direct_mappings() - Attaching it to devices - Store group->default_domain This adjusts the domain allocation from the prior patch to be able to detect if each of the allocation steps is already the domain we already have, which is a more robust version of what change default domain was already doing. Reviewed-by: Lu Baolu Reviewed-by: Kevin Tian Tested-by: Heiko Stuebner Tested-by: Niklas Schnelle Signed-off-by: Jason Gunthorpe Link: https://lore.kernel.org/r/14-v5-1b99ae392328+44574-iommu_err_unwind_jgg@nvidia.com Signed-off-by: Joerg Roedel --- drivers/iommu/iommu.c | 202 +++++++++++++++++++----------------------- 1 file changed, 89 insertions(+), 113 deletions(-) diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c index 7120f57c8028..34f721434b28 100644 --- a/drivers/iommu/iommu.c +++ b/drivers/iommu/iommu.c @@ -93,9 +93,6 @@ static const char * const iommu_group_resv_type_string[] = { static int iommu_bus_notifier(struct notifier_block *nb, unsigned long action, void *data); static void iommu_release_device(struct device *dev); -static struct iommu_domain * -iommu_group_alloc_default_domain(struct iommu_group *group, int req_type); -static int iommu_get_def_domain_type(struct device *dev); static struct iommu_domain *__iommu_domain_alloc(const struct bus_type *bus, unsigned type); static int __iommu_attach_device(struct iommu_domain *domain, @@ -126,7 +123,9 @@ static void __iommu_group_set_domain_nofail(struct iommu_group *group, group, new_domain, IOMMU_SET_DOMAIN_MUST_SUCCEED)); } -static int iommu_create_device_direct_mappings(struct iommu_group *group, +static int iommu_setup_default_domain(struct iommu_group *group, + int target_type); +static int iommu_create_device_direct_mappings(struct iommu_domain *domain, struct device *dev); static struct iommu_group *iommu_group_get_for_dev(struct device *dev); static ssize_t iommu_group_store_type(struct iommu_group *group, @@ -424,33 +423,18 @@ int iommu_probe_device(struct device *dev) mutex_lock(&group->mutex); - iommu_create_device_direct_mappings(group, dev); + if (group->default_domain) + iommu_create_device_direct_mappings(group->default_domain, dev); if (group->domain) { ret = __iommu_device_set_domain(group, dev, group->domain, 0); + if (ret) + goto err_unlock; } else if (!group->default_domain) { - /* - * Try to allocate a default domain - needs support from the - * IOMMU driver. There are still some drivers which don't - * support default domains, so the return value is not yet - * checked. - */ - group->default_domain = iommu_group_alloc_default_domain( - group, iommu_get_def_domain_type(dev)); - if (group->default_domain) { - iommu_create_device_direct_mappings(group, dev); - ret = __iommu_group_set_domain(group, - group->default_domain); - } - - /* - * We assume that the iommu driver starts up the device in - * 'set_platform_dma_ops' mode if it does not support default - * domains. - */ + ret = iommu_setup_default_domain(group, 0); + if (ret) + goto err_unlock; } - if (ret) - goto err_unlock; mutex_unlock(&group->mutex); iommu_group_put(group); @@ -967,16 +951,15 @@ int iommu_group_set_name(struct iommu_group *group, const char *name) } EXPORT_SYMBOL_GPL(iommu_group_set_name); -static int iommu_create_device_direct_mappings(struct iommu_group *group, +static int iommu_create_device_direct_mappings(struct iommu_domain *domain, struct device *dev) { - struct iommu_domain *domain = group->default_domain; struct iommu_resv_region *entry; struct list_head mappings; unsigned long pg_size; int ret = 0; - if (!domain || !iommu_is_dma_domain(domain)) + if (!iommu_is_dma_domain(domain)) return 0; BUG_ON(!domain->pgsize_bitmap); @@ -1647,6 +1630,15 @@ static int iommu_get_def_domain_type(struct device *dev) return 0; } +static struct iommu_domain * +__iommu_group_alloc_default_domain(const struct bus_type *bus, + struct iommu_group *group, int req_type) +{ + if (group->default_domain && group->default_domain->type == req_type) + return group->default_domain; + return __iommu_domain_alloc(bus, req_type); +} + /* * req_type of 0 means "auto" which means to select a domain based on * iommu_def_domain_type or what the driver actually supports. @@ -1662,17 +1654,17 @@ iommu_group_alloc_default_domain(struct iommu_group *group, int req_type) lockdep_assert_held(&group->mutex); if (req_type) - return __iommu_domain_alloc(bus, req_type); + return __iommu_group_alloc_default_domain(bus, group, req_type); /* The driver gave no guidance on what type to use, try the default */ - dom = __iommu_domain_alloc(bus, iommu_def_domain_type); + dom = __iommu_group_alloc_default_domain(bus, group, iommu_def_domain_type); if (dom) return dom; /* Otherwise IDENTITY and DMA_FQ defaults will try DMA */ if (iommu_def_domain_type == IOMMU_DOMAIN_DMA) return NULL; - dom = __iommu_domain_alloc(bus, IOMMU_DOMAIN_DMA); + dom = __iommu_group_alloc_default_domain(bus, group, IOMMU_DOMAIN_DMA); if (!dom) return NULL; @@ -1815,21 +1807,6 @@ static void __iommu_group_dma_finalize(struct iommu_group *group) iommu_group_do_probe_finalize); } -static int iommu_do_create_direct_mappings(struct device *dev, void *data) -{ - struct iommu_group *group = data; - - iommu_create_device_direct_mappings(group, dev); - - return 0; -} - -static int iommu_group_create_direct_mappings(struct iommu_group *group) -{ - return __iommu_group_for_each_dev(group, group, - iommu_do_create_direct_mappings); -} - int bus_iommu_probe(const struct bus_type *bus) { struct iommu_group *group, *next; @@ -1851,27 +1828,16 @@ int bus_iommu_probe(const struct bus_type *bus) /* Remove item from the list */ list_del_init(&group->entry); - /* Try to allocate default domain */ - group->default_domain = iommu_group_alloc_default_domain( - group, iommu_get_default_domain_type(group, 0)); - if (!group->default_domain) { + ret = iommu_setup_default_domain(group, 0); + if (ret) { mutex_unlock(&group->mutex); - continue; + return ret; } - - iommu_group_create_direct_mappings(group); - - ret = __iommu_group_set_domain(group, group->default_domain); - mutex_unlock(&group->mutex); - - if (ret) - break; - __iommu_group_dma_finalize(group); } - return ret; + return 0; } bool iommu_present(const struct bus_type *bus) @@ -2860,68 +2826,83 @@ int iommu_dev_disable_feature(struct device *dev, enum iommu_dev_features feat) } EXPORT_SYMBOL_GPL(iommu_dev_disable_feature); -/* - * Changes the default domain of an iommu group +/** + * iommu_setup_default_domain - Set the default_domain for the group + * @group: Group to change + * @target_type: Domain type to set as the default_domain * - * @group: The group for which the default domain should be changed - * @dev: The first device in the group - * @type: The type of the new default domain that gets associated with the group - * - * Returns 0 on success and error code on failure - * - * Note: - * 1. Presently, this function is called only when user requests to change the - * group's default domain type through /sys/kernel/iommu_groups//type - * Please take a closer look if intended to use for other purposes. + * Allocate a default domain and set it as the current domain on the group. If + * the group already has a default domain it will be changed to the target_type. + * When target_type is 0 the default domain is selected based on driver and + * system preferences. */ -static int iommu_change_dev_def_domain(struct iommu_group *group, - struct device *dev, int type) +static int iommu_setup_default_domain(struct iommu_group *group, + int target_type) { - struct iommu_domain *prev_dom; + struct iommu_domain *old_dom = group->default_domain; + struct group_device *gdev; + struct iommu_domain *dom; + int req_type; int ret; lockdep_assert_held(&group->mutex); - prev_dom = group->default_domain; - type = iommu_get_default_domain_type(group, type); - if (type < 0) + req_type = iommu_get_default_domain_type(group, target_type); + if (req_type < 0) return -EINVAL; /* - * Switch to a new domain only if the requested domain type is different - * from the existing default domain type + * There are still some drivers which don't support default domains, so + * we ignore the failure and leave group->default_domain NULL. + * + * We assume that the iommu driver starts up the device in + * 'set_platform_dma_ops' mode if it does not support default domains. */ - if (prev_dom->type == type) + dom = iommu_group_alloc_default_domain(group, req_type); + if (!dom) { + /* Once in default_domain mode we never leave */ + if (group->default_domain) + return -ENODEV; + group->default_domain = NULL; return 0; - - group->default_domain = NULL; - group->domain = NULL; - - /* Sets group->default_domain to the newly allocated domain */ - group->default_domain = iommu_group_alloc_default_domain(group, type); - if (!group->default_domain) { - ret = -EINVAL; - goto restore_old_domain; } - group->domain = prev_dom; - ret = iommu_create_device_direct_mappings(group, dev); - if (ret) - goto free_new_domain; + if (group->default_domain == dom) + return 0; - ret = __iommu_group_set_domain(group, group->default_domain); - if (ret) - goto free_new_domain; + /* + * IOMMU_RESV_DIRECT and IOMMU_RESV_DIRECT_RELAXABLE regions must be + * mapped before their device is attached, in order to guarantee + * continuity with any FW activity + */ + for_each_group_device(group, gdev) + iommu_create_device_direct_mappings(dom, gdev->dev); - iommu_domain_free(prev_dom); - - return 0; - -free_new_domain: - iommu_domain_free(group->default_domain); -restore_old_domain: - group->default_domain = prev_dom; + /* We must set default_domain early for __iommu_device_set_domain */ + group->default_domain = dom; + if (!group->domain) { + /* + * Drivers are not allowed to fail the first domain attach. + * The only way to recover from this is to fail attaching the + * iommu driver and call ops->release_device. Put the domain + * in group->default_domain so it is freed after. + */ + ret = __iommu_group_set_domain_internal( + group, dom, IOMMU_SET_DOMAIN_MUST_SUCCEED); + if (WARN_ON(ret)) + goto out_free; + } else { + ret = __iommu_group_set_domain(group, dom); + if (ret) { + iommu_domain_free(dom); + group->default_domain = old_dom; + return ret; + } + } +out_free: + if (old_dom) + iommu_domain_free(old_dom); return ret; } @@ -2937,8 +2918,6 @@ restore_old_domain: static ssize_t iommu_group_store_type(struct iommu_group *group, const char *buf, size_t count) { - struct group_device *grp_dev; - struct device *dev; int ret, req_type; if (!capable(CAP_SYS_ADMIN) || !capable(CAP_SYS_RAWIO)) @@ -2976,10 +2955,7 @@ static ssize_t iommu_group_store_type(struct iommu_group *group, return -EPERM; } - grp_dev = list_first_entry(&group->devices, struct group_device, list); - dev = grp_dev->dev; - - ret = iommu_change_dev_def_domain(group, dev, req_type); + ret = iommu_setup_default_domain(group, req_type); /* * Release the mutex here because ops->probe_finalize() call-back of From 1000dccd5d134951d5fd37a7ad85ad7b19b825fc Mon Sep 17 00:00:00 2001 From: Jason Gunthorpe Date: Thu, 11 May 2023 01:42:13 -0300 Subject: [PATCH 21/50] iommu: Allow IOMMU_RESV_DIRECT to work on ARM For now several ARM drivers do not allow mappings to be created until a domain is attached. This means they do not technically support IOMMU_RESV_DIRECT as it requires the 1:1 maps to work continuously. Currently if the platform requests these maps on ARM systems they are silently ignored. Work around this by trying again to establish the direct mappings after the domain is attached if the pre-attach attempt failed. In the long run the drivers will be fixed to fully setup domains when they are created without waiting for attachment. Reviewed-by: Lu Baolu Reviewed-by: Kevin Tian Tested-by: Heiko Stuebner Tested-by: Niklas Schnelle Signed-off-by: Jason Gunthorpe Link: https://lore.kernel.org/r/15-v5-1b99ae392328+44574-iommu_err_unwind_jgg@nvidia.com Signed-off-by: Joerg Roedel --- drivers/iommu/iommu.c | 33 +++++++++++++++++++++++++++++++-- 1 file changed, 31 insertions(+), 2 deletions(-) diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c index 34f721434b28..197d46a1d068 100644 --- a/drivers/iommu/iommu.c +++ b/drivers/iommu/iommu.c @@ -2842,6 +2842,7 @@ static int iommu_setup_default_domain(struct iommu_group *group, struct iommu_domain *old_dom = group->default_domain; struct group_device *gdev; struct iommu_domain *dom; + bool direct_failed; int req_type; int ret; @@ -2875,8 +2876,15 @@ static int iommu_setup_default_domain(struct iommu_group *group, * mapped before their device is attached, in order to guarantee * continuity with any FW activity */ - for_each_group_device(group, gdev) - iommu_create_device_direct_mappings(dom, gdev->dev); + direct_failed = false; + for_each_group_device(group, gdev) { + if (iommu_create_device_direct_mappings(dom, gdev->dev)) { + direct_failed = true; + dev_warn_once( + gdev->dev->iommu->iommu_dev->dev, + "IOMMU driver was not able to establish FW requested direct mapping."); + } + } /* We must set default_domain early for __iommu_device_set_domain */ group->default_domain = dom; @@ -2900,6 +2908,27 @@ static int iommu_setup_default_domain(struct iommu_group *group, } } + /* + * Drivers are supposed to allow mappings to be installed in a domain + * before device attachment, but some don't. Hack around this defect by + * trying again after attaching. If this happens it means the device + * will not continuously have the IOMMU_RESV_DIRECT map. + */ + if (direct_failed) { + for_each_group_device(group, gdev) { + ret = iommu_create_device_direct_mappings(dom, gdev->dev); + if (ret) + goto err_restore; + } + } + +err_restore: + if (old_dom) { + __iommu_group_set_domain_internal( + group, old_dom, IOMMU_SET_DOMAIN_MUST_SUCCEED); + iommu_domain_free(dom); + old_dom = NULL; + } out_free: if (old_dom) iommu_domain_free(old_dom); From e996c12d76d0b1aa8d5f082c6074e82398061943 Mon Sep 17 00:00:00 2001 From: Jason Gunthorpe Date: Thu, 11 May 2023 01:42:14 -0300 Subject: [PATCH 22/50] iommu: Remove __iommu_group_for_each_dev() The last two users of it are quite trivial, just open code the one line loop. Reviewed-by: Lu Baolu Tested-by: Heiko Stuebner Tested-by: Niklas Schnelle Signed-off-by: Jason Gunthorpe Link: https://lore.kernel.org/r/16-v5-1b99ae392328+44574-iommu_err_unwind_jgg@nvidia.com Signed-off-by: Joerg Roedel --- drivers/iommu/iommu.c | 53 ++++++++++++++++++++----------------------- 1 file changed, 25 insertions(+), 28 deletions(-) diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c index 197d46a1d068..1aaf3eb6fcca 100644 --- a/drivers/iommu/iommu.c +++ b/drivers/iommu/iommu.c @@ -1110,20 +1110,6 @@ void iommu_group_remove_device(struct device *dev) } EXPORT_SYMBOL_GPL(iommu_group_remove_device); -static int __iommu_group_for_each_dev(struct iommu_group *group, void *data, - int (*fn)(struct device *, void *)) -{ - struct group_device *device; - int ret = 0; - - for_each_group_device(group, device) { - ret = fn(device->dev, data); - if (ret) - break; - } - return ret; -} - /** * iommu_group_for_each_dev - iterate over each device in the group * @group: the group @@ -1138,10 +1124,15 @@ static int __iommu_group_for_each_dev(struct iommu_group *group, void *data, int iommu_group_for_each_dev(struct iommu_group *group, void *data, int (*fn)(struct device *, void *)) { - int ret; + struct group_device *device; + int ret = 0; mutex_lock(&group->mutex); - ret = __iommu_group_for_each_dev(group, data, fn); + for_each_group_device(group, device) { + ret = fn(device->dev, data); + if (ret) + break; + } mutex_unlock(&group->mutex); return ret; @@ -1791,20 +1782,12 @@ static int iommu_get_default_domain_type(struct iommu_group *group, return best_type; } -static int iommu_group_do_probe_finalize(struct device *dev, void *data) +static void iommu_group_do_probe_finalize(struct device *dev) { const struct iommu_ops *ops = dev_iommu_ops(dev); if (ops->probe_finalize) ops->probe_finalize(dev); - - return 0; -} - -static void __iommu_group_dma_finalize(struct iommu_group *group) -{ - __iommu_group_for_each_dev(group, group->default_domain, - iommu_group_do_probe_finalize); } int bus_iommu_probe(const struct bus_type *bus) @@ -1823,6 +1806,8 @@ int bus_iommu_probe(const struct bus_type *bus) return ret; list_for_each_entry_safe(group, next, &group_list, entry) { + struct group_device *gdev; + mutex_lock(&group->mutex); /* Remove item from the list */ @@ -1834,7 +1819,15 @@ int bus_iommu_probe(const struct bus_type *bus) return ret; } mutex_unlock(&group->mutex); - __iommu_group_dma_finalize(group); + + /* + * FIXME: Mis-locked because the ops->probe_finalize() call-back + * of some IOMMU drivers calls arm_iommu_attach_device() which + * in-turn might call back into IOMMU core code, where it tries + * to take group->mutex, resulting in a deadlock. + */ + for_each_group_device(group, gdev) + iommu_group_do_probe_finalize(gdev->dev); } return 0; @@ -2995,8 +2988,12 @@ static ssize_t iommu_group_store_type(struct iommu_group *group, mutex_unlock(&group->mutex); /* Make sure dma_ops is appropriatley set */ - if (!ret) - __iommu_group_dma_finalize(group); + if (!ret) { + struct group_device *gdev; + + for_each_group_device(group, gdev) + iommu_group_do_probe_finalize(gdev->dev); + } return ret ?: count; } From 5957c19305b10c73090b1390653ddf7e5e21aa35 Mon Sep 17 00:00:00 2001 From: Jason Gunthorpe Date: Thu, 11 May 2023 01:42:15 -0300 Subject: [PATCH 23/50] iommu: Tidy the control flow in iommu_group_store_type() Use a normal "goto unwind" instead of trying to be clever with checking !ret and manually managing the unlock. Reviewed-by: Lu Baolu Reviewed-by: Kevin Tian Tested-by: Heiko Stuebner Tested-by: Niklas Schnelle Signed-off-by: Jason Gunthorpe Link: https://lore.kernel.org/r/17-v5-1b99ae392328+44574-iommu_err_unwind_jgg@nvidia.com Signed-off-by: Joerg Roedel --- drivers/iommu/iommu.c | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c index 1aaf3eb6fcca..9e0228ef612b 100644 --- a/drivers/iommu/iommu.c +++ b/drivers/iommu/iommu.c @@ -2940,6 +2940,7 @@ out_free: static ssize_t iommu_group_store_type(struct iommu_group *group, const char *buf, size_t count) { + struct group_device *gdev; int ret, req_type; if (!capable(CAP_SYS_ADMIN) || !capable(CAP_SYS_RAWIO)) @@ -2964,20 +2965,23 @@ static ssize_t iommu_group_store_type(struct iommu_group *group, if (req_type == IOMMU_DOMAIN_DMA_FQ && group->default_domain->type == IOMMU_DOMAIN_DMA) { ret = iommu_dma_init_fq(group->default_domain); - if (!ret) - group->default_domain->type = IOMMU_DOMAIN_DMA_FQ; - mutex_unlock(&group->mutex); + if (ret) + goto out_unlock; - return ret ?: count; + group->default_domain->type = IOMMU_DOMAIN_DMA_FQ; + ret = count; + goto out_unlock; } /* Otherwise, ensure that device exists and no driver is bound. */ if (list_empty(&group->devices) || group->owner_cnt) { - mutex_unlock(&group->mutex); - return -EPERM; + ret = -EPERM; + goto out_unlock; } ret = iommu_setup_default_domain(group, req_type); + if (ret) + goto out_unlock; /* * Release the mutex here because ops->probe_finalize() call-back of @@ -2988,13 +2992,12 @@ static ssize_t iommu_group_store_type(struct iommu_group *group, mutex_unlock(&group->mutex); /* Make sure dma_ops is appropriatley set */ - if (!ret) { - struct group_device *gdev; - - for_each_group_device(group, gdev) - iommu_group_do_probe_finalize(gdev->dev); - } + for_each_group_device(group, gdev) + iommu_group_do_probe_finalize(gdev->dev); + return count; +out_unlock: + mutex_unlock(&group->mutex); return ret ?: count; } From 809d0810e3520da669d231303608cdf5fe5c1a70 Mon Sep 17 00:00:00 2001 From: Jean-Philippe Brucker Date: Mon, 15 May 2023 12:39:48 +0100 Subject: [PATCH 24/50] iommu/virtio: Detach domain on endpoint release When an endpoint is released, for example a PCIe VF being destroyed or a function hot-unplugged, it should be detached from its domain. Send a DETACH request. Fixes: edcd69ab9a32 ("iommu: Add virtio-iommu driver") Reported-by: Akihiko Odaki Link: https://lore.kernel.org/all/15bf1b00-3aa0-973a-3a86-3fa5c4d41d2c@daynix.com/ Signed-off-by: Jean-Philippe Brucker Tested-by: Akihiko Odaki Link: https://lore.kernel.org/r/20230515113946.1017624-2-jean-philippe@linaro.org Signed-off-by: Joerg Roedel --- drivers/iommu/virtio-iommu.c | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/drivers/iommu/virtio-iommu.c b/drivers/iommu/virtio-iommu.c index 5b8fe9bfa9a5..fd316a37d756 100644 --- a/drivers/iommu/virtio-iommu.c +++ b/drivers/iommu/virtio-iommu.c @@ -788,6 +788,29 @@ static int viommu_attach_dev(struct iommu_domain *domain, struct device *dev) return 0; } +static void viommu_detach_dev(struct viommu_endpoint *vdev) +{ + int i; + struct virtio_iommu_req_detach req; + struct viommu_domain *vdomain = vdev->vdomain; + struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(vdev->dev); + + if (!vdomain) + return; + + req = (struct virtio_iommu_req_detach) { + .head.type = VIRTIO_IOMMU_T_DETACH, + .domain = cpu_to_le32(vdomain->id), + }; + + for (i = 0; i < fwspec->num_ids; i++) { + req.endpoint = cpu_to_le32(fwspec->ids[i]); + WARN_ON(viommu_send_req_sync(vdev->viommu, &req, sizeof(req))); + } + vdomain->nr_endpoints--; + vdev->vdomain = NULL; +} + static int viommu_map_pages(struct iommu_domain *domain, unsigned long iova, phys_addr_t paddr, size_t pgsize, size_t pgcount, int prot, gfp_t gfp, size_t *mapped) @@ -990,6 +1013,7 @@ static void viommu_release_device(struct device *dev) { struct viommu_endpoint *vdev = dev_iommu_priv_get(dev); + viommu_detach_dev(vdev); iommu_put_resv_regions(dev, &vdev->resv_regions); kfree(vdev); } From 7061b6af34686e7e2364b7240cfb061293218f2d Mon Sep 17 00:00:00 2001 From: Jean-Philippe Brucker Date: Mon, 15 May 2023 12:39:50 +0100 Subject: [PATCH 25/50] iommu/virtio: Return size mapped for a detached domain When map() is called on a detached domain, the domain does not exist in the device so we do not send a MAP request, but we do update the internal mapping tree, to be replayed on the next attach. Since this constitutes a successful iommu_map() call, return *mapped in this case too. Fixes: 7e62edd7a33a ("iommu/virtio: Add map/unmap_pages() callbacks implementation") Signed-off-by: Jean-Philippe Brucker Reviewed-by: Jason Gunthorpe Link: https://lore.kernel.org/r/20230515113946.1017624-3-jean-philippe@linaro.org Signed-off-by: Joerg Roedel --- drivers/iommu/virtio-iommu.c | 33 +++++++++++++++++---------------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/drivers/iommu/virtio-iommu.c b/drivers/iommu/virtio-iommu.c index fd316a37d756..3551ed057774 100644 --- a/drivers/iommu/virtio-iommu.c +++ b/drivers/iommu/virtio-iommu.c @@ -833,25 +833,26 @@ static int viommu_map_pages(struct iommu_domain *domain, unsigned long iova, if (ret) return ret; - map = (struct virtio_iommu_req_map) { - .head.type = VIRTIO_IOMMU_T_MAP, - .domain = cpu_to_le32(vdomain->id), - .virt_start = cpu_to_le64(iova), - .phys_start = cpu_to_le64(paddr), - .virt_end = cpu_to_le64(end), - .flags = cpu_to_le32(flags), - }; + if (vdomain->nr_endpoints) { + map = (struct virtio_iommu_req_map) { + .head.type = VIRTIO_IOMMU_T_MAP, + .domain = cpu_to_le32(vdomain->id), + .virt_start = cpu_to_le64(iova), + .phys_start = cpu_to_le64(paddr), + .virt_end = cpu_to_le64(end), + .flags = cpu_to_le32(flags), + }; - if (!vdomain->nr_endpoints) - return 0; - - ret = viommu_send_req_sync(vdomain->viommu, &map, sizeof(map)); - if (ret) - viommu_del_mappings(vdomain, iova, end); - else if (mapped) + ret = viommu_send_req_sync(vdomain->viommu, &map, sizeof(map)); + if (ret) { + viommu_del_mappings(vdomain, iova, end); + return ret; + } + } + if (mapped) *mapped = size; - return ret; + return 0; } static size_t viommu_unmap_pages(struct iommu_domain *domain, unsigned long iova, From 5f6489723df9a292328a8defc02227d660eea1b9 Mon Sep 17 00:00:00 2001 From: Jason Gunthorpe Date: Tue, 16 May 2023 21:35:26 -0300 Subject: [PATCH 26/50] iommu/fsl: Always allocate a group for non-pci devices fsl_pamu_device_group() is only called if dev->iommu_group is NULL, so iommu_group_get() always returns NULL. Remove this test and just allocate a group. Call generic_device_group() for this like the other drivers. Signed-off-by: Jason Gunthorpe Tested-by: Michael Ellerman Link: https://lore.kernel.org/r/1-v2-ce71068deeec+4cf6-fsl_rm_groups_jgg@nvidia.com Signed-off-by: Joerg Roedel --- drivers/iommu/fsl_pamu_domain.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/iommu/fsl_pamu_domain.c b/drivers/iommu/fsl_pamu_domain.c index bce372297099..cd0c60b40215 100644 --- a/drivers/iommu/fsl_pamu_domain.c +++ b/drivers/iommu/fsl_pamu_domain.c @@ -437,7 +437,7 @@ static struct iommu_group *fsl_pamu_device_group(struct device *dev) if (dev_is_pci(dev)) group = get_pci_device_group(to_pci_dev(dev)); else if (of_get_property(dev->of_node, "fsl,liodn", &len)) - group = get_device_iommu_group(dev); + return generic_device_group(dev); return group; } From 7977a08e113268edd2f69432596b3a2a56f27298 Mon Sep 17 00:00:00 2001 From: Jason Gunthorpe Date: Tue, 16 May 2023 21:35:27 -0300 Subject: [PATCH 27/50] iommu/fsl: Move ENODEV to fsl_pamu_probe_device() The expectation is for the probe op to return ENODEV if the iommu is not able to support the device. Move the check for fsl,liodn to fsl_pamu_probe_device() simplify fsl_pamu_device_group() Signed-off-by: Jason Gunthorpe Tested-by: Michael Ellerman Link: https://lore.kernel.org/r/2-v2-ce71068deeec+4cf6-fsl_rm_groups_jgg@nvidia.com Signed-off-by: Joerg Roedel --- drivers/iommu/fsl_pamu_domain.c | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/drivers/iommu/fsl_pamu_domain.c b/drivers/iommu/fsl_pamu_domain.c index cd0c60b40215..d0683daa900f 100644 --- a/drivers/iommu/fsl_pamu_domain.c +++ b/drivers/iommu/fsl_pamu_domain.c @@ -427,23 +427,28 @@ static struct iommu_group *get_pci_device_group(struct pci_dev *pdev) static struct iommu_group *fsl_pamu_device_group(struct device *dev) { - struct iommu_group *group = ERR_PTR(-ENODEV); - int len; - /* * For platform devices we allocate a separate group for * each of the devices. */ - if (dev_is_pci(dev)) - group = get_pci_device_group(to_pci_dev(dev)); - else if (of_get_property(dev->of_node, "fsl,liodn", &len)) + if (!dev_is_pci(dev)) return generic_device_group(dev); - return group; + return get_pci_device_group(to_pci_dev(dev)); } static struct iommu_device *fsl_pamu_probe_device(struct device *dev) { + int len; + + /* + * uboot must fill the fsl,liodn for platform devices to be supported by + * the iommu. + */ + if (!dev_is_pci(dev) && + !of_get_property(dev->of_node, "fsl,liodn", &len)) + return ERR_PTR(-ENODEV); + return &pamu_iommu; } From 139a57a9918ede7205e56070e41ba00a5f62799e Mon Sep 17 00:00:00 2001 From: Jason Gunthorpe Date: Tue, 16 May 2023 21:35:28 -0300 Subject: [PATCH 28/50] iommu/fsl: Use driver_managed_dma to allow VFIO to work The FSL driver is mangling the iommu_groups to not have a group for its PCI bridge/controller (eg the thing passed to fsl_add_bridge()). Robin says this is so FSL could work with VFIO which would be blocked by having a probed driver on the platform_device in the same group. This is supported by comments from FSL: https://lore.kernel.org/all/C5ECD7A89D1DC44195F34B25E172658D459471@039-SN2MPN1-013.039d.mgd.msft.net .. PCIe devices share the same device group as the PCI controller. This becomes a problem while assigning the devices to the guest, as you are required to unbind all the PCIe devices including the controller from the host. PCIe controller can't be unbound from the host, so we simply delete the controller iommu_group. However, today, we use driver_managed_dma to allow PCI infrastructure devices that are 'security safe' to co-exist in groups and still allow VFIO to work. Set this flag for the fsl_pci_driver. Change fsl_pamu_device_group() so that it no longer removes the controller from any groups. For check_pci_ctl_endpt_part() mode this creates an extra group that contains only the controller. Otherwise force the controller's single group to be the group of all the PCI devices on the controller's hose. VFIO continues to work because of driver_managed_dma. Remove the iommu_group_remove_device() calls from fsl_pamu and lightly restructure its fsl_pamu_device_group() function. Signed-off-by: Jason Gunthorpe Tested-by: Michael Ellerman Link: https://lore.kernel.org/r/3-v2-ce71068deeec+4cf6-fsl_rm_groups_jgg@nvidia.com Signed-off-by: Joerg Roedel --- arch/powerpc/sysdev/fsl_pci.c | 1 + drivers/iommu/fsl_pamu_domain.c | 108 ++++++++------------------------ 2 files changed, 26 insertions(+), 83 deletions(-) diff --git a/arch/powerpc/sysdev/fsl_pci.c b/arch/powerpc/sysdev/fsl_pci.c index b7232c46b244..6daf620b63a4 100644 --- a/arch/powerpc/sysdev/fsl_pci.c +++ b/arch/powerpc/sysdev/fsl_pci.c @@ -1353,6 +1353,7 @@ static struct platform_driver fsl_pci_driver = { .of_match_table = pci_ids, }, .probe = fsl_pci_probe, + .driver_managed_dma = true, }; static int __init fsl_pci_init(void) diff --git a/drivers/iommu/fsl_pamu_domain.c b/drivers/iommu/fsl_pamu_domain.c index d0683daa900f..4ac0e247ec2b 100644 --- a/drivers/iommu/fsl_pamu_domain.c +++ b/drivers/iommu/fsl_pamu_domain.c @@ -334,17 +334,6 @@ int fsl_pamu_configure_l1_stash(struct iommu_domain *domain, u32 cpu) return ret; } -static struct iommu_group *get_device_iommu_group(struct device *dev) -{ - struct iommu_group *group; - - group = iommu_group_get(dev); - if (!group) - group = iommu_group_alloc(); - - return group; -} - static bool check_pci_ctl_endpt_part(struct pci_controller *pci_ctl) { u32 version; @@ -356,85 +345,38 @@ static bool check_pci_ctl_endpt_part(struct pci_controller *pci_ctl) return version >= 0x204; } -/* Get iommu group information from peer devices or devices on the parent bus */ -static struct iommu_group *get_shared_pci_device_group(struct pci_dev *pdev) -{ - struct pci_dev *tmp; - struct iommu_group *group; - struct pci_bus *bus = pdev->bus; - - /* - * Traverese the pci bus device list to get - * the shared iommu group. - */ - while (bus) { - list_for_each_entry(tmp, &bus->devices, bus_list) { - if (tmp == pdev) - continue; - group = iommu_group_get(&tmp->dev); - if (group) - return group; - } - - bus = bus->parent; - } - - return NULL; -} - -static struct iommu_group *get_pci_device_group(struct pci_dev *pdev) -{ - struct pci_controller *pci_ctl; - bool pci_endpt_partitioning; - struct iommu_group *group = NULL; - - pci_ctl = pci_bus_to_host(pdev->bus); - pci_endpt_partitioning = check_pci_ctl_endpt_part(pci_ctl); - /* We can partition PCIe devices so assign device group to the device */ - if (pci_endpt_partitioning) { - group = pci_device_group(&pdev->dev); - - /* - * PCIe controller is not a paritionable entity - * free the controller device iommu_group. - */ - if (pci_ctl->parent->iommu_group) - iommu_group_remove_device(pci_ctl->parent); - } else { - /* - * All devices connected to the controller will share the - * PCI controllers device group. If this is the first - * device to be probed for the pci controller, copy the - * device group information from the PCI controller device - * node and remove the PCI controller iommu group. - * For subsequent devices, the iommu group information can - * be obtained from sibling devices (i.e. from the bus_devices - * link list). - */ - if (pci_ctl->parent->iommu_group) { - group = get_device_iommu_group(pci_ctl->parent); - iommu_group_remove_device(pci_ctl->parent); - } else { - group = get_shared_pci_device_group(pdev); - } - } - - if (!group) - group = ERR_PTR(-ENODEV); - - return group; -} - static struct iommu_group *fsl_pamu_device_group(struct device *dev) { + struct iommu_group *group; + struct pci_dev *pdev; + /* - * For platform devices we allocate a separate group for - * each of the devices. + * For platform devices we allocate a separate group for each of the + * devices. */ if (!dev_is_pci(dev)) return generic_device_group(dev); - return get_pci_device_group(to_pci_dev(dev)); + /* + * We can partition PCIe devices so assign device group to the device + */ + pdev = to_pci_dev(dev); + if (check_pci_ctl_endpt_part(pci_bus_to_host(pdev->bus))) + return pci_device_group(&pdev->dev); + + /* + * All devices connected to the controller will share the same device + * group. + * + * Due to ordering between fsl_pamu_init() and fsl_pci_init() it is + * guaranteed that the pci_ctl->parent platform_device will have the + * iommu driver bound and will already have a group set. So we just + * re-use this group as the group for every device in the hose. + */ + group = iommu_group_get(pci_bus_to_host(pdev->bus)->parent); + if (WARN_ON(!group)) + return ERR_PTR(-EINVAL); + return group; } static struct iommu_device *fsl_pamu_probe_device(struct device *dev) From 84b8a7fe29205016cffd4eff91b45830d318b53d Mon Sep 17 00:00:00 2001 From: Bjorn Andersson Date: Mon, 22 May 2023 18:04:41 -0700 Subject: [PATCH 29/50] dt-bindings: arm-smmu: Fix SC8280XP Adreno binding The qcom,sc8280xp-smmu-500 Adreno SMMU binding has clocks, so fix up the binding to allow this. Fixes: 38db6b41b2f4 ("dt-bindings: arm-smmu: Add compatible for Qualcomm SC8280XP") Signed-off-by: Bjorn Andersson Acked-by: Conor Dooley Link: https://lore.kernel.org/r/20230523010441.63236-1-quic_bjorande@quicinc.com Signed-off-by: Will Deacon --- Documentation/devicetree/bindings/iommu/arm,smmu.yaml | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml index ba677d401e24..6cb04f35642a 100644 --- a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml @@ -80,6 +80,7 @@ properties: items: - enum: - qcom,sc7280-smmu-500 + - qcom,sc8280xp-smmu-500 - qcom,sm6115-smmu-500 - qcom,sm6125-smmu-500 - qcom,sm8150-smmu-500 @@ -331,7 +332,9 @@ allOf: properties: compatible: contains: - const: qcom,sc7280-smmu-500 + enum: + - qcom,sc7280-smmu-500 + - qcom,sc8280xp-smmu-500 then: properties: clock-names: @@ -416,7 +419,6 @@ allOf: - qcom,sa8775p-smmu-500 - qcom,sc7180-smmu-500 - qcom,sc8180x-smmu-500 - - qcom,sc8280xp-smmu-500 - qcom,sdm670-smmu-500 - qcom,sdm845-smmu-500 - qcom,sdx55-smmu-500 From 387a80a7412543e2efe6bbf7dc150100900fc415 Mon Sep 17 00:00:00 2001 From: Bartosz Golaszewski Date: Mon, 17 Apr 2023 14:58:43 +0200 Subject: [PATCH 30/50] dt-bindings: iommu: arm,smmu: enable clocks for sa8775p Adreno SMMU The GPU SMMU will require the clocks property to be set so put the relevant compatible into the adreno if-then block. Signed-off-by: Bartosz Golaszewski Acked-by: Krzysztof Kozlowski Link: https://lore.kernel.org/r/20230417125844.400782-5-brgl@bgdev.pl [will: Fixed conflict with 'qcom,sc8280xp-smmu-500' entry] Signed-off-by: Will Deacon --- Documentation/devicetree/bindings/iommu/arm,smmu.yaml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml index 6cb04f35642a..29965ea7dcd8 100644 --- a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml @@ -79,6 +79,7 @@ properties: - description: Qcom Adreno GPUs implementing "qcom,smmu-500" and "arm,mmu-500" items: - enum: + - qcom,sa8775p-smmu-500 - qcom,sc7280-smmu-500 - qcom,sc8280xp-smmu-500 - qcom,sm6115-smmu-500 @@ -333,6 +334,7 @@ allOf: compatible: contains: enum: + - qcom,sa8775p-smmu-500 - qcom,sc7280-smmu-500 - qcom,sc8280xp-smmu-500 then: @@ -416,7 +418,6 @@ allOf: - nvidia,smmu-500 - qcom,qcm2290-smmu-500 - qcom,qdu1000-smmu-500 - - qcom,sa8775p-smmu-500 - qcom,sc7180-smmu-500 - qcom,sc8180x-smmu-500 - qcom,sdm670-smmu-500 From 44984d56e0598d7e80b9e544aaffd6700f5cb578 Mon Sep 17 00:00:00 2001 From: Konrad Dybcio Date: Wed, 31 May 2023 17:04:23 +0200 Subject: [PATCH 31/50] dt-bindings: arm-smmu: Add SM6375 GPU SMMU SM6375 has a "Qualcomm SMMU V2" implementation for its GPU SMMU. It does not however qualify for the qcom,adreno-smmu compatible, as it can not do split pagetables. It consumes a single clock and a single genpd. Signed-off-by: Konrad Dybcio Acked-by: Krzysztof Kozlowski Link: https://lore.kernel.org/r/20230531-topic-sm6375_gpusmmu-v1-1-860943894c71@linaro.org Signed-off-by: Will Deacon --- Documentation/devicetree/bindings/iommu/arm,smmu.yaml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml index 29965ea7dcd8..b86b7544f231 100644 --- a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml @@ -29,6 +29,7 @@ properties: - qcom,msm8996-smmu-v2 - qcom,msm8998-smmu-v2 - qcom,sdm630-smmu-v2 + - qcom,sm6375-smmu-v2 - const: qcom,smmu-v2 - description: Qcom SoCs implementing "qcom,smmu-500" and "arm,mmu-500" @@ -269,6 +270,7 @@ allOf: enum: - qcom,msm8998-smmu-v2 - qcom,sdm630-smmu-v2 + - qcom,sm6375-smmu-v2 then: anyOf: - properties: From 48989c0b25ca6ed75f3ea81053936ff0b64d02e7 Mon Sep 17 00:00:00 2001 From: Rohit Agarwal Date: Fri, 19 May 2023 14:39:06 +0530 Subject: [PATCH 32/50] dt-bindings: arm-smmu: Add SDX75 SMMU compatible Add devicetree binding for Qualcomm SDX75 SMMU. Signed-off-by: Rohit Agarwal Acked-by: Conor Dooley Link: https://lore.kernel.org/r/1684487350-30476-5-git-send-email-quic_rohiagar@quicinc.com Signed-off-by: Will Deacon --- Documentation/devicetree/bindings/iommu/arm,smmu.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml index b86b7544f231..3a31a979709b 100644 --- a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml @@ -46,6 +46,7 @@ properties: - qcom,sdm845-smmu-500 - qcom,sdx55-smmu-500 - qcom,sdx65-smmu-500 + - qcom,sdx75-smmu-500 - qcom,sm6115-smmu-500 - qcom,sm6125-smmu-500 - qcom,sm6350-smmu-500 From f322e8af35c7f23a8c08b595c38d6c855b2d836f Mon Sep 17 00:00:00 2001 From: Robin Murphy Date: Wed, 10 May 2023 16:38:43 +0100 Subject: [PATCH 33/50] iommu/arm-smmu-v3: Work around MMU-600 erratum 1076982 MMU-600 versions prior to r1p0 fail to correctly generate a WFE wakeup event when the command queue transitions fom full to non-full. We can easily work around this by simply hiding the SEV capability such that we fall back to polling for space in the queue - since MMU-600 implements MSIs we wouldn't expect to need SEV for sync completion either, so this should have little to no impact. Signed-off-by: Robin Murphy Reviewed-by: Nicolin Chen Tested-by: Nicolin Chen Link: https://lore.kernel.org/r/08adbe3d01024d8382a478325f73b56851f76e49.1683731256.git.robin.murphy@arm.com Signed-off-by: Will Deacon --- Documentation/arm64/silicon-errata.rst | 2 ++ drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 29 +++++++++++++++++++++ drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 6 +++++ 3 files changed, 37 insertions(+) diff --git a/Documentation/arm64/silicon-errata.rst b/Documentation/arm64/silicon-errata.rst index 9e311bc43e05..951d8d42c248 100644 --- a/Documentation/arm64/silicon-errata.rst +++ b/Documentation/arm64/silicon-errata.rst @@ -140,6 +140,8 @@ stable kernels. +----------------+-----------------+-----------------+-----------------------------+ | ARM | MMU-500 | #841119,826419 | N/A | +----------------+-----------------+-----------------+-----------------------------+ +| ARM | MMU-600 | #1076982 | N/A | ++----------------+-----------------+-----------------+-----------------------------+ +----------------+-----------------+-----------------+-----------------------------+ | Broadcom | Brahma-B53 | N/A | ARM64_ERRATUM_845719 | +----------------+-----------------+-----------------+-----------------------------+ diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c index 3fd83fb75722..667e7a90706e 100644 --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c @@ -3429,6 +3429,33 @@ static int arm_smmu_device_reset(struct arm_smmu_device *smmu, bool bypass) return 0; } +#define IIDR_IMPLEMENTER_ARM 0x43b +#define IIDR_PRODUCTID_ARM_MMU_600 0x483 + +static void arm_smmu_device_iidr_probe(struct arm_smmu_device *smmu) +{ + u32 reg; + unsigned int implementer, productid, variant, revision; + + reg = readl_relaxed(smmu->base + ARM_SMMU_IIDR); + implementer = FIELD_GET(IIDR_IMPLEMENTER, reg); + productid = FIELD_GET(IIDR_PRODUCTID, reg); + variant = FIELD_GET(IIDR_VARIANT, reg); + revision = FIELD_GET(IIDR_REVISION, reg); + + switch (implementer) { + case IIDR_IMPLEMENTER_ARM: + switch (productid) { + case IIDR_PRODUCTID_ARM_MMU_600: + /* Arm erratum 1076982 */ + if (variant == 0 && revision <= 2) + smmu->features &= ~ARM_SMMU_FEAT_SEV; + break; + } + break; + } +} + static int arm_smmu_device_hw_probe(struct arm_smmu_device *smmu) { u32 reg; @@ -3635,6 +3662,8 @@ static int arm_smmu_device_hw_probe(struct arm_smmu_device *smmu) smmu->ias = max(smmu->ias, smmu->oas); + arm_smmu_device_iidr_probe(smmu); + if (arm_smmu_sva_supported(smmu)) smmu->features |= ARM_SMMU_FEAT_SVA; diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h index b574c58a3487..5ce47f2e3402 100644 --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h @@ -69,6 +69,12 @@ #define IDR5_VAX GENMASK(11, 10) #define IDR5_VAX_52_BIT 1 +#define ARM_SMMU_IIDR 0x18 +#define IIDR_PRODUCTID GENMASK(31, 20) +#define IIDR_VARIANT GENMASK(19, 16) +#define IIDR_REVISION GENMASK(15, 12) +#define IIDR_IMPLEMENTER GENMASK(11, 0) + #define ARM_SMMU_CR0 0x20 #define CR0_ATSCHK (1 << 4) #define CR0_CMDQEN (1 << 3) From 309a15cb16bb075da1c99d46fb457db6a1a2669e Mon Sep 17 00:00:00 2001 From: Robin Murphy Date: Wed, 10 May 2023 16:38:44 +0100 Subject: [PATCH 34/50] iommu/arm-smmu-v3: Document MMU-700 erratum 2812531 To work around MMU-700 erratum 2812531 we need to ensure that certain sequences of commands cannot be issued without an intervening sync. In practice this falls out of our current command-batching machinery anyway - each batch only contains a single type of invalidation command, and ends with a sync. The only exception is when a batch is sufficiently large to need issuing across multiple command queue slots, wherein the earlier slots will not contain a sync and thus may in theory interleave with another batch being issued in parallel to create an affected sequence across the slot boundary. Since MMU-700 supports range invalidate commands and thus we will prefer to use them (which also happens to avoid conditions for other errata), I'm not entirely sure it's even possible for a single high-level invalidate call to generate a batch of more than 63 commands, but for the sake of robustness and documentation, wire up an option to enforce that a sync is always inserted for every slot issued. The other aspect is that the relative order of DVM commands cannot be controlled, so DVM cannot be used. Again that is already the status quo, but since we have at least defined ARM_SMMU_FEAT_BTM, we can explicitly disable it for documentation purposes even if it's not wired up anywhere yet. Signed-off-by: Robin Murphy Reviewed-by: Nicolin Chen Link: https://lore.kernel.org/r/330221cdfd0003cd51b6c04e7ff3566741ad8374.1683731256.git.robin.murphy@arm.com Signed-off-by: Will Deacon --- Documentation/arm64/silicon-errata.rst | 2 ++ drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 12 ++++++++++++ drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 1 + 3 files changed, 15 insertions(+) diff --git a/Documentation/arm64/silicon-errata.rst b/Documentation/arm64/silicon-errata.rst index 951d8d42c248..84b58985a061 100644 --- a/Documentation/arm64/silicon-errata.rst +++ b/Documentation/arm64/silicon-errata.rst @@ -142,6 +142,8 @@ stable kernels. +----------------+-----------------+-----------------+-----------------------------+ | ARM | MMU-600 | #1076982 | N/A | +----------------+-----------------+-----------------+-----------------------------+ +| ARM | MMU-700 | #2812531 | N/A | ++----------------+-----------------+-----------------+-----------------------------+ +----------------+-----------------+-----------------+-----------------------------+ | Broadcom | Brahma-B53 | N/A | ARM64_ERRATUM_845719 | +----------------+-----------------+-----------------+-----------------------------+ diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c index 667e7a90706e..b0ccd735f8bb 100644 --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c @@ -894,6 +894,12 @@ static void arm_smmu_cmdq_batch_add(struct arm_smmu_device *smmu, { int index; + if (cmds->num == CMDQ_BATCH_ENTRIES - 1 && + (smmu->options & ARM_SMMU_OPT_CMDQ_FORCE_SYNC)) { + arm_smmu_cmdq_issue_cmdlist(smmu, cmds->cmds, cmds->num, true); + cmds->num = 0; + } + if (cmds->num == CMDQ_BATCH_ENTRIES) { arm_smmu_cmdq_issue_cmdlist(smmu, cmds->cmds, cmds->num, false); cmds->num = 0; @@ -3431,6 +3437,7 @@ static int arm_smmu_device_reset(struct arm_smmu_device *smmu, bool bypass) #define IIDR_IMPLEMENTER_ARM 0x43b #define IIDR_PRODUCTID_ARM_MMU_600 0x483 +#define IIDR_PRODUCTID_ARM_MMU_700 0x487 static void arm_smmu_device_iidr_probe(struct arm_smmu_device *smmu) { @@ -3451,6 +3458,11 @@ static void arm_smmu_device_iidr_probe(struct arm_smmu_device *smmu) if (variant == 0 && revision <= 2) smmu->features &= ~ARM_SMMU_FEAT_SEV; break; + case IIDR_PRODUCTID_ARM_MMU_700: + /* Arm erratum 2812531 */ + smmu->features &= ~ARM_SMMU_FEAT_BTM; + smmu->options |= ARM_SMMU_OPT_CMDQ_FORCE_SYNC; + break; } break; } diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h index 5ce47f2e3402..1555c8220381 100644 --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h @@ -650,6 +650,7 @@ struct arm_smmu_device { #define ARM_SMMU_OPT_SKIP_PREFETCH (1 << 0) #define ARM_SMMU_OPT_PAGE0_REGS_ONLY (1 << 1) #define ARM_SMMU_OPT_MSIPOLL (1 << 2) +#define ARM_SMMU_OPT_CMDQ_FORCE_SYNC (1 << 3) u32 options; struct arm_smmu_cmdq cmdq; From 1d9777b9f3d55b4b6faf186ba4f1d6fb560c0523 Mon Sep 17 00:00:00 2001 From: Robin Murphy Date: Wed, 10 May 2023 16:38:45 +0100 Subject: [PATCH 35/50] iommu/arm-smmu-v3: Add explicit feature for nesting In certain cases we may want to refuse to allow nested translation even when both stages are implemented, so let's add an explicit feature for nesting support which we can control in its own right. For now this merely serves as documentation, but it means a nice convenient check will be ready and waiting for the future nesting code. Signed-off-by: Robin Murphy Reviewed-by: Nicolin Chen Link: https://lore.kernel.org/r/136c3f4a3a84cc14a5a1978ace57dfd3ed67b688.1683731256.git.robin.murphy@arm.com Signed-off-by: Will Deacon --- drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 4 ++++ drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 1 + 2 files changed, 5 insertions(+) diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c index b0ccd735f8bb..7e08ec55df2a 100644 --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c @@ -3674,6 +3674,10 @@ static int arm_smmu_device_hw_probe(struct arm_smmu_device *smmu) smmu->ias = max(smmu->ias, smmu->oas); + if ((smmu->features & ARM_SMMU_FEAT_TRANS_S1) && + (smmu->features & ARM_SMMU_FEAT_TRANS_S2)) + smmu->features |= ARM_SMMU_FEAT_NESTING; + arm_smmu_device_iidr_probe(smmu); if (arm_smmu_sva_supported(smmu)) diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h index 1555c8220381..dcab85698a4e 100644 --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h @@ -645,6 +645,7 @@ struct arm_smmu_device { #define ARM_SMMU_FEAT_BTM (1 << 16) #define ARM_SMMU_FEAT_SVA (1 << 17) #define ARM_SMMU_FEAT_E2H (1 << 18) +#define ARM_SMMU_FEAT_NESTING (1 << 19) u32 features; #define ARM_SMMU_OPT_SKIP_PREFETCH (1 << 0) From 0bfbfc526c70606bf0fad302e4821087cbecfaf4 Mon Sep 17 00:00:00 2001 From: Robin Murphy Date: Wed, 10 May 2023 16:38:46 +0100 Subject: [PATCH 36/50] iommu/arm-smmu-v3: Document nesting-related errata Both MMU-600 and MMU-700 have similar errata around TLB invalidation while both stages of translation are active, which will need some consideration once nesting support is implemented. For now, though, it's very easy to make our implicit lack of nesting support explicit for those cases, so they're less likely to be missed in future. Signed-off-by: Robin Murphy Reviewed-by: Nicolin Chen Link: https://lore.kernel.org/r/696da78d32bb4491f898f11b0bb4d850a8aa7c6a.1683731256.git.robin.murphy@arm.com Signed-off-by: Will Deacon --- Documentation/arm64/silicon-errata.rst | 4 ++-- drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 5 +++++ 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/Documentation/arm64/silicon-errata.rst b/Documentation/arm64/silicon-errata.rst index 84b58985a061..8a9e33d86dfd 100644 --- a/Documentation/arm64/silicon-errata.rst +++ b/Documentation/arm64/silicon-errata.rst @@ -140,9 +140,9 @@ stable kernels. +----------------+-----------------+-----------------+-----------------------------+ | ARM | MMU-500 | #841119,826419 | N/A | +----------------+-----------------+-----------------+-----------------------------+ -| ARM | MMU-600 | #1076982 | N/A | +| ARM | MMU-600 | #1076982,1209401| N/A | +----------------+-----------------+-----------------+-----------------------------+ -| ARM | MMU-700 | #2812531 | N/A | +| ARM | MMU-700 | #2268618,2812531| N/A | +----------------+-----------------+-----------------+-----------------------------+ +----------------+-----------------+-----------------+-----------------------------+ | Broadcom | Brahma-B53 | N/A | ARM64_ERRATUM_845719 | diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c index 7e08ec55df2a..bbad54aa6c8c 100644 --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c @@ -3457,11 +3457,16 @@ static void arm_smmu_device_iidr_probe(struct arm_smmu_device *smmu) /* Arm erratum 1076982 */ if (variant == 0 && revision <= 2) smmu->features &= ~ARM_SMMU_FEAT_SEV; + /* Arm erratum 1209401 */ + if (variant < 2) + smmu->features &= ~ARM_SMMU_FEAT_NESTING; break; case IIDR_PRODUCTID_ARM_MMU_700: /* Arm erratum 2812531 */ smmu->features &= ~ARM_SMMU_FEAT_BTM; smmu->options |= ARM_SMMU_OPT_CMDQ_FORCE_SYNC; + /* Arm errata 2268618, 2812531 */ + smmu->features &= ~ARM_SMMU_FEAT_NESTING; break; } break; From 6833b8f2e19945a41e4d5efd8c6d9f4cae9a5b7d Mon Sep 17 00:00:00 2001 From: Robin Murphy Date: Thu, 1 Jun 2023 17:43:33 +0100 Subject: [PATCH 37/50] iommu/arm-smmu-v3: Set TTL invalidation hint better When io-pgtable unmaps a whole table, rather than waste time walking it to find the leaf entries to invalidate exactly, it simply expects .tlb_flush_walk with nominal last-level granularity to invalidate any leaf entries at higher intermediate levels as well. This works fine with page-based invalidation, but with range commands we need to be careful with the TTL hint - unconditionally setting it based on the given level 3 granule means that an invalidation for a level 1 table would strictly not be required to affect level 2 block entries. It's easy to comply with the expected behaviour by simply not setting the TTL hint for non-leaf invalidations, so let's do that. Signed-off-by: Robin Murphy Link: https://lore.kernel.org/r/b409d9a17c52dc0db51faee91d92737bb7975f5b.1685637456.git.robin.murphy@arm.com Signed-off-by: Will Deacon --- drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c index bbad54aa6c8c..bbafb9639e5a 100644 --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c @@ -1898,8 +1898,13 @@ static void __arm_smmu_tlb_inv_range(struct arm_smmu_cmdq_ent *cmd, /* Convert page size of 12,14,16 (log2) to 1,2,3 */ cmd->tlbi.tg = (tg - 10) / 2; - /* Determine what level the granule is at */ - cmd->tlbi.ttl = 4 - ((ilog2(granule) - 3) / (tg - 3)); + /* + * Determine what level the granule is at. For non-leaf, io-pgtable + * assumes .tlb_flush_walk can invalidate multiple levels at once, + * so ignore the nominal last-level granule and leave TTL=0. + */ + if (cmd->tlbi.leaf) + cmd->tlbi.ttl = 4 - ((ilog2(granule) - 3) / (tg - 3)); num_pages = size >> tg; } From a42f0c7a4118ffa395866cc7f5522d71a86fc4dd Mon Sep 17 00:00:00 2001 From: Joao Martins Date: Tue, 30 May 2023 10:11:33 -0400 Subject: [PATCH 38/50] iommu/amd: Switch amd_iommu_update_ga() to use modify_irte_ga() The modify_irte_ga() uses cmpxchg_double() to update the IRTE in one shot, which is necessary when adding IRTE cache disabling support since the driver no longer need to flush the IRT for hardware to take effect. Please note that there is a functional change where the IsRun and Destination bits of IRTE are now cached in the struct amd_ir_data.entry. Reviewed-by: Jerry Snitselaar Signed-off-by: Joao Martins Signed-off-by: Suravee Suthikulpanit Link: https://lore.kernel.org/r/20230530141137.14376-2-suravee.suthikulpanit@amd.com Signed-off-by: Joerg Roedel --- drivers/iommu/amd/iommu.c | 38 ++++++++++---------------------------- 1 file changed, 10 insertions(+), 28 deletions(-) diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c index 4a314647d1f7..12d6844128f9 100644 --- a/drivers/iommu/amd/iommu.c +++ b/drivers/iommu/amd/iommu.c @@ -3702,44 +3702,26 @@ int amd_iommu_create_irq_domain(struct amd_iommu *iommu) int amd_iommu_update_ga(int cpu, bool is_run, void *data) { - unsigned long flags; - struct amd_iommu *iommu; - struct irq_remap_table *table; struct amd_ir_data *ir_data = (struct amd_ir_data *)data; - int devid = ir_data->irq_2_irte.devid; struct irte_ga *entry = (struct irte_ga *) ir_data->entry; - struct irte_ga *ref = (struct irte_ga *) ir_data->ref; if (!AMD_IOMMU_GUEST_IR_VAPIC(amd_iommu_guest_ir) || - !ref || !entry || !entry->lo.fields_vapic.guest_mode) + !entry || !entry->lo.fields_vapic.guest_mode) return 0; - iommu = ir_data->iommu; - if (!iommu) + if (!ir_data->iommu) return -ENODEV; - table = get_irq_table(iommu, devid); - if (!table) - return -ENODEV; - - raw_spin_lock_irqsave(&table->lock, flags); - - if (ref->lo.fields_vapic.guest_mode) { - if (cpu >= 0) { - ref->lo.fields_vapic.destination = - APICID_TO_IRTE_DEST_LO(cpu); - ref->hi.fields.destination = - APICID_TO_IRTE_DEST_HI(cpu); - } - ref->lo.fields_vapic.is_run = is_run; - barrier(); + if (cpu >= 0) { + entry->lo.fields_vapic.destination = + APICID_TO_IRTE_DEST_LO(cpu); + entry->hi.fields.destination = + APICID_TO_IRTE_DEST_HI(cpu); } + entry->lo.fields_vapic.is_run = is_run; - raw_spin_unlock_irqrestore(&table->lock, flags); - - iommu_flush_irt(iommu, devid); - iommu_completion_wait(iommu); - return 0; + return modify_irte_ga(ir_data->iommu, ir_data->irq_2_irte.devid, + ir_data->irq_2_irte.index, entry, ir_data); } EXPORT_SYMBOL(amd_iommu_update_ga); #endif From 74a37817bd1567330fb372eb01223e31b45b1cc0 Mon Sep 17 00:00:00 2001 From: Suravee Suthikulpanit Date: Tue, 30 May 2023 10:11:34 -0400 Subject: [PATCH 39/50] iommu/amd: Remove the unused struct amd_ir_data.ref Since the amd_iommu_update_ga() has been switched to use the modify_irte_ga() helper function to update the IRTE, the parameter is no longer needed. Reviewed-by: Jerry Snitselaar Suggested-by: Vasant Hegde Signed-off-by: Suravee Suthikulpanit Link: https://lore.kernel.org/r/20230530141137.14376-3-suravee.suthikulpanit@amd.com Signed-off-by: Joerg Roedel --- drivers/iommu/amd/amd_iommu_types.h | 1 - drivers/iommu/amd/iommu.c | 17 +++++++---------- 2 files changed, 7 insertions(+), 11 deletions(-) diff --git a/drivers/iommu/amd/amd_iommu_types.h b/drivers/iommu/amd/amd_iommu_types.h index 2ddbda3a4374..8c072be68875 100644 --- a/drivers/iommu/amd/amd_iommu_types.h +++ b/drivers/iommu/amd/amd_iommu_types.h @@ -1001,7 +1001,6 @@ struct amd_ir_data { struct irq_2_irte irq_2_irte; struct msi_msg msi_entry; void *entry; /* Pointer to union irte or struct irte_ga */ - void *ref; /* Pointer to the actual irte */ /** * Store information for activate/de-activate diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c index 12d6844128f9..e6daf0f39fd8 100644 --- a/drivers/iommu/amd/iommu.c +++ b/drivers/iommu/amd/iommu.c @@ -3001,7 +3001,7 @@ out: } static int modify_irte_ga(struct amd_iommu *iommu, u16 devid, int index, - struct irte_ga *irte, struct amd_ir_data *data) + struct irte_ga *irte) { bool ret; struct irq_remap_table *table; @@ -3028,9 +3028,6 @@ static int modify_irte_ga(struct amd_iommu *iommu, u16 devid, int index, */ WARN_ON(!ret); - if (data) - data->ref = entry; - raw_spin_unlock_irqrestore(&table->lock, flags); iommu_flush_irt(iommu, devid); @@ -3119,7 +3116,7 @@ static void irte_ga_activate(struct amd_iommu *iommu, void *entry, u16 devid, u1 struct irte_ga *irte = (struct irte_ga *) entry; irte->lo.fields_remap.valid = 1; - modify_irte_ga(iommu, devid, index, irte, NULL); + modify_irte_ga(iommu, devid, index, irte); } static void irte_deactivate(struct amd_iommu *iommu, void *entry, u16 devid, u16 index) @@ -3135,7 +3132,7 @@ static void irte_ga_deactivate(struct amd_iommu *iommu, void *entry, u16 devid, struct irte_ga *irte = (struct irte_ga *) entry; irte->lo.fields_remap.valid = 0; - modify_irte_ga(iommu, devid, index, irte, NULL); + modify_irte_ga(iommu, devid, index, irte); } static void irte_set_affinity(struct amd_iommu *iommu, void *entry, u16 devid, u16 index, @@ -3159,7 +3156,7 @@ static void irte_ga_set_affinity(struct amd_iommu *iommu, void *entry, u16 devid APICID_TO_IRTE_DEST_LO(dest_apicid); irte->hi.fields.destination = APICID_TO_IRTE_DEST_HI(dest_apicid); - modify_irte_ga(iommu, devid, index, irte, NULL); + modify_irte_ga(iommu, devid, index, irte); } } @@ -3510,7 +3507,7 @@ int amd_iommu_activate_guest_mode(void *data) entry->lo.fields_vapic.ga_tag = ir_data->ga_tag; return modify_irte_ga(ir_data->iommu, ir_data->irq_2_irte.devid, - ir_data->irq_2_irte.index, entry, ir_data); + ir_data->irq_2_irte.index, entry); } EXPORT_SYMBOL(amd_iommu_activate_guest_mode); @@ -3540,7 +3537,7 @@ int amd_iommu_deactivate_guest_mode(void *data) APICID_TO_IRTE_DEST_HI(cfg->dest_apicid); return modify_irte_ga(ir_data->iommu, ir_data->irq_2_irte.devid, - ir_data->irq_2_irte.index, entry, ir_data); + ir_data->irq_2_irte.index, entry); } EXPORT_SYMBOL(amd_iommu_deactivate_guest_mode); @@ -3721,7 +3718,7 @@ int amd_iommu_update_ga(int cpu, bool is_run, void *data) entry->lo.fields_vapic.is_run = is_run; return modify_irte_ga(ir_data->iommu, ir_data->irq_2_irte.devid, - ir_data->irq_2_irte.index, entry, ir_data); + ir_data->irq_2_irte.index, entry); } EXPORT_SYMBOL(amd_iommu_update_ga); #endif From 66419036f68a838c00cbccacd6cb2e99da6e5710 Mon Sep 17 00:00:00 2001 From: Suravee Suthikulpanit Date: Tue, 30 May 2023 10:11:35 -0400 Subject: [PATCH 40/50] iommu/amd: Introduce Disable IRTE Caching Support An Interrupt Remapping Table (IRT) stores interrupt remapping configuration for each device. In a normal operation, the AMD IOMMU caches the table to optimize subsequent data accesses. This requires the IOMMU driver to invalidate IRT whenever it updates the table. The invalidation process includes issuing an INVALIDATE_INTERRUPT_TABLE command following by a COMPLETION_WAIT command. However, there are cases in which the IRT is updated at a high rate. For example, for IOMMU AVIC, the IRTE[IsRun] bit is updated on every vcpu scheduling (i.e. amd_iommu_update_ga()). On system with large amount of vcpus and VFIO PCI pass-through devices, the invalidation process could potentially become a performance bottleneck. Introducing a new kernel boot option: amd_iommu=irtcachedis which disables IRTE caching by setting the IRTCachedis bit in each IOMMU Control register, and bypass the IRT invalidation process. Reviewed-by: Jerry Snitselaar Co-developed-by: Alejandro Jimenez Signed-off-by: Alejandro Jimenez Signed-off-by: Suravee Suthikulpanit Link: https://lore.kernel.org/r/20230530141137.14376-4-suravee.suthikulpanit@amd.com Signed-off-by: Joerg Roedel --- .../admin-guide/kernel-parameters.txt | 1 + drivers/iommu/amd/amd_iommu_types.h | 4 +++ drivers/iommu/amd/init.c | 36 +++++++++++++++++++ 3 files changed, 41 insertions(+) diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt index 9e5bab29685f..986ac2b73ea2 100644 --- a/Documentation/admin-guide/kernel-parameters.txt +++ b/Documentation/admin-guide/kernel-parameters.txt @@ -323,6 +323,7 @@ option with care. pgtbl_v1 - Use v1 page table for DMA-API (Default). pgtbl_v2 - Use v2 page table for DMA-API. + irtcachedis - Disable Interrupt Remapping Table (IRT) caching. amd_iommu_dump= [HW,X86-64] Enable AMD IOMMU driver option to dump the ACPI table diff --git a/drivers/iommu/amd/amd_iommu_types.h b/drivers/iommu/amd/amd_iommu_types.h index 8c072be68875..8eeea1f25e69 100644 --- a/drivers/iommu/amd/amd_iommu_types.h +++ b/drivers/iommu/amd/amd_iommu_types.h @@ -174,6 +174,7 @@ #define CONTROL_GAINT_EN 29 #define CONTROL_XT_EN 50 #define CONTROL_INTCAPXT_EN 51 +#define CONTROL_IRTCACHEDIS 59 #define CONTROL_SNPAVIC_EN 61 #define CTRL_INV_TO_MASK (7 << CONTROL_INV_TIMEOUT) @@ -716,6 +717,9 @@ struct amd_iommu { /* if one, we need to send a completion wait command */ bool need_sync; + /* true if disable irte caching */ + bool irtcachedis_enabled; + /* Handle for IOMMU core code */ struct iommu_device iommu; diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c index 329a406cc37d..418da641ee3d 100644 --- a/drivers/iommu/amd/init.c +++ b/drivers/iommu/amd/init.c @@ -162,6 +162,7 @@ static int amd_iommu_xt_mode = IRQ_REMAP_XAPIC_MODE; static bool amd_iommu_detected; static bool amd_iommu_disabled __initdata; static bool amd_iommu_force_enable __initdata; +static bool amd_iommu_irtcachedis; static int amd_iommu_target_ivhd_type; /* Global EFR and EFR2 registers */ @@ -484,6 +485,9 @@ static void iommu_disable(struct amd_iommu *iommu) /* Disable IOMMU hardware itself */ iommu_feature_disable(iommu, CONTROL_IOMMU_EN); + + /* Clear IRTE cache disabling bit */ + iommu_feature_disable(iommu, CONTROL_IRTCACHEDIS); } /* @@ -2686,6 +2690,33 @@ static void iommu_enable_ga(struct amd_iommu *iommu) #endif } +static void iommu_disable_irtcachedis(struct amd_iommu *iommu) +{ + iommu_feature_disable(iommu, CONTROL_IRTCACHEDIS); +} + +static void iommu_enable_irtcachedis(struct amd_iommu *iommu) +{ + u64 ctrl; + + if (!amd_iommu_irtcachedis) + return; + + /* + * Note: + * The support for IRTCacheDis feature is dertermined by + * checking if the bit is writable. + */ + iommu_feature_enable(iommu, CONTROL_IRTCACHEDIS); + ctrl = readq(iommu->mmio_base + MMIO_CONTROL_OFFSET); + ctrl &= (1ULL << CONTROL_IRTCACHEDIS); + if (ctrl) + iommu->irtcachedis_enabled = true; + pr_info("iommu%d (%#06x) : IRT cache is %s\n", + iommu->index, iommu->devid, + iommu->irtcachedis_enabled ? "disabled" : "enabled"); +} + static void early_enable_iommu(struct amd_iommu *iommu) { iommu_disable(iommu); @@ -2696,6 +2727,7 @@ static void early_enable_iommu(struct amd_iommu *iommu) iommu_set_exclusion_range(iommu); iommu_enable_ga(iommu); iommu_enable_xt(iommu); + iommu_enable_irtcachedis(iommu); iommu_enable(iommu); iommu_flush_all_caches(iommu); } @@ -2746,10 +2778,12 @@ static void early_enable_iommus(void) for_each_iommu(iommu) { iommu_disable_command_buffer(iommu); iommu_disable_event_buffer(iommu); + iommu_disable_irtcachedis(iommu); iommu_enable_command_buffer(iommu); iommu_enable_event_buffer(iommu); iommu_enable_ga(iommu); iommu_enable_xt(iommu); + iommu_enable_irtcachedis(iommu); iommu_set_device_table(iommu); iommu_flush_all_caches(iommu); } @@ -3402,6 +3436,8 @@ static int __init parse_amd_iommu_options(char *str) amd_iommu_pgtable = AMD_IOMMU_V1; } else if (strncmp(str, "pgtbl_v2", 8) == 0) { amd_iommu_pgtable = AMD_IOMMU_V2; + } else if (strncmp(str, "irtcachedis", 11) == 0) { + amd_iommu_irtcachedis = true; } else { pr_notice("Unknown option - '%s'\n", str); } From 98aeb4ea5599c5f7fbb1645bdd2050d0be96dfa3 Mon Sep 17 00:00:00 2001 From: Suravee Suthikulpanit Date: Tue, 30 May 2023 10:11:36 -0400 Subject: [PATCH 41/50] iommu/amd: Do not Invalidate IRT when IRTE caching is disabled With the Interrupt Remapping Table cache disabled, there is no need to issue invalidate IRT and wait for its completion. Therefore, add logic to bypass the operation. Reviewed-by: Jerry Snitselaar Suggested-by: Joao Martins Signed-off-by: Suravee Suthikulpanit Link: https://lore.kernel.org/r/20230530141137.14376-5-suravee.suthikulpanit@amd.com Signed-off-by: Joerg Roedel --- drivers/iommu/amd/iommu.c | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c index e6daf0f39fd8..8fdd6ebf8711 100644 --- a/drivers/iommu/amd/iommu.c +++ b/drivers/iommu/amd/iommu.c @@ -1266,12 +1266,24 @@ static void amd_iommu_flush_irt_all(struct amd_iommu *iommu) u32 devid; u16 last_bdf = iommu->pci_seg->last_bdf; + if (iommu->irtcachedis_enabled) + return; + for (devid = 0; devid <= last_bdf; devid++) iommu_flush_irt(iommu, devid); iommu_completion_wait(iommu); } +static void iommu_flush_irt_and_complete(struct amd_iommu *iommu, u16 devid) +{ + if (iommu->irtcachedis_enabled) + return; + + iommu_flush_irt(iommu, devid); + iommu_completion_wait(iommu); +} + void iommu_flush_all_caches(struct amd_iommu *iommu) { if (iommu_feature(iommu, FEATURE_IA)) { @@ -3030,8 +3042,7 @@ static int modify_irte_ga(struct amd_iommu *iommu, u16 devid, int index, raw_spin_unlock_irqrestore(&table->lock, flags); - iommu_flush_irt(iommu, devid); - iommu_completion_wait(iommu); + iommu_flush_irt_and_complete(iommu, devid); return 0; } @@ -3050,8 +3061,7 @@ static int modify_irte(struct amd_iommu *iommu, table->table[index] = irte->val; raw_spin_unlock_irqrestore(&table->lock, flags); - iommu_flush_irt(iommu, devid); - iommu_completion_wait(iommu); + iommu_flush_irt_and_complete(iommu, devid); return 0; } @@ -3069,8 +3079,7 @@ static void free_irte(struct amd_iommu *iommu, u16 devid, int index) iommu->irte_ops->clear_allocated(table, index); raw_spin_unlock_irqrestore(&table->lock, flags); - iommu_flush_irt(iommu, devid); - iommu_completion_wait(iommu); + iommu_flush_irt_and_complete(iommu, devid); } static void irte_prepare(void *entry, From bccc37a8a2fb002a302a526656c56a793a708670 Mon Sep 17 00:00:00 2001 From: Suravee Suthikulpanit Date: Tue, 30 May 2023 10:11:37 -0400 Subject: [PATCH 42/50] iommu/amd: Improving Interrupt Remapping Table Invalidation Invalidating Interrupt Remapping Table (IRT) requires, the AMD IOMMU driver to issue INVALIDATE_INTERRUPT_TABLE and COMPLETION_WAIT commands. Currently, the driver issues the two commands separately, which requires calling raw_spin_lock_irqsave() twice. In addition, the COMPLETION_WAIT could potentially be interleaved with other commands causing delay of the COMPLETION_WAIT command. Therefore, combine issuing of the two commands in one spin-lock, and changing struct amd_iommu.cmd_sem_val to use atomic64 to minimize locking. Reviewed-by: Jerry Snitselaar Signed-off-by: Suravee Suthikulpanit Link: https://lore.kernel.org/r/20230530141137.14376-6-suravee.suthikulpanit@amd.com Signed-off-by: Joerg Roedel --- drivers/iommu/amd/amd_iommu_types.h | 2 +- drivers/iommu/amd/init.c | 2 +- drivers/iommu/amd/iommu.c | 27 ++++++++++++++++++++++----- 3 files changed, 24 insertions(+), 7 deletions(-) diff --git a/drivers/iommu/amd/amd_iommu_types.h b/drivers/iommu/amd/amd_iommu_types.h index 8eeea1f25e69..5a4e04404cfd 100644 --- a/drivers/iommu/amd/amd_iommu_types.h +++ b/drivers/iommu/amd/amd_iommu_types.h @@ -752,7 +752,7 @@ struct amd_iommu { u32 flags; volatile u64 *cmd_sem; - u64 cmd_sem_val; + atomic64_t cmd_sem_val; #ifdef CONFIG_AMD_IOMMU_DEBUGFS /* DebugFS Info */ diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c index 418da641ee3d..4c68dc382286 100644 --- a/drivers/iommu/amd/init.c +++ b/drivers/iommu/amd/init.c @@ -1733,7 +1733,7 @@ static int __init init_iommu_one(struct amd_iommu *iommu, struct ivhd_header *h, iommu->pci_seg = pci_seg; raw_spin_lock_init(&iommu->lock); - iommu->cmd_sem_val = 0; + atomic64_set(&iommu->cmd_sem_val, 0); /* Add IOMMU to internal data structures */ list_add_tail(&iommu->list, &amd_iommu_list); diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c index 8fdd6ebf8711..e47c8c520708 100644 --- a/drivers/iommu/amd/iommu.c +++ b/drivers/iommu/amd/iommu.c @@ -1175,11 +1175,11 @@ static int iommu_completion_wait(struct amd_iommu *iommu) if (!iommu->need_sync) return 0; - raw_spin_lock_irqsave(&iommu->lock, flags); - - data = ++iommu->cmd_sem_val; + data = atomic64_add_return(1, &iommu->cmd_sem_val); build_completion_wait(&cmd, iommu, data); + raw_spin_lock_irqsave(&iommu->lock, flags); + ret = __iommu_queue_command_sync(iommu, &cmd, false); if (ret) goto out_unlock; @@ -1277,11 +1277,28 @@ static void amd_iommu_flush_irt_all(struct amd_iommu *iommu) static void iommu_flush_irt_and_complete(struct amd_iommu *iommu, u16 devid) { + int ret; + u64 data; + unsigned long flags; + struct iommu_cmd cmd, cmd2; + if (iommu->irtcachedis_enabled) return; - iommu_flush_irt(iommu, devid); - iommu_completion_wait(iommu); + build_inv_irt(&cmd, devid); + data = atomic64_add_return(1, &iommu->cmd_sem_val); + build_completion_wait(&cmd2, iommu, data); + + raw_spin_lock_irqsave(&iommu->lock, flags); + ret = __iommu_queue_command_sync(iommu, &cmd, true); + if (ret) + goto out; + ret = __iommu_queue_command_sync(iommu, &cmd2, false); + if (ret) + goto out; + wait_on_sem(iommu, data); +out: + raw_spin_unlock_irqrestore(&iommu->lock, flags); } void iommu_flush_all_caches(struct amd_iommu *iommu) From 1ce018df87640adb139c8418785ad3b6e4376bd3 Mon Sep 17 00:00:00 2001 From: Joerg Roedel Date: Fri, 9 Jun 2023 15:15:08 +0200 Subject: [PATCH 43/50] iommu/amd: Fix compile error for unused function MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Recent changes introduced a compile error: drivers/iommu/amd/iommu.c:1285:13: error: ‘iommu_flush_irt_and_complete’ defined but not used [-Werror=unused-function] 1285 | static void iommu_flush_irt_and_complete(struct amd_iommu *iommu, u16 devid) | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~ This happens with defconfig-x86_64 because AMD IOMMU is enabled but CONFIG_IRQ_REMAP is disabled. Move the function under #ifdef CONFIG_IRQ_REMAP to fix the error. Signed-off-by: Joerg Roedel --- drivers/iommu/amd/iommu.c | 52 +++++++++++++++++++-------------------- 1 file changed, 26 insertions(+), 26 deletions(-) diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c index e47c8c520708..13097619fc4c 100644 --- a/drivers/iommu/amd/iommu.c +++ b/drivers/iommu/amd/iommu.c @@ -1275,32 +1275,6 @@ static void amd_iommu_flush_irt_all(struct amd_iommu *iommu) iommu_completion_wait(iommu); } -static void iommu_flush_irt_and_complete(struct amd_iommu *iommu, u16 devid) -{ - int ret; - u64 data; - unsigned long flags; - struct iommu_cmd cmd, cmd2; - - if (iommu->irtcachedis_enabled) - return; - - build_inv_irt(&cmd, devid); - data = atomic64_add_return(1, &iommu->cmd_sem_val); - build_completion_wait(&cmd2, iommu, data); - - raw_spin_lock_irqsave(&iommu->lock, flags); - ret = __iommu_queue_command_sync(iommu, &cmd, true); - if (ret) - goto out; - ret = __iommu_queue_command_sync(iommu, &cmd2, false); - if (ret) - goto out; - wait_on_sem(iommu, data); -out: - raw_spin_unlock_irqrestore(&iommu->lock, flags); -} - void iommu_flush_all_caches(struct amd_iommu *iommu) { if (iommu_feature(iommu, FEATURE_IA)) { @@ -2831,6 +2805,32 @@ EXPORT_SYMBOL(amd_iommu_device_info); static struct irq_chip amd_ir_chip; static DEFINE_SPINLOCK(iommu_table_lock); +static void iommu_flush_irt_and_complete(struct amd_iommu *iommu, u16 devid) +{ + int ret; + u64 data; + unsigned long flags; + struct iommu_cmd cmd, cmd2; + + if (iommu->irtcachedis_enabled) + return; + + build_inv_irt(&cmd, devid); + data = atomic64_add_return(1, &iommu->cmd_sem_val); + build_completion_wait(&cmd2, iommu, data); + + raw_spin_lock_irqsave(&iommu->lock, flags); + ret = __iommu_queue_command_sync(iommu, &cmd, true); + if (ret) + goto out; + ret = __iommu_queue_command_sync(iommu, &cmd2, false); + if (ret) + goto out; + wait_on_sem(iommu, data); +out: + raw_spin_unlock_irqrestore(&iommu->lock, flags); +} + static void set_dte_irq_entry(struct amd_iommu *iommu, u16 devid, struct irq_remap_table *table) { From 85751a8af5c9cf8724a71735ba0962bbd5fccfad Mon Sep 17 00:00:00 2001 From: Vasant Hegde Date: Fri, 9 Jun 2023 09:03:27 +0000 Subject: [PATCH 44/50] iommu/amd: Fix DTE_IRQ_PHYS_ADDR_MASK macro Interrupt Table Root Pointer is 52 bit and table must be aligned to start on a 128-byte boundary. Hence first 6 bits are ignored. Current code uses address mask as 45 instead of 46bit. Use GENMASK_ULL macro instead of manually generating address mask. Signed-off-by: Vasant Hegde Reviewed-by: Jerry Snitselaar Link: https://lore.kernel.org/r/20230609090327.5923-1-vasant.hegde@amd.com Signed-off-by: Joerg Roedel --- drivers/iommu/amd/amd_iommu_types.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/iommu/amd/amd_iommu_types.h b/drivers/iommu/amd/amd_iommu_types.h index 5a4e04404cfd..7d957864c77e 100644 --- a/drivers/iommu/amd/amd_iommu_types.h +++ b/drivers/iommu/amd/amd_iommu_types.h @@ -284,7 +284,7 @@ #define AMD_IOMMU_PGSIZES_V2 (PAGE_SIZE | (1ULL << 21) | (1ULL << 30)) /* Bit value definition for dte irq remapping fields*/ -#define DTE_IRQ_PHYS_ADDR_MASK (((1ULL << 45)-1) << 6) +#define DTE_IRQ_PHYS_ADDR_MASK GENMASK_ULL(51, 6) #define DTE_IRQ_REMAP_INTCTL_MASK (0x3ULL << 60) #define DTE_IRQ_REMAP_INTCTL (2ULL << 60) #define DTE_IRQ_REMAP_ENABLE 1ULL From d18f4ee219824f2a0d84590fe537b575eeb5e413 Mon Sep 17 00:00:00 2001 From: Vasant Hegde Date: Fri, 9 Jun 2023 09:06:30 +0000 Subject: [PATCH 45/50] iommu/amd: Use BIT/BIT_ULL macro to define bit fields Make use of BIT macro when defining bitfields which makes it easy to read. No functional change intended. Suggested-by: Yazen Ghannam Signed-off-by: Vasant Hegde Link: https://lore.kernel.org/r/20230609090631.6052-1-vasant.hegde@amd.com Signed-off-by: Joerg Roedel --- drivers/iommu/amd/amd_iommu_types.h | 74 ++++++++++++++--------------- 1 file changed, 37 insertions(+), 37 deletions(-) diff --git a/drivers/iommu/amd/amd_iommu_types.h b/drivers/iommu/amd/amd_iommu_types.h index 7d957864c77e..4e65bba5f7dc 100644 --- a/drivers/iommu/amd/amd_iommu_types.h +++ b/drivers/iommu/amd/amd_iommu_types.h @@ -84,21 +84,21 @@ /* Extended Feature Bits */ -#define FEATURE_PREFETCH (1ULL<<0) -#define FEATURE_PPR (1ULL<<1) -#define FEATURE_X2APIC (1ULL<<2) -#define FEATURE_NX (1ULL<<3) -#define FEATURE_GT (1ULL<<4) -#define FEATURE_IA (1ULL<<6) -#define FEATURE_GA (1ULL<<7) -#define FEATURE_HE (1ULL<<8) -#define FEATURE_PC (1ULL<<9) +#define FEATURE_PREFETCH BIT_ULL(0) +#define FEATURE_PPR BIT_ULL(1) +#define FEATURE_X2APIC BIT_ULL(2) +#define FEATURE_NX BIT_ULL(3) +#define FEATURE_GT BIT_ULL(4) +#define FEATURE_IA BIT_ULL(6) +#define FEATURE_GA BIT_ULL(7) +#define FEATURE_HE BIT_ULL(8) +#define FEATURE_PC BIT_ULL(9) #define FEATURE_GATS_SHIFT (12) #define FEATURE_GATS_MASK (3ULL) -#define FEATURE_GAM_VAPIC (1ULL<<21) -#define FEATURE_GIOSUP (1ULL<<48) -#define FEATURE_EPHSUP (1ULL<<50) -#define FEATURE_SNP (1ULL<<63) +#define FEATURE_GAM_VAPIC BIT_ULL(21) +#define FEATURE_GIOSUP BIT_ULL(48) +#define FEATURE_EPHSUP BIT_ULL(50) +#define FEATURE_SNP BIT_ULL(63) #define FEATURE_PASID_SHIFT 32 #define FEATURE_PASID_MASK (0x1fULL << FEATURE_PASID_SHIFT) @@ -120,13 +120,13 @@ #define PASID_MASK 0x0000ffff /* MMIO status bits */ -#define MMIO_STATUS_EVT_OVERFLOW_INT_MASK (1 << 0) -#define MMIO_STATUS_EVT_INT_MASK (1 << 1) -#define MMIO_STATUS_COM_WAIT_INT_MASK (1 << 2) -#define MMIO_STATUS_PPR_INT_MASK (1 << 6) -#define MMIO_STATUS_GALOG_RUN_MASK (1 << 8) -#define MMIO_STATUS_GALOG_OVERFLOW_MASK (1 << 9) -#define MMIO_STATUS_GALOG_INT_MASK (1 << 10) +#define MMIO_STATUS_EVT_OVERFLOW_INT_MASK BIT(0) +#define MMIO_STATUS_EVT_INT_MASK BIT(1) +#define MMIO_STATUS_COM_WAIT_INT_MASK BIT(2) +#define MMIO_STATUS_PPR_INT_MASK BIT(6) +#define MMIO_STATUS_GALOG_RUN_MASK BIT(8) +#define MMIO_STATUS_GALOG_OVERFLOW_MASK BIT(9) +#define MMIO_STATUS_GALOG_INT_MASK BIT(10) /* event logging constants */ #define EVENT_ENTRY_SIZE 0x10 @@ -370,23 +370,23 @@ /* * Bit value definition for I/O PTE fields */ -#define IOMMU_PTE_PR (1ULL << 0) -#define IOMMU_PTE_U (1ULL << 59) -#define IOMMU_PTE_FC (1ULL << 60) -#define IOMMU_PTE_IR (1ULL << 61) -#define IOMMU_PTE_IW (1ULL << 62) +#define IOMMU_PTE_PR BIT_ULL(0) +#define IOMMU_PTE_U BIT_ULL(59) +#define IOMMU_PTE_FC BIT_ULL(60) +#define IOMMU_PTE_IR BIT_ULL(61) +#define IOMMU_PTE_IW BIT_ULL(62) /* * Bit value definition for DTE fields */ -#define DTE_FLAG_V (1ULL << 0) -#define DTE_FLAG_TV (1ULL << 1) -#define DTE_FLAG_IR (1ULL << 61) -#define DTE_FLAG_IW (1ULL << 62) +#define DTE_FLAG_V BIT_ULL(0) +#define DTE_FLAG_TV BIT_ULL(1) +#define DTE_FLAG_IR BIT_ULL(61) +#define DTE_FLAG_IW BIT_ULL(62) -#define DTE_FLAG_IOTLB (1ULL << 32) -#define DTE_FLAG_GIOV (1ULL << 54) -#define DTE_FLAG_GV (1ULL << 55) +#define DTE_FLAG_IOTLB BIT_ULL(32) +#define DTE_FLAG_GIOV BIT_ULL(54) +#define DTE_FLAG_GV BIT_ULL(55) #define DTE_FLAG_MASK (0x3ffULL << 32) #define DTE_GLX_SHIFT (56) #define DTE_GLX_MASK (3) @@ -440,13 +440,13 @@ #define MAX_DOMAIN_ID 65536 /* Protection domain flags */ -#define PD_DMA_OPS_MASK (1UL << 0) /* domain used for dma_ops */ -#define PD_DEFAULT_MASK (1UL << 1) /* domain is a default dma_ops +#define PD_DMA_OPS_MASK BIT(0) /* domain used for dma_ops */ +#define PD_DEFAULT_MASK BIT(1) /* domain is a default dma_ops domain for an IOMMU */ -#define PD_PASSTHROUGH_MASK (1UL << 2) /* domain has no page +#define PD_PASSTHROUGH_MASK BIT(2) /* domain has no page translation */ -#define PD_IOMMUV2_MASK (1UL << 3) /* domain has gcr3 table */ -#define PD_GIOV_MASK (1UL << 4) /* domain enable GIOV support */ +#define PD_IOMMUV2_MASK BIT(3) /* domain has gcr3 table */ +#define PD_GIOV_MASK BIT(4) /* domain enable GIOV support */ extern bool amd_iommu_dump; #define DUMP_printk(format, arg...) \ From 78db2985c2f6617cffb302618917400ad313153d Mon Sep 17 00:00:00 2001 From: Vasant Hegde Date: Fri, 9 Jun 2023 09:06:31 +0000 Subject: [PATCH 46/50] iommu/amd: Remove extern from function prototypes The kernel coding style does not require 'extern' in function prototypes. Hence remove them from header file. No functional change intended. Suggested-by: Yazen Ghannam Signed-off-by: Vasant Hegde Reviewed-by: Jerry Snitselaar Link: https://lore.kernel.org/r/20230609090631.6052-2-vasant.hegde@amd.com Signed-off-by: Joerg Roedel --- drivers/iommu/amd/amd_iommu.h | 90 ++++++++++++++--------------- drivers/iommu/amd/amd_iommu_types.h | 2 +- 2 files changed, 45 insertions(+), 47 deletions(-) diff --git a/drivers/iommu/amd/amd_iommu.h b/drivers/iommu/amd/amd_iommu.h index e98f20a9bdd8..fde217552b8d 100644 --- a/drivers/iommu/amd/amd_iommu.h +++ b/drivers/iommu/amd/amd_iommu.h @@ -11,14 +11,14 @@ #include "amd_iommu_types.h" -extern irqreturn_t amd_iommu_int_thread(int irq, void *data); -extern irqreturn_t amd_iommu_int_handler(int irq, void *data); -extern void amd_iommu_apply_erratum_63(struct amd_iommu *iommu, u16 devid); -extern void amd_iommu_restart_event_logging(struct amd_iommu *iommu); -extern int amd_iommu_init_devices(void); -extern void amd_iommu_uninit_devices(void); -extern void amd_iommu_init_notifier(void); -extern void amd_iommu_set_rlookup_table(struct amd_iommu *iommu, u16 devid); +irqreturn_t amd_iommu_int_thread(int irq, void *data); +irqreturn_t amd_iommu_int_handler(int irq, void *data); +void amd_iommu_apply_erratum_63(struct amd_iommu *iommu, u16 devid); +void amd_iommu_restart_event_logging(struct amd_iommu *iommu); +int amd_iommu_init_devices(void); +void amd_iommu_uninit_devices(void); +void amd_iommu_init_notifier(void); +void amd_iommu_set_rlookup_table(struct amd_iommu *iommu, u16 devid); #ifdef CONFIG_AMD_IOMMU_DEBUGFS void amd_iommu_debugfs_setup(struct amd_iommu *iommu); @@ -27,11 +27,11 @@ static inline void amd_iommu_debugfs_setup(struct amd_iommu *iommu) {} #endif /* Needed for interrupt remapping */ -extern int amd_iommu_prepare(void); -extern int amd_iommu_enable(void); -extern void amd_iommu_disable(void); -extern int amd_iommu_reenable(int); -extern int amd_iommu_enable_faulting(void); +int amd_iommu_prepare(void); +int amd_iommu_enable(void); +void amd_iommu_disable(void); +int amd_iommu_reenable(int mode); +int amd_iommu_enable_faulting(void); extern int amd_iommu_guest_ir; extern enum io_pgtable_fmt amd_iommu_pgtable; extern int amd_iommu_gpt_level; @@ -39,33 +39,32 @@ extern int amd_iommu_gpt_level; /* IOMMUv2 specific functions */ struct iommu_domain; -extern bool amd_iommu_v2_supported(void); -extern struct amd_iommu *get_amd_iommu(unsigned int idx); -extern u8 amd_iommu_pc_get_max_banks(unsigned int idx); -extern bool amd_iommu_pc_supported(void); -extern u8 amd_iommu_pc_get_max_counters(unsigned int idx); -extern int amd_iommu_pc_get_reg(struct amd_iommu *iommu, u8 bank, u8 cntr, - u8 fxn, u64 *value); -extern int amd_iommu_pc_set_reg(struct amd_iommu *iommu, u8 bank, u8 cntr, - u8 fxn, u64 *value); +bool amd_iommu_v2_supported(void); +struct amd_iommu *get_amd_iommu(unsigned int idx); +u8 amd_iommu_pc_get_max_banks(unsigned int idx); +bool amd_iommu_pc_supported(void); +u8 amd_iommu_pc_get_max_counters(unsigned int idx); +int amd_iommu_pc_get_reg(struct amd_iommu *iommu, u8 bank, u8 cntr, + u8 fxn, u64 *value); +int amd_iommu_pc_set_reg(struct amd_iommu *iommu, u8 bank, u8 cntr, + u8 fxn, u64 *value); -extern int amd_iommu_register_ppr_notifier(struct notifier_block *nb); -extern int amd_iommu_unregister_ppr_notifier(struct notifier_block *nb); -extern void amd_iommu_domain_direct_map(struct iommu_domain *dom); -extern int amd_iommu_domain_enable_v2(struct iommu_domain *dom, int pasids); -extern int amd_iommu_flush_page(struct iommu_domain *dom, u32 pasid, - u64 address); -extern void amd_iommu_update_and_flush_device_table(struct protection_domain *domain); -extern void amd_iommu_domain_update(struct protection_domain *domain); -extern void amd_iommu_domain_flush_complete(struct protection_domain *domain); -extern void amd_iommu_domain_flush_tlb_pde(struct protection_domain *domain); -extern int amd_iommu_flush_tlb(struct iommu_domain *dom, u32 pasid); -extern int amd_iommu_domain_set_gcr3(struct iommu_domain *dom, u32 pasid, - unsigned long cr3); -extern int amd_iommu_domain_clear_gcr3(struct iommu_domain *dom, u32 pasid); +int amd_iommu_register_ppr_notifier(struct notifier_block *nb); +int amd_iommu_unregister_ppr_notifier(struct notifier_block *nb); +void amd_iommu_domain_direct_map(struct iommu_domain *dom); +int amd_iommu_domain_enable_v2(struct iommu_domain *dom, int pasids); +int amd_iommu_flush_page(struct iommu_domain *dom, u32 pasid, u64 address); +void amd_iommu_update_and_flush_device_table(struct protection_domain *domain); +void amd_iommu_domain_update(struct protection_domain *domain); +void amd_iommu_domain_flush_complete(struct protection_domain *domain); +void amd_iommu_domain_flush_tlb_pde(struct protection_domain *domain); +int amd_iommu_flush_tlb(struct iommu_domain *dom, u32 pasid); +int amd_iommu_domain_set_gcr3(struct iommu_domain *dom, u32 pasid, + unsigned long cr3); +int amd_iommu_domain_clear_gcr3(struct iommu_domain *dom, u32 pasid); #ifdef CONFIG_IRQ_REMAP -extern int amd_iommu_create_irq_domain(struct amd_iommu *iommu); +int amd_iommu_create_irq_domain(struct amd_iommu *iommu); #else static inline int amd_iommu_create_irq_domain(struct amd_iommu *iommu) { @@ -77,8 +76,8 @@ static inline int amd_iommu_create_irq_domain(struct amd_iommu *iommu) #define PPR_INVALID 0x1 #define PPR_FAILURE 0xf -extern int amd_iommu_complete_ppr(struct pci_dev *pdev, u32 pasid, - int status, int tag); +int amd_iommu_complete_ppr(struct pci_dev *pdev, u32 pasid, + int status, int tag); static inline bool is_rd890_iommu(struct pci_dev *pdev) { @@ -131,10 +130,9 @@ static inline void *alloc_pgtable_page(int nid, gfp_t gfp) return page ? page_address(page) : NULL; } -extern bool translation_pre_enabled(struct amd_iommu *iommu); -extern bool amd_iommu_is_attach_deferred(struct device *dev); -extern int __init add_special_device(u8 type, u8 id, u32 *devid, - bool cmd_line); +bool translation_pre_enabled(struct amd_iommu *iommu); +bool amd_iommu_is_attach_deferred(struct device *dev); +int __init add_special_device(u8 type, u8 id, u32 *devid, bool cmd_line); #ifdef CONFIG_DMI void amd_iommu_apply_ivrs_quirks(void); @@ -142,9 +140,9 @@ void amd_iommu_apply_ivrs_quirks(void); static inline void amd_iommu_apply_ivrs_quirks(void) { } #endif -extern void amd_iommu_domain_set_pgtable(struct protection_domain *domain, - u64 *root, int mode); -extern struct dev_table_entry *get_dev_table(struct amd_iommu *iommu); +void amd_iommu_domain_set_pgtable(struct protection_domain *domain, + u64 *root, int mode); +struct dev_table_entry *get_dev_table(struct amd_iommu *iommu); extern u64 amd_iommu_efr; extern u64 amd_iommu_efr2; diff --git a/drivers/iommu/amd/amd_iommu_types.h b/drivers/iommu/amd/amd_iommu_types.h index 4e65bba5f7dc..53443d6f7b13 100644 --- a/drivers/iommu/amd/amd_iommu_types.h +++ b/drivers/iommu/amd/amd_iommu_types.h @@ -886,7 +886,7 @@ extern int amd_iommu_max_glx_val; * This function flushes all internal caches of * the IOMMU used by this driver. */ -extern void iommu_flush_all_caches(struct amd_iommu *iommu); +void iommu_flush_all_caches(struct amd_iommu *iommu); static inline int get_ioapic_devid(int id) { From 82d9654f92fd858d3b36c6f189dcfa850f2dfa29 Mon Sep 17 00:00:00 2001 From: Suhui Date: Wed, 14 Jun 2023 10:47:02 +0800 Subject: [PATCH 47/50] iommu/vt-d: Remove unnecessary (void*) conversions No need cast (void*) to (struct root_entry *). Signed-off-by: Suhui Link: https://lore.kernel.org/r/20230425033743.75986-1-suhui@nfschina.com Signed-off-by: Lu Baolu Signed-off-by: Joerg Roedel --- drivers/iommu/intel/iommu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c index b871a6afd803..323fa1a93765 100644 --- a/drivers/iommu/intel/iommu.c +++ b/drivers/iommu/intel/iommu.c @@ -1185,7 +1185,7 @@ static int iommu_alloc_root_entry(struct intel_iommu *iommu) { struct root_entry *root; - root = (struct root_entry *)alloc_pgtable_page(iommu->node, GFP_ATOMIC); + root = alloc_pgtable_page(iommu->node, GFP_ATOMIC); if (!root) { pr_err("Allocating root entry for %s failed\n", iommu->name); From a0e9911ac14baf46d8a8bea322c5bd7b3845825c Mon Sep 17 00:00:00 2001 From: Yanfei Xu Date: Wed, 14 Jun 2023 10:47:03 +0800 Subject: [PATCH 48/50] iommu/vt-d: Handle the failure case of dmar_reenable_qi() dmar_reenable_qi() may not succeed. Check and return when it fails. Signed-off-by: Yanfei Xu Link: https://lore.kernel.org/r/20230605112659.308981-2-yanfei.xu@intel.com Signed-off-by: Lu Baolu Signed-off-by: Joerg Roedel --- drivers/iommu/intel/iommu.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c index 323fa1a93765..e83fe243680b 100644 --- a/drivers/iommu/intel/iommu.c +++ b/drivers/iommu/intel/iommu.c @@ -2967,10 +2967,15 @@ static int init_iommu_hw(void) { struct dmar_drhd_unit *drhd; struct intel_iommu *iommu = NULL; + int ret; - for_each_active_iommu(iommu, drhd) - if (iommu->qi) - dmar_reenable_qi(iommu); + for_each_active_iommu(iommu, drhd) { + if (iommu->qi) { + ret = dmar_reenable_qi(iommu); + if (ret) + return ret; + } + } for_each_iommu(iommu, drhd) { if (drhd->ignored) { From 3f13f72787bcd15792c01a34c2aed8bdf38397bb Mon Sep 17 00:00:00 2001 From: Yanfei Xu Date: Wed, 14 Jun 2023 10:47:04 +0800 Subject: [PATCH 49/50] iommu/vt-d: Remove two WARN_ON in domain_context_mapping_one() Remove the WARN_ON(did == 0) as the domain id 0 is reserved and set once the domain_ids is allocated. So iommu_init_domains will never return 0. Remove the WARN_ON(!table) as this pointer will be accessed in the following code, if empty "table" really happens, the kernel will report a NULL pointer reference warning at the first place. Signed-off-by: Yanfei Xu Link: https://lore.kernel.org/r/20230605112659.308981-3-yanfei.xu@intel.com Signed-off-by: Lu Baolu Signed-off-by: Joerg Roedel --- drivers/iommu/intel/iommu.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c index e83fe243680b..4c0b7424c45e 100644 --- a/drivers/iommu/intel/iommu.c +++ b/drivers/iommu/intel/iommu.c @@ -1897,8 +1897,6 @@ static int domain_context_mapping_one(struct dmar_domain *domain, struct context_entry *context; int ret; - WARN_ON(did == 0); - if (hw_pass_through && domain_type_is_si(domain)) translation = CONTEXT_TT_PASS_THROUGH; @@ -1944,8 +1942,6 @@ static int domain_context_mapping_one(struct dmar_domain *domain, if (sm_supported(iommu)) { unsigned long pds; - WARN_ON(!table); - /* Setup the PASID DIR pointer: */ pds = context_get_sm_pds(table); context->lo = (u64)virt_to_phys(table->table) | From b4da4e112ade4e7c59cf915c702b14d69b528824 Mon Sep 17 00:00:00 2001 From: Lu Baolu Date: Wed, 14 Jun 2023 10:47:05 +0800 Subject: [PATCH 50/50] iommu/vt-d: Remove commented-out code These lines of code were commented out when they were first added in commit ba39592764ed ("Intel IOMMU: Intel IOMMU driver"). We do not want to restore them because the VT-d spec has deprecated the read/write draining hit. VT-d spec (section 11.4.2): " Hardware implementation with Major Version 2 or higher (VER_REG), always performs required drain without software explicitly requesting a drain in IOTLB invalidation. This field is deprecated and hardware will always report it as 1 to maintain backward compatibility with software. " Remove the code to make the code cleaner. Signed-off-by: Lu Baolu Reviewed-by: Jerry Snitselaar Link: https://lore.kernel.org/r/20230609060514.15154-1-baolu.lu@linux.intel.com Signed-off-by: Joerg Roedel --- drivers/iommu/intel/iommu.c | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c index 4c0b7424c45e..e5c111ff4dd9 100644 --- a/drivers/iommu/intel/iommu.c +++ b/drivers/iommu/intel/iommu.c @@ -1312,15 +1312,7 @@ static void __iommu_flush_iotlb(struct intel_iommu *iommu, u16 did, iommu->name, type); return; } - /* Note: set drain read/write */ -#if 0 - /* - * This is probably to be super secure.. Looks like we can - * ignore it without any impact. - */ - if (cap_read_drain(iommu->cap)) - val |= DMA_TLB_READ_DRAIN; -#endif + if (cap_write_drain(iommu->cap)) val |= DMA_TLB_WRITE_DRAIN;