Re: [PATCH RFC v6 5/5] iommu/arm-smmu-v3: Wire up shared-SID STE ordering and feature gating

[Nicolin Chen <nicolinc@nvidia.com>]

On Mon, Oct 12, 2026 at 09:23:32PM +0800, Peng Fan wrote:
> >+static bool arm_smmu_skip_shared_ste(struct arm_smmu_device *smmu,
> >+				     u32 sid, bool is_abort)
> >+{
> >+	struct arm_smmu_stream *stream;
> >+
> >+	lockdep_assert_held(&smmu->streams_mutex);
> >+
> >+	stream = arm_smmu_find_stream(smmu, sid);
> >+	if (!stream || list_empty(&stream->shared_masters))
> >+		return false;
> >+
> >+	if (is_abort) {
> >+		stream->ste_installed = false;
> >+		return true;
> >+	}
> >+	if (stream->ste_installed)
> >+		return true;
> >+	stream->ste_installed = true;
> >+	return false;
> 
> sashiko has comments: https://sashiko.dev/#/patchset/20261011-smmu-shared-sid-v6-0-cb76b89dc612%40nxp.com
> 
> We just need to support a simple case that platform devices(single SID) which
> could share there SIDs, such as MMC-0 and MMC-1 share one SID.
> 
> Shared-SID masters are restricted to platform devices sharing a single SID.
> SVA, IOPF, and vSMMU nesting are all rejected for shared-SID masters (patch 5),
> so VFIO passthrough and direct domain replacement scenarios do not apply.
> The only domain transitions are the default DMA domain attach at probe time
> and ABORT on unbind. Or I may miss something.
 
Domain can be switched between DMA and IDENTITY via sysfs.

I think there should be a way to support that. But, without a concrete
use case, it might be easier to add this into the attach_dev callbacks
(except arm_smmu_attach_dev_blocked):

	if (arm_smmu_master_shares_sid(master) &&
	    old_domain && old_domain != domain &&
	    old_domain != &arm_smmu_blocked_domain)
		return -EBUSY;

And Sashiko's second remark does reveal a real bug:
"Additionally, when devices are detached from a domain, is_abort is true.
 arm_smmu_skip_shared_ste() clears ste_installed but unconditionally returns
 true for all masters. Will this cause the ABORT STE to never be written to
 hardware?"

Given that your use case always group SID-sharing masters in the same
iommu_group and attach/replace a domain on those masters together, I
think a simpler solution can be to always let stream->master install
the shared STE.

One pitfall would be the release domain's attach_dev, which would go
through arm_smmu_attach_dev_blocked() just like a blocked domain. So,
it has to skip correctly if the shared stream->master == master. This
means we might have to add a separate arm_smmu_attach_dev_release().

And I realized that there is another problem in your design: the CD
table is still allocated per master; see arm_smmu_alloc_cd_ptr(). If
STE is shared, its CD table must be shared too.

You'd need to redirect all the master->cd_table with something like:

static struct arm_smmu_master *arm_smmu_cd_owner(struct arm_smmu_master *master)
{
	if (arm_smmu_master_shares_sid(master)) {
		int i;

		for (i = 0; i < master->num_streams; i++) {
			struct arm_smmu_stream *stream = master->streams[i];

			if (!list_empty(&stream->shared_masters))
				return stream->master;
		}
	}

	return master;
}

> >@@ -4360,9 +4391,11 @@ static int arm_smmu_insert_master(struct arm_smmu_device *smmu,
> I am thinking to simplify reject if stall_enabled is true:
>  			if (list_empty(&stream->shared_masters)) {
> +				if (stream->master->stall_enabled) {
> +					dev_warn(master->dev,
> +						 "Shared StreamID 0x%x rejected: existing master has stall enabled\n",
> +						 sid);
> +					ret = -EBUSY;
> +					break;
> +				}
 
I'd move it upwards: next to the existing "num_streams != 1" guard.

Nicolin