* [PATCH] iommu/arm-smmu-v2: Workaround for ThunderX errata#27704
@ 2016-02-05 18:47 ` tchalamarla at caviumnetworks.com
0 siblings, 0 replies; 26+ messages in thread
From: tchalamarla-M3mlKVOIwJVv6pq1l3V1OdBPR1lH4CV8 @ 2016-02-05 18:47 UTC (permalink / raw)
To: will.deacon-5wv7dgnIgG8, robin.murphy-5wv7dgnIgG8
Cc: iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
Geethasowjanya.Akula-M3mlKVOIwJVv6pq1l3V1OdBPR1lH4CV8,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
tchalamarla-M3mlKVOIwJVv6pq1l3V1OdBPR1lH4CV8
From: Tirumalesh Chalamarla <tchalamarla-M3mlKVOIwJVv6pq1l3V1OdBPR1lH4CV8@public.gmane.org>
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.
Signed-off-by: Akula Geethasowjanya <Geethasowjanya.Akula-M3mlKVOIwJVv6pq1l3V1OdBPR1lH4CV8@public.gmane.org>
Signed-off-by: Tirumalesh Chalamarla <tchalamarla-M3mlKVOIwJVv6pq1l3V1OdBPR1lH4CV8@public.gmane.org>
---
.../devicetree/bindings/iommu/arm,smmu.txt | 16 ++++++
drivers/iommu/arm-smmu.c | 59 ++++++++++++++++++----
2 files changed, 65 insertions(+), 10 deletions(-)
diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.txt b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
index 7180745..eef06d0 100644
--- a/Documentation/devicetree/bindings/iommu/arm,smmu.txt
+++ b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
@@ -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 {
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 59ee4b8..8047834 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -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,
--
2.1.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH] iommu/arm-smmu-v2: Workaround for ThunderX errata#27704
@ 2016-02-05 18:47 ` tchalamarla at caviumnetworks.com
0 siblings, 0 replies; 26+ messages in thread
From: tchalamarla at caviumnetworks.com @ 2016-02-05 18:47 UTC (permalink / raw)
To: linux-arm-kernel
From: Tirumalesh Chalamarla <tchalamarla@caviumnetworks.com>
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.
Signed-off-by: Akula Geethasowjanya <Geethasowjanya.Akula@caviumnetworks.com>
Signed-off-by: Tirumalesh Chalamarla <tchalamarla@caviumnetworks.com>
---
.../devicetree/bindings/iommu/arm,smmu.txt | 16 ++++++
drivers/iommu/arm-smmu.c | 59 ++++++++++++++++++----
2 files changed, 65 insertions(+), 10 deletions(-)
diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.txt b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
index 7180745..eef06d0 100644
--- a/Documentation/devicetree/bindings/iommu/arm,smmu.txt
+++ b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
@@ -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 {
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 59ee4b8..8047834 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -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,
--
2.1.0
^ permalink raw reply related [flat|nested] 26+ messages in thread[parent not found: <1454698027-20911-1-git-send-email-tchalamarla-M3mlKVOIwJVv6pq1l3V1OdBPR1lH4CV8@public.gmane.org>]
* Re: [PATCH] iommu/arm-smmu-v2: Workaround for ThunderX errata#27704
2016-02-05 18:47 ` tchalamarla at caviumnetworks.com
@ 2016-02-05 20:02 ` Mark Rutland
-1 siblings, 0 replies; 26+ messages in thread
From: Mark Rutland @ 2016-02-05 20:02 UTC (permalink / raw)
To: tchalamarla-M3mlKVOIwJVv6pq1l3V1OdBPR1lH4CV8
Cc: iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
will.deacon-5wv7dgnIgG8,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
Geethasowjanya.Akula-M3mlKVOIwJVv6pq1l3V1OdBPR1lH4CV8
On Fri, Feb 05, 2016 at 10:47:07AM -0800, tchalamarla-M3mlKVOIwJVv6pq1l3V1OdBPR1lH4CV8@public.gmane.org wrote:
> From: Tirumalesh Chalamarla <tchalamarla-M3mlKVOIwJVv6pq1l3V1OdBPR1lH4CV8@public.gmane.org>
>
> 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.
> 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 <Geethasowjanya.Akula-M3mlKVOIwJVv6pq1l3V1OdBPR1lH4CV8@public.gmane.org>
> Signed-off-by: Tirumalesh Chalamarla <tchalamarla-M3mlKVOIwJVv6pq1l3V1OdBPR1lH4CV8@public.gmane.org>
> ---
> .../devicetree/bindings/iommu/arm,smmu.txt | 16 ++++++
> drivers/iommu/arm-smmu.c | 59 ++++++++++++++++++----
> 2 files changed, 65 insertions(+), 10 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.txt b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
> index 7180745..eef06d0 100644
> --- a/Documentation/devicetree/bindings/iommu/arm,smmu.txt
> +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
> @@ -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.
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?
>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.
> +
> Example:
>
> smmu {
> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> index 59ee4b8..8047834 100644
> --- a/drivers/iommu/arm-smmu.c
> +++ b/drivers/iommu/arm-smmu.c
> @@ -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,
> --
> 2.1.0
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
^ permalink raw reply [flat|nested] 26+ messages in thread* [PATCH] iommu/arm-smmu-v2: Workaround for ThunderX errata#27704
@ 2016-02-05 20:02 ` Mark Rutland
0 siblings, 0 replies; 26+ messages in thread
From: Mark Rutland @ 2016-02-05 20:02 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Feb 05, 2016 at 10:47:07AM -0800, tchalamarla at caviumnetworks.com wrote:
> From: Tirumalesh Chalamarla <tchalamarla@caviumnetworks.com>
>
> 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.
> 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 <Geethasowjanya.Akula@caviumnetworks.com>
> Signed-off-by: Tirumalesh Chalamarla <tchalamarla@caviumnetworks.com>
> ---
> .../devicetree/bindings/iommu/arm,smmu.txt | 16 ++++++
> drivers/iommu/arm-smmu.c | 59 ++++++++++++++++++----
> 2 files changed, 65 insertions(+), 10 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.txt b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
> index 7180745..eef06d0 100644
> --- a/Documentation/devicetree/bindings/iommu/arm,smmu.txt
> +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
> @@ -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.
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?
>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.
> +
> Example:
>
> smmu {
> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> index 59ee4b8..8047834 100644
> --- a/drivers/iommu/arm-smmu.c
> +++ b/drivers/iommu/arm-smmu.c
> @@ -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,
> --
> 2.1.0
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH] iommu/arm-smmu-v2: Workaround for ThunderX errata#27704
2016-02-05 20:02 ` Mark Rutland
@ 2016-02-05 20:15 ` Mark Rutland
-1 siblings, 0 replies; 26+ messages in thread
From: Mark Rutland @ 2016-02-05 20:15 UTC (permalink / raw)
To: tchalamarla-M3mlKVOIwJVv6pq1l3V1OdBPR1lH4CV8
Cc: will.deacon-5wv7dgnIgG8,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
Geethasowjanya.Akula-M3mlKVOIwJVv6pq1l3V1OdBPR1lH4CV8
On Fri, Feb 05, 2016 at 08:02:09PM +0000, Mark Rutland wrote:
> On Fri, Feb 05, 2016 at 10:47:07AM -0800, tchalamarla-M3mlKVOIwJVv6pq1l3V1OdBPR1lH4CV8@public.gmane.org wrote:
> > From: Tirumalesh Chalamarla <tchalamarla-M3mlKVOIwJVv6pq1l3V1OdBPR1lH4CV8@public.gmane.org>
> >
> > 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.
>
> > 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 <Geethasowjanya.Akula-M3mlKVOIwJVv6pq1l3V1OdBPR1lH4CV8@public.gmane.org>
> > Signed-off-by: Tirumalesh Chalamarla <tchalamarla-M3mlKVOIwJVv6pq1l3V1OdBPR1lH4CV8@public.gmane.org>
> > ---
> > .../devicetree/bindings/iommu/arm,smmu.txt | 16 ++++++
> > drivers/iommu/arm-smmu.c | 59 ++++++++++++++++++----
> > 2 files changed, 65 insertions(+), 10 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.txt b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
> > index 7180745..eef06d0 100644
> > --- a/Documentation/devicetree/bindings/iommu/arm,smmu.txt
> > +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
> > @@ -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.
>
> 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?
>
> 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.
^ permalink raw reply [flat|nested] 26+ messages in thread* [PATCH] iommu/arm-smmu-v2: Workaround for ThunderX errata#27704
@ 2016-02-05 20:15 ` Mark Rutland
0 siblings, 0 replies; 26+ messages in thread
From: Mark Rutland @ 2016-02-05 20:15 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Feb 05, 2016 at 08:02:09PM +0000, Mark Rutland wrote:
> On Fri, Feb 05, 2016 at 10:47:07AM -0800, tchalamarla at caviumnetworks.com wrote:
> > From: Tirumalesh Chalamarla <tchalamarla@caviumnetworks.com>
> >
> > 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.
>
> > 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 <Geethasowjanya.Akula@caviumnetworks.com>
> > Signed-off-by: Tirumalesh Chalamarla <tchalamarla@caviumnetworks.com>
> > ---
> > .../devicetree/bindings/iommu/arm,smmu.txt | 16 ++++++
> > drivers/iommu/arm-smmu.c | 59 ++++++++++++++++++----
> > 2 files changed, 65 insertions(+), 10 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.txt b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
> > index 7180745..eef06d0 100644
> > --- a/Documentation/devicetree/bindings/iommu/arm,smmu.txt
> > +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
> > @@ -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.
>
> 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?
>
> 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.
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH] iommu/arm-smmu-v2: Workaround for ThunderX errata#27704
2016-02-05 20:15 ` Mark Rutland
@ 2016-02-05 20:29 ` Chalamarla, Tirumalesh
-1 siblings, 0 replies; 26+ messages in thread
From: Chalamarla, Tirumalesh @ 2016-02-05 20:29 UTC (permalink / raw)
To: Mark Rutland
Cc: will.deacon-5wv7dgnIgG8,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Akula,
Geethasowjanya
On 2/5/16, 12:15 PM, "Mark Rutland" <mark.rutland@arm.com> wrote:
>On Fri, Feb 05, 2016 at 08:02:09PM +0000, Mark Rutland wrote:
>> On Fri, Feb 05, 2016 at 10:47:07AM -0800, tchalamarla@caviumnetworks.com wrote:
>> > From: Tirumalesh Chalamarla <tchalamarla@caviumnetworks.com>
>> >
>> > 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.
>>
>> > 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 <Geethasowjanya.Akula@caviumnetworks.com>
>> > Signed-off-by: Tirumalesh Chalamarla <tchalamarla@caviumnetworks.com>
>> > ---
>> > .../devicetree/bindings/iommu/arm,smmu.txt | 16 ++++++
>> > drivers/iommu/arm-smmu.c | 59 ++++++++++++++++++----
>> > 2 files changed, 65 insertions(+), 10 deletions(-)
>> >
>> > diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.txt b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
>> > index 7180745..eef06d0 100644
>> > --- a/Documentation/devicetree/bindings/iommu/arm,smmu.txt
>> > +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
>> > @@ -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.
>>
>> 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?
>>
>> 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.
The problem is, Will don’t want this to be part of general change, he want to treat it as Errata.
>
>Mark.
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH] iommu/arm-smmu-v2: Workaround for ThunderX errata#27704
@ 2016-02-05 20:29 ` Chalamarla, Tirumalesh
0 siblings, 0 replies; 26+ messages in thread
From: Chalamarla, Tirumalesh @ 2016-02-05 20:29 UTC (permalink / raw)
To: linux-arm-kernel
On 2/5/16, 12:15 PM, "Mark Rutland" <mark.rutland@arm.com> wrote:
>On Fri, Feb 05, 2016 at 08:02:09PM +0000, Mark Rutland wrote:
>> On Fri, Feb 05, 2016 at 10:47:07AM -0800, tchalamarla at caviumnetworks.com wrote:
>> > From: Tirumalesh Chalamarla <tchalamarla@caviumnetworks.com>
>> >
>> > 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.
>>
>> > 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 <Geethasowjanya.Akula@caviumnetworks.com>
>> > Signed-off-by: Tirumalesh Chalamarla <tchalamarla@caviumnetworks.com>
>> > ---
>> > .../devicetree/bindings/iommu/arm,smmu.txt | 16 ++++++
>> > drivers/iommu/arm-smmu.c | 59 ++++++++++++++++++----
>> > 2 files changed, 65 insertions(+), 10 deletions(-)
>> >
>> > diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.txt b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
>> > index 7180745..eef06d0 100644
>> > --- a/Documentation/devicetree/bindings/iommu/arm,smmu.txt
>> > +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
>> > @@ -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.
>>
>> 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?
>>
>> 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.
The problem is, Will don?t want this to be part of general change, he want to treat it as Errata.
>
>Mark.
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] iommu/arm-smmu-v2: Workaround for ThunderX errata#27704
2016-02-05 20:02 ` Mark Rutland
@ 2016-02-05 20:32 ` Chalamarla, Tirumalesh
-1 siblings, 0 replies; 26+ messages in thread
From: Chalamarla, Tirumalesh @ 2016-02-05 20:32 UTC (permalink / raw)
To: Mark Rutland
Cc: iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
will.deacon-5wv7dgnIgG8,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Akula,
Geethasowjanya
On 2/5/16, 12:02 PM, "Mark Rutland" <mark.rutland-5wv7dgnIgG8@public.gmane.org> wrote:
>On Fri, Feb 05, 2016 at 10:47:07AM -0800, tchalamarla-M3mlKVOIwJVv6pq1l3V1OdBPR1lH4CV8@public.gmane.org wrote:
>> From: Tirumalesh Chalamarla <tchalamarla-M3mlKVOIwJVv6pq1l3V1OdBPR1lH4CV8@public.gmane.org>
>>
>> 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.
>
>> 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 <Geethasowjanya.Akula-M3mlKVOIwJVv6pq1l3V1OdBPR1lH4CV8@public.gmane.org>
>> Signed-off-by: Tirumalesh Chalamarla <tchalamarla-M3mlKVOIwJVv6pq1l3V1OdBPR1lH4CV8@public.gmane.org>
>> ---
>> .../devicetree/bindings/iommu/arm,smmu.txt | 16 ++++++
>> drivers/iommu/arm-smmu.c | 59 ++++++++++++++++++----
>> 2 files changed, 65 insertions(+), 10 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.txt b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
>> index 7180745..eef06d0 100644
>> --- a/Documentation/devicetree/bindings/iommu/arm,smmu.txt
>> +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
>> @@ -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.
>
>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.
Sure.
>
>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.
>
>Mark.
>
>> +
>> Example:
>>
>> smmu {
>> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
>> index 59ee4b8..8047834 100644
>> --- a/drivers/iommu/arm-smmu.c
>> +++ b/drivers/iommu/arm-smmu.c
>> @@ -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,
>> --
>> 2.1.0
>>
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>>
^ permalink raw reply [flat|nested] 26+ messages in thread* [PATCH] iommu/arm-smmu-v2: Workaround for ThunderX errata#27704
@ 2016-02-05 20:32 ` Chalamarla, Tirumalesh
0 siblings, 0 replies; 26+ messages in thread
From: Chalamarla, Tirumalesh @ 2016-02-05 20:32 UTC (permalink / raw)
To: linux-arm-kernel
On 2/5/16, 12:02 PM, "Mark Rutland" <mark.rutland@arm.com> wrote:
>On Fri, Feb 05, 2016 at 10:47:07AM -0800, tchalamarla at caviumnetworks.com wrote:
>> From: Tirumalesh Chalamarla <tchalamarla@caviumnetworks.com>
>>
>> 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.
>
>> 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 <Geethasowjanya.Akula@caviumnetworks.com>
>> Signed-off-by: Tirumalesh Chalamarla <tchalamarla@caviumnetworks.com>
>> ---
>> .../devicetree/bindings/iommu/arm,smmu.txt | 16 ++++++
>> drivers/iommu/arm-smmu.c | 59 ++++++++++++++++++----
>> 2 files changed, 65 insertions(+), 10 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.txt b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
>> index 7180745..eef06d0 100644
>> --- a/Documentation/devicetree/bindings/iommu/arm,smmu.txt
>> +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
>> @@ -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.
>
>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.
Sure.
>
>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.
>
>Mark.
>
>> +
>> Example:
>>
>> smmu {
>> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
>> index 59ee4b8..8047834 100644
>> --- a/drivers/iommu/arm-smmu.c
>> +++ b/drivers/iommu/arm-smmu.c
>> @@ -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,
>> --
>> 2.1.0
>>
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel at lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] iommu/arm-smmu-v2: Workaround for ThunderX errata#27704
2016-02-05 18:47 ` tchalamarla at caviumnetworks.com
@ 2016-02-09 11:52 ` Robin Murphy
-1 siblings, 0 replies; 26+ messages in thread
From: Robin Murphy @ 2016-02-09 11:52 UTC (permalink / raw)
To: tchalamarla-M3mlKVOIwJVv6pq1l3V1OdBPR1lH4CV8, will.deacon-5wv7dgnIgG8
Cc: iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
Geethasowjanya.Akula-M3mlKVOIwJVv6pq1l3V1OdBPR1lH4CV8,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
On 05/02/16 18:47, tchalamarla-M3mlKVOIwJVv6pq1l3V1OdBPR1lH4CV8@public.gmane.org wrote:
> From: Tirumalesh Chalamarla <tchalamarla-M3mlKVOIwJVv6pq1l3V1OdBPR1lH4CV8@public.gmane.org>
>
> 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 ;)
> 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?...
> This patch tries to address these issuee by supplying asid and vmid
> transformations from devicetree.
>
> Signed-off-by: Akula Geethasowjanya <Geethasowjanya.Akula-M3mlKVOIwJVv6pq1l3V1OdBPR1lH4CV8@public.gmane.org>
> Signed-off-by: Tirumalesh Chalamarla <tchalamarla-M3mlKVOIwJVv6pq1l3V1OdBPR1lH4CV8@public.gmane.org>
> ---
> .../devicetree/bindings/iommu/arm,smmu.txt | 16 ++++++
> drivers/iommu/arm-smmu.c | 59 ++++++++++++++++++----
> 2 files changed, 65 insertions(+), 10 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.txt b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
> index 7180745..eef06d0 100644
> --- a/Documentation/devicetree/bindings/iommu/arm,smmu.txt
> +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
> @@ -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.
> +
...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.
> Example:
>
> smmu {
> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> index 59ee4b8..8047834 100644
> --- a/drivers/iommu/arm-smmu.c
> +++ b/drivers/iommu/arm-smmu.c
> @@ -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,
>
^ permalink raw reply [flat|nested] 26+ messages in thread* [PATCH] iommu/arm-smmu-v2: Workaround for ThunderX errata#27704
@ 2016-02-09 11:52 ` Robin Murphy
0 siblings, 0 replies; 26+ messages in thread
From: Robin Murphy @ 2016-02-09 11:52 UTC (permalink / raw)
To: linux-arm-kernel
On 05/02/16 18:47, tchalamarla at caviumnetworks.com wrote:
> From: Tirumalesh Chalamarla <tchalamarla@caviumnetworks.com>
>
> 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 ;)
> 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?...
> This patch tries to address these issuee by supplying asid and vmid
> transformations from devicetree.
>
> Signed-off-by: Akula Geethasowjanya <Geethasowjanya.Akula@caviumnetworks.com>
> Signed-off-by: Tirumalesh Chalamarla <tchalamarla@caviumnetworks.com>
> ---
> .../devicetree/bindings/iommu/arm,smmu.txt | 16 ++++++
> drivers/iommu/arm-smmu.c | 59 ++++++++++++++++++----
> 2 files changed, 65 insertions(+), 10 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.txt b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
> index 7180745..eef06d0 100644
> --- a/Documentation/devicetree/bindings/iommu/arm,smmu.txt
> +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
> @@ -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.
> +
...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.
> Example:
>
> smmu {
> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> index 59ee4b8..8047834 100644
> --- a/drivers/iommu/arm-smmu.c
> +++ b/drivers/iommu/arm-smmu.c
> @@ -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,
>
^ permalink raw reply [flat|nested] 26+ messages in thread[parent not found: <56B9D2F4.8070308-5wv7dgnIgG8@public.gmane.org>]
* Re: [PATCH] iommu/arm-smmu-v2: Workaround for ThunderX errata#27704
2016-02-09 11:52 ` Robin Murphy
@ 2016-02-09 17:07 ` Chalamarla, Tirumalesh
-1 siblings, 0 replies; 26+ messages in thread
From: Chalamarla, Tirumalesh @ 2016-02-09 17:07 UTC (permalink / raw)
To: Robin Murphy, will.deacon-5wv7dgnIgG8
Cc: iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, Akula,
Geethasowjanya,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
On 2/9/16, 3:52 AM, "Robin Murphy" <robin.murphy-5wv7dgnIgG8@public.gmane.org> wrote:
>On 05/02/16 18:47, tchalamarla-M3mlKVOIwJVv6pq1l3V1OdBPR1lH4CV8@public.gmane.org wrote:
>> From: Tirumalesh Chalamarla <tchalamarla-M3mlKVOIwJVv6pq1l3V1OdBPR1lH4CV8@public.gmane.org>
>>
>> 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 ;)
>
>> 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?...
>
>> This patch tries to address these issuee by supplying asid and vmid
>> transformations from devicetree.
>>
>> Signed-off-by: Akula Geethasowjanya <Geethasowjanya.Akula-M3mlKVOIwJVv6pq1l3V1OdBPR1lH4CV8@public.gmane.org>
>> Signed-off-by: Tirumalesh Chalamarla <tchalamarla-M3mlKVOIwJVv6pq1l3V1OdBPR1lH4CV8@public.gmane.org>
>> ---
>> .../devicetree/bindings/iommu/arm,smmu.txt | 16 ++++++
>> drivers/iommu/arm-smmu.c | 59 ++++++++++++++++++----
>> 2 files changed, 65 insertions(+), 10 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.txt b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
>> index 7180745..eef06d0 100644
>> --- a/Documentation/devicetree/bindings/iommu/arm,smmu.txt
>> +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
>> @@ -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.
>> +
>
>...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.
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.
>
>Robin.
>
>> Example:
>>
>> smmu {
>> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
>> index 59ee4b8..8047834 100644
>> --- a/drivers/iommu/arm-smmu.c
>> +++ b/drivers/iommu/arm-smmu.c
>> @@ -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,
>>
>
^ permalink raw reply [flat|nested] 26+ messages in thread* [PATCH] iommu/arm-smmu-v2: Workaround for ThunderX errata#27704
@ 2016-02-09 17:07 ` Chalamarla, Tirumalesh
0 siblings, 0 replies; 26+ messages in thread
From: Chalamarla, Tirumalesh @ 2016-02-09 17:07 UTC (permalink / raw)
To: linux-arm-kernel
On 2/9/16, 3:52 AM, "Robin Murphy" <robin.murphy@arm.com> wrote:
>On 05/02/16 18:47, tchalamarla at caviumnetworks.com wrote:
>> From: Tirumalesh Chalamarla <tchalamarla@caviumnetworks.com>
>>
>> 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 ;)
>
>> 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?...
>
>> This patch tries to address these issuee by supplying asid and vmid
>> transformations from devicetree.
>>
>> Signed-off-by: Akula Geethasowjanya <Geethasowjanya.Akula@caviumnetworks.com>
>> Signed-off-by: Tirumalesh Chalamarla <tchalamarla@caviumnetworks.com>
>> ---
>> .../devicetree/bindings/iommu/arm,smmu.txt | 16 ++++++
>> drivers/iommu/arm-smmu.c | 59 ++++++++++++++++++----
>> 2 files changed, 65 insertions(+), 10 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.txt b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
>> index 7180745..eef06d0 100644
>> --- a/Documentation/devicetree/bindings/iommu/arm,smmu.txt
>> +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
>> @@ -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.
>> +
>
>...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.
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.
>
>Robin.
>
>> Example:
>>
>> smmu {
>> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
>> index 59ee4b8..8047834 100644
>> --- a/drivers/iommu/arm-smmu.c
>> +++ b/drivers/iommu/arm-smmu.c
>> @@ -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,
>>
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH] iommu/arm-smmu-v2: Workaround for ThunderX errata#27704
@ 2016-02-24 21:13 ` Tirumalesh Chalamarla
0 siblings, 0 replies; 26+ messages in thread
From: Tirumalesh Chalamarla @ 2016-02-24 21:13 UTC (permalink / raw)
To: will.deacon-5wv7dgnIgG8, robin.murphy-5wv7dgnIgG8,
mark.rutland-5wv7dgnIgG8
Cc: iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
Geethasowjanya.Akula-M3mlKVOIwJVv6pq1l3V1OdBPR1lH4CV8,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
tchalamarla-M3mlKVOIwJVv6pq1l3V1OdBPR1lH4CV8
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 address these issuee by supplying asid and vmid
while calculating ASID and VMID for Thunder SMMUv2.
NOTE: resending with commit message fix.
changes from V2:
- removed *_base from DT, and replaced with compatible string
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: Tirumalesh Chalamarla <tchalamarla-M3mlKVOIwJVv6pq1l3V1OdBPR1lH4CV8@public.gmane.org>
Signed-off-by: Akula Geethasowjanya <Geethasowjanya.Akula-M3mlKVOIwJVv6pq1l3V1OdBPR1lH4CV8@public.gmane.org>
---
.../devicetree/bindings/iommu/arm,smmu.txt | 1 +
drivers/iommu/arm-smmu.c | 48 +++++++++++++++++-----
2 files changed, 38 insertions(+), 11 deletions(-)
diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.txt b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
index 7180745..19fe6f2 100644
--- a/Documentation/devicetree/bindings/iommu/arm,smmu.txt
+++ b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
@@ -16,6 +16,7 @@ conditions.
"arm,mmu-400"
"arm,mmu-401"
"arm,mmu-500"
+ "cavium,smmu-v2"
depending on the particular implementation and/or the
version of the architecture implemented.
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 247a469..c704f88 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -326,6 +326,12 @@ struct arm_smmu_device {
struct list_head list;
struct rb_root masters;
+ /*
+ *The following fields are specific to Cavium, Thunder
+ */
+ u32 cavium_smmu_id;
+ u32 cavium_id_base;
+
};
struct arm_smmu_cfg {
@@ -335,8 +341,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)->cavium_id_base + (cfg)->cbndx)
+#define ARM_SMMU_CB_VMID(smmu, cfg) ((u16)(smmu)->cavium_id_base + (cfg)->cbndx + 1)
enum arm_smmu_domain_stage {
ARM_SMMU_DOMAIN_S1 = 0,
@@ -364,6 +370,8 @@ struct arm_smmu_option_prop {
const char *prop;
};
+static int cavium_smmu_count;
+
static struct arm_smmu_option_prop arm_smmu_options[] = {
{ ARM_SMMU_OPT_SECURE_CFG_ACCESS, "calxeda,smmu-secure-config-access" },
{ 0, NULL},
@@ -575,11 +583,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);
}
@@ -601,7 +609,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;
@@ -609,7 +617,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;
@@ -629,7 +637,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);
}
}
@@ -738,7 +746,7 @@ static void arm_smmu_init_context_bank(struct arm_smmu_domain *smmu_domain,
#endif
/* if 16bit VMID supported set VMID in CBA2R */
if (smmu->features & ARM_SMMU_FEAT_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));
}
@@ -757,7 +765,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->features & ARM_SMMU_FEAT_VMID16)) {
/*16 bit VMID is not supported 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));
@@ -765,11 +773,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;
@@ -1717,6 +1725,7 @@ static const struct of_device_id arm_smmu_of_match[] = {
{ .compatible = "arm,mmu-400", .data = (void *)ARM_SMMU_V1 },
{ .compatible = "arm,mmu-401", .data = (void *)ARM_SMMU_V1 },
{ .compatible = "arm,mmu-500", .data = (void *)ARM_SMMU_V2 },
+ { .compatible = "cavium,smmu-v2", .data = (void *)ARM_SMMU_V2 },
{ },
};
MODULE_DEVICE_TABLE(of, arm_smmu_of_match);
@@ -1827,6 +1836,23 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev)
}
}
+ /*
+ * 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. see if this is a Cavium SMMU, if so
+ * set asid and vmid base such that each SMMU gets unique
+ * asid/vmid space.
+ */
+ if (!strcasecmp(of_id->compatible, "cavium,smmu-v2")) {
+ /* VMID16 must be present on Cavium SMMUv2*/
+ if (!(smmu->features & ARM_SMMU_FEAT_VMID16))
+ goto out_free_irqs;
+ smmu->cavium_smmu_id = cavium_smmu_count;
+ cavium_smmu_count++;
+ smmu->cavium_id_base =
+ (smmu->cavium_smmu_id * ARM_SMMU_MAX_CBS);
+ }
+
INIT_LIST_HEAD(&smmu->list);
spin_lock(&arm_smmu_devices_lock);
list_add(&smmu->list, &arm_smmu_devices);
--
2.1.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH] iommu/arm-smmu-v2: Workaround for ThunderX errata#27704
@ 2016-02-24 21:13 ` Tirumalesh Chalamarla
0 siblings, 0 replies; 26+ messages in thread
From: Tirumalesh Chalamarla @ 2016-02-24 21:13 UTC (permalink / raw)
To: linux-arm-kernel
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 address these issuee by supplying asid and vmid
while calculating ASID and VMID for Thunder SMMUv2.
NOTE: resending with commit message fix.
changes from V2:
- removed *_base from DT, and replaced with compatible string
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: Tirumalesh Chalamarla <tchalamarla@caviumnetworks.com>
Signed-off-by: Akula Geethasowjanya <Geethasowjanya.Akula@caviumnetworks.com>
---
.../devicetree/bindings/iommu/arm,smmu.txt | 1 +
drivers/iommu/arm-smmu.c | 48 +++++++++++++++++-----
2 files changed, 38 insertions(+), 11 deletions(-)
diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.txt b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
index 7180745..19fe6f2 100644
--- a/Documentation/devicetree/bindings/iommu/arm,smmu.txt
+++ b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
@@ -16,6 +16,7 @@ conditions.
"arm,mmu-400"
"arm,mmu-401"
"arm,mmu-500"
+ "cavium,smmu-v2"
depending on the particular implementation and/or the
version of the architecture implemented.
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 247a469..c704f88 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -326,6 +326,12 @@ struct arm_smmu_device {
struct list_head list;
struct rb_root masters;
+ /*
+ *The following fields are specific to Cavium, Thunder
+ */
+ u32 cavium_smmu_id;
+ u32 cavium_id_base;
+
};
struct arm_smmu_cfg {
@@ -335,8 +341,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)->cavium_id_base + (cfg)->cbndx)
+#define ARM_SMMU_CB_VMID(smmu, cfg) ((u16)(smmu)->cavium_id_base + (cfg)->cbndx + 1)
enum arm_smmu_domain_stage {
ARM_SMMU_DOMAIN_S1 = 0,
@@ -364,6 +370,8 @@ struct arm_smmu_option_prop {
const char *prop;
};
+static int cavium_smmu_count;
+
static struct arm_smmu_option_prop arm_smmu_options[] = {
{ ARM_SMMU_OPT_SECURE_CFG_ACCESS, "calxeda,smmu-secure-config-access" },
{ 0, NULL},
@@ -575,11 +583,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);
}
@@ -601,7 +609,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;
@@ -609,7 +617,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;
@@ -629,7 +637,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);
}
}
@@ -738,7 +746,7 @@ static void arm_smmu_init_context_bank(struct arm_smmu_domain *smmu_domain,
#endif
/* if 16bit VMID supported set VMID in CBA2R */
if (smmu->features & ARM_SMMU_FEAT_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));
}
@@ -757,7 +765,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->features & ARM_SMMU_FEAT_VMID16)) {
/*16 bit VMID is not supported 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));
@@ -765,11 +773,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;
@@ -1717,6 +1725,7 @@ static const struct of_device_id arm_smmu_of_match[] = {
{ .compatible = "arm,mmu-400", .data = (void *)ARM_SMMU_V1 },
{ .compatible = "arm,mmu-401", .data = (void *)ARM_SMMU_V1 },
{ .compatible = "arm,mmu-500", .data = (void *)ARM_SMMU_V2 },
+ { .compatible = "cavium,smmu-v2", .data = (void *)ARM_SMMU_V2 },
{ },
};
MODULE_DEVICE_TABLE(of, arm_smmu_of_match);
@@ -1827,6 +1836,23 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev)
}
}
+ /*
+ * 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. see if this is a Cavium SMMU, if so
+ * set asid and vmid base such that each SMMU gets unique
+ * asid/vmid space.
+ */
+ if (!strcasecmp(of_id->compatible, "cavium,smmu-v2")) {
+ /* VMID16 must be present on Cavium SMMUv2*/
+ if (!(smmu->features & ARM_SMMU_FEAT_VMID16))
+ goto out_free_irqs;
+ smmu->cavium_smmu_id = cavium_smmu_count;
+ cavium_smmu_count++;
+ smmu->cavium_id_base =
+ (smmu->cavium_smmu_id * ARM_SMMU_MAX_CBS);
+ }
+
INIT_LIST_HEAD(&smmu->list);
spin_lock(&arm_smmu_devices_lock);
list_add(&smmu->list, &arm_smmu_devices);
--
2.1.0
^ permalink raw reply related [flat|nested] 26+ messages in thread[parent not found: <1456348433-3337-1-git-send-email-tchalamarla-M3mlKVOIwJVv6pq1l3V1OdBPR1lH4CV8@public.gmane.org>]
* Re: [PATCH] iommu/arm-smmu-v2: Workaround for ThunderX errata#27704
2016-02-24 21:13 ` Tirumalesh Chalamarla
@ 2016-03-02 3:07 ` Will Deacon
-1 siblings, 0 replies; 26+ messages in thread
From: Will Deacon @ 2016-03-02 3:07 UTC (permalink / raw)
To: Tirumalesh Chalamarla
Cc: mark.rutland-5wv7dgnIgG8,
Geethasowjanya.Akula-M3mlKVOIwJVv6pq1l3V1OdBPR1lH4CV8,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
On Wed, Feb 24, 2016 at 01:13:53PM -0800, Tirumalesh Chalamarla wrote:
> 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 address these issuee by supplying asid and vmid
> while calculating ASID and VMID for Thunder SMMUv2.
>
> NOTE: resending with commit message fix.
>
> changes from V2:
> - removed *_base from DT, and replaced with compatible string
>
> 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: Tirumalesh Chalamarla <tchalamarla-M3mlKVOIwJVv6pq1l3V1OdBPR1lH4CV8@public.gmane.org>
> Signed-off-by: Akula Geethasowjanya <Geethasowjanya.Akula-M3mlKVOIwJVv6pq1l3V1OdBPR1lH4CV8@public.gmane.org>
> ---
> .../devicetree/bindings/iommu/arm,smmu.txt | 1 +
> drivers/iommu/arm-smmu.c | 48 +++++++++++++++++-----
> 2 files changed, 38 insertions(+), 11 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.txt b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
> index 7180745..19fe6f2 100644
> --- a/Documentation/devicetree/bindings/iommu/arm,smmu.txt
> +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
> @@ -16,6 +16,7 @@ conditions.
> "arm,mmu-400"
> "arm,mmu-401"
> "arm,mmu-500"
> + "cavium,smmu-v2"
>
> depending on the particular implementation and/or the
> version of the architecture implemented.
> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> index 247a469..c704f88 100644
> --- a/drivers/iommu/arm-smmu.c
> +++ b/drivers/iommu/arm-smmu.c
> @@ -326,6 +326,12 @@ struct arm_smmu_device {
>
> struct list_head list;
> struct rb_root masters;
> + /*
> + *The following fields are specific to Cavium, Thunder
> + */
> + u32 cavium_smmu_id;
Why do you need this field as well as the base?
> + u32 cavium_id_base;
> +
> };
>
> struct arm_smmu_cfg {
> @@ -335,8 +341,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)->cavium_id_base + (cfg)->cbndx)
> +#define ARM_SMMU_CB_VMID(smmu, cfg) ((u16)(smmu)->cavium_id_base + (cfg)->cbndx + 1)
>
> enum arm_smmu_domain_stage {
> ARM_SMMU_DOMAIN_S1 = 0,
> @@ -364,6 +370,8 @@ struct arm_smmu_option_prop {
> const char *prop;
> };
>
> +static int cavium_smmu_count;
I'd be more comfortable if this was an atomic_t.
> + /*
> + * 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. see if this is a Cavium SMMU, if so
> + * set asid and vmid base such that each SMMU gets unique
> + * asid/vmid space.
> + */
> + if (!strcasecmp(of_id->compatible, "cavium,smmu-v2")) {
of_device_is_compatible
> + /* VMID16 must be present on Cavium SMMUv2*/
> + if (!(smmu->features & ARM_SMMU_FEAT_VMID16))
> + goto out_free_irqs;
This check seems a bit overkill. I think you can remove it.
> + smmu->cavium_smmu_id = cavium_smmu_count;
> + cavium_smmu_count++;
> + smmu->cavium_id_base =
> + (smmu->cavium_smmu_id * ARM_SMMU_MAX_CBS);
Can you not use num_context_banks here, instead of the constant?
Will
^ permalink raw reply [flat|nested] 26+ messages in thread* [PATCH] iommu/arm-smmu-v2: Workaround for ThunderX errata#27704
@ 2016-03-02 3:07 ` Will Deacon
0 siblings, 0 replies; 26+ messages in thread
From: Will Deacon @ 2016-03-02 3:07 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Feb 24, 2016 at 01:13:53PM -0800, Tirumalesh Chalamarla wrote:
> 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 address these issuee by supplying asid and vmid
> while calculating ASID and VMID for Thunder SMMUv2.
>
> NOTE: resending with commit message fix.
>
> changes from V2:
> - removed *_base from DT, and replaced with compatible string
>
> 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: Tirumalesh Chalamarla <tchalamarla@caviumnetworks.com>
> Signed-off-by: Akula Geethasowjanya <Geethasowjanya.Akula@caviumnetworks.com>
> ---
> .../devicetree/bindings/iommu/arm,smmu.txt | 1 +
> drivers/iommu/arm-smmu.c | 48 +++++++++++++++++-----
> 2 files changed, 38 insertions(+), 11 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.txt b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
> index 7180745..19fe6f2 100644
> --- a/Documentation/devicetree/bindings/iommu/arm,smmu.txt
> +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
> @@ -16,6 +16,7 @@ conditions.
> "arm,mmu-400"
> "arm,mmu-401"
> "arm,mmu-500"
> + "cavium,smmu-v2"
>
> depending on the particular implementation and/or the
> version of the architecture implemented.
> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> index 247a469..c704f88 100644
> --- a/drivers/iommu/arm-smmu.c
> +++ b/drivers/iommu/arm-smmu.c
> @@ -326,6 +326,12 @@ struct arm_smmu_device {
>
> struct list_head list;
> struct rb_root masters;
> + /*
> + *The following fields are specific to Cavium, Thunder
> + */
> + u32 cavium_smmu_id;
Why do you need this field as well as the base?
> + u32 cavium_id_base;
> +
> };
>
> struct arm_smmu_cfg {
> @@ -335,8 +341,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)->cavium_id_base + (cfg)->cbndx)
> +#define ARM_SMMU_CB_VMID(smmu, cfg) ((u16)(smmu)->cavium_id_base + (cfg)->cbndx + 1)
>
> enum arm_smmu_domain_stage {
> ARM_SMMU_DOMAIN_S1 = 0,
> @@ -364,6 +370,8 @@ struct arm_smmu_option_prop {
> const char *prop;
> };
>
> +static int cavium_smmu_count;
I'd be more comfortable if this was an atomic_t.
> + /*
> + * 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. see if this is a Cavium SMMU, if so
> + * set asid and vmid base such that each SMMU gets unique
> + * asid/vmid space.
> + */
> + if (!strcasecmp(of_id->compatible, "cavium,smmu-v2")) {
of_device_is_compatible
> + /* VMID16 must be present on Cavium SMMUv2*/
> + if (!(smmu->features & ARM_SMMU_FEAT_VMID16))
> + goto out_free_irqs;
This check seems a bit overkill. I think you can remove it.
> + smmu->cavium_smmu_id = cavium_smmu_count;
> + cavium_smmu_count++;
> + smmu->cavium_id_base =
> + (smmu->cavium_smmu_id * ARM_SMMU_MAX_CBS);
Can you not use num_context_banks here, instead of the constant?
Will
^ permalink raw reply [flat|nested] 26+ messages in thread[parent not found: <20160302030756.GC7637-5wv7dgnIgG8@public.gmane.org>]
* Re: [PATCH] iommu/arm-smmu-v2: Workaround for ThunderX errata#27704
2016-03-02 3:07 ` Will Deacon
@ 2016-03-02 3:23 ` Chalamarla, Tirumalesh
-1 siblings, 0 replies; 26+ messages in thread
From: Chalamarla, Tirumalesh @ 2016-03-02 3:23 UTC (permalink / raw)
To: Will Deacon
Cc: mark.rutland-5wv7dgnIgG8, Akula, Geethasowjanya,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
On 3/1/16, 7:07 PM, "Will Deacon" <will.deacon@arm.com> wrote:
>On Wed, Feb 24, 2016 at 01:13:53PM -0800, Tirumalesh Chalamarla wrote:
>> 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 address these issuee by supplying asid and vmid
>> while calculating ASID and VMID for Thunder SMMUv2.
>>
>> NOTE: resending with commit message fix.
>>
>> changes from V2:
>> - removed *_base from DT, and replaced with compatible string
>>
>> 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: Tirumalesh Chalamarla <tchalamarla@caviumnetworks.com>
>> Signed-off-by: Akula Geethasowjanya <Geethasowjanya.Akula@caviumnetworks.com>
>> ---
>> .../devicetree/bindings/iommu/arm,smmu.txt | 1 +
>> drivers/iommu/arm-smmu.c | 48 +++++++++++++++++-----
>> 2 files changed, 38 insertions(+), 11 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.txt b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
>> index 7180745..19fe6f2 100644
>> --- a/Documentation/devicetree/bindings/iommu/arm,smmu.txt
>> +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
>> @@ -16,6 +16,7 @@ conditions.
>> "arm,mmu-400"
>> "arm,mmu-401"
>> "arm,mmu-500"
>> + "cavium,smmu-v2"
>>
>> depending on the particular implementation and/or the
>> version of the architecture implemented.
>> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
>> index 247a469..c704f88 100644
>> --- a/drivers/iommu/arm-smmu.c
>> +++ b/drivers/iommu/arm-smmu.c
>> @@ -326,6 +326,12 @@ struct arm_smmu_device {
>>
>> struct list_head list;
>> struct rb_root masters;
>> + /*
>> + *The following fields are specific to Cavium, Thunder
>> + */
>> + u32 cavium_smmu_id;
>
>Why do you need this field as well as the base?
Will be removed.
>
>> + u32 cavium_id_base;
>> +
>> };
>>
>> struct arm_smmu_cfg {
>> @@ -335,8 +341,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)->cavium_id_base + (cfg)->cbndx)
>> +#define ARM_SMMU_CB_VMID(smmu, cfg) ((u16)(smmu)->cavium_id_base + (cfg)->cbndx + 1)
>>
>> enum arm_smmu_domain_stage {
>> ARM_SMMU_DOMAIN_S1 = 0,
>> @@ -364,6 +370,8 @@ struct arm_smmu_option_prop {
>> const char *prop;
>> };
>>
>> +static int cavium_smmu_count;
>
>I'd be more comfortable if this was an atomic_t.
Sure.
>
>> + /*
>> + * 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. see if this is a Cavium SMMU, if so
>> + * set asid and vmid base such that each SMMU gets unique
>> + * asid/vmid space.
>> + */
>> + if (!strcasecmp(of_id->compatible, "cavium,smmu-v2")) {
>
>of_device_is_compatible
Sure.
>
>> + /* VMID16 must be present on Cavium SMMUv2*/
>> + if (!(smmu->features & ARM_SMMU_FEAT_VMID16))
>> + goto out_free_irqs;
>
>This check seems a bit overkill. I think you can remove it.
I don’t think it is possible to support unique ASID space with out 16 bit VMID.
But I can remove this check if that’s implicit.
>
>> + smmu->cavium_smmu_id = cavium_smmu_count;
>> + cavium_smmu_count++;
>> + smmu->cavium_id_base =
>> + (smmu->cavium_smmu_id * ARM_SMMU_MAX_CBS);
>
>Can you not use num_context_banks here, instead of the constant?
We need total context banks so far, so ARM_SMMU_MAX_CBS is best option.
For Thunder both are same anyway.
>
>Will
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply [flat|nested] 26+ messages in thread* [PATCH] iommu/arm-smmu-v2: Workaround for ThunderX errata#27704
@ 2016-03-02 3:23 ` Chalamarla, Tirumalesh
0 siblings, 0 replies; 26+ messages in thread
From: Chalamarla, Tirumalesh @ 2016-03-02 3:23 UTC (permalink / raw)
To: linux-arm-kernel
On 3/1/16, 7:07 PM, "Will Deacon" <will.deacon@arm.com> wrote:
>On Wed, Feb 24, 2016 at 01:13:53PM -0800, Tirumalesh Chalamarla wrote:
>> 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 address these issuee by supplying asid and vmid
>> while calculating ASID and VMID for Thunder SMMUv2.
>>
>> NOTE: resending with commit message fix.
>>
>> changes from V2:
>> - removed *_base from DT, and replaced with compatible string
>>
>> 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: Tirumalesh Chalamarla <tchalamarla@caviumnetworks.com>
>> Signed-off-by: Akula Geethasowjanya <Geethasowjanya.Akula@caviumnetworks.com>
>> ---
>> .../devicetree/bindings/iommu/arm,smmu.txt | 1 +
>> drivers/iommu/arm-smmu.c | 48 +++++++++++++++++-----
>> 2 files changed, 38 insertions(+), 11 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.txt b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
>> index 7180745..19fe6f2 100644
>> --- a/Documentation/devicetree/bindings/iommu/arm,smmu.txt
>> +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
>> @@ -16,6 +16,7 @@ conditions.
>> "arm,mmu-400"
>> "arm,mmu-401"
>> "arm,mmu-500"
>> + "cavium,smmu-v2"
>>
>> depending on the particular implementation and/or the
>> version of the architecture implemented.
>> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
>> index 247a469..c704f88 100644
>> --- a/drivers/iommu/arm-smmu.c
>> +++ b/drivers/iommu/arm-smmu.c
>> @@ -326,6 +326,12 @@ struct arm_smmu_device {
>>
>> struct list_head list;
>> struct rb_root masters;
>> + /*
>> + *The following fields are specific to Cavium, Thunder
>> + */
>> + u32 cavium_smmu_id;
>
>Why do you need this field as well as the base?
Will be removed.
>
>> + u32 cavium_id_base;
>> +
>> };
>>
>> struct arm_smmu_cfg {
>> @@ -335,8 +341,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)->cavium_id_base + (cfg)->cbndx)
>> +#define ARM_SMMU_CB_VMID(smmu, cfg) ((u16)(smmu)->cavium_id_base + (cfg)->cbndx + 1)
>>
>> enum arm_smmu_domain_stage {
>> ARM_SMMU_DOMAIN_S1 = 0,
>> @@ -364,6 +370,8 @@ struct arm_smmu_option_prop {
>> const char *prop;
>> };
>>
>> +static int cavium_smmu_count;
>
>I'd be more comfortable if this was an atomic_t.
Sure.
>
>> + /*
>> + * 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. see if this is a Cavium SMMU, if so
>> + * set asid and vmid base such that each SMMU gets unique
>> + * asid/vmid space.
>> + */
>> + if (!strcasecmp(of_id->compatible, "cavium,smmu-v2")) {
>
>of_device_is_compatible
Sure.
>
>> + /* VMID16 must be present on Cavium SMMUv2*/
>> + if (!(smmu->features & ARM_SMMU_FEAT_VMID16))
>> + goto out_free_irqs;
>
>This check seems a bit overkill. I think you can remove it.
I don?t think it is possible to support unique ASID space with out 16 bit VMID.
But I can remove this check if that?s implicit.
>
>> + smmu->cavium_smmu_id = cavium_smmu_count;
>> + cavium_smmu_count++;
>> + smmu->cavium_id_base =
>> + (smmu->cavium_smmu_id * ARM_SMMU_MAX_CBS);
>
>Can you not use num_context_banks here, instead of the constant?
We need total context banks so far, so ARM_SMMU_MAX_CBS is best option.
For Thunder both are same anyway.
>
>Will
^ permalink raw reply [flat|nested] 26+ messages in thread[parent not found: <C2D36A36-8247-4432-BE50-14129D9D8E5B-M3mlKVOIwJVv6pq1l3V1OdBPR1lH4CV8@public.gmane.org>]
* Re: [PATCH] iommu/arm-smmu-v2: Workaround for ThunderX errata#27704
2016-03-02 3:23 ` Chalamarla, Tirumalesh
@ 2016-03-02 13:35 ` Will Deacon
-1 siblings, 0 replies; 26+ messages in thread
From: Will Deacon @ 2016-03-02 13:35 UTC (permalink / raw)
To: Chalamarla, Tirumalesh
Cc: mark.rutland-5wv7dgnIgG8, Akula, Geethasowjanya,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
On Wed, Mar 02, 2016 at 03:23:57AM +0000, Chalamarla, Tirumalesh wrote:
> On 3/1/16, 7:07 PM, "Will Deacon" <will.deacon-5wv7dgnIgG8@public.gmane.org> wrote:
> >On Wed, Feb 24, 2016 at 01:13:53PM -0800, Tirumalesh Chalamarla wrote:
> >> + smmu->cavium_smmu_id = cavium_smmu_count;
> >> + cavium_smmu_count++;
> >> + smmu->cavium_id_base =
> >> + (smmu->cavium_smmu_id * ARM_SMMU_MAX_CBS);
> >
> >Can you not use num_context_banks here, instead of the constant?
> We need total context banks so far, so ARM_SMMU_MAX_CBS is best option.
> For Thunder both are same anyway.
Hmm, so couldn't you instead just update a running total as you go along?
That is, initialise it to zero, then atomic_add_return(num_context_banks)
when you probe?
Will
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH] iommu/arm-smmu-v2: Workaround for ThunderX errata#27704
@ 2016-03-02 13:35 ` Will Deacon
0 siblings, 0 replies; 26+ messages in thread
From: Will Deacon @ 2016-03-02 13:35 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Mar 02, 2016 at 03:23:57AM +0000, Chalamarla, Tirumalesh wrote:
> On 3/1/16, 7:07 PM, "Will Deacon" <will.deacon@arm.com> wrote:
> >On Wed, Feb 24, 2016 at 01:13:53PM -0800, Tirumalesh Chalamarla wrote:
> >> + smmu->cavium_smmu_id = cavium_smmu_count;
> >> + cavium_smmu_count++;
> >> + smmu->cavium_id_base =
> >> + (smmu->cavium_smmu_id * ARM_SMMU_MAX_CBS);
> >
> >Can you not use num_context_banks here, instead of the constant?
> We need total context banks so far, so ARM_SMMU_MAX_CBS is best option.
> For Thunder both are same anyway.
Hmm, so couldn't you instead just update a running total as you go along?
That is, initialise it to zero, then atomic_add_return(num_context_banks)
when you probe?
Will
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] iommu/arm-smmu-v2: Workaround for ThunderX errata#27704
2016-02-24 21:13 ` Tirumalesh Chalamarla
@ 2016-03-02 13:10 ` Robin Murphy
-1 siblings, 0 replies; 26+ messages in thread
From: Robin Murphy @ 2016-03-02 13:10 UTC (permalink / raw)
To: Tirumalesh Chalamarla, will.deacon-5wv7dgnIgG8, mark.rutland-5wv7dgnIgG8
Cc: iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
Geethasowjanya.Akula-M3mlKVOIwJVv6pq1l3V1OdBPR1lH4CV8,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
On 24/02/16 21:13, Tirumalesh Chalamarla wrote:
> 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 address these issuee by supplying asid and vmid
> while calculating ASID and VMID for Thunder SMMUv2.
>
> NOTE: resending with commit message fix.
>
> changes from V2:
> - removed *_base from DT, and replaced with compatible string
>
> 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: Tirumalesh Chalamarla <tchalamarla-M3mlKVOIwJVv6pq1l3V1OdBPR1lH4CV8@public.gmane.org>
> Signed-off-by: Akula Geethasowjanya <Geethasowjanya.Akula-M3mlKVOIwJVv6pq1l3V1OdBPR1lH4CV8@public.gmane.org>
> ---
> .../devicetree/bindings/iommu/arm,smmu.txt | 1 +
> drivers/iommu/arm-smmu.c | 48 +++++++++++++++++-----
I guess Documentation/arm64/silicon-errata.txt wants updating too?
> 2 files changed, 38 insertions(+), 11 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.txt b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
> index 7180745..19fe6f2 100644
> --- a/Documentation/devicetree/bindings/iommu/arm,smmu.txt
> +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
> @@ -16,6 +16,7 @@ conditions.
> "arm,mmu-400"
> "arm,mmu-401"
> "arm,mmu-500"
> + "cavium,smmu-v2"
That seems a rather broad name for a compatible - will there definitely
never be another Cavium implementation of SMMUv2? Even then, if there is
future respin of CN88xx silicon where the SMMU needs some other
workaround for some other issue, will that be discoverable from
SMMU_IDR7 or similar? Even an absurdly over-specific compatible such as
"cavium,thunderx-cn88xx-pass2-smmu" would be preferable to something
which later turns out to be insufficient to uniquely identify a specific
implementation.
Otherwise, modulo Will's comments, this looks a lot more reasonable than
before, thanks.
Robin.
> depending on the particular implementation and/or the
> version of the architecture implemented.
> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> index 247a469..c704f88 100644
> --- a/drivers/iommu/arm-smmu.c
> +++ b/drivers/iommu/arm-smmu.c
> @@ -326,6 +326,12 @@ struct arm_smmu_device {
>
> struct list_head list;
> struct rb_root masters;
> + /*
> + *The following fields are specific to Cavium, Thunder
> + */
> + u32 cavium_smmu_id;
> + u32 cavium_id_base;
> +
> };
>
> struct arm_smmu_cfg {
> @@ -335,8 +341,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)->cavium_id_base + (cfg)->cbndx)
> +#define ARM_SMMU_CB_VMID(smmu, cfg) ((u16)(smmu)->cavium_id_base + (cfg)->cbndx + 1)
>
> enum arm_smmu_domain_stage {
> ARM_SMMU_DOMAIN_S1 = 0,
> @@ -364,6 +370,8 @@ struct arm_smmu_option_prop {
> const char *prop;
> };
>
> +static int cavium_smmu_count;
> +
> static struct arm_smmu_option_prop arm_smmu_options[] = {
> { ARM_SMMU_OPT_SECURE_CFG_ACCESS, "calxeda,smmu-secure-config-access" },
> { 0, NULL},
> @@ -575,11 +583,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);
> }
>
> @@ -601,7 +609,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;
> @@ -609,7 +617,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;
> @@ -629,7 +637,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);
> }
> }
>
> @@ -738,7 +746,7 @@ static void arm_smmu_init_context_bank(struct arm_smmu_domain *smmu_domain,
> #endif
> /* if 16bit VMID supported set VMID in CBA2R */
> if (smmu->features & ARM_SMMU_FEAT_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));
> }
> @@ -757,7 +765,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->features & ARM_SMMU_FEAT_VMID16)) {
> /*16 bit VMID is not supported 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));
>
> @@ -765,11 +773,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;
> @@ -1717,6 +1725,7 @@ static const struct of_device_id arm_smmu_of_match[] = {
> { .compatible = "arm,mmu-400", .data = (void *)ARM_SMMU_V1 },
> { .compatible = "arm,mmu-401", .data = (void *)ARM_SMMU_V1 },
> { .compatible = "arm,mmu-500", .data = (void *)ARM_SMMU_V2 },
> + { .compatible = "cavium,smmu-v2", .data = (void *)ARM_SMMU_V2 },
> { },
> };
> MODULE_DEVICE_TABLE(of, arm_smmu_of_match);
> @@ -1827,6 +1836,23 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev)
> }
> }
>
> + /*
> + * 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. see if this is a Cavium SMMU, if so
> + * set asid and vmid base such that each SMMU gets unique
> + * asid/vmid space.
> + */
> + if (!strcasecmp(of_id->compatible, "cavium,smmu-v2")) {
> + /* VMID16 must be present on Cavium SMMUv2*/
> + if (!(smmu->features & ARM_SMMU_FEAT_VMID16))
> + goto out_free_irqs;
> + smmu->cavium_smmu_id = cavium_smmu_count;
> + cavium_smmu_count++;
> + smmu->cavium_id_base =
> + (smmu->cavium_smmu_id * ARM_SMMU_MAX_CBS);
> + }
> +
> INIT_LIST_HEAD(&smmu->list);
> spin_lock(&arm_smmu_devices_lock);
> list_add(&smmu->list, &arm_smmu_devices);
>
^ permalink raw reply [flat|nested] 26+ messages in thread* [PATCH] iommu/arm-smmu-v2: Workaround for ThunderX errata#27704
@ 2016-03-02 13:10 ` Robin Murphy
0 siblings, 0 replies; 26+ messages in thread
From: Robin Murphy @ 2016-03-02 13:10 UTC (permalink / raw)
To: linux-arm-kernel
On 24/02/16 21:13, Tirumalesh Chalamarla wrote:
> 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 address these issuee by supplying asid and vmid
> while calculating ASID and VMID for Thunder SMMUv2.
>
> NOTE: resending with commit message fix.
>
> changes from V2:
> - removed *_base from DT, and replaced with compatible string
>
> 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: Tirumalesh Chalamarla <tchalamarla@caviumnetworks.com>
> Signed-off-by: Akula Geethasowjanya <Geethasowjanya.Akula@caviumnetworks.com>
> ---
> .../devicetree/bindings/iommu/arm,smmu.txt | 1 +
> drivers/iommu/arm-smmu.c | 48 +++++++++++++++++-----
I guess Documentation/arm64/silicon-errata.txt wants updating too?
> 2 files changed, 38 insertions(+), 11 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.txt b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
> index 7180745..19fe6f2 100644
> --- a/Documentation/devicetree/bindings/iommu/arm,smmu.txt
> +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
> @@ -16,6 +16,7 @@ conditions.
> "arm,mmu-400"
> "arm,mmu-401"
> "arm,mmu-500"
> + "cavium,smmu-v2"
That seems a rather broad name for a compatible - will there definitely
never be another Cavium implementation of SMMUv2? Even then, if there is
future respin of CN88xx silicon where the SMMU needs some other
workaround for some other issue, will that be discoverable from
SMMU_IDR7 or similar? Even an absurdly over-specific compatible such as
"cavium,thunderx-cn88xx-pass2-smmu" would be preferable to something
which later turns out to be insufficient to uniquely identify a specific
implementation.
Otherwise, modulo Will's comments, this looks a lot more reasonable than
before, thanks.
Robin.
> depending on the particular implementation and/or the
> version of the architecture implemented.
> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> index 247a469..c704f88 100644
> --- a/drivers/iommu/arm-smmu.c
> +++ b/drivers/iommu/arm-smmu.c
> @@ -326,6 +326,12 @@ struct arm_smmu_device {
>
> struct list_head list;
> struct rb_root masters;
> + /*
> + *The following fields are specific to Cavium, Thunder
> + */
> + u32 cavium_smmu_id;
> + u32 cavium_id_base;
> +
> };
>
> struct arm_smmu_cfg {
> @@ -335,8 +341,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)->cavium_id_base + (cfg)->cbndx)
> +#define ARM_SMMU_CB_VMID(smmu, cfg) ((u16)(smmu)->cavium_id_base + (cfg)->cbndx + 1)
>
> enum arm_smmu_domain_stage {
> ARM_SMMU_DOMAIN_S1 = 0,
> @@ -364,6 +370,8 @@ struct arm_smmu_option_prop {
> const char *prop;
> };
>
> +static int cavium_smmu_count;
> +
> static struct arm_smmu_option_prop arm_smmu_options[] = {
> { ARM_SMMU_OPT_SECURE_CFG_ACCESS, "calxeda,smmu-secure-config-access" },
> { 0, NULL},
> @@ -575,11 +583,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);
> }
>
> @@ -601,7 +609,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;
> @@ -609,7 +617,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;
> @@ -629,7 +637,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);
> }
> }
>
> @@ -738,7 +746,7 @@ static void arm_smmu_init_context_bank(struct arm_smmu_domain *smmu_domain,
> #endif
> /* if 16bit VMID supported set VMID in CBA2R */
> if (smmu->features & ARM_SMMU_FEAT_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));
> }
> @@ -757,7 +765,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->features & ARM_SMMU_FEAT_VMID16)) {
> /*16 bit VMID is not supported 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));
>
> @@ -765,11 +773,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;
> @@ -1717,6 +1725,7 @@ static const struct of_device_id arm_smmu_of_match[] = {
> { .compatible = "arm,mmu-400", .data = (void *)ARM_SMMU_V1 },
> { .compatible = "arm,mmu-401", .data = (void *)ARM_SMMU_V1 },
> { .compatible = "arm,mmu-500", .data = (void *)ARM_SMMU_V2 },
> + { .compatible = "cavium,smmu-v2", .data = (void *)ARM_SMMU_V2 },
> { },
> };
> MODULE_DEVICE_TABLE(of, arm_smmu_of_match);
> @@ -1827,6 +1836,23 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev)
> }
> }
>
> + /*
> + * 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. see if this is a Cavium SMMU, if so
> + * set asid and vmid base such that each SMMU gets unique
> + * asid/vmid space.
> + */
> + if (!strcasecmp(of_id->compatible, "cavium,smmu-v2")) {
> + /* VMID16 must be present on Cavium SMMUv2*/
> + if (!(smmu->features & ARM_SMMU_FEAT_VMID16))
> + goto out_free_irqs;
> + smmu->cavium_smmu_id = cavium_smmu_count;
> + cavium_smmu_count++;
> + smmu->cavium_id_base =
> + (smmu->cavium_smmu_id * ARM_SMMU_MAX_CBS);
> + }
> +
> INIT_LIST_HEAD(&smmu->list);
> spin_lock(&arm_smmu_devices_lock);
> list_add(&smmu->list, &arm_smmu_devices);
>
^ permalink raw reply [flat|nested] 26+ messages in thread[parent not found: <56D6E658.1060201-5wv7dgnIgG8@public.gmane.org>]
* Re: [PATCH] iommu/arm-smmu-v2: Workaround for ThunderX errata#27704
2016-03-02 13:10 ` Robin Murphy
@ 2016-03-02 18:12 ` Tirumalesh Chalamarla
-1 siblings, 0 replies; 26+ messages in thread
From: Tirumalesh Chalamarla @ 2016-03-02 18:12 UTC (permalink / raw)
To: Robin Murphy, will.deacon-5wv7dgnIgG8, mark.rutland-5wv7dgnIgG8
Cc: iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
Geethasowjanya.Akula-M3mlKVOIwJVv6pq1l3V1OdBPR1lH4CV8,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
On 03/02/2016 05:10 AM, Robin Murphy wrote:
> On 24/02/16 21:13, Tirumalesh Chalamarla wrote:
>> 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 address these issuee by supplying asid and vmid
>> while calculating ASID and VMID for Thunder SMMUv2.
>>
>> NOTE: resending with commit message fix.
>>
>> changes from V2:
>> - removed *_base from DT, and replaced with compatible string
>>
>> 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: Tirumalesh Chalamarla <tchalamarla-M3mlKVOIwJVv6pq1l3V1OdBPR1lH4CV8@public.gmane.org>
>> Signed-off-by: Akula Geethasowjanya
>> <Geethasowjanya.Akula-M3mlKVOIwJVv6pq1l3V1OdBPR1lH4CV8@public.gmane.org>
>> ---
>> .../devicetree/bindings/iommu/arm,smmu.txt | 1 +
>> drivers/iommu/arm-smmu.c | 48
>> +++++++++++++++++-----
>
> I guess Documentation/arm64/silicon-errata.txt wants updating too?
>
YES, will update it.
>> 2 files changed, 38 insertions(+), 11 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.txt
>> b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
>> index 7180745..19fe6f2 100644
>> --- a/Documentation/devicetree/bindings/iommu/arm,smmu.txt
>> +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
>> @@ -16,6 +16,7 @@ conditions.
>> "arm,mmu-400"
>> "arm,mmu-401"
>> "arm,mmu-500"
>> + "cavium,smmu-v2"
>
> That seems a rather broad name for a compatible - will there definitely
> never be another Cavium implementation of SMMUv2? Even then, if there is
> future respin of CN88xx silicon where the SMMU needs some other
> workaround for some other issue, will that be discoverable from
> SMMU_IDR7 or similar? Even an absurdly over-specific compatible such as
> "cavium,thunderx-cn88xx-pass2-smmu" would be preferable to something
> which later turns out to be insufficient to uniquely identify a specific
> implementation.
>
i don't think there is a SMMUv2 from Cavium again, there will be SMMUv3
of course.
there will be other flavors like 86xx etc, which has same implementation
as 88xx so will have same errata. i think this name is more appropriate.
> Otherwise, modulo Will's comments, this looks a lot more reasonable than
> before, thanks.
>
> Robin.
>
>> depending on the particular implementation and/or the
>> version of the architecture implemented.
>> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
>> index 247a469..c704f88 100644
>> --- a/drivers/iommu/arm-smmu.c
>> +++ b/drivers/iommu/arm-smmu.c
>> @@ -326,6 +326,12 @@ struct arm_smmu_device {
>>
>> struct list_head list;
>> struct rb_root masters;
>> + /*
>> + *The following fields are specific to Cavium, Thunder
>> + */
>> + u32 cavium_smmu_id;
>> + u32 cavium_id_base;
>> +
>> };
>>
>> struct arm_smmu_cfg {
>> @@ -335,8 +341,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)->cavium_id_base +
>> (cfg)->cbndx)
>> +#define ARM_SMMU_CB_VMID(smmu, cfg) ((u16)(smmu)->cavium_id_base +
>> (cfg)->cbndx + 1)
>>
>> enum arm_smmu_domain_stage {
>> ARM_SMMU_DOMAIN_S1 = 0,
>> @@ -364,6 +370,8 @@ struct arm_smmu_option_prop {
>> const char *prop;
>> };
>>
>> +static int cavium_smmu_count;
>> +
>> static struct arm_smmu_option_prop arm_smmu_options[] = {
>> { ARM_SMMU_OPT_SECURE_CFG_ACCESS,
>> "calxeda,smmu-secure-config-access" },
>> { 0, NULL},
>> @@ -575,11 +583,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);
>> }
>>
>> @@ -601,7 +609,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;
>> @@ -609,7 +617,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;
>> @@ -629,7 +637,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);
>> }
>> }
>>
>> @@ -738,7 +746,7 @@ static void arm_smmu_init_context_bank(struct
>> arm_smmu_domain *smmu_domain,
>> #endif
>> /* if 16bit VMID supported set VMID in CBA2R */
>> if (smmu->features & ARM_SMMU_FEAT_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));
>> }
>> @@ -757,7 +765,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->features & ARM_SMMU_FEAT_VMID16)) {
>> /*16 bit VMID is not supported 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));
>>
>> @@ -765,11 +773,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;
>> @@ -1717,6 +1725,7 @@ static const struct of_device_id
>> arm_smmu_of_match[] = {
>> { .compatible = "arm,mmu-400", .data = (void *)ARM_SMMU_V1 },
>> { .compatible = "arm,mmu-401", .data = (void *)ARM_SMMU_V1 },
>> { .compatible = "arm,mmu-500", .data = (void *)ARM_SMMU_V2 },
>> + { .compatible = "cavium,smmu-v2", .data = (void *)ARM_SMMU_V2 },
>> { },
>> };
>> MODULE_DEVICE_TABLE(of, arm_smmu_of_match);
>> @@ -1827,6 +1836,23 @@ static int arm_smmu_device_dt_probe(struct
>> platform_device *pdev)
>> }
>> }
>>
>> + /*
>> + * 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. see if this is a Cavium SMMU, if so
>> + * set asid and vmid base such that each SMMU gets unique
>> + * asid/vmid space.
>> + */
>> + if (!strcasecmp(of_id->compatible, "cavium,smmu-v2")) {
>> + /* VMID16 must be present on Cavium SMMUv2*/
>> + if (!(smmu->features & ARM_SMMU_FEAT_VMID16))
>> + goto out_free_irqs;
>> + smmu->cavium_smmu_id = cavium_smmu_count;
>> + cavium_smmu_count++;
>> + smmu->cavium_id_base =
>> + (smmu->cavium_smmu_id * ARM_SMMU_MAX_CBS);
>> + }
>> +
>> INIT_LIST_HEAD(&smmu->list);
>> spin_lock(&arm_smmu_devices_lock);
>> list_add(&smmu->list, &arm_smmu_devices);
>>
>
^ permalink raw reply [flat|nested] 26+ messages in thread* [PATCH] iommu/arm-smmu-v2: Workaround for ThunderX errata#27704
@ 2016-03-02 18:12 ` Tirumalesh Chalamarla
0 siblings, 0 replies; 26+ messages in thread
From: Tirumalesh Chalamarla @ 2016-03-02 18:12 UTC (permalink / raw)
To: linux-arm-kernel
On 03/02/2016 05:10 AM, Robin Murphy wrote:
> On 24/02/16 21:13, Tirumalesh Chalamarla wrote:
>> 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 address these issuee by supplying asid and vmid
>> while calculating ASID and VMID for Thunder SMMUv2.
>>
>> NOTE: resending with commit message fix.
>>
>> changes from V2:
>> - removed *_base from DT, and replaced with compatible string
>>
>> 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: Tirumalesh Chalamarla <tchalamarla@caviumnetworks.com>
>> Signed-off-by: Akula Geethasowjanya
>> <Geethasowjanya.Akula@caviumnetworks.com>
>> ---
>> .../devicetree/bindings/iommu/arm,smmu.txt | 1 +
>> drivers/iommu/arm-smmu.c | 48
>> +++++++++++++++++-----
>
> I guess Documentation/arm64/silicon-errata.txt wants updating too?
>
YES, will update it.
>> 2 files changed, 38 insertions(+), 11 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.txt
>> b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
>> index 7180745..19fe6f2 100644
>> --- a/Documentation/devicetree/bindings/iommu/arm,smmu.txt
>> +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
>> @@ -16,6 +16,7 @@ conditions.
>> "arm,mmu-400"
>> "arm,mmu-401"
>> "arm,mmu-500"
>> + "cavium,smmu-v2"
>
> That seems a rather broad name for a compatible - will there definitely
> never be another Cavium implementation of SMMUv2? Even then, if there is
> future respin of CN88xx silicon where the SMMU needs some other
> workaround for some other issue, will that be discoverable from
> SMMU_IDR7 or similar? Even an absurdly over-specific compatible such as
> "cavium,thunderx-cn88xx-pass2-smmu" would be preferable to something
> which later turns out to be insufficient to uniquely identify a specific
> implementation.
>
i don't think there is a SMMUv2 from Cavium again, there will be SMMUv3
of course.
there will be other flavors like 86xx etc, which has same implementation
as 88xx so will have same errata. i think this name is more appropriate.
> Otherwise, modulo Will's comments, this looks a lot more reasonable than
> before, thanks.
>
> Robin.
>
>> depending on the particular implementation and/or the
>> version of the architecture implemented.
>> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
>> index 247a469..c704f88 100644
>> --- a/drivers/iommu/arm-smmu.c
>> +++ b/drivers/iommu/arm-smmu.c
>> @@ -326,6 +326,12 @@ struct arm_smmu_device {
>>
>> struct list_head list;
>> struct rb_root masters;
>> + /*
>> + *The following fields are specific to Cavium, Thunder
>> + */
>> + u32 cavium_smmu_id;
>> + u32 cavium_id_base;
>> +
>> };
>>
>> struct arm_smmu_cfg {
>> @@ -335,8 +341,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)->cavium_id_base +
>> (cfg)->cbndx)
>> +#define ARM_SMMU_CB_VMID(smmu, cfg) ((u16)(smmu)->cavium_id_base +
>> (cfg)->cbndx + 1)
>>
>> enum arm_smmu_domain_stage {
>> ARM_SMMU_DOMAIN_S1 = 0,
>> @@ -364,6 +370,8 @@ struct arm_smmu_option_prop {
>> const char *prop;
>> };
>>
>> +static int cavium_smmu_count;
>> +
>> static struct arm_smmu_option_prop arm_smmu_options[] = {
>> { ARM_SMMU_OPT_SECURE_CFG_ACCESS,
>> "calxeda,smmu-secure-config-access" },
>> { 0, NULL},
>> @@ -575,11 +583,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);
>> }
>>
>> @@ -601,7 +609,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;
>> @@ -609,7 +617,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;
>> @@ -629,7 +637,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);
>> }
>> }
>>
>> @@ -738,7 +746,7 @@ static void arm_smmu_init_context_bank(struct
>> arm_smmu_domain *smmu_domain,
>> #endif
>> /* if 16bit VMID supported set VMID in CBA2R */
>> if (smmu->features & ARM_SMMU_FEAT_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));
>> }
>> @@ -757,7 +765,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->features & ARM_SMMU_FEAT_VMID16)) {
>> /*16 bit VMID is not supported 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));
>>
>> @@ -765,11 +773,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;
>> @@ -1717,6 +1725,7 @@ static const struct of_device_id
>> arm_smmu_of_match[] = {
>> { .compatible = "arm,mmu-400", .data = (void *)ARM_SMMU_V1 },
>> { .compatible = "arm,mmu-401", .data = (void *)ARM_SMMU_V1 },
>> { .compatible = "arm,mmu-500", .data = (void *)ARM_SMMU_V2 },
>> + { .compatible = "cavium,smmu-v2", .data = (void *)ARM_SMMU_V2 },
>> { },
>> };
>> MODULE_DEVICE_TABLE(of, arm_smmu_of_match);
>> @@ -1827,6 +1836,23 @@ static int arm_smmu_device_dt_probe(struct
>> platform_device *pdev)
>> }
>> }
>>
>> + /*
>> + * 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. see if this is a Cavium SMMU, if so
>> + * set asid and vmid base such that each SMMU gets unique
>> + * asid/vmid space.
>> + */
>> + if (!strcasecmp(of_id->compatible, "cavium,smmu-v2")) {
>> + /* VMID16 must be present on Cavium SMMUv2*/
>> + if (!(smmu->features & ARM_SMMU_FEAT_VMID16))
>> + goto out_free_irqs;
>> + smmu->cavium_smmu_id = cavium_smmu_count;
>> + cavium_smmu_count++;
>> + smmu->cavium_id_base =
>> + (smmu->cavium_smmu_id * ARM_SMMU_MAX_CBS);
>> + }
>> +
>> INIT_LIST_HEAD(&smmu->list);
>> spin_lock(&arm_smmu_devices_lock);
>> list_add(&smmu->list, &arm_smmu_devices);
>>
>
^ permalink raw reply [flat|nested] 26+ messages in thread
end of thread, other threads:[~2016-03-02 18:12 UTC | newest]
Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-05 18:47 [PATCH] iommu/arm-smmu-v2: Workaround for ThunderX errata#27704 tchalamarla-M3mlKVOIwJVv6pq1l3V1OdBPR1lH4CV8
2016-02-05 18:47 ` tchalamarla at caviumnetworks.com
[not found] ` <1454698027-20911-1-git-send-email-tchalamarla-M3mlKVOIwJVv6pq1l3V1OdBPR1lH4CV8@public.gmane.org>
2016-02-05 20:02 ` Mark Rutland
2016-02-05 20:02 ` Mark Rutland
2016-02-05 20:15 ` Mark Rutland
2016-02-05 20:15 ` Mark Rutland
2016-02-05 20:29 ` Chalamarla, Tirumalesh
2016-02-05 20:29 ` Chalamarla, Tirumalesh
2016-02-05 20:32 ` Chalamarla, Tirumalesh
2016-02-05 20:32 ` Chalamarla, Tirumalesh
2016-02-09 11:52 ` Robin Murphy
2016-02-09 11:52 ` Robin Murphy
[not found] ` <56B9D2F4.8070308-5wv7dgnIgG8@public.gmane.org>
2016-02-09 17:07 ` Chalamarla, Tirumalesh
2016-02-09 17:07 ` Chalamarla, Tirumalesh
2016-02-24 21:13 Tirumalesh Chalamarla
2016-02-24 21:13 ` Tirumalesh Chalamarla
[not found] ` <1456348433-3337-1-git-send-email-tchalamarla-M3mlKVOIwJVv6pq1l3V1OdBPR1lH4CV8@public.gmane.org>
2016-03-02 3:07 ` Will Deacon
2016-03-02 3:07 ` Will Deacon
[not found] ` <20160302030756.GC7637-5wv7dgnIgG8@public.gmane.org>
2016-03-02 3:23 ` Chalamarla, Tirumalesh
2016-03-02 3:23 ` Chalamarla, Tirumalesh
[not found] ` <C2D36A36-8247-4432-BE50-14129D9D8E5B-M3mlKVOIwJVv6pq1l3V1OdBPR1lH4CV8@public.gmane.org>
2016-03-02 13:35 ` Will Deacon
2016-03-02 13:35 ` Will Deacon
2016-03-02 13:10 ` Robin Murphy
2016-03-02 13:10 ` Robin Murphy
[not found] ` <56D6E658.1060201-5wv7dgnIgG8@public.gmane.org>
2016-03-02 18:12 ` Tirumalesh Chalamarla
2016-03-02 18:12 ` Tirumalesh Chalamarla
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.