All of 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:21 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.