[PATCH] iommu/arm-smmu-v3: Invalidate a new level-1 stream table descriptor

[PATCH] iommu/arm-smmu-v3: Invalidate a new level-1 stream table descriptor

[Xuheng Li via B4 Relay]

From: Xuheng Li <xuheng@cs.columbia.edu>

arm_smmu_init_l2_strtab() allocates a level-2 stream table when the first
master in its range is added, and makes the level-1 descriptor valid. It
issues no invalidation, and the attach that follows sends only the leaf
form of CMD_CFGI_STE.

IHI 0070 5.1.1 requires the non-leaf form of CMD_CFGI_STE when an L1STD
changes, including when an inactive descriptor with Span == 0 becomes
active. An SMMU that cached the inactive descriptor can keep terminating
the master's transactions as if its StreamID were invalid.

Issue a non-leaf CMD_CFGI_STE after writing the descriptor. The RMR path
runs before the stream table is programmed into the SMMU, and
arm_smmu_device_reset() then issues CMD_CFGI_ALL, so it needs none.

Fixes: 48ec83bcbcf5 ("iommu/arm-smmu: Add initial driver support for ARM SMMUv3 devices")
Signed-off-by: Xuheng Li <xuheng@cs.columbia.edu>
---
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

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 6d4520c47..f61cc6d86 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -2030,7 +2030,8 @@ static void arm_smmu_init_initial_stes(struct arm_smmu_ste *strtab,
 	}
 }
 
-static int arm_smmu_init_l2_strtab(struct arm_smmu_device *smmu, u32 sid)
+static int arm_smmu_init_l2_strtab(struct arm_smmu_device *smmu, u32 sid,
+				   bool live)
 {
 	dma_addr_t l2ptr_dma;
 	struct arm_smmu_strtab_cfg *cfg = &smmu->strtab_cfg;
@@ -2053,6 +2054,14 @@ static int arm_smmu_init_l2_strtab(struct arm_smmu_device *smmu, u32 sid)
 				   ARRAY_SIZE((*l2table)->stes));
 	arm_smmu_write_strtab_l1_desc(&cfg->l2.l1tab[arm_smmu_strtab_l1_idx(sid)],
 				      l2ptr_dma);
+
+	/*
+	 * The SMMU may have cached the old, invalid L1 descriptor. Making it
+	 * valid needs the non-leaf form of CMD_CFGI_STE (IHI 0070 5.1.1).
+	 */
+	if (live)
+		arm_smmu_cmdq_issue_cmd_with_sync(smmu,
+						  arm_smmu_make_cmd_cfgi_ste(sid, false));
 	return 0;
 }
 
@@ -4261,7 +4270,8 @@ static bool arm_smmu_sid_in_range(struct arm_smmu_device *smmu, u32 sid)
 	return sid < smmu->strtab_cfg.linear.num_ents;
 }
 
-static int arm_smmu_init_sid_strtab(struct arm_smmu_device *smmu, u32 sid)
+static int arm_smmu_init_sid_strtab(struct arm_smmu_device *smmu, u32 sid,
+				    bool live)
 {
 	/* Check the SIDs are in range of the SMMU and our stream table */
 	if (!arm_smmu_sid_in_range(smmu, sid))
@@ -4269,7 +4279,7 @@ static int arm_smmu_init_sid_strtab(struct arm_smmu_device *smmu, u32 sid)
 
 	/* Ensure l2 strtab is initialised */
 	if (smmu->features & ARM_SMMU_FEAT_2_LVL_STRTAB)
-		return arm_smmu_init_l2_strtab(smmu, sid);
+		return arm_smmu_init_l2_strtab(smmu, sid, live);
 
 	return 0;
 }
@@ -4333,7 +4343,7 @@ static int arm_smmu_insert_master(struct arm_smmu_device *smmu,
 		struct rb_node *existing;
 		u32 sid = new_stream->id;
 
-		ret = arm_smmu_init_sid_strtab(smmu, sid);
+		ret = arm_smmu_init_sid_strtab(smmu, sid, true);
 		if (ret)
 			break;
 
@@ -5713,7 +5723,8 @@ static void arm_smmu_rmr_install_bypass_ste(struct arm_smmu_device *smmu)
 
 		rmr = container_of(e, struct iommu_iort_rmr_data, rr);
 		for (i = 0; i < rmr->num_sids; i++) {
-			ret = arm_smmu_init_sid_strtab(smmu, rmr->sids[i]);
+			/* The stream table is not programmed into the SMMU yet */
+			ret = arm_smmu_init_sid_strtab(smmu, rmr->sids[i], false);
 			if (ret) {
 				dev_err(smmu->dev, "RMR SID(0x%x) bypass failed\n",
 					rmr->sids[i]);

---
base-commit: 05441d697f7f9a927264f4058cbbc6a03dfcac9d
change-id: 20261109-smmu-l1std-cfgi-c194b2aafea4

Best regards,
-- 
Xuheng Li <xuheng@cs.columbia.edu>

Re: [PATCH] iommu/arm-smmu-v3: Invalidate a new level-1 stream table descriptor

[Mostafa Saleh]

On Mon, Nov 09, 2026 at 09:36:54PM +0000, Xuheng Li via B4 Relay wrote:
> From: Xuheng Li <xuheng@cs.columbia.edu>
> 
> arm_smmu_init_l2_strtab() allocates a level-2 stream table when the first
> master in its range is added, and makes the level-1 descriptor valid. It
> issues no invalidation, and the attach that follows sends only the leaf
> form of CMD_CFGI_STE.
> 
> IHI 0070 5.1.1 requires the non-leaf form of CMD_CFGI_STE when an L1STD
> changes, including when an inactive descriptor with Span == 0 becomes
> active. An SMMU that cached the inactive descriptor can keep terminating
> the master's transactions as if its StreamID were invalid.
> 

Looking at the spec it says:
	An STE or L1STD that is successfully fetched might be cached by the
	SMMU in any state, therefore any modification, commissioning or
	decommissioning of an STE must be followed by a CMD_CFGI_STE command.

Which checks out. However, I am curious why this was not seen for
over 10 years with the current behaviour as I doubt any existing
implementation will do negative caching. Is that something that was
observed or found by inspection?

Interestingly, that is not the case for L1CD:
	/* An invalid L1CD can be cached */
	arm_smmu_sync_cd(master, ssid, false);

> Issue a non-leaf CMD_CFGI_STE after writing the descriptor. The RMR path
> runs before the stream table is programmed into the SMMU, and
> arm_smmu_device_reset() then issues CMD_CFGI_ALL, so it needs none.
> 
> Fixes: 48ec83bcbcf5 ("iommu/arm-smmu: Add initial driver support for ARM SMMUv3 devices")

With the few nits below:
Reviewed-by: Mostafa Saleh <smostafa@google.com>

Thanks,
Mostafa

> Signed-off-by: Xuheng Li <xuheng@cs.columbia.edu>
> ---
>  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 21 ++++++++++++++++-----
>  1 file changed, 16 insertions(+), 5 deletions(-)
> 
> 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 6d4520c47..f61cc6d86 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> @@ -2030,7 +2030,8 @@ static void arm_smmu_init_initial_stes(struct arm_smmu_ste *strtab,
>  	}
>  }
>  
> -static int arm_smmu_init_l2_strtab(struct arm_smmu_device *smmu, u32 sid)
> +static int arm_smmu_init_l2_strtab(struct arm_smmu_device *smmu, u32 sid,
> +				   bool live)
>  {
>  	dma_addr_t l2ptr_dma;
>  	struct arm_smmu_strtab_cfg *cfg = &smmu->strtab_cfg;
> @@ -2053,6 +2054,14 @@ static int arm_smmu_init_l2_strtab(struct arm_smmu_device *smmu, u32 sid)
>  				   ARRAY_SIZE((*l2table)->stes));
>  	arm_smmu_write_strtab_l1_desc(&cfg->l2.l1tab[arm_smmu_strtab_l1_idx(sid)],
>  				      l2ptr_dma);
> +
> +	/*
> +	 * The SMMU may have cached the old, invalid L1 descriptor. Making it
> +	 * valid needs the non-leaf form of CMD_CFGI_STE (IHI 0070 5.1.1).
> +	 */

A similar comment to the CD case is enough.

> +	if (live)
> +		arm_smmu_cmdq_issue_cmd_with_sync(smmu,
> +						  arm_smmu_make_cmd_cfgi_ste(sid, false));
>  	return 0;
>  }
>  
> @@ -4261,7 +4270,8 @@ static bool arm_smmu_sid_in_range(struct arm_smmu_device *smmu, u32 sid)
>  	return sid < smmu->strtab_cfg.linear.num_ents;
>  }
>  
> -static int arm_smmu_init_sid_strtab(struct arm_smmu_device *smmu, u32 sid)
> +static int arm_smmu_init_sid_strtab(struct arm_smmu_device *smmu, u32 sid,
> +				    bool live)
>  {
>  	/* Check the SIDs are in range of the SMMU and our stream table */
>  	if (!arm_smmu_sid_in_range(smmu, sid))
> @@ -4269,7 +4279,7 @@ static int arm_smmu_init_sid_strtab(struct arm_smmu_device *smmu, u32 sid)
>  
>  	/* Ensure l2 strtab is initialised */
>  	if (smmu->features & ARM_SMMU_FEAT_2_LVL_STRTAB)
> -		return arm_smmu_init_l2_strtab(smmu, sid);
> +		return arm_smmu_init_l2_strtab(smmu, sid, live);
>  
>  	return 0;
>  }
> @@ -4333,7 +4343,7 @@ static int arm_smmu_insert_master(struct arm_smmu_device *smmu,
>  		struct rb_node *existing;
>  		u32 sid = new_stream->id;
>  
> -		ret = arm_smmu_init_sid_strtab(smmu, sid);
> +		ret = arm_smmu_init_sid_strtab(smmu, sid, true);
>  		if (ret)
>  			break;
>  
> @@ -5713,7 +5723,8 @@ static void arm_smmu_rmr_install_bypass_ste(struct arm_smmu_device *smmu)
>  
>  		rmr = container_of(e, struct iommu_iort_rmr_data, rr);
>  		for (i = 0; i < rmr->num_sids; i++) {
> -			ret = arm_smmu_init_sid_strtab(smmu, rmr->sids[i]);
> +			/* The stream table is not programmed into the SMMU yet */

I don't think this comment is adding anything, there is another one a
couple of line below.

> +			ret = arm_smmu_init_sid_strtab(smmu, rmr->sids[i], false);
>  			if (ret) {
>  				dev_err(smmu->dev, "RMR SID(0x%x) bypass failed\n",
>  					rmr->sids[i]);
> 
> ---
> base-commit: 05441d697f7f9a927264f4058cbbc6a03dfcac9d
> change-id: 20261109-smmu-l1std-cfgi-c194b2aafea4
> 
> Best regards,
> -- 
> Xuheng Li <xuheng@cs.columbia.edu>
> 
> 

Re: [PATCH] iommu/arm-smmu-v3: Invalidate a new level-1 stream table descriptor

[Xuheng Li]

On Tue, Nov 10, 2026 at 02:58:21PM +0000, Mostafa Saleh wrote:
> Which checks out. However, I am curious why this was not seen for
> over 10 years with the current behaviour as I doubt any existing
> implementation will do negative caching. Is that something that was
> observed or found by inspection?

Thanks, Mostafa. Found by inspection against the spec, not observed on
hardware; we don't know whether any current implementation caches an
invalid L1STD. Either way, the driver should follow 5.1.1 as the L1CD
path already does. v2 will take both comment nits and your R-b.

Thanks,
Xuheng

Re: [PATCH] iommu/arm-smmu-v3: Invalidate a new level-1 stream table descriptor

[Jason Gunthorpe]

On Tue, Nov 10, 2026 at 02:58:21PM +0000, Mostafa Saleh wrote:
> On Mon, Nov 09, 2026 at 09:36:54PM +0000, Xuheng Li via B4 Relay wrote:
> > From: Xuheng Li <xuheng@cs.columbia.edu>
> > 
> > arm_smmu_init_l2_strtab() allocates a level-2 stream table when the first
> > master in its range is added, and makes the level-1 descriptor valid. It
> > issues no invalidation, and the attach that follows sends only the leaf
> > form of CMD_CFGI_STE.
> > 
> > IHI 0070 5.1.1 requires the non-leaf form of CMD_CFGI_STE when an L1STD
> > changes, including when an inactive descriptor with Span == 0 becomes
> > active. An SMMU that cached the inactive descriptor can keep terminating
> > the master's transactions as if its StreamID were invalid.
> > 
> 
> Looking at the spec it says:
> 	An STE or L1STD that is successfully fetched might be cached by the
> 	SMMU in any state, therefore any modification, commissioning or
> 	decommissioning of an STE must be followed by a CMD_CFGI_STE command.

I think the patch is pointing at this:

* Changing an inactive L1STD with Span == 0 to a non-zero active Span
  (introducing a new section of level-2 Stream table) requires an
  invalidation of the L1STD only. As no STEs were reachable for
  StreamIDs within the span, none require invalidation.

Which is really explicit what to do..

> Which checks out. However, I am curious why this was not seen for
> over 10 years with the current behaviour as I doubt any existing
> implementation will do negative caching. Is that something that was
> observed or found by inspection?

Honestly I don't think there is much value in negatively caching table
structure. Negatively caching the resulting leaf is good enough.

> > @@ -2053,6 +2054,14 @@ static int arm_smmu_init_l2_strtab(struct arm_smmu_device *smmu, u32 sid)
> >  				   ARRAY_SIZE((*l2table)->stes));
> >  	arm_smmu_write_strtab_l1_desc(&cfg->l2.l1tab[arm_smmu_strtab_l1_idx(sid)],
> >  				      l2ptr_dma);
> > +
> > +	/*
> > +	 * The SMMU may have cached the old, invalid L1 descriptor. Making it
> > +	 * valid needs the non-leaf form of CMD_CFGI_STE (IHI 0070 5.1.1).
> > +	 */

All the versions are 0070, you should specify rev H.a if citing the spec

Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>

Jason