[PATCH v5 00/17] Update SMMUv3 to the modern iommu API (part 1/3)

Jason Gunthorpe
2024-02-06T15:14:21Z

The SMMUv3 driver was originally written in 2015 when the iommu driver
facing API looked quite different. The API has evolved, especially lately,
and the driver has fallen behind.

This work aims to bring make the SMMUv3 driver the best IOMMU driver with
the most comprehensive implementation of the API. After all parts it
addresses:

- Global static BLOCKED and IDENTITY domains with 'never fail' attach
semantics. BLOCKED is desired for efficient VFIO.

- Support map before attach for PAGING iommu_domains.

- attach_dev failure does not change the HW configuration.

- Fully hitless transitions between IDENTITY -> DMA -> IDENTITY.
The API has IOMMU_RESV_DIRECT which is expected to be
continuously translating.

- Safe transitions between PAGING -> BLOCKED, do not ever temporarily
do IDENTITY. This is required for iommufd security.

- Full PASID API support including:
- S1/SVA domains attached to PASIDs
- IDENTITY/BLOCKED/S1 attached to RID
- Change of the RID domain while PASIDs are attached

- Streamlined SVA support using the core infrastructure

- Hitless, whenever possible, change between two domains

- iommufd IOMMU_GET_HW_INFO, IOMMU_HWPT_ALLOC_NEST_PARENT, and
IOMMU_DOMAIN_NESTED support

Over all these things are going to become more accessible to iommufd, and
exposed to VMs, so it is important for the driver to have a robust
implementation of the API.

The work is split into three parts, with this part largely focusing on the
STE and building up to the BLOCKED & IDENTITY global static domains.

The second part largely focuses on the CD and builds up to having a common
PASID infrastructure that SVA and S1 domains equally use.

The third part has some random cleanups and the iommufd related parts.

Overall this takes the approach of turning the STE/CD programming upside
down where the CD/STE value is computed right at a driver callback
function and then pushed down into programming logic. The programming
logic hides the details of the required CD/STE tear-less update. This
makes the CD/STE functions independent of the arm_smmu_domain which makes
it fairly straightforward to untangle all the different call chains, and
add news ones.

Further, this frees the arm_smmu_domain related logic from keeping track
of what state the STE/CD is currently in so it can carefully sequence the
correct update. There are many new update pairs that are subtly introduced
as the work progresses.

The locking to support BTM via arm_smmu_asid_lock is a bit subtle right
now and patches throughout this work adjust and tighten this so that it is
clearer and doesn't get broken.

Once the lower STE layers no longer need to touch arm_smmu_domain we can
isolate struct arm_smmu_domain to be only used for PAGING domains, audit
all the to_smmu_domain() calls to be only in PAGING domain ops, and
introduce the normal global static BLOCKED/IDENTITY domains using the new
STE infrastructure. Part 2 will ultimately migrate SVA over to use
arm_smmu_domain as well.

All parts are on github:

https://github.com/jgunthorpe/linux/commits/smmuv3_newapi

v5:
- Rebase on v6.8-rc3
- Remove the writer argument to arm_smmu_entry_writer_ops get_used()
- Swap order of hweight tests so one call to hweight8() can be removed
- Add STRTAB_STE_2_S2VMID used for STRTAB_STE_0_CFG_S1_TRANS, for
S2 bypass the VMID is used but 0
- Be more exact when generating STEs and store 0's to document the HW
is using that value and 0 is actually a deliberate choice for VMID and
SHCFG.
- Remove cd_table argument to arm_smmu_make_cdtable_ste()
- Put arm_smmu_rmr_install_bypass_ste() after setting up a 2 level table
- Pull patch "Check that the RID domain is S1 in SVA" from part 2 to
guard against memory corruption on failure paths
- Tighten the used logic for SHCFG to accommodate nesting patches in
part 3
- Additional comments and commit message adjustments
v4: https://lore.kernel.org/r/0-v4-c93b774edcc4+42d2b-smmuv3_newapi_p1_jgg@nvidia.com
- Rebase on v6.8-rc1. Patches 1-3 merged
- Replace patch "Make STE programming independent of the callers" with
Michael's version
* Describe the core API desire for hitless updates
* Replace the iterator with STE/CD specific function pointers.
This lets the logic be written top down instead of rolled into an
iterator
* Optimize away a sync when the critical qword is the only qword
to update
- Pass master not smmu to arm_smmu_write_ste() throughout
- arm_smmu_make_s2_domain_ste() should use data[1] = not |= since
it is known to be zero
- Return errno's from domain_alloc() paths
v3: https://lore.kernel.org/r/0-v3-d794f8d934da+411a-smmuv3_newapi_p1_jgg@nvidia.com
- Use some local variables in arm_smmu_get_step_for_sid() for clarity
- White space and spelling changes
- Commit message updates
- Keep master->domain_head initialized to avoid a list_del corruption
v2: https://lore.kernel.org/r/0-v2-de8b10590bf5+400-smmuv3_newapi_p1_jgg@nvidia.com
- Rebased on v6.7-rc1
- Improve the comment for arm_smmu_write_entry_step()
- Fix the botched memcmp
- Document the spec justification for the SHCFG exclusion in used
- Include STRTAB_STE_1_SHCFG for STRTAB_STE_0_CFG_S2_TRANS in used
- WARN_ON for unknown STEs in used
- Fix error unwind in arm_smmu_attach_dev()
- Whitespace, spelling, and checkpatch related items
v1: https://lore.kernel.org/r/0-v1-e289ca9121be+2be-smmuv3_newapi_p1_jgg@nvidia.com

Jason Gunthorpe (17):
iommu/arm-smmu-v3: Make STE programming independent of the callers
iommu/arm-smmu-v3: Consolidate the STE generation for abort/bypass
iommu/arm-smmu-v3: Move arm_smmu_rmr_install_bypass_ste()
iommu/arm-smmu-v3: Move the STE generation for S1 and S2 domains into
functions
iommu/arm-smmu-v3: Build the whole STE in
arm_smmu_make_s2_domain_ste()
iommu/arm-smmu-v3: Hold arm_smmu_asid_lock during all of attach_dev
iommu/arm-smmu-v3: Compute the STE only once for each master
iommu/arm-smmu-v3: Do not change the STE twice during
arm_smmu_attach_dev()
iommu/arm-smmu-v3: Put writing the context descriptor in the right
order
iommu/arm-smmu-v3: Pass smmu_domain to arm_enable/disable_ats()
iommu/arm-smmu-v3: Remove arm_smmu_master->domain
iommu/arm-smmu-v3: Check that the RID domain is S1 in SVA
iommu/arm-smmu-v3: Add a global static IDENTITY domain
iommu/arm-smmu-v3: Add a global static BLOCKED domain
iommu/arm-smmu-v3: Use the identity/blocked domain during release
iommu/arm-smmu-v3: Pass arm_smmu_domain and arm_smmu_device to
finalize
iommu/arm-smmu-v3: Convert to domain_alloc_paging()

.../iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c | 8 +-
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 778 ++++++++++++------
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 5 +-
3 files changed, 549 insertions(+), 242 deletions(-)

The diff against v4 is small:

--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -57,8 +57,7 @@ struct arm_smmu_entry_writer {
struct arm_smmu_entry_writer_ops {
unsigned int num_entry_qwords;
__le64 v_bit;
- void (*get_used)(struct arm_smmu_entry_writer *writer, const __le64 *entry,
- __le64 *used);
+ void (*get_used)(const __le64 *entry, __le64 *used);
void (*sync)(struct arm_smmu_entry_writer *writer);
};

@@ -1006,8 +1005,8 @@ static u8 arm_smmu_entry_qword_diff(struct arm_smmu_entry_writer *writer,
u8 used_qword_diff = 0;
unsigned int i;

- writer->ops->get_used(writer, entry, cur_used);
- writer->ops->get_used(writer, target, target_used);
+ writer->ops->get_used(entry, cur_used);
+ writer->ops->get_used(target, target_used);

for (i = 0; i != writer->ops->num_entry_qwords; i++) {
/*
@@ -1084,17 +1083,7 @@ static void arm_smmu_write_entry(struct arm_smmu_entry_writer *writer,

used_qword_diff =
arm_smmu_entry_qword_diff(writer, entry, target, unused_update);
- if (hweight8(used_qword_diff) > 1) {
- /*
- * At least two qwords need their inuse bits to be changed. This
- * requires a breaking update, zero the V bit, write all qwords
- * but 0, then set qword 0
- */
- unused_update[0] = entry[0] & (~writer->ops->v_bit);
- entry_set(writer, entry, unused_update, 0, 1);
- entry_set(writer, entry, target, 1, num_entry_qwords - 1);
- entry_set(writer, entry, target, 0, 1);
- } else if (hweight8(used_qword_diff) == 1) {
+ if (hweight8(used_qword_diff) == 1) {
/*
* Only one qword needs its used bits to be changed. This is a
* hitless update, update all bits the current STE is ignoring
@@ -1114,6 +1103,16 @@ static void arm_smmu_write_entry(struct arm_smmu_entry_writer *writer,
entry_set(writer, entry, unused_update, 0, num_entry_qwords);
entry_set(writer, entry, target, critical_qword_index, 1);
entry_set(writer, entry, target, 0, num_entry_qwords);
+ } else if (used_qword_diff) {
+ /*
+ * At least two qwords need their inuse bits to be changed. This
+ * requires a breaking update, zero the V bit, write all qwords
+ * but 0, then set qword 0
+ */
+ unused_update[0] = entry[0] & (~writer->ops->v_bit);
+ entry_set(writer, entry, unused_update, 0, 1);
+ entry_set(writer, entry, target, 1, num_entry_qwords - 1);
+ entry_set(writer, entry, target, 0, 1);
} else {
/*
* No inuse bit changed. Sanity check that all unused bits are 0
@@ -1402,28 +1401,30 @@ struct arm_smmu_ste_writer {
* would be nice if this was complete according to the spec, but minimally it
* has to capture the bits this driver uses.
*/
-static void arm_smmu_get_ste_used(struct arm_smmu_entry_writer *writer,
- const __le64 *ent, __le64 *used_bits)
+static void arm_smmu_get_ste_used(const __le64 *ent, __le64 *used_bits)
{
+ unsigned int cfg = FIELD_GET(STRTAB_STE_0_CFG, le64_to_cpu(ent[0]));
+
used_bits[0] = cpu_to_le64(STRTAB_STE_0_V);
if (!(ent[0] & cpu_to_le64(STRTAB_STE_0_V)))
return;

/*
- * If S1 is enabled S1DSS is valid, see 13.5 Summary of
- * attribute/permission configuration fields for the SHCFG behavior.
+ * See 13.5 Summary of attribute/permission configuration fields for the
+ * SHCFG behavior. It is only used for BYPASS, including S1DSS BYPASS,
+ * and S2 only.
*/
- if (FIELD_GET(STRTAB_STE_0_CFG, le64_to_cpu(ent[0])) & 1 &&
- FIELD_GET(STRTAB_STE_1_S1DSS, le64_to_cpu(ent[1])) ==
- STRTAB_STE_1_S1DSS_BYPASS)
+ if (cfg == STRTAB_STE_0_CFG_BYPASS ||
+ cfg == STRTAB_STE_0_CFG_S2_TRANS ||
+ (cfg == STRTAB_STE_0_CFG_S1_TRANS &&
+ FIELD_GET(STRTAB_STE_1_S1DSS, le64_to_cpu(ent[1])) ==
+ STRTAB_STE_1_S1DSS_BYPASS))
used_bits[1] |= cpu_to_le64(STRTAB_STE_1_SHCFG);

used_bits[0] |= cpu_to_le64(STRTAB_STE_0_CFG);
- switch (FIELD_GET(STRTAB_STE_0_CFG, le64_to_cpu(ent[0]))) {
+ switch (cfg) {
case STRTAB_STE_0_CFG_ABORT:
- break;
case STRTAB_STE_0_CFG_BYPASS:
- used_bits[1] |= cpu_to_le64(STRTAB_STE_1_SHCFG);
break;
case STRTAB_STE_0_CFG_S1_TRANS:
used_bits[0] |= cpu_to_le64(STRTAB_STE_0_S1FMT |
@@ -1434,10 +1435,11 @@ static void arm_smmu_get_ste_used(struct arm_smmu_entry_writer *writer,
STRTAB_STE_1_S1COR | STRTAB_STE_1_S1CSH |
STRTAB_STE_1_S1STALLD | STRTAB_STE_1_STRW);
used_bits[1] |= cpu_to_le64(STRTAB_STE_1_EATS);
+ used_bits[2] |= cpu_to_le64(STRTAB_STE_2_S2VMID);
break;
case STRTAB_STE_0_CFG_S2_TRANS:
used_bits[1] |=
- cpu_to_le64(STRTAB_STE_1_EATS | STRTAB_STE_1_SHCFG);
+ cpu_to_le64(STRTAB_STE_1_EATS);
used_bits[2] |=
cpu_to_le64(STRTAB_STE_2_S2VMID | STRTAB_STE_2_VTCR |
STRTAB_STE_2_S2AA64 | STRTAB_STE_2_S2ENDI |
@@ -1519,9 +1521,9 @@ static void arm_smmu_make_bypass_ste(struct arm_smmu_ste *target)
}

static void arm_smmu_make_cdtable_ste(struct arm_smmu_ste *target,
- struct arm_smmu_master *master,
- struct arm_smmu_ctx_desc_cfg *cd_table)
+ struct arm_smmu_master *master)
{
+ struct arm_smmu_ctx_desc_cfg *cd_table = &master->cd_table;
struct arm_smmu_device *smmu = master->smmu;

memset(target, 0, sizeof(*target));
@@ -1542,11 +1544,30 @@ static void arm_smmu_make_cdtable_ste(struct arm_smmu_ste *target,
STRTAB_STE_1_S1STALLD :
0) |
FIELD_PREP(STRTAB_STE_1_EATS,
- master->ats_enabled ? STRTAB_STE_1_EATS_TRANS : 0) |
- FIELD_PREP(STRTAB_STE_1_STRW,
- (smmu->features & ARM_SMMU_FEAT_E2H) ?
- STRTAB_STE_1_STRW_EL2 :
- STRTAB_STE_1_STRW_NSEL1));
+ master->ats_enabled ? STRTAB_STE_1_EATS_TRANS : 0));
+
+ if (smmu->features & ARM_SMMU_FEAT_E2H) {
+ /*
+ * To support BTM the streamworld needs to match the
+ * configuration of the CPU so that the ASID broadcasts are
+ * properly matched. This means either S/NS-EL2-E2H (hypervisor)
+ * or NS-EL1 (guest). Since an SVA domain can be installed in a
+ * PASID this should always use a BTM compatible configuration
+ * if the HW supports it.
+ */
+ target->data[1] |= cpu_to_le64(
+ FIELD_PREP(STRTAB_STE_1_STRW, STRTAB_STE_1_STRW_EL2));
+ } else {
+ target->data[1] |= cpu_to_le64(
+ FIELD_PREP(STRTAB_STE_1_STRW, STRTAB_STE_1_STRW_NSEL1));
+
+ /*
+ * VMID 0 is reserved for stage-2 bypass EL1 STEs, see
+ * arm_smmu_domain_alloc_id()
+ */
+ target->data[2] =
+ cpu_to_le64(FIELD_PREP(STRTAB_STE_2_S2VMID, 0));
+ }
}

static void arm_smmu_make_s2_domain_ste(struct arm_smmu_ste *target,
@@ -1567,7 +1588,9 @@ static void arm_smmu_make_s2_domain_ste(struct arm_smmu_ste *target,

target->data[1] = cpu_to_le64(
FIELD_PREP(STRTAB_STE_1_EATS,
- master->ats_enabled ? STRTAB_STE_1_EATS_TRANS : 0));
+ master->ats_enabled ? STRTAB_STE_1_EATS_TRANS : 0) |
+ FIELD_PREP(STRTAB_STE_1_SHCFG,
+ STRTAB_STE_1_SHCFG_NON_SHARABLE));

vtcr_val = FIELD_PREP(STRTAB_STE_2_VTCR_S2T0SZ, vtcr->tsz) |
FIELD_PREP(STRTAB_STE_2_VTCR_S2SL0, vtcr->sl) |
@@ -1590,6 +1613,10 @@ static void arm_smmu_make_s2_domain_ste(struct arm_smmu_ste *target,
STRTAB_STE_3_S2TTB_MASK);
}

+/*
+ * 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,
unsigned int nent)
{
@@ -2632,7 +2659,7 @@ static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev)
if (ret)
goto out_list_del;

- arm_smmu_make_cdtable_ste(&target, master, &master->cd_table);
+ arm_smmu_make_cdtable_ste(&target, master);
arm_smmu_install_ste_for_dev(master, &target);
break;
case ARM_SMMU_DOMAIN_S2:
@@ -3325,8 +3352,6 @@ static int arm_smmu_init_strtab_linear(struct arm_smmu_device *smmu)

arm_smmu_init_bypass_stes(strtab, cfg->num_l1_ents);

- /* Check for RMRs and install bypass STEs if any */
- arm_smmu_rmr_install_bypass_ste(smmu);
return 0;
}

@@ -3350,6 +3375,8 @@ static int arm_smmu_init_strtab(struct arm_smmu_device *smmu)

ida_init(&smmu->vmid_map);

+ /* Check for RMRs and install bypass STEs if any */
+ arm_smmu_rmr_install_bypass_ste(smmu);
return 0;
}

@@ -4049,6 +4076,10 @@ static void arm_smmu_rmr_install_bypass_ste(struct arm_smmu_device *smmu)
continue;
}

+ /*
+ * 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]));
}
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
index 23baf117e7e4b5..23d8ab9a937aa6 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
@@ -249,6 +249,7 @@ struct arm_smmu_ste {
#define STRTAB_STE_1_STRW_EL2 2UL

#define STRTAB_STE_1_SHCFG GENMASK_ULL(45, 44)
+#define STRTAB_STE_1_SHCFG_NON_SHARABLE 0UL
#define STRTAB_STE_1_SHCFG_INCOMING 1UL

#define STRTAB_STE_2_S2VMID GENMASK_ULL(15, 0)

base-commit: 54be6c6c5ae8e0d93a6c4641cb7528eb0b6ba478
--
2.43.0