[PATCH v4 0/8] Tidy some minor things in the stream table/cd table area
Jason Gunthorpe
jgg at nvidia.com
Fri Sep 6 08:47:47 PDT 2024
Will pointed out that two places referring to the CD/STE struct did not
get the new types. While auditing this code a few more oddities were
noticed. Based on a feedback from Mostafa and Nicolin a few more things
were fixed up too
- Use types for all the HW structures everywhere even for the L1
descriptors that are just a single 8 bytes. This helps with clarity of
what everthing is pointing at
- Use indexing helpers for the STE/CD two level calculations
- Use sizeof(struct X) instead of open coded math on constants. The sizeof
naturally follows the type of the related variable in almost all cases
- Remove redundant dma_addr_t's and save some memory
- Remove redundant devm usage
- Use the modern rbtree API
Parts of this have been sitting in my tree for a while now, it grew a bit
since v1, but nothing is particularly profound here. Enough is merged now
that they can be cleanly based and are seperate from my other series.
v4:
- Rebase to v6.11-rc3 & Will's tree
- Adjust whitespace
- Keep goto error unwind
v3: https://patch.msgid.link/r/0-v3-9fef8cdc2ff6+150d1-smmuv3_tidy_jgg@nvidia.com
- Rebase to v6.11-rc2
- Preserve the "2-level strtab only covers %u/%u bits of SID" without
change
- Vertically align some of the constants
- Use u32 for the type of the index and sid
- Fix missing * in le64_to_cpu() in interior patch
- Bring back accidently lost "Use the new rb tree helpers" patch
v2: https://lore.kernel.org/r/0-v2-318ed5f6983b+198f-smmuv3_tidy_jgg@nvidia.com
- Add a patch to add structs for the L1/L2 HW layouts and use their
sizeof and type instead of constants and generic __le64 *.
- Add a patch for L1/L2 indexing helpers for clarity
- Reorder patches
- Redo the union layout in the cfg for both cases
- Fully remove some more defines
v1: https://lore.kernel.org/r/0-v1-1b720dce51d1+4f44-smmuv3_tidy_jgg@nvidia.com
Diff of all applied compared to v3 showing the white spaces I adjusted. Several
interior patches were changed too, but later patches change them some more:
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 f1b6d434009f91..edc625ec261dd1 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -1406,6 +1406,8 @@ void arm_smmu_clear_cd(struct arm_smmu_master *master, ioasid_t ssid)
static int arm_smmu_alloc_cd_tables(struct arm_smmu_master *master)
{
+ int ret;
+ size_t l1size;
size_t max_contexts;
struct arm_smmu_device *smmu = master->smmu;
struct arm_smmu_ctx_desc_cfg *cd_table = &master->cd_table;
@@ -1418,9 +1420,10 @@ static int arm_smmu_alloc_cd_tables(struct arm_smmu_master *master)
cd_table->s1fmt = STRTAB_STE_0_S1FMT_LINEAR;
cd_table->linear.num_ents = max_contexts;
- cd_table->linear.table = dma_alloc_coherent(
- smmu->dev, max_contexts * sizeof(struct arm_smmu_cd),
- &cd_table->cdtab_dma, GFP_KERNEL);
+ l1size = max_contexts * sizeof(struct arm_smmu_cd),
+ cd_table->linear.table = dma_alloc_coherent(smmu->dev, l1size,
+ &cd_table->cdtab_dma,
+ GFP_KERNEL);
if (!cd_table->linear.table)
return -ENOMEM;
} else {
@@ -1434,18 +1437,21 @@ static int arm_smmu_alloc_cd_tables(struct arm_smmu_master *master)
if (!cd_table->l2.l2ptrs)
return -ENOMEM;
- cd_table->l2.l1tab = dma_alloc_coherent(
- smmu->dev,
- cd_table->l2.num_l1_ents *
- sizeof(struct arm_smmu_cdtab_l1),
- &cd_table->cdtab_dma, GFP_KERNEL);
- if (!cd_table->l2.l1tab) {
- kfree(cd_table->l2.l2ptrs);
- cd_table->l2.l2ptrs = NULL;
- return -ENOMEM;
+ l1size = cd_table->l2.num_l1_ents * sizeof(struct arm_smmu_cdtab_l1);
+ cd_table->l2.l1tab = dma_alloc_coherent(smmu->dev, l1size,
+ &cd_table->cdtab_dma,
+ GFP_KERNEL);
+ if (!cd_table->l2.l2ptrs) {
+ ret = -ENOMEM;
+ goto err_free_l2ptrs;
}
}
return 0;
+
+err_free_l2ptrs:
+ kfree(cd_table->l2.l2ptrs);
+ cd_table->l2.l2ptrs = NULL;
+ return ret;
}
static void arm_smmu_free_cd_tables(struct arm_smmu_master *master)
@@ -1462,8 +1468,7 @@ static void arm_smmu_free_cd_tables(struct arm_smmu_master *master)
dma_free_coherent(smmu->dev,
sizeof(*cd_table->l2.l2ptrs[i]),
cd_table->l2.l2ptrs[i],
- arm_smmu_cd_l1_get_desc(
- &cd_table->l2.l1tab[i]));
+ arm_smmu_cd_l1_get_desc(&cd_table->l2.l1tab[i]));
}
kfree(cd_table->l2.l2ptrs);
@@ -1696,9 +1701,9 @@ static int arm_smmu_init_l2_strtab(struct arm_smmu_device *smmu, u32 sid)
{
dma_addr_t l2ptr_dma;
struct arm_smmu_strtab_cfg *cfg = &smmu->strtab_cfg;
- struct arm_smmu_strtab_l2 **l2table =
- &cfg->l2.l2ptrs[arm_smmu_strtab_l1_idx(sid)];
+ struct arm_smmu_strtab_l2 **l2table;
+ l2table = &cfg->l2.l2ptrs[arm_smmu_strtab_l1_idx(sid)];
if (*l2table)
return 0;
@@ -1713,8 +1718,8 @@ static int arm_smmu_init_l2_strtab(struct arm_smmu_device *smmu, u32 sid)
arm_smmu_init_initial_stes((*l2table)->stes,
ARRAY_SIZE((*l2table)->stes));
- arm_smmu_write_strtab_l1_desc(
- &cfg->l2.l1tab[arm_smmu_strtab_l1_idx(sid)], l2ptr_dma);
+ arm_smmu_write_strtab_l1_desc(&cfg->l2.l1tab[arm_smmu_strtab_l1_idx(sid)],
+ l2ptr_dma);
return 0;
}
@@ -3175,8 +3180,7 @@ struct arm_smmu_device *arm_smmu_get_by_fwnode(struct fwnode_handle *fwnode)
static bool arm_smmu_sid_in_range(struct arm_smmu_device *smmu, u32 sid)
{
if (smmu->features & ARM_SMMU_FEAT_2_LVL_STRTAB)
- return arm_smmu_strtab_l1_idx(sid) <
- smmu->strtab_cfg.l2.num_l1_ents;
+ return arm_smmu_strtab_l1_idx(sid) < smmu->strtab_cfg.l2.num_l1_ents;
return sid < smmu->strtab_cfg.linear.num_ents;
}
@@ -3649,8 +3653,9 @@ static int arm_smmu_init_strtab_linear(struct arm_smmu_device *smmu)
struct arm_smmu_strtab_cfg *cfg = &smmu->strtab_cfg;
size = (1 << smmu->sid_bits) * sizeof(struct arm_smmu_ste);
- cfg->linear.table = dmam_alloc_coherent(
- smmu->dev, size, &cfg->linear.ste_dma, GFP_KERNEL);
+ cfg->linear.table = dmam_alloc_coherent(smmu->dev, size,
+ &cfg->linear.ste_dma,
+ GFP_KERNEL);
if (!cfg->linear.table) {
dev_err(smmu->dev,
"failed to allocate linear stream table (%u bytes)\n",
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 dc720f57679652..1e9952ca989f87 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
@@ -209,8 +209,10 @@ struct arm_smmu_device;
#define STRTAB_L1_DESC_SPAN GENMASK_ULL(4, 0)
#define STRTAB_L1_DESC_L2PTR_MASK GENMASK_ULL(51, 6)
+#define STRTAB_STE_DWORDS 8
+
struct arm_smmu_ste {
- __le64 data[8];
+ __le64 data[STRTAB_STE_DWORDS];
};
#define STRTAB_NUM_L2_STES (1 << STRTAB_SPLIT)
@@ -302,8 +304,10 @@ static inline u32 arm_smmu_strtab_l2_idx(u32 sid)
#define CTXDESC_L1_DESC_V (1UL << 0)
#define CTXDESC_L1_DESC_L2PTR_MASK GENMASK_ULL(51, 12)
+#define CTXDESC_CD_DWORDS 8
+
struct arm_smmu_cd {
- __le64 data[8];
+ __le64 data[CTXDESC_CD_DWORDS];
};
struct arm_smmu_cdtab_l2 {
@@ -354,7 +358,7 @@ static inline unsigned int arm_smmu_cdtab_l2_idx(unsigned int ssid)
* When the SMMU only supports linear context descriptor tables, pick a
* reasonable size limit (64kB).
*/
-#define CTXDESC_LINEAR_CDMAX ilog2(SZ_64K / sizeof(struct arm_smmu_cd))
+#define CTXDESC_LINEAR_CDMAX ilog2(SZ_64K / sizeof(struct arm_smmu_cd))
/* Command queue */
#define CMDQ_ENT_SZ_SHIFT 4
Jason Gunthorpe (8):
iommu/arm-smmu-v3: Add arm_smmu_strtab_l1/2_idx()
iommu/arm-smmu-v3: Add types for each level of the 2 level stream
table
iommu/arm-smmu-v3: Reorganize struct arm_smmu_strtab_cfg
iommu/arm-smmu-v3: Remove strtab_base/cfg
iommu/arm-smmu-v3: Do not use devm for the cd table allocations
iommu/arm-smmu-v3: Shrink the cdtab l1_desc array
iommu/arm-smmu-v3: Add types for each level of the CD table
iommu/arm-smmu-v3: Reorganize struct arm_smmu_ctx_desc_cfg
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 284 +++++++++-----------
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 92 +++++--
2 files changed, 200 insertions(+), 176 deletions(-)
base-commit: a2bb820e862d61f9ca1499e500915f9f505a2655
--
2.46.0
More information about the linux-arm-kernel
mailing list