mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
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

  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