All of lore.kernel.org
 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:50 UTC|newest]

Thread overview: 30+ 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-09-08 15:51       ` Jason Gunthorpe
2025-09-08 18:20         ` 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-09-08 15:36       ` Jason Gunthorpe
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-09-08 15:39       ` Jason Gunthorpe
2025-09-08 18:19         ` Nicolin Chen
2025-09-08 18:24           ` Jason Gunthorpe
2025-09-08 18:45             ` 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.