From 98b64741d61124a12fb05a7595acb1fd6c1dc55d Mon Sep 17 00:00:00 2001 From: Christophe JAILLET Date: Mon, 7 Feb 2022 23:50:48 +0100 Subject: [PATCH 1/6] iommu/arm-smmu-v3: Avoid open coded arithmetic in memory allocation kmalloc_array()/kcalloc() should be used to avoid potential overflow when a multiplication is needed to compute the size of the requested memory. So turn a devm_kzalloc()+explicit size computation into an equivalent devm_kcalloc(). Signed-off-by: Christophe JAILLET Acked-by: Robin Murphy Link: https://lore.kernel.org/r/3f7b9b202c6b6f5edc234ab7af5f208fbf8bc944.1644274051.git.christophe.jaillet@wanadoo.fr Signed-off-by: Will Deacon --- drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 4 ++-- 1 file changed, 2 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 6dc6d8b6b368..14d06aad0726 100644 --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c @@ -2981,10 +2981,10 @@ static int arm_smmu_init_l1_strtab(struct arm_smmu_device *smmu) { unsigned int i; struct arm_smmu_strtab_cfg *cfg = &smmu->strtab_cfg; - size_t size = sizeof(*cfg->l1_desc) * cfg->num_l1_ents; void *strtab = smmu->strtab_cfg.strtab; - cfg->l1_desc = devm_kzalloc(smmu->dev, size, GFP_KERNEL); + cfg->l1_desc = devm_kcalloc(smmu->dev, cfg->num_l1_ents, + sizeof(*cfg->l1_desc), GFP_KERNEL); if (!cfg->l1_desc) return -ENOMEM; From fcdeb8c34043a192c881b0594b26d195d5e57997 Mon Sep 17 00:00:00 2001 From: Christophe JAILLET Date: Mon, 7 Feb 2022 23:47:55 +0100 Subject: [PATCH 2/6] iommu/arm-smmu-v3: Simplify memory allocation Use devm_bitmap_zalloc() instead of hand writing it. Signed-off-by: Christophe JAILLET Link: https://lore.kernel.org/r/598bd905103dcbe5653a54bb0dfb5a8597728214.1644274051.git.christophe.jaillet@wanadoo.fr Signed-off-by: Will Deacon --- drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 22 +++++---------------- 1 file changed, 5 insertions(+), 17 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 14d06aad0726..bbc4eeb42811 100644 --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c @@ -2911,32 +2911,20 @@ static int arm_smmu_init_one_queue(struct arm_smmu_device *smmu, return 0; } -static void arm_smmu_cmdq_free_bitmap(void *data) -{ - unsigned long *bitmap = data; - bitmap_free(bitmap); -} - static int arm_smmu_cmdq_init(struct arm_smmu_device *smmu) { - int ret = 0; struct arm_smmu_cmdq *cmdq = &smmu->cmdq; unsigned int nents = 1 << cmdq->q.llq.max_n_shift; - atomic_long_t *bitmap; atomic_set(&cmdq->owner_prod, 0); atomic_set(&cmdq->lock, 0); - bitmap = (atomic_long_t *)bitmap_zalloc(nents, GFP_KERNEL); - if (!bitmap) { - dev_err(smmu->dev, "failed to allocate cmdq bitmap\n"); - ret = -ENOMEM; - } else { - cmdq->valid_map = bitmap; - devm_add_action(smmu->dev, arm_smmu_cmdq_free_bitmap, bitmap); - } + cmdq->valid_map = (atomic_long_t *)devm_bitmap_zalloc(smmu->dev, nents, + GFP_KERNEL); + if (!cmdq->valid_map) + return -ENOMEM; - return ret; + return 0; } static int arm_smmu_init_queues(struct arm_smmu_device *smmu) From 93665e0275a2e2badfcfca5ada3b78b22df4a01a Mon Sep 17 00:00:00 2001 From: Miaoqian Lin Date: Wed, 5 Jan 2022 10:16:19 +0000 Subject: [PATCH 3/6] iommu/arm-smmu: Add missing pm_runtime_disable() in qcom_iommu_device_probe If the probe fails, we should use pm_runtime_disable() to balance pm_runtime_enable(). Add missing pm_runtime_disable() for error handling. Signed-off-by: Miaoqian Lin Link: https://lore.kernel.org/r/20220105101619.29108-1-linmq006@gmail.com Signed-off-by: Will Deacon --- drivers/iommu/arm/arm-smmu/qcom_iommu.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/drivers/iommu/arm/arm-smmu/qcom_iommu.c b/drivers/iommu/arm/arm-smmu/qcom_iommu.c index b91874cb6cf3..2f227bc88bd9 100644 --- a/drivers/iommu/arm/arm-smmu/qcom_iommu.c +++ b/drivers/iommu/arm/arm-smmu/qcom_iommu.c @@ -827,20 +827,20 @@ static int qcom_iommu_device_probe(struct platform_device *pdev) ret = devm_of_platform_populate(dev); if (ret) { dev_err(dev, "Failed to populate iommu contexts\n"); - return ret; + goto err_pm_disable; } ret = iommu_device_sysfs_add(&qcom_iommu->iommu, dev, NULL, dev_name(dev)); if (ret) { dev_err(dev, "Failed to register iommu in sysfs\n"); - return ret; + goto err_pm_disable; } ret = iommu_device_register(&qcom_iommu->iommu, &qcom_iommu_ops, dev); if (ret) { dev_err(dev, "Failed to register iommu\n"); - return ret; + goto err_pm_disable; } bus_set_iommu(&platform_bus_type, &qcom_iommu_ops); @@ -852,6 +852,10 @@ static int qcom_iommu_device_probe(struct platform_device *pdev) } return 0; + +err_pm_disable: + pm_runtime_disable(dev); + return ret; } static int qcom_iommu_device_remove(struct platform_device *pdev) From 30de2b541af98179780054836b48825fcfba4408 Mon Sep 17 00:00:00 2001 From: Zhou Guanghui Date: Wed, 19 Jan 2022 07:07:54 +0000 Subject: [PATCH 4/6] iommu/arm-smmu-v3: fix event handling soft lockup During event processing, events are read from the event queue one by one until the queue is empty.If the master device continuously requests address access at the same time and the SMMU generates events, the cyclic processing of the event takes a long time and softlockup warnings may be reported. arm-smmu-v3 arm-smmu-v3.34.auto: event 0x0a received: arm-smmu-v3 arm-smmu-v3.34.auto: 0x00007f220000280a arm-smmu-v3 arm-smmu-v3.34.auto: 0x000010000000007e arm-smmu-v3 arm-smmu-v3.34.auto: 0x00000000034e8670 watchdog: BUG: soft lockup - CPU#0 stuck for 22s! [irq/268-arm-smm:247] Call trace: _dev_info+0x7c/0xa0 arm_smmu_evtq_thread+0x1c0/0x230 irq_thread_fn+0x30/0x80 irq_thread+0x128/0x210 kthread+0x134/0x138 ret_from_fork+0x10/0x1c Kernel panic - not syncing: softlockup: hung tasks Fix this by calling cond_resched() after the event information is printed. Signed-off-by: Zhou Guanghui Link: https://lore.kernel.org/r/20220119070754.26528-1-zhouguanghui1@huawei.com Signed-off-by: Will Deacon --- drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 1 + 1 file changed, 1 insertion(+) 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 bbc4eeb42811..e92974173861 100644 --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c @@ -1558,6 +1558,7 @@ static irqreturn_t arm_smmu_evtq_thread(int irq, void *dev) dev_info(smmu->dev, "\t0x%016llx\n", (unsigned long long)evt[i]); + cond_resched(); } /* From 8ddf4eff71e12e4295b54ac8b02439ad22271fd2 Mon Sep 17 00:00:00 2001 From: Andy Shevchenko Date: Wed, 9 Feb 2022 20:47:58 +0200 Subject: [PATCH 5/6] perf/smmuv3: Don't cast parameter in bit operations While in this particular case it would not be a (critical) issue, the pattern itself is bad and error prone in case somebody blindly copies to their code. Don't cast parameter to unsigned long pointer in the bit operations. Instead copy to a local variable on stack of a proper type and use. Note, new compilers might warn on this line for potential outbound access. Signed-off-by: Andy Shevchenko Reviewed-by: Robin Murphy Link: https://lore.kernel.org/r/20220209184758.56578-1-andriy.shevchenko@linux.intel.com Signed-off-by: Will Deacon --- drivers/perf/arm_smmuv3_pmu.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/perf/arm_smmuv3_pmu.c b/drivers/perf/arm_smmuv3_pmu.c index c49108a72865..00d4c45a8017 100644 --- a/drivers/perf/arm_smmuv3_pmu.c +++ b/drivers/perf/arm_smmuv3_pmu.c @@ -654,6 +654,7 @@ static int smmu_pmu_offline_cpu(unsigned int cpu, struct hlist_node *node) static irqreturn_t smmu_pmu_handle_irq(int irq_num, void *data) { struct smmu_pmu *smmu_pmu = data; + DECLARE_BITMAP(ovs, BITS_PER_TYPE(u64)); u64 ovsr; unsigned int idx; @@ -663,7 +664,8 @@ static irqreturn_t smmu_pmu_handle_irq(int irq_num, void *data) writeq(ovsr, smmu_pmu->reloc_base + SMMU_PMCG_OVSCLR0); - for_each_set_bit(idx, (unsigned long *)&ovsr, smmu_pmu->num_counters) { + bitmap_from_u64(ovs, ovsr); + for_each_set_bit(idx, ovs, smmu_pmu->num_counters) { struct perf_event *event = smmu_pmu->events[idx]; struct hw_perf_event *hwc; From 97dfad194ca8de04c7292d4f4c8dc493c0d20f85 Mon Sep 17 00:00:00 2001 From: Robin Murphy Date: Thu, 17 Feb 2022 14:24:15 +0000 Subject: [PATCH 6/6] iommu/arm-smmu: Account for PMU interrupts In preparation for SMMUv2 PMU support, rejig our IRQ setup code to account for PMU interrupts as additional resources. We can simplify the whole flow by only storing the context IRQs, since the global IRQs are devres-managed and we never refer to them beyond the initial request. CC: Lad Prabhakar Signed-off-by: Robin Murphy Link: https://lore.kernel.org/r/b2a40caaf1622eb35c555074a0d72f4f0513cff9.1645106346.git.robin.murphy@arm.com Signed-off-by: Will Deacon --- drivers/iommu/arm/arm-smmu/arm-smmu.c | 93 +++++++++++++-------------- drivers/iommu/arm/arm-smmu/arm-smmu.h | 5 +- 2 files changed, 46 insertions(+), 52 deletions(-) diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c b/drivers/iommu/arm/arm-smmu/arm-smmu.c index 4bc75c4ce402..e50fcf37af58 100644 --- a/drivers/iommu/arm/arm-smmu/arm-smmu.c +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c @@ -807,7 +807,7 @@ static int arm_smmu_init_domain_context(struct iommu_domain *domain, * Request context fault interrupt. Do this last to avoid the * handler seeing a half-initialised domain state. */ - irq = smmu->irqs[smmu->num_global_irqs + cfg->irptndx]; + irq = smmu->irqs[cfg->irptndx]; if (smmu->impl && smmu->impl->context_fault) context_fault = smmu->impl->context_fault; @@ -858,7 +858,7 @@ static void arm_smmu_destroy_domain_context(struct iommu_domain *domain) arm_smmu_write_context_bank(smmu, cfg->cbndx); if (cfg->irptndx != ARM_SMMU_INVALID_IRPTNDX) { - irq = smmu->irqs[smmu->num_global_irqs + cfg->irptndx]; + irq = smmu->irqs[cfg->irptndx]; devm_free_irq(smmu->dev, irq, domain); } @@ -1951,8 +1951,8 @@ static int acpi_smmu_get_data(u32 model, struct arm_smmu_device *smmu) return ret; } -static int arm_smmu_device_acpi_probe(struct platform_device *pdev, - struct arm_smmu_device *smmu) +static int arm_smmu_device_acpi_probe(struct arm_smmu_device *smmu, + u32 *global_irqs, u32 *pmu_irqs) { struct device *dev = smmu->dev; struct acpi_iort_node *node = @@ -1968,7 +1968,8 @@ static int arm_smmu_device_acpi_probe(struct platform_device *pdev, return ret; /* Ignore the configuration access interrupt */ - smmu->num_global_irqs = 1; + *global_irqs = 1; + *pmu_irqs = 0; if (iort_smmu->flags & ACPI_IORT_SMMU_COHERENT_WALK) smmu->features |= ARM_SMMU_FEAT_COHERENT_WALK; @@ -1976,25 +1977,24 @@ static int arm_smmu_device_acpi_probe(struct platform_device *pdev, return 0; } #else -static inline int arm_smmu_device_acpi_probe(struct platform_device *pdev, - struct arm_smmu_device *smmu) +static inline int arm_smmu_device_acpi_probe(struct arm_smmu_device *smmu, + u32 *global_irqs, u32 *pmu_irqs) { return -ENODEV; } #endif -static int arm_smmu_device_dt_probe(struct platform_device *pdev, - struct arm_smmu_device *smmu) +static int arm_smmu_device_dt_probe(struct arm_smmu_device *smmu, + u32 *global_irqs, u32 *pmu_irqs) { const struct arm_smmu_match_data *data; - struct device *dev = &pdev->dev; + struct device *dev = smmu->dev; bool legacy_binding; - if (of_property_read_u32(dev->of_node, "#global-interrupts", - &smmu->num_global_irqs)) { - dev_err(dev, "missing #global-interrupts property\n"); - return -ENODEV; - } + if (of_property_read_u32(dev->of_node, "#global-interrupts", global_irqs)) + return dev_err_probe(dev, -ENODEV, + "missing #global-interrupts property\n"); + *pmu_irqs = 0; data = of_device_get_match_data(dev); smmu->version = data->version; @@ -2073,6 +2073,7 @@ static int arm_smmu_device_probe(struct platform_device *pdev) struct arm_smmu_device *smmu; struct device *dev = &pdev->dev; int num_irqs, i, err; + u32 global_irqs, pmu_irqs; irqreturn_t (*global_fault)(int irq, void *dev); smmu = devm_kzalloc(dev, sizeof(*smmu), GFP_KERNEL); @@ -2083,10 +2084,9 @@ static int arm_smmu_device_probe(struct platform_device *pdev) smmu->dev = dev; if (dev->of_node) - err = arm_smmu_device_dt_probe(pdev, smmu); + err = arm_smmu_device_dt_probe(smmu, &global_irqs, &pmu_irqs); else - err = arm_smmu_device_acpi_probe(pdev, smmu); - + err = arm_smmu_device_acpi_probe(smmu, &global_irqs, &pmu_irqs); if (err) return err; @@ -2105,31 +2105,25 @@ static int arm_smmu_device_probe(struct platform_device *pdev) if (IS_ERR(smmu)) return PTR_ERR(smmu); - num_irqs = 0; - while ((res = platform_get_resource(pdev, IORESOURCE_IRQ, num_irqs))) { - num_irqs++; - if (num_irqs > smmu->num_global_irqs) - smmu->num_context_irqs++; - } + num_irqs = platform_irq_count(pdev); - if (!smmu->num_context_irqs) { - dev_err(dev, "found %d interrupts but expected at least %d\n", - num_irqs, smmu->num_global_irqs + 1); - return -ENODEV; - } + smmu->num_context_irqs = num_irqs - global_irqs - pmu_irqs; + if (smmu->num_context_irqs <= 0) + return dev_err_probe(dev, -ENODEV, + "found %d interrupts but expected at least %d\n", + num_irqs, global_irqs + pmu_irqs + 1); - smmu->irqs = devm_kcalloc(dev, num_irqs, sizeof(*smmu->irqs), - GFP_KERNEL); - if (!smmu->irqs) { - dev_err(dev, "failed to allocate %d irqs\n", num_irqs); - return -ENOMEM; - } + smmu->irqs = devm_kcalloc(dev, smmu->num_context_irqs, + sizeof(*smmu->irqs), GFP_KERNEL); + if (!smmu->irqs) + return dev_err_probe(dev, -ENOMEM, "failed to allocate %d irqs\n", + smmu->num_context_irqs); - for (i = 0; i < num_irqs; ++i) { - int irq = platform_get_irq(pdev, i); + for (i = 0; i < smmu->num_context_irqs; i++) { + int irq = platform_get_irq(pdev, global_irqs + pmu_irqs + i); if (irq < 0) - return -ENODEV; + return irq; smmu->irqs[i] = irq; } @@ -2165,17 +2159,18 @@ static int arm_smmu_device_probe(struct platform_device *pdev) else global_fault = arm_smmu_global_fault; - for (i = 0; i < smmu->num_global_irqs; ++i) { - err = devm_request_irq(smmu->dev, smmu->irqs[i], - global_fault, - IRQF_SHARED, - "arm-smmu global fault", - smmu); - if (err) { - dev_err(dev, "failed to request global IRQ %d (%u)\n", - i, smmu->irqs[i]); - return err; - } + for (i = 0; i < global_irqs; i++) { + int irq = platform_get_irq(pdev, i); + + if (irq < 0) + return irq; + + err = devm_request_irq(dev, irq, global_fault, IRQF_SHARED, + "arm-smmu global fault", smmu); + if (err) + return dev_err_probe(dev, err, + "failed to request global IRQ %d (%u)\n", + i, irq); } err = iommu_device_sysfs_add(&smmu->iommu, smmu->dev, NULL, diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.h b/drivers/iommu/arm/arm-smmu/arm-smmu.h index 432de2f742c3..2b9b42fb6f30 100644 --- a/drivers/iommu/arm/arm-smmu/arm-smmu.h +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.h @@ -318,11 +318,10 @@ struct arm_smmu_device { unsigned long pa_size; unsigned long pgsize_bitmap; - u32 num_global_irqs; - u32 num_context_irqs; + int num_context_irqs; + int num_clks; unsigned int *irqs; struct clk_bulk_data *clks; - int num_clks; spinlock_t global_sync_lock;