Re: [PATCH v9 6/7] iommu/arm-smmu-v3: Add arm_smmu_invs based arm_smmu_domain_inv_range()

Will Deacon
2026-01-23T17:05:31Z

On Fri, Dec 19, 2025 at 12:11:28PM -0800, Nicolin Chen wrote:
> +void arm_smmu_domain_inv_range(struct arm_smmu_domain *smmu_domain,
> + unsigned long iova, size_t size,
> + unsigned int granule, bool leaf)
> +{
> + struct arm_smmu_invs *invs;
> +
> + /*
> + * An invalidation request must follow some IOPTE change and then load
> + * an invalidation array. In the meantime, a domain attachment mutates
> + * the array and then stores an STE/CD asking SMMU HW to acquire those
> + * changed IOPTEs. In other word, these two are interdependent and can
> + * race.
> + *
> + * In a race, the RCU design (with its underlying memory barriers) can
> + * ensure the invalidation array to always get updated before loaded.
> + *
> + * smp_mb() is used here, paired with the smp_mb() following the array
> + * update in a concurrent attach, to ensure:
> + * - HW sees the new IOPTEs if it walks after STE installation
> + * - Invalidation thread sees the updated array with the new ASID.
> + *
> + * [CPU0] | [CPU1]
> + * |
> + * change IOPTEs and TLB flush: |
> + * arm_smmu_domain_inv_range() { | arm_smmu_install_new_domain_invs {
> + * ... | rcu_assign_pointer(new_invs);
> + * smp_mb(); // ensure IOPTEs | smp_mb(); // ensure new_invs
> + * ... | kfree_rcu(old_invs, rcu);
> + * // load invalidation array | }
> + * invs = rcu_dereference(); | arm_smmu_install_ste_for_dev {
> + * | STE = TTB0 // read new IOPTEs
> + */
> + smp_mb();
> +
> + rcu_read_lock();
> + invs = rcu_dereference(smmu_domain->invs);
> +
> + /*
> + * Avoid locking unless ATS is being used. No ATC invalidation can be
> + * going on after a domain is detached.
> + */
> + if (invs->has_ats) {
> + read_lock(&invs->rwlock);
> + __arm_smmu_domain_inv_range(invs, iova, size, granule, leaf);
> + read_unlock(&invs->rwlock);
> + } else {
> + __arm_smmu_domain_inv_range(invs, iova, size, granule, leaf);
> + }
> +
> + rcu_read_unlock();
> +}
> +
> static void arm_smmu_tlb_inv_page_nosync(struct iommu_iotlb_gather *gather,
> unsigned long iova, size_t granule,
> void *cookie)
> @@ -3285,6 +3483,23 @@ arm_smmu_install_new_domain_invs(struct arm_smmu_attach_state *state)
> return;
>
> rcu_assign_pointer(*invst->invs_ptr, invst->new_invs);
> + /*
> + * We are committed to updating the STE. Ensure the invalidation array
> + * is visible to concurrent map/unmap threads, and acquire any racing
> + * IOPTE updates.
> + *
> + * [CPU0] | [CPU1]
> + * |
> + * change IOPTEs and TLB flush: |
> + * arm_smmu_domain_inv_range() { | arm_smmu_install_new_domain_invs {
> + * ... | rcu_assign_pointer(new_invs);
> + * smp_mb(); // ensure IOPTEs | smp_mb(); // ensure new_invs
> + * ... | kfree_rcu(old_invs, rcu);
> + * // load invalidation array | }
> + * invs = rcu_dereference(); | arm_smmu_install_ste_for_dev {
> + * | STE = TTB0 // read new IOPTEs
> + */
> + smp_mb();
> kfree_rcu(invst->old_invs, rcu);
> }
>
> @@ -3334,6 +3549,23 @@ arm_smmu_install_old_domain_invs(struct arm_smmu_attach_state *state)
> return;
>
> rcu_assign_pointer(*invst->invs_ptr, new_invs);
> + /*
> + * We are committed to updating the STE. Ensure the invalidation array
> + * is visible to concurrent map/unmap threads, and acquire any racing
> + * IOPTE updates.
> + *
> + * [CPU0] | [CPU1]
> + * |
> + * change IOPTEs and TLB flush: |
> + * arm_smmu_domain_inv_range() { | arm_smmu_install_old_domain_invs {
> + * ... | rcu_assign_pointer(new_invs);
> + * smp_mb(); // ensure IOPTEs | smp_mb(); // ensure new_invs
> + * ... | kfree_rcu(old_invs, rcu);
> + * // load invalidation array | }
> + * invs = rcu_dereference(); | arm_smmu_install_ste_for_dev {
> + * | STE = TTB0 // read new IOPTEs
> + */
> + smp_mb();

I don't think we need to duplicate this comment three times, you can just
refer to the first function (e.g. "See ordering comment in
arm_smmu_domain_inv_range()").

However, isn't the comment above misleading for this case?
arm_smmu_install_old_domain_invs() has the sequencing the other way
around on CPU 1: we should update the STE first.

Will