From: Jason Gunthorpe <jgg@nvidia.com>
To: Nicolin Chen <nicolinc@nvidia.com>
Cc: joro@8bytes.org, will@kernel.org, robin.murphy@arm.com,
rafael@kernel.org, lenb@kernel.org, bhelgaas@google.com,
iommu@lists.linux.dev, linux-kernel@vger.kernel.org,
linux-acpi@vger.kernel.org, linux-pci@vger.kernel.org,
patches@lists.linux.dev, pjaroszynski@nvidia.com,
vsethi@nvidia.com, helgaas@kernel.org, baolu.lu@linux.intel.com
Subject: Re: [PATCH RFC v2 3/4] iommu: Introduce iommu_dev_reset_prepare() and iommu_dev_reset_done()
Date: Sun, 27 Jul 2025 13:25:01 -0300 [thread overview]
Message-ID: <20250727162501.GD7551@nvidia.com> (raw)
In-Reply-To: <aIAJfYMKYKyZZRqx@Asurada-Nvidia>
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.
> 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.
> > And of course it is already quite crazy to be doing FLR during a
> > device probe so this is not a realistic scenario.
>
> Hmm, I am not sure about that, as I see iommu_deferred_attach() get
> mostly invoked by a dma_alloc() or even a dma_map(). So, this might
> not be confined to a device probe?
Once you do deferred_attach the first time it is done and won't have
any further impact. So long as the dev->iommu->attach_deferred guards
any changes to domains it is unlikely to be racing with FLR.
> > Either ignore this condition with the rational that we are about to
> > reset it so it doesn't matter, or we need to establish a new paging
> > domain for isolation purposes that has the RMR setup.
>
> Ah, you are right. ARM MSI in a VM uses RMR and sets this.
>
> But does it also raise a question that a VM having RMR can't use
> the blocked_domain, as __iommu_device_set_domain() has the exact
> same check rejecting blocked_domain? Not sure if there would be
> some unintended consequnce though...
Sounds like it needs some sorting out.. For the purposes of FLR I
think the blocked domain is OK, so maybe just move some of those
checks around?
Jason
next prev parent reply other threads:[~2025-07-27 16:25 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-06-28 7:42 [PATCH RFC v2 0/4] Disable ATS via iommu during PCI resets Nicolin Chen
2025-06-28 7:42 ` [PATCH RFC v2 1/4] iommu: Lock group->mutex in iommu_deferred_attach Nicolin Chen
2025-07-04 15:22 ` Jason Gunthorpe
2025-06-28 7:42 ` [PATCH RFC v2 2/4] iommu: Pass in gdev to __iommu_device_set_domain Nicolin Chen
2025-07-04 15:23 ` Jason Gunthorpe
2025-06-28 7:42 ` [PATCH RFC v2 3/4] iommu: Introduce iommu_dev_reset_prepare() and iommu_dev_reset_done() Nicolin Chen
2025-06-28 13:28 ` Baolu Lu
2025-06-30 12:38 ` Jason Gunthorpe
2025-06-30 17:29 ` Nicolin Chen
2025-06-30 22:49 ` Jason Gunthorpe
2025-07-04 15:43 ` Jason Gunthorpe
2025-07-22 21:58 ` Nicolin Chen
2025-07-23 2:21 ` Baolu Lu
2025-07-23 2:53 ` Nicolin Chen
2025-07-27 16:25 ` Jason Gunthorpe [this message]
2025-07-28 19:07 ` Nicolin Chen
2025-07-29 13:02 ` Jason Gunthorpe
2025-06-28 7:42 ` [PATCH RFC v2 4/4] pci: Suspend iommu function prior to resetting a device Nicolin Chen
2025-07-24 6:50 ` [PATCH RFC v2 0/4] Disable ATS via iommu during PCI resets Ethan Zhao
2025-07-25 16:41 ` Nicolin Chen
2025-07-27 12:48 ` Ethan Zhao
2025-07-27 16:20 ` Jason Gunthorpe
2025-07-29 6:16 ` Ethan Zhao
2025-07-29 12:59 ` Jason Gunthorpe
2025-07-31 1:10 ` Ethan Zhao
2025-07-31 13:47 ` Jason Gunthorpe
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20250727162501.GD7551@nvidia.com \
--to=jgg@nvidia.com \
--cc=baolu.lu@linux.intel.com \
--cc=bhelgaas@google.com \
--cc=helgaas@kernel.org \
--cc=iommu@lists.linux.dev \
--cc=joro@8bytes.org \
--cc=lenb@kernel.org \
--cc=linux-acpi@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=nicolinc@nvidia.com \
--cc=patches@lists.linux.dev \
--cc=pjaroszynski@nvidia.com \
--cc=rafael@kernel.org \
--cc=robin.murphy@arm.com \
--cc=vsethi@nvidia.com \
--cc=will@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Powered by JetHome