Messages in this thread | | | Date | Tue, 27 Jun 2023 10:00:18 +0100 | Subject | Re: [PATCH v1] iommu/arm-smmu-v3: Allow default substream bypass with a pasid support | From | Robin Murphy <> |
| |
On 2023-06-27 04:33, Nicolin Chen wrote: > When an iommu_domain is set to IOMMU_DOMAIN_IDENTITY, the driver would > skip the allocation of a CD table and set the CONFIG field of the STE > to STRTAB_STE_0_CFG_BYPASS. This works well for devices that only have > one substream, i.e. PASID disabled. > > However, there could be a use case, for a pasid capable device, that > allows bypassing the translation at the default substream while still > enabling the pasid feature, which means the driver should not skip the > allocation of a CD table nor simply bypass the CONFIG field. Instead, > the S1DSS field should be set to STRTAB_STE_1_S1DSS_BYPASS and the > SHCFG field should be set to STRTAB_STE_1_SHCFG_INCOMING. > > Add s1dss and shcfg in struct arm_smmu_s1_cfg, to allow configurations > in the finalise() to support that use case. Then, set them accordingly > depending on the iommu_domain->type and the master->ssid_bits. > > Also, add STRTAB_STE_1_SHCFG_NONSHAREABLE of the default configuration > to distinguish from STRTAB_STE_1_SHCFG_INCOMING of the bypass one.
Why? The "default configuration" is that the S1 shareability attribute is determined by the S1 translation itself, so the incoming value is irrelevant.
> Signed-off-by: Nicolin Chen <nicolinc@nvidia.com> > --- > drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 18 +++++++++++++++--- > drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 3 +++ > 2 files changed, 18 insertions(+), 3 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 9b0dc3505601..8dc7934a0175 100644 > --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c > +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c > @@ -1350,11 +1350,12 @@ static void arm_smmu_write_strtab_ent(struct arm_smmu_master *master, u32 sid, > > BUG_ON(ste_live); > dst[1] = cpu_to_le64( > - FIELD_PREP(STRTAB_STE_1_S1DSS, STRTAB_STE_1_S1DSS_SSID0) | > + FIELD_PREP(STRTAB_STE_1_S1DSS, s1_cfg->s1dss) | > FIELD_PREP(STRTAB_STE_1_S1CIR, STRTAB_STE_1_S1C_CACHE_WBRA) | > FIELD_PREP(STRTAB_STE_1_S1COR, STRTAB_STE_1_S1C_CACHE_WBRA) | > FIELD_PREP(STRTAB_STE_1_S1CSH, ARM_SMMU_SH_ISH) | > - FIELD_PREP(STRTAB_STE_1_STRW, strw)); > + FIELD_PREP(STRTAB_STE_1_STRW, strw) | > + FIELD_PREP(STRTAB_STE_1_SHCFG, s1_cfg->shcfg)); > > if (smmu->features & ARM_SMMU_FEAT_STALLS && > !master->stall_enabled) > @@ -2119,6 +2120,13 @@ static int arm_smmu_domain_finalise_s1(struct arm_smmu_domain *smmu_domain, > goto out_unlock; > > cfg->s1cdmax = master->ssid_bits; > + if (smmu_domain->domain.type == IOMMU_DOMAIN_IDENTITY) { > + cfg->s1dss = STRTAB_STE_1_S1DSS_BYPASS; > + cfg->shcfg = STRTAB_STE_1_SHCFG_INCOMING; > + } else { > + cfg->s1dss = STRTAB_STE_1_S1DSS_SSID0; > + cfg->shcfg = STRTAB_STE_1_SHCFG_NONSHAREABLE; > + } > > smmu_domain->stall_enabled = master->stall_enabled; > > @@ -2198,7 +2206,11 @@ static int arm_smmu_domain_finalise(struct iommu_domain *domain, > struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain); > struct arm_smmu_device *smmu = smmu_domain->smmu; > > - if (domain->type == IOMMU_DOMAIN_IDENTITY) { > + /* > + * A master with a pasid capability might need a CD table, so only set > + * ARM_SMMU_DOMAIN_BYPASS if IOMMU_DOMAIN_IDENTITY and non-pasid master > + */ > + if (domain->type == IOMMU_DOMAIN_IDENTITY && !master->ssid_bits) { > smmu_domain->stage = ARM_SMMU_DOMAIN_BYPASS; > return 0; > }
This means we'll now go on to allocate a pagetable for an identity domain, which doesn't seem ideal :/
Thanks, Robin.
> 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 dcab85698a4e..8052d02770d0 100644 > --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h > +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h > @@ -244,6 +244,7 @@ > #define STRTAB_STE_1_STRW_EL2 2UL > > #define STRTAB_STE_1_SHCFG GENMASK_ULL(45, 44) > +#define STRTAB_STE_1_SHCFG_NONSHAREABLE 0UL > #define STRTAB_STE_1_SHCFG_INCOMING 1UL > > #define STRTAB_STE_2_S2VMID GENMASK_ULL(15, 0) > @@ -601,6 +602,8 @@ struct arm_smmu_s1_cfg { > struct arm_smmu_ctx_desc_cfg cdcfg; > struct arm_smmu_ctx_desc cd; > u8 s1fmt; > + u8 s1dss; > + u8 shcfg; > u8 s1cdmax; > }; >
| |