Re: [PATCH rfcv1 5/8] iommu/arm-smmu-v3: Pre-allocate a per-master invalidation array

Nicolin Chen
2025-09-06T07:47:02Z

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