[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20260123191654.GD1134360@nvidia.com>
Date: Fri, 23 Jan 2026 15:16:54 -0400
From: Jason Gunthorpe <jgg@...dia.com>
To: Nicolin Chen <nicolinc@...dia.com>
Cc: Will Deacon <will@...nel.org>, jean-philippe@...aro.org,
robin.murphy@....com, joro@...tes.org, 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 09:56:01AM -0800, Nicolin Chen wrote:
> On Fri, Jan 23, 2026 at 05:51:52PM +0000, Will Deacon wrote:
> > On Fri, Jan 23, 2026 at 09:35:58AM -0800, Nicolin Chen wrote:
> > > 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?
> >
> > My question, really, is why do you need the atomic properties in this patch
> > series? It just looks like overhead at the moment.
>
> Hmm, shouldn't it be atomic, since..
>
> (might be IRQ, no mutex) __arm_smmu_domain_inv_range() reads it.
> (mutex protected) arm_smmu_attach_dev() writes it.
>
> ..?
It doesn't need to be a full atomic, you can use WRITE_ONCE/READ_ONCE
instead for this case.
The general argument of this scheme is it doesn't matter if the
invalidation side is doing extra invalidation, that is supposed to
always be safe except for ATS. For ATS we hold locks and then it
doesn't to be atomic because of the lock.
So I think Will has it right, just use READ_ONCE/WRITE_ONCE on a naked
unsigned int.
Jason
Powered by blists - more mailing lists