From: Robert Richter <[email protected]>
This patch series adds gicv3 updates and workarounds for HW errata in
Cavium's ThunderX GICV3.
The first one is an unchanged resubmission of a patch from a gicv3
series I sent a while ago.
The next patches implement the workarounds for ThunderX's gicv3. Patch
#2 adds generic code to parse the hw revision provided by an IIDR or
MIDR register value and runs specific code if hw matches. For MIDR
detection it uses the arm64 errata framework. This patch is used for
the implementation of the actual errata fixes in patch #3 (gicv3) and
#5 (gicv3-its). Patch #4 is a prerequisit for patch #5. Patch #6 is a
change to the errata framework to only check for cpu features if the
capability value is non-zero.
All current review comments addressed so far with v3.
v3:
* use arm64 errata framework for midr check
* fix mixup of errata to be dependend from midr/iidr
v2:
* Workaround for 23154:
* implement code in a single asm() to keep instruction sequence
* added comment to the code that explains the erratum
* apply workaround also if running as guest, thus check MIDR
* adding MIDR check
Robert Richter (6):
irqchip, gicv3-its: Add range check for number of allocated pages
irqchip, gicv3: Add HW revision detection and configuration
irqchip, gicv3: Workaround for Cavium ThunderX erratum 23154
irqchip, gicv3-its: Read typer register outside the loop
irqchip, gicv3-its: Workaround for Cavium ThunderX errata 22375, 24313
arm64: errata: Match all cpus if capability value is zero
arch/arm64/Kconfig | 11 +++++++
arch/arm64/include/asm/cpufeature.h | 16 +++++++---
arch/arm64/include/asm/cputype.h | 18 ++++++-----
arch/arm64/kernel/cpu_errata.c | 9 ++++++
drivers/irqchip/irq-gic-common.c | 13 ++++++++
drivers/irqchip/irq-gic-common.h | 10 ++++++
drivers/irqchip/irq-gic-v3-its.c | 62 ++++++++++++++++++++++++++++++++----
drivers/irqchip/irq-gic-v3.c | 63 ++++++++++++++++++++++++++++++++++++-
include/linux/irqchip/arm-gic-v3.h | 1 +
9 files changed, 184 insertions(+), 19 deletions(-)
--
2.1.1
From: Robert Richter <[email protected]>
The number of pages for the its table may exceed the maximum of 256.
Adding a range check and limitting the number to its maximum.
Based on a patch from Tirumalesh Chalamarla <[email protected]>.
Signed-off-by: Tirumalesh Chalamarla <[email protected]>
Reviewed-by: Marc Zyngier <[email protected]>
Signed-off-by: Robert Richter <[email protected]>
---
drivers/irqchip/irq-gic-v3-its.c | 11 ++++++++++-
include/linux/irqchip/arm-gic-v3.h | 1 +
2 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 1b7e155869f6..466edf8a7477 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -810,6 +810,7 @@ static int its_alloc_tables(struct its_node *its)
u64 entry_size = GITS_BASER_ENTRY_SIZE(val);
int order = get_order(psz);
int alloc_size;
+ int alloc_pages;
u64 tmp;
void *base;
@@ -844,6 +845,14 @@ static int its_alloc_tables(struct its_node *its)
}
alloc_size = (1 << order) * PAGE_SIZE;
+ alloc_pages = (alloc_size / psz);
+ if (alloc_pages > GITS_BASER_PAGES_MAX) {
+ alloc_pages = GITS_BASER_PAGES_MAX;
+ order = get_order(GITS_BASER_PAGES_MAX * psz);
+ pr_warn("%s: Device Table too large, reduce its page order to %u (%u pages)\n",
+ its->msi_chip.of_node->full_name, order, alloc_pages);
+ }
+
base = (void *)__get_free_pages(GFP_KERNEL | __GFP_ZERO, order);
if (!base) {
err = -ENOMEM;
@@ -872,7 +881,7 @@ static int its_alloc_tables(struct its_node *its)
break;
}
- val |= (alloc_size / psz) - 1;
+ val |= alloc_pages - 1;
writeq_relaxed(val, its->base + GITS_BASER + i * 8);
tmp = readq_relaxed(its->base + GITS_BASER + i * 8);
diff --git a/include/linux/irqchip/arm-gic-v3.h b/include/linux/irqchip/arm-gic-v3.h
index ffbc034c8810..f28da189c4aa 100644
--- a/include/linux/irqchip/arm-gic-v3.h
+++ b/include/linux/irqchip/arm-gic-v3.h
@@ -229,6 +229,7 @@
#define GITS_BASER_PAGE_SIZE_16K (1UL << GITS_BASER_PAGE_SIZE_SHIFT)
#define GITS_BASER_PAGE_SIZE_64K (2UL << GITS_BASER_PAGE_SIZE_SHIFT)
#define GITS_BASER_PAGE_SIZE_MASK (3UL << GITS_BASER_PAGE_SIZE_SHIFT)
+#define GITS_BASER_PAGES_MAX 256
#define GITS_BASER_TYPE_NONE 0
#define GITS_BASER_TYPE_DEVICE 1
--
2.1.1
From: Robert Richter <[email protected]>
Some GIC revisions require an individual configuration to esp. add
workarounds for HW bugs. This patch implements generic code to parse
the hw revision provided by an IIDR register value and runs specific
code if hw matches. There are functions that read the IIDR registers
for GICV3 and ITS (GICD_IIDR/GITS_IIDR) and then go through a list of
init functions to be called for specific versions.
A MIDR register value may also be used, this is especially useful for
hw detection from a guest.
The patch is needed to implement workarounds for HW errata in Cavium's
ThunderX GICV3.
v3:
* use arm64 errata framework for midr check
v2:
* adding MIDR check
Signed-off-by: Robert Richter <[email protected]>
---
drivers/irqchip/irq-gic-common.c | 13 +++++++++++++
drivers/irqchip/irq-gic-common.h | 10 ++++++++++
drivers/irqchip/irq-gic-v3-its.c | 15 +++++++++++++++
drivers/irqchip/irq-gic-v3.c | 14 ++++++++++++++
4 files changed, 52 insertions(+)
diff --git a/drivers/irqchip/irq-gic-common.c b/drivers/irqchip/irq-gic-common.c
index 9448e391cb71..f5e901a62ea5 100644
--- a/drivers/irqchip/irq-gic-common.c
+++ b/drivers/irqchip/irq-gic-common.c
@@ -21,6 +21,19 @@
#include "irq-gic-common.h"
+void gic_check_capabilities(u32 iidr, const struct gic_capabilities *cap,
+ void *data)
+{
+ for (; cap->desc; cap++) {
+ if (cap->cpu_cap && !cpus_have_cap(cap->cpu_cap))
+ continue;
+ if (cap->iidr != (cap->mask & iidr))
+ continue;
+ cap->init(data);
+ pr_info("%s\n", cap->desc);
+ }
+}
+
int gic_configure_irq(unsigned int irq, unsigned int type,
void __iomem *base, void (*sync_access)(void))
{
diff --git a/drivers/irqchip/irq-gic-common.h b/drivers/irqchip/irq-gic-common.h
index 35a9884778bd..6d019a238f3b 100644
--- a/drivers/irqchip/irq-gic-common.h
+++ b/drivers/irqchip/irq-gic-common.h
@@ -20,10 +20,20 @@
#include <linux/of.h>
#include <linux/irqdomain.h>
+struct gic_capabilities {
+ const char *desc;
+ void (*init)(void *data);
+ u32 iidr;
+ u32 mask;
+ u16 cpu_cap;
+};
+
int gic_configure_irq(unsigned int irq, unsigned int type,
void __iomem *base, void (*sync_access)(void));
void gic_dist_config(void __iomem *base, int gic_irqs,
void (*sync_access)(void));
void gic_cpu_config(void __iomem *base, void (*sync_access)(void));
+void gic_check_capabilities(u32 iidr, const struct gic_capabilities *cap,
+ void *data);
#endif /* _IRQ_GIC_COMMON_H */
diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 466edf8a7477..105674037618 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -36,6 +36,7 @@
#include <asm/cputype.h>
#include <asm/exception.h>
+#include "irq-gic-common.h"
#include "irqchip.h"
#define ITS_FLAGS_CMDQ_NEEDS_FLUSHING (1 << 0)
@@ -1391,6 +1392,18 @@ static int its_force_quiescent(void __iomem *base)
}
}
+static const struct gic_capabilities its_errata[] = {
+ {
+ }
+};
+
+static void its_check_capabilities(struct its_node *its)
+{
+ u32 iidr = readl_relaxed(its->base + GITS_IIDR);
+
+ gic_check_capabilities(iidr, its_errata, its);
+}
+
static int its_probe(struct device_node *node, struct irq_domain *parent)
{
struct resource res;
@@ -1449,6 +1462,8 @@ static int its_probe(struct device_node *node, struct irq_domain *parent)
}
its->cmd_write = its->cmd_base;
+ its_check_capabilities(its);
+
err = its_alloc_tables(its);
if (err)
goto out_free_cmd;
diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
index c52f7ba205b4..1a91902be0b1 100644
--- a/drivers/irqchip/irq-gic-v3.c
+++ b/drivers/irqchip/irq-gic-v3.c
@@ -766,6 +766,18 @@ static const struct irq_domain_ops gic_irq_domain_ops = {
.free = gic_irq_domain_free,
};
+static const struct gic_capabilities gicv3_errata[] = {
+ {
+ }
+};
+
+static void gicv3_check_capabilities(void)
+{
+ u32 iidr = readl_relaxed(gic_data.dist_base + GICD_IIDR);
+
+ gic_check_capabilities(iidr, gicv3_errata, NULL);
+}
+
static int __init gic_of_init(struct device_node *node, struct device_node *parent)
{
void __iomem *dist_base;
@@ -825,6 +837,8 @@ static int __init gic_of_init(struct device_node *node, struct device_node *pare
gic_data.nr_redist_regions = nr_redist_regions;
gic_data.redist_stride = redist_stride;
+ gicv3_check_capabilities();
+
/*
* Find out how many interrupts are supported.
* The GIC only supports up to 1020 interrupt sources (SGI+PPI+SPI)
--
2.1.1
From: Robert Richter <[email protected]>
This patch implements Cavium ThunderX erratum 23154.
The gicv3 of ThunderX requires a modified version for reading the IAR
status to ensure data synchronization. Since this is in the fast-path
and called with each interrupt, runtime patching is used using jump
label patching for smallest overhead (no-op). This is the same
technique as used for tracepoints.
v3:
* fix erratum to be dependend from midr
* use arm64 errata framework
v2:
* implement code in a single asm() to keep instruction sequence
* added comment to the code that explains the erratum
* apply workaround also if running as guest, thus check MIDR
Signed-off-by: Robert Richter <[email protected]>
---
arch/arm64/Kconfig | 11 +++++++++
arch/arm64/include/asm/cpufeature.h | 3 ++-
arch/arm64/include/asm/cputype.h | 18 ++++++++------
arch/arm64/kernel/cpu_errata.c | 9 +++++++
drivers/irqchip/irq-gic-v3.c | 49 ++++++++++++++++++++++++++++++++++++-
5 files changed, 81 insertions(+), 9 deletions(-)
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 0f6edb14b7e4..4f866a4c6536 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -417,6 +417,17 @@ config ARM64_ERRATUM_845719
If unsure, say Y.
+config CAVIUM_ERRATUM_23154
+ bool "Cavium erratum 23154: Access to ICC_IAR1_EL1 is not sync'ed"
+ depends on ARCH_THUNDER
+ default y
+ help
+ The gicv3 of ThunderX requires a modified version for
+ reading the IAR status to ensure data synchronization
+ (access to icc_iar1_el1 is not sync'ed before and after).
+
+ If unsure, say Y.
+
endmenu
diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
index c1044218a63a..2a5e4c163ee5 100644
--- a/arch/arm64/include/asm/cpufeature.h
+++ b/arch/arm64/include/asm/cpufeature.h
@@ -25,8 +25,9 @@
#define ARM64_WORKAROUND_DEVICE_LOAD_ACQUIRE 1
#define ARM64_WORKAROUND_845719 2
#define ARM64_HAS_SYSREG_GIC_CPUIF 3
+#define ARM64_WORKAROUND_CAVIUM_23154 4
-#define ARM64_NCAPS 4
+#define ARM64_NCAPS 5
#ifndef __ASSEMBLY__
diff --git a/arch/arm64/include/asm/cputype.h b/arch/arm64/include/asm/cputype.h
index a84ec605bed8..3f0c7683f252 100644
--- a/arch/arm64/include/asm/cputype.h
+++ b/arch/arm64/include/asm/cputype.h
@@ -62,15 +62,19 @@
(0xf << MIDR_ARCHITECTURE_SHIFT) | \
((partnum) << MIDR_PARTNUM_SHIFT))
-#define ARM_CPU_IMP_ARM 0x41
-#define ARM_CPU_IMP_APM 0x50
+#define ARM_CPU_IMP_ARM 0x41
+#define ARM_CPU_IMP_APM 0x50
+#define ARM_CPU_IMP_CAVIUM 0x43
-#define ARM_CPU_PART_AEM_V8 0xD0F
-#define ARM_CPU_PART_FOUNDATION 0xD00
-#define ARM_CPU_PART_CORTEX_A57 0xD07
-#define ARM_CPU_PART_CORTEX_A53 0xD03
+#define ARM_CPU_PART_AEM_V8 0xD0F
+#define ARM_CPU_PART_FOUNDATION 0xD00
+#define ARM_CPU_PART_CORTEX_A57 0xD07
+#define ARM_CPU_PART_CORTEX_A53 0xD03
+
+#define APM_CPU_PART_POTENZA 0x000
+
+#define CAVIUM_CPU_PART_THUNDERX 0x0A1
-#define APM_CPU_PART_POTENZA 0x000
#define ID_AA64MMFR0_BIGENDEL0_SHIFT 16
#define ID_AA64MMFR0_BIGENDEL0_MASK (0xf << ID_AA64MMFR0_BIGENDEL0_SHIFT)
diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
index 6ffd91438560..574450c257a4 100644
--- a/arch/arm64/kernel/cpu_errata.c
+++ b/arch/arm64/kernel/cpu_errata.c
@@ -23,6 +23,7 @@
#define MIDR_CORTEX_A53 MIDR_CPU_PART(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_A53)
#define MIDR_CORTEX_A57 MIDR_CPU_PART(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_A57)
+#define MIDR_THUNDERX MIDR_CPU_PART(ARM_CPU_IMP_CAVIUM, CAVIUM_CPU_PART_THUNDERX)
#define CPU_MODEL_MASK (MIDR_IMPLEMENTOR_MASK | MIDR_PARTNUM_MASK | \
MIDR_ARCHITECTURE_MASK)
@@ -82,6 +83,14 @@ const struct arm64_cpu_capabilities arm64_errata[] = {
MIDR_RANGE(MIDR_CORTEX_A53, 0x00, 0x04),
},
#endif
+#ifdef CONFIG_CAVIUM_ERRATUM_23154
+ {
+ /* Cavium ThunderX, pass 1.x */
+ .desc = "Cavium erratum 23154",
+ .capability = ARM64_WORKAROUND_CAVIUM_23154,
+ MIDR_RANGE(MIDR_THUNDERX, 0x00, 0x01),
+ },
+#endif
{
}
};
diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
index 1a91902be0b1..59a5f13f3c10 100644
--- a/drivers/irqchip/irq-gic-v3.c
+++ b/drivers/irqchip/irq-gic-v3.c
@@ -107,7 +107,7 @@ static void gic_redist_wait_for_rwp(void)
}
/* Low level accessors */
-static u64 __maybe_unused gic_read_iar(void)
+static u64 gic_read_iar_common(void)
{
u64 irqstat;
@@ -115,6 +115,38 @@ static u64 __maybe_unused gic_read_iar(void)
return irqstat;
}
+/*
+ * Cavium ThunderX erratum 23154
+ *
+ * The gicv3 of ThunderX requires a modified version for reading the
+ * IAR status to ensure data synchronization (access to icc_iar1_el1
+ * is not sync'ed before and after).
+ */
+static u64 gic_read_iar_cavium_thunderx(void)
+{
+ u64 irqstat;
+
+ asm volatile(
+ "nop;nop;nop;nop\n\t"
+ "nop;nop;nop;nop\n\t"
+ "mrs_s %0, " __stringify(ICC_IAR1_EL1) "\n\t"
+ "nop;nop;nop;nop"
+ : "=r" (irqstat));
+ mb();
+
+ return irqstat;
+}
+
+struct static_key is_cavium_thunderx = STATIC_KEY_INIT_FALSE;
+
+static u64 __maybe_unused gic_read_iar(void)
+{
+ if (static_key_false(&is_cavium_thunderx))
+ return gic_read_iar_common();
+ else
+ return gic_read_iar_cavium_thunderx();
+}
+
static void __maybe_unused gic_write_pmr(u64 val)
{
asm volatile("msr_s " __stringify(ICC_PMR_EL1) ", %0" : : "r" (val));
@@ -766,8 +798,23 @@ static const struct irq_domain_ops gic_irq_domain_ops = {
.free = gic_irq_domain_free,
};
+static void gicv3_enable_cavium_thunderx(void *data)
+{
+ static_key_slow_inc(&is_cavium_thunderx);
+}
+
static const struct gic_capabilities gicv3_errata[] = {
{
+ /*
+ * Need to be applied in guests too, thus use cpu
+ * detection as iidr is emulated with different vendor
+ * id.
+ */
+ .desc = "GIC: Cavium erratum 23154",
+ .cpu_cap = ARM64_WORKAROUND_CAVIUM_23154,
+ .init = gicv3_enable_cavium_thunderx,
+ },
+ {
}
};
--
2.1.1
From: Robert Richter <[email protected]>
No need to read the typer register in the loop. Values do not change.
This patch is basically a prerequisite for a follow-on patch that adds
errata code for Cavium ThunderX. It moves the calculation of the
number of id entries to the beginning of the function close to other
setup values that are needed to allocate the its table. Now we have a
central location to modify the setup parameters and the errata code
can be implemented in a single block.
Acked-by: Marc Zyngier <[email protected]>
Signed-off-by: Robert Richter <[email protected]>
---
drivers/irqchip/irq-gic-v3-its.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 105674037618..bf0659821683 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -804,6 +804,8 @@ static int its_alloc_tables(struct its_node *its)
int psz = SZ_64K;
u64 shr = GITS_BASER_InnerShareable;
u64 cache = GITS_BASER_WaWb;
+ u64 typer = readq_relaxed(its->base + GITS_TYPER);
+ u32 ids = GITS_TYPER_DEVBITS(typer);
for (i = 0; i < GITS_BASER_NR_REGS; i++) {
u64 val = readq_relaxed(its->base + GITS_BASER + i * 8);
@@ -827,9 +829,6 @@ static int its_alloc_tables(struct its_node *its)
* For other tables, only allocate a single page.
*/
if (type == GITS_BASER_TYPE_DEVICE) {
- u64 typer = readq_relaxed(its->base + GITS_TYPER);
- u32 ids = GITS_TYPER_DEVBITS(typer);
-
/*
* 'order' was initialized earlier to the default page
* granule of the the ITS. We can't have an allocation
--
2.1.1
From: Robert Richter <[email protected]>
This implements two gicv3-its errata workarounds for ThunderX. Both
with small impact affecting only ITS table allocation.
erratum 22375: only alloc 8MB table size
erratum 24313: ignore memory access type
The fixes are in ITS initialization and basically ignore memory access
type and table size provided by the TYPER and BASER registers.
v3:
* fix erratum to be dependend from iidr
Signed-off-by: Robert Richter <[email protected]>
---
drivers/irqchip/irq-gic-v3-its.c | 35 +++++++++++++++++++++++++++++++----
1 file changed, 31 insertions(+), 4 deletions(-)
diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index bf0659821683..ab888eeef4b1 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -39,7 +39,8 @@
#include "irq-gic-common.h"
#include "irqchip.h"
-#define ITS_FLAGS_CMDQ_NEEDS_FLUSHING (1 << 0)
+#define ITS_FLAGS_CMDQ_NEEDS_FLUSHING (1ULL << 0)
+#define ITS_FLAGS_CAVIUM_THUNDERX (1ULL << 1)
#define RDIST_FLAGS_PROPBASE_NEEDS_FLUSHING (1 << 0)
@@ -803,9 +804,22 @@ static int its_alloc_tables(struct its_node *its)
int i;
int psz = SZ_64K;
u64 shr = GITS_BASER_InnerShareable;
- u64 cache = GITS_BASER_WaWb;
- u64 typer = readq_relaxed(its->base + GITS_TYPER);
- u32 ids = GITS_TYPER_DEVBITS(typer);
+ u64 cache;
+ u64 typer;
+ u32 ids;
+
+ if (its->flags & ITS_FLAGS_CAVIUM_THUNDERX) {
+ /*
+ * erratum 22375: only alloc 8MB table size
+ * erratum 24313: ignore memory access type
+ */
+ cache = 0;
+ ids = 0x13; /* 20 bits, 8MB */
+ } else {
+ cache = GITS_BASER_WaWb;
+ typer = readq_relaxed(its->base + GITS_TYPER);
+ ids = GITS_TYPER_DEVBITS(typer);
+ }
for (i = 0; i < GITS_BASER_NR_REGS; i++) {
u64 val = readq_relaxed(its->base + GITS_BASER + i * 8);
@@ -1391,8 +1405,21 @@ static int its_force_quiescent(void __iomem *base)
}
}
+static void its_enable_cavium_thunderx(void *data)
+{
+ struct its_node *its = data;
+
+ its->flags |= ITS_FLAGS_CAVIUM_THUNDERX;
+}
+
static const struct gic_capabilities its_errata[] = {
{
+ .desc = "ITS: Cavium errata 22375, 24313",
+ .iidr = 0xa100034c, /* ThunderX pass 1.x */
+ .mask = 0xffff0fff,
+ .init = its_enable_cavium_thunderx,
+ },
+ {
}
};
--
2.1.1
From: Robert Richter <[email protected]>
This patch make caps usable optionally. If its value is zero, then all
cpus are matched. E.g. if caps resides in a stuct for marking it
dependend on a certain cpu feature/errata, then an empty value will
indicate not to use caps detection. An empty value will match all cpus
and thus just skip the cpu feature test.
Signed-off-by: Robert Richter <[email protected]>
---
arch/arm64/include/asm/cpufeature.h | 17 +++++++++++------
drivers/irqchip/irq-gic-common.c | 2 +-
2 files changed, 12 insertions(+), 7 deletions(-)
diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
index 2a5e4c163ee5..a5f8a007d982 100644
--- a/arch/arm64/include/asm/cpufeature.h
+++ b/arch/arm64/include/asm/cpufeature.h
@@ -21,13 +21,15 @@
#define MAX_CPU_FEATURES (8 * sizeof(elf_hwcap))
#define cpu_feature(x) ilog2(HWCAP_ ## x)
-#define ARM64_WORKAROUND_CLEAN_CACHE 0
-#define ARM64_WORKAROUND_DEVICE_LOAD_ACQUIRE 1
-#define ARM64_WORKAROUND_845719 2
-#define ARM64_HAS_SYSREG_GIC_CPUIF 3
-#define ARM64_WORKAROUND_CAVIUM_23154 4
+/* reserve zero to match all cpus for use of caps in empty structs */
+#define ARM64_ALL_CPUS 0
+#define ARM64_WORKAROUND_CLEAN_CACHE 1
+#define ARM64_WORKAROUND_DEVICE_LOAD_ACQUIRE 2
+#define ARM64_WORKAROUND_845719 3
+#define ARM64_HAS_SYSREG_GIC_CPUIF 4
+#define ARM64_WORKAROUND_CAVIUM_23154 5
-#define ARM64_NCAPS 5
+#define ARM64_NCAPS 6
#ifndef __ASSEMBLY__
@@ -57,6 +59,9 @@ static inline bool cpu_have_feature(unsigned int num)
static inline bool cpus_have_cap(unsigned int num)
{
+ /* zero matches for all cpus */
+ if (!num)
+ return true;
if (num >= ARM64_NCAPS)
return false;
return test_bit(num, cpu_hwcaps);
diff --git a/drivers/irqchip/irq-gic-common.c b/drivers/irqchip/irq-gic-common.c
index f5e901a62ea5..cb55aaf960bb 100644
--- a/drivers/irqchip/irq-gic-common.c
+++ b/drivers/irqchip/irq-gic-common.c
@@ -25,7 +25,7 @@ void gic_check_capabilities(u32 iidr, const struct gic_capabilities *cap,
void *data)
{
for (; cap->desc; cap++) {
- if (cap->cpu_cap && !cpus_have_cap(cap->cpu_cap))
+ if (!cpus_have_cap(cap->cpu_cap))
continue;
if (cap->iidr != (cap->mask & iidr))
continue;
--
2.1.1
On 14/08/15 14:18, Robert Richter wrote:
> From: Robert Richter <[email protected]>
>
> This patch implements Cavium ThunderX erratum 23154.
>
> The gicv3 of ThunderX requires a modified version for reading the IAR
> status to ensure data synchronization. Since this is in the fast-path
> and called with each interrupt, runtime patching is used using jump
> label patching for smallest overhead (no-op). This is the same
> technique as used for tracepoints.
>
> v3:
> * fix erratum to be dependend from midr
> * use arm64 errata framework
>
> v2:
> * implement code in a single asm() to keep instruction sequence
> * added comment to the code that explains the erratum
> * apply workaround also if running as guest, thus check MIDR
>
> Signed-off-by: Robert Richter <[email protected]>
> ---
> arch/arm64/Kconfig | 11 +++++++++
> arch/arm64/include/asm/cpufeature.h | 3 ++-
> arch/arm64/include/asm/cputype.h | 18 ++++++++------
> arch/arm64/kernel/cpu_errata.c | 9 +++++++
> drivers/irqchip/irq-gic-v3.c | 49 ++++++++++++++++++++++++++++++++++++-
> 5 files changed, 81 insertions(+), 9 deletions(-)
>
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 0f6edb14b7e4..4f866a4c6536 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -417,6 +417,17 @@ config ARM64_ERRATUM_845719
>
> If unsure, say Y.
>
> +config CAVIUM_ERRATUM_23154
> + bool "Cavium erratum 23154: Access to ICC_IAR1_EL1 is not sync'ed"
> + depends on ARCH_THUNDER
> + default y
> + help
> + The gicv3 of ThunderX requires a modified version for
> + reading the IAR status to ensure data synchronization
> + (access to icc_iar1_el1 is not sync'ed before and after).
> +
> + If unsure, say Y.
> +
> endmenu
>
>
> diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
> index c1044218a63a..2a5e4c163ee5 100644
> --- a/arch/arm64/include/asm/cpufeature.h
> +++ b/arch/arm64/include/asm/cpufeature.h
> @@ -25,8 +25,9 @@
> #define ARM64_WORKAROUND_DEVICE_LOAD_ACQUIRE 1
> #define ARM64_WORKAROUND_845719 2
> #define ARM64_HAS_SYSREG_GIC_CPUIF 3
> +#define ARM64_WORKAROUND_CAVIUM_23154 4
>
> -#define ARM64_NCAPS 4
> +#define ARM64_NCAPS 5
>
> #ifndef __ASSEMBLY__
>
> diff --git a/arch/arm64/include/asm/cputype.h b/arch/arm64/include/asm/cputype.h
> index a84ec605bed8..3f0c7683f252 100644
> --- a/arch/arm64/include/asm/cputype.h
> +++ b/arch/arm64/include/asm/cputype.h
> @@ -62,15 +62,19 @@
> (0xf << MIDR_ARCHITECTURE_SHIFT) | \
> ((partnum) << MIDR_PARTNUM_SHIFT))
>
> -#define ARM_CPU_IMP_ARM 0x41
> -#define ARM_CPU_IMP_APM 0x50
> +#define ARM_CPU_IMP_ARM 0x41
> +#define ARM_CPU_IMP_APM 0x50
> +#define ARM_CPU_IMP_CAVIUM 0x43
>
> -#define ARM_CPU_PART_AEM_V8 0xD0F
> -#define ARM_CPU_PART_FOUNDATION 0xD00
> -#define ARM_CPU_PART_CORTEX_A57 0xD07
> -#define ARM_CPU_PART_CORTEX_A53 0xD03
> +#define ARM_CPU_PART_AEM_V8 0xD0F
> +#define ARM_CPU_PART_FOUNDATION 0xD00
> +#define ARM_CPU_PART_CORTEX_A57 0xD07
> +#define ARM_CPU_PART_CORTEX_A53 0xD03
> +
> +#define APM_CPU_PART_POTENZA 0x000
> +
> +#define CAVIUM_CPU_PART_THUNDERX 0x0A1
>
> -#define APM_CPU_PART_POTENZA 0x000
>
> #define ID_AA64MMFR0_BIGENDEL0_SHIFT 16
> #define ID_AA64MMFR0_BIGENDEL0_MASK (0xf << ID_AA64MMFR0_BIGENDEL0_SHIFT)
> diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
> index 6ffd91438560..574450c257a4 100644
> --- a/arch/arm64/kernel/cpu_errata.c
> +++ b/arch/arm64/kernel/cpu_errata.c
> @@ -23,6 +23,7 @@
>
> #define MIDR_CORTEX_A53 MIDR_CPU_PART(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_A53)
> #define MIDR_CORTEX_A57 MIDR_CPU_PART(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_A57)
> +#define MIDR_THUNDERX MIDR_CPU_PART(ARM_CPU_IMP_CAVIUM, CAVIUM_CPU_PART_THUNDERX)
>
> #define CPU_MODEL_MASK (MIDR_IMPLEMENTOR_MASK | MIDR_PARTNUM_MASK | \
> MIDR_ARCHITECTURE_MASK)
> @@ -82,6 +83,14 @@ const struct arm64_cpu_capabilities arm64_errata[] = {
> MIDR_RANGE(MIDR_CORTEX_A53, 0x00, 0x04),
> },
> #endif
> +#ifdef CONFIG_CAVIUM_ERRATUM_23154
> + {
> + /* Cavium ThunderX, pass 1.x */
> + .desc = "Cavium erratum 23154",
> + .capability = ARM64_WORKAROUND_CAVIUM_23154,
> + MIDR_RANGE(MIDR_THUNDERX, 0x00, 0x01),
> + },
> +#endif
> {
> }
> };
> diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
> index 1a91902be0b1..59a5f13f3c10 100644
> --- a/drivers/irqchip/irq-gic-v3.c
> +++ b/drivers/irqchip/irq-gic-v3.c
> @@ -107,7 +107,7 @@ static void gic_redist_wait_for_rwp(void)
> }
>
> /* Low level accessors */
> -static u64 __maybe_unused gic_read_iar(void)
> +static u64 gic_read_iar_common(void)
> {
> u64 irqstat;
>
> @@ -115,6 +115,38 @@ static u64 __maybe_unused gic_read_iar(void)
> return irqstat;
> }
>
> +/*
> + * Cavium ThunderX erratum 23154
> + *
> + * The gicv3 of ThunderX requires a modified version for reading the
> + * IAR status to ensure data synchronization (access to icc_iar1_el1
> + * is not sync'ed before and after).
> + */
> +static u64 gic_read_iar_cavium_thunderx(void)
> +{
> + u64 irqstat;
> +
> + asm volatile(
> + "nop;nop;nop;nop\n\t"
> + "nop;nop;nop;nop\n\t"
> + "mrs_s %0, " __stringify(ICC_IAR1_EL1) "\n\t"
> + "nop;nop;nop;nop"
> + : "=r" (irqstat));
> + mb();
> +
> + return irqstat;
> +}
> +
> +struct static_key is_cavium_thunderx = STATIC_KEY_INIT_FALSE;
> +
> +static u64 __maybe_unused gic_read_iar(void)
> +{
> + if (static_key_false(&is_cavium_thunderx))
> + return gic_read_iar_common();
> + else
> + return gic_read_iar_cavium_thunderx();
> +}
> +
> static void __maybe_unused gic_write_pmr(u64 val)
> {
> asm volatile("msr_s " __stringify(ICC_PMR_EL1) ", %0" : : "r" (val));
> @@ -766,8 +798,23 @@ static const struct irq_domain_ops gic_irq_domain_ops = {
> .free = gic_irq_domain_free,
> };
>
> +static void gicv3_enable_cavium_thunderx(void *data)
> +{
> + static_key_slow_inc(&is_cavium_thunderx);
> +}
> +
> static const struct gic_capabilities gicv3_errata[] = {
> {
> + /*
> + * Need to be applied in guests too, thus use cpu
> + * detection as iidr is emulated with different vendor
> + * id.
> + */
> + .desc = "GIC: Cavium erratum 23154",
> + .cpu_cap = ARM64_WORKAROUND_CAVIUM_23154,
> + .init = gicv3_enable_cavium_thunderx,
> + },
> + {
> }
> };
>
>
Robert,
I probably didn't express myself very well, because this not what I had
in mind, sorry...
I think this is very much overdesigned for something that should be very
simple. Why can't you have a simple:
if (cpu_have_feature(ARM64_WORKAROUND_CAVIUM_23154))
static_key_slow_inc(&is_cavium_thunderx);
No need for a cpu_cap field, no need to special-case feature 0, and it
keeps the code fairly readable. You can even hide it in a
gic_enable_cpu_quirks() function.
Thanks,
M.
--
Jazz is not dead. It just smells funny...