Re: [PATCH v2 3/3] iommu/arm-smmu-v3: Allow ATS to be always on

[Jonathan Cameron]

On Mon, 23 Feb 2026 14:52:22 -0800
Nicolin Chen <nicolinc@xxxxxxxxxx> wrote:

> When a device's default substream attaches to an identity domain, the SMMU
> driver currently sets the device's STE between two modes:
> 
>   Mode 1: Cfg=Translate, S1DSS=Bypass, EATS=1
>   Mode 2: Cfg=bypass (EATS is ignored by HW)
> 
> When there is an active PASID (non-default substream), mode 1 is used. And
> when there is no PASID support or no active PASID, mode 2 is used.
> 
> The driver will also downgrade an STE from mode 1 to mode 2, when the last
> active substream becomes inactive.
> 
> However, there are PCIe devices that demand ATS to be always on. For these
> devices, their STEs have to use the mode 1 as HW ignores EATS with mode 2.
> 
> Change the driver accordingly:
>   - always use the mode 1
>   - never downgrade to mode 2
>   - allocate and retain a CD table (see note below)
> 
> Note that these devices might not support PASID, i.e. doing non-PASID ATS.
> In such a case, the ssid_bits is set to 0. However, s1cdmax must be set to
> a !0 value in order to keep the S1DSS field effective. Thus, when a master
> requires ats_always_on, set its s1cdmax to minimal 1, meaning the CD table
> will have a dummy entry (SSID=1) that will be never used.
> 
> Now, for these device, arm_smmu_cdtab_allocated() will always return true,
> v.s. false prior to this change. When its default substream is attached to
> an IDENTITY domain, its first CD is NULL in the table, which is a totally
> valid case. Thus, add "!master->ats_always_on" to the condition.
> 
> Signed-off-by: Nicolin Chen <nicolinc@xxxxxxxxxx>

In my head at least, it would be nice if a driver had to explicitly opt in
to this behavior. The presence of CXL.cache support for instance doesn't
actually mean the driver is going to enable it.  However it is probably
harmless so lets go with this as path of least resistance.

One trivial thing inline (ignore if you like!) Otherwise LGTM
Reviewed-by: Jonathan Cameron <jonathan.cameron@xxxxxxxxxx>

> 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 4d00d796f0783..69d40918bc22f 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c

> @@ -3274,7 +3294,10 @@ static void arm_smmu_attach_dev_ste(struct iommu_domain *domain,
>  		.old_domain = old_domain,
>  		.ssid = IOMMU_NO_PASID,
>  	};
> +	bool ats_always_on = false;
>  
> +	if (master->ats_always_on && s1dss != STRTAB_STE_1_S1DSS_TERMINATE)
> +		ats_always_on = true;
Maybe flatten this to not do the off then on dance.

	bool ats_always_on = master->ats_always_on && s1dss != STRTAB_STE_1_S1DSS_TERMINATE;