Re: [PATCH v3 3/3] iommu/arm-smmu-v3: Issue CFGI/TLBI twice on Tegra264

[Ashish Mhetre <amhetre@nvidia.com>]

On 6/3/2026 5:03 PM, Will Deacon wrote:
> External email: Use caution opening links or attachments
>
>
> On Tue, Jun 02, 2026 at 10:01:14PM -0300, Jason Gunthorpe wrote:
>> On Tue, Jun 02, 2026 at 09:22:15PM +0100, Will Deacon wrote:
>>>> + /*
>>>> +  * On Tegra264 (see ARM_SMMU_OPT_TLBI_TWICE) re-issue the same
>>>> +  * cmdlist with another CMD_SYNC to satisfy the erratum.
>>>> +  * Callers must ensure the batch carries a uniform opcode class
>>>> +  * so that checking the first command is enough; the iommufd
>>>> +  * VSMMU path enforces this with arm_vsmmu_can_batch_cmd().
>>>> +  */
>>>> + if (!ret && sync && arm_smmu_cmd_needs_tlbi_twice(smmu, &cmds[0]))
>>> Can you move the arm_smmu_cmd_... part to the start of the conjunction,
>>> please? If you make it a static key as I mentioned previously, then
>>> hopefully that should mean everything else is moved out of line.
>>>
>>>> +         ret = __arm_smmu_cmdq_issue_cmdlist(smmu, cmdq, cmds, n, sync);
>>> Sashiko is also unhappy here if n == 0 because we probably shouldn't
>>> be inspecting the command array in that case.
>> That seems like an AI mistake though, we shouldn't actually ever pass
>> in 0 sized batches? 'zero streams' should fail probe, not get into
>> this code, that might be missing but I wouldn't add checks here..
> I was wondering about the case where the first command in a batch is
> unsupported. Can we issue a bare sync in that case?

Seems a valid case. But considering the static branch approach, will
it be fine to add "n != 0" check before arm_smmu_cmd_needs_tlbi_twice() 
? Something like: int ret = __arm_smmu_cmdq_issue_cmdlist(smmu, cmdq, 
cmds, n, sync);
if(!n) return ret; if (arm_smmu_erratum_cmd_needs_repeating(smmu, 
&cmds[0]) && !ret && sync) ret = __arm_smmu_cmdq_issue_cmdlist(smmu, 
cmdq, cmds, n, sync);
> Will

Thanks,
Ashish Mhetre