iommu/tegra241-cmdqv: Fix two bugs in __tegra241_cmdqv_probe
Subsystems (4)
PATCH 1/2 iommu/tegra241-cmdqv: Drop static at local variable
In reply to Nicolin ChenThis is likely a typo. Drop it.
--- drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c +1 -1
@@ -761,8 +761,8 @@ static struct arm_smmu_device * __tegra241_cmdqv_probe(struct arm_smmu_device *smmu, struct resource *res, int irq) { - static struct arm_smmu_device *new_smmu; struct tegra241_cmdqv *cmdqv = NULL; +struct arm_smmu_device *new_smmu;struct tegra241_vintf *vintf;void __iomem *base;u32 regval;
Signature · 1 line
2.43.0
cover iommu/tegra241-cmdqv: Fix two bugs in __tegra241_cmdqv_probe
Found two more bugs today. And here are the fixes.
Based on Will's for-joerg/arm-smmu/updates branch.
Available on Github:
https://github.com/nicolinc/iommufd/commits/vcmdq_in_kernel-fix
Thanks
Nicolin
Nicolin Chen (2):
iommu/tegra241-cmdqv: Drop static at local variable iommu/tegra241-cmdqv: Do not allocate vcmdq until
dma_set_mask_and_coherent
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 9 +- drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 1 + .../iommu/arm/arm-smmu-v3/tegra241-cmdqv.c | 85 ++++++++++++------- 3 files changed, 61 insertions(+), 34 deletions(-)
Signature · 1 line
2.43.0
PATCH 2/2 iommu/tegra241-cmdqv: Do not allocate vcmdq until dma_set_mask_and_coherent
It's observed that, when the first 4GB of system memory was reserved, all VCMDQ allocations failed (even with the smallest qsz in the last attempt):
arm-smmu-v3: found companion CMDQV device: NVDA200C:00
arm-smmu-v3: option mask 0x10
arm-smmu-v3: failed to allocate queue (0x8000 bytes) for vcmdq0 acpi NVDA200C:00: tegra241_cmdqv: Falling back to standard SMMU CMDQ
arm-smmu-v3: ias 48-bit, oas 48-bit (features 0x001e1fbf)
arm-smmu-v3: allocated 524288 entries for cmdq
arm-smmu-v3: allocated 524288 entries for evtq
arm-smmu-v3: allocated 524288 entries for priq
This is because the 4GB reserved memory shifted the entire DMA zone from a lower 32-bit range (on a system without the 4GB carveout) to higher range, while the dev->coherent_dma_mask was set to DMA_BIT_MASK(32) by default.
The dma_set_mask_and_coherent() call is done in arm_smmu_device_hw_probe() of the SMMU driver. So any DMA allocation from tegra241_cmdqv_probe() must wait until the coherent_dma_mask is correctly set.
Move the vintf/vcmdq structure initialization routine into a different op, "init_structures". Call it at the end of arm_smmu_init_structures(), where standard SMMU queues get allocated.
Most of the impl_ops aren't ready until vintf/vcmdq structure are init-ed. So replace the full impl_ops with an init_ops in __tegra241_cmdqv_probe().
And switch to tegra241_cmdqv_impl_ops later in arm_smmu_init_structures(). Note that tegra241_cmdqv_impl_ops does not link to the new init_structures op after this switch, since there is no point in having it once it's done.
--- drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 9 +- drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 1 + .../iommu/arm/arm-smmu-v3/tegra241-cmdqv.c | 83 ++++++++++++------- 3 files changed, 60 insertions(+), 33 deletions(-)
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c +8 -1
@@ -3744,7 +3744,14 @@ static int arm_smmu_init_structures(struct arm_smmu_device *smmu)if (ret)return ret; -return arm_smmu_init_strtab (smmu); + ret =arm_smmu_init_strtab (smmu); +if (ret) +return ret; + +if (smmu->impl_ops && smmu->impl_ops->init_structures) +return smmu->impl_ops->init_structures (smmu); + +return 0 ; } static int arm_smmu_write_reg_sync(struct arm_smmu_device *smmu, u32 val,
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h +1 -0
@@ -643,6 +643,7 @@ struct arm_smmu_strtab_cfg {struct arm_smmu_impl_ops {int (*device_reset)(struct arm_smmu_device *smmu);void (*device_remove)(struct arm_smmu_device *smmu); +int (*init_structures)(struct arm_smmu_device *smmu);struct arm_smmu_cmdq *(*get_secondary_cmdq)(struct arm_smmu_device *smmu,struct arm_smmu_cmdq_ent *ent); };
drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c +51 -32
114 lines not on this page · raw message
Signature · 1 line
2.43.0
Re: [PATCH next 1/2] iommu/tegra241-cmdqv: Drop static at local variable
In reply to Nicolin ChenQuoted 7 lines · Nicolin Chen
This is likely a typo. Drop it.
Fixes 918eb5c856f6 ("iommu/arm-smmu-v3: Add in-kernel support for NVIDIA Tegra241 (Grace) CMDQV")Signed-off-by Nicolin Chen <[email protected]>--- drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Jason
Re: [PATCH next 2/2] iommu/tegra241-cmdqv: Do not allocate vcmdq until dma_set_mask_and_coherent
In reply to Nicolin ChenQuoted 30 lines · Nicolin Chen
It's observed that, when the first 4GB of system memory was reserved, all VCMDQ allocations failed (even with the smallest qsz in the last attempt):
arm-smmu-v3: found companion CMDQV device: NVDA200C:00
arm-smmu-v3: option mask0x10
arm-smmu-v3: failed to allocate queue (0x8000bytes) for vcmdq0 acpi NVDA200C:00:tegra241_cmdqv: Falling back to standard SMMU CMDQ
arm-smmu-v3: ias 48-bit, oas 48-bit (features0x001e1fbf)
arm-smmu-v3: allocated 524288 entries for cmdq
arm-smmu-v3: allocated 524288 entries for evtq
arm-smmu-v3: allocated 524288 entries for priqThis is because the 4GB reserved memory shifted the entire DMA zone from a lower 32-bit range (on a system without the 4GB carveout) to higher range, while the
dev->coherent_dma_maskwas set toDMA_BIT_MASK(32)by default.The
dma_set_mask_and_coherent()call is done inarm_smmu_device_hw_probe()of the SMMU driver. So any DMA allocation fromtegra241_cmdqv_probe()must wait until thecoherent_dma_maskis correctly set.Move the vintf/vcmdq structure initialization routine into a different op, "
init_structures". Call it at the end ofarm_smmu_init_structures(), where standard SMMU queues get allocated.Most of the
impl_opsaren't ready until vintf/vcmdq structure are init-ed. So replace the fullimpl_opswith aninit_opsin__tegra241_cmdqv_probe().And switch to
tegra241_cmdqv_impl_opslater inarm_smmu_init_structures(). Note thattegra241_cmdqv_impl_opsdoes not link to the newinit_structuresop after this switch, since there is no point in having it once it's done.
Quoted 8 lines
Fixes 918eb5c856f6 ("iommu/arm-smmu-v3: Add in-kernel support for NVIDIA Tegra241 (Grace) CMDQV")Reported-by Matt Ochs <[email protected]>Signed-off-by Nicolin Chen <[email protected]>--- drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 9 +- drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 1 + .../iommu/arm/arm-smmu-v3/tegra241-cmdqv.c | 83 ++++++++++++------- 3 files changed, 60 insertions(+), 33 deletions(-)
Jason
Quoted 8 lines · Nicolin Chen
Found two more bugs today. And here are the fixes.
Based on Will's for-joerg/arm-smmu/updates branch.
Available on Github:
https://github.com/nicolinc/iommufd/commits/vcmdq_in_kernel-fix[...]
Applied to will (for-joerg/arm-smmu/updates), thanks!
[1/2] iommu/tegra241-cmdqv: Drop static at local variable
https://git.kernel.org/will/c/2408b81f817b [2/2] iommu/tegra241-cmdqv: Do not allocate vcmdq until dma_set_mask_and_coherent https://git.kernel.org/will/c/483e0bd8883a
Cheers,
Signature · 5 lines
Will https://fixes.arm64.dev https://next.arm64.dev https://will.arm64.dev
Re: [PATCH next 2/2] iommu/tegra241-cmdqv: Do not allocate vcmdq until dma_set_mask_and_coherent
In reply to Nicolin ChenQuoted 28 lines · Nicolin Chen
It's observed that, when the first 4GB of system memory was reserved, all VCMDQ allocations failed (even with the smallest qsz in the last attempt):
arm-smmu-v3: found companion CMDQV device: NVDA200C:00
arm-smmu-v3: option mask0x10
arm-smmu-v3: failed to allocate queue (0x8000bytes) for vcmdq0 acpi NVDA200C:00:tegra241_cmdqv: Falling back to standard SMMU CMDQ
arm-smmu-v3: ias 48-bit, oas 48-bit (features0x001e1fbf)
arm-smmu-v3: allocated 524288 entries for cmdq
arm-smmu-v3: allocated 524288 entries for evtq
arm-smmu-v3: allocated 524288 entries for priqThis is because the 4GB reserved memory shifted the entire DMA zone from a lower 32-bit range (on a system without the 4GB carveout) to higher range, while the
dev->coherent_dma_maskwas set toDMA_BIT_MASK(32)by default.The
dma_set_mask_and_coherent()call is done inarm_smmu_device_hw_probe()of the SMMU driver. So any DMA allocation fromtegra241_cmdqv_probe()must wait until thecoherent_dma_maskis correctly set.Move the vintf/vcmdq structure initialization routine into a different op, "
init_structures". Call it at the end ofarm_smmu_init_structures(), where standard SMMU queues get allocated.Most of the
impl_opsaren't ready until vintf/vcmdq structure are init-ed. So replace the fullimpl_opswith aninit_opsin__tegra241_cmdqv_probe().
Quoted 8 lines
And switch to
tegra241_cmdqv_impl_opslater inarm_smmu_init_structures(). Note thattegra241_cmdqv_impl_opsdoes not link to the newinit_structuresop after this switch, since there is no point in having it once it's done.Fixes 918eb5c856f6 ("iommu/arm-smmu-v3: Add in-kernel support for NVIDIA Tegra241 (Grace) CMDQV")Reported-by Matt Ochs <[email protected]>Signed-off-by Nicolin Chen <[email protected]>—