Re: [PATCH v11 06/11] iommu/arm-smmu-v3: Add ARM_SMMU_OPT_KDUMP_ADOPT for kdump kernel

[Nicolin Chen <nicolinc@nvidia.com>]

On Tue, Oct 20, 2026 at 11:38:21AM +0000, Mostafa Saleh wrote:
> On Mon, Oct 05, 2026 at 12:30:49PM -0700, Nicolin Chen wrote:
> > +	/*
> > +	 * According to spec (6.3.24), HW aligns the base down to the table size
> > +	 * and ignores the low bits, so an unaligned base would make this kernel
> > +	 * read a different table.
> > +	 */
> > +	if (!IS_ALIGNED(base, size)) {
> > +		dev_err(smmu->dev, "unaligned stream table base %pa\n", &base);
> 
> I understand these are best effort, but what happens if the address
> corrupted and happens to be aligned?

This is a basic sanity so HW wouldn't immediately fail. A corrupted
address would be trickier to test out; also we don't have a perfect
way to do that. Earlier versions had some address range check. I've
dropped it, following Jason's comments.

This is also in the kdocs of the whole smmu kdump functions:

 *  - Only a coherent SMMU (ARM_SMMU_FEAT_COHERENCY) is supported, as the stream
 *    table adoption is done by memremap with MEMREMAP_WB, which is verified on
 *    the real hardware. Callers of these functions are responsible for gating
 *    ARM_SMMU_FEAT_COHERENCY once during the probe.
 */

> > +int arm_smmu_kdump_adopt_deferred_l2_strtab(struct arm_smmu_device *smmu,
> > +					    u32 sid, phys_addr_t base, u32 span,
> > +					    struct arm_smmu_strtab_l2 **l2table)
> > +{
> > +	struct arm_smmu_strtab_l2 *table;
> > +	size_t size;
> > +
> > +	/*
> > +	 * Retest the span in case the L1 descriptor has been overwritten since
> > +	 * the adopt. Reject this master's insert; panic or SMMU-disable would
> > +	 * either lose the vmcore or cascade aborts. Do not try to fix it, as it
> > +	 * would break all other SIDs in the same bus (PCI case). The corruption
> > +	 * blast radius is already bounded to that bus range.
> > +	 */
> > +	if (span != STRTAB_SPLIT + 1) {
> > +		dev_err(smmu->dev,
> > +			"L1[%u] span %u changed since adopt (was %u)\n",
> > +			arm_smmu_strtab_l1_idx(sid), span, STRTAB_SPLIT + 1);
> > +		return -EINVAL;
> > +	}
> > +
> > +	size = (1UL << (span - 1)) * sizeof(struct arm_smmu_ste);
> > +
> > +	/* Same live-corruption check as the span; reject an overwritten base */
> > +	if (!base || !IS_ALIGNED(base, size)) {
> > +		dev_err(smmu->dev, "L1[%u] bad l2 table base %pa\n",
> > +			arm_smmu_strtab_l1_idx(sid), &base);
> > +		return -EINVAL;
> > +	}
> > +
> 
> Those are a bit similar to arm_smmu_kexec_parse_strtab_2lvl() is it
> possible to have some helpers instead?

Sure. I can do that.

> > +int arm_smmu_kdump_adopt_strtab(struct arm_smmu_device *smmu)
> > +{
> > +	u32 cfg_reg = readl_relaxed(smmu->base + ARM_SMMU_STRTAB_BASE_CFG);
> > +	u64 base_reg = readq_relaxed(smmu->base + ARM_SMMU_STRTAB_BASE);
> > +	bool was_2lvl = smmu->features & ARM_SMMU_FEAT_2_LVL_STRTAB;
> > +	phys_addr_t base = base_reg & STRTAB_BASE_ADDR_MASK;
> > +	u32 fmt = FIELD_GET(STRTAB_BASE_CFG_FMT, cfg_reg);
> > +	int ret;
> > +
> > +	dev_dbg(smmu->dev, "adopting crashed kernel's stream table\n");
> > +
> > +	if (fmt == STRTAB_BASE_CFG_FMT_2LVL) {
> > +		/*
> > +		 * Both kernels run on the same hardware, so it's impossible for
> > +		 * kdump kernel to see the support for linear stream table only.
> > +		 */
> 
> STRTAB_SPLIT which influences ARM_SMMU_FEAT_2_LVL_STRTAB is a SW choice, so it is

I missed that.

> possible to get such mismatch if STRTAB_SPLIT of the newer kernel is larger than the
> older kernel.
> However, I agree that in that case it's better to refuse the adoption.
>
> > +		if (WARN_ON(!(smmu->features & ARM_SMMU_FEAT_2_LVL_STRTAB)))
> > +			ret = -EINVAL;

I can replace the FEAT guard here with

	if (WARN_ON(FIELD_GET(IDR0_ST_LVL, idr0) != IDR0_ST_LVL_2LVL))
		ret = -EINVAL;

> > +		else
> > +			ret = arm_smmu_kdump_adopt_strtab_2lvl(smmu, cfg_reg,
> > +							       base);
> > +	} else if (fmt == STRTAB_BASE_CFG_FMT_LINEAR) {
> > +		/*
> > +		 * The kdump kernel need not match the crashed kernel. An older
> > +		 * crashed kernel that predates two-level stream table support
> > +		 * may have used a linear table on 2-level-capable hardware, so
> > +		 * enforce the same format here to match the adopted table.
> > +		 */
> > +		ret = arm_smmu_kdump_adopt_strtab_linear(smmu, cfg_reg, base);
> > +		if (!ret)
> > +			smmu->features &= ~ARM_SMMU_FEAT_2_LVL_STRTAB;
> 
> For simplicity, it is better to enforce the same table formats between
> kernels as the rest of the file, for example,
> arm_smmu_kexec_parse_strtab_2lvl() refuses different STRTAB_SPLIT.
> Some checks can be removed and the was_2lvl error path. It's
> very unlikely to hit such case anyway.

Sashiko pointed out a corner case that a modified kernel kdumps
into a vanilla kernel. It is not really a big effort to support
it. So, I would rather keep this.

> > +	} else {
> > +		dev_err(smmu->dev, "invalid STRTAB format %u\n", fmt);
> > +		ret = -EINVAL;
> > +	}
> > +
> > +	if (ret) {
> > +		arm_smmu_kdump_adopt_cleanup(smmu);
> 
> Probably easier to inline the error handling after each call and remove this?

I don't quite follow this. The clean function is used by three
sites. How can we remove it?

Thanks
Nicolin