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

[Pranjal Shrivastava <praan@google.com>]

On Thu, May 21, 2026 at 01:34:22PM -0700, Nicolin Chen 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 at least 1, meaning that the CD
> table will have a dummy entry (SSID=1) that will never be used.
> 
> Now for these devices, 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.
> 
> Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
> Tested-by: Nirmoy Das <nirmoyd@nvidia.com>
> Acked-by: Nirmoy Das <nirmoyd@nvidia.com>
> Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
> Reviewed-by: Kevin Tian <kevin.tian@intel.com>
> Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
> ---
>  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h |  1 +
>  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 81 ++++++++++++++++++---
>  2 files changed, 73 insertions(+), 9 deletions(-)
>

Reviewed-by: Pranjal Shrivastava <praan@google.com>

>  
> +static int arm_smmu_master_prepare_ats(struct arm_smmu_master *master)
> +{
> +	bool s1p = master->smmu->features & ARM_SMMU_FEAT_TRANS_S1;
> +	unsigned int stu = __ffs(master->smmu->pgsize_bitmap);
> +	struct pci_dev *pdev;
> +	int ret;
> +
> +	if (!dev_is_pci(master->dev))
> +		return 0;
> +	pdev = to_pci_dev(master->dev);
> +
> +	if (!arm_smmu_ats_supported(master)) {
> +		if (pci_ats_required(pdev)) {
> +			dev_err_once(master->dev, "SMMU doesn't support ATS\n");
> +			return -EOPNOTSUPP;
> +		}
> +		return 0;
> +	}
> +
> +	ret = pci_prepare_ats(pdev, stu);
> +	if (ret || !pci_ats_required(pdev))
> +		return ret;
> +
> +	/*
> +	 * S1DSS is required for ATS to be always on for identity domain cases.
> +	 * However, the S1DSS field is ignored if !IDR0_S1P or !IDR1_SSIDSIZE.
> +	 */
> +	if (!s1p || !master->smmu->ssid_bits) {
> +		dev_err_once(master->dev,
> +			     "SMMU doesn't support ATS to be always on\n");
> +		return -EOPNOTSUPP;
> +	}
> +
> +	master->ats_always_on = true;
> +
> +	return arm_smmu_alloc_cd_tables(master);

Nit: I'm not sure if I'm getting this right, are we saying we *need* to
allocate CDs for CXL.cache cases in the probe itself because STE.EATS
requires Config=Translate with S1DSS in bypass?

Does this imply that active transactions can be occurring *before* the
first domain attach? Are 

Thanks,
Praan