[PATCH v5 1/6] iommu/arm-smmu-v3: Add arm_smmu_kdump_adopt_strtab() for kdump
Nicolin Chen
nicolinc at nvidia.com
Tue May 19 11:11:09 PDT 2026
On Tue, May 19, 2026 at 02:10:03PM -0300, Jason Gunthorpe wrote:
> On Sun, May 10, 2026 at 02:23:00PM -0700, Nicolin Chen wrote:
>
> > +#include <linux/dma-direct.h>
>
> Nope, never do this, it is an internal header.
Hmm, I have included it for a wrong reason, yet it does mention
"IOMMU drivers".
/*
* Internals of the DMA direct mapping implementation. Only for use by the
* DMA mapping code and IOMMU drivers.
*/
> > +/*
> > + * Adopting the crashed kernel's stream table has risks: the physical addresses
> > + * read from ARM_SMMU_STRTAB_BASE / L1 descriptors may be corrupted. Reject any
> > + * range that overlaps the kdump kernel's critical regions.
> > + */
> > +static bool arm_smmu_kdump_phys_is_corrupted(phys_addr_t base, size_t size)
[..]
> Something like this should not be in the smmu driver, this is some
> core kdump code. I'd drop it, I don't see other drivers doing this?
OK.
> > +static int arm_smmu_kdump_adopt_l2_strtab(struct arm_smmu_device *smmu, u32 sid,
> > + u32 l1_idx, u64 l2_dma, u32 span,
> > + struct arm_smmu_strtab_l2 **l2table)
> > +{
> > + phys_addr_t base = dma_to_phys(smmu->dev, l2_dma);
>
> The thing stored in the L2PTR is a *phys*, the HW doesn't support any
> kind of translation. When using dma_alloc_coherent we never get a phys
> so it uses the dma_addr_t and assumes it is == phys.
>
> But on this flow this is *phys* and should remain phys. Never touch
> dma_addr_t.
Fixing that and other places too.
> > +static void arm_smmu_kdump_adopt_cleanup(struct arm_smmu_device *smmu, u32 fmt)
> > +{
> > + struct arm_smmu_strtab_cfg *cfg = &smmu->strtab_cfg;
> > +
> > + if (fmt == STRTAB_BASE_CFG_FMT_2LVL) {
> > + if (cfg->l2.l2ptrs)
> > + devm_kfree(smmu->dev, cfg->l2.l2ptrs);
> > + if (!IS_ERR_OR_NULL(cfg->l2.l1tab))
> > + devm_memunmap(smmu->dev, cfg->l2.l1tab);
> > + } else if (fmt == STRTAB_BASE_CFG_FMT_LINEAR) {
> > + if (!IS_ERR_OR_NULL(cfg->linear.table))
> > + devm_memunmap(smmu->dev, cfg->linear.table);
> > + }
> > +}
>
> If we have a cleanup function why is it using devm? Call the cleanup
> function during remove too?
Dropping "devm_"s.
Thanks
Nicolin
More information about the linux-arm-kernel
mailing list