Re: [PATCH v4 07/13] iommu/arm-smmu-v3: Keep track of attached ssids

From: Nicolin Chen <nicolinc@nvidia.com>
To: Michael Shavit <mshavit@google.com>
Cc: Will Deacon <will@kernel.org>,
	Robin Murphy <robin.murphy@arm.com>,
	"Joerg Roedel" <joro@8bytes.org>, <jean-philippe@linaro.org>,
	<jgg@nvidia.com>, <baolu.lu@linux.intel.com>,
	<linux-arm-kernel@lists.infradead.org>, <iommu@lists.linux.dev>,
	<linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v4 07/13] iommu/arm-smmu-v3: Keep track of attached ssids
Date: Wed, 12 Jul 2023 21:45:26 -0700	[thread overview]
Message-ID: <ZK+BZhwbo8JLMPI2@Asurada-Nvidia> (raw)
In-Reply-To: <20230621063825.268890-8-mshavit@google.com>

On Wed, Jun 21, 2023 at 02:37:19PM +0800, Michael Shavit wrote:

> -int arm_smmu_atc_inv_domain(struct arm_smmu_domain *smmu_domain, int ssid,
> -                           unsigned long iova, size_t size)
> +/*
> + * If ssid is non-zero, issue atc invalidations with the given ssid instead of
> + * the one the domain is attached to. This is used by SVA since it's pasid
> + * attachments aren't recorded in smmu_domain yet.
> + */
> +int arm_smmu_atc_inv_domain_ssid(struct arm_smmu_domain *smmu_domain, int ssid,
> +                                unsigned long iova, size_t size)
[..]
> @@ -1823,25 +1835,37 @@ int arm_smmu_atc_inv_domain(struct arm_smmu_domain *smmu_domain, int ssid,
>         if (!atomic_read(&smmu_domain->nr_ats_masters))
>                 return 0;
> 
> -       arm_smmu_atc_inv_to_cmd(ssid, iova, size, &cmd);
> +       arm_smmu_atc_inv_to_cmd(iova, size, &cmd);
> 
>         cmds.num = 0;
> 
> -       spin_lock_irqsave(&smmu_domain->devices_lock, flags);
> -       list_for_each_entry(master, &smmu_domain->devices, domain_head) {
> +       spin_lock_irqsave(&smmu_domain->attached_domains_lock, flags);
> +       list_for_each_entry(attached_domain, &smmu_domain->attached_domains,
> +                           domain_head) {
> +               master = attached_domain->master;
>                 if (!master->ats_enabled)
>                         continue;
> +               if (ssid != 0)
> +                       arm_smmu_atc_inv_cmd_set_ssid(ssid, &cmd);
> +               else
> +                       arm_smmu_atc_inv_cmd_set_ssid(attached_domain->ssid, &cmd);
> 
>                 for (i = 0; i < master->num_streams; i++) {
>                         cmd.atc.sid = master->streams[i].id;
>                         arm_smmu_cmdq_batch_add(smmu_domain->smmu, &cmds, &cmd);
>                 }
>         }

And I don't quite get this part. Prior to this change, it issues
one ATC_INV command covering all ATC entries per comments inside
arm_smmu_atc_inv_to_cmd(). But now we replace that single command
with all attached subdomains in the list? Any reason for such a
change here?

Thanks
Nicolin