[PATCH v5 02/17] iommu/arm-smmu-v3: Consolidate the STE generation for abort/bypass
Robin Murphy
robin.murphy at arm.com
Thu Feb 15 09:27:09 PST 2024
On 06/02/2024 3:12 pm, Jason Gunthorpe wrote:
> This allows writing the flow of arm_smmu_write_strtab_ent() around abort
> and bypass domains more naturally.
>
> Note that the core code no longer supplies NULL domains, though there is
> still a flow in the driver that end up in arm_smmu_write_strtab_ent() with
> NULL. A later patch will remove it.
>
> Remove the duplicate calculation of the STE in arm_smmu_init_bypass_stes()
> and remove the force parameter. arm_smmu_rmr_install_bypass_ste() can now
> simply invoke arm_smmu_make_bypass_ste() directly.
>
> Reviewed-by: Michael Shavit <mshavit at google.com>
> Reviewed-by: Nicolin Chen <nicolinc at nvidia.com>
> Reviewed-by: Mostafa Saleh <smostafa at google.com>
> Tested-by: Shameer Kolothum <shameerali.kolothum.thodi at huawei.com>
> Tested-by: Nicolin Chen <nicolinc at nvidia.com>
> Tested-by: Moritz Fischer <moritzf at google.com>
> Signed-off-by: Jason Gunthorpe <jgg at nvidia.com>
> ---
> drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 97 ++++++++++++---------
> 1 file changed, 55 insertions(+), 42 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 f0b915567cbcdc..6123e5ad95822c 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> @@ -1498,6 +1498,24 @@ static void arm_smmu_write_ste(struct arm_smmu_master *master, u32 sid,
> }
> }
>
> +static void arm_smmu_make_abort_ste(struct arm_smmu_ste *target)
> +{
> + memset(target, 0, sizeof(*target));
> + target->data[0] = cpu_to_le64(
> + STRTAB_STE_0_V |
> + FIELD_PREP(STRTAB_STE_0_CFG, STRTAB_STE_0_CFG_ABORT));
> +}
> +
> +static void arm_smmu_make_bypass_ste(struct arm_smmu_ste *target)
> +{
> + memset(target, 0, sizeof(*target));
> + target->data[0] = cpu_to_le64(
> + STRTAB_STE_0_V |
> + FIELD_PREP(STRTAB_STE_0_CFG, STRTAB_STE_0_CFG_BYPASS));
> + target->data[1] = cpu_to_le64(
> + FIELD_PREP(STRTAB_STE_1_SHCFG, STRTAB_STE_1_SHCFG_INCOMING));
> +}
> +
> static void arm_smmu_write_strtab_ent(struct arm_smmu_master *master, u32 sid,
> struct arm_smmu_ste *dst)
> {
> @@ -1508,37 +1526,31 @@ static void arm_smmu_write_strtab_ent(struct arm_smmu_master *master, u32 sid,
> struct arm_smmu_domain *smmu_domain = master->domain;
> struct arm_smmu_ste target = {};
>
> - if (smmu_domain) {
> - switch (smmu_domain->stage) {
> - case ARM_SMMU_DOMAIN_S1:
> - cd_table = &master->cd_table;
> - break;
> - case ARM_SMMU_DOMAIN_S2:
> - s2_cfg = &smmu_domain->s2_cfg;
> - break;
> - default:
> - break;
> - }
> + 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:
> + cd_table = &master->cd_table;
> + break;
> + case ARM_SMMU_DOMAIN_S2:
> + s2_cfg = &smmu_domain->s2_cfg;
> + break;
> + case ARM_SMMU_DOMAIN_BYPASS:
> + arm_smmu_make_bypass_ste(&target);
> + arm_smmu_write_ste(master, sid, dst, &target);
> + return;
> }
>
> /* Nuke the existing STE_0 value, as we're going to rewrite it */
> val = STRTAB_STE_0_V;
>
> - /* Bypass/fault */
> - if (!smmu_domain || !(cd_table || s2_cfg)) {
> - if (!smmu_domain && disable_bypass)
> - val |= FIELD_PREP(STRTAB_STE_0_CFG, STRTAB_STE_0_CFG_ABORT);
> - else
> - val |= FIELD_PREP(STRTAB_STE_0_CFG, STRTAB_STE_0_CFG_BYPASS);
> -
> - target.data[0] = cpu_to_le64(val);
> - target.data[1] = cpu_to_le64(FIELD_PREP(STRTAB_STE_1_SHCFG,
> - STRTAB_STE_1_SHCFG_INCOMING));
> - target.data[2] = 0; /* Nuke the VMID */
> - arm_smmu_write_ste(master, sid, dst, &target);
> - return;
> - }
> -
> if (cd_table) {
> u64 strw = smmu->features & ARM_SMMU_FEAT_E2H ?
> STRTAB_STE_1_STRW_EL2 : STRTAB_STE_1_STRW_NSEL1;
> @@ -1583,22 +1595,20 @@ static void arm_smmu_write_strtab_ent(struct arm_smmu_master *master, u32 sid,
> arm_smmu_write_ste(master, sid, dst, &target);
> }
>
> +/*
> + * This can safely directly manipulate the STE memory without a sync sequence
> + * because the STE table has not been installed in the SMMU yet.
> + */
> static void arm_smmu_init_bypass_stes(struct arm_smmu_ste *strtab,
This name is long out-of-date - if we're refreshing this area, please
rename to something relevant to what it actually does, e.g.
s/bypass/initial/.
Although frankly I also think that at this point we should just get rid
of the disable_bypass parameter altogether - it's been almost entirely
meaningless since default domain support was added, and any tenuous
cases for wanting inital STEs to be bypass should probably be using RMRs
now anyway.
Thanks,
Robin.
> - unsigned int nent, bool force)
> + unsigned int nent)
> {
> unsigned int i;
> - u64 val = STRTAB_STE_0_V;
> -
> - if (disable_bypass && !force)
> - val |= FIELD_PREP(STRTAB_STE_0_CFG, STRTAB_STE_0_CFG_ABORT);
> - else
> - val |= FIELD_PREP(STRTAB_STE_0_CFG, STRTAB_STE_0_CFG_BYPASS);
>
> for (i = 0; i < nent; ++i) {
> - strtab->data[0] = cpu_to_le64(val);
> - strtab->data[1] = cpu_to_le64(FIELD_PREP(
> - STRTAB_STE_1_SHCFG, STRTAB_STE_1_SHCFG_INCOMING));
> - strtab->data[2] = 0;
> + if (disable_bypass)
> + arm_smmu_make_abort_ste(strtab);
> + else
> + arm_smmu_make_bypass_ste(strtab);
> strtab++;
> }
> }
> @@ -1626,7 +1636,7 @@ static int arm_smmu_init_l2_strtab(struct arm_smmu_device *smmu, u32 sid)
> return -ENOMEM;
> }
>
> - arm_smmu_init_bypass_stes(desc->l2ptr, 1 << STRTAB_SPLIT, false);
> + arm_smmu_init_bypass_stes(desc->l2ptr, 1 << STRTAB_SPLIT);
> arm_smmu_write_strtab_l1_desc(strtab, desc);
> return 0;
> }
> @@ -3245,7 +3255,7 @@ static int arm_smmu_init_strtab_linear(struct arm_smmu_device *smmu)
> reg |= FIELD_PREP(STRTAB_BASE_CFG_LOG2SIZE, smmu->sid_bits);
> cfg->strtab_base_cfg = reg;
>
> - arm_smmu_init_bypass_stes(strtab, cfg->num_l1_ents, false);
> + arm_smmu_init_bypass_stes(strtab, cfg->num_l1_ents);
> return 0;
> }
>
> @@ -3956,7 +3966,6 @@ static void arm_smmu_rmr_install_bypass_ste(struct arm_smmu_device *smmu)
> iort_get_rmr_sids(dev_fwnode(smmu->dev), &rmr_list);
>
> list_for_each_entry(e, &rmr_list, list) {
> - struct arm_smmu_ste *step;
> struct iommu_iort_rmr_data *rmr;
> int ret, i;
>
> @@ -3969,8 +3978,12 @@ static void arm_smmu_rmr_install_bypass_ste(struct arm_smmu_device *smmu)
> continue;
> }
>
> - step = arm_smmu_get_step_for_sid(smmu, rmr->sids[i]);
> - arm_smmu_init_bypass_stes(step, 1, true);
> + /*
> + * STE table is not programmed to HW, see
> + * arm_smmu_init_bypass_stes()
> + */
> + arm_smmu_make_bypass_ste(
> + arm_smmu_get_step_for_sid(smmu, rmr->sids[i]));
> }
> }
>
More information about the linux-arm-kernel
mailing list