[PATCH v2 2/2] iommu/arm-smmu-v3: Issue CFGI/TLBI twice on Tegra264
Ashish Mhetre
amhetre at nvidia.com
Mon Jun 1 02:20:10 PDT 2026
On 5/30/2026 2:40 AM, Nicolin Chen wrote:
> On Fri, May 29, 2026 at 02:08:30PM +0000, Ashish Mhetre wrote:
>> +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);
>> +
>> + /*
>> + * The driver's batch invariants keep a single submission's
>> + * opcode class uniform, so checking the first command is enough.
>> + */
>> + if (!ret && sync && (smmu->options & ARM_SMMU_OPT_TLBI_TWICE) &&
>> + arm_smmu_cmd_needs_tlbi_twice(FIELD_GET(CMDQ_0_OP,
>> + cmds[0].data[0])))
>> + ret = __arm_smmu_cmdq_issue_cmdlist(smmu, cmdq, cmds, n, sync);
> https://sashiko.dev/#/patchset/20260529140830.629738-1-amhetre%40nvidia.com
> Sashiko pointed out that the iommufd path might mix commands when
> calling arm_smmu_cmdq_issue_cmdlist(), which is valid I think.
Okay, I'll update the batching for iommufd path as suggested by you in
next version.
>> static int arm_smmu_cmdq_issue_cmd_p(struct arm_smmu_device *smmu,
>> struct arm_smmu_cmd *cmd, bool sync)
>> {
>> @@ -863,8 +909,18 @@ static void arm_smmu_cmdq_batch_add_cmd_p(struct arm_smmu_device *smmu,
>> }
>>
>> if (cmds->num == CMDQ_BATCH_ENTRIES) {
>> + /*
>> + * Force a SYNC only when the batch carries commands that
>> + * have to be doubled (see ARM_SMMU_OPT_TLBI_TWICE).
>> + * The batch holds a uniform opcode class, so checking
>> + * the first command is sufficient.
>> + */
>> + bool need_sync = (smmu->options & ARM_SMMU_OPT_TLBI_TWICE) &&
>> + arm_smmu_cmd_needs_tlbi_twice(FIELD_GET(CMDQ_0_OP,
>> + cmds->cmds[0].data[0]));
>> +
> Also, given that this does "force a sync", I think it might be nicer
> to go to the force_sync path. One of my ongoing series also needs to
> add another force_sync condition, so I think it would be cleaner to
> have a helper function.
>
> Maybe try the following diff. That arm_smmu_cmdq_batch_force_sync()
> might be added with a preparatory patch, but it's up to you.
>
> --------------------------------------------------------------------
> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-iommufd.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-iommufd.c
> index 1e9f7d2de3441..4c9ce974d31a8 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-iommufd.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-iommufd.c
> @@ -350,6 +350,18 @@ static int arm_vsmmu_convert_user_cmd(struct arm_vsmmu *vsmmu,
> return 0;
> }
>
> +static bool arm_vsmmu_can_batch_cmd(struct arm_smmu_device *smmu,
> + struct arm_vsmmu_invalidation_cmd *last,
> + struct arm_vsmmu_invalidation_cmd *next)
> +{
> + struct arm_smmu_cmd next_cmd = {
> + .data[0] = le64_to_cpu(next->ucmd.cmd[0]),
> + };
> +
> + return arm_smmu_cmd_needs_tlbi_twice(smmu, &last->cmd) ==
> + arm_smmu_cmd_needs_tlbi_twice(smmu, &next_cmd);
> +}
> +
> int arm_vsmmu_cache_invalidate(struct iommufd_viommu *viommu,
> struct iommu_user_data_array *array)
> {
> @@ -382,7 +394,8 @@ int arm_vsmmu_cache_invalidate(struct iommufd_viommu *viommu,
>
> /* FIXME work in blocks of CMDQ_BATCH_ENTRIES and copy each block? */
> cur++;
> - if (cur != end && (cur - last) != CMDQ_BATCH_ENTRIES - 1)
> + if (cur != end && (cur - last) != CMDQ_BATCH_ENTRIES - 1 &&
> + arm_vsmmu_can_batch_cmd(smmu, last, cur))
> continue;
>
> /* FIXME always uses the main cmdq rather than trying to group by type */
> 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 a63155e9e7f28..9b150e3145034 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> @@ -820,33 +820,6 @@ static int __arm_smmu_cmdq_issue_cmdlist(struct arm_smmu_device *smmu,
> return ret;
> }
>
> -/*
> - * Returns true if @opcode is a CFGI_* or TLBI_* command, i.e. one of the
> - * invalidations covered by Tegra264 erratum (see ARM_SMMU_OPT_TLBI_TWICE).
> - */
> -static bool arm_smmu_cmd_needs_tlbi_twice(u8 opcode)
> -{
> - switch (opcode) {
> - case CMDQ_OP_CFGI_STE:
> - case CMDQ_OP_CFGI_ALL:
> - case CMDQ_OP_CFGI_CD:
> - case CMDQ_OP_CFGI_CD_ALL:
> - case CMDQ_OP_TLBI_NH_ALL:
> - case CMDQ_OP_TLBI_NH_ASID:
> - case CMDQ_OP_TLBI_NH_VA:
> - case CMDQ_OP_TLBI_NH_VAA:
> - case CMDQ_OP_TLBI_EL2_ALL:
> - case CMDQ_OP_TLBI_EL2_ASID:
> - case CMDQ_OP_TLBI_EL2_VA:
> - case CMDQ_OP_TLBI_S12_VMALL:
> - case CMDQ_OP_TLBI_S2_IPA:
> - case CMDQ_OP_TLBI_NSNH_ALL:
> - return true;
> - default:
> - return false;
> - }
> -}
> -
> int arm_smmu_cmdq_issue_cmdlist(struct arm_smmu_device *smmu,
> struct arm_smmu_cmdq *cmdq,
> struct arm_smmu_cmd *cmds, int n,
> @@ -858,9 +831,7 @@ int arm_smmu_cmdq_issue_cmdlist(struct arm_smmu_device *smmu,
> * The driver's batch invariants keep a single submission's
> * opcode class uniform, so checking the first command is enough.
> */
> - if (!ret && sync && (smmu->options & ARM_SMMU_OPT_TLBI_TWICE) &&
> - arm_smmu_cmd_needs_tlbi_twice(FIELD_GET(CMDQ_0_OP,
> - cmds[0].data[0])))
> + if (!ret && sync && arm_smmu_cmd_needs_tlbi_twice(smmu, &cmds[0]))
> ret = __arm_smmu_cmdq_issue_cmdlist(smmu, cmdq, cmds, n, sync);
>
> return ret;
> @@ -893,34 +864,48 @@ static void arm_smmu_cmdq_batch_init_cmd(struct arm_smmu_device *smmu,
> cmds->cmdq = arm_smmu_get_cmdq(smmu, cmd);
> }
>
> +static bool arm_smmu_cmdq_batch_force_sync(struct arm_smmu_device *smmu,
> + struct arm_smmu_cmdq_batch *cmds,
> + struct arm_smmu_cmd *cmd)
> +{
> + if (!cmds->num)
> + return false;
> +
> + /* The batch's pre-assigned cmdq doesn't support the new command */
> + if (!arm_smmu_cmdq_supports_cmd(cmds->cmdq, cmd))
> + return true;
> +
> + /* Arm erratum 2812531 */
> + if (cmds->num == CMDQ_BATCH_ENTRIES - 1 &&
> + (smmu->options & ARM_SMMU_OPT_CMDQ_FORCE_SYNC))
> + return true;
> +
> + /*
> + * Tegra264 erratum (see ARM_SMMU_OPT_TLBI_TWICE). The batch holds a
> + * uniform opcode class, so checking the first command is enough.
> + */
> + if ((cmds->num == CMDQ_BATCH_ENTRIES) &&
> + arm_smmu_cmd_needs_tlbi_twice(smmu, &cmds->cmds[0]))
> + return true;
> +
> + return false;
> +}
> +
> static void arm_smmu_cmdq_batch_add_cmd_p(struct arm_smmu_device *smmu,
> struct arm_smmu_cmdq_batch *cmds,
> struct arm_smmu_cmd *cmd)
> {
> - bool force_sync = (cmds->num == CMDQ_BATCH_ENTRIES - 1) &&
> - (smmu->options & ARM_SMMU_OPT_CMDQ_FORCE_SYNC);
> - bool unsupported_cmd;
> + bool force_sync = arm_smmu_cmdq_batch_force_sync(smmu, cmds, cmd);
>
> - unsupported_cmd = !arm_smmu_cmdq_supports_cmd(cmds->cmdq, cmd);
> - if (force_sync || unsupported_cmd) {
> + if (force_sync) {
> arm_smmu_cmdq_issue_cmdlist(smmu, cmds->cmdq, cmds->cmds,
> cmds->num, true);
> arm_smmu_cmdq_batch_init_cmd(smmu, cmds, cmd);
> }
>
> if (cmds->num == CMDQ_BATCH_ENTRIES) {
> - /*
> - * Force a SYNC only when the batch carries commands that
> - * have to be doubled (see ARM_SMMU_OPT_TLBI_TWICE).
> - * The batch holds a uniform opcode class, so checking
> - * the first command is sufficient.
> - */
> - bool need_sync = (smmu->options & ARM_SMMU_OPT_TLBI_TWICE) &&
> - arm_smmu_cmd_needs_tlbi_twice(FIELD_GET(CMDQ_0_OP,
> - cmds->cmds[0].data[0]));
> -
> arm_smmu_cmdq_issue_cmdlist(smmu, cmds->cmdq, cmds->cmds,
> - cmds->num, need_sync);
> + cmds->num, false);
> arm_smmu_cmdq_batch_init_cmd(smmu, cmds, cmd);
> }
>
> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
> index 08d1abaf31ae2..e6afc832c0078 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
> @@ -1219,6 +1219,37 @@ int arm_smmu_cmdq_issue_cmdlist(struct arm_smmu_device *smmu,
> struct arm_smmu_cmd *cmds, int n,
> bool sync);
>
> +/*
> + * Returns true if @cmd opcode is a CFGI_* or TLBI_* command, i.e. one of the
> + * invalidations covered by Tegra264 erratum (see ARM_SMMU_OPT_TLBI_TWICE).
> + */
> +static inline bool arm_smmu_cmd_needs_tlbi_twice(struct arm_smmu_device *smmu,
> + struct arm_smmu_cmd *cmd)
> +{
> + if (!(smmu->options & ARM_SMMU_OPT_TLBI_TWICE))
> + return false;
> +
> + switch (FIELD_GET(CMDQ_0_OP, cmd->data[0])) {
> + case CMDQ_OP_CFGI_STE:
> + case CMDQ_OP_CFGI_ALL:
> + case CMDQ_OP_CFGI_CD:
> + case CMDQ_OP_CFGI_CD_ALL:
> + case CMDQ_OP_TLBI_NH_ALL:
> + case CMDQ_OP_TLBI_NH_ASID:
> + case CMDQ_OP_TLBI_NH_VA:
> + case CMDQ_OP_TLBI_NH_VAA:
> + case CMDQ_OP_TLBI_EL2_ALL:
> + case CMDQ_OP_TLBI_EL2_ASID:
> + case CMDQ_OP_TLBI_EL2_VA:
> + case CMDQ_OP_TLBI_S12_VMALL:
> + case CMDQ_OP_TLBI_S2_IPA:
> + case CMDQ_OP_TLBI_NSNH_ALL:
> + return true;
> + default:
> + return false;
> + }
> +}
> +
> #ifdef CONFIG_ARM_SMMU_V3_SVA
> bool arm_smmu_sva_supported(struct arm_smmu_device *smmu);
> void arm_smmu_sva_notifier_synchronize(void);
> --------------------------------------------------------------------
>
> Nicolinc
Ack, I'll add force_sync patch by you as a preparatory patch and rebase my
changes on top of it and send it in V3.
Thanks,
Ashish Mhetre
More information about the linux-arm-kernel
mailing list