| Message ID | 7-v4-c93b774edcc4+42d2b-smmuv3_newapi_p1_jgg@nvidia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Update SMMUv3 to the modern iommu API (part 1/3) | expand |
Hi Jason, On Thu, Jan 25, 2024 at 07:57:17PM -0400, Jason Gunthorpe 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(). > > Reviewed-by: Michael Shavit <mshavit@google.com> > Reviewed-by: Nicolin Chen <nicolinc@nvidia.com> > Tested-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com> > Tested-by: Nicolin Chen <nicolinc@nvidia.com> > Tested-by: Moritz Fischer <moritzf@google.com> > Signed-off-by: Jason Gunthorpe <jgg@nvidia.com> > --- > drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 57 ++++++++------------- > 1 file changed, 22 insertions(+), 35 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 539ef380f457fa..cf3e348cb9abe1 100644 > --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c > +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c > @@ -1588,35 +1588,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_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(master, 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(master, sid, dst, &target); > -} > - > static void arm_smmu_init_bypass_stes(struct arm_smmu_ste *strtab, > unsigned int nent) > { > @@ -2439,7 +2410,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; > @@ -2456,7 +2428,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(master, sid, step, target); > } > } > > @@ -2563,6 +2535,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) > @@ -2576,7 +2549,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 > @@ -2591,6 +2568,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); > @@ -2652,7 +2630,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) { > @@ -2666,9 +2645,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.43.0 > Reviewed-by: Mostafa Saleh <smostafa@google.com> Thanks, Mostafa
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 539ef380f457fa..cf3e348cb9abe1 100644 --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c @@ -1588,35 +1588,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_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(master, 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(master, sid, dst, &target); -} - static void arm_smmu_init_bypass_stes(struct arm_smmu_ste *strtab, unsigned int nent) { @@ -2439,7 +2410,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; @@ -2456,7 +2428,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(master, sid, step, target); } } @@ -2563,6 +2535,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) @@ -2576,7 +2549,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 @@ -2591,6 +2568,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); @@ -2652,7 +2630,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) { @@ -2666,9 +2645,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;