public inbox for linux-arm-kernel@lists.infradead.org
 help / color / mirror / Atom feed
From: Baolu Lu <baolu.lu@linux.intel.com>
To: Nicolin Chen <nicolinc@nvidia.com>
Cc: Will Deacon <will@kernel.org>,
	Robin Murphy <robin.murphy@arm.com>,
	Joerg Roedel <joro@8bytes.org>,
	Bjorn Helgaas <bhelgaas@google.com>,
	Jason Gunthorpe <jgg@nvidia.com>,
	"Rafael J . Wysocki" <rafael@kernel.org>,
	Len Brown <lenb@kernel.org>,
	Pranjal Shrivastava <praan@google.com>,
	Mostafa Saleh <smostafa@google.com>,
	Kevin Tian <kevin.tian@intel.com>,
	linux-arm-kernel@lists.infradead.org, iommu@lists.linux.dev,
	linux-kernel@vger.kernel.org, linux-acpi@vger.kernel.org,
	linux-pci@vger.kernel.org, vsethi@nvidia.com,
	Shuai Xue <xueshuai@linux.alibaba.com>
Subject: Re: [PATCH v3 06/11] iommu: Defer __iommu_group_free_device() to be outside group->mutex
Date: Fri, 24 Apr 2026 10:29:41 +0800	[thread overview]
Message-ID: <667647da-c8e3-483f-aa57-c3730194c8a5@linux.intel.com> (raw)
In-Reply-To: <aeo/BDwYM9yYHctI@Asurada-Nvidia>

On 4/23/26 23:47, Nicolin Chen wrote:
> On Thu, Apr 23, 2026 at 03:55:02PM +0800, Baolu Lu wrote:
>> On 4/17/26 07:28, Nicolin Chen wrote:
>>> +static void __iommu_group_empty_assert_owner_cnt(struct iommu_group *group)
>>> +{
>>> +	lockdep_assert_held(&group->mutex);
>>> +	/*
>>> +	 * If the group has become empty then ownership must have been
>>> +	 * released, and the current domain must be set back to NULL or
>>> +	 * the default domain.
>>> +	 */
>>
>> Nit: this comment doesn't quite match the following code. The code
>> doesn't check "group->domain != NULL". Or perhaps in that case,
>> group->default_domain must be NULL?
> 
> This is the original patch from Jason:
> https://lore.kernel.org/r/4-v3-328044aa278c+45e49-iommu_probe_jgg@nvidia.com
> 
> I kept the comments as-is, though It might be slightly confusing?
> 
> I think it means:
> If group->default_domain == NULL, it does check "set back to NULL".
> If group->default_domain != NULL, it then checks "default domain".
> 
> Maybe it could be "must be set back to the default domain (which
> itself can be NULL"?

This is clearer. As I understand it, when the last device leaves the
iommu_group, the group->domain should be one of the static system
domains, either the default domain or the blocking domain.

> 
>> Furthermore, if a device is currently quarantined, group->domain will be
>> the blocking_domain. If that quarantined device is then hot-removed and
>> happens to be the last device in the group, will this WARN_ON trigger
>> unnecessarily?
> 
> If a device is quarantined, its group->domain is retained to the
> previously attached domain. Its blocking state is logged in the
> gdev->blocked flag. So, I think it can pass the test.

Oh, my mistake. I thought group->domain would point to the blocking
domain when a device is quarantined. It's fine if group->domain remains
set to the previous domain.

> Thanks
> Nicolin

Thanks,
baolu


  reply	other threads:[~2026-04-24  2:32 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-16 23:28 [PATCH v3 00/11] iommu/arm-smmu-v3: Quarantine device upon ATC invalidation timeout Nicolin Chen
2026-04-16 23:28 ` [PATCH v3 01/11] PCI: Propagate FLR return values to callers Nicolin Chen
2026-04-22  6:13   ` Baolu Lu
2026-04-23  4:29     ` Nicolin Chen
2026-04-16 23:28 ` [PATCH v3 02/11] iommu: Pass in reset result to pci_dev_reset_iommu_done() Nicolin Chen
2026-04-24  2:38   ` Baolu Lu
2026-04-24  2:46     ` Nicolin Chen
2026-04-24  2:56       ` Baolu Lu
2026-04-16 23:28 ` [PATCH v3 03/11] iommu: Add reset_device_done callback for hardware fault recovery Nicolin Chen
2026-04-24  2:40   ` Baolu Lu
2026-04-16 23:28 ` [PATCH v3 04/11] iommu: Add __iommu_group_block_device helper Nicolin Chen
2026-04-24  2:40   ` Baolu Lu
2026-04-16 23:28 ` [PATCH v3 05/11] iommu: Change group->devices to RCU-protected list Nicolin Chen
2026-04-24  2:53   ` Baolu Lu
2026-04-24  3:08     ` Nicolin Chen
2026-04-16 23:28 ` [PATCH v3 06/11] iommu: Defer __iommu_group_free_device() to be outside group->mutex Nicolin Chen
2026-04-23  7:55   ` Baolu Lu
2026-04-23 15:47     ` Nicolin Chen
2026-04-24  2:29       ` Baolu Lu [this message]
2026-04-16 23:28 ` [PATCH v3 07/11] iommu: Add iommu_report_device_broken() to quarantine a broken device Nicolin Chen
2026-04-16 23:28 ` [PATCH v3 08/11] iommu/arm-smmu-v3: Mark ATC invalidate timeouts via lockless bitmap Nicolin Chen
2026-04-16 23:28 ` [PATCH v3 09/11] iommu/arm-smmu-v3: Replace smmu with master in arm_smmu_inv Nicolin Chen
2026-04-16 23:28 ` [PATCH v3 10/11] iommu/arm-smmu-v3: Introduce master->ats_broken flag Nicolin Chen
2026-04-16 23:28 ` [PATCH v3 11/11] iommu/arm-smmu-v3: Block ATS upon an ATC invalidation timeout Nicolin Chen

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=667647da-c8e3-483f-aa57-c3730194c8a5@linux.intel.com \
    --to=baolu.lu@linux.intel.com \
    --cc=bhelgaas@google.com \
    --cc=iommu@lists.linux.dev \
    --cc=jgg@nvidia.com \
    --cc=joro@8bytes.org \
    --cc=kevin.tian@intel.com \
    --cc=lenb@kernel.org \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=nicolinc@nvidia.com \
    --cc=praan@google.com \
    --cc=rafael@kernel.org \
    --cc=robin.murphy@arm.com \
    --cc=smostafa@google.com \
    --cc=vsethi@nvidia.com \
    --cc=will@kernel.org \
    --cc=xueshuai@linux.alibaba.com \
    /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