From: Will Deacon <will@kernel•org>
To: Ashish Mhetre <amhetre@nvidia•com>
Cc: robin.murphy@arm•com, joro@8bytes•org, jgg@ziepe•ca,
nicolinc@nvidia•com, linux-arm-kernel@lists•infradead.org,
iommu@lists•linux.dev, linux-kernel@vger•kernel.org,
linux-tegra@vger•kernel.org
Subject: Re: [PATCH v3 3/3] iommu/arm-smmu-v3: Issue CFGI/TLBI twice on Tegra264
Date: Tue, 2 Jun 2026 21:22:15 +0100 [thread overview]
Message-ID: <ah87dzHOOTNGBBmt@willie-the-truck> (raw)
In-Reply-To: <20260601104845.995005-4-amhetre@nvidia.com>
On Mon, Jun 01, 2026 at 10:48:45AM +0000, Ashish Mhetre wrote:
> 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 1e9f7d2de344..78c96a2b652b 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,26 @@ static int arm_vsmmu_convert_user_cmd(struct arm_vsmmu *vsmmu,
> return 0;
> }
>
> +/*
> + * On Tegra264, arm_smmu_cmdq_issue_cmdlist() doubles every CFGI/TLBI
> + * submission (see ARM_SMMU_OPT_TLBI_TWICE). The doubling decision is
> + * taken once per cmdlist based on the first command, so a single
> + * batch must not mix commands that need doubling with commands that
> + * do not. Split the iommufd batch whenever the next user command
> + * crosses that boundary.
> + */
Again, I wouldn't bother with this comment. You probably _should_ update
Documentation/arch/arm64/silicon-errata.rst, however.
> +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 +402,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;
FYI: Sashiko is unhappy with the existing code here, so somebody should
check that out:
https://sashiko.dev/#/patchset/20260601104845.995005-2-amhetre@nvidia.com
> /* 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 08684bd40a6d..f38c21b56f28 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> @@ -698,10 +698,10 @@ static void arm_smmu_cmdq_write_entries(struct arm_smmu_cmdq *cmdq,
> * insert their own list of commands then all of the commands from one
> * CPU will appear before any of the commands from the other CPU.
> */
> -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)
> +static 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)
> {
> struct arm_smmu_cmd cmd_sync;
> u32 prod;
> @@ -820,6 +820,26 @@ int arm_smmu_cmdq_issue_cmdlist(struct arm_smmu_device *smmu,
> return ret;
> }
>
> +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);
> +
> + /*
> + * 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. Generally, it's a pity
that we can't handle this all a bit further up in the stack when we know
exactly what operationg we're trying to perform, but I suppose with all
the different users of the invalidation commands that's hard to catch in
one place?
> +
> + return ret;
> +}
> +
> static int arm_smmu_cmdq_issue_cmd_p(struct arm_smmu_device *smmu,
> struct arm_smmu_cmd *cmd, bool sync)
> {
> @@ -863,6 +883,14 @@ static bool arm_smmu_cmdq_batch_force_sync(struct arm_smmu_device *smmu,
> (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.
> + */
Again, please drop the Tegra264 mention and just refer to the option.
Will
next prev parent reply other threads:[~2026-06-02 20:22 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-06-01 10:48 [PATCH v3 0/3] iommu/arm-smmu-v3: Tegra264 invalidation workaround Ashish Mhetre
2026-06-01 10:48 ` [PATCH v3 1/3] iommu/arm-smmu-v3: Factor out CMDQ batch force-sync conditions Ashish Mhetre
2026-06-02 19:55 ` Will Deacon
2026-06-02 20:08 ` Nicolin Chen
2026-06-02 20:23 ` Will Deacon
2026-06-01 10:48 ` [PATCH v3 2/3] iommu/arm-smmu-v3: Detect Tegra264 erratum Ashish Mhetre
2026-06-02 20:13 ` Will Deacon
2026-06-02 20:31 ` Nicolin Chen
2026-06-02 20:59 ` Will Deacon
2026-06-02 21:06 ` Nicolin Chen
2026-06-01 10:48 ` [PATCH v3 3/3] iommu/arm-smmu-v3: Issue CFGI/TLBI twice on Tegra264 Ashish Mhetre
2026-06-01 18:37 ` Nicolin Chen
2026-06-02 20:22 ` Will Deacon [this message]
2026-06-02 20:35 ` Nicolin Chen
2026-06-03 0:59 ` Jason Gunthorpe
2026-06-03 1:01 ` Jason Gunthorpe
2026-06-03 11:33 ` Will Deacon
2026-06-02 16:31 ` [PATCH v3 0/3] iommu/arm-smmu-v3: Tegra264 invalidation workaround Mostafa Saleh
2026-06-02 18:23 ` Jason Gunthorpe
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ah87dzHOOTNGBBmt@willie-the-truck \
--to=will@kernel$(echo .)org \
--cc=amhetre@nvidia$(echo .)com \
--cc=iommu@lists$(echo .)linux.dev \
--cc=jgg@ziepe$(echo .)ca \
--cc=joro@8bytes$(echo .)org \
--cc=linux-arm-kernel@lists$(echo .)infradead.org \
--cc=linux-kernel@vger$(echo .)kernel.org \
--cc=linux-tegra@vger$(echo .)kernel.org \
--cc=nicolinc@nvidia$(echo .)com \
--cc=robin.murphy@arm$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox