Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Mostafa Saleh <smostafa@google.com>
To: Jason Gunthorpe <jgg@nvidia.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>,
	Jonathan Corbet <corbet@lwn.net>,
	iommu@lists.linux.dev, "Joerg Roedel (AMD)" <joro@8bytes.org>,
	Jean-Philippe Brucker <jpb@kernel.org>,
	linux-arm-kernel@lists.infradead.org, linux-doc@vger.kernel.org,
	Mark Rutland <mark.rutland@arm.com>,
	Randy Dunlap <rdunlap@infradead.org>,
	Robin Murphy <robin.murphy@arm.com>,
	Shuah Khan <skhan@linuxfoundation.org>,
	Will Deacon <will@kernel.org>,
	David Matlack <dmatlack@google.com>,
	Jean-Philippe Brucker <jean-philippe@linaro.org>,
	Jonathan Cameron <Jonathan.Cameron@huawei.com>,
	Nicolin Chen <nicolinc@nvidia.com>,
	Pasha Tatashin <pasha.tatashin@soleen.com>,
	patches@lists.linux.dev, Pranjal Shrivastava <praan@google.com>,
	Samiullah Khawaja <skhawaja@google.com>,
	stable@vger.kernel.org,
	Vijayanand Jitta <vijayanand.jitta@oss.qualcomm.com>
Subject: Re: [PATCH v5 1/9] iommu/arm-smmu-v3: Handle ARM erratum for CONT under invalidation with SVA
Date: Mon, 7 Sep 2026 14:21:44 +0000	[thread overview]
Message-ID: <ap7IeAfVPZL3Hqt_@google.com> (raw)
In-Reply-To: <1-v5-b810cf379bfc+13d738-smmu_tlbi_jgg@nvidia.com>

On Tue, Sep 01, 2026 at 02:49:50PM -0300, Jason Gunthorpe wrote:
> The erratum (MMU-700: #3777127, S3: #3673557) deals with under
> invalidation of a CONT PTE grouping in the SMMU. The recommended work
> around is to use a Range Invalidate (RIL) that spans the entire CONT. The
> only user of CONT in the kernel right now is through SVA sharing a CPU
> page table that contains a CONT created by the mm.
> 
> Previously it was thought that this errata was dealt with because the
> driver always uses RIL. However, there is a subtle detail in the errata
> that the RIL range must fully enclose the entire CONT for it to work.
> 
> It seems that two sequential RILs, with a split point falling inside a
> CONT grouping, will not prevent the errata.
> 
> The SMMU's RIL generation algorithm does not produce a single RIL for a
> single SVA invalidation request, nor does the mm carefully align the SVA
> invalidation ranges to accommodate the RIL splitting.
> 
> Thus, when processing a SVA invalidation, the RIL splitting routine can
> generate a RIL that is split in the middle of the CONT and risk under
> invalidation from this errata. This condition could be triggered by a
> malicious userspace manipulating the TLB gathers via mmap/mprotect/munmap.
> 
> Update the errata list to the include the S3 variation, detect the IOMMUs
> that have it, and then have SVA invalidations use a simplified version of
> the over invalidation algorithm from the tlbi rework series. This ensures
> that a single RIL is issued for a single MMU notifier callback and now the
> RIL is guarenteed to cover any posible CONT.
> 
> Future work to add CONT to iommu_domain page tables should either use this
> one-invalidate/one-RIL algorithm or disable CONT support in the
> iommu_domain.
> 
> Cc: stable@vger.kernel.org
> Fixes: 3f1ce8e85ee0 ("iommu/arm-smmu-v3: Share process page tables")
> Cc: Vijayanand Jitta <vijayanand.jitta@oss.qualcomm.com>
> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
> ---
>  Documentation/arch/arm64/silicon-errata.rst   |  3 +-
>  .../iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c   |  7 ++
>  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c   | 87 +++++++++++++++----
>  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h   |  2 +
>  4 files changed, 81 insertions(+), 18 deletions(-)
> 
> diff --git a/Documentation/arch/arm64/silicon-errata.rst b/Documentation/arch/arm64/silicon-errata.rst
> index ac3248b9f2f3bb..68018bf75b7910 100644
> --- a/Documentation/arch/arm64/silicon-errata.rst
> +++ b/Documentation/arch/arm64/silicon-errata.rst
> @@ -271,7 +271,8 @@ stable kernels.
>  +----------------+-----------------+-----------------+-----------------------------+
>  | ARM            | MMU L1          | #3878312        | N/A                         |
>  +----------------+-----------------+-----------------+-----------------------------+
> -| ARM            | MMU S3          | #3995052        | N/A                         |
> +| ARM            | MMU S3          | #3995052,       | N/A                         |
> +|                |                 | #3673557        |                             |
>  +----------------+-----------------+-----------------+-----------------------------+
>  | ARM            | GIC-700         | #2941627        | ARM64_ERRATUM_2941627       |
>  +----------------+-----------------+-----------------+-----------------------------+
> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
> index 0a429c64fbf3e7..a0c9078646fb55 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
> @@ -215,6 +215,13 @@ bool arm_smmu_sva_supported(struct arm_smmu_device *smmu)
>  	if (system_supports_haft())
>  		feat_mask |= ARM_SMMU_FEAT_HAFT;
>  
> +	/*
> +	 * The workaround for ARM_SMMU_OPT_FULL_CONT_RIL requires range
> +	 * invalidation support.
> +	 */
> +	if (smmu->options & ARM_SMMU_OPT_FULL_CONT_RIL)
> +		feat_mask |= ARM_SMMU_FEAT_RANGE_INV;
> +
>  	if ((smmu->features & feat_mask) != feat_mask)
>  		return false;
>  
> 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 5732f3ba0122d6..d6896e25b6632a 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> @@ -2538,6 +2538,36 @@ static void arm_smmu_cmdq_batch_add_range(struct arm_smmu_device *smmu,
>  	}
>  }
>  
> +/*
> + * Generate a RIL for ARM_SMMU_OPT_FULL_CONT_RIL by ensuring the entire SVA
> + * requested range is covered with a single RIL command. The scale is adjusted
> + * so that the RIL may extend past the end of the requested range. This ensures
> + * that any CONT the MM is invalidating is covered by a single RIL. TTL and LEAF
> + * are always 0 because this is only used by SVA.
> + */
> +static bool arm_smmu_cmdq_batch_add_ril(struct arm_smmu_device *smmu,
> +					struct arm_smmu_cmdq_batch *cmds,
> +					struct arm_smmu_cmd *cmd,
> +					unsigned long iova, size_t size,
> +					u8 tgsz_lg2)
> +{
> +	u64 cur_tg = iova >> tgsz_lg2;

Nit: tg suffix is a bit confusing, I guess pfn is more accurate, but
no strong opinion.

Reviewed-by: Mostafa Saleh <smostafa@google.com>

Thanks,
Mostafa

> +	u64 num_tg = ((iova + size - 1) >> tgsz_lg2) - cur_tg + 1;
> +	unsigned int scale = fls64((num_tg - 1) / 32);
> +
> +	if (scale > 31)
> +		return false;
> +
> +	cmd->data[0] |=
> +		FIELD_PREP(CMDQ_TLBI_0_NUM,
> +			   DIV_ROUND_UP_ULL(num_tg, 1ULL << scale) - 1) |
> +		FIELD_PREP(CMDQ_TLBI_0_SCALE, scale);
> +	cmd->data[1] = FIELD_PREP(CMDQ_TLBI_1_TG, (tgsz_lg2 - 10) / 2) |
> +		       (cur_tg << tgsz_lg2);
> +	arm_smmu_cmdq_batch_add_cmd_p(smmu, cmds, cmd);
> +	return true;
> +}
> +
>  static bool arm_smmu_inv_size_too_big(struct arm_smmu_device *smmu, size_t size,
>  				      size_t granule)
>  {
> @@ -2565,21 +2595,30 @@ static bool arm_smmu_inv_size_too_big(struct arm_smmu_device *smmu, size_t size,
>  static void arm_smmu_inv_to_cmdq_batch(struct arm_smmu_inv *inv,
>  				       struct arm_smmu_cmdq_batch *cmds,
>  				       struct arm_smmu_cmd *cmd,
> -				       bool leaf,
> +				       bool single_ril, bool leaf,
>  				       unsigned long iova, size_t size,
>  				       unsigned int granule)
>  {
> -	if (arm_smmu_inv_size_too_big(inv->smmu, size, granule)) {
> -		struct arm_smmu_cmd nsize_cmd = *cmd;
> +	struct arm_smmu_cmd nsize_cmd;
>  
> -		u64p_replace_bits(&nsize_cmd.data[0], inv->nsize_opcode,
> -				  CMDQ_0_OP);
> -		arm_smmu_cmdq_batch_add_cmd_p(inv->smmu, cmds, &nsize_cmd);
> +	if (arm_smmu_inv_size_too_big(inv->smmu, size, granule))
> +		goto full_inv;
> +
> +	if (single_ril && size > granule) {
> +		if (!arm_smmu_cmdq_batch_add_ril(inv->smmu, cmds, cmd, iova,
> +						 size, inv->pgsize))
> +			goto full_inv;
>  		return;
>  	}
>  
> -	arm_smmu_cmdq_batch_add_range(inv->smmu, cmds, cmd, leaf,
> -				      iova, size, granule, inv->pgsize);
> +	arm_smmu_cmdq_batch_add_range(inv->smmu, cmds, cmd, leaf, iova, size,
> +				      granule, inv->pgsize);
> +	return;
> +
> +full_inv:
> +	nsize_cmd = *cmd;
> +	u64p_replace_bits(&nsize_cmd.data[0], inv->nsize_opcode, CMDQ_0_OP);
> +	arm_smmu_cmdq_batch_add_cmd_p(inv->smmu, cmds, &nsize_cmd);
>  }
>  
>  static inline bool arm_smmu_invs_end_batch(struct arm_smmu_inv *cur,
> @@ -2600,7 +2639,8 @@ static inline bool arm_smmu_invs_end_batch(struct arm_smmu_inv *cur,
>  
>  static void __arm_smmu_domain_inv_range(struct arm_smmu_invs *invs,
>  					unsigned long iova, size_t size,
> -					unsigned int granule, bool leaf)
> +					unsigned int granule, bool single_ril,
> +					bool leaf)
>  {
>  	struct arm_smmu_cmdq_batch cmds = {};
>  	struct arm_smmu_inv *cur;
> @@ -2630,14 +2670,14 @@ static void __arm_smmu_domain_inv_range(struct arm_smmu_invs *invs,
>  		case INV_TYPE_S1_ASID:
>  			cmd = arm_smmu_make_cmd_tlbi(cur->size_opcode,
>  						     cur->id, 0);
> -			arm_smmu_inv_to_cmdq_batch(cur, &cmds, &cmd, leaf,
> -						   iova, size, granule);
> +			arm_smmu_inv_to_cmdq_batch(cur, &cmds, &cmd, single_ril,
> +						   leaf, iova, size, granule);
>  			break;
>  		case INV_TYPE_S2_VMID:
>  			cmd = arm_smmu_make_cmd_tlbi(cur->size_opcode,
>  						     0, cur->id);
> -			arm_smmu_inv_to_cmdq_batch(cur, &cmds, &cmd, leaf,
> -						   iova, size, granule);
> +			arm_smmu_inv_to_cmdq_batch(cur, &cmds, &cmd, single_ril,
> +						   leaf, iova, size, granule);
>  			break;
>  		case INV_TYPE_S2_VMID_S1_CLEAR:
>  			/* CMDQ_OP_TLBI_S12_VMALL already flushed S1 entries */
> @@ -2684,6 +2724,9 @@ void arm_smmu_domain_inv_range(struct arm_smmu_domain *smmu_domain,
>  			       unsigned int granule, bool leaf)
>  {
>  	struct arm_smmu_invs *invs;
> +	bool single_ril =
> +		smmu_domain->stage == ARM_SMMU_DOMAIN_SVA &&
> +		(smmu_domain->smmu->options & ARM_SMMU_OPT_FULL_CONT_RIL);
>  
>  	/*
>  	 * An invalidation request must follow some IOPTE change and then load
> @@ -2723,10 +2766,12 @@ void arm_smmu_domain_inv_range(struct arm_smmu_domain *smmu_domain,
>  		unsigned long flags;
>  
>  		read_lock_irqsave(&invs->rwlock, flags);
> -		__arm_smmu_domain_inv_range(invs, iova, size, granule, leaf);
> +		__arm_smmu_domain_inv_range(invs, iova, size, granule,
> +					    single_ril, leaf);
>  		read_unlock_irqrestore(&invs->rwlock, flags);
>  	} else {
> -		__arm_smmu_domain_inv_range(invs, iova, size, granule, leaf);
> +		__arm_smmu_domain_inv_range(invs, iova, size, granule,
> +					    single_ril, leaf);
>  	}
>  
>  	rcu_read_unlock();
> @@ -5009,12 +5054,20 @@ static void arm_smmu_device_iidr_probe(struct arm_smmu_device *smmu)
>  				/* Arm errata 2268618, 2812531 */
>  				smmu->features &= ~ARM_SMMU_FEAT_NESTING;
>  			}
> +			/* Arm errata 3777127 */
> +			smmu->options |= ARM_SMMU_OPT_FULL_CONT_RIL;
>  			break;
>  		case IIDR_PRODUCTID_ARM_MMU_L1:
> -		case IIDR_PRODUCTID_ARM_MMU_S3:
> -			/* Arm errata 3878312/3995052 */
> +			/* Arm errata 3878312 */
>  			smmu->features &= ~ARM_SMMU_FEAT_BTM;
>  			break;
> +		case IIDR_PRODUCTID_ARM_MMU_S3:
> +			/* Arm errata 3995052 */
> +			smmu->features &= ~ARM_SMMU_FEAT_BTM;
> +			/* Arm errata 3673557 */
> +			if (variant < 1 || (variant == 1 && revision < 1))
> +				smmu->options |= ARM_SMMU_OPT_FULL_CONT_RIL;
> +			break;
>  		}
>  		break;
>  	}
> 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 50f8321e979cef..065d76eb148119 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
> @@ -934,6 +934,8 @@ struct arm_smmu_device {
>  #define ARM_SMMU_OPT_MSIPOLL		(1 << 2)
>  #define ARM_SMMU_OPT_CMDQ_FORCE_SYNC	(1 << 3)
>  #define ARM_SMMU_OPT_TEGRA241_CMDQV	(1 << 4)
> +/* RANGE_INV is mandatory and one RIL must fully span an invalidated CONT */
> +#define ARM_SMMU_OPT_FULL_CONT_RIL	(1 << 5)
>  	u32				options;
>  
>  	struct arm_smmu_cmdq		cmdq;
> -- 
> 2.43.0
> 


  reply	other threads:[~2026-09-07 14:22 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-01 17:49 [PATCH v5 0/9] Organize the SMMUv3 invalidation flow so iommupt can use it Jason Gunthorpe
2026-09-01 17:49 ` [PATCH v5 1/9] iommu/arm-smmu-v3: Handle ARM erratum for CONT under invalidation with SVA Jason Gunthorpe
2026-09-07 14:21   ` Mostafa Saleh [this message]
2026-09-07 15:11     ` Jason Gunthorpe
2026-09-07 15:17       ` Mostafa Saleh
2026-09-07 15:18         ` Jason Gunthorpe
2026-09-01 17:49 ` [PATCH v5 2/9] iommu/arm-smmu-v3: Pass the parameters for the invalidation in a struct Jason Gunthorpe
2026-09-01 17:49 ` [PATCH v5 3/9] iommu/arm-smmu-v3: Move pgsize out of arm_smmu_inv Jason Gunthorpe
2026-09-07 14:22   ` Mostafa Saleh
2026-09-01 17:49 ` [PATCH v5 4/9] iommu/arm-smmu-v3: Optimize range invalidation for latency Jason Gunthorpe
2026-09-07 14:26   ` Mostafa Saleh
2026-09-07 15:23     ` Jason Gunthorpe
2026-09-07 19:34       ` Jason Gunthorpe
2026-09-01 17:49 ` [PATCH v5 5/9] iommu/arm-smmu-v3: Keep track in the arm_smmu_invs if RIL is used Jason Gunthorpe
2026-09-01 17:49 ` [PATCH v5 6/9] iommu/arm-smmu-v3: Precompute the invalidation commands Jason Gunthorpe
2026-09-07 14:27   ` Mostafa Saleh
2026-09-07 15:19     ` Jason Gunthorpe
2026-09-01 17:49 ` [PATCH v5 7/9] iommu/arm-smmu-v3: Populate the tlbi at the top of the call chain Jason Gunthorpe
2026-09-01 17:49 ` [PATCH v5 8/9] iommu/arm-smmu-v3: Change how the tlbi describes the invalidation Jason Gunthorpe
2026-09-07 14:42   ` Mostafa Saleh
2026-09-07 15:34     ` Jason Gunthorpe
2026-09-09 15:57       ` Jason Gunthorpe
2026-09-01 17:49 ` [PATCH v5 9/9] iommu/arm-smmu-v3: Support the DS expansion of RIL's SCALE Jason Gunthorpe
2026-09-07 14:44   ` Mostafa Saleh
2026-09-07 19:39     ` 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=ap7IeAfVPZL3Hqt_@google.com \
    --to=smostafa@google.com \
    --cc=Jonathan.Cameron@huawei.com \
    --cc=catalin.marinas@arm.com \
    --cc=corbet@lwn.net \
    --cc=dmatlack@google.com \
    --cc=iommu@lists.linux.dev \
    --cc=jean-philippe@linaro.org \
    --cc=jgg@nvidia.com \
    --cc=joro@8bytes.org \
    --cc=jpb@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=nicolinc@nvidia.com \
    --cc=pasha.tatashin@soleen.com \
    --cc=patches@lists.linux.dev \
    --cc=praan@google.com \
    --cc=rdunlap@infradead.org \
    --cc=robin.murphy@arm.com \
    --cc=skhan@linuxfoundation.org \
    --cc=skhawaja@google.com \
    --cc=stable@vger.kernel.org \
    --cc=vijayanand.jitta@oss.qualcomm.com \
    --cc=will@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
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