iommu/arm-smmu-v2: Workaround for ThunderX errata#27704
Subsystems (3)
PATCH 1/1 iommu/arm-smmu-v2: Workaround for ThunderX errata#27704
From: Tirumalesh Chalamarla <[email protected]>
An issue exists whereby the Linux kernel requires that ASIDs are a unique namespace per SMMU. That ASID-local choice conflicts with the CN88xx SMMU, where only shared ASID namespaces are supported; specifically within a given node SMMU0 and SMMU1 share, as does SMMU2 and SMMU3. CN88xx SMMU also requires global VMIDs.
This patch tries to address these issuee by supplying asid and vmid transformations from devicetree.
--- .../devicetree/bindings/iommu/arm,smmu.txt | 16 ++++++ drivers/iommu/arm-smmu.c | 59 ++++++++++++++++++---- 2 files changed, 65 insertions(+), 10 deletions(-)
Documentation/devicetree/bindings/iommu/arm,smmu.txt +16 -0
@@ -55,6 +55,22 @@ conditions. aliases of secure registers have to be used during SMMU configuration. +- thunderx,smmu-asid-transform : Enable proper handling for buggy + implementations that require special transformations + for smmu asid. if this property exists asid-transform + property must be present. + +- thunderx,smmu-vmid-transform : Enable proper handling for buggy + implementations that require special transformations + for smmu vmid. if this property exists vmid-transform + property must be present. + +- asid-transform : Transform mask that needs to be applied to asid. + This property has to be specified as '/bits/ 8' value. + +- vmid-transform : Transform mask that needs to be applied to vmid. + This property has to be specified as '/bits/ 8' value. + Example: smmu {
drivers/iommu/arm-smmu.c +49 -10
127 lines not on this page · raw message
Signature · 1 line
2.1.0
PATCH 1/1 iommu/arm-smmu-v2: Workaround for ThunderX errata#27704
Quoted 4 lines · tchalamarla at caviumnetworks.com
From: Tirumalesh Chalamarla <[email protected]>
An issue exists whereby the Linux kernel requires that ASIDs are a unique namespace per SMMU.
The SMMU architecture requires this, and Linux relies on hardware following the architecture.
Please describe the problem correctly. i.e. that the hardware erroneously shares TLBs between SMMU instances.
Quoted 28 lines
That ASID-local choice conflicts with the
CN88xx SMMU, where only shared ASID namespaces are supported; specifically within a given node SMMU0 and SMMU1 share, as does SMMU2 and SMMU3. CN88xx SMMU also requires global VMIDs.This patch tries to address these issuee by supplying asid and vmid transformations from devicetree.
Signed-off-by Akula Geethasowjanya <[email protected]>Signed-off-by Tirumalesh Chalamarla <[email protected]>--- .../devicetree/bindings/iommu/arm,smmu.txt | 16 ++++++ drivers/iommu/arm-smmu.c | 59 ++++++++++++++++++---- 2 files changed, 65 insertions(+), 10 deletions(-)Documentation/devicetree/bindings/iommu/arm,smmu.txt +5
-0@@ -55,6 +55,22 @@ conditions. aliases of secure registers have to be used during SMMU configuration. +- thunderx,smmu-asid-transform : Enable proper handling for buggy + implementations that require special transformations + for smmu asid. if this property exists asid-transform + property must be present. +
Documentation/devicetree/bindings/iommu/arm,smmu.txt conditions. · line 72
Quoted 10 lines
Documentation/devicetree/bindings/iommu/arm,smmu.txt +10
-0+- thunderx,smmu-vmid-transform : Enable proper handling for buggy + implementations that require special transformations + for smmu vmid. if this property exists vmid-transform + property must be present. + +- asid-transform : Transform mask that needs to be applied to asid. + This property has to be specified as '/bits/ 8' value. + +- vmid-transform : Transform mask that needs to be applied to vmid. + This property has to be specified as '/bits/ 8' value.
Don't bother with /bits/ 8. It saves no space in the DTB and only adds to confusion. Use a full cell, and of_property_read_u32, and validate that the value is in range.
Having two properties for each seems redundant. The presence of a mask should be sufficient to make it clear that the use of the mask is required.
These properties aren't sufficiently described. How is the mask "applied", and what is it applied to? What constraints does it impose on the OS's selection of ASIDs/VMIDs?
Quoted 1 line
From the looks of the patch, it's ANDed into the value Linux has
selected, so it sounds like an OS must avoid using any bits set in any mask on any SMMU instance. That sounds very difficult to ensure in general, and very fragile.
Mark.
Quoted 132 lines
Documentation/devicetree/bindings/iommu/arm,smmu.txt +1
-0+ Example: smmu {drivers/iommu/arm-smmu.c +39
-10@@ -300,8 +300,12 @@ struct arm_smmu_device {u32 features;#define ARM_SMMU_OPT_SECURE_CFG_ACCESS (1 << 0) +#define ARM_SMMU_OPT_TRANSFORM_ASID (1 << 1) +#define ARM_SMMU_OPT_TRANSFORM_VMID (1 << 2)u32 options;enum arm_smmu_arch_version version; +u8 asid_transform; +u8 vmid_transform;u32 num_context_banks;u32 num_s2_context_banks;@@ -330,8 +334,25 @@ 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) +/* +* ThunderX has a unique requirement because of ERRATA#27704 +* An issue exists whereby the Linux kernel requires that ASIDs are a +* unique namespace per SMMU. That ASID-local choice conflicts with the +* CN88xx SMMU, where only shared ASID namespaces are supported; +* specifically within a given node SMMU0 and SMMU1 share, as does SMMU2 +* and SMMU3. +* CN88xx SMMU also requires global VMIDs. +*/ + +#define ARM_SMMU_CB_ASID (smmu, cfg) \ + (((smmu)->options & ARM_SMMU_OPT_TRANSFORM_ASID) \ + ? ((cfg)->cbndx | (smmu)->asid_transform) \ + : ((cfg)->cbndx)) + +#define ARM_SMMU_CB_VMID (smmu, cfg) \ + (((smmu)->options & ARM_SMMU_OPT_TRANSFORM_VMID) \ + ? (((cfg)->cbndx + 1) | (smmu)->vmid_transform) \ + : ((cfg)->cbndx + 1))enum arm_smmu_domain_stage {ARM_SMMU_DOMAIN_S1 =0 ,@@ -361,6 +382,8 @@ struct arm_smmu_option_prop {static struct arm_smmu_option_prop arm_smmu_options[] = { {ARM_SMMU_OPT_SECURE_CFG_ACCESS ,"calxeda,smmu-secure-config-access" }, + {ARM_SMMU_OPT_TRANSFORM_ASID ,"thunderx,smmu-asid-transform" }, + {ARM_SMMU_OPT_TRANSFORM_VMID ,"thunderx,smmu-vmid-transform" }, {0 ,NULL }, };@@ -570,11 +593,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 ); }@@ -596,7 +619,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;@@ -604,7 +627,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;@@ -624,7 +647,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); } }@@ -752,7 +775,7 @@ static void arm_smmu_init_context_bank(struct arm_smmu_domain *smmu_domain, reg |= (CBAR_S1_BPSHCFG_NSH <<CBAR_S1_BPSHCFG_SHIFT ) | (CBAR_S1_MEMATTR_WB <<CBAR_S1_MEMATTR_SHIFT ); }else { - 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));@@ -760,11 +783,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;@@ -1793,6 +1816,22 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev)parse_driver_options (smmu); +if (smmu->options &ARM_SMMU_OPT_TRANSFORM_ASID ) { +if (of_property_read_u8 (dev->of_node,"#asid-transform" , + &smmu->asid_transform)) { +dev_err (dev,"missing #asid-transform property\n" ); +return -ENODEV ; + }
drivers/iommu/arm-smmu.c static int arm_smmu_device_dt_probe(struct platform_device *pdev) · line 1834
Quoted 10 lines
drivers/iommu/arm-smmu.c +10
-0+ } + +if (smmu->options &ARM_SMMU_OPT_TRANSFORM_VMID ) { +if (of_property_read_u8 (dev->of_node,"#vmid-transform" , + &smmu->vmid_transform)) { +dev_err (dev,"missing #vmid-transform property\n" ); +return -ENODEV ; + } + } +
` if (smmu->version > ARM_SMMU_V1 &&
> smmu->num_context_banks != smmu->num_context_irqs) {
> dev_err(dev,Quoted 9 lines
Signature · 7 lines
2.1.0 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel at lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
PATCH 1/1 iommu/arm-smmu-v2: Workaround for ThunderX errata#27704
Quoted 59 lines · Mark Rutland
Quoted 4 lines · tchalamarla at caviumnetworks.com
From: Tirumalesh Chalamarla <[email protected]>
An issue exists whereby the Linux kernel requires that ASIDs are a unique namespace per SMMU.
The SMMU architecture requires this, and Linux relies on hardware following the architecture.
Please describe the problem correctly. i.e. that the hardware erroneously shares TLBs between SMMU instances.
Quoted 28 lines
That ASID-local choice conflicts with the
CN88xx SMMU, where only shared ASID namespaces are supported; specifically within a given node SMMU0 and SMMU1 share, as does SMMU2 and SMMU3. CN88xx SMMU also requires global VMIDs.This patch tries to address these issuee by supplying asid and vmid transformations from devicetree.
Signed-off-by Akula Geethasowjanya <[email protected]>Signed-off-by Tirumalesh Chalamarla <[email protected]>--- .../devicetree/bindings/iommu/arm,smmu.txt | 16 ++++++ drivers/iommu/arm-smmu.c | 59 ++++++++++++++++++---- 2 files changed, 65 insertions(+), 10 deletions(-)Documentation/devicetree/bindings/iommu/arm,smmu.txt +5
-0@@ -55,6 +55,22 @@ conditions. aliases of secure registers have to be used during SMMU configuration. +- thunderx,smmu-asid-transform : Enable proper handling for buggy + implementations that require special transformations + for smmu asid. if this property exists asid-transform + property must be present. +Documentation/devicetree/bindings/iommu/arm,smmu.txt conditions. · line 72
Quoted 10 lines
Documentation/devicetree/bindings/iommu/arm,smmu.txt +10
-0+- thunderx,smmu-vmid-transform : Enable proper handling for buggy + implementations that require special transformations + for smmu vmid. if this property exists vmid-transform + property must be present. + +- asid-transform : Transform mask that needs to be applied to asid. + This property has to be specified as '/bits/ 8' value. + +- vmid-transform : Transform mask that needs to be applied to vmid. + This property has to be specified as '/bits/ 8' value.Don't bother with /bits/ 8. It saves no space in the DTB and only adds to confusion. Use a full cell, and
of_property_read_u32, and validate that the value is in range.Having two properties for each seems redundant. The presence of a mask should be sufficient to make it clear that the use of the mask is required.
Quoted 8 lines
These properties aren't sufficiently described. How is the mask "applied", and what is it applied to? What constraints does it impose on the OS's selection of ASIDs/VMIDs?
From the looks of the patch, it's ANDed into the value Linux has selected, so it sounds like an OS must avoid using any bits set in any mask on any SMMU instance. That sounds very difficult to ensure in general, and very fragile.
A much better approach would be to have something like:
valid-asid-range : two u32 cells describing the minimum and maximum
ASIDs that may be used on this SMMU instance. The OS must allocate ASIDs in the interval [minimum, maximum].
That way the OS can prevent itself from allocating conflicting ASIDs, which is not possible with a single base value as you have with your asid-transform property.
Mark.
PATCH 1/1 iommu/arm-smmu-v2: Workaround for ThunderX errata#27704
Quoted 71 lines · Mark Rutland
Quoted 59 lines · Mark Rutland
Quoted 4 lines · tchalamarla at caviumnetworks.com
From: Tirumalesh Chalamarla <[email protected]>
An issue exists whereby the Linux kernel requires that ASIDs are a unique namespace per SMMU.
The SMMU architecture requires this, and Linux relies on hardware following the architecture.
Please describe the problem correctly. i.e. that the hardware erroneously shares TLBs between SMMU instances.
Quoted 28 lines
That ASID-local choice conflicts with the
CN88xx SMMU, where only shared ASID namespaces are supported; specifically within a given node SMMU0 and SMMU1 share, as does SMMU2 and SMMU3. CN88xx SMMU also requires global VMIDs.This patch tries to address these issuee by supplying asid and vmid transformations from devicetree.
Signed-off-by Akula Geethasowjanya <[email protected]>Signed-off-by Tirumalesh Chalamarla <[email protected]>--- .../devicetree/bindings/iommu/arm,smmu.txt | 16 ++++++ drivers/iommu/arm-smmu.c | 59 ++++++++++++++++++---- 2 files changed, 65 insertions(+), 10 deletions(-)Documentation/devicetree/bindings/iommu/arm,smmu.txt +5
-0@@ -55,6 +55,22 @@ conditions. aliases of secure registers have to be used during SMMU configuration. +- thunderx,smmu-asid-transform : Enable proper handling for buggy + implementations that require special transformations + for smmu asid. if this property exists asid-transform + property must be present. +Documentation/devicetree/bindings/iommu/arm,smmu.txt conditions. · line 72
Quoted 10 lines
Documentation/devicetree/bindings/iommu/arm,smmu.txt +10
-0+- thunderx,smmu-vmid-transform : Enable proper handling for buggy + implementations that require special transformations + for smmu vmid. if this property exists vmid-transform + property must be present. + +- asid-transform : Transform mask that needs to be applied to asid. + This property has to be specified as '/bits/ 8' value. + +- vmid-transform : Transform mask that needs to be applied to vmid. + This property has to be specified as '/bits/ 8' value.Don't bother with /bits/ 8. It saves no space in the DTB and only adds to confusion. Use a full cell, and
of_property_read_u32, and validate that the value is in range.Having two properties for each seems redundant. The presence of a mask should be sufficient to make it clear that the use of the mask is required.
Quoted 8 lines
These properties aren't sufficiently described. How is the mask "applied", and what is it applied to? What constraints does it impose on the OS's selection of ASIDs/VMIDs?
From the looks of the patch, it's ANDed into the value Linux has selected, so it sounds like an OS must avoid using any bits set in any mask on any SMMU instance. That sounds very difficult to ensure in general, and very fragile.
A much better approach would be to have something like:
Quoted 8 lines
valid-asid-range : two u32 cells describing the minimum and maximum
ASIDs that may be used on this SMMU instance. The OS
must allocate ASIDs in the interval [minimum, maximum].That way the OS can prevent itself from allocating conflicting ASIDs, which is not possible with a single base value as you have with your asid-transform property.
The problem is, Will don?t want this to be part of general change, he want to treat it as Errata.
Quoted 2 lines
Mark.
PATCH 1/1 iommu/arm-smmu-v2: Workaround for ThunderX errata#27704
In reply to Mark RutlandQuoted 55 lines · Mark Rutland
Quoted 4 lines · tchalamarla at caviumnetworks.com
From: Tirumalesh Chalamarla <[email protected]>
An issue exists whereby the Linux kernel requires that ASIDs are a unique namespace per SMMU.
The SMMU architecture requires this, and Linux relies on hardware following the architecture.
Please describe the problem correctly. i.e. that the hardware erroneously shares TLBs between SMMU instances.
Quoted 28 lines
That ASID-local choice conflicts with the
CN88xx SMMU, where only shared ASID namespaces are supported; specifically within a given node SMMU0 and SMMU1 share, as does SMMU2 and SMMU3. CN88xx SMMU also requires global VMIDs.This patch tries to address these issuee by supplying asid and vmid transformations from devicetree.
Signed-off-by Akula Geethasowjanya <[email protected]>Signed-off-by Tirumalesh Chalamarla <[email protected]>--- .../devicetree/bindings/iommu/arm,smmu.txt | 16 ++++++ drivers/iommu/arm-smmu.c | 59 ++++++++++++++++++---- 2 files changed, 65 insertions(+), 10 deletions(-)Documentation/devicetree/bindings/iommu/arm,smmu.txt +5
-0@@ -55,6 +55,22 @@ conditions. aliases of secure registers have to be used during SMMU configuration. +- thunderx,smmu-asid-transform : Enable proper handling for buggy + implementations that require special transformations + for smmu asid. if this property exists asid-transform + property must be present. +Documentation/devicetree/bindings/iommu/arm,smmu.txt conditions. · line 72
Quoted 10 lines
Documentation/devicetree/bindings/iommu/arm,smmu.txt +10
-0+- thunderx,smmu-vmid-transform : Enable proper handling for buggy + implementations that require special transformations + for smmu vmid. if this property exists vmid-transform + property must be present. + +- asid-transform : Transform mask that needs to be applied to asid. + This property has to be specified as '/bits/ 8' value. + +- vmid-transform : Transform mask that needs to be applied to vmid. + This property has to be specified as '/bits/ 8' value.Don't bother with /bits/ 8. It saves no space in the DTB and only adds to confusion. Use a full cell, and
of_property_read_u32, and validate that the value is in range.
Quoted 3 lines
Having two properties for each seems redundant. The presence of a mask should be sufficient to make it clear that the use of the mask is required.
Sure.
Quoted 9 lines
These properties aren't sufficiently described. How is the mask "applied", and what is it applied to? What constraints does it impose on the OS's selection of ASIDs/VMIDs?
From the looks of the patch, it's ANDed into the value Linux has selected, so it sounds like an OS must avoid using any bits set in any mask on any SMMU instance. That sounds very difficult to ensure in general, and very fragile.
For now it is only specific to ThunderX errata, its not a general requirement. So ThunderX DTS will make sure it defined proper values.
If all agree to make it a general requirement, I am happy to re write it with ASID and VMID coming from DTS.
Quoted 157 lines
Mark.
Quoted 132 lines
Documentation/devicetree/bindings/iommu/arm,smmu.txt +1
-0+ Example: smmu {drivers/iommu/arm-smmu.c +39
-10@@ -300,8 +300,12 @@ struct arm_smmu_device {u32 features;#define ARM_SMMU_OPT_SECURE_CFG_ACCESS (1 << 0) +#define ARM_SMMU_OPT_TRANSFORM_ASID (1 << 1) +#define ARM_SMMU_OPT_TRANSFORM_VMID (1 << 2)u32 options;enum arm_smmu_arch_version version; +u8 asid_transform; +u8 vmid_transform;u32 num_context_banks;u32 num_s2_context_banks;@@ -330,8 +334,25 @@ 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) +/* +* ThunderX has a unique requirement because of ERRATA#27704 +* An issue exists whereby the Linux kernel requires that ASIDs are a +* unique namespace per SMMU. That ASID-local choice conflicts with the +* CN88xx SMMU, where only shared ASID namespaces are supported; +* specifically within a given node SMMU0 and SMMU1 share, as does SMMU2 +* and SMMU3. +* CN88xx SMMU also requires global VMIDs. +*/ + +#define ARM_SMMU_CB_ASID (smmu, cfg) \ + (((smmu)->options & ARM_SMMU_OPT_TRANSFORM_ASID) \ + ? ((cfg)->cbndx | (smmu)->asid_transform) \ + : ((cfg)->cbndx)) + +#define ARM_SMMU_CB_VMID (smmu, cfg) \ + (((smmu)->options & ARM_SMMU_OPT_TRANSFORM_VMID) \ + ? (((cfg)->cbndx + 1) | (smmu)->vmid_transform) \ + : ((cfg)->cbndx + 1))enum arm_smmu_domain_stage {ARM_SMMU_DOMAIN_S1 =0 ,@@ -361,6 +382,8 @@ struct arm_smmu_option_prop {static struct arm_smmu_option_prop arm_smmu_options[] = { {ARM_SMMU_OPT_SECURE_CFG_ACCESS ,"calxeda,smmu-secure-config-access" }, + {ARM_SMMU_OPT_TRANSFORM_ASID ,"thunderx,smmu-asid-transform" }, + {ARM_SMMU_OPT_TRANSFORM_VMID ,"thunderx,smmu-vmid-transform" }, {0 ,NULL }, };@@ -570,11 +593,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 ); }@@ -596,7 +619,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;@@ -604,7 +627,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;@@ -624,7 +647,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); } }@@ -752,7 +775,7 @@ static void arm_smmu_init_context_bank(struct arm_smmu_domain *smmu_domain, reg |= (CBAR_S1_BPSHCFG_NSH <<CBAR_S1_BPSHCFG_SHIFT ) | (CBAR_S1_MEMATTR_WB <<CBAR_S1_MEMATTR_SHIFT ); }else { - 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));@@ -760,11 +783,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;@@ -1793,6 +1816,22 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev)parse_driver_options (smmu); +if (smmu->options &ARM_SMMU_OPT_TRANSFORM_ASID ) { +if (of_property_read_u8 (dev->of_node,"#asid-transform" , + &smmu->asid_transform)) { +dev_err (dev,"missing #asid-transform property\n" ); +return -ENODEV ; + }drivers/iommu/arm-smmu.c static int arm_smmu_device_dt_probe(struct platform_device *pdev) · line 1834
Quoted 10 lines
drivers/iommu/arm-smmu.c +10
-0+ } + +if (smmu->options &ARM_SMMU_OPT_TRANSFORM_VMID ) { +if (of_property_read_u8 (dev->of_node,"#vmid-transform" , + &smmu->vmid_transform)) { +dev_err (dev,"missing #vmid-transform property\n" ); +return -ENODEV ; + } + } +` if (smmu->version > ARM_SMMU_V1 && > smmu->num_context_banks != smmu->num_context_irqs) { > dev_err(dev,Quoted 9 lines
Signature · 7 lines
2.1.0 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel at lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Robin Murphy
reviewerPATCH 1/1 iommu/arm-smmu-v2: Workaround for ThunderX errata#27704
In reply to tchalamarla at caviumnetworks.comQuoted 4 lines · tchalamarla at caviumnetworks.com
From: Tirumalesh Chalamarla <[email protected]>
An issue exists whereby the Linux kernel requires that ASIDs are a unique namespace per SMMU.
I too tend to consider the SMMUv2 architecture an "issue", but we should probably still call it by its name ;)
Quoted 4 lines
That ASID-local choice conflicts with the
CN88xx SMMU, where only shared ASID namespaces are supported; specifically within a given node SMMU0 and SMMU1 share, as does SMMU2 and SMMU3. CN88xx SMMU also requires global VMIDs.
"Global" meaning unique across all 4 SMMUs? Or just unique across each pair as per ASIDs?...
Quoted 24 lines
This patch tries to address these issuee by supplying asid and vmid transformations from devicetree.
Signed-off-by Akula Geethasowjanya <[email protected]>Signed-off-by Tirumalesh Chalamarla <[email protected]>--- .../devicetree/bindings/iommu/arm,smmu.txt | 16 ++++++ drivers/iommu/arm-smmu.c | 59 ++++++++++++++++++---- 2 files changed, 65 insertions(+), 10 deletions(-)Documentation/devicetree/bindings/iommu/arm,smmu.txt +6
-0@@ -55,6 +55,22 @@ conditions. aliases of secure registers have to be used during SMMU configuration. +- thunderx,smmu-asid-transform : Enable proper handling for buggy + implementations that require special transformations + for smmu asid. if this property exists asid-transform + property must be present. + +- thunderx,smmu-vmid-transform : Enable proper handling for buggy
Documentation/devicetree/bindings/iommu/arm,smmu.txt conditions. · line 73
Quoted 10 lines
Documentation/devicetree/bindings/iommu/arm,smmu.txt +10
-0+ implementations that require special transformations + for smmu vmid. if this property exists vmid-transform + property must be present. + +- asid-transform : Transform mask that needs to be applied to asid. + This property has to be specified as '/bits/ 8' value. + +- vmid-transform : Transform mask that needs to be applied to vmid. + This property has to be specified as '/bits/ 8' value. +
...because I've seen a ThunderX boot log indicating 4x128 context banks, and I don't see how applying an 8-bit mask to an 8-bit value is going to produce anything _globally_ unique in that case.
Regardless, I'm not sure this is an accurate description of the hardware as DT should be - this is more implying that each SMMU _requires_ a particular set of ASIDs/VMIDs to be used, which is not the same thing that the commit message and other comments say.
Robin.
Quoted 145 lines
Documentation/devicetree/bindings/iommu/arm,smmu.txt +0
-0Example: smmu {drivers/iommu/arm-smmu.c +49
-10@@ -300,8 +300,12 @@ struct arm_smmu_device {u32 features;#define ARM_SMMU_OPT_SECURE_CFG_ACCESS (1 << 0) +#define ARM_SMMU_OPT_TRANSFORM_ASID (1 << 1) +#define ARM_SMMU_OPT_TRANSFORM_VMID (1 << 2)u32 options;enum arm_smmu_arch_version version; +u8 asid_transform; +u8 vmid_transform;u32 num_context_banks;u32 num_s2_context_banks;@@ -330,8 +334,25 @@ 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) +/* +* ThunderX has a unique requirement because of ERRATA#27704 +* An issue exists whereby the Linux kernel requires that ASIDs are a +* unique namespace per SMMU. That ASID-local choice conflicts with the +* CN88xx SMMU, where only shared ASID namespaces are supported; +* specifically within a given node SMMU0 and SMMU1 share, as does SMMU2 +* and SMMU3. +* CN88xx SMMU also requires global VMIDs. +*/ + +#define ARM_SMMU_CB_ASID (smmu, cfg) \ + (((smmu)->options & ARM_SMMU_OPT_TRANSFORM_ASID) \ + ? ((cfg)->cbndx | (smmu)->asid_transform) \ + : ((cfg)->cbndx)) + +#define ARM_SMMU_CB_VMID (smmu, cfg) \ + (((smmu)->options & ARM_SMMU_OPT_TRANSFORM_VMID) \ + ? (((cfg)->cbndx + 1) | (smmu)->vmid_transform) \ + : ((cfg)->cbndx + 1))enum arm_smmu_domain_stage {ARM_SMMU_DOMAIN_S1 =0 ,@@ -361,6 +382,8 @@ struct arm_smmu_option_prop {static struct arm_smmu_option_prop arm_smmu_options[] = { {ARM_SMMU_OPT_SECURE_CFG_ACCESS ,"calxeda,smmu-secure-config-access" }, + {ARM_SMMU_OPT_TRANSFORM_ASID ,"thunderx,smmu-asid-transform" }, + {ARM_SMMU_OPT_TRANSFORM_VMID ,"thunderx,smmu-vmid-transform" }, {0 ,NULL }, };@@ -570,11 +593,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 ); }@@ -596,7 +619,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;@@ -604,7 +627,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;@@ -624,7 +647,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); } }@@ -752,7 +775,7 @@ static void arm_smmu_init_context_bank(struct arm_smmu_domain *smmu_domain, reg |= (CBAR_S1_BPSHCFG_NSH <<CBAR_S1_BPSHCFG_SHIFT ) | (CBAR_S1_MEMATTR_WB <<CBAR_S1_MEMATTR_SHIFT ); }else { - 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));@@ -760,11 +783,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;@@ -1793,6 +1816,22 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev)parse_driver_options (smmu); +if (smmu->options &ARM_SMMU_OPT_TRANSFORM_ASID ) { +if (of_property_read_u8 (dev->of_node,"#asid-transform" , + &smmu->asid_transform)) { +dev_err (dev,"missing #asid-transform property\n" ); +return -ENODEV ; + } + } + +if (smmu->options &ARM_SMMU_OPT_TRANSFORM_VMID ) { +if (of_property_read_u8 (dev->of_node,"#vmid-transform" , + &smmu->vmid_transform)) { +dev_err (dev,"missing #vmid-transform property\n" ); +return -ENODEV ; + } + } +if (smmu->version >ARM_SMMU_V1 && smmu->num_context_banks != smmu->num_context_irqs) { dev_err(dev,
PATCH 1/1 iommu/arm-smmu-v2: Workaround for ThunderX errata#27704
Quoted 56 lines · Robin Murphy
Quoted 4 lines · tchalamarla at caviumnetworks.com
From: Tirumalesh Chalamarla <[email protected]>
An issue exists whereby the Linux kernel requires that ASIDs are a unique namespace per SMMU.
I too tend to consider the SMMUv2 architecture an "issue", but we should probably still call it by its name ;)
Quoted 4 lines
That ASID-local choice conflicts with the
CN88xx SMMU, where only shared ASID namespaces are supported; specifically within a given node SMMU0 and SMMU1 share, as does SMMU2 and SMMU3. CN88xx SMMU also requires global VMIDs."Global" meaning unique across all 4 SMMUs? Or just unique across each pair as per ASIDs?...
Quoted 24 lines
This patch tries to address these issuee by supplying asid and vmid transformations from devicetree.
Signed-off-by Akula Geethasowjanya <[email protected]>Signed-off-by Tirumalesh Chalamarla <[email protected]>--- .../devicetree/bindings/iommu/arm,smmu.txt | 16 ++++++ drivers/iommu/arm-smmu.c | 59 ++++++++++++++++++---- 2 files changed, 65 insertions(+), 10 deletions(-)Documentation/devicetree/bindings/iommu/arm,smmu.txt +6
-0@@ -55,6 +55,22 @@ conditions. aliases of secure registers have to be used during SMMU configuration. +- thunderx,smmu-asid-transform : Enable proper handling for buggy + implementations that require special transformations + for smmu asid. if this property exists asid-transform + property must be present. + +- thunderx,smmu-vmid-transform : Enable proper handling for buggyDocumentation/devicetree/bindings/iommu/arm,smmu.txt conditions. · line 73
Quoted 10 lines
Documentation/devicetree/bindings/iommu/arm,smmu.txt +10
-0+ implementations that require special transformations + for smmu vmid. if this property exists vmid-transform + property must be present. + +- asid-transform : Transform mask that needs to be applied to asid. + This property has to be specified as '/bits/ 8' value. + +- vmid-transform : Transform mask that needs to be applied to vmid. + This property has to be specified as '/bits/ 8' value. +...because I've seen a ThunderX boot log indicating 4x128 context banks, and I don't see how applying an 8-bit mask to an 8-bit value is going to produce anything _globally_ unique in that case.
Quoted 4 lines
Regardless, I'm not sure this is an accurate description of the hardware as DT should be - this is more implying that each SMMU _requires_ a particular set of ASIDs/VMIDs to be used, which is not the same thing that the commit message and other comments say.
I am rewriting this patch to include additional patches to support 16bit VMID and ASID. Will rebase this patch on top of it. Though for 128 bit 7 bits are sufficient I thought it is still good idea to have this on top of 16bit VMID and ASID.
Quoted 149 lines
Robin.
Quoted 145 lines
Documentation/devicetree/bindings/iommu/arm,smmu.txt +0
-0Example: smmu {drivers/iommu/arm-smmu.c +49
-10@@ -300,8 +300,12 @@ struct arm_smmu_device {u32 features;#define ARM_SMMU_OPT_SECURE_CFG_ACCESS (1 << 0) +#define ARM_SMMU_OPT_TRANSFORM_ASID (1 << 1) +#define ARM_SMMU_OPT_TRANSFORM_VMID (1 << 2)u32 options;enum arm_smmu_arch_version version; +u8 asid_transform; +u8 vmid_transform;u32 num_context_banks;u32 num_s2_context_banks;@@ -330,8 +334,25 @@ 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) +/* +* ThunderX has a unique requirement because of ERRATA#27704 +* An issue exists whereby the Linux kernel requires that ASIDs are a +* unique namespace per SMMU. That ASID-local choice conflicts with the +* CN88xx SMMU, where only shared ASID namespaces are supported; +* specifically within a given node SMMU0 and SMMU1 share, as does SMMU2 +* and SMMU3. +* CN88xx SMMU also requires global VMIDs. +*/ + +#define ARM_SMMU_CB_ASID (smmu, cfg) \ + (((smmu)->options & ARM_SMMU_OPT_TRANSFORM_ASID) \ + ? ((cfg)->cbndx | (smmu)->asid_transform) \ + : ((cfg)->cbndx)) + +#define ARM_SMMU_CB_VMID (smmu, cfg) \ + (((smmu)->options & ARM_SMMU_OPT_TRANSFORM_VMID) \ + ? (((cfg)->cbndx + 1) | (smmu)->vmid_transform) \ + : ((cfg)->cbndx + 1))enum arm_smmu_domain_stage {ARM_SMMU_DOMAIN_S1 =0 ,@@ -361,6 +382,8 @@ struct arm_smmu_option_prop {static struct arm_smmu_option_prop arm_smmu_options[] = { {ARM_SMMU_OPT_SECURE_CFG_ACCESS ,"calxeda,smmu-secure-config-access" }, + {ARM_SMMU_OPT_TRANSFORM_ASID ,"thunderx,smmu-asid-transform" }, + {ARM_SMMU_OPT_TRANSFORM_VMID ,"thunderx,smmu-vmid-transform" }, {0 ,NULL }, };@@ -570,11 +593,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 ); }@@ -596,7 +619,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;@@ -604,7 +627,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;@@ -624,7 +647,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); } }@@ -752,7 +775,7 @@ static void arm_smmu_init_context_bank(struct arm_smmu_domain *smmu_domain, reg |= (CBAR_S1_BPSHCFG_NSH <<CBAR_S1_BPSHCFG_SHIFT ) | (CBAR_S1_MEMATTR_WB <<CBAR_S1_MEMATTR_SHIFT ); }else { - 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));@@ -760,11 +783,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;@@ -1793,6 +1816,22 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev)parse_driver_options (smmu); +if (smmu->options &ARM_SMMU_OPT_TRANSFORM_ASID ) { +if (of_property_read_u8 (dev->of_node,"#asid-transform" , + &smmu->asid_transform)) { +dev_err (dev,"missing #asid-transform property\n" ); +return -ENODEV ; + } + } + +if (smmu->options &ARM_SMMU_OPT_TRANSFORM_VMID ) { +if (of_property_read_u8 (dev->of_node,"#vmid-transform" , + &smmu->vmid_transform)) { +dev_err (dev,"missing #vmid-transform property\n" ); +return -ENODEV ; + } + } +if (smmu->version >ARM_SMMU_V1 && smmu->num_context_banks != smmu->num_context_irqs) { dev_err(dev,