Re: [RFC PATCH 3/3] iommu/arm-smmu-v3: Add support for hitless replace of S1 domains

[Samiullah Khawaja <skhawaja@google.com>]

On Fri, Oct 23, 2026 at 02:43:28PM +0100, Robin Murphy wrote:
>On 25/09/2026 2:13 am, Samiullah Khawaja wrote:
>>S1 domains are programmed by CD entries in the CD table. During a new
>>domain attach these are updated using write_cd_entry. Since ASID and
>>TTB0 are in different 64-bit words, the write_cd_entry considers it a
>>non-hitless update and sets V=0. This means such an update is disruptive
>>and would generate translation faults if there are ongoing DMAs.
>>
>>Replace the CD entries by using an unused temporary ASID in following
>>sequence to allow hitless replacement of CD entry,
>>
>>- Update the CD entry with temporary ASID.
>>- Update the CD entry with the target TTB0.
>>- Update the CD entry with the target ASID.
>>
>>Signed-off-by: Samiullah Khawaja <skhawaja@google.com>
>>---
>>  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 66 ++++++++++++++++++++-
>>  1 file changed, 64 insertions(+), 2 deletions(-)
>>
>>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 5732f3ba0122..59f4b8cf89f6 100644
>>--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
>>+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
>>@@ -1641,6 +1641,68 @@ void arm_smmu_write_cd_entry(struct arm_smmu_master *master, int ssid,
>>  	arm_smmu_write_entry(&cd_writer.writer, cdptr->data, target->data);
>>  }
>>+/* Invalidate every stage-1 TLB entry tagged with @asid. */
>>+static void arm_smmu_tlb_inv_asid(struct arm_smmu_device *smmu, u16 asid)
>>+{
>>+	enum arm_smmu_cmdq_opcode op = (smmu->features & ARM_SMMU_FEAT_E2H) ?
>>+					       CMDQ_OP_TLBI_EL2_ASID :
>>+					       CMDQ_OP_TLBI_NH_ASID;
>>+
>>+	arm_smmu_cmdq_issue_cmd_with_sync(smmu,
>>+					  arm_smmu_make_cmd_tlbi(op, asid, 0));
>>+}
>
>Feels like this should probably use the existing invalidation machinery...

I see, I can use the arm_smmu_inv_flush_iotlb_tag() to issue
invalidation for the ASID.
>
>>+
>>+static void arm_smmu_cd_set_asid(struct arm_smmu_cd *cd, u16 asid)
>>+{
>>+	cd->data[0] &= ~cpu_to_le64(CTXDESC_CD_0_ASID);
>>+	cd->data[0] |= cpu_to_le64(FIELD_PREP(CTXDESC_CD_0_ASID, asid));
>>+}
>>+
>>+static void arm_smmu_replace_cd_entry(struct arm_smmu_master *master, int ssid,
>>+				      struct arm_smmu_cd *cdptr,
>>+				      const struct arm_smmu_cd *target)
>>+{
>>+	struct arm_smmu_device *smmu = master->smmu;
>>+	struct arm_smmu_cd stage;
>>+	u32 tmp_asid;
>>+
>>+	lockdep_assert_held(&arm_smmu_asid_lock);
>>+
>>+	/*
>>+	 * Nothing is translating through an invalid CD, and an update that
>>+	 * invalidates one cannot be made hitless anyway.
>>+	 */
>>+	if (!(cdptr->data[0] & cpu_to_le64(CTXDESC_CD_0_V)) ||
>>+	    !(target->data[0] & cpu_to_le64(CTXDESC_CD_0_V)))
>>+		goto write_directly;
>>+
>>+	if (xa_alloc(&arm_smmu_asid_xa, &tmp_asid, XA_ZERO_ENTRY,
>>+		     XA_LIMIT(1, (1 << smmu->asid_bits) - 1), GFP_KERNEL))
>>+		goto write_directly;
>>+
>>+	/* Copy the existing cd entry and update only the ASID. */
>>+	stage = *cdptr;
>>+	arm_smmu_cd_set_asid(&stage, tmp_asid);
>>+	arm_smmu_write_cd_entry(master, ssid, cdptr, &stage);
>>+
>>+	/* Copy the target cd entry and update ASID to the temporary ASID. */
>>+	stage = *target;
>>+	arm_smmu_cd_set_asid(&stage, tmp_asid);
>>+	arm_smmu_write_cd_entry(master, ssid, cdptr, &stage);
>>+
>>+	arm_smmu_tlb_inv_asid(smmu, tmp_asid);
>>+
>>+	/* Update to the target cd entry as it should be hitless now. */
>>+	arm_smmu_write_cd_entry(master, ssid, cdptr, target);
>>+
>>+	arm_smmu_tlb_inv_asid(smmu, tmp_asid);
>>+	xa_erase(&arm_smmu_asid_xa, tmp_asid);
>>+	return;
>>+
>>+write_directly:
>>+	arm_smmu_write_cd_entry(master, ssid, cdptr, target);
>>+}
>>+
>>  void arm_smmu_make_s1_cd(struct arm_smmu_cd *target,
>>  			 struct arm_smmu_master *master,
>>  			 struct arm_smmu_domain *smmu_domain)
>>@@ -3664,8 +3726,8 @@ static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev,
>>  		struct arm_smmu_cd target_cd;
>>  		arm_smmu_make_s1_cd(&target_cd, master, smmu_domain);
>>-		arm_smmu_write_cd_entry(master, IOMMU_NO_PASID, cdptr,
>>-					&target_cd);
>>+		arm_smmu_replace_cd_entry(master, IOMMU_NO_PASID, cdptr,
>>+					  &target_cd);
>
>No, we cannot blindly do this for arbitrary ->attach_dev calls - the 
>majority of S1->S1 transitions are going to be moving back and forth 
>between the default domain and user domains, where there is zero 
>expectation, let alone guarantee, that the translations would be 
>equivalent and thus not liable to lead to TLB conflict aborts or 
>amalgamation hell.
>
>This needs to be in a dedicated "I know what I'm doing and I fully 
>accept the consequences if I get it wrong" call path.

Agreed.

I will update this to only be triggered with explicit request from the
Core to satisfy my live update use case where a preserved iommu domain
is being replaced by a new domain setup by the user.

Thanks,
Sami
>
>Thanks,
>Robin.
>
>>  		arm_smmu_make_cdtable_ste(&target, master, state.ats_enabled,
>>  					  STRTAB_STE_1_S1DSS_SSID0);
>>  		arm_smmu_install_ste_for_dev(master, &target);
>