Re: [PATCH v9 3/7] iommu/arm-smmu-v3: Introduce a per-domain arm_smmu_invs array

Nicolin Chen
2026-01-23T17:36:41Z

On Fri, Jan 23, 2026 at 05:03:10PM +0000, Will Deacon wrote:
> On Fri, Dec 19, 2025 at 12:11:25PM -0800, Nicolin Chen wrote:
> > +struct arm_smmu_inv {
> > + struct arm_smmu_device *smmu;
> > + u8 type;
> > + u8 size_opcode;
> > + u8 nsize_opcode;
> > + u32 id; /* ASID or VMID or SID */
> > + union {
> > + size_t pgsize; /* ARM_SMMU_FEAT_RANGE_INV */
> > + u32 ssid; /* INV_TYPE_ATS */
> > + };
> > +
> > + refcount_t users; /* users=0 to mark as a trash to be purged */
>
> The refcount_t API uses atomics with barrier semantics. Do we actually
> need those properties when updating the refcounts here? The ASID lock
> gives us pretty strong serialisation even after this patch series and
> we rely heavily on that.

But we can't use that mutex in the invalidation function that
might be an IRQ context?

> > + /* Test6: purge test_b (new array) */
> > + test_a = arm_smmu_invs_purge(test_b);
> > + kfree(test_b);
> > + arm_smmu_v3_invs_test_verify(test, test_a, ARRAY_SIZE(results6[0]),
> > + results6[0], results6[1]);
> > +
> > + /* Test7: unref invs3 (same array) */
> > + arm_smmu_invs_unref(test_a, &invs3, NULL);
> > + KUNIT_EXPECT_EQ(test, test_a->num_invs, 0);
> > + KUNIT_EXPECT_EQ(test, test_a->num_trashes, 0);
>
> Wouldn't we be better off testing num_trashes == 0 after test 6 has
> completed?

OK.

> > +/**
> > + * arm_smmu_invs_for_each_entry - Iterate over two sorted arrays computing for
> > + * arm_smmu_invs_merge() or arm_smmu_invs_unref()
> > + * @invs_l: the base invalidation array
> > + * @idx_l: a stack variable of 'size_t', to store the base array index
> > + * @invs_r: the build_invs array as to_merge or to_unref
> > + * @idx_r: a stack variable of 'size_t', to store the build_invs index
> > + * @cmp: a stack variable of 'int', to store return value (-1, 0, or 1)
> > + */
> > +#define arm_smmu_invs_for_each_cmp(invs_l, idx_l, invs_r, idx_r, cmp) \
>
> nit: the kerneldoc comment doesn't match the name of this function.

I will fix this.

Thanks
Nicolin