From 698e136164109d6e55a85471eebf435dfec48fca Mon Sep 17 00:00:00 2001 From: Patrick Daly Date: Thu, 9 Jul 2020 13:50:07 -0700 Subject: [PATCH 1/2] iommu-debug: Remove trigger-fault functionality Deprecate this option in favor of CONFIG_ARM_SMMU_SELFTEST Change-Id: Ic5014c4bef9bb3d509bef20443281f1b57612096 Signed-off-by: Patrick Daly --- drivers/iommu/arm-smmu.c | 27 --------------------------- drivers/iommu/iommu-debug.c | 36 ------------------------------------ drivers/iommu/iommu.c | 17 ----------------- include/linux/iommu.h | 10 ---------- 4 files changed, 90 deletions(-) diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c index a190f62923ab..80d52f6c1b91 100644 --- a/drivers/iommu/arm-smmu.c +++ b/drivers/iommu/arm-smmu.c @@ -4037,32 +4037,6 @@ static bool arm_smmu_is_iova_coherent(struct iommu_domain *domain, return ret; } -static void arm_smmu_trigger_fault(struct iommu_domain *domain, - unsigned long flags) -{ - struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain); - struct arm_smmu_cfg *cfg = &smmu_domain->cfg; - struct arm_smmu_device *smmu; - int idx = cfg->cbndx; - - if (!smmu_domain->smmu) { - pr_err("Can't trigger faults on non-attached domains\n"); - return; - } - - smmu = smmu_domain->smmu; - if (arm_smmu_power_on(smmu->pwr)) - return; - - dev_err(smmu->dev, "Writing 0x%lx to FSRRESTORE on cb %d\n", - flags, cfg->cbndx); - arm_smmu_cb_write(smmu, idx, ARM_SMMU_CB_FSRRESTORE, flags); - /* give the interrupt time to fire... */ - msleep(1000); - - arm_smmu_power_off(smmu, smmu->pwr); -} - static void arm_smmu_tlbi_domain(struct iommu_domain *domain) { arm_smmu_tlb_inv_context_s1(to_smmu_domain(domain)); @@ -4086,7 +4060,6 @@ static struct msm_iommu_ops arm_smmu_ops = { .map_sg = arm_smmu_map_sg, .iova_to_phys_hard = arm_smmu_iova_to_phys_hard, .is_iova_coherent = arm_smmu_is_iova_coherent, - .trigger_fault = arm_smmu_trigger_fault, .tlbi_domain = arm_smmu_tlbi_domain, .enable_config_clocks = arm_smmu_enable_config_clocks, .disable_config_clocks = arm_smmu_disable_config_clocks, diff --git a/drivers/iommu/iommu-debug.c b/drivers/iommu/iommu-debug.c index 7559f4fe2c5b..f2ec2a37f2f9 100644 --- a/drivers/iommu/iommu-debug.c +++ b/drivers/iommu/iommu-debug.c @@ -2074,35 +2074,6 @@ static const struct file_operations iommu_debug_config_clocks_fops = { .write = iommu_debug_config_clocks_write, }; -static ssize_t iommu_debug_trigger_fault_write( - struct file *file, const char __user *ubuf, size_t count, - loff_t *offset) -{ - struct iommu_debug_device *ddev = file->private_data; - unsigned long flags; - - if (kstrtoul_from_user(ubuf, count, 0, &flags)) { - pr_err_ratelimited("Invalid flags format\n"); - return -EFAULT; - } - - mutex_lock(&ddev->state_lock); - if (!ddev->domain) { - pr_err_ratelimited("No domain. Did you already attach?\n"); - mutex_unlock(&ddev->state_lock); - return -EINVAL; - } - iommu_trigger_fault(ddev->domain, flags); - - mutex_unlock(&ddev->state_lock); - return count; -} - -static const struct file_operations iommu_debug_trigger_fault_fops = { - .open = simple_open, - .write = iommu_debug_trigger_fault_write, -}; - #ifdef CONFIG_ARM64_PTDUMP_CORE static int ptdump_show(struct seq_file *s, void *v) { @@ -2305,13 +2276,6 @@ static int iommu_debug_device_setup(struct device *dev) goto err_rmdir; } - if (!debugfs_create_file("trigger-fault", 0200, dir, ddev, - &iommu_debug_trigger_fault_fops)) { - pr_err_ratelimited("Couldn't create iommu/devices/%s/trigger-fault debugfs file\n", - dev_name(dev)); - goto err_rmdir; - } - #ifdef CONFIG_ARM64_PTDUMP_CORE if (!debugfs_create_file("iommu_page_tables", 0200, dir, ddev, &ptdump_fops)) { diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c index 647a604be522..2193417f39d9 100644 --- a/drivers/iommu/iommu.c +++ b/drivers/iommu/iommu.c @@ -2234,23 +2234,6 @@ void iommu_put_resv_regions(struct device *dev, struct list_head *list) ops->put_resv_regions(dev, list); } -/** - * iommu_trigger_fault() - trigger an IOMMU fault - * @domain: iommu domain - * - * Triggers a fault on the device to which this domain is attached. - * - * This function should only be used for debugging purposes, for obvious - * reasons. - */ -void iommu_trigger_fault(struct iommu_domain *domain, unsigned long flags) -{ - struct msm_iommu_ops *ops = to_msm_iommu_ops(domain->ops); - - if (ops->trigger_fault) - ops->trigger_fault(domain, flags); -} - struct iommu_resv_region *iommu_alloc_resv_region(phys_addr_t start, size_t length, int prot, enum iommu_resv_type type) diff --git a/include/linux/iommu.h b/include/linux/iommu.h index aafec83c32ac..7a35a2d07df6 100644 --- a/include/linux/iommu.h +++ b/include/linux/iommu.h @@ -390,7 +390,6 @@ struct iommu_ops { * to an iommu domain * @iova_to_phys_hard: translate iova to physical address using IOMMU hardware * @is_iova_coherent: checks coherency of the given iova - * @trigger_fault: trigger a fault on the device attached to an iommu domain * @tlbi_domain: Invalidate all TLBs covering an iommu domain * @enable_config_clocks: Enable all config clocks for this domain's IOMMU * @disable_config_clocks: Disable all config clocks for this domain's IOMMU @@ -404,7 +403,6 @@ struct msm_iommu_ops { dma_addr_t iova, unsigned long trans_flags); bool (*is_iova_coherent)(struct iommu_domain *domain, dma_addr_t iova); - void (*trigger_fault)(struct iommu_domain *domain, unsigned long flags); void (*tlbi_domain)(struct iommu_domain *domain); int (*enable_config_clocks)(struct iommu_domain *domain); void (*disable_config_clocks)(struct iommu_domain *domain); @@ -658,9 +656,6 @@ static inline void iommu_iotlb_gather_add_page(struct iommu_domain *domain, gather->start = start; } -extern void iommu_trigger_fault(struct iommu_domain *domain, - unsigned long flags); - /* PCI device grouping function */ extern struct iommu_group *pci_device_group(struct device *dev); /* Generic device grouping function */ @@ -1086,11 +1081,6 @@ static inline void iommu_device_unlink(struct device *dev, struct device *link) { } -static inline void iommu_trigger_fault(struct iommu_domain *domain, - unsigned long flags) -{ -} - static inline void iommu_tlbiall(struct iommu_domain *domain) { } From 62a31c82e2996c328146b0228adc456687b55bc7 Mon Sep 17 00:00:00 2001 From: Patrick Daly Date: Thu, 9 Jul 2020 13:57:02 -0700 Subject: [PATCH 2/2] iommu-debug: Remove calls to iommu_enable_config_clocks The iommu driver is responsible for enabling the required clocks for map/unmap operations, not clients. For the fastmap case, clocks are theoretically left enabled forever after the initial attach, although in practice this does not occur because there are no clocks required for Apps smmu. Change-Id: Ibdfced1f62c79fbb3234d80bc9c65808d818c27a Signed-off-by: Patrick Daly --- drivers/iommu/arm-smmu.c | 16 ------- drivers/iommu/iommu-debug.c | 84 +------------------------------------ include/linux/iommu.h | 30 ------------- 3 files changed, 1 insertion(+), 129 deletions(-) diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c index 80d52f6c1b91..d8da7e03cda1 100644 --- a/drivers/iommu/arm-smmu.c +++ b/drivers/iommu/arm-smmu.c @@ -4042,27 +4042,11 @@ static void arm_smmu_tlbi_domain(struct iommu_domain *domain) arm_smmu_tlb_inv_context_s1(to_smmu_domain(domain)); } -static int arm_smmu_enable_config_clocks(struct iommu_domain *domain) -{ - struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain); - - return arm_smmu_power_on(smmu_domain->smmu->pwr); -} - -static void arm_smmu_disable_config_clocks(struct iommu_domain *domain) -{ - struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain); - - arm_smmu_power_off(smmu_domain->smmu, smmu_domain->smmu->pwr); -} - static struct msm_iommu_ops arm_smmu_ops = { .map_sg = arm_smmu_map_sg, .iova_to_phys_hard = arm_smmu_iova_to_phys_hard, .is_iova_coherent = arm_smmu_is_iova_coherent, .tlbi_domain = arm_smmu_tlbi_domain, - .enable_config_clocks = arm_smmu_enable_config_clocks, - .disable_config_clocks = arm_smmu_disable_config_clocks, .iova_to_pte = arm_smmu_iova_to_pte, .iommu_ops = { diff --git a/drivers/iommu/iommu-debug.c b/drivers/iommu/iommu-debug.c index f2ec2a37f2f9..8e823159f142 100644 --- a/drivers/iommu/iommu-debug.c +++ b/drivers/iommu/iommu-debug.c @@ -126,8 +126,6 @@ struct iommu_debug_device { u64 phys; size_t len; struct list_head list; - struct mutex clk_lock; - unsigned int clk_count; /* Protects domain */ struct mutex state_lock; #ifdef CONFIG_ARM64_PTDUMP_CORE @@ -654,10 +652,6 @@ static int iommu_debug_profiling_fast_dma_api_show(struct seq_file *s, } domain = ddev->domain; - if (iommu_enable_config_clocks(domain)) { - seq_puts(s, "Couldn't enable clocks\n"); - goto out_detach; - } for (experiment = 0; experiment < 2; ++experiment) { size_t map_avg = 0, unmap_avg = 0; @@ -674,7 +668,7 @@ static int iommu_debug_profiling_fast_dma_api_show(struct seq_file *s, ns = ktime_to_ns(diff); if (dma_mapping_error(dev, dma_addr)) { seq_puts(s, "dma_map_single failed\n"); - goto out_disable_config_clocks; + goto out_detach; } map_elapsed_ns[i] = ns; @@ -709,8 +703,6 @@ static int iommu_debug_profiling_fast_dma_api_show(struct seq_file *s, seq_printf(s, "] (avg: %zu)\n", unmap_avg); } -out_disable_config_clocks: - iommu_disable_config_clocks(domain); out_detach: iommu_debug_dma_deconfigure(ddev); out_kfree: @@ -1278,12 +1270,7 @@ static int __apply_to_new_mapping(struct seq_file *s, } dev_err_ratelimited(dev, "testing with pgtables at %pa\n", &pt_phys); - if (iommu_enable_config_clocks(domain)) { - ds_printf(dev, s, "Couldn't enable clocks\n"); - goto out_release_mapping; - } ret = fn(dev, s, domain, priv); - iommu_disable_config_clocks(domain); out_release_mapping: iommu_debug_dma_deconfigure(ddev); @@ -2013,67 +2000,6 @@ static const struct file_operations iommu_debug_dma_unmap_fops = { .write = iommu_debug_dma_unmap_write, }; -static ssize_t iommu_debug_config_clocks_write(struct file *file, - const char __user *ubuf, - size_t count, loff_t *offset) -{ - char buf; - struct iommu_debug_device *ddev = file->private_data; - struct device *dev = ddev->dev; - - /* we're expecting a single character plus (optionally) a newline */ - if (count > 2) { - dev_err_ratelimited(dev, "Invalid value\n"); - return -EINVAL; - } - - if (!ddev->domain) { - dev_err_ratelimited(dev, "No domain. Did you already attach?\n"); - return -EINVAL; - } - - if (copy_from_user(&buf, ubuf, 1)) { - dev_err_ratelimited(dev, "Couldn't copy from user\n"); - return -EFAULT; - } - - mutex_lock(&ddev->clk_lock); - switch (buf) { - case '0': - if (ddev->clk_count == 0) { - dev_err_ratelimited(dev, "Config clocks already disabled\n"); - break; - } - - if (--ddev->clk_count > 0) - break; - - dev_err_ratelimited(dev, "Disabling config clocks\n"); - iommu_disable_config_clocks(ddev->domain); - break; - case '1': - if (ddev->clk_count++ > 0) - break; - - dev_err_ratelimited(dev, "Enabling config clocks\n"); - if (iommu_enable_config_clocks(ddev->domain)) - dev_err_ratelimited(dev, "Failed!\n"); - break; - default: - dev_err_ratelimited(dev, "Invalid value. Should be 0 or 1.\n"); - mutex_unlock(&ddev->clk_lock); - return -EINVAL; - } - mutex_unlock(&ddev->clk_lock); - - return count; -} - -static const struct file_operations iommu_debug_config_clocks_fops = { - .open = simple_open, - .write = iommu_debug_config_clocks_write, -}; - #ifdef CONFIG_ARM64_PTDUMP_CORE static int ptdump_show(struct seq_file *s, void *v) { @@ -2133,7 +2059,6 @@ static int iommu_debug_device_setup(struct device *dev) if (!ddev) return -ENOMEM; - mutex_init(&ddev->clk_lock); mutex_init(&ddev->state_lock); ddev->dev = dev; dir = debugfs_create_dir(dev_name(dev), debugfs_tests_dir); @@ -2269,13 +2194,6 @@ static int iommu_debug_device_setup(struct device *dev) goto err_rmdir; } - if (!debugfs_create_file("config_clocks", 0200, dir, ddev, - &iommu_debug_config_clocks_fops)) { - pr_err_ratelimited("Couldn't create iommu/devices/%s/config_clocks debugfs file\n", - dev_name(dev)); - goto err_rmdir; - } - #ifdef CONFIG_ARM64_PTDUMP_CORE if (!debugfs_create_file("iommu_page_tables", 0200, dir, ddev, &ptdump_fops)) { diff --git a/include/linux/iommu.h b/include/linux/iommu.h index 7a35a2d07df6..3fb3d62ddc97 100644 --- a/include/linux/iommu.h +++ b/include/linux/iommu.h @@ -391,8 +391,6 @@ struct iommu_ops { * @iova_to_phys_hard: translate iova to physical address using IOMMU hardware * @is_iova_coherent: checks coherency of the given iova * @tlbi_domain: Invalidate all TLBs covering an iommu domain - * @enable_config_clocks: Enable all config clocks for this domain's IOMMU - * @disable_config_clocks: Disable all config clocks for this domain's IOMMU * @iova_to_pte: translate iova to Page Table Entry (PTE). * @iommu_ops: the standard iommu ops */ @@ -404,8 +402,6 @@ struct msm_iommu_ops { unsigned long trans_flags); bool (*is_iova_coherent)(struct iommu_domain *domain, dma_addr_t iova); void (*tlbi_domain)(struct iommu_domain *domain); - int (*enable_config_clocks)(struct iommu_domain *domain); - void (*disable_config_clocks)(struct iommu_domain *domain); uint64_t (*iova_to_pte)(struct iommu_domain *domain, dma_addr_t iova); struct iommu_ops iommu_ops; }; @@ -671,23 +667,6 @@ static inline void iommu_tlbiall(struct iommu_domain *domain) ops->tlbi_domain(domain); } -static inline int iommu_enable_config_clocks(struct iommu_domain *domain) -{ - struct msm_iommu_ops *ops = to_msm_iommu_ops(domain->ops); - - if (ops->enable_config_clocks) - return ops->enable_config_clocks(domain); - return 0; -} - -static inline void iommu_disable_config_clocks(struct iommu_domain *domain) -{ - struct msm_iommu_ops *ops = to_msm_iommu_ops(domain->ops); - - if (ops->disable_config_clocks) - ops->disable_config_clocks(domain); -} - /** * struct iommu_fwspec - per-device IOMMU instance data * @ops: ops for this device's IOMMU @@ -1085,15 +1064,6 @@ static inline void iommu_tlbiall(struct iommu_domain *domain) { } -static inline int iommu_enable_config_clocks(struct iommu_domain *domain) -{ - return 0; -} - -static inline void iommu_disable_config_clocks(struct iommu_domain *domain) -{ -} - static inline int iommu_fwspec_init(struct device *dev, struct fwnode_handle *iommu_fwnode, const struct iommu_ops *ops)