iommu/arm-smmu-v2: Workaround for ThunderX errata#27704
Subsystems (3)
PATCH v2 1/1 iommu/arm-smmu-v2: Workaround for ThunderX errata#27704
From: Tirumalesh Chalamarla <[email protected]>
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
--- .../devicetree/bindings/iommu/arm,smmu.txt | 8 +++++ drivers/iommu/arm-smmu.c | 37 +++++++++++++++------- 2 files changed, 34 insertions(+), 11 deletions(-)
Documentation/devicetree/bindings/iommu/arm,smmu.txt +8 -0
@@ -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. + +- vmid-base : Buggy SMMUv2 implementations which doesn't satisfy the VMID + namespace needs, use this field to specify starting VMID + for the SMMU. + Example: smmu {
drivers/iommu/arm-smmu.c +26 -11
@@ -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))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 ; + } + +if (of_property_read_u32 (dev->of_node,"#vmid-base" , + &smmu->vmid_base)) { + smmu->vmid_base =1 ; + } +if (smmu->vmid_base ==0 ) + smmu->vmid_base =1 ; +if (smmu->version >ARM_SMMU_V1 && smmu->num_context_banks != smmu->num_context_irqs) { dev_err(dev,
Signature · 1 line
2.1.0
PATCH v2 1/1 iommu/arm-smmu-v2: Workaround for ThunderX errata#27704
Quoted 26 lines · tchalamarla at caviumnetworks.com
From: Tirumalesh Chalamarla <[email protected]>
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/VMIDSigned-off-by Akula Geethasowjanya <[email protected]>Signed-off-by Tirumalesh Chalamarla <[email protected]>--- .../devicetree/bindings/iommu/arm,smmu.txt | 8 +++++ drivers/iommu/arm-smmu.c | 37 +++++++++++++++------- 2 files changed, 34 insertions(+), 11 deletions(-)Documentation/devicetree/bindings/iommu/arm,smmu.txt +0
-0
Documentation/devicetree/bindings/iommu/arm,smmu.txt conditions. · line 62
Quoted 6 lines
Documentation/devicetree/bindings/iommu/arm,smmu.txt +3
-0@@ -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?
Documentation/devicetree/bindings/iommu/arm,smmu.txt conditions. · line 66
Quoted 4 lines
Documentation/devicetree/bindings/iommu/arm,smmu.txt +4
-0+ +- 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.
Quoted 19 lines
Documentation/devicetree/bindings/iommu/arm,smmu.txt +1
-0+ Example: smmu {drivers/iommu/arm-smmu.c +3
-0@@ -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;
drivers/iommu/arm-smmu.c struct arm_smmu_cfg { · line 342
Quoted 7 lines
drivers/iommu/arm-smmu.c +2
-2@@ -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).
Quoted 77 lines
drivers/iommu/arm-smmu.c +9
-9enum 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;
drivers/iommu/arm-smmu.c static int arm_smmu_device_dt_probe(struct platform_device *pdev) · line 1820
Quoted 6 lines
drivers/iommu/arm-smmu.c +3
-0@@ -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
Robin Murphy
reviewerPATCH v2 1/1 iommu/arm-smmu-v2: Workaround for ThunderX errata#27704
In reply to tchalamarla at caviumnetworks.comQuoted 27 lines · tchalamarla at caviumnetworks.com
From: Tirumalesh Chalamarla <[email protected]>
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/VMIDSigned-off-by Akula Geethasowjanya <[email protected]>Signed-off-by Tirumalesh Chalamarla <[email protected]>--- .../devicetree/bindings/iommu/arm,smmu.txt | 8 +++++ drivers/iommu/arm-smmu.c | 37 +++++++++++++++------- 2 files changed, 34 insertions(+), 11 deletions(-)Documentation/devicetree/bindings/iommu/arm,smmu.txt +0
-0@@ -57,6 +57,14 @@ conditions.
Documentation/devicetree/bindings/iommu/arm,smmu.txt conditions. · line 67
Quoted 10 lines
Documentation/devicetree/bindings/iommu/arm,smmu.txt +8
-0- 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. + +- vmid-base : Buggy SMMUv2 implementations which doesn't satisfy the VMID + namespace needs, use this field to specify starting VMID + for the SMMU. +
From how you've described the situation, this is still just some
arbitrary number for the sake of a driver workaround and not an actual property of the hardware. Either way there should be a proper compatible string for this implementation, e.g. something like "cavium,cn88xx-smmu".
Given that, you then shouldn't actually need anything else, because you can simply keep track of how many Cavium SMMUs have been probed and automatically generate a suitable offset, e.g. (smmu->index * ARM_SMMU_MAX_CBS) to ensure global ASID/VMID uniqueness.
Robin.
Quoted 121 lines
Documentation/devicetree/bindings/iommu/arm,smmu.txt +0
-0Example: smmu {drivers/iommu/arm-smmu.c +26
-11@@ -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))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 ; + } + +if (of_property_read_u32 (dev->of_node,"#vmid-base" , + &smmu->vmid_base)) { + smmu->vmid_base =1 ; + } +if (smmu->vmid_base ==0 ) + smmu->vmid_base =1 ; +if (smmu->version >ARM_SMMU_V1 && smmu->num_context_banks != smmu->num_context_irqs) { dev_err(dev,
PATCH v2 1/1 iommu/arm-smmu-v2: Workaround for ThunderX errata#27704
Quoted 48 lines · Robin Murphy
Quoted 38 lines · tchalamarla at caviumnetworks.com
From: Tirumalesh Chalamarla <[email protected]>
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/VMIDSigned-off-by: Akula Geethasowjanya
<[email protected]>Signed-off-by Tirumalesh Chalamarla <[email protected]>--- .../devicetree/bindings/iommu/arm,smmu.txt | 8 +++++ drivers/iommu/arm-smmu.c | 37 +++++++++++++++------- 2 files changed, 34 insertions(+), 11 deletions(-)Documentation/devicetree/bindings/iommu/arm,smmu.txt +5
-0@@ -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. + +- vmid-base : Buggy SMMUv2 implementations which doesn't satisfythe VMID
Documentation/devicetree/bindings/iommu/arm,smmu.txt conditions. · line 67
Quoted 3 lines
Documentation/devicetree/bindings/iommu/arm,smmu.txt +3
-0+ namespace needs, use this field to specify starting VMID + for the SMMU. +From how you've described the situation, this is still just some
arbitrary number for the sake of a driver workaround and not an actual property of the hardware. Either way there should be a proper compatible string for this implementation, e.g. something like "cavium,cn88xx-smmu".
Quoted 4 lines
Given that, you then shouldn't actually need anything else, because you can simply keep track of how many Cavium SMMUs have been probed and automatically generate a suitable offset, e.g. (
smmu->index*ARM_SMMU_MAX_CBS) to ensure global ASID/VMID uniqueness.
This particular solution might not be accurate in all cases. what happens if SMMU stage 1 is used by a guest?
Quoted 135 lines
Robin.
Quoted 131 lines
Example:
smmu {drivers/iommu/arm-smmu.c +5
-2@@ -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))enum arm_smmu_domain_stage { ARM_SMMU_DOMAIN_S1 = 0,drivers/iommu/arm-smmu.c +3
-3@@ -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;drivers/iommu/arm-smmu.c +1
-1@@ -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;drivers/iommu/arm-smmu.c +2
-2@@ -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));}drivers/iommu/arm-smmu.c +3
-3@@ -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(structarm_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;drivers/iommu/arm-smmu.c +12
-0@@ -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 ; + } + +if (of_property_read_u32 (dev->of_node,"#vmid-base" , + &smmu->vmid_base)) { + smmu->vmid_base =1 ; + } +if (smmu->vmid_base ==0 ) + smmu->vmid_base =1 ; +if (smmu->version >ARM_SMMU_V1 && smmu->num_context_banks != smmu->num_context_irqs) {dev_err(dev,
Robin Murphy
reviewerPATCH v2 1/1 iommu/arm-smmu-v2: Workaround for ThunderX errata#27704
Quoted 51 lines · Tirumalesh Chalamarla
Quoted 48 lines · Robin Murphy
Quoted 38 lines · tchalamarla at caviumnetworks.com
From: Tirumalesh Chalamarla <[email protected]>
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/VMIDSigned-off-by: Akula Geethasowjanya
<[email protected]>Signed-off-by Tirumalesh Chalamarla <[email protected]>--- .../devicetree/bindings/iommu/arm,smmu.txt | 8 +++++ drivers/iommu/arm-smmu.c | 37 +++++++++++++++------- 2 files changed, 34 insertions(+), 11 deletions(-)Documentation/devicetree/bindings/iommu/arm,smmu.txt +5
-0@@ -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. + +- vmid-base : Buggy SMMUv2 implementations which doesn't satisfythe VMID
Documentation/devicetree/bindings/iommu/arm,smmu.txt conditions. · line 67
Quoted 3 lines
Documentation/devicetree/bindings/iommu/arm,smmu.txt +3
-0+ namespace needs, use this field to specify starting VMID + for the SMMU. +From how you've described the situation, this is still just some
arbitrary number for the sake of a driver workaround and not an actual property of the hardware. Either way there should be a proper compatible string for this implementation, e.g. something like "cavium,cn88xx-smmu".
Quoted 7 lines
Quoted 4 lines
Given that, you then shouldn't actually need anything else, because you can simply keep track of how many Cavium SMMUs have been probed and automatically generate a suitable offset, e.g. (
smmu->index*ARM_SMMU_MAX_CBS) to ensure global ASID/VMID uniqueness.This particular solution might not be accurate in all cases. what happens if SMMU stage 1 is used by a guest?
Even if we supported passing through stage 1 contexts, which we don't, a malicious guest can freely ignore your arbitrary numbers in the DT, put whatever nonsense it feels like in the TTBRs, and corrupt I/O in other guests and/or the host.
You cannot safely pass through raw stage 1 contexts on this hardware, so it's a non-issue.
Robin.
Quoted 136 lines
Quoted 135 lines
Quoted 134 lines
Example:
smmu {drivers/iommu/arm-smmu.c +5
-2@@ -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))enum arm_smmu_domain_stage { ARM_SMMU_DOMAIN_S1 = 0,drivers/iommu/arm-smmu.c +3
-3@@ -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;drivers/iommu/arm-smmu.c +1
-1@@ -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;drivers/iommu/arm-smmu.c +2
-2@@ -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)); }drivers/iommu/arm-smmu.c +3
-3@@ -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(structarm_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;drivers/iommu/arm-smmu.c +12
-0@@ -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 ; + } + +if (of_property_read_u32 (dev->of_node,"#vmid-base" , + &smmu->vmid_base)) { + smmu->vmid_base =1 ; + } +if (smmu->vmid_base ==0 ) + smmu->vmid_base =1 ; +if (smmu->version >ARM_SMMU_V1 && smmu->num_context_banks != smmu->num_context_irqs) {dev_err(dev,
PATCH v2 1/1 iommu/arm-smmu-v2: Workaround for ThunderX errata#27704
Quoted 62 lines · Robin Murphy
Quoted 53 lines · Tirumalesh Chalamarla
Quoted 50 lines · Robin Murphy
Quoted 39 lines · tchalamarla at caviumnetworks.com
From: Tirumalesh Chalamarla <[email protected]>
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/VMIDSigned-off-by: Akula Geethasowjanya
<[email protected]>Signed-off-by Tirumalesh Chalamarla <[email protected]>--- .../devicetree/bindings/iommu/arm,smmu.txt | 8 +++++ drivers/iommu/arm-smmu.c | 37 +++++++++++++++------- 2 files changed, 34 insertions(+), 11 deletions(-)Documentation/devicetree/bindings/iommu/arm,smmu.txt +1
-0@@ -57,6 +57,14 @@ conditions. - smmu-enable-vmid16 : Enable 16 bit VMID, if allowed. +- asid-base : Buggy SMMUv2 implementations which doesn't satisfythe
Documentation/devicetree/bindings/iommu/arm,smmu.txt +4
-0+ ASID namespace needs, use this field to specify starting + ASID for the SMMU. + +- vmid-base : Buggy SMMUv2 implementations which doesn't satisfythe VMID
Documentation/devicetree/bindings/iommu/arm,smmu.txt conditions. · line 67
Quoted 3 lines
Documentation/devicetree/bindings/iommu/arm,smmu.txt +3
-0+ namespace needs, use this field to specify starting VMID + for the SMMU. +From how you've described the situation, this is still just some
arbitrary number for the sake of a driver workaround and not an actual property of the hardware. Either way there should be a proper compatible string for this implementation, e.g. something like "cavium,cn88xx-smmu".Quoted 7 lines
Quoted 4 lines
Given that, you then shouldn't actually need anything else, because you can simply keep track of how many Cavium SMMUs have been probed and automatically generate a suitable offset, e.g. (
smmu->index*ARM_SMMU_MAX_CBS) to ensure global ASID/VMID uniqueness.This particular solution might not be accurate in all cases. what happens if SMMU stage 1 is used by a guest?
Quoted 8 lines
Even if we supported passing through stage 1 contexts, which we don't, a malicious guest can freely ignore your arbitrary numbers in the DT, put whatever nonsense it feels like in the TTBRs, and corrupt I/O in other guests and/or the host.
You cannot safely pass through raw stage 1 contexts on this hardware, so it's a non-issue.
Agreed. will re post based on the suggestions.
Quoted 141 lines
Robin.
Quoted 138 lines
Quoted 137 lines
Quoted 136 lines
Example:
smmu {drivers/iommu/arm-smmu.c +5
-2@@ -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))enum arm_smmu_domain_stage { ARM_SMMU_DOMAIN_S1 = 0,drivers/iommu/arm-smmu.c +2
-2@@ -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 );}drivers/iommu/arm-smmu.c +1
-1@@ -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;drivers/iommu/arm-smmu.c +1
-1@@ -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;drivers/iommu/arm-smmu.c +2
-2@@ -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)); }drivers/iommu/arm-smmu.c +3
-3@@ -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(structarm_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;drivers/iommu/arm-smmu.c +12
-0@@ -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 ; + } + +if (of_property_read_u32 (dev->of_node,"#vmid-base" , + &smmu->vmid_base)) { + smmu->vmid_base =1 ; + } +if (smmu->vmid_base ==0 ) + smmu->vmid_base =1 ; +if (smmu->version >ARM_SMMU_V1 && smmu->num_context_banks != smmu->num_context_irqs) {dev_err(dev,
PATCH v2 1/1 iommu/arm-smmu-v2: Workaround for ThunderX errata#27704
In reply to tchalamarla at caviumnetworks.comQuoted 26 lines · tchalamarla at caviumnetworks.com
From: Tirumalesh Chalamarla <[email protected]>
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/VMIDSigned-off-by Akula Geethasowjanya <[email protected]>Signed-off-by Tirumalesh Chalamarla <[email protected]>--- .../devicetree/bindings/iommu/arm,smmu.txt | 8 +++++ drivers/iommu/arm-smmu.c | 37 +++++++++++++++------- 2 files changed, 34 insertions(+), 11 deletions(-)Documentation/devicetree/bindings/iommu/arm,smmu.txt +0
-0
Documentation/devicetree/bindings/iommu/arm,smmu.txt conditions. · line 66
Quoted 10 lines
Documentation/devicetree/bindings/iommu/arm,smmu.txt +7
-0@@ -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. + +- vmid-base : Buggy SMMUv2 implementations which doesn't satisfy the VMID + namespace needs, use this field to specify starting VMID + for the SMMU.
As has been pointed out, these are not strictly properties of the hardware, and are insufficient to aovid the issue in general (adding an arbitrary base does not enforce IDs fall within a particular range).
So NAK for *-base properties alone.
Documentation/devicetree/bindings/iommu/arm,smmu.txt conditions.
Quoted 9 lines
Documentation/devicetree/bindings/iommu/arm,smmu.txt +9
-0+ if (of_property_read_u32(dev->of_node, "#asid-base", + &smmu->asid_base)) { + smmu->asid_base = 0; + } + + if (of_property_read_u32(dev->of_node, "#vmid-base", + &smmu->vmid_base)) { + smmu->vmid_base = 1; + }
These do not match the documentation above.
Mark.
PATCH v2 1/1 iommu/arm-smmu-v2: Workaround for ThunderX errata#27704
in Summary,
if i change asid-base to cavium,asid-base and still use DT for supplying base value, is this a solution that will be accepted, of course i will do range check to see we are not supplying 16bit VMID for 8 bit systems even though the property now indicates Cavium only.
Thanks,
Tirumalesh.
Quoted 58 lines · Mark Rutland
Quoted 26 lines · tchalamarla at caviumnetworks.com
From: Tirumalesh Chalamarla <[email protected]>
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/VMIDSigned-off-by Akula Geethasowjanya <[email protected]>Signed-off-by Tirumalesh Chalamarla <[email protected]>--- .../devicetree/bindings/iommu/arm,smmu.txt | 8 +++++ drivers/iommu/arm-smmu.c | 37 +++++++++++++++------- 2 files changed, 34 insertions(+), 11 deletions(-)Documentation/devicetree/bindings/iommu/arm,smmu.txt +0
-0Documentation/devicetree/bindings/iommu/arm,smmu.txt conditions. · line 66
Quoted 10 lines
Documentation/devicetree/bindings/iommu/arm,smmu.txt +7
-0@@ -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. + +- vmid-base : Buggy SMMUv2 implementations which doesn't satisfy the VMID + namespace needs, use this field to specify starting VMID + for the SMMU.As has been pointed out, these are not strictly properties of the hardware, and are insufficient to aovid the issue in general (adding an arbitrary base does not enforce IDs fall within a particular range).
So NAK for *-base properties alone.
Documentation/devicetree/bindings/iommu/arm,smmu.txt conditions.
Quoted 9 lines
Documentation/devicetree/bindings/iommu/arm,smmu.txt +9
-0+ if (of_property_read_u32(dev->of_node, "#asid-base", + &smmu->asid_base)) { + smmu->asid_base = 0; + } + + if (of_property_read_u32(dev->of_node, "#vmid-base", + &smmu->vmid_base)) { + smmu->vmid_base = 1; + }These do not match the documentation above.
Mark.
PATCH v2 1/1 iommu/arm-smmu-v2: Workaround for ThunderX errata#27704
Quoted 4 lines · Tirumalesh Chalamarla
in Summary,
if i change asid-base to cavium,asid-base and still use DT for supplying base value, is this a solution that will be accepted,
No. The property is _insufficient_, regardless of its name. This has been pointed out more than once.
A base alone does not tell you what set of IDs it is valid to use without risking a clash. The OS is well within its rights to allocate _any_ ID above that base. It is not a requirement of the hardware nor the binding that the OS allocate a contiguous set of IDs starting at the base.
Consider:
smmu_a {
whatver,*id-base = <128>;
};smmu_b {
whatever,*id-base = <64>;
};In both cases, the *IDs 129+ could be allocated on both SMMUs.
Mark.
Quoted 66 lines
of course i will do range check to see we are not supplying 16bit VMID for 8 bit systems even though the property now indicates Cavium only.
Thanks,
Tirumalesh.Quoted 58 lines · Mark Rutland
Quoted 26 lines · tchalamarla at caviumnetworks.com
From: Tirumalesh Chalamarla <[email protected]>
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/VMIDSigned-off-by Akula Geethasowjanya <[email protected]>Signed-off-by Tirumalesh Chalamarla <[email protected]>--- .../devicetree/bindings/iommu/arm,smmu.txt | 8 +++++ drivers/iommu/arm-smmu.c | 37 +++++++++++++++------- 2 files changed, 34 insertions(+), 11 deletions(-)Documentation/devicetree/bindings/iommu/arm,smmu.txt +0
-0Documentation/devicetree/bindings/iommu/arm,smmu.txt conditions. · line 66
Quoted 10 lines
Documentation/devicetree/bindings/iommu/arm,smmu.txt +7
-0@@ -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. + +- vmid-base : Buggy SMMUv2 implementations which doesn't satisfy the VMID + namespace needs, use this field to specify starting VMID + for the SMMU.As has been pointed out, these are not strictly properties of the hardware, and are insufficient to aovid the issue in general (adding an arbitrary base does not enforce IDs fall within a particular range).
So NAK for *-base properties alone.
Documentation/devicetree/bindings/iommu/arm,smmu.txt conditions.
Quoted 9 lines
Documentation/devicetree/bindings/iommu/arm,smmu.txt +9
-0+ if (of_property_read_u32(dev->of_node, "#asid-base", + &smmu->asid_base)) { + smmu->asid_base = 0; + } + + if (of_property_read_u32(dev->of_node, "#vmid-base", + &smmu->vmid_base)) { + smmu->vmid_base = 1; + }These do not match the documentation above.
Mark.
PATCH v2 1/1 iommu/arm-smmu-v2: Workaround for ThunderX errata#27704
Quoted 21 lines · Mark Rutland
Quoted 4 lines · Tirumalesh Chalamarla
in Summary,
if i change asid-base to cavium,asid-base and still use DT for supplying base value, is this a solution that will be accepted,
No. The property is _insufficient_, regardless of its name. This has been pointed out more than once.
A base alone does not tell you what set of IDs it is valid to use without risking a clash. The OS is well within its rights to allocate _any_ ID above that base. It is not a requirement of the hardware nor the binding that the OS allocate a contiguous set of IDs starting at the base.
Consider:
smmu_a { whatver,*id-base = <128>; };
Quoted 5 lines
smmu_b { whatever,*id-base = <64>; };In both cases, the *IDs 129+ could be allocated on both SMMUs.
Does adding a check to see if base + number of contexts supported will not overlap with each other Make it acceptable.
Or do you want the size be provided from DT also?
Thanks,
Tirumalesh.
Quoted 69 lines
Mark.
Quoted 66 lines
of course i will do range check to see we are not supplying 16bit VMID for 8 bit systems even though the property now indicates Cavium only.
Thanks,
Tirumalesh.Quoted 58 lines · Mark Rutland
Quoted 26 lines · tchalamarla at caviumnetworks.com
From: Tirumalesh Chalamarla <[email protected]>
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/VMIDSigned-off-by Akula Geethasowjanya <[email protected]>Signed-off-by Tirumalesh Chalamarla <[email protected]>--- .../devicetree/bindings/iommu/arm,smmu.txt | 8 +++++ drivers/iommu/arm-smmu.c | 37 +++++++++++++++------- 2 files changed, 34 insertions(+), 11 deletions(-)Documentation/devicetree/bindings/iommu/arm,smmu.txt +0
-0Documentation/devicetree/bindings/iommu/arm,smmu.txt conditions. · line 66
Quoted 10 lines
Documentation/devicetree/bindings/iommu/arm,smmu.txt +7
-0@@ -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. + +- vmid-base : Buggy SMMUv2 implementations which doesn't satisfy the VMID + namespace needs, use this field to specify starting VMID + for the SMMU.As has been pointed out, these are not strictly properties of the hardware, and are insufficient to aovid the issue in general (adding an arbitrary base does not enforce IDs fall within a particular range).
So NAK for *-base properties alone.
Documentation/devicetree/bindings/iommu/arm,smmu.txt conditions.
Quoted 9 lines
Documentation/devicetree/bindings/iommu/arm,smmu.txt +9
-0+ if (of_property_read_u32(dev->of_node, "#asid-base", + &smmu->asid_base)) { + smmu->asid_base = 0; + } + + if (of_property_read_u32(dev->of_node, "#vmid-base", + &smmu->vmid_base)) { + smmu->vmid_base = 1; + }These do not match the documentation above.
Mark.
PATCH v2 1/1 iommu/arm-smmu-v2: Workaround for ThunderX errata#27704
Quoted 28 lines · Tirumalesh
Quoted 25 lines · Mark Rutland
Quoted 4 lines · Tirumalesh Chalamarla
in Summary,
if i change asid-base to cavium,asid-base and still use DT for supplying base value, is this a solution that will be accepted,
No. The property is _insufficient_, regardless of its name. This has been pointed out more than once.
A base alone does not tell you what set of IDs it is valid to use without risking a clash. The OS is well within its rights to allocate _any_ ID above that base. It is not a requirement of the hardware nor the binding that the OS allocate a contiguous set of IDs starting at the base.
Consider:
smmu_a { whatver,*id-base = <128>; };smmu_b { whatever,*id-base = <64>; };
Quoted 5 lines
Quoted 1 line
In both cases, the *IDs 129+ could be allocated on both SMMUs.
Does adding a check to see if base + number of contexts supported will not overlap with each other Make it acceptable.
Or do you want the size be provided from DT also?
At this point I think Robin's suggestion of giving the ThunderX SMMU a different compatible string and treating that as a separate case entirely is the best thing to do.
Then the only requirement is that _all_ the ThunderX SMMUs with shared TLBs are under the control of the OS, and it can allocate IDs as it chooses, so long as it ensures that there are no conflicts across all the SMMUs it is in control of.
Mark.
PATCH v2 1/1 iommu/arm-smmu-v2: Workaround for ThunderX errata#27704
Quoted 35 lines · Mark Rutland
Quoted 28 lines · Tirumalesh
Quoted 25 lines · Mark Rutland
Quoted 4 lines · Tirumalesh Chalamarla
in Summary,
if i change asid-base to cavium,asid-base and still use DT for supplying base value, is this a solution that will be accepted,
No. The property is _insufficient_, regardless of its name. This has been pointed out more than once.
A base alone does not tell you what set of IDs it is valid to use without risking a clash. The OS is well within its rights to allocate _any_ ID above that base. It is not a requirement of the hardware nor the binding that the OS allocate a contiguous set of IDs starting at the base.
Consider:
smmu_a { whatver,*id-base = <128>; };smmu_b { whatever,*id-base = <64>; };Quoted 5 lines
Quoted 1 line
In both cases, the *IDs 129+ could be allocated on both SMMUs.
Does adding a check to see if base + number of contexts supported will not overlap with each other Make it acceptable.
Or do you want the size be provided from DT also?
Quoted 9 lines
At this point I think Robin's suggestion of giving the ThunderX SMMU a different compatible string and treating that as a separate case entirely is the best thing to do.
Then the only requirement is that _all_ the ThunderX SMMUs with shared TLBs are under the control of the OS, and it can allocate IDs as it chooses, so long as it ensures that there are no conflicts across all the SMMUs it is in control of.
yes, resending based on the suggestions.
Quoted 2 lines
Mark.
PATCH v2 1/1 iommu/arm-smmu-v2: Workaround for ThunderX errata#27704
In reply to Mark RutlandQuoted 30 lines · Mark Rutland
Quoted 29 lines · tchalamarla at caviumnetworks.com
From: Tirumalesh Chalamarla <[email protected]>
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/VMIDSigned-off-by Akula Geethasowjanya <[email protected]>Signed-off-by Tirumalesh Chalamarla <[email protected]>--- .../devicetree/bindings/iommu/arm,smmu.txt | 8 +++++ drivers/iommu/arm-smmu.c | 37 +++++++++++++++------- 2 files changed, 34 insertions(+), 11 deletions(-)Documentation/devicetree/bindings/iommu/arm,smmu.txt +0
-0@@ -57,6 +57,14 @@ conditions. - smmu-enable-vmid16 : Enable 16 bit VMID, if allowed.
Quoted 12 lines
Documentation/devicetree/bindings/iommu/arm,smmu.txt conditions. · line 66
Quoted 7 lines
Documentation/devicetree/bindings/iommu/arm,smmu.txt +7
-0+- asid-base : Buggy SMMUv2 implementations which doesn't satisfy the + ASID namespace needs, use this field to specify starting + ASID for the SMMU. + +- vmid-base : Buggy SMMUv2 implementations which doesn't satisfy the VMID + namespace needs, use this field to specify starting VMID + for the SMMU.As has been pointed out, these are not strictly properties of the hardware, and are insufficient to aovid the issue in general (adding an arbitrary base does not enforce IDs fall within a particular range).
pardon my ignorance, are you saying what happens if DT don't know what values to send?
Quoted 16 lines
So NAK for *-base properties alone.
Documentation/devicetree/bindings/iommu/arm,smmu.txt conditions.
Quoted 9 lines
Documentation/devicetree/bindings/iommu/arm,smmu.txt +9
-0+ if (of_property_read_u32(dev->of_node, "#asid-base", + &smmu->asid_base)) { + smmu->asid_base = 0; + } + + if (of_property_read_u32(dev->of_node, "#vmid-base", + &smmu->vmid_base)) { + smmu->vmid_base = 1; + }These do not match the documentation above.
Mark.