[PATCH V2] iommu/arm-smmu-v2: Workaround for ThunderX errata#27704
Will Deacon
will.deacon at arm.com
Tue Feb 23 03:49:50 PST 2016
On Thu, Feb 18, 2016 at 10:29:18AM -0800, tchalamarla at caviumnetworks.com wrote:
> From: Tirumalesh Chalamarla <tchalamarla at caviumnetworks.com>
>
> Due to Errata#27704 CN88xx SMMUv2,supports only shared ASID and VMID
> namespaces; specifically within a given node SMMU0 and SMMU1 share,
> as does SMMU2 and SMMU3.
>
> This patch tries to address these issuee by supplying asid and vmid
> base from devicetree.
>
> changes from V1:
> - rebased on top of 16 bit VMID patch
> - removed redundent options from DT
> - insted of transform, DT now supplies starting ASID/VMID
>
> Signed-off-by: Akula Geethasowjanya <Geethasowjanya.Akula at caviumnetworks.com>
> Signed-off-by: Tirumalesh Chalamarla <tchalamarla at caviumnetworks.com>
> ---
> .../devicetree/bindings/iommu/arm,smmu.txt | 8 +++++
> drivers/iommu/arm-smmu.c | 37 +++++++++++++++-------
> 2 files changed, 34 insertions(+), 11 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.txt b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
> index bb7e569..80b8484 100644
> --- a/Documentation/devicetree/bindings/iommu/arm,smmu.txt
> +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
> @@ -57,6 +57,14 @@ conditions.
>
> - smmu-enable-vmid16 : Enable 16 bit VMID, if allowed.
>
> +- asid-base : Buggy SMMUv2 implementations which doesn't satisfy the
> + ASID namespace needs, use this field to specify starting
> + ASID for the SMMU.
Can you make this cavium,asid-base please?
> +
> +- vmid-base : Buggy SMMUv2 implementations which doesn't satisfy the VMID
> + namespace needs, use this field to specify starting VMID
> + for the SMMU.
Similarly here.
> +
> Example:
>
> smmu {
> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> index 003c442..dc46b9a 100644
> --- a/drivers/iommu/arm-smmu.c
> +++ b/drivers/iommu/arm-smmu.c
> @@ -320,6 +320,9 @@ struct arm_smmu_device {
> unsigned long ipa_size;
> unsigned long pa_size;
>
> + u32 asid_base;
> + u32 vmid_base;
> +
> u32 num_global_irqs;
> u32 num_context_irqs;
> unsigned int *irqs;
> @@ -335,8 +338,8 @@ struct arm_smmu_cfg {
> };
> #define INVALID_IRPTNDX 0xff
>
> -#define ARM_SMMU_CB_ASID(cfg) ((cfg)->cbndx)
> -#define ARM_SMMU_CB_VMID(cfg) ((cfg)->cbndx + 1)
> +#define ARM_SMMU_CB_ASID(smmu, cfg) ((u16)((smmu)->asid_base + (cfg)->cbndx))
> +#define ARM_SMMU_CB_VMID(smmu, cfg) ((u16)((smmu)->vmid_base + (cfg)->cbndx))
Why are you removing the +1 from the macro and then adding it to all the
callers instead? I also think we should be performing a range-check
somewhere so that we don't silently truncate values (e.g. if we try to
program more than 8 bits on a system with 8-bit VMIDs).
>
> enum arm_smmu_domain_stage {
> ARM_SMMU_DOMAIN_S1 = 0,
> @@ -576,11 +579,11 @@ static void arm_smmu_tlb_inv_context(void *cookie)
>
> if (stage1) {
> base = ARM_SMMU_CB_BASE(smmu) + ARM_SMMU_CB(smmu, cfg->cbndx);
> - writel_relaxed(ARM_SMMU_CB_ASID(cfg),
> + writel_relaxed(ARM_SMMU_CB_ASID(smmu, cfg),
> base + ARM_SMMU_CB_S1_TLBIASID);
> } else {
> base = ARM_SMMU_GR0(smmu);
> - writel_relaxed(ARM_SMMU_CB_VMID(cfg),
> + writel_relaxed(ARM_SMMU_CB_VMID(smmu, cfg),
> base + ARM_SMMU_GR0_TLBIVMID);
> }
>
> @@ -602,7 +605,7 @@ static void arm_smmu_tlb_inv_range_nosync(unsigned long iova, size_t size,
>
> if (!IS_ENABLED(CONFIG_64BIT) || smmu->version == ARM_SMMU_V1) {
> iova &= ~12UL;
> - iova |= ARM_SMMU_CB_ASID(cfg);
> + iova |= ARM_SMMU_CB_ASID(smmu, cfg);
> do {
> writel_relaxed(iova, reg);
> iova += granule;
> @@ -610,7 +613,7 @@ static void arm_smmu_tlb_inv_range_nosync(unsigned long iova, size_t size,
> #ifdef CONFIG_64BIT
> } else {
> iova >>= 12;
> - iova |= (u64)ARM_SMMU_CB_ASID(cfg) << 48;
> + iova |= (u64)ARM_SMMU_CB_ASID(smmu, cfg) << 48;
> do {
> writeq_relaxed(iova, reg);
> iova += granule >> 12;
> @@ -630,7 +633,7 @@ static void arm_smmu_tlb_inv_range_nosync(unsigned long iova, size_t size,
> #endif
> } else {
> reg = ARM_SMMU_GR0(smmu) + ARM_SMMU_GR0_TLBIVMID;
> - writel_relaxed(ARM_SMMU_CB_VMID(cfg), reg);
> + writel_relaxed(ARM_SMMU_CB_VMID(smmu, cfg), reg);
> }
> }
>
> @@ -744,7 +747,7 @@ static void arm_smmu_init_context_bank(struct arm_smmu_domain *smmu_domain,
> #endif
> /* if 16bit VMID required set VMID in CBA2R */
> if (smmu->options & ARM_SMMU_OPT_ENABLE_VMID16)
> - reg |= ARM_SMMU_CB_VMID(cfg) << CBA2R_VMID_SHIFT;
> + reg |= ARM_SMMU_CB_VMID(smmu, cfg) << CBA2R_VMID_SHIFT;
>
> writel_relaxed(reg, gr1_base + ARM_SMMU_GR1_CBA2R(cfg->cbndx));
> }
> @@ -763,7 +766,7 @@ static void arm_smmu_init_context_bank(struct arm_smmu_domain *smmu_domain,
> (CBAR_S1_MEMATTR_WB << CBAR_S1_MEMATTR_SHIFT);
> } else if (!(smmu->options & ARM_SMMU_OPT_ENABLE_VMID16)) {
> /*16 bit VMID is not enabled set 8 bit VMID here */
> - reg |= ARM_SMMU_CB_VMID(cfg) << CBAR_VMID_SHIFT;
> + reg |= ARM_SMMU_CB_VMID(smmu, cfg) << CBAR_VMID_SHIFT;
> }
> writel_relaxed(reg, gr1_base + ARM_SMMU_GR1_CBAR(cfg->cbndx));
>
> @@ -771,11 +774,11 @@ static void arm_smmu_init_context_bank(struct arm_smmu_domain *smmu_domain,
> if (stage1) {
> reg64 = pgtbl_cfg->arm_lpae_s1_cfg.ttbr[0];
>
> - reg64 |= ((u64)ARM_SMMU_CB_ASID(cfg)) << TTBRn_ASID_SHIFT;
> + reg64 |= ((u64)ARM_SMMU_CB_ASID(smmu, cfg)) << TTBRn_ASID_SHIFT;
> smmu_writeq(reg64, cb_base + ARM_SMMU_CB_TTBR0);
>
> reg64 = pgtbl_cfg->arm_lpae_s1_cfg.ttbr[1];
> - reg64 |= ((u64)ARM_SMMU_CB_ASID(cfg)) << TTBRn_ASID_SHIFT;
> + reg64 |= ((u64)ARM_SMMU_CB_ASID(smmu, cfg)) << TTBRn_ASID_SHIFT;
> smmu_writeq(reg64, cb_base + ARM_SMMU_CB_TTBR1);
> } else {
> reg64 = pgtbl_cfg->arm_lpae_s2_cfg.vttbr;
> @@ -1812,6 +1815,18 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev)
>
> parse_driver_options(smmu);
>
> + if (of_property_read_u32(dev->of_node, "#asid-base",
> + &smmu->asid_base)) {
> + smmu->asid_base = 0;
The smmu structure is zeroed on allocation, so no need for this.
Will
More information about the linux-arm-kernel
mailing list