linux-kernel - Re: [PATCH v9 3/7] iommu/arm-smmu-v3: Introduce a per-domain arm_smmu

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aXOxfi8ZQAkr4zHU@Asurada-Nvidia>
Date: Fri, 23 Jan 2026 09:35:58 -0800
From: Nicolin Chen <nicolinc@...dia.com>
To: Will Deacon <will@...nel.org>
CC: <jean-philippe@...aro.org>, <robin.murphy@....com>, <joro@...tes.org>,
	<jgg@...dia.com>, <balbirs@...dia.com>, <miko.lenczewski@....com>,
	<peterz@...radead.org>, <kevin.tian@...el.com>, <praan@...gle.com>,
	<linux-arm-kernel@...ts.infradead.org>, <iommu@...ts.linux.dev>,
	<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v9 3/7] iommu/arm-smmu-v3: Introduce a per-domain
 arm_smmu_invs array

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