linux-kernel - Re: [PATCH RFC v2 3/4] iommu: Introduce iommu_dev_reset_prepare() and iommu_dev_reset

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] [day] [month] [year] [list]

Message-ID: <20250729130236.GI36037@nvidia.com>
Date: Tue, 29 Jul 2025 10:02:36 -0300
From: Jason Gunthorpe <jgg@...dia.com>
To: Nicolin Chen <nicolinc@...dia.com>
Cc: joro@...tes.org, will@...nel.org, robin.murphy@....com,
	rafael@...nel.org, lenb@...nel.org, bhelgaas@...gle.com,
	iommu@...ts.linux.dev, linux-kernel@...r.kernel.org,
	linux-acpi@...r.kernel.org, linux-pci@...r.kernel.org,
	patches@...ts.linux.dev, pjaroszynski@...dia.com, vsethi@...dia.com,
	helgaas@...nel.org, baolu.lu@...ux.intel.com
Subject: Re: [PATCH RFC v2 3/4] iommu: Introduce iommu_dev_reset_prepare()
 and iommu_dev_reset_done()

On Mon, Jul 28, 2025 at 12:07:59PM -0700, Nicolin Chen wrote:
> On Sun, Jul 27, 2025 at 01:25:01PM -0300, Jason Gunthorpe wrote:
> > On Tue, Jul 22, 2025 at 02:58:21PM -0700, Nicolin Chen wrote:
> > > > /*
> > > >  * This is called on the dma mapping fast path so avoid locking. This
> > > >  * is racy, but we have an expectation that the driver will setup its
> > > >  * DMAs inside probe while still single threaded to avoid racing.
> > > >  */
> > > > if (dev->iommu && !READ_ONCE(dev->iommu->attach_deferred))
> > > 
> > > This triggers a build error as attach_deferred is a bit-field. So I
> > > am changing it from "u32 attach_deferred:1" to "bool" for this.
> > 
> > Bleck, that seems undesirable.
> 
> But inevitable for READ_ONCE :(

I guess drop the READ_ONCE change
 
> > > And, to keep the original logic, I think it should be:
> > > 	if (!dev->iommu || !READ_ONCE(dev->iommu->attach_deferred))
> > 
> > That doesn't seem right, if there is no iommu by the time a driver is
> > probed there never will be an iommu and this device should be running
> > in direct mode only.
> 
> Well, the current function does:
> 	if (dev->iommu && dev->iommu->attach_deferred)
> 		return __iommu_attach_device(domain, dev);
> 	return 0;
> 
> So, matching to that logic, it would be:
> 	if (!dev->iommu || !dev->iommu->attach_deferred)
> 		return 0;
> 	return __iommu_attach_device(domain, dev);
> then add guard(mutex).

Yeah Ok

Jason