[v1,7/7] iommu/arm-smmu-v3: Move CD table to arm_smmu_master
Commit Message
Each master is now allocated a CD table at probe time, and attaching a
stage 1 domain now 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.
Signed-off-by: Michael Shavit <mshavit@google.com>
---
v4->v5: 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 | 65 +++++++++------------
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 4 +-
2 files changed, 28 insertions(+), 41 deletions(-)
Comments
On Fri, Jul 28, 2023 at 02:26:23AM +0800, Michael Shavit wrote:
> Each master is now allocated a CD table at probe time,
Currently it is allocated during arm_smmu_domain_finalise_s1(), so it
isn't allocated at probe time.
I think the right place to put the allocation is during the attach op,
the first time we need a CD table then go and allocate it. If we can't
then domain attach fails with -ENOMEM.
Then you can put the free in a detach op once the CD table becomes
empty and it behaves much like it already does.
Jason
On Fri, Jul 28, 2023 at 2:43 AM Jason Gunthorpe <jgg@nvidia.com> wrote:
>
> On Fri, Jul 28, 2023 at 02:26:23AM +0800, Michael Shavit wrote:
> > Each master is now allocated a CD table at probe time,
>
> Currently it is allocated during arm_smmu_domain_finalise_s1(), so it
> isn't allocated at probe time.
Urh right, I meant that this patch moves the allocation to the probe,
but that was misleading wording for sure.
> I think the right place to put the allocation is during the attach op,
> the first time we need a CD table then go and allocate it. If we can't
> then domain attach fails with -ENOMEM.
>
> Then you can put the free in a detach op once the CD table becomes
> empty and it behaves much like it already does.
Hmmm fair. I can't think of a reason that the table *must* be
pre-allocated for the PASID feature, and that could always be a
different commit if it turns out to be necessary.
On Fri, Jul 28, 2023 at 2:43 AM Jason Gunthorpe <jgg@nvidia.com> wrote:
> Then you can put the free in a detach op once the CD table becomes
> empty and it behaves much like it already does.
This turns out to be a bit tricky; the SMMU driver detaches the
currently attached domain whenever a new domain is attached with
attach_dev(). More generally, do we really want to be de-allocating
the table whenever we switch between an S1 domain and other domain
types that don't make use of the table (such as IDENTITY or NESTED)?
One solution is to defer the allocation to the first attach_op, but
only free when the master is freed (this patch's v1 behavior).
On Fri, Jul 28, 2023 at 07:11:46PM +0800, Michael Shavit wrote:
> On Fri, Jul 28, 2023 at 2:43 AM Jason Gunthorpe <jgg@nvidia.com> wrote:
>
> > Then you can put the free in a detach op once the CD table becomes
> > empty and it behaves much like it already does.
>
> This turns out to be a bit tricky; the SMMU driver detaches the
> currently attached domain whenever a new domain is attached with
> attach_dev().
Oh, yeah, it is a bit quirky that way
> More generally, do we really want to be de-allocating
> the table whenever we switch between an S1 domain and other domain
> types that don't make use of the table (such as IDENTITY or NESTED)?
Probably.
> One solution is to defer the allocation to the first attach_op, but
> only free when the master is freed (this patch's v1 behavior).
That seems reasonable, just don't allocate it at probe time since
PASID is very rarely used.
Jason
@@ -985,7 +985,7 @@ static void arm_smmu_sync_cd(struct arm_smmu_master *master,
},
};
- if (!master->domain->cd_table.installed)
+ if (!master->cd_table.installed)
return;
smmu = master->smmu;
@@ -1028,7 +1028,7 @@ 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_ctx_desc_cfg *cdcfg = &master->domain->cd_table;
+ struct arm_smmu_ctx_desc_cfg *cdcfg = &master->cd_table;
if (!cdcfg->l1_desc)
return cdcfg->cdtab + ssid * CTXDESC_CD_DWORDS;
@@ -1065,7 +1065,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->max_cds_bits)))
return -E2BIG;
@@ -1128,14 +1128,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->max_cds_bits = master->ssid_bits;
@@ -1177,12 +1176,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);
@@ -1290,7 +1289,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:
@@ -2081,14 +2080,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 *cdcfg = &smmu_domain->cd_table;
-
/* Prevent SVA from touching the CD while we're freeing it */
mutex_lock(&arm_smmu_asid_lock);
- if (cdcfg->cdtab)
- arm_smmu_free_cd_tables(smmu_domain);
arm_smmu_free_asid(&smmu_domain->cd);
mutex_unlock(&arm_smmu_asid_lock);
} else {
@@ -2100,7 +2095,7 @@ static void arm_smmu_domain_free(struct iommu_domain *domain)
kfree(smmu_domain);
}
-static int arm_smmu_domain_finalise_s1(struct arm_smmu_domain *smmu_domain,
+static int arm_smmu_domain_finalise_cd(struct arm_smmu_domain *smmu_domain,
struct arm_smmu_master *master,
struct io_pgtable_cfg *pgtbl_cfg)
{
@@ -2119,10 +2114,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) |
@@ -2134,17 +2125,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;
@@ -2207,7 +2190,7 @@ static int arm_smmu_domain_finalise(struct iommu_domain *domain,
ias = min_t(unsigned long, ias, VA_BITS);
oas = smmu->ias;
fmt = ARM_64_LPAE_S1;
- finalise_stage_fn = arm_smmu_domain_finalise_s1;
+ finalise_stage_fn = arm_smmu_domain_finalise_cd;
break;
case ARM_SMMU_DOMAIN_NESTED:
case ARM_SMMU_DOMAIN_S2:
@@ -2447,16 +2430,7 @@ static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev)
} 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.max_cds_bits) {
- 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;
}
-
master->domain = smmu_domain;
/*
@@ -2469,6 +2443,14 @@ 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);
+ if (smmu_domain->stage == ARM_SMMU_DOMAIN_S1) {
+ ret = arm_smmu_write_ctx_desc(master, 0, &smmu_domain->cd);
+ if (ret) {
+ master->domain = NULL;
+ goto out_unlock;
+ }
+ }
+
arm_smmu_install_ste_for_dev(master);
spin_lock_irqsave(&smmu_domain->devices_lock, flags);
@@ -2706,6 +2688,12 @@ static struct iommu_device *arm_smmu_probe_device(struct device *dev)
smmu->features & ARM_SMMU_FEAT_STALL_FORCE)
master->stall_enabled = true;
+ ret = arm_smmu_alloc_cd_tables(master);
+ if (ret) {
+ arm_smmu_disable_pasid(master);
+ arm_smmu_remove_master(master);
+ goto err_free_master;
+ }
return &smmu->iommu;
err_free_master:
@@ -2723,6 +2711,7 @@ static void arm_smmu_release_device(struct device *dev)
arm_smmu_detach_dev(master);
arm_smmu_disable_pasid(master);
arm_smmu_remove_master(master);
+ arm_smmu_free_cd_tables(master);
kfree(master);
}
@@ -696,6 +696,7 @@ struct arm_smmu_master {
struct arm_smmu_domain *domain;
struct list_head domain_head;
struct arm_smmu_stream *streams;
+ struct arm_smmu_ctx_desc_cfg cd_table;
unsigned int num_streams;
bool ats_enabled;
bool stall_enabled;
@@ -722,10 +723,7 @@ 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;
};