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 Sat, Nov 07, 2026 at 09:08:18PM +0800, Peng Fan wrote:
> On Fri, Oct 30, 2026 at 06:07:16PM -0700, Nicolin Chen wrote:
> >On Fri, Oct 30, 2026 at 08:09:36PM +0800, Peng Fan (OSS) wrote:
> >>  	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)
> 
> Thanks, I agree the reachable set here is only S1/identity/blocked, and the
> outer condition is effectively !ats_always_on. The separate downgrade
> check is gone in v8.
> 
> However, I don't think we can fold the inner check into a single
> attach_dev() call for all cases. For an S1 RID domain this is a PASID
> teardown path, and arm_smmu_attach_dev() -> arm_smmu_attach_prepare() can
> allocate with GFP_KERNEL and return -ENOMEM. The IOMMU core requires
> set_dev_pasid() not to fail here,

You got the point that S1 shouldn't propagate the return code.

The non-S1 case is the previous "downgrade" path, which didn't
propagate the return code either.

So what I was saying is simply:
-		if (!ats_always_on && downgrade)
+		if (!ats_always_on)
 			sid_domain->ops->attach_dev(sid_domain, dev,
 						    sid_domain);

Nicolin