[PATCH v2 10/19] iommu/arm-smmu-v3: Compute the STE only once for each master

[Michael Shavit]

On Tue, Nov 14, 2023 at 1:53 AM Jason Gunthorpe <jgg at nvidia.com> wrote:
>
> Currently arm_smmu_install_ste_for_dev() iterates over every SID and
> computes from scratch an identical STE. Every SID should have the same STE
> contents. Turn this inside out so that the STE is supplied by the caller
> and arm_smmu_install_ste_for_dev() simply installs it to every SID.
>
> This is possible now that the STE generation does not inform what sequence
> should be used to program it.
>
> This allows splitting the STE calculation up according to the call site,
> which following patches will make use of, and removes the confusing NULL
> domain special case that only supported arm_smmu_detach_dev().
>
> Signed-off-by: Jason Gunthorpe <jgg at nvidia.com>
Reviewed-by: Michael Shavit <mshavit at google.com>
> ---
>  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 58 ++++++++-------------
>  1 file changed, 22 insertions(+), 36 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 b11dc03ee16880..4b157c2ddf9a80 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> @@ -1536,36 +1536,6 @@ static void arm_smmu_make_s2_domain_ste(struct arm_smmu_ste *target,
>                                       STRTAB_STE_3_S2TTB_MASK);
>  }
>
> -static void arm_smmu_write_strtab_ent(struct arm_smmu_master *master, u32 sid,
> -                                     struct arm_smmu_ste *dst)
> -{
> -       struct arm_smmu_device *smmu = master->smmu;
> -       struct arm_smmu_domain *smmu_domain = master->domain;
> -       struct arm_smmu_ste target = {};
> -
> -       if (!smmu_domain) {
> -               if (disable_bypass)
> -                       arm_smmu_make_abort_ste(&target);
> -               else
> -                       arm_smmu_make_bypass_ste(&target);
> -               arm_smmu_write_ste(smmu, sid, dst, &target);
> -               return;
> -       }
> -
> -       switch (smmu_domain->stage) {
> -       case ARM_SMMU_DOMAIN_S1:
> -               arm_smmu_make_cdtable_ste(&target, master, &master->cd_table);
> -               break;
> -       case ARM_SMMU_DOMAIN_S2:
> -               arm_smmu_make_s2_domain_ste(&target, master, smmu_domain);
> -               break;
> -       case ARM_SMMU_DOMAIN_BYPASS:
> -               arm_smmu_make_bypass_ste(&target);
> -               break;
> -       }
> -       arm_smmu_write_ste(smmu, sid, dst, &target);
> -}
> -
>  static void arm_smmu_init_bypass_stes(struct arm_smmu_ste *strtab,
>                                       unsigned int nent)
>  {
> @@ -2387,7 +2357,8 @@ arm_smmu_get_step_for_sid(struct arm_smmu_device *smmu, u32 sid)
>         }
>  }
>
> -static void arm_smmu_install_ste_for_dev(struct arm_smmu_master *master)
> +static void arm_smmu_install_ste_for_dev(struct arm_smmu_master *master,
> +                                        const struct arm_smmu_ste *target)
>  {
>         int i, j;
>         struct arm_smmu_device *smmu = master->smmu;
> @@ -2404,7 +2375,7 @@ static void arm_smmu_install_ste_for_dev(struct arm_smmu_master *master)
>                 if (j < i)
>                         continue;
>
> -               arm_smmu_write_strtab_ent(master, sid, step);
> +               arm_smmu_write_ste(smmu, sid, step, target);
>         }
>  }
>
> @@ -2511,6 +2482,7 @@ static void arm_smmu_disable_pasid(struct arm_smmu_master *master)
>  static void arm_smmu_detach_dev(struct arm_smmu_master *master)
>  {
>         unsigned long flags;
> +       struct arm_smmu_ste target;
>         struct arm_smmu_domain *smmu_domain = master->domain;
>
>         if (!smmu_domain)
> @@ -2524,7 +2496,11 @@ static void arm_smmu_detach_dev(struct arm_smmu_master *master)
>
>         master->domain = NULL;
>         master->ats_enabled = false;
> -       arm_smmu_install_ste_for_dev(master);
> +       if (disable_bypass)
> +               arm_smmu_make_abort_ste(&target);
> +       else
> +               arm_smmu_make_bypass_ste(&target);
> +       arm_smmu_install_ste_for_dev(master, &target);
>         /*
>          * Clearing the CD entry isn't strictly required to detach the domain
>          * since the table is uninstalled anyway, but it helps avoid confusion
> @@ -2539,6 +2515,7 @@ static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev)
>  {
>         int ret = 0;
>         unsigned long flags;
> +       struct arm_smmu_ste target;
>         struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
>         struct arm_smmu_device *smmu;
>         struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
> @@ -2600,7 +2577,8 @@ static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev)
>         list_add(&master->domain_head, &smmu_domain->devices);
>         spin_unlock_irqrestore(&smmu_domain->devices_lock, flags);
>
> -       if (smmu_domain->stage == ARM_SMMU_DOMAIN_S1) {
> +       switch (smmu_domain->stage) {
> +       case ARM_SMMU_DOMAIN_S1:
>                 if (!master->cd_table.cdtab) {
>                         ret = arm_smmu_alloc_cd_tables(master);
>                         if (ret) {
> @@ -2614,9 +2592,17 @@ static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev)
>                         master->domain = NULL;
>                         goto out_list_del;
>                 }
> -       }
>
> -       arm_smmu_install_ste_for_dev(master);
> +               arm_smmu_make_cdtable_ste(&target, master, &master->cd_table);
> +               break;
> +       case ARM_SMMU_DOMAIN_S2:
> +               arm_smmu_make_s2_domain_ste(&target, master, smmu_domain);
> +               break;
> +       case ARM_SMMU_DOMAIN_BYPASS:
> +               arm_smmu_make_bypass_ste(&target);
> +               break;
> +       }
> +       arm_smmu_install_ste_for_dev(master, &target);
>
>         arm_smmu_enable_ats(master);
>         goto out_unlock;
> --
> 2.42.0
>