patches.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
From: Nicolin Chen <nicolinc@nvidia.com>
To: Jason Gunthorpe <jgg@nvidia.com>
Cc: <will@kernel.org>, <robin.murphy@arm.com>, <joro@8bytes.org>,
	<jean-philippe@linaro.org>, <miko.lenczewski@arm.com>,
	<balbirs@nvidia.com>, <peterz@infradead.org>,
	<smostafa@google.com>, <kevin.tian@intel.com>, <praan@google.com>,
	<zhangzekun11@huawei.com>, <linux-arm-kernel@lists.infradead.org>,
	<iommu@lists.linux.dev>, <linux-kernel@vger.kernel.org>,
	<patches@lists.linux.dev>
Subject: Re: [PATCH rfcv1 5/8] iommu/arm-smmu-v3: Pre-allocate a per-master invalidation array
Date: Sat, 6 Sep 2025 00:45:58 -0700	[thread overview]
Message-ID: <aLvmgXW8BpAyXMVV@nvidia.com> (raw)
In-Reply-To: <20250826195641.GB2151485@nvidia.com>

On Tue, Aug 26, 2025 at 04:56:41PM -0300, Jason Gunthorpe wrote:
> On Wed, Aug 13, 2025 at 06:25:36PM -0700, Nicolin Chen wrote:
> > @@ -3730,6 +3731,13 @@ static int arm_smmu_insert_master(struct arm_smmu_device *smmu,
> >  		return -ENOMEM;
> >  	master->num_streams = fwspec->num_ids;
> >  
> > +	/* Max possible num_invs: two for ASID/VMIDs and num_ats for ATC_INVs */
> > +	master->invs = arm_smmu_invs_alloc(2 + num_ats);
> > +	if (IS_ERR(master->invs)) {
> > +		kfree(master->streams);
> > +		return PTR_ERR(master->invs);
> > +	}
> 
> This seems like a nice solution, but I would add a comment here that
> it is locked by the group mutex, and check if ATS is supported:
> 
> 	/*
> 	 * Scratch memory to build the per-domain invalidation list. locked by
> 	 * the group_mutex. Max possible num_invs: two for ASID/VMIDs and
> 	 * num_streams for ATC_INVs
> 	 */
> 	if (dev_is_pci(master->dev) &&
> 	    pci_ats_supported(to_pci_dev(master->dev)))
> 		master->invs = arm_smmu_invs_alloc(2 + master->num_streams);
> 	else
> 		master->invs = arm_smmu_invs_alloc(2);
> 
> And probably rename it scratch_invs or something to indicate it is
> temporary memory.

I renamed it to master->build_invs:

@@ -919,7 +931,14 @@ struct arm_smmu_master {
 	struct arm_smmu_device		*smmu;
 	struct device			*dev;
 	struct arm_smmu_stream		*streams;
-	struct arm_smmu_invs		*invs;
+	/*
+	 * Scratch memory for a to_merge or to_unref array to build a per-domain
+	 * invalidation array. It'll be pre-allocated with enough enries for all
+	 * possible build scenarios. It can be used by only one caller at a time
+	 * until the arm_smmu_invs_merge/unref() finishes. Must be locked by the
+	 * iommu_group mutex.
+	 */
+	struct arm_smmu_invs		*build_invs;
 	struct arm_smmu_vmaster		*vmaster; /* use smmu->streams_mutex */
 	/* Locked by the iommu core using the group mutex */
 	struct arm_smmu_ctx_desc_cfg	cd_table;

One thing that I noticed is that group mutex alone isn't enough,
because there can be two arm_smmu_build_invs() calls during the
same attach_dev callback. And the second one would overwrite.

Thanks
Nicolin

  reply	other threads:[~2025-09-06  7:47 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-08-14  1:25 [PATCH rfcv1 0/8] iommu/arm-smmu-v3: Introduce an RCU-protected invalidation array Nicolin Chen
2025-08-14  1:25 ` [PATCH rfcv1 1/8] iommu/arm-smmu-v3: Clear cmds->num after arm_smmu_cmdq_batch_submit Nicolin Chen
2025-08-14  1:25 ` [PATCH rfcv1 2/8] iommu/arm-smmu-v3: Explicitly set smmu_domain->stage for SVA Nicolin Chen
2025-08-14  1:25 ` [PATCH rfcv1 3/8] iommu/arm-smmu-v3: Add an inline arm_smmu_domain_free() Nicolin Chen
2025-08-14  1:25 ` [PATCH rfcv1 4/8] iommu/arm-smmu-v3: Introduce a per-domain arm_smmu_invs array Nicolin Chen
2025-08-26 19:50   ` Jason Gunthorpe
2025-08-27  0:49     ` Nicolin Chen
2025-08-27 16:48   ` Jason Gunthorpe
2025-08-27 17:19     ` Nicolin Chen
2025-08-28 12:37       ` Jason Gunthorpe
2025-08-27 20:00   ` Jason Gunthorpe
2025-09-06  8:16     ` Nicolin Chen
2025-08-14  1:25 ` [PATCH rfcv1 5/8] iommu/arm-smmu-v3: Pre-allocate a per-master invalidation array Nicolin Chen
2025-08-26 19:56   ` Jason Gunthorpe
2025-09-06  7:45     ` Nicolin Chen [this message]
2025-08-14  1:25 ` [PATCH rfcv1 6/8] iommu/arm-smmu-v3: Populate smmu_domain->invs when attaching masters Nicolin Chen
2025-08-27 18:21   ` Jason Gunthorpe
2025-09-06  7:52     ` Nicolin Chen
2025-09-06  8:20     ` Nicolin Chen
2025-08-14  1:25 ` [PATCH rfcv1 7/8] iommu/arm-smmu-v3: Add arm_smmu_invs based arm_smmu_domain_inv_range() Nicolin Chen
2025-08-27 18:49   ` Jason Gunthorpe
2025-09-06  8:12     ` Nicolin Chen
2025-08-14  1:25 ` [PATCH rfcv1 8/8] iommu/arm-smmu-v3: Perform per-domain invalidations using arm_smmu_invs 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=aLvmgXW8BpAyXMVV@nvidia.com \
    --to=nicolinc@nvidia.com \
    --cc=balbirs@nvidia.com \
    --cc=iommu@lists.linux.dev \
    --cc=jean-philippe@linaro.org \
    --cc=jgg@nvidia.com \
    --cc=joro@8bytes.org \
    --cc=kevin.tian@intel.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=miko.lenczewski@arm.com \
    --cc=patches@lists.linux.dev \
    --cc=peterz@infradead.org \
    --cc=praan@google.com \
    --cc=robin.murphy@arm.com \
    --cc=smostafa@google.com \
    --cc=will@kernel.org \
    --cc=zhangzekun11@huawei.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;
as well as URLs for NNTP newsgroup(s).