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: <aL8ePHvQ25LUU81J@nvidia.com>
Date: Mon, 8 Sep 2025 11:19:40 -0700
From: Nicolin Chen <nicolinc@...dia.com>
To: Jason Gunthorpe <jgg@...dia.com>
CC: <will@...nel.org>, <robin.murphy@....com>, <joro@...tes.org>,
	<jean-philippe@...aro.org>, <miko.lenczewski@....com>, <balbirs@...dia.com>,
	<peterz@...radead.org>, <smostafa@...gle.com>, <kevin.tian@...el.com>,
	<praan@...gle.com>, <zhangzekun11@...wei.com>,
	<linux-arm-kernel@...ts.infradead.org>, <iommu@...ts.linux.dev>,
	<linux-kernel@...r.kernel.org>, <patches@...ts.linux.dev>
Subject: Re: [PATCH rfcv1 7/8] iommu/arm-smmu-v3: Add arm_smmu_invs based
 arm_smmu_domain_inv_range()

On Mon, Sep 08, 2025 at 12:39:11PM -0300, Jason Gunthorpe wrote:
> On Sat, Sep 06, 2025 at 01:12:33AM -0700, Nicolin Chen wrote:
> 
> > I know that performance-wise, this piece will be a quick respin,
> > as the attach side releases the lock very fast. It still looks
> > a bit complicated. And practically, it would respin even if the
> > attachment removes a non-PCI device, right?
> 
> If you are paying the cost of taking the lock then it should become
> fully locked and consistent.

Well, the point is that the reader doesn't know if an ATS entry
is getting removed, and it can only speculate by looking at the
full list.

So, would it be better to just always take the read lock, while
applying the ATS condition to the writer side:

[Reader]
+	/* A concurrent attachment has changed the array. Do a respin */
+	if (unlikely(!read_trylock(&invs->rwlock)))
+		goto again;
+	if (unlikely(!invs->old)) {
+		read_unlock(&invs->rwlock);
+		goto again;
+	}
...
+	read_unlock(&invs->rwlock);

[Writer]
+	bool ats_disabled = master->ats_enabled && !state->ats_enabled;
...
+	if (ats_disabled)
+		write_lock_irqsave(&old_invs->rwlock, flags);
+	WRITE_ONCE(old_invs->old, true);
+	if (ats_disabled)
+		write_unlock_irqrestore(&old_invs->rwlock, flags);

?

Thanks
Nicolin

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ