* [PATCH v6 00/10] Refactor the SMMU's CD table ownership
@ 2023-08-16 13:18 Michael Shavit
2023-08-16 13:18 ` [PATCH v6 01/10] iommu/arm-smmu-v3: Move ctx_desc out of s1_cfg Michael Shavit
` (10 more replies)
0 siblings, 11 replies; 22+ messages in thread
From: Michael Shavit @ 2023-08-16 13:18 UTC (permalink / raw)
To: iommu, linux-arm-kernel, linux-kernel
Cc: robin.murphy, will, jean-philippe, jgg, nicolinc, Michael Shavit
Hi all,
This series refactors stage 1 domains so that they describe a single CD
entry. These entries are now inserted into a CD table that is owned by
the arm_smmu_master instead of the domain.
This is conceptually cleaner and unblocks other features, such as
attaching domains with PASID (for unmanaged/dma domains).
This patch series was originally part of a larger patch series that
implemented the set_dev_pasid callback for non-SVA domains but is now
split into a distinct series.
This patch series is also available on gerrit with Jean's SMMU test
engine patches cherry-picked on top for testing:
https://linux-review.googlesource.com/c/linux/kernel/git/torvalds/linux/+/24770/9
Thanks,
Michael Shavit
Changes in v6:
- Undo removal of s1fmt and renaming of s1cdmax
- Unwind the loop in amr_smmu_write_ctx_desc_devices to NULL out the CD
entries we succesfully wrote on failure.
- Add a comment clarifying the different usages of
amr_smmu_write_ctx_desc_devices
- Grab the asid lock while writing the RID CD to prevent a race with
SVA.
- Add the device to the devices list before writing the CD to the table
and installing the CD table.
- Link to v5: https://lore.kernel.org/all/20230808171446.2187795-1-mshavit@google.com/
Changes in v5:
- Clear the 0th CD entry when the domain is detached. Not clearing it
caused a bug in arm_smmu_write_ctx_desc which doesn't expect the entry
to already be set.
- Fix an issue where cd_table.installed wasn't correctly updated.
- Added commit to clean-up now unused master parameter in
arm_smmu_domain_finalise
- Link to v4: https://lore.kernel.org/all/20230802163328.2623773-1-mshavit@google.com/
Changes in v4:
- Added comment about the cd_table's dependency on the iommu core's
group mutex.
- Narrowed the range of code for which the domain's init_mutex is held
on attach since it now only protects the arm_smmu_domain_finalise
call.
- Link to v3: https://lore.kernel.org/all/20230801183845.4026101-1-mshavit@google.com/
Changes in v3:
- Add a helper to write a CD to all masters that a domain is attached
to.
- Fixed an issue where an arm_smmu_write_ctx_desc error return wasn't
correctly handled by its caller.
- Flip the cd_table.installed bit back off when table is detached
- re-order the commit later in the series since flipping the installed
bit to off isn't obvious when the cd_table is still shared by multiple
masters.
- Link to v2: https://lore.kernel.org/all/20230731104833.800114-1-mshavit@google.com/
Changes in v2:
- Allocate CD table when it's first needed instead of on probe.
- Minor changes
- Added commit to rename remaining usages of cdcfg to cd_table
- Link to v1: https://lore.kernel.org/all/20230727182647.4106140-1-mshavit@google.com/#r
Changes in v1:
- Replace s1_cfg with arm_smmu_ctx_desc_cfg representing the CD table
- Assume that the CD table is owned by the SMMU master for most
operations. This is forward-compatible with the nested patch series as
these operations wouldn't be called when the installed CD table comes
from nested domains.
- Split off as a distinct patch series from https://lore.kernel.org/all/20230621063825.268890-1-mshavit@google.com/
Michael Shavit (10):
iommu/arm-smmu-v3: Move ctx_desc out of s1_cfg
iommu/arm-smmu-v3: Replace s1_cfg with cdtab_cfg
iommu/arm-smmu-v3: Encapsulate ctx_desc_cfg init in alloc_cd_tables
iommu/arm-smmu-v3: move stall_enabled to the cd table
iommu/arm-smmu-v3: Refactor write_ctx_desc
iommu/arm-smmu-v3: Move CD table to arm_smmu_master
iommu/arm-smmu-v3: Cleanup arm_smmu_domain_finalise
iommu/arm-smmu-v3: Update comment about STE liveness
iommu/arm-smmu-v3: Skip cd sync if CD table isn't active
iommu/arm-smmu-v3: Rename cdcfg to cd_table
.../iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c | 40 ++-
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 259 +++++++++---------
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 19 +-
3 files changed, 176 insertions(+), 142 deletions(-)
base-commit: 6eaae198076080886b9e7d57f4ae06fa782f90ef
--
2.41.0.694.ge786442a9b-goog
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v6 01/10] iommu/arm-smmu-v3: Move ctx_desc out of s1_cfg
2023-08-16 13:18 [PATCH v6 00/10] Refactor the SMMU's CD table ownership Michael Shavit
@ 2023-08-16 13:18 ` Michael Shavit
2023-08-16 13:18 ` [PATCH v6 02/10] iommu/arm-smmu-v3: Replace s1_cfg with cdtab_cfg Michael Shavit
` (9 subsequent siblings)
10 siblings, 0 replies; 22+ messages in thread
From: Michael Shavit @ 2023-08-16 13:18 UTC (permalink / raw)
To: iommu, linux-arm-kernel, linux-kernel
Cc: robin.murphy, will, jean-philippe, jgg, nicolinc, Michael Shavit
s1_cfg describes the CD table that is inserted into an SMMU's STEs. It's
weird for s1_cfg to also own ctx_desc which describes a CD that is
inserted into that table. It is more appropriate for arm_smmu_domain to
own ctx_desc.
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Reviewed-by: Nicolin Chen <nicolinc@nvidia.com>
Signed-off-by: Michael Shavit <mshavit@google.com>
---
(no changes since v2)
Changes in v2:
- Undo over-reaching column alignment change
.../iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c | 2 +-
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 23 ++++++++++---------
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 6 +++--
3 files changed, 17 insertions(+), 14 deletions(-)
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
index a5a63b1c947eb..968559d625c40 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
@@ -62,7 +62,7 @@ arm_smmu_share_asid(struct mm_struct *mm, u16 asid)
return cd;
}
- smmu_domain = container_of(cd, struct arm_smmu_domain, s1_cfg.cd);
+ smmu_domain = container_of(cd, struct arm_smmu_domain, cd);
smmu = smmu_domain->smmu;
ret = xa_alloc(&arm_smmu_asid_xa, &new_asid, cd,
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 9b0dc35056019..bb277ff86f65f 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -1869,7 +1869,7 @@ static void arm_smmu_tlb_inv_context(void *cookie)
* careful, 007.
*/
if (smmu_domain->stage == ARM_SMMU_DOMAIN_S1) {
- arm_smmu_tlb_inv_asid(smmu, smmu_domain->s1_cfg.cd.asid);
+ arm_smmu_tlb_inv_asid(smmu, smmu_domain->cd.asid);
} else {
cmd.opcode = CMDQ_OP_TLBI_S12_VMALL;
cmd.tlbi.vmid = smmu_domain->s2_cfg.vmid;
@@ -1957,7 +1957,7 @@ static void arm_smmu_tlb_inv_range_domain(unsigned long iova, size_t size,
if (smmu_domain->stage == ARM_SMMU_DOMAIN_S1) {
cmd.opcode = smmu_domain->smmu->features & ARM_SMMU_FEAT_E2H ?
CMDQ_OP_TLBI_EL2_VA : CMDQ_OP_TLBI_NH_VA;
- cmd.tlbi.asid = smmu_domain->s1_cfg.cd.asid;
+ cmd.tlbi.asid = smmu_domain->cd.asid;
} else {
cmd.opcode = CMDQ_OP_TLBI_S2_IPA;
cmd.tlbi.vmid = smmu_domain->s2_cfg.vmid;
@@ -2088,7 +2088,7 @@ static void arm_smmu_domain_free(struct iommu_domain *domain)
mutex_lock(&arm_smmu_asid_lock);
if (cfg->cdcfg.cdtab)
arm_smmu_free_cd_tables(smmu_domain);
- arm_smmu_free_asid(&cfg->cd);
+ arm_smmu_free_asid(&smmu_domain->cd);
mutex_unlock(&arm_smmu_asid_lock);
} else {
struct arm_smmu_s2_cfg *cfg = &smmu_domain->s2_cfg;
@@ -2107,13 +2107,14 @@ static int arm_smmu_domain_finalise_s1(struct arm_smmu_domain *smmu_domain,
u32 asid;
struct arm_smmu_device *smmu = smmu_domain->smmu;
struct arm_smmu_s1_cfg *cfg = &smmu_domain->s1_cfg;
+ struct arm_smmu_ctx_desc *cd = &smmu_domain->cd;
typeof(&pgtbl_cfg->arm_lpae_s1_cfg.tcr) tcr = &pgtbl_cfg->arm_lpae_s1_cfg.tcr;
- refcount_set(&cfg->cd.refs, 1);
+ refcount_set(&cd->refs, 1);
/* Prevent SVA from modifying the ASID until it is written to the CD */
mutex_lock(&arm_smmu_asid_lock);
- ret = xa_alloc(&arm_smmu_asid_xa, &asid, &cfg->cd,
+ ret = xa_alloc(&arm_smmu_asid_xa, &asid, cd,
XA_LIMIT(1, (1 << smmu->asid_bits) - 1), GFP_KERNEL);
if (ret)
goto out_unlock;
@@ -2126,23 +2127,23 @@ static int arm_smmu_domain_finalise_s1(struct arm_smmu_domain *smmu_domain,
if (ret)
goto out_free_asid;
- cfg->cd.asid = (u16)asid;
- cfg->cd.ttbr = pgtbl_cfg->arm_lpae_s1_cfg.ttbr;
- cfg->cd.tcr = FIELD_PREP(CTXDESC_CD_0_TCR_T0SZ, tcr->tsz) |
+ cd->asid = (u16)asid;
+ cd->ttbr = pgtbl_cfg->arm_lpae_s1_cfg.ttbr;
+ cd->tcr = FIELD_PREP(CTXDESC_CD_0_TCR_T0SZ, tcr->tsz) |
FIELD_PREP(CTXDESC_CD_0_TCR_TG0, tcr->tg) |
FIELD_PREP(CTXDESC_CD_0_TCR_IRGN0, tcr->irgn) |
FIELD_PREP(CTXDESC_CD_0_TCR_ORGN0, tcr->orgn) |
FIELD_PREP(CTXDESC_CD_0_TCR_SH0, tcr->sh) |
FIELD_PREP(CTXDESC_CD_0_TCR_IPS, tcr->ips) |
CTXDESC_CD_0_TCR_EPD1 | CTXDESC_CD_0_AA64;
- cfg->cd.mair = pgtbl_cfg->arm_lpae_s1_cfg.mair;
+ cd->mair = pgtbl_cfg->arm_lpae_s1_cfg.mair;
/*
* Note that this will end up calling arm_smmu_sync_cd() before
* the master has been added to the devices list for this domain.
* This isn't an issue because the STE hasn't been installed yet.
*/
- ret = arm_smmu_write_ctx_desc(smmu_domain, 0, &cfg->cd);
+ ret = arm_smmu_write_ctx_desc(smmu_domain, 0, cd);
if (ret)
goto out_free_cd_tables;
@@ -2152,7 +2153,7 @@ static int arm_smmu_domain_finalise_s1(struct arm_smmu_domain *smmu_domain,
out_free_cd_tables:
arm_smmu_free_cd_tables(smmu_domain);
out_free_asid:
- arm_smmu_free_asid(&cfg->cd);
+ arm_smmu_free_asid(cd);
out_unlock:
mutex_unlock(&arm_smmu_asid_lock);
return ret;
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 dcab85698a4e2..f841383a55a35 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
@@ -599,7 +599,6 @@ struct arm_smmu_ctx_desc_cfg {
struct arm_smmu_s1_cfg {
struct arm_smmu_ctx_desc_cfg cdcfg;
- struct arm_smmu_ctx_desc cd;
u8 s1fmt;
u8 s1cdmax;
};
@@ -724,7 +723,10 @@ struct arm_smmu_domain {
enum arm_smmu_domain_stage stage;
union {
- struct arm_smmu_s1_cfg s1_cfg;
+ struct {
+ struct arm_smmu_ctx_desc cd;
+ struct arm_smmu_s1_cfg s1_cfg;
+ };
struct arm_smmu_s2_cfg s2_cfg;
};
--
2.41.0.694.ge786442a9b-goog
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v6 02/10] iommu/arm-smmu-v3: Replace s1_cfg with cdtab_cfg
2023-08-16 13:18 [PATCH v6 00/10] Refactor the SMMU's CD table ownership Michael Shavit
2023-08-16 13:18 ` [PATCH v6 01/10] iommu/arm-smmu-v3: Move ctx_desc out of s1_cfg Michael Shavit
@ 2023-08-16 13:18 ` Michael Shavit
2023-08-16 13:18 ` [PATCH v6 03/10] iommu/arm-smmu-v3: Encapsulate ctx_desc_cfg init in alloc_cd_tables Michael Shavit
` (8 subsequent siblings)
10 siblings, 0 replies; 22+ messages in thread
From: Michael Shavit @ 2023-08-16 13:18 UTC (permalink / raw)
To: iommu, linux-arm-kernel, linux-kernel
Cc: robin.murphy, will, jean-philippe, jgg, nicolinc, Michael Shavit
Remove struct arm_smmu_s1_cfg. This is really just a CD table with a
bit of extra information. Move other attributes of the CD table that
were held there into the existing CD table structure, struct
arm_smmu_ctx_desc_cfg, and replace all usages of arm_smmu_s1_cfg with
arm_smmu_ctx_desc_cfg.
For clarity, use the name "cd_table" for the variables pointing to
arm_smmu_ctx_desc_cfg in the new code instead of cdcfg. A later patch
will make this fully consistent.
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Reviewed-by: Nicolin Chen <nicolinc@nvidia.com>
Signed-off-by: Michael Shavit <mshavit@google.com>
---
Changes in v6:
- Undo removal of s1fmt and renaming of s1cdmax
Changes in v3:
- Updated commit messages again
- Replace more usages of cdcfg with cdtable (lines that were already
touched by this commit anyways).
Changes in v2:
- Updated commit message
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 41 ++++++++++-----------
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 7 +---
2 files changed, 22 insertions(+), 26 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 bb277ff86f65f..5d1977027d2c4 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -1033,9 +1033,9 @@ static __le64 *arm_smmu_get_cd_ptr(struct arm_smmu_domain *smmu_domain,
unsigned int idx;
struct arm_smmu_l1_ctx_desc *l1_desc;
struct arm_smmu_device *smmu = smmu_domain->smmu;
- struct arm_smmu_ctx_desc_cfg *cdcfg = &smmu_domain->s1_cfg.cdcfg;
+ struct arm_smmu_ctx_desc_cfg *cdcfg = &smmu_domain->cd_table;
- if (smmu_domain->s1_cfg.s1fmt == STRTAB_STE_0_S1FMT_LINEAR)
+ if (cdcfg->s1fmt == STRTAB_STE_0_S1FMT_LINEAR)
return cdcfg->cdtab + ssid * CTXDESC_CD_DWORDS;
idx = ssid >> CTXDESC_SPLIT;
@@ -1071,7 +1071,7 @@ int arm_smmu_write_ctx_desc(struct arm_smmu_domain *smmu_domain, int ssid,
bool cd_live;
__le64 *cdptr;
- if (WARN_ON(ssid >= (1 << smmu_domain->s1_cfg.s1cdmax)))
+ if (WARN_ON(ssid >= (1 << smmu_domain->cd_table.s1cdmax)))
return -E2BIG;
cdptr = arm_smmu_get_cd_ptr(smmu_domain, ssid);
@@ -1138,19 +1138,18 @@ static int arm_smmu_alloc_cd_tables(struct arm_smmu_domain *smmu_domain)
size_t l1size;
size_t max_contexts;
struct arm_smmu_device *smmu = smmu_domain->smmu;
- struct arm_smmu_s1_cfg *cfg = &smmu_domain->s1_cfg;
- struct arm_smmu_ctx_desc_cfg *cdcfg = &cfg->cdcfg;
+ struct arm_smmu_ctx_desc_cfg *cdcfg = &smmu_domain->cd_table;
- max_contexts = 1 << cfg->s1cdmax;
+ max_contexts = 1 << cdcfg->s1cdmax;
if (!(smmu->features & ARM_SMMU_FEAT_2_LVL_CDTAB) ||
max_contexts <= CTXDESC_L2_ENTRIES) {
- cfg->s1fmt = STRTAB_STE_0_S1FMT_LINEAR;
+ cdcfg->s1fmt = STRTAB_STE_0_S1FMT_LINEAR;
cdcfg->num_l1_ents = max_contexts;
l1size = max_contexts * (CTXDESC_CD_DWORDS << 3);
} else {
- cfg->s1fmt = STRTAB_STE_0_S1FMT_64K_L2;
+ cdcfg->s1fmt = STRTAB_STE_0_S1FMT_64K_L2;
cdcfg->num_l1_ents = DIV_ROUND_UP(max_contexts,
CTXDESC_L2_ENTRIES);
@@ -1186,7 +1185,7 @@ static void arm_smmu_free_cd_tables(struct arm_smmu_domain *smmu_domain)
int i;
size_t size, l1size;
struct arm_smmu_device *smmu = smmu_domain->smmu;
- struct arm_smmu_ctx_desc_cfg *cdcfg = &smmu_domain->s1_cfg.cdcfg;
+ struct arm_smmu_ctx_desc_cfg *cdcfg = &smmu_domain->cd_table;
if (cdcfg->l1_desc) {
size = CTXDESC_L2_ENTRIES * (CTXDESC_CD_DWORDS << 3);
@@ -1276,7 +1275,7 @@ static void arm_smmu_write_strtab_ent(struct arm_smmu_master *master, u32 sid,
u64 val = le64_to_cpu(dst[0]);
bool ste_live = false;
struct arm_smmu_device *smmu = NULL;
- struct arm_smmu_s1_cfg *s1_cfg = NULL;
+ struct arm_smmu_ctx_desc_cfg *cd_table = NULL;
struct arm_smmu_s2_cfg *s2_cfg = NULL;
struct arm_smmu_domain *smmu_domain = NULL;
struct arm_smmu_cmdq_ent prefetch_cmd = {
@@ -1294,7 +1293,7 @@ static void arm_smmu_write_strtab_ent(struct arm_smmu_master *master, u32 sid,
if (smmu_domain) {
switch (smmu_domain->stage) {
case ARM_SMMU_DOMAIN_S1:
- s1_cfg = &smmu_domain->s1_cfg;
+ cd_table = &smmu_domain->cd_table;
break;
case ARM_SMMU_DOMAIN_S2:
case ARM_SMMU_DOMAIN_NESTED:
@@ -1325,7 +1324,7 @@ static void arm_smmu_write_strtab_ent(struct arm_smmu_master *master, u32 sid,
val = STRTAB_STE_0_V;
/* Bypass/fault */
- if (!smmu_domain || !(s1_cfg || s2_cfg)) {
+ 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
@@ -1344,7 +1343,7 @@ static void arm_smmu_write_strtab_ent(struct arm_smmu_master *master, u32 sid,
return;
}
- if (s1_cfg) {
+ if (cd_table) {
u64 strw = smmu->features & ARM_SMMU_FEAT_E2H ?
STRTAB_STE_1_STRW_EL2 : STRTAB_STE_1_STRW_NSEL1;
@@ -1360,10 +1359,10 @@ static void arm_smmu_write_strtab_ent(struct arm_smmu_master *master, u32 sid,
!master->stall_enabled)
dst[1] |= cpu_to_le64(STRTAB_STE_1_S1STALLD);
- val |= (s1_cfg->cdcfg.cdtab_dma & STRTAB_STE_0_S1CTXPTR_MASK) |
+ val |= (cd_table->cdtab_dma & STRTAB_STE_0_S1CTXPTR_MASK) |
FIELD_PREP(STRTAB_STE_0_CFG, STRTAB_STE_0_CFG_S1_TRANS) |
- FIELD_PREP(STRTAB_STE_0_S1CDMAX, s1_cfg->s1cdmax) |
- FIELD_PREP(STRTAB_STE_0_S1FMT, s1_cfg->s1fmt);
+ FIELD_PREP(STRTAB_STE_0_S1CDMAX, cd_table->s1cdmax) |
+ FIELD_PREP(STRTAB_STE_0_S1FMT, cd_table->s1fmt);
}
if (s2_cfg) {
@@ -2082,11 +2081,11 @@ static void arm_smmu_domain_free(struct iommu_domain *domain)
/* Free the CD and ASID, if we allocated them */
if (smmu_domain->stage == ARM_SMMU_DOMAIN_S1) {
- struct arm_smmu_s1_cfg *cfg = &smmu_domain->s1_cfg;
+ struct arm_smmu_ctx_desc_cfg *cd_table = &smmu_domain->cd_table;
/* Prevent SVA from touching the CD while we're freeing it */
mutex_lock(&arm_smmu_asid_lock);
- if (cfg->cdcfg.cdtab)
+ if (cd_table->cdtab)
arm_smmu_free_cd_tables(smmu_domain);
arm_smmu_free_asid(&smmu_domain->cd);
mutex_unlock(&arm_smmu_asid_lock);
@@ -2106,7 +2105,7 @@ static int arm_smmu_domain_finalise_s1(struct arm_smmu_domain *smmu_domain,
int ret;
u32 asid;
struct arm_smmu_device *smmu = smmu_domain->smmu;
- struct arm_smmu_s1_cfg *cfg = &smmu_domain->s1_cfg;
+ struct arm_smmu_ctx_desc_cfg *cd_table = &smmu_domain->cd_table;
struct arm_smmu_ctx_desc *cd = &smmu_domain->cd;
typeof(&pgtbl_cfg->arm_lpae_s1_cfg.tcr) tcr = &pgtbl_cfg->arm_lpae_s1_cfg.tcr;
@@ -2119,7 +2118,7 @@ static int arm_smmu_domain_finalise_s1(struct arm_smmu_domain *smmu_domain,
if (ret)
goto out_unlock;
- cfg->s1cdmax = master->ssid_bits;
+ cd_table->s1cdmax = master->ssid_bits;
smmu_domain->stall_enabled = master->stall_enabled;
@@ -2457,7 +2456,7 @@ static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev)
ret = -EINVAL;
goto out_unlock;
} else if (smmu_domain->stage == ARM_SMMU_DOMAIN_S1 &&
- master->ssid_bits != smmu_domain->s1_cfg.s1cdmax) {
+ master->ssid_bits != smmu_domain->cd_table.s1cdmax) {
ret = -EINVAL;
goto out_unlock;
} else if (smmu_domain->stage == ARM_SMMU_DOMAIN_S1 &&
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 f841383a55a35..5f0e7468db5f3 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
@@ -595,11 +595,8 @@ struct arm_smmu_ctx_desc_cfg {
dma_addr_t cdtab_dma;
struct arm_smmu_l1_ctx_desc *l1_desc;
unsigned int num_l1_ents;
-};
-
-struct arm_smmu_s1_cfg {
- struct arm_smmu_ctx_desc_cfg cdcfg;
u8 s1fmt;
+ /* log2 of the maximum number of CDs supported by this table */
u8 s1cdmax;
};
@@ -725,7 +722,7 @@ struct arm_smmu_domain {
union {
struct {
struct arm_smmu_ctx_desc cd;
- struct arm_smmu_s1_cfg s1_cfg;
+ struct arm_smmu_ctx_desc_cfg cd_table;
};
struct arm_smmu_s2_cfg s2_cfg;
};
--
2.41.0.694.ge786442a9b-goog
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v6 03/10] iommu/arm-smmu-v3: Encapsulate ctx_desc_cfg init in alloc_cd_tables
2023-08-16 13:18 [PATCH v6 00/10] Refactor the SMMU's CD table ownership Michael Shavit
2023-08-16 13:18 ` [PATCH v6 01/10] iommu/arm-smmu-v3: Move ctx_desc out of s1_cfg Michael Shavit
2023-08-16 13:18 ` [PATCH v6 02/10] iommu/arm-smmu-v3: Replace s1_cfg with cdtab_cfg Michael Shavit
@ 2023-08-16 13:18 ` Michael Shavit
2023-08-16 13:18 ` [PATCH v6 04/10] iommu/arm-smmu-v3: move stall_enabled to the cd table Michael Shavit
` (7 subsequent siblings)
10 siblings, 0 replies; 22+ messages in thread
From: Michael Shavit @ 2023-08-16 13:18 UTC (permalink / raw)
To: iommu, linux-arm-kernel, linux-kernel
Cc: robin.murphy, will, jean-philippe, jgg, nicolinc, Michael Shavit
This is slighlty cleaner: arm_smmu_ctx_desc_cfg is initialized in a
single function instead of having pieces set ahead-of time by its caller.
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Reviewed-by: Nicolin Chen <nicolinc@nvidia.com>
Signed-off-by: Michael Shavit <mshavit@google.com>
---
(no changes since v1)
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 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 5d1977027d2c4..5bb13fadb41ad 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -1132,7 +1132,8 @@ int arm_smmu_write_ctx_desc(struct arm_smmu_domain *smmu_domain, int ssid,
return 0;
}
-static int arm_smmu_alloc_cd_tables(struct arm_smmu_domain *smmu_domain)
+static int arm_smmu_alloc_cd_tables(struct arm_smmu_domain *smmu_domain,
+ struct arm_smmu_master *master)
{
int ret;
size_t l1size;
@@ -1140,6 +1141,7 @@ static int arm_smmu_alloc_cd_tables(struct arm_smmu_domain *smmu_domain)
struct arm_smmu_device *smmu = smmu_domain->smmu;
struct arm_smmu_ctx_desc_cfg *cdcfg = &smmu_domain->cd_table;
+ cdcfg->s1cdmax = master->ssid_bits;
max_contexts = 1 << cdcfg->s1cdmax;
if (!(smmu->features & ARM_SMMU_FEAT_2_LVL_CDTAB) ||
@@ -2105,7 +2107,6 @@ static int arm_smmu_domain_finalise_s1(struct arm_smmu_domain *smmu_domain,
int ret;
u32 asid;
struct arm_smmu_device *smmu = smmu_domain->smmu;
- struct arm_smmu_ctx_desc_cfg *cd_table = &smmu_domain->cd_table;
struct arm_smmu_ctx_desc *cd = &smmu_domain->cd;
typeof(&pgtbl_cfg->arm_lpae_s1_cfg.tcr) tcr = &pgtbl_cfg->arm_lpae_s1_cfg.tcr;
@@ -2118,11 +2119,9 @@ static int arm_smmu_domain_finalise_s1(struct arm_smmu_domain *smmu_domain,
if (ret)
goto out_unlock;
- cd_table->s1cdmax = master->ssid_bits;
-
smmu_domain->stall_enabled = master->stall_enabled;
- ret = arm_smmu_alloc_cd_tables(smmu_domain);
+ ret = arm_smmu_alloc_cd_tables(smmu_domain, master);
if (ret)
goto out_free_asid;
--
2.41.0.694.ge786442a9b-goog
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v6 04/10] iommu/arm-smmu-v3: move stall_enabled to the cd table
2023-08-16 13:18 [PATCH v6 00/10] Refactor the SMMU's CD table ownership Michael Shavit
` (2 preceding siblings ...)
2023-08-16 13:18 ` [PATCH v6 03/10] iommu/arm-smmu-v3: Encapsulate ctx_desc_cfg init in alloc_cd_tables Michael Shavit
@ 2023-08-16 13:18 ` Michael Shavit
2023-08-16 13:18 ` [PATCH v6 05/10] iommu/arm-smmu-v3: Refactor write_ctx_desc Michael Shavit
` (6 subsequent siblings)
10 siblings, 0 replies; 22+ messages in thread
From: Michael Shavit @ 2023-08-16 13:18 UTC (permalink / raw)
To: iommu, linux-arm-kernel, linux-kernel
Cc: robin.murphy, will, jean-philippe, jgg, nicolinc, Michael Shavit
A domain can be attached to multiple masters with different
master->stall_enabled values. The stall bit of a CD entry should follow
master->stall_enabled and has an inverse relationship with the
STE.S1STALLD bit.
The stall_enabled bit does not depend on any property of the domain, so
move it out of the arm_smmu_domain struct. Move it to the CD table
struct so that it can fully describe how CD entries should be written to
it.
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Reviewed-by: Nicolin Chen <nicolinc@nvidia.com>
Signed-off-by: Michael Shavit <mshavit@google.com>
---
(no changes since v5)
Changes in v5:
- Reword commit
Changes in v2:
- Use a bitfield instead of a bool for stall_enabled
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 8 ++++----
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 3 ++-
2 files changed, 6 insertions(+), 5 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 5bb13fadb41ad..44df7c0926802 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -1114,7 +1114,7 @@ int arm_smmu_write_ctx_desc(struct arm_smmu_domain *smmu_domain, int ssid,
FIELD_PREP(CTXDESC_CD_0_ASID, cd->asid) |
CTXDESC_CD_0_V;
- if (smmu_domain->stall_enabled)
+ if (smmu_domain->cd_table.stall_enabled)
val |= CTXDESC_CD_0_S;
}
@@ -1141,6 +1141,7 @@ static int arm_smmu_alloc_cd_tables(struct arm_smmu_domain *smmu_domain,
struct arm_smmu_device *smmu = smmu_domain->smmu;
struct arm_smmu_ctx_desc_cfg *cdcfg = &smmu_domain->cd_table;
+ cdcfg->stall_enabled = master->stall_enabled;
cdcfg->s1cdmax = master->ssid_bits;
max_contexts = 1 << cdcfg->s1cdmax;
@@ -2119,8 +2120,6 @@ static int arm_smmu_domain_finalise_s1(struct arm_smmu_domain *smmu_domain,
if (ret)
goto out_unlock;
- smmu_domain->stall_enabled = master->stall_enabled;
-
ret = arm_smmu_alloc_cd_tables(smmu_domain, master);
if (ret)
goto out_free_asid;
@@ -2459,7 +2458,8 @@ static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev)
ret = -EINVAL;
goto out_unlock;
} else if (smmu_domain->stage == ARM_SMMU_DOMAIN_S1 &&
- smmu_domain->stall_enabled != master->stall_enabled) {
+ smmu_domain->cd_table.stall_enabled !=
+ master->stall_enabled) {
ret = -EINVAL;
goto out_unlock;
}
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 5f0e7468db5f3..007758df57610 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
@@ -598,6 +598,8 @@ struct arm_smmu_ctx_desc_cfg {
u8 s1fmt;
/* log2 of the maximum number of CDs supported by this table */
u8 s1cdmax;
+ /* Whether CD entries in this table have the stall bit set. */
+ u8 stall_enabled:1;
};
struct arm_smmu_s2_cfg {
@@ -715,7 +717,6 @@ struct arm_smmu_domain {
struct mutex init_mutex; /* Protects smmu pointer */
struct io_pgtable_ops *pgtbl_ops;
- bool stall_enabled;
atomic_t nr_ats_masters;
enum arm_smmu_domain_stage stage;
--
2.41.0.694.ge786442a9b-goog
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v6 05/10] iommu/arm-smmu-v3: Refactor write_ctx_desc
2023-08-16 13:18 [PATCH v6 00/10] Refactor the SMMU's CD table ownership Michael Shavit
` (3 preceding siblings ...)
2023-08-16 13:18 ` [PATCH v6 04/10] iommu/arm-smmu-v3: move stall_enabled to the cd table Michael Shavit
@ 2023-08-16 13:18 ` Michael Shavit
2023-08-16 22:36 ` Nicolin Chen
2023-08-16 13:18 ` [PATCH v6 06/10] iommu/arm-smmu-v3: Move CD table to arm_smmu_master Michael Shavit
` (5 subsequent siblings)
10 siblings, 1 reply; 22+ messages in thread
From: Michael Shavit @ 2023-08-16 13:18 UTC (permalink / raw)
To: iommu, linux-arm-kernel, linux-kernel
Cc: robin.murphy, will, jean-philippe, jgg, nicolinc, Michael Shavit
Update arm_smmu_write_ctx_desc and downstream functions to operate on
a master instead of an smmu domain. We expect arm_smmu_write_ctx_desc()
to only be called to write a CD entry into a CD table owned by the
master. Under the hood, arm_smmu_write_ctx_desc still fetches the CD
table from the domain that is attached to the master, but a subsequent
commit will move that table's ownership to the master.
Note that this change isn't a nop refactor since SVA will call
arm_smmu_write_ctx_desc in a loop for every master the domain is
attached to despite the fact that they all share the same CD table. This
loop may look weird but becomes necessary when the CD table becomes
per-master in a subsequent commit.
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Reviewed-by: Nicolin Chen <nicolinc@nvidia.com>
Signed-off-by: Michael Shavit <mshavit@google.com>
---
Changes in v6:
- Unwind the loop in amr_smmu_write_ctx_desc_devices to NULL out the CD
entries we succesfully wrote on failure.
- Add a comment clarifying the different usages of
amr_smmu_write_ctx_desc_devices
Changes in v3:
- Add a helper to write a CD to all masters that a domain is attached
to.
- Fixed an issue where an arm_smmu_write_ctx_desc error return wasn't
correctly handled by its caller.
Changes in v2:
- minor style fixes
Changes in v1:
- arm_smmu_write_ctx_desc now get's the CD table to write to from the
master parameter instead of a distinct parameter. This works well
because the CD table being written to should always be owned by the
master by the end of this series. This version no longer allows master
to be NULL.
.../iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c | 38 ++++++++++++--
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 51 +++++++------------
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 2 +-
3 files changed, 54 insertions(+), 37 deletions(-)
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
index 968559d625c40..238ede8368d10 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
@@ -37,6 +37,35 @@ struct arm_smmu_bond {
static DEFINE_MUTEX(sva_lock);
+/*
+ * Write the CD to the CD tables for all masters that this domain is attached
+ * to. Note that this is used to update the non-pasid CD entry when SVA takes
+ * over an existing ASID, as well as to write new pasid CD entries when
+ * attaching an SVA domain (although the domain passed as the parameter is the
+ * RID domain that this domain is mapped to).
+ */
+static int arm_smmu_write_ctx_desc_devices(struct arm_smmu_domain *smmu_domain,
+ int ssid,
+ struct arm_smmu_ctx_desc *cd)
+{
+ struct arm_smmu_master *master;
+ unsigned long flags;
+ int ret;
+
+ spin_lock_irqsave(&smmu_domain->devices_lock, flags);
+ list_for_each_entry(master, &smmu_domain->devices, domain_head) {
+ ret = arm_smmu_write_ctx_desc(master, ssid, cd);
+ if (ret) {
+ list_for_each_entry_from_reverse(master, &smmu_domain->devices, domain_head)
+ arm_smmu_write_ctx_desc(master, ssid, NULL);
+ break;
+ }
+ }
+
+ spin_unlock_irqrestore(&smmu_domain->devices_lock, flags);
+ return ret;
+}
+
/*
* Check if the CPU ASID is available on the SMMU side. If a private context
* descriptor is using it, try to replace it.
@@ -80,7 +109,7 @@ arm_smmu_share_asid(struct mm_struct *mm, u16 asid)
* be some overlap between use of both ASIDs, until we invalidate the
* TLB.
*/
- arm_smmu_write_ctx_desc(smmu_domain, 0, cd);
+ arm_smmu_write_ctx_desc_devices(smmu_domain, 0, cd);
/* Invalidate TLB entries previously associated with that context */
arm_smmu_tlb_inv_asid(smmu, asid);
@@ -222,7 +251,7 @@ static void arm_smmu_mm_release(struct mmu_notifier *mn, struct mm_struct *mm)
* DMA may still be running. Keep the cd valid to avoid C_BAD_CD events,
* but disable translation.
*/
- arm_smmu_write_ctx_desc(smmu_domain, mm->pasid, &quiet_cd);
+ arm_smmu_write_ctx_desc_devices(smmu_domain, mm->pasid, &quiet_cd);
arm_smmu_tlb_inv_asid(smmu_domain->smmu, smmu_mn->cd->asid);
arm_smmu_atc_inv_domain(smmu_domain, mm->pasid, 0, 0);
@@ -279,7 +308,7 @@ arm_smmu_mmu_notifier_get(struct arm_smmu_domain *smmu_domain,
goto err_free_cd;
}
- ret = arm_smmu_write_ctx_desc(smmu_domain, mm->pasid, cd);
+ ret = arm_smmu_write_ctx_desc_devices(smmu_domain, mm->pasid, cd);
if (ret)
goto err_put_notifier;
@@ -304,7 +333,8 @@ static void arm_smmu_mmu_notifier_put(struct arm_smmu_mmu_notifier *smmu_mn)
return;
list_del(&smmu_mn->list);
- arm_smmu_write_ctx_desc(smmu_domain, mm->pasid, NULL);
+
+ arm_smmu_write_ctx_desc_devices(smmu_domain, mm->pasid, NULL);
/*
* If we went through clear(), we've already invalidated, and no
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 44df7c0926802..64c1ec557dbc1 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -971,14 +971,12 @@ void arm_smmu_tlb_inv_asid(struct arm_smmu_device *smmu, u16 asid)
arm_smmu_cmdq_issue_cmd_with_sync(smmu, &cmd);
}
-static void arm_smmu_sync_cd(struct arm_smmu_domain *smmu_domain,
+static void arm_smmu_sync_cd(struct arm_smmu_master *master,
int ssid, bool leaf)
{
size_t i;
- unsigned long flags;
- struct arm_smmu_master *master;
struct arm_smmu_cmdq_batch cmds;
- struct arm_smmu_device *smmu = smmu_domain->smmu;
+ struct arm_smmu_device *smmu = master->smmu;
struct arm_smmu_cmdq_ent cmd = {
.opcode = CMDQ_OP_CFGI_CD,
.cfgi = {
@@ -988,15 +986,10 @@ static void arm_smmu_sync_cd(struct arm_smmu_domain *smmu_domain,
};
cmds.num = 0;
-
- spin_lock_irqsave(&smmu_domain->devices_lock, flags);
- list_for_each_entry(master, &smmu_domain->devices, domain_head) {
- for (i = 0; i < master->num_streams; i++) {
- cmd.cfgi.sid = master->streams[i].id;
- arm_smmu_cmdq_batch_add(smmu, &cmds, &cmd);
- }
+ for (i = 0; i < master->num_streams; i++) {
+ cmd.cfgi.sid = master->streams[i].id;
+ arm_smmu_cmdq_batch_add(smmu, &cmds, &cmd);
}
- spin_unlock_irqrestore(&smmu_domain->devices_lock, flags);
arm_smmu_cmdq_batch_submit(smmu, &cmds);
}
@@ -1026,14 +1019,13 @@ static void arm_smmu_write_cd_l1_desc(__le64 *dst,
WRITE_ONCE(*dst, cpu_to_le64(val));
}
-static __le64 *arm_smmu_get_cd_ptr(struct arm_smmu_domain *smmu_domain,
- u32 ssid)
+static __le64 *arm_smmu_get_cd_ptr(struct arm_smmu_master *master, u32 ssid)
{
__le64 *l1ptr;
unsigned int idx;
struct arm_smmu_l1_ctx_desc *l1_desc;
- struct arm_smmu_device *smmu = smmu_domain->smmu;
- struct arm_smmu_ctx_desc_cfg *cdcfg = &smmu_domain->cd_table;
+ struct arm_smmu_device *smmu = master->smmu;
+ struct arm_smmu_ctx_desc_cfg *cdcfg = &master->domain->cd_table;
if (cdcfg->s1fmt == STRTAB_STE_0_S1FMT_LINEAR)
return cdcfg->cdtab + ssid * CTXDESC_CD_DWORDS;
@@ -1047,13 +1039,13 @@ static __le64 *arm_smmu_get_cd_ptr(struct arm_smmu_domain *smmu_domain,
l1ptr = cdcfg->cdtab + idx * CTXDESC_L1_DESC_DWORDS;
arm_smmu_write_cd_l1_desc(l1ptr, l1_desc);
/* An invalid L1CD can be cached */
- arm_smmu_sync_cd(smmu_domain, ssid, false);
+ arm_smmu_sync_cd(master, ssid, false);
}
idx = ssid & (CTXDESC_L2_ENTRIES - 1);
return l1_desc->l2ptr + idx * CTXDESC_CD_DWORDS;
}
-int arm_smmu_write_ctx_desc(struct arm_smmu_domain *smmu_domain, int ssid,
+int arm_smmu_write_ctx_desc(struct arm_smmu_master *master, int ssid,
struct arm_smmu_ctx_desc *cd)
{
/*
@@ -1070,11 +1062,12 @@ int arm_smmu_write_ctx_desc(struct arm_smmu_domain *smmu_domain, int ssid,
u64 val;
bool cd_live;
__le64 *cdptr;
+ struct arm_smmu_ctx_desc_cfg *cd_table = &master->domain->cd_table;
- if (WARN_ON(ssid >= (1 << smmu_domain->cd_table.s1cdmax)))
+ if (WARN_ON(ssid >= (1 << cd_table->s1cdmax)))
return -E2BIG;
- cdptr = arm_smmu_get_cd_ptr(smmu_domain, ssid);
+ cdptr = arm_smmu_get_cd_ptr(master, ssid);
if (!cdptr)
return -ENOMEM;
@@ -1102,7 +1095,7 @@ int arm_smmu_write_ctx_desc(struct arm_smmu_domain *smmu_domain, int ssid,
* order. Ensure that it observes valid values before reading
* V=1.
*/
- arm_smmu_sync_cd(smmu_domain, ssid, true);
+ arm_smmu_sync_cd(master, ssid, true);
val = cd->tcr |
#ifdef __BIG_ENDIAN
@@ -1114,7 +1107,7 @@ int arm_smmu_write_ctx_desc(struct arm_smmu_domain *smmu_domain, int ssid,
FIELD_PREP(CTXDESC_CD_0_ASID, cd->asid) |
CTXDESC_CD_0_V;
- if (smmu_domain->cd_table.stall_enabled)
+ if (cd_table->stall_enabled)
val |= CTXDESC_CD_0_S;
}
@@ -1128,7 +1121,7 @@ int arm_smmu_write_ctx_desc(struct arm_smmu_domain *smmu_domain, int ssid,
* without first making the structure invalid.
*/
WRITE_ONCE(cdptr[0], cpu_to_le64(val));
- arm_smmu_sync_cd(smmu_domain, ssid, true);
+ arm_smmu_sync_cd(master, ssid, true);
return 0;
}
@@ -1138,7 +1131,7 @@ static int arm_smmu_alloc_cd_tables(struct arm_smmu_domain *smmu_domain,
int ret;
size_t l1size;
size_t max_contexts;
- struct arm_smmu_device *smmu = smmu_domain->smmu;
+ struct arm_smmu_device *smmu = master->smmu;
struct arm_smmu_ctx_desc_cfg *cdcfg = &smmu_domain->cd_table;
cdcfg->stall_enabled = master->stall_enabled;
@@ -2135,12 +2128,7 @@ static int arm_smmu_domain_finalise_s1(struct arm_smmu_domain *smmu_domain,
CTXDESC_CD_0_TCR_EPD1 | CTXDESC_CD_0_AA64;
cd->mair = pgtbl_cfg->arm_lpae_s1_cfg.mair;
- /*
- * Note that this will end up calling arm_smmu_sync_cd() before
- * the master has been added to the devices list for this domain.
- * This isn't an issue because the STE hasn't been installed yet.
- */
- ret = arm_smmu_write_ctx_desc(smmu_domain, 0, cd);
+ ret = arm_smmu_write_ctx_desc(master, 0, cd);
if (ret)
goto out_free_cd_tables;
@@ -2458,8 +2446,7 @@ static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev)
ret = -EINVAL;
goto out_unlock;
} else if (smmu_domain->stage == ARM_SMMU_DOMAIN_S1 &&
- smmu_domain->cd_table.stall_enabled !=
- master->stall_enabled) {
+ smmu_domain->cd_table.stall_enabled != master->stall_enabled) {
ret = -EINVAL;
goto out_unlock;
}
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 007758df57610..00f8e6388848e 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
@@ -745,7 +745,7 @@ extern struct xarray arm_smmu_asid_xa;
extern struct mutex arm_smmu_asid_lock;
extern struct arm_smmu_ctx_desc quiet_cd;
-int arm_smmu_write_ctx_desc(struct arm_smmu_domain *smmu_domain, int ssid,
+int arm_smmu_write_ctx_desc(struct arm_smmu_master *smmu_master, int ssid,
struct arm_smmu_ctx_desc *cd);
void arm_smmu_tlb_inv_asid(struct arm_smmu_device *smmu, u16 asid);
void arm_smmu_tlb_inv_range_asid(unsigned long iova, size_t size, int asid,
--
2.41.0.694.ge786442a9b-goog
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v6 06/10] iommu/arm-smmu-v3: Move CD table to arm_smmu_master
2023-08-16 13:18 [PATCH v6 00/10] Refactor the SMMU's CD table ownership Michael Shavit
` (4 preceding siblings ...)
2023-08-16 13:18 ` [PATCH v6 05/10] iommu/arm-smmu-v3: Refactor write_ctx_desc Michael Shavit
@ 2023-08-16 13:18 ` Michael Shavit
2023-08-16 23:44 ` Nicolin Chen
2023-08-17 0:50 ` Nicolin Chen
2023-08-16 13:18 ` [PATCH v6 07/10] iommu/arm-smmu-v3: Cleanup arm_smmu_domain_finalise Michael Shavit
` (4 subsequent siblings)
10 siblings, 2 replies; 22+ messages in thread
From: Michael Shavit @ 2023-08-16 13:18 UTC (permalink / raw)
To: iommu, linux-arm-kernel, linux-kernel
Cc: robin.murphy, will, jean-philippe, jgg, nicolinc, Michael Shavit
With this change, each master will now own its own CD table instead of
sharing one with other masters attached to the same domain. Attaching a
stage 1 domain installs CD entries into the master's CD table. SVA
writes its CD entries into each master's CD table if the domain is
shared across masters.
Also add the device to the devices list before writing the CD to the
table so that SVA is always aware of all the CD tables that need
re-writing when it updates a CD.
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Signed-off-by: Michael Shavit <mshavit@google.com>
---
Changes in v6:
- Grab the asid lock while writing the RID CD to prevent a race with
SVA.
- Add the device to the devices list before writing the CD to the table
and installing the CD table.
- Undo arm_smmu_finalise_s1 rename
- Minor comment fix
- Consistently check cdtab pointer instead of cdtab_dma
Changes in v5:
- Clear the 0th CD entry when the domain is detached. Not clearing it
caused a bug in arm_smmu_write_ctx_desc which doesn't expect the entry
to already be set.
Changes in v4:
- Added comment about the cd_table's dependency on the iommu core's
group mutex.
- Narrowed the range of code for which the domain's init_mutex is held
on attach since it now only protects the arm_smmu_domain_finalise
call.
Changes in v2:
- Allocate CD table when it's first needed instead of on probe.
Changes in v1:
- The master's CD table allocation was previously split to a different
commit. This change now atomically allocates the new CD table, uses
it, and removes the old one.
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 100 +++++++++++---------
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 7 +-
2 files changed, 60 insertions(+), 47 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 64c1ec557dbc1..8c5e5fcd55713 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -1025,7 +1025,7 @@ static __le64 *arm_smmu_get_cd_ptr(struct arm_smmu_master *master, u32 ssid)
unsigned int idx;
struct arm_smmu_l1_ctx_desc *l1_desc;
struct arm_smmu_device *smmu = master->smmu;
- struct arm_smmu_ctx_desc_cfg *cdcfg = &master->domain->cd_table;
+ struct arm_smmu_ctx_desc_cfg *cdcfg = &master->cd_table;
if (cdcfg->s1fmt == STRTAB_STE_0_S1FMT_LINEAR)
return cdcfg->cdtab + ssid * CTXDESC_CD_DWORDS;
@@ -1062,7 +1062,7 @@ int arm_smmu_write_ctx_desc(struct arm_smmu_master *master, int ssid,
u64 val;
bool cd_live;
__le64 *cdptr;
- struct arm_smmu_ctx_desc_cfg *cd_table = &master->domain->cd_table;
+ struct arm_smmu_ctx_desc_cfg *cd_table = &master->cd_table;
if (WARN_ON(ssid >= (1 << cd_table->s1cdmax)))
return -E2BIG;
@@ -1125,14 +1125,13 @@ int arm_smmu_write_ctx_desc(struct arm_smmu_master *master, int ssid,
return 0;
}
-static int arm_smmu_alloc_cd_tables(struct arm_smmu_domain *smmu_domain,
- struct arm_smmu_master *master)
+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 *cdcfg = &smmu_domain->cd_table;
+ struct arm_smmu_ctx_desc_cfg *cdcfg = &master->cd_table;
cdcfg->stall_enabled = master->stall_enabled;
cdcfg->s1cdmax = master->ssid_bits;
@@ -1176,12 +1175,12 @@ static int arm_smmu_alloc_cd_tables(struct arm_smmu_domain *smmu_domain,
return ret;
}
-static void arm_smmu_free_cd_tables(struct arm_smmu_domain *smmu_domain)
+static void arm_smmu_free_cd_tables(struct arm_smmu_master *master)
{
int i;
size_t size, l1size;
- struct arm_smmu_device *smmu = smmu_domain->smmu;
- struct arm_smmu_ctx_desc_cfg *cdcfg = &smmu_domain->cd_table;
+ struct arm_smmu_device *smmu = master->smmu;
+ struct arm_smmu_ctx_desc_cfg *cdcfg = &master->cd_table;
if (cdcfg->l1_desc) {
size = CTXDESC_L2_ENTRIES * (CTXDESC_CD_DWORDS << 3);
@@ -1289,7 +1288,7 @@ static void arm_smmu_write_strtab_ent(struct arm_smmu_master *master, u32 sid,
if (smmu_domain) {
switch (smmu_domain->stage) {
case ARM_SMMU_DOMAIN_S1:
- cd_table = &smmu_domain->cd_table;
+ cd_table = &master->cd_table;
break;
case ARM_SMMU_DOMAIN_S2:
case ARM_SMMU_DOMAIN_NESTED:
@@ -2075,14 +2074,10 @@ static void arm_smmu_domain_free(struct iommu_domain *domain)
free_io_pgtable_ops(smmu_domain->pgtbl_ops);
- /* Free the CD and ASID, if we allocated them */
+ /* Free the ASID or VMID */
if (smmu_domain->stage == ARM_SMMU_DOMAIN_S1) {
- struct arm_smmu_ctx_desc_cfg *cd_table = &smmu_domain->cd_table;
-
/* Prevent SVA from touching the CD while we're freeing it */
mutex_lock(&arm_smmu_asid_lock);
- if (cd_table->cdtab)
- arm_smmu_free_cd_tables(smmu_domain);
arm_smmu_free_asid(&smmu_domain->cd);
mutex_unlock(&arm_smmu_asid_lock);
} else {
@@ -2113,10 +2108,6 @@ static int arm_smmu_domain_finalise_s1(struct arm_smmu_domain *smmu_domain,
if (ret)
goto out_unlock;
- ret = arm_smmu_alloc_cd_tables(smmu_domain, master);
- if (ret)
- goto out_free_asid;
-
cd->asid = (u16)asid;
cd->ttbr = pgtbl_cfg->arm_lpae_s1_cfg.ttbr;
cd->tcr = FIELD_PREP(CTXDESC_CD_0_TCR_T0SZ, tcr->tsz) |
@@ -2128,17 +2119,9 @@ static int arm_smmu_domain_finalise_s1(struct arm_smmu_domain *smmu_domain,
CTXDESC_CD_0_TCR_EPD1 | CTXDESC_CD_0_AA64;
cd->mair = pgtbl_cfg->arm_lpae_s1_cfg.mair;
- ret = arm_smmu_write_ctx_desc(master, 0, cd);
- if (ret)
- goto out_free_cd_tables;
-
mutex_unlock(&arm_smmu_asid_lock);
return 0;
-out_free_cd_tables:
- arm_smmu_free_cd_tables(smmu_domain);
-out_free_asid:
- arm_smmu_free_asid(cd);
out_unlock:
mutex_unlock(&arm_smmu_asid_lock);
return ret;
@@ -2400,6 +2383,16 @@ 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);
+ /*
+ * The table is uninstalled before clearing the CD to prevent an
+ * unnecessary sync in arm_smmu_write_ctx_desc. Although clearing the
+ * CD entry isn't strictly required to detach the domain since the
+ * table is uninstalled anyway, it helps avoid confusion in the call to
+ * arm_smmu_write_ctx_desc on the next attach (which expects the entry
+ * to be empty).
+ */
+ if (smmu_domain->stage == ARM_SMMU_DOMAIN_S1 && master->cd_table.cdtab)
+ arm_smmu_write_ctx_desc(master, 0, NULL);
}
static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev)
@@ -2434,22 +2427,14 @@ static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev)
if (!smmu_domain->smmu) {
smmu_domain->smmu = smmu;
ret = arm_smmu_domain_finalise(domain, master);
- if (ret) {
+ if (ret)
smmu_domain->smmu = NULL;
- goto out_unlock;
- }
- } else if (smmu_domain->smmu != smmu) {
- ret = -EINVAL;
- goto out_unlock;
- } else if (smmu_domain->stage == ARM_SMMU_DOMAIN_S1 &&
- master->ssid_bits != smmu_domain->cd_table.s1cdmax) {
+ } else if (smmu_domain->smmu != smmu)
ret = -EINVAL;
- goto out_unlock;
- } else if (smmu_domain->stage == ARM_SMMU_DOMAIN_S1 &&
- smmu_domain->cd_table.stall_enabled != master->stall_enabled) {
- ret = -EINVAL;
- goto out_unlock;
- }
+
+ mutex_unlock(&smmu_domain->init_mutex);
+ if (ret)
+ return ret;
master->domain = smmu_domain;
@@ -2463,16 +2448,43 @@ static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev)
if (smmu_domain->stage != ARM_SMMU_DOMAIN_BYPASS)
master->ats_enabled = arm_smmu_ats_supported(master);
- arm_smmu_install_ste_for_dev(master);
spin_lock_irqsave(&smmu_domain->devices_lock, flags);
list_add(&master->domain_head, &smmu_domain->devices);
spin_unlock_irqrestore(&smmu_domain->devices_lock, flags);
+ if (smmu_domain->stage == ARM_SMMU_DOMAIN_S1) {
+ if (!master->cd_table.cdtab) {
+ ret = arm_smmu_alloc_cd_tables(master);
+ if (ret) {
+ master->domain = NULL;
+ goto out_list_del;
+ }
+ }
+
+ /*
+ * Prevent SVA from concurrently modifying the CD or writing to
+ * the CD entry
+ */
+ mutex_lock(&arm_smmu_asid_lock);
+ ret = arm_smmu_write_ctx_desc(master, 0, &smmu_domain->cd);
+ mutex_unlock(&arm_smmu_asid_lock);
+ if (ret) {
+ master->domain = NULL;
+ goto out_list_del;
+ }
+ }
+
+ arm_smmu_install_ste_for_dev(master);
+
arm_smmu_enable_ats(master);
+ return 0;
+
+out_list_del:
+ spin_lock_irqsave(&smmu_domain->devices_lock, flags);
+ list_del(&master->domain_head);
+ spin_unlock_irqrestore(&smmu_domain->devices_lock, flags);
-out_unlock:
- mutex_unlock(&smmu_domain->init_mutex);
return ret;
}
@@ -2717,6 +2729,8 @@ static void arm_smmu_release_device(struct device *dev)
arm_smmu_detach_dev(master);
arm_smmu_disable_pasid(master);
arm_smmu_remove_master(master);
+ if (master->cd_table.cdtab)
+ arm_smmu_free_cd_tables(master);
kfree(master);
}
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 00f8e6388848e..2f4b832e0deb4 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
@@ -695,6 +695,8 @@ struct arm_smmu_master {
struct arm_smmu_domain *domain;
struct list_head domain_head;
struct arm_smmu_stream *streams;
+ /* Locked by the iommu core using the group mutex */
+ struct arm_smmu_ctx_desc_cfg cd_table;
unsigned int num_streams;
bool ats_enabled;
bool stall_enabled;
@@ -721,11 +723,8 @@ struct arm_smmu_domain {
enum arm_smmu_domain_stage stage;
union {
- struct {
struct arm_smmu_ctx_desc cd;
- struct arm_smmu_ctx_desc_cfg cd_table;
- };
- struct arm_smmu_s2_cfg s2_cfg;
+ struct arm_smmu_s2_cfg s2_cfg;
};
struct iommu_domain domain;
--
2.41.0.694.ge786442a9b-goog
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v6 07/10] iommu/arm-smmu-v3: Cleanup arm_smmu_domain_finalise
2023-08-16 13:18 [PATCH v6 00/10] Refactor the SMMU's CD table ownership Michael Shavit
` (5 preceding siblings ...)
2023-08-16 13:18 ` [PATCH v6 06/10] iommu/arm-smmu-v3: Move CD table to arm_smmu_master Michael Shavit
@ 2023-08-16 13:18 ` Michael Shavit
2023-08-16 23:45 ` Nicolin Chen
2023-08-16 13:18 ` [PATCH v6 08/10] iommu/arm-smmu-v3: Update comment about STE liveness Michael Shavit
` (3 subsequent siblings)
10 siblings, 1 reply; 22+ messages in thread
From: Michael Shavit @ 2023-08-16 13:18 UTC (permalink / raw)
To: iommu, linux-arm-kernel, linux-kernel
Cc: robin.murphy, will, jean-philippe, jgg, nicolinc, Michael Shavit
Remove unused master parameter now that the CD table is allocated
elsewhere.
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Signed-off-by: Michael Shavit <mshavit@google.com>
---
(no changes since v5)
Changes in v5:
- New commit
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 10 +++-------
1 file changed, 3 insertions(+), 7 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 8c5e5fcd55713..de87150cd0242 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -2090,7 +2090,6 @@ static void arm_smmu_domain_free(struct iommu_domain *domain)
}
static int arm_smmu_domain_finalise_s1(struct arm_smmu_domain *smmu_domain,
- struct arm_smmu_master *master,
struct io_pgtable_cfg *pgtbl_cfg)
{
int ret;
@@ -2128,7 +2127,6 @@ static int arm_smmu_domain_finalise_s1(struct arm_smmu_domain *smmu_domain,
}
static int arm_smmu_domain_finalise_s2(struct arm_smmu_domain *smmu_domain,
- struct arm_smmu_master *master,
struct io_pgtable_cfg *pgtbl_cfg)
{
int vmid;
@@ -2153,8 +2151,7 @@ static int arm_smmu_domain_finalise_s2(struct arm_smmu_domain *smmu_domain,
return 0;
}
-static int arm_smmu_domain_finalise(struct iommu_domain *domain,
- struct arm_smmu_master *master)
+static int arm_smmu_domain_finalise(struct iommu_domain *domain)
{
int ret;
unsigned long ias, oas;
@@ -2162,7 +2159,6 @@ static int arm_smmu_domain_finalise(struct iommu_domain *domain,
struct io_pgtable_cfg pgtbl_cfg;
struct io_pgtable_ops *pgtbl_ops;
int (*finalise_stage_fn)(struct arm_smmu_domain *,
- struct arm_smmu_master *,
struct io_pgtable_cfg *);
struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
struct arm_smmu_device *smmu = smmu_domain->smmu;
@@ -2214,7 +2210,7 @@ static int arm_smmu_domain_finalise(struct iommu_domain *domain,
domain->geometry.aperture_end = (1UL << pgtbl_cfg.ias) - 1;
domain->geometry.force_aperture = true;
- ret = finalise_stage_fn(smmu_domain, master, &pgtbl_cfg);
+ ret = finalise_stage_fn(smmu_domain, &pgtbl_cfg);
if (ret < 0) {
free_io_pgtable_ops(pgtbl_ops);
return ret;
@@ -2426,7 +2422,7 @@ static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev)
if (!smmu_domain->smmu) {
smmu_domain->smmu = smmu;
- ret = arm_smmu_domain_finalise(domain, master);
+ ret = arm_smmu_domain_finalise(domain);
if (ret)
smmu_domain->smmu = NULL;
} else if (smmu_domain->smmu != smmu)
--
2.41.0.694.ge786442a9b-goog
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v6 08/10] iommu/arm-smmu-v3: Update comment about STE liveness
2023-08-16 13:18 [PATCH v6 00/10] Refactor the SMMU's CD table ownership Michael Shavit
` (6 preceding siblings ...)
2023-08-16 13:18 ` [PATCH v6 07/10] iommu/arm-smmu-v3: Cleanup arm_smmu_domain_finalise Michael Shavit
@ 2023-08-16 13:18 ` Michael Shavit
2023-08-16 14:00 ` Jason Gunthorpe
2023-08-16 23:46 ` Nicolin Chen
2023-08-16 13:18 ` [PATCH v6 09/10] iommu/arm-smmu-v3: Skip cd sync if CD table isn't active Michael Shavit
` (2 subsequent siblings)
10 siblings, 2 replies; 22+ messages in thread
From: Michael Shavit @ 2023-08-16 13:18 UTC (permalink / raw)
To: iommu, linux-arm-kernel, linux-kernel
Cc: robin.murphy, will, jean-philippe, jgg, nicolinc, Michael Shavit
Update the comment to reflect the fact that the STE is not always
installed. arm_smmu_domain_finalise_s1 intentionnaly calls
arm_smmu_write_ctx_desc while the STE is not installed.
Signed-off-by: Michael Shavit <mshavit@google.com>
---
Changes in v6:
- New commit
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
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 de87150cd0242..3c8bfeca89d5c 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -1091,7 +1091,7 @@ int arm_smmu_write_ctx_desc(struct arm_smmu_master *master, int ssid,
cdptr[3] = cpu_to_le64(cd->mair);
/*
- * STE is live, and the SMMU might read dwords of this CD in any
+ * STE may be live, and the SMMU might read dwords of this CD in any
* order. Ensure that it observes valid values before reading
* V=1.
*/
--
2.41.0.694.ge786442a9b-goog
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v6 09/10] iommu/arm-smmu-v3: Skip cd sync if CD table isn't active
2023-08-16 13:18 [PATCH v6 00/10] Refactor the SMMU's CD table ownership Michael Shavit
` (7 preceding siblings ...)
2023-08-16 13:18 ` [PATCH v6 08/10] iommu/arm-smmu-v3: Update comment about STE liveness Michael Shavit
@ 2023-08-16 13:18 ` Michael Shavit
2023-08-21 10:16 ` Michael Shavit
2023-08-16 13:18 ` [PATCH v6 10/10] iommu/arm-smmu-v3: Rename cdcfg to cd_table Michael Shavit
2023-08-16 23:59 ` [PATCH v6 00/10] Refactor the SMMU's CD table ownership Nicolin Chen
10 siblings, 1 reply; 22+ messages in thread
From: Michael Shavit @ 2023-08-16 13:18 UTC (permalink / raw)
To: iommu, linux-arm-kernel, linux-kernel
Cc: robin.murphy, will, jean-philippe, jgg, nicolinc, Michael Shavit
This commit explicitly keeps track of whether a CD table is installed in
an STE so that arm_smmu_sync_cd can skip the sync when unnecessary. This
was previously achieved through the domain->devices list, but we are
moving to a model where arm_smmu_sync_cd directly operates on a master
and the master's CD table instead of a domain.
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Reviewed-by: Nicolin Chen <nicolinc@nvidia.com>
Signed-off-by: Michael Shavit <mshavit@google.com>
---
Happy to drop this commit if we don't think it's useful.
(no changes since v5)
Changes in v5:
- Fix an issue where cd_table.installed wasn't correctly updated.
Changes in v3:
- Flip the cd_table.installed bit back off when table is detached
- re-order the commit later in the series since flipping the installed
bit to off isn't obvious when the cd_table is still shared by multiple
masters.
Changes in v2:
- Store field as a bit instead of a bool. Fix comment about STE being
live before the sync in write_ctx_desc().
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 7 +++++++
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 2 ++
2 files changed, 9 insertions(+)
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 3c8bfeca89d5c..104b8d6ea5972 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -985,6 +985,9 @@ static void arm_smmu_sync_cd(struct arm_smmu_master *master,
},
};
+ if (!master->cd_table.installed)
+ return;
+
cmds.num = 0;
for (i = 0; i < master->num_streams; i++) {
cmd.cfgi.sid = master->streams[i].id;
@@ -1335,6 +1338,7 @@ static void arm_smmu_write_strtab_ent(struct arm_smmu_master *master, u32 sid,
*/
if (smmu)
arm_smmu_sync_ste_for_sid(smmu, sid);
+ master->cd_table.installed = false;
return;
}
@@ -1358,6 +1362,9 @@ static void arm_smmu_write_strtab_ent(struct arm_smmu_master *master, u32 sid,
FIELD_PREP(STRTAB_STE_0_CFG, STRTAB_STE_0_CFG_S1_TRANS) |
FIELD_PREP(STRTAB_STE_0_S1CDMAX, cd_table->s1cdmax) |
FIELD_PREP(STRTAB_STE_0_S1FMT, cd_table->s1fmt);
+ cd_table->installed = true;
+ } else {
+ master->cd_table.installed = false;
}
if (s2_cfg) {
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 2f4b832e0deb4..b7a91c8e9b523 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
@@ -600,6 +600,8 @@ struct arm_smmu_ctx_desc_cfg {
u8 s1cdmax;
/* Whether CD entries in this table have the stall bit set. */
u8 stall_enabled:1;
+ /* Whether this CD table is installed in any STE */
+ u8 installed:1;
};
struct arm_smmu_s2_cfg {
--
2.41.0.694.ge786442a9b-goog
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v6 10/10] iommu/arm-smmu-v3: Rename cdcfg to cd_table
2023-08-16 13:18 [PATCH v6 00/10] Refactor the SMMU's CD table ownership Michael Shavit
` (8 preceding siblings ...)
2023-08-16 13:18 ` [PATCH v6 09/10] iommu/arm-smmu-v3: Skip cd sync if CD table isn't active Michael Shavit
@ 2023-08-16 13:18 ` Michael Shavit
2023-08-16 23:59 ` [PATCH v6 00/10] Refactor the SMMU's CD table ownership Nicolin Chen
10 siblings, 0 replies; 22+ messages in thread
From: Michael Shavit @ 2023-08-16 13:18 UTC (permalink / raw)
To: iommu, linux-arm-kernel, linux-kernel
Cc: robin.murphy, will, jean-philippe, jgg, nicolinc, Michael Shavit
cdcfg is a confusing name, especially given other variables with the cfg
suffix in this driver. cd_table more clearly describes what is being
operated on.
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Reviewed-by: Nicolin Chen <nicolinc@nvidia.com>
Signed-off-by: Michael Shavit <mshavit@google.com>
---
(no changes since v3)
Changes in v3:
- Commit message update
Changes in v2:
- New commit
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 70 ++++++++++-----------
1 file changed, 35 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 104b8d6ea5972..b27011b2bec9b 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -1028,18 +1028,18 @@ static __le64 *arm_smmu_get_cd_ptr(struct arm_smmu_master *master, u32 ssid)
unsigned int idx;
struct arm_smmu_l1_ctx_desc *l1_desc;
struct arm_smmu_device *smmu = master->smmu;
- struct arm_smmu_ctx_desc_cfg *cdcfg = &master->cd_table;
+ struct arm_smmu_ctx_desc_cfg *cd_table = &master->cd_table;
- if (cdcfg->s1fmt == STRTAB_STE_0_S1FMT_LINEAR)
- return cdcfg->cdtab + ssid * CTXDESC_CD_DWORDS;
+ if (cd_table->s1fmt == STRTAB_STE_0_S1FMT_LINEAR)
+ return cd_table->cdtab + ssid * CTXDESC_CD_DWORDS;
idx = ssid >> CTXDESC_SPLIT;
- l1_desc = &cdcfg->l1_desc[idx];
+ l1_desc = &cd_table->l1_desc[idx];
if (!l1_desc->l2ptr) {
if (arm_smmu_alloc_cd_leaf_table(smmu, l1_desc))
return NULL;
- l1ptr = cdcfg->cdtab + idx * CTXDESC_L1_DESC_DWORDS;
+ l1ptr = cd_table->cdtab + idx * CTXDESC_L1_DESC_DWORDS;
arm_smmu_write_cd_l1_desc(l1ptr, l1_desc);
/* An invalid L1CD can be cached */
arm_smmu_sync_cd(master, ssid, false);
@@ -1134,35 +1134,35 @@ static int arm_smmu_alloc_cd_tables(struct arm_smmu_master *master)
size_t l1size;
size_t max_contexts;
struct arm_smmu_device *smmu = master->smmu;
- struct arm_smmu_ctx_desc_cfg *cdcfg = &master->cd_table;
+ struct arm_smmu_ctx_desc_cfg *cd_table = &master->cd_table;
- cdcfg->stall_enabled = master->stall_enabled;
- cdcfg->s1cdmax = master->ssid_bits;
- max_contexts = 1 << cdcfg->s1cdmax;
+ cd_table->stall_enabled = master->stall_enabled;
+ cd_table->s1cdmax = master->ssid_bits;
+ max_contexts = 1 << cd_table->s1cdmax;
if (!(smmu->features & ARM_SMMU_FEAT_2_LVL_CDTAB) ||
max_contexts <= CTXDESC_L2_ENTRIES) {
- cdcfg->s1fmt = STRTAB_STE_0_S1FMT_LINEAR;
- cdcfg->num_l1_ents = max_contexts;
+ cd_table->s1fmt = STRTAB_STE_0_S1FMT_LINEAR;
+ cd_table->num_l1_ents = max_contexts;
l1size = max_contexts * (CTXDESC_CD_DWORDS << 3);
} else {
- cdcfg->s1fmt = STRTAB_STE_0_S1FMT_64K_L2;
- cdcfg->num_l1_ents = DIV_ROUND_UP(max_contexts,
+ cd_table->s1fmt = STRTAB_STE_0_S1FMT_64K_L2;
+ cd_table->num_l1_ents = DIV_ROUND_UP(max_contexts,
CTXDESC_L2_ENTRIES);
- cdcfg->l1_desc = devm_kcalloc(smmu->dev, cdcfg->num_l1_ents,
- sizeof(*cdcfg->l1_desc),
+ cd_table->l1_desc = devm_kcalloc(smmu->dev, cd_table->num_l1_ents,
+ sizeof(*cd_table->l1_desc),
GFP_KERNEL);
- if (!cdcfg->l1_desc)
+ if (!cd_table->l1_desc)
return -ENOMEM;
- l1size = cdcfg->num_l1_ents * (CTXDESC_L1_DESC_DWORDS << 3);
+ l1size = cd_table->num_l1_ents * (CTXDESC_L1_DESC_DWORDS << 3);
}
- cdcfg->cdtab = dmam_alloc_coherent(smmu->dev, l1size, &cdcfg->cdtab_dma,
+ cd_table->cdtab = dmam_alloc_coherent(smmu->dev, l1size, &cd_table->cdtab_dma,
GFP_KERNEL);
- if (!cdcfg->cdtab) {
+ if (!cd_table->cdtab) {
dev_warn(smmu->dev, "failed to allocate context descriptor\n");
ret = -ENOMEM;
goto err_free_l1;
@@ -1171,9 +1171,9 @@ static int arm_smmu_alloc_cd_tables(struct arm_smmu_master *master)
return 0;
err_free_l1:
- if (cdcfg->l1_desc) {
- devm_kfree(smmu->dev, cdcfg->l1_desc);
- cdcfg->l1_desc = NULL;
+ if (cd_table->l1_desc) {
+ devm_kfree(smmu->dev, cd_table->l1_desc);
+ cd_table->l1_desc = NULL;
}
return ret;
}
@@ -1183,30 +1183,30 @@ static void arm_smmu_free_cd_tables(struct arm_smmu_master *master)
int i;
size_t size, l1size;
struct arm_smmu_device *smmu = master->smmu;
- struct arm_smmu_ctx_desc_cfg *cdcfg = &master->cd_table;
+ struct arm_smmu_ctx_desc_cfg *cd_table = &master->cd_table;
- if (cdcfg->l1_desc) {
+ if (cd_table->l1_desc) {
size = CTXDESC_L2_ENTRIES * (CTXDESC_CD_DWORDS << 3);
- for (i = 0; i < cdcfg->num_l1_ents; i++) {
- if (!cdcfg->l1_desc[i].l2ptr)
+ for (i = 0; i < cd_table->num_l1_ents; i++) {
+ if (!cd_table->l1_desc[i].l2ptr)
continue;
dmam_free_coherent(smmu->dev, size,
- cdcfg->l1_desc[i].l2ptr,
- cdcfg->l1_desc[i].l2ptr_dma);
+ cd_table->l1_desc[i].l2ptr,
+ cd_table->l1_desc[i].l2ptr_dma);
}
- devm_kfree(smmu->dev, cdcfg->l1_desc);
- cdcfg->l1_desc = NULL;
+ devm_kfree(smmu->dev, cd_table->l1_desc);
+ cd_table->l1_desc = NULL;
- l1size = cdcfg->num_l1_ents * (CTXDESC_L1_DESC_DWORDS << 3);
+ l1size = cd_table->num_l1_ents * (CTXDESC_L1_DESC_DWORDS << 3);
} else {
- l1size = cdcfg->num_l1_ents * (CTXDESC_CD_DWORDS << 3);
+ l1size = cd_table->num_l1_ents * (CTXDESC_CD_DWORDS << 3);
}
- dmam_free_coherent(smmu->dev, l1size, cdcfg->cdtab, cdcfg->cdtab_dma);
- cdcfg->cdtab_dma = 0;
- cdcfg->cdtab = NULL;
+ dmam_free_coherent(smmu->dev, l1size, cd_table->cdtab, cd_table->cdtab_dma);
+ cd_table->cdtab_dma = 0;
+ cd_table->cdtab = NULL;
}
bool arm_smmu_free_asid(struct arm_smmu_ctx_desc *cd)
--
2.41.0.694.ge786442a9b-goog
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH v6 08/10] iommu/arm-smmu-v3: Update comment about STE liveness
2023-08-16 13:18 ` [PATCH v6 08/10] iommu/arm-smmu-v3: Update comment about STE liveness Michael Shavit
@ 2023-08-16 14:00 ` Jason Gunthorpe
2023-08-16 23:46 ` Nicolin Chen
1 sibling, 0 replies; 22+ messages in thread
From: Jason Gunthorpe @ 2023-08-16 14:00 UTC (permalink / raw)
To: Michael Shavit
Cc: iommu, linux-arm-kernel, linux-kernel, robin.murphy, will,
jean-philippe, nicolinc
On Wed, Aug 16, 2023 at 09:18:48PM +0800, Michael Shavit wrote:
> Update the comment to reflect the fact that the STE is not always
> installed. arm_smmu_domain_finalise_s1 intentionnaly calls
> arm_smmu_write_ctx_desc while the STE is not installed.
>
> Signed-off-by: Michael Shavit <mshavit@google.com>
> ---
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Jason
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v6 05/10] iommu/arm-smmu-v3: Refactor write_ctx_desc
2023-08-16 13:18 ` [PATCH v6 05/10] iommu/arm-smmu-v3: Refactor write_ctx_desc Michael Shavit
@ 2023-08-16 22:36 ` Nicolin Chen
2023-08-17 8:00 ` Michael Shavit
0 siblings, 1 reply; 22+ messages in thread
From: Nicolin Chen @ 2023-08-16 22:36 UTC (permalink / raw)
To: Michael Shavit
Cc: iommu, linux-arm-kernel, linux-kernel, robin.murphy, will,
jean-philippe, jgg
On Wed, Aug 16, 2023 at 09:18:45PM +0800, Michael Shavit wrote:
> +/*
> + * Write the CD to the CD tables for all masters that this domain is attached
> + * to. Note that this is used to update the non-pasid CD entry when SVA takes
> + * over an existing ASID, as well as to write new pasid CD entries when
> + * attaching an SVA domain (although the domain passed as the parameter is the
> + * RID domain that this domain is mapped to).
> + */
> +static int arm_smmu_write_ctx_desc_devices(struct arm_smmu_domain *smmu_domain,
Iterating the entire device list of a domain looks like the
arm_smmu_atc_inv_domain(). So it feels to me that it could be
called arm_smmu_write_ctx_desc_domain()? Not a critical thing
though..
> + int ssid,
> + struct arm_smmu_ctx_desc *cd)
> +{
> + struct arm_smmu_master *master;
> + unsigned long flags;
> + int ret;
> +
> + spin_lock_irqsave(&smmu_domain->devices_lock, flags);
> + list_for_each_entry(master, &smmu_domain->devices, domain_head) {
> + ret = arm_smmu_write_ctx_desc(master, ssid, cd);
> + if (ret) {
> + list_for_each_entry_from_reverse(master, &smmu_domain->devices, domain_head)
> + arm_smmu_write_ctx_desc(master, ssid, NULL);
^
Here it always reverts back to NULL, which isn't ideal since
an CD entry could be something valid other than NULL prior to
this function call. IIUIC the conversation in v5, we'd need
another SVA series to clean up domain sharing, so this would
be cleaned up too after that? If so, perhaps we could note it
down in the comments above too.
> @@ -2458,8 +2446,7 @@ static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev)
> ret = -EINVAL;
> goto out_unlock;
> } else if (smmu_domain->stage == ARM_SMMU_DOMAIN_S1 &&
> - smmu_domain->cd_table.stall_enabled !=
> - master->stall_enabled) {
> + smmu_domain->cd_table.stall_enabled != master->stall_enabled) {
> ret = -EINVAL;
> goto out_unlock;
This doesn't seem to be a related change? Probably should be in
one of the previous patches, or just dropped.
Thanks
Nicolin
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v6 06/10] iommu/arm-smmu-v3: Move CD table to arm_smmu_master
2023-08-16 13:18 ` [PATCH v6 06/10] iommu/arm-smmu-v3: Move CD table to arm_smmu_master Michael Shavit
@ 2023-08-16 23:44 ` Nicolin Chen
2023-08-17 8:14 ` Michael Shavit
2023-08-17 0:50 ` Nicolin Chen
1 sibling, 1 reply; 22+ messages in thread
From: Nicolin Chen @ 2023-08-16 23:44 UTC (permalink / raw)
To: Michael Shavit
Cc: iommu, linux-arm-kernel, linux-kernel, robin.murphy, will,
jean-philippe, jgg
On Wed, Aug 16, 2023 at 09:18:46PM +0800, Michael Shavit wrote:
> Also add the device to the devices list before writing the CD to the
> table so that SVA is always aware of all the CD tables that need
> re-writing when it updates a CD.
Can you please elaborate this a bit more?
Thanks
Nicolin
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v6 07/10] iommu/arm-smmu-v3: Cleanup arm_smmu_domain_finalise
2023-08-16 13:18 ` [PATCH v6 07/10] iommu/arm-smmu-v3: Cleanup arm_smmu_domain_finalise Michael Shavit
@ 2023-08-16 23:45 ` Nicolin Chen
0 siblings, 0 replies; 22+ messages in thread
From: Nicolin Chen @ 2023-08-16 23:45 UTC (permalink / raw)
To: Michael Shavit
Cc: iommu, linux-arm-kernel, linux-kernel, robin.murphy, will,
jean-philippe, jgg
On Wed, Aug 16, 2023 at 09:18:47PM +0800, Michael Shavit wrote:
> Remove unused master parameter now that the CD table is allocated
> elsewhere.
>
> Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
> Signed-off-by: Michael Shavit <mshavit@google.com>
Reviewed-by: Nicolin Chen <nicolinc@nvidia.com>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v6 08/10] iommu/arm-smmu-v3: Update comment about STE liveness
2023-08-16 13:18 ` [PATCH v6 08/10] iommu/arm-smmu-v3: Update comment about STE liveness Michael Shavit
2023-08-16 14:00 ` Jason Gunthorpe
@ 2023-08-16 23:46 ` Nicolin Chen
1 sibling, 0 replies; 22+ messages in thread
From: Nicolin Chen @ 2023-08-16 23:46 UTC (permalink / raw)
To: Michael Shavit
Cc: iommu, linux-arm-kernel, linux-kernel, robin.murphy, will,
jean-philippe, jgg
On Wed, Aug 16, 2023 at 09:18:48PM +0800, Michael Shavit wrote:
> Update the comment to reflect the fact that the STE is not always
> installed. arm_smmu_domain_finalise_s1 intentionnaly calls
> arm_smmu_write_ctx_desc while the STE is not installed.
>
> Signed-off-by: Michael Shavit <mshavit@google.com>
Reviewed-by: Nicolin Chen <nicolinc@nvidia.com>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v6 00/10] Refactor the SMMU's CD table ownership
2023-08-16 13:18 [PATCH v6 00/10] Refactor the SMMU's CD table ownership Michael Shavit
` (9 preceding siblings ...)
2023-08-16 13:18 ` [PATCH v6 10/10] iommu/arm-smmu-v3: Rename cdcfg to cd_table Michael Shavit
@ 2023-08-16 23:59 ` Nicolin Chen
10 siblings, 0 replies; 22+ messages in thread
From: Nicolin Chen @ 2023-08-16 23:59 UTC (permalink / raw)
To: Michael Shavit
Cc: iommu, linux-arm-kernel, linux-kernel, robin.murphy, will,
jean-philippe, jgg
On Wed, Aug 16, 2023 at 09:18:40PM +0800, Michael Shavit wrote:
> Hi all,
>
> This series refactors stage 1 domains so that they describe a single CD
> entry. These entries are now inserted into a CD table that is owned by
> the arm_smmu_master instead of the domain.
> This is conceptually cleaner and unblocks other features, such as
> attaching domains with PASID (for unmanaged/dma domains).
>
> This patch series was originally part of a larger patch series that
> implemented the set_dev_pasid callback for non-SVA domains but is now
> split into a distinct series.
>
> This patch series is also available on gerrit with Jean's SMMU test
> engine patches cherry-picked on top for testing:
> https://linux-review.googlesource.com/c/linux/kernel/git/torvalds/linux/+/24770/9
>
> Thanks,
> Michael Shavit
>
> Changes in v6:
> - Undo removal of s1fmt and renaming of s1cdmax
> - Unwind the loop in amr_smmu_write_ctx_desc_devices to NULL out the CD
> entries we succesfully wrote on failure.
> - Add a comment clarifying the different usages of
> amr_smmu_write_ctx_desc_devices
> - Grab the asid lock while writing the RID CD to prevent a race with
> SVA.
> - Add the device to the devices list before writing the CD to the table
> and installing the CD table.
Though there's a thing in PATCH-6 that I couldn't fully understand
yet, SVA/ATS sanity works with the series. So,
Tested-by: Nicolin Chen <nicolinc@nvidia.com>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v6 06/10] iommu/arm-smmu-v3: Move CD table to arm_smmu_master
2023-08-16 13:18 ` [PATCH v6 06/10] iommu/arm-smmu-v3: Move CD table to arm_smmu_master Michael Shavit
2023-08-16 23:44 ` Nicolin Chen
@ 2023-08-17 0:50 ` Nicolin Chen
1 sibling, 0 replies; 22+ messages in thread
From: Nicolin Chen @ 2023-08-17 0:50 UTC (permalink / raw)
To: Michael Shavit
Cc: iommu, linux-arm-kernel, linux-kernel, robin.murphy, will,
jean-philippe, jgg
On Wed, Aug 16, 2023 at 09:18:46PM +0800, Michael Shavit wrote:
> @@ -2463,16 +2448,43 @@ static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev)
> if (smmu_domain->stage != ARM_SMMU_DOMAIN_BYPASS)
> master->ats_enabled = arm_smmu_ats_supported(master);
>
> - arm_smmu_install_ste_for_dev(master);
>
> spin_lock_irqsave(&smmu_domain->devices_lock, flags);
Nit: there'd be two empty lines in between; nicer to drop one.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v6 05/10] iommu/arm-smmu-v3: Refactor write_ctx_desc
2023-08-16 22:36 ` Nicolin Chen
@ 2023-08-17 8:00 ` Michael Shavit
0 siblings, 0 replies; 22+ messages in thread
From: Michael Shavit @ 2023-08-17 8:00 UTC (permalink / raw)
To: Nicolin Chen
Cc: iommu, linux-arm-kernel, linux-kernel, robin.murphy, will,
jean-philippe, jgg
On Thu, Aug 17, 2023 at 6:37 AM Nicolin Chen <nicolinc@nvidia.com> wrote:
>
> On Wed, Aug 16, 2023 at 09:18:45PM +0800, Michael Shavit wrote:
>
> > +/*
> > + * Write the CD to the CD tables for all masters that this domain is attached
> > + * to. Note that this is used to update the non-pasid CD entry when SVA takes
> > + * over an existing ASID, as well as to write new pasid CD entries when
> > + * attaching an SVA domain (although the domain passed as the parameter is the
> > + * RID domain that this domain is mapped to).
> > + */
> > +static int arm_smmu_write_ctx_desc_devices(struct arm_smmu_domain *smmu_domain,
>
> Iterating the entire device list of a domain looks like the
> arm_smmu_atc_inv_domain(). So it feels to me that it could be
> called arm_smmu_write_ctx_desc_domain()? Not a critical thing
> though..
>
> > + int ssid,
> > + struct arm_smmu_ctx_desc *cd)
> > +{
> > + struct arm_smmu_master *master;
> > + unsigned long flags;
> > + int ret;
> > +
> > + spin_lock_irqsave(&smmu_domain->devices_lock, flags);
> > + list_for_each_entry(master, &smmu_domain->devices, domain_head) {
> > + ret = arm_smmu_write_ctx_desc(master, ssid, cd);
> > + if (ret) {
> > + list_for_each_entry_from_reverse(master, &smmu_domain->devices, domain_head)
> > + arm_smmu_write_ctx_desc(master, ssid, NULL);
> ^
> Here it always reverts back to NULL, which isn't ideal since
> an CD entry could be something valid other than NULL prior to
> this function call. IIUIC the conversation in v5, we'd need
> another SVA series to clean up domain sharing, so this would
> be cleaned up too after that? If so, perhaps we could note it
> down in the comments above too.
Technically this is OK because it can only fail when an entirely new
CD entry is written. Honestly I'm tempted to revert the addition of
this helper function, especially as it ends up further complicated in
next patches since it has to deal with the fact that it's used to
update PASID-entries-on-RID-domains as well as the non pasid CD
entries for the asid-sharing case.
>
> > @@ -2458,8 +2446,7 @@ static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev)
> > ret = -EINVAL;
> > goto out_unlock;
> > } else if (smmu_domain->stage == ARM_SMMU_DOMAIN_S1 &&
> > - smmu_domain->cd_table.stall_enabled !=
> > - master->stall_enabled) {
> > + smmu_domain->cd_table.stall_enabled != master->stall_enabled) {
> > ret = -EINVAL;
> > goto out_unlock;
>
> This doesn't seem to be a related change? Probably should be in
> one of the previous patches, or just dropped.
Whoops, must have sneaked in during a rebase.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v6 06/10] iommu/arm-smmu-v3: Move CD table to arm_smmu_master
2023-08-16 23:44 ` Nicolin Chen
@ 2023-08-17 8:14 ` Michael Shavit
0 siblings, 0 replies; 22+ messages in thread
From: Michael Shavit @ 2023-08-17 8:14 UTC (permalink / raw)
To: Nicolin Chen
Cc: iommu, linux-arm-kernel, linux-kernel, robin.murphy, will,
jean-philippe, jgg
On Thu, Aug 17, 2023 at 7:44 AM Nicolin Chen <nicolinc@nvidia.com> wrote:
>
> On Wed, Aug 16, 2023 at 09:18:46PM +0800, Michael Shavit wrote:
>
> > Also add the device to the devices list before writing the CD to the
> > table so that SVA is always aware of all the CD tables that need
> > re-writing when it updates a CD.
>
> Can you please elaborate this a bit more?
Yeah I can update the message. Something like:
Also add the device to the devices list before writing the CD to the
table so that SVA will know that the CD needs to be re-written to this
device's CD table as well if it decides to update the CD's ASID
concurrently with this function.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v6 09/10] iommu/arm-smmu-v3: Skip cd sync if CD table isn't active
2023-08-16 13:18 ` [PATCH v6 09/10] iommu/arm-smmu-v3: Skip cd sync if CD table isn't active Michael Shavit
@ 2023-08-21 10:16 ` Michael Shavit
2023-08-21 10:42 ` Will Deacon
0 siblings, 1 reply; 22+ messages in thread
From: Michael Shavit @ 2023-08-21 10:16 UTC (permalink / raw)
To: iommu, linux-arm-kernel, linux-kernel, Will Deacon
Cc: robin.murphy, jean-philippe, jgg, nicolinc
On Wed, Aug 16, 2023 at 9:20 PM Michael Shavit <mshavit@google.com> wrote:
>
> This commit explicitly keeps track of whether a CD table is installed in
> an STE so that arm_smmu_sync_cd can skip the sync when unnecessary. This
> was previously achieved through the domain->devices list, but we are
> moving to a model where arm_smmu_sync_cd directly operates on a master
> and the master's CD table instead of a domain.
>
> Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
> Reviewed-by: Nicolin Chen <nicolinc@nvidia.com>
> Signed-off-by: Michael Shavit <mshavit@google.com>
> ---
> Happy to drop this commit if we don't think it's useful.
Hi Will,
Do you have any recommendations for keeping or dropping this commit in
the end? It's only purpose is to minimize any potential performance
impact from the refactor but can certainly be dropped if you don't
think it's worth the complication.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v6 09/10] iommu/arm-smmu-v3: Skip cd sync if CD table isn't active
2023-08-21 10:16 ` Michael Shavit
@ 2023-08-21 10:42 ` Will Deacon
0 siblings, 0 replies; 22+ messages in thread
From: Will Deacon @ 2023-08-21 10:42 UTC (permalink / raw)
To: Michael Shavit
Cc: iommu, linux-arm-kernel, linux-kernel, robin.murphy,
jean-philippe, jgg, nicolinc
On Mon, Aug 21, 2023 at 06:16:08PM +0800, Michael Shavit wrote:
> On Wed, Aug 16, 2023 at 9:20 PM Michael Shavit <mshavit@google.com> wrote:
> >
> > This commit explicitly keeps track of whether a CD table is installed in
> > an STE so that arm_smmu_sync_cd can skip the sync when unnecessary. This
> > was previously achieved through the domain->devices list, but we are
> > moving to a model where arm_smmu_sync_cd directly operates on a master
> > and the master's CD table instead of a domain.
> >
> > Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
> > Reviewed-by: Nicolin Chen <nicolinc@nvidia.com>
> > Signed-off-by: Michael Shavit <mshavit@google.com>
> > ---
> > Happy to drop this commit if we don't think it's useful.
>
> Hi Will,
> Do you have any recommendations for keeping or dropping this commit in
> the end? It's only purpose is to minimize any potential performance
> impact from the refactor but can certainly be dropped if you don't
> think it's worth the complication.
I'd prefer to drop it unless somebody can measure the benefits that it
brings.
Will
^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2023-08-21 10:42 UTC | newest]
Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-08-16 13:18 [PATCH v6 00/10] Refactor the SMMU's CD table ownership Michael Shavit
2023-08-16 13:18 ` [PATCH v6 01/10] iommu/arm-smmu-v3: Move ctx_desc out of s1_cfg Michael Shavit
2023-08-16 13:18 ` [PATCH v6 02/10] iommu/arm-smmu-v3: Replace s1_cfg with cdtab_cfg Michael Shavit
2023-08-16 13:18 ` [PATCH v6 03/10] iommu/arm-smmu-v3: Encapsulate ctx_desc_cfg init in alloc_cd_tables Michael Shavit
2023-08-16 13:18 ` [PATCH v6 04/10] iommu/arm-smmu-v3: move stall_enabled to the cd table Michael Shavit
2023-08-16 13:18 ` [PATCH v6 05/10] iommu/arm-smmu-v3: Refactor write_ctx_desc Michael Shavit
2023-08-16 22:36 ` Nicolin Chen
2023-08-17 8:00 ` Michael Shavit
2023-08-16 13:18 ` [PATCH v6 06/10] iommu/arm-smmu-v3: Move CD table to arm_smmu_master Michael Shavit
2023-08-16 23:44 ` Nicolin Chen
2023-08-17 8:14 ` Michael Shavit
2023-08-17 0:50 ` Nicolin Chen
2023-08-16 13:18 ` [PATCH v6 07/10] iommu/arm-smmu-v3: Cleanup arm_smmu_domain_finalise Michael Shavit
2023-08-16 23:45 ` Nicolin Chen
2023-08-16 13:18 ` [PATCH v6 08/10] iommu/arm-smmu-v3: Update comment about STE liveness Michael Shavit
2023-08-16 14:00 ` Jason Gunthorpe
2023-08-16 23:46 ` Nicolin Chen
2023-08-16 13:18 ` [PATCH v6 09/10] iommu/arm-smmu-v3: Skip cd sync if CD table isn't active Michael Shavit
2023-08-21 10:16 ` Michael Shavit
2023-08-21 10:42 ` Will Deacon
2023-08-16 13:18 ` [PATCH v6 10/10] iommu/arm-smmu-v3: Rename cdcfg to cd_table Michael Shavit
2023-08-16 23:59 ` [PATCH v6 00/10] Refactor the SMMU's CD table ownership Nicolin Chen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).