[PATCH v5 19/27] iommu/arm-smmu-v3: Keep track of arm_smmu_master_domain for SVA

[Michael Shavit]

On Tue, Mar 5, 2024 at 7:44 AM Jason Gunthorpe <jgg at nvidia.com> wrote:
>
> Currently the smmu_domain->devices list is unused for SVA domains.
> Fill it in with the SSID and master of every arm_smmu_set_pasid()
> using the same logic as the RID attach.
>
> Tested-by: Nicolin Chen <nicolinc at nvidia.com>
> Signed-off-by: Jason Gunthorpe <jgg at nvidia.com>
> ---
>  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 23 +++++++++++++++++++--
>  1 file changed, 21 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> index 2db2b822292a87..6d15fe3a160acf 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> @@ -2603,7 +2603,8 @@ to_smmu_domain_devices(struct iommu_domain *domain)
>         /* The domain can be NULL only when processing the first attach */
>         if (!domain)
>                 return NULL;
> -       if (domain->type & __IOMMU_DOMAIN_PAGING)
> +       if ((domain->type & __IOMMU_DOMAIN_PAGING) ||
> +           domain->type == IOMMU_DOMAIN_SVA)
>                 return to_smmu_domain(domain);
>         return NULL;
>  }
> @@ -2813,7 +2814,9 @@ int arm_smmu_set_pasid(struct arm_smmu_master *master,
>                        struct arm_smmu_domain *smmu_domain, ioasid_t pasid,
>                        const struct arm_smmu_cd *cd)
>  {
> +       struct attach_state state = {.ssid = pasid};
>         struct arm_smmu_cd *cdptr;
> +       int ret;
>
>         if (!arm_smmu_is_s1_domain(iommu_get_domain_for_dev(master->dev)))
>                 return -ENODEV;
> @@ -2821,14 +2824,30 @@ int arm_smmu_set_pasid(struct arm_smmu_master *master,
>         cdptr = arm_smmu_get_cd_ptr(master, pasid);
>         if (!cdptr)
>                 return -ENOMEM;
> +
> +       mutex_lock(&arm_smmu_asid_lock);
> +       ret = arm_smmu_attach_prepare(master, &smmu_domain->domain, &state);
> +       if (ret)
> +               goto out_unlock;
> +
>         arm_smmu_write_cd_entry(master, pasid, cdptr, cd);
> -       return 0;
> +
> +       arm_smmu_attach_commit(master, &state);
> +
> +out_unlock:
> +       mutex_unlock(&arm_smmu_asid_lock);
> +       return ret;
>  }

arm_smmu_attach_commit tries to remove the master_domain entry from
the previous domain that this master was attached to. It gets a
pointer to the previous domain from the iommu framework with
iommu_get_domain_for_dev().
But in this path, arm_smmu_attach_prepare is creating a master_domain
entry for the pasid domain, which may be different from the one
returned by iommu_get_domain_for_dev() on the next attach.

I think this ended up being safe in the end because afaict the iommu
framework requires detaching the previous pasid domain before
attaching a new one. But nonetheless this is pretty fragile and
doesn't look intentional.


>
>  void arm_smmu_remove_pasid(struct arm_smmu_master *master,
>                            struct arm_smmu_domain *smmu_domain, ioasid_t pasid)
>  {
> +       mutex_lock(&arm_smmu_asid_lock);
>         arm_smmu_clear_cd(master, pasid);
> +       if (master->ats_enabled)
> +               arm_smmu_atc_inv_master(master, pasid);
> +       arm_smmu_remove_master_domain(master, &smmu_domain->domain, pasid);
> +       mutex_unlock(&arm_smmu_asid_lock);
>  }
>
>  static int arm_smmu_attach_dev_ste(struct iommu_domain *domain,
> --
> 2.43.2
>
>