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

[Ashish Mhetre <amhetre@nvidia.com>]

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@nvidia.com>
>> Signed-off-by: Ashish Mhetre <amhetre@nvidia.com>
>> 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