Skip to content

Commit 767e220

Browse files
nicolincjoergroedel
authored andcommitted
iommu/tegra241-cmdqv: Fix warnings due to dmam_free_coherent()
Two WARNINGs are observed when SMMU driver rolls back upon failure: arm-smmu-v3.9.auto: Failed to register iommu arm-smmu-v3.9.auto: probe with driver arm-smmu-v3 failed with error -22 ------------[ cut here ]------------ WARNING: CPU: 5 PID: 1 at kernel/dma/mapping.c:74 dmam_free_coherent+0xc0/0xd8 Call trace: dmam_free_coherent+0xc0/0xd8 (P) tegra241_vintf_free_lvcmdq+0x74/0x188 tegra241_cmdqv_remove_vintf+0x60/0x148 tegra241_cmdqv_remove+0x48/0xc8 arm_smmu_impl_remove+0x28/0x60 devm_action_release+0x1c/0x40 ------------[ cut here ]------------ 128 pages are still in use! WARNING: CPU: 16 PID: 1 at mm/page_alloc.c:6902 free_contig_range+0x18c/0x1c8 Call trace: free_contig_range+0x18c/0x1c8 (P) cma_release+0x154/0x2f0 dma_free_contiguous+0x38/0xa0 dma_direct_free+0x10c/0x248 dma_free_attrs+0x100/0x290 dmam_free_coherent+0x78/0xd8 tegra241_vintf_free_lvcmdq+0x74/0x160 tegra241_cmdqv_remove+0x98/0x198 arm_smmu_impl_remove+0x28/0x60 devm_action_release+0x1c/0x40 This is because the LVCMDQ queue memory are managed by devres, while that dmam_free_coherent() is called in the context of devm_action_release(). Jason pointed out that "arm_smmu_impl_probe() has mis-ordered the devres callbacks if ops->device_remove() is going to be manually freeing things that probe allocated": https://lore.kernel.org/linux-iommu/20250407174408.GB1722458@nvidia.com/ In fact, tegra241_cmdqv_init_structures() only allocates memory resources which means any failure that it generates would be similar to -ENOMEM, so there is no point in having that "falling back to standard SMMU" routine, as the standard SMMU would likely fail to allocate memory too. Remove the unwind part in tegra241_cmdqv_init_structures(), and return a proper error code to ask SMMU driver to call tegra241_cmdqv_remove() via impl_ops->device_remove(). Then, drop tegra241_vintf_free_lvcmdq() since devres will take care of that. Fixes: 483e0bd ("iommu/tegra241-cmdqv: Do not allocate vcmdq until dma_set_mask_and_coherent") Cc: stable@vger.kernel.org Suggested-by: Jason Gunthorpe <jgg@nvidia.com> Signed-off-by: Nicolin Chen <nicolinc@nvidia.com> Reviewed-by: Jason Gunthorpe <jgg@nvidia.com> Link: https://lore.kernel.org/r/20250407201908.172225-1-nicolinc@nvidia.com Signed-off-by: Joerg Roedel <jroedel@suse.de>
1 parent ae4814a commit 767e220

File tree

1 file changed

+5
-27
lines changed

1 file changed

+5
-27
lines changed

drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c

Lines changed: 5 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -487,17 +487,6 @@ static int tegra241_cmdqv_hw_reset(struct arm_smmu_device *smmu)
487487

488488
/* VCMDQ Resource Helpers */
489489

490-
static void tegra241_vcmdq_free_smmu_cmdq(struct tegra241_vcmdq *vcmdq)
491-
{
492-
struct arm_smmu_queue *q = &vcmdq->cmdq.q;
493-
size_t nents = 1 << q->llq.max_n_shift;
494-
size_t qsz = nents << CMDQ_ENT_SZ_SHIFT;
495-
496-
if (!q->base)
497-
return;
498-
dmam_free_coherent(vcmdq->cmdqv->smmu.dev, qsz, q->base, q->base_dma);
499-
}
500-
501490
static int tegra241_vcmdq_alloc_smmu_cmdq(struct tegra241_vcmdq *vcmdq)
502491
{
503492
struct arm_smmu_device *smmu = &vcmdq->cmdqv->smmu;
@@ -560,7 +549,8 @@ static void tegra241_vintf_free_lvcmdq(struct tegra241_vintf *vintf, u16 lidx)
560549
struct tegra241_vcmdq *vcmdq = vintf->lvcmdqs[lidx];
561550
char header[64];
562551

563-
tegra241_vcmdq_free_smmu_cmdq(vcmdq);
552+
/* Note that the lvcmdq queue memory space is managed by devres */
553+
564554
tegra241_vintf_deinit_lvcmdq(vintf, lidx);
565555

566556
dev_dbg(vintf->cmdqv->dev,
@@ -768,13 +758,13 @@ static int tegra241_cmdqv_init_structures(struct arm_smmu_device *smmu)
768758

769759
vintf = kzalloc(sizeof(*vintf), GFP_KERNEL);
770760
if (!vintf)
771-
goto out_fallback;
761+
return -ENOMEM;
772762

773763
/* Init VINTF0 for in-kernel use */
774764
ret = tegra241_cmdqv_init_vintf(cmdqv, 0, vintf);
775765
if (ret) {
776766
dev_err(cmdqv->dev, "failed to init vintf0: %d\n", ret);
777-
goto free_vintf;
767+
return ret;
778768
}
779769

780770
/* Preallocate logical VCMDQs to VINTF0 */
@@ -783,24 +773,12 @@ static int tegra241_cmdqv_init_structures(struct arm_smmu_device *smmu)
783773

784774
vcmdq = tegra241_vintf_alloc_lvcmdq(vintf, lidx);
785775
if (IS_ERR(vcmdq))
786-
goto free_lvcmdq;
776+
return PTR_ERR(vcmdq);
787777
}
788778

789779
/* Now, we are ready to run all the impl ops */
790780
smmu->impl_ops = &tegra241_cmdqv_impl_ops;
791781
return 0;
792-
793-
free_lvcmdq:
794-
for (lidx--; lidx >= 0; lidx--)
795-
tegra241_vintf_free_lvcmdq(vintf, lidx);
796-
tegra241_cmdqv_deinit_vintf(cmdqv, vintf->idx);
797-
free_vintf:
798-
kfree(vintf);
799-
out_fallback:
800-
dev_info(smmu->impl_dev, "Falling back to standard SMMU CMDQ\n");
801-
smmu->options &= ~ARM_SMMU_OPT_TEGRA241_CMDQV;
802-
tegra241_cmdqv_remove(smmu);
803-
return 0;
804782
}
805783

806784
#ifdef CONFIG_IOMMU_DEBUGFS

0 commit comments

Comments
 (0)