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

Message-ID: <aGLIEhoIiUIjI/MP@Asurada-Nvidia>
Date: Mon, 30 Jun 2025 10:29:12 -0700
From: Nicolin Chen <nicolinc@...dia.com>
To: Jason Gunthorpe <jgg@...dia.com>
CC: Baolu Lu <baolu.lu@...ux.intel.com>, <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>
Subject: Re: [PATCH RFC v2 3/4] iommu: Introduce iommu_dev_reset_prepare()
 and iommu_dev_reset_done()

On Mon, Jun 30, 2025 at 09:38:14AM -0300, Jason Gunthorpe wrote:
> On Sat, Jun 28, 2025 at 09:28:12PM +0800, Baolu Lu wrote:
>  
> > Does this mean the IOMMU driver should disable ATS when ops-
> > >blocked_domain is used? This might not be feasible because ops-
> > >blocked_domain might possibly be attached to a PASID of a device,
> > while other PASIDs still use ATS for functionality.
> 
> No.. The above should be setting everything, including PASIDs to the
> blocked domain.
> 
> The driver doesn't have to disable ATS at the device, but ARM does.

Oh, the code is expecting a pci_disable_ats() call, as the next
patch will check if ats is disabled on the PCI side..

If that's the case, we'd have to leave the ATS enabled but only
trust that iommu driver won't issue any new ATS invalidation?

Or should we ask driver to be "must" v.s. "doesn't have to"?

> > > +	/* Device is already attached to the blocked_domain. Nothing to do */
> > > +	if (group->domain->type == IOMMU_DOMAIN_BLOCKED)
> > > +		goto unlock;
> > 
> > "group->domain->type == IOMMU_DOMAIN_BLOCKED" means that IOMMU_NO_PASID
> > is docked in the blocking DMA state, but it doesn't imply that other
> > PASIDs are also in the blocking DMA state. Therefore, we might still
> > need the following lines to handle other PASIDs.
> 
> Yes, we always have to check the xarray.

OK. This check should apply to the RID domain attach only then.

> > On the other hand, perhaps we should use "group->domain == ops-
> > >blocked_domain" instead of "group->domain->type ==
> > IOMMU_DOMAIN_BLOCKED" to make the code consistent with the commit
> > message.
> 
> ops->blocked_domain is not good, we support devices without static
> blocking domain. But yes, using DOMAIN_BLOCKED is not greap, there is
> a group->blocked_domain that should be used and will dynamicaly create
> an empty paging domain if needed.

You mean we should use the group->blocking_domain, even if it was
allocated to be a paging domain as the driver doesn't understand
a IOMMU_DOMAIN_BLOCKED yet?

Thanks
Nicolin