Re: [PATCH RFC v7 4/7] iommu/arm-smmu-v3: Use a per-domain CD table for non-PASID streams

[Nicolin Chen <nicolinc@nvidia.com>]

On Fri, Oct 30, 2026 at 08:09:36PM +0800, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
> 
> The CD describes the domain context, so non-PASID devices attached to the
> same S1 domain do not need private copies of it. Store a CD table in
> struct arm_smmu_domain and make non-PASID streams point their STE at that
> table.

> @@ -3044,6 +3055,9 @@ static void arm_smmu_domain_free_paging(struct iommu_domain *domain)
>  		mutex_lock(&arm_smmu_asid_lock);
>  		xa_erase(&arm_smmu_asid_xa, smmu_domain->cd.asid);
>  		mutex_unlock(&arm_smmu_asid_lock);
> +
> +		if (arm_smmu_cdtab_allocated(&smmu_domain->cd_table))
> +			arm_smmu_free_domain_cd_tables(smmu_domain);

Same comments as previous: fold arm_smmu_cdtab_allocated in.

> @@ -3854,11 +3879,27 @@ static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev,
>  		return -EINVAL;
>  
>  	if (smmu_domain->stage == ARM_SMMU_DOMAIN_S1) {
> -		cdptr = arm_smmu_alloc_cd_ptr(master, IOMMU_NO_PASID);
> +		/*
> +		 * Non-PASID devices use the domain's shared CD table. The
> +		 * private table is needed for substreams (PASID), for
> +		 * ATS-always-on (S1DSS), and for the stall model: CD.S is a
> +		 * per-CD property and cannot be shared by devices that
> +		 * terminate faults.
> +		 */
> +		if (master->ats_always_on ||
> +		    master->stall_enabled ||

For readability, let's add a helper:

static bool arm_smmu_master_needs_private_cdtab(struct arm_smmu_master *master)
{
	/* Reason 1 */
	if (master->ats_always_on)
		return true;
	/* Reason 2 */
	if (master->stall_enabled)
		return true;
	return false;
}

> @@ -3978,10 +4022,17 @@ int arm_smmu_set_pasid(struct arm_smmu_master *master,
>  	    sid_domain->type != IOMMU_DOMAIN_BLOCKED)
>  		return -EINVAL;
>  
> +	first = !arm_smmu_ssids_in_use(&master->cd_table);
>  	cdptr = arm_smmu_alloc_cd_ptr(master, pasid);
>  	if (!cdptr)
>  		return -ENOMEM;
>  
> +	if (first) {
> +		cd0ptr = arm_smmu_alloc_cd_ptr(master, IOMMU_NO_PASID);
> +		if (!cd0ptr)
> +			return -ENOMEM;
> +	}

Maybe allocate cd0ptr in arm_smmu_alloc_cd_tables()?

We can also have a helper to share with arm_smmu_alloc_cd_ptr().

static int arm_smmu_alloc_cd_ptr_l2(struct arm_smmu_master *master, u32 ssid)
{
	[...]
	*l2ptr = dma_alloc_coherent(master->smmu->dev, sizeof(**l2ptr),
				    &l2ptr_dma, GFP_KERNEL);
	if (!*l2ptr)
		return -ENOMEM;

	arm_smmu_write_cd_l1_desc(&cd_table->l2.l1tab[idx], l2ptr_dma);
	return 0;
}

> @@ -3997,8 +4048,39 @@ int arm_smmu_set_pasid(struct arm_smmu_master *master,
>  	cd->data[0] |= cpu_to_le64(
>  		FIELD_PREP(CTXDESC_CD_0_ASID, smmu_domain->cd.asid));
>  
> +	/*
> +	 * The private CD table is taking over the STE now. Make sure its
> +	 * default substream matches the RID domain before the STE points at
> +	 * it, since non-PASID traffic follows CD[0].
> +	 */
> +	if (first) {
> +		if ((sid_domain->type & __IOMMU_DOMAIN_PAGING) &&
> +		    to_smmu_domain(sid_domain)->stage == ARM_SMMU_DOMAIN_S1) {
> +			struct arm_smmu_cd cd0;
> +
> +			arm_smmu_make_s1_cd(&cd0, master,
> +					    to_smmu_domain(sid_domain));
> +			arm_smmu_write_cd_entry(master, IOMMU_NO_PASID,
> +						cd0ptr, &cd0);
> +		} else if (sid_domain->type == IOMMU_DOMAIN_IDENTITY ||
> +			   sid_domain->type == IOMMU_DOMAIN_BLOCKED) {
> +			arm_smmu_clear_cd(master, IOMMU_NO_PASID);
> +		} else {
> +			WARN_ON(1);
> +		}
> +	}
> +
>  	arm_smmu_write_cd_entry(master, pasid, cdptr, cd);
> -	arm_smmu_update_ste(master, sid_domain, state.ats_enabled);
> +
> +	/*
> +	 * A repeat PASID attach leaves the STE on the private table with the
> +	 * same S1DSS/EATS. Only the first attach (or an ATS change) needs to
> +	 * rewrite it; arm_smmu_write_ste() would otherwise issue a redundant
> +	 * CMD_PREFETCH_CFG for every stream.
> +	 */
> +	if (first || !master->cd_table.in_ste ||
> +	    master->ste_ats_enabled != state.ats_enabled)
> +		arm_smmu_update_ste(master, sid_domain, state.ats_enabled);

master->cd_table.in_ste could be set when STE walks master's CD
table. Then, you don't need to check "first". And move the guard
back into arm_smmu_update_ste.

> @@ -4017,35 +4099,48 @@ static int arm_smmu_blocking_set_dev_pasid(struct iommu_domain *new_domain,
>  		.old_domain = old_domain,
>  		.ssid = pasid,
>  	};
> +	int ret = 0;
>  
>  	mutex_lock(&arm_smmu_asid_lock);
>  	arm_smmu_attach_prepare_invs(&state, NULL);
> +	/*
> +	 * Unlink the PASID from the domain before invalidating its CD.
> +	 * arm_smmu_mm_release() walks this list with only devices_lock held
> +	 * and writes a valid, faulting CD; unlinking first prevents it from
> +	 * resurrecting used_ssids after the last PASID is detached.
> +	 */
> +	state.old_master_domain = arm_smmu_remove_master_domain(
> +		master, &smmu_domain->domain, pasid);
>  	arm_smmu_clear_cd(master, pasid);

Sashiko pointed out that it needs devices_lock here.

>  	/*
> -	 * When the last user of the CD table goes away downgrade the STE back
> -	 * to a non-cd_table one, by re-attaching its sid_domain.
> +	 * When the last user of the CD table goes away, switch the STE back to
> +	 * the domain's shared CD table (S1) or to the native bypass/abort STE
> +	 * (identity/blocked), by re-attaching its sid_domain. The private CD
> +	 * table is kept allocated for reuse and freed when the device goes
> +	 * away.
>  	 */
>  	if (!arm_smmu_ssids_in_use(&master->cd_table)) {
>  		struct iommu_domain *sid_domain =
>  			iommu_driver_get_domain_for_dev(master->dev);
>  		bool ats_always_on = master->ats_always_on &&
>  				     sid_domain->type != IOMMU_DOMAIN_BLOCKED;
> -		bool downgrade = sid_domain->type == IOMMU_DOMAIN_IDENTITY ||
> -				 sid_domain->type == IOMMU_DOMAIN_BLOCKED;
> +		bool s1 = (sid_domain->type & __IOMMU_DOMAIN_PAGING) &&
> +			  to_smmu_domain(sid_domain)->stage ==
> +				  ARM_SMMU_DOMAIN_S1;
>  
> -		if (!ats_always_on && downgrade)
> -			sid_domain->ops->attach_dev(sid_domain, dev,
> -						    sid_domain);
> +		if (!ats_always_on &&
> +		    (s1 || sid_domain->type == IOMMU_DOMAIN_IDENTITY ||
> +		     sid_domain->type == IOMMU_DOMAIN_BLOCKED))
> +			ret = sid_domain->ops->attach_dev(sid_domain, dev,
> +							  sid_domain);

This covers almost all the possible domain types here. So, just
drop that downgrade, and it should cover the other cases:

		if (!ats_always_on)

Nicolin