Re: [PATCH v5 2/3] iommu/arm-smmu-v3: Introduce CFGI/TLBI-repeat workaround infrastructure

[Ashish Mhetre]

On 7/10/2026 9:50 AM, Nicolin Chen wrote:
> On Thu, Jul 09, 2026 at 09:56:08AM +0000, Ashish Mhetre wrote:
> > No callers enable the static key yet, so this patch introduces no
> > functional change. A subsequent change hooks the DT probe to enable
> > the key on affected instances.
> Nit: once a patch is merged, it's no longer a "patch" but "commit".
>
> Instead,
>
> No callers enable the static key yet, so there is no functional change. A
> subsequent change will enable the key on affected instances.

Ack.

> > Suggested-by: Nicolin Chen <nicolinc@xxxxxxxxxx>
> > Signed-off-by: Ashish Mhetre <amhetre@xxxxxxxxxx>
> > 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 76efe479e80f..15b9d0170520 100644
> > --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> > +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> > @@ -41,6 +41,7 @@ MODULE_PARM_DESC(disable_msipolling,
> >   
> >   static const struct iommu_ops arm_smmu_ops;
> >   static struct iommu_dirty_ops arm_smmu_dirty_ops;
> > +DEFINE_STATIC_KEY_FALSE(arm_smmu_erratum_repeat_tlbi_cfgi_key);
> It's an erratum; it should have an inline description, like the
> one you had at ARM_SMMU_OPT_TLBI_TWICE previously.

Okay, I'll restore the description I had for option bit here.

> > +int arm_smmu_cmdq_issue_cmdlist(struct arm_smmu_device *smmu,
> > +				struct arm_smmu_cmdq *cmdq,
> > +				struct arm_smmu_cmd *cmds, int n,
> > +				bool sync)
> > +{
> > +	int ret = __arm_smmu_cmdq_issue_cmdlist(smmu, cmdq, cmds, n, sync);
> > +
> > +	/*
> > +	 * arm_smmu_cmdq_batch_add_cmd_p() can flush its current batch
> > +	 * with sync=true and n=0 (bare SYNC) when the next command is
> > +	 * not supported by the batch's pre-selected cmdq, so the
> After a second thought, this "supported" case is not reachable.
>
> When a batch is init-ed, it's given an opcode (same as cmd[0]'s).
>
> If an opcode is not supported, it's init-ed with smmu->cmdq that
> must support all commands.
>
> So, adding the !n is more like a defensive play. But the comments
> here would be misleading.

You're right, the unsupported-cmdq force-sync path cannot run with n == 0.
I'll fix the misleading comment. The !n guard stays as a safety check 
for empty
batch_submit(), where we must not inspect cmds[0].

> > +	 * repeat path must not inspect cmds[0].
> > +	 */
> > +	if (!n || ret || !sync)
> > +		return ret;
> > +
> > +	if (arm_smmu_erratum_cmd_needs_repeating(smmu, &cmds[0]))
> > +		ret = __arm_smmu_cmdq_issue_cmdlist(smmu, cmdq, cmds, n, sync);
> > +
> > +	return ret;
> > +}
> > +
> >   static int arm_smmu_cmdq_issue_cmd_p(struct arm_smmu_device *smmu,
> >   				     struct arm_smmu_cmd *cmd, bool sync)
> >   {
> > @@ -860,6 +883,11 @@ static bool arm_smmu_cmdq_batch_force_sync(struct arm_smmu_device *smmu,
> >   	    (smmu->options & ARM_SMMU_OPT_CMDQ_FORCE_SYNC))
> >   		return true;
> >   
> > +	/* See arm_smmu_erratum_cmd_needs_repeating() */
> > +	if (cmds->num == CMDQ_BATCH_ENTRIES &&
> > +	    arm_smmu_erratum_cmd_needs_repeating(smmu, &cmds->cmds[0]))
> The comment above is useless; arm_smmu_erratum_cmd_needs_repeating
> doesn't give any useful explanation either.
>
> Like I suggested, add the missing piece of war description to the
> arm_smmu_erratum_repeat_tlbi_cfgi_key at the top. Then,
>
> 	/* See the description at arm_smmu_erratum_repeat_tlbi_cfgi_key */

Ack.

> > diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
> > @@ -1211,6 +1212,20 @@ int arm_smmu_cmdq_issue_cmdlist(struct arm_smmu_device *smmu,
> >   				struct arm_smmu_cmdq *cmdq,
> >   				struct arm_smmu_cmd *cmds, int n,
> >   				bool sync);
> > +DECLARE_STATIC_KEY_FALSE(arm_smmu_erratum_repeat_tlbi_cfgi_key);
> > +
> > +static inline bool
> > +arm_smmu_erratum_cmd_needs_repeating(struct arm_smmu_device *smmu,
> > +				     struct arm_smmu_cmd *cmd)
> smmu is unused.

Thanks for pointing. With static key, smmu pointer is not used anymore.
I'll remove it.

> > +{
> > +	u8 opcode;
> > +
> > +	if (!static_branch_unlikely(&arm_smmu_erratum_repeat_tlbi_cfgi_key))
> > +		return false;
> > +
> > +	opcode = FIELD_GET(CMDQ_0_OP, cmd->data[0]);
> > +	return opcode >= CMDQ_OP_CFGI_STE && opcode < CMDQ_OP_ATC_INV;
> > +}
> Maybe move this to the first caller in arm-smmu-v3.c? Instead, add
> its stub here, so we can drop that DECLARE_STATIC_KEY_FALSE. Then,
> DEFINE_STATIC_KEY_FALSE will need a "static".
>
> Nicolin

Makes sense as iommufd only needs the classifier, not the key itself. 
I'll move
arm_smmu_erratum_cmd_needs_repeating() into arm-smmu-v3.c, leave a
declaration in the header.

Thanks,
Ashish Mhetre