Re: [PATCH rfcv1 6/8] iommu/arm-smmu-v3: Populate smmu_domain->invs when attaching masters

Nicolin Chen
2025-09-06T07:52:51Z

On Wed, Aug 27, 2025 at 03:21:23PM -0300, Jason Gunthorpe wrote:
> On Wed, Aug 13, 2025 at 06:25:37PM -0700, Nicolin Chen wrote:
> > +static int arm_smmu_attach_prepare_invs(struct arm_smmu_attach_state *state,
> > + struct arm_smmu_domain *new_smmu_domain)
> > +{
>
> How about a comment:
>
> /*
> * During attachment the invalidation lists on the two domains are sequenced:
> * 1. old domain is invalidating master
> * 2. new and old domain are invalidating master
> * 3. new domain is invalidating master
> *
> * This uses two updated invalidation lists, one with master added to new domain
> * and one with master removed from old domain. Prepare these lists in advance
> * of changing anything. arm_smmu_asid_lock ensures that the invalidation list
> * in the domains doesn't change while we are sequencing to update it.
> */

Having addressed the other places following your remarks, I had
a little trouble to understand that 1-2-3. But I think that can
be elaborated with:

+/*
+ * During attachment, the updates of the two domain->invs arrays are sequenced:
+ * 1. new domain updates its invs array, merging master->build_invs
+ * 2. new domain starts to include the master during its invalidation
+ * 3. master updates its STE switching from the old domain to the new domain
+ * 4. old domain still includes the master during its invalidation
+ * 5. old domain updates its invs array, unreferencing master->build_invs
+ *
+ * For 1 and 5, prepare the two updated arrays in advance, handling any changes
+ * that can possibly failure. So the actual update of either 1 or 5 won't fail.
+ * arm_smmu_asid_lock ensures that the old invs in the domains are intact while
+ * we are sequencing to update them.
+ */

Thanks
Nicolin