All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 00/18] clocksource/arch_timer: Errata workaround infrastructure rework
@ 2017-04-04 17:18 ` Marc Zyngier
  0 siblings, 0 replies; 59+ messages in thread
From: Marc Zyngier @ 2017-04-04 17:18 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel
  Cc: Mark Rutland, Catalin Marinas, Daniel Lezcano, Will Deacon,
	Scott Wood, Hanjun Guo, Ding Tianhong, dann frazier,
	Thomas Gleixner

It has recently become obvious that a number of arm64 systems have
been blessed with a set of timers that are slightly less than perfect,
and require a bit of hand-holding. We already have a bunch of
errata-specific code to deal with this, but as we're adding more
potential detection methods (DT, ACPI, capability), things are getting
a bit out of hands.

Instead of adding more ad-hoc fixes to an already difficult code base,
let's give ourselves a bit of an infrastructure that can deal with
this and hide most of the uggliness behind semi-frendly accessors.

The series is roughly structured as such:

- A bunch of arm64 specific patches that allow the rest of the
  workaround infrastructure to be built upon (such as being able to
  trap userspace access to the virtual counter). These are now
  separate in order to allow the creation of a shared branch between
  the arm64 and clocksource trees.

- The following patches rework the existing workarounds, allowing
  errata to be matched using a given detection method (DT, ACPI or
  core specific)

- Another set of patches allowing a workaround to affect a subset of
  the CPUs, and not the whole system

- We then work around a Cortex-A73 erratum, whose counter can return a
  wrong value if read while crossing a 32bit boundary. Yup.

- Finally, we add some ACPI-specific workarounds for HiSilicon
  platforms that have the HISILICON_ERRATUM_161010101 defect.

Note that so far, we only deal with arm64. Once the infrastructure is
agreed upon, we can look at generalizing it (to some extent) to 32bit
ARM (typical use case would be a 32bit guest running on an affected
host).

This has been tested on an ls2085 platform (with the
"fsl,erratum-a008585" erratum) and a set of models configured to
simulate various configurations of affected/unaffected CPUs, both as
hosts and as KVM guests.

* From v2:
  - Dropped the "global capability" detection method, which was
    currently unused, simplifying the code a bit
  - Turned saved_cntkctl into a per-CPU variable, allowing a mix of
    affected/unaffected CPUs to retrieve their own configuration
    (new patch)
  - Fixed a bug that could leave cntvct_el0 untrapped if reset with
    the user access bit set
  - Added default clause to the match_id switch
  - Added Daniel's Acks on the relevant patches
  - Rebased on 4.11-rc5

* From v1:
  - Addressed Hanjun and Mark review comments
  - Moved arm64 specific patches to the beginning of the series,
    leaving the clocksource patches at the end, resulting in an extra
    patch.
  - Added RBs, TBs, and Acks.

Marc Zyngier (18):
  arm64: Allow checking of a CPU-local erratum
  arm64: Add CNTVCT_EL0 trap handler
  arm64: Define Cortex-A73 MIDR
  arm64: cpu_errata: Allow an erratum to be match for all revisions of a
    core
  arm64: cpu_errata: Add capability to advertise Cortex-A73 erratum
    858921
  arm64: arch_timer: Add infrastructure for multiple erratum detection
    methods
  arm64: arch_timer: Add erratum handler for CPU-specific capability
  arm64: arch_timer: Move arch_timer_reg_read/write around
  arm64: arch_timer: Get rid of erratum_workaround_set_sne
  arm64: arch_timer: Rework the set_next_event workarounds
  arm64: arch_timer: Make workaround methods optional
  arm64: arch_timer: Allows a CPU-specific erratum to only affect a
    subset of CPUs
  arm64: arch_timer: Move clocksource_counter and co around
  arm64: arch_timer: Save cntkctl_el1 as a per-cpu variable
  arm64: arch_timer: Enable CNTVCT_EL0 trap if workaround is enabled
  arm64: arch_timer: Workaround for Cortex-A73 erratum 858921
  arm64: arch_timer: Allow erratum matching with ACPI OEM information
  arm64: arch_timer: Add HISILICON_ERRATUM_161010101 ACPI matching data

 Documentation/arm64/silicon-errata.txt |   1 +
 arch/arm64/include/asm/arch_timer.h    |  43 ++-
 arch/arm64/include/asm/cpucaps.h       |   3 +-
 arch/arm64/include/asm/cputype.h       |   2 +
 arch/arm64/include/asm/esr.h           |   2 +
 arch/arm64/kernel/cpu_errata.c         |  15 +
 arch/arm64/kernel/cpufeature.c         |  13 +-
 arch/arm64/kernel/traps.c              |  14 +
 drivers/clocksource/Kconfig            |  11 +
 drivers/clocksource/arm_arch_timer.c   | 535 +++++++++++++++++++++++----------
 10 files changed, 466 insertions(+), 173 deletions(-)

-- 
2.11.0

^ permalink raw reply	[flat|nested] 59+ messages in thread

* [PATCH v3 00/18] clocksource/arch_timer: Errata workaround infrastructure rework
@ 2017-04-04 17:18 ` Marc Zyngier
  0 siblings, 0 replies; 59+ messages in thread
From: Marc Zyngier @ 2017-04-04 17:18 UTC (permalink / raw)
  To: linux-arm-kernel

It has recently become obvious that a number of arm64 systems have
been blessed with a set of timers that are slightly less than perfect,
and require a bit of hand-holding. We already have a bunch of
errata-specific code to deal with this, but as we're adding more
potential detection methods (DT, ACPI, capability), things are getting
a bit out of hands.

Instead of adding more ad-hoc fixes to an already difficult code base,
let's give ourselves a bit of an infrastructure that can deal with
this and hide most of the uggliness behind semi-frendly accessors.

The series is roughly structured as such:

- A bunch of arm64 specific patches that allow the rest of the
  workaround infrastructure to be built upon (such as being able to
  trap userspace access to the virtual counter). These are now
  separate in order to allow the creation of a shared branch between
  the arm64 and clocksource trees.

- The following patches rework the existing workarounds, allowing
  errata to be matched using a given detection method (DT, ACPI or
  core specific)

- Another set of patches allowing a workaround to affect a subset of
  the CPUs, and not the whole system

- We then work around a Cortex-A73 erratum, whose counter can return a
  wrong value if read while crossing a 32bit boundary. Yup.

- Finally, we add some ACPI-specific workarounds for HiSilicon
  platforms that have the HISILICON_ERRATUM_161010101 defect.

Note that so far, we only deal with arm64. Once the infrastructure is
agreed upon, we can look at generalizing it (to some extent) to 32bit
ARM (typical use case would be a 32bit guest running on an affected
host).

This has been tested on an ls2085 platform (with the
"fsl,erratum-a008585" erratum) and a set of models configured to
simulate various configurations of affected/unaffected CPUs, both as
hosts and as KVM guests.

* From v2:
  - Dropped the "global capability" detection method, which was
    currently unused, simplifying the code a bit
  - Turned saved_cntkctl into a per-CPU variable, allowing a mix of
    affected/unaffected CPUs to retrieve their own configuration
    (new patch)
  - Fixed a bug that could leave cntvct_el0 untrapped if reset with
    the user access bit set
  - Added default clause to the match_id switch
  - Added Daniel's Acks on the relevant patches
  - Rebased on 4.11-rc5

* From v1:
  - Addressed Hanjun and Mark review comments
  - Moved arm64 specific patches to the beginning of the series,
    leaving the clocksource patches at the end, resulting in an extra
    patch.
  - Added RBs, TBs, and Acks.

Marc Zyngier (18):
  arm64: Allow checking of a CPU-local erratum
  arm64: Add CNTVCT_EL0 trap handler
  arm64: Define Cortex-A73 MIDR
  arm64: cpu_errata: Allow an erratum to be match for all revisions of a
    core
  arm64: cpu_errata: Add capability to advertise Cortex-A73 erratum
    858921
  arm64: arch_timer: Add infrastructure for multiple erratum detection
    methods
  arm64: arch_timer: Add erratum handler for CPU-specific capability
  arm64: arch_timer: Move arch_timer_reg_read/write around
  arm64: arch_timer: Get rid of erratum_workaround_set_sne
  arm64: arch_timer: Rework the set_next_event workarounds
  arm64: arch_timer: Make workaround methods optional
  arm64: arch_timer: Allows a CPU-specific erratum to only affect a
    subset of CPUs
  arm64: arch_timer: Move clocksource_counter and co around
  arm64: arch_timer: Save cntkctl_el1 as a per-cpu variable
  arm64: arch_timer: Enable CNTVCT_EL0 trap if workaround is enabled
  arm64: arch_timer: Workaround for Cortex-A73 erratum 858921
  arm64: arch_timer: Allow erratum matching with ACPI OEM information
  arm64: arch_timer: Add HISILICON_ERRATUM_161010101 ACPI matching data

 Documentation/arm64/silicon-errata.txt |   1 +
 arch/arm64/include/asm/arch_timer.h    |  43 ++-
 arch/arm64/include/asm/cpucaps.h       |   3 +-
 arch/arm64/include/asm/cputype.h       |   2 +
 arch/arm64/include/asm/esr.h           |   2 +
 arch/arm64/kernel/cpu_errata.c         |  15 +
 arch/arm64/kernel/cpufeature.c         |  13 +-
 arch/arm64/kernel/traps.c              |  14 +
 drivers/clocksource/Kconfig            |  11 +
 drivers/clocksource/arm_arch_timer.c   | 535 +++++++++++++++++++++++----------
 10 files changed, 466 insertions(+), 173 deletions(-)

-- 
2.11.0

^ permalink raw reply	[flat|nested] 59+ messages in thread

* [PATCH v3 01/18] arm64: Allow checking of a CPU-local erratum
  2017-04-04 17:18 ` Marc Zyngier
@ 2017-04-04 17:18   ` Marc Zyngier
  -1 siblings, 0 replies; 59+ messages in thread
From: Marc Zyngier @ 2017-04-04 17:18 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel
  Cc: Mark Rutland, Catalin Marinas, Daniel Lezcano, Will Deacon,
	Scott Wood, Hanjun Guo, Ding Tianhong, dann frazier,
	Thomas Gleixner

this_cpu_has_cap() only checks the feature array, and not the errata
one. In order to be able to check for a CPU-local erratum, allow it
to inspect the latter as well.

This is consistent with cpus_have_cap()'s behaviour, which includes
errata already.

Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm64/kernel/cpufeature.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index abda8e861865..6eb77ae99b79 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -1090,20 +1090,29 @@ static void __init setup_feature_capabilities(void)
  * Check if the current CPU has a given feature capability.
  * Should be called from non-preemptible context.
  */
-bool this_cpu_has_cap(unsigned int cap)
+static bool __this_cpu_has_cap(const struct arm64_cpu_capabilities *cap_array,
+			       unsigned int cap)
 {
 	const struct arm64_cpu_capabilities *caps;
 
 	if (WARN_ON(preemptible()))
 		return false;
 
-	for (caps = arm64_features; caps->desc; caps++)
+	for (caps = cap_array; caps->desc; caps++)
 		if (caps->capability == cap && caps->matches)
 			return caps->matches(caps, SCOPE_LOCAL_CPU);
 
 	return false;
 }
 
+extern const struct arm64_cpu_capabilities arm64_errata[];
+
+bool this_cpu_has_cap(unsigned int cap)
+{
+	return (__this_cpu_has_cap(arm64_features, cap) ||
+		__this_cpu_has_cap(arm64_errata, cap));
+}
+
 void __init setup_cpu_features(void)
 {
 	u32 cwg;
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 59+ messages in thread

* [PATCH v3 01/18] arm64: Allow checking of a CPU-local erratum
@ 2017-04-04 17:18   ` Marc Zyngier
  0 siblings, 0 replies; 59+ messages in thread
From: Marc Zyngier @ 2017-04-04 17:18 UTC (permalink / raw)
  To: linux-arm-kernel

this_cpu_has_cap() only checks the feature array, and not the errata
one. In order to be able to check for a CPU-local erratum, allow it
to inspect the latter as well.

This is consistent with cpus_have_cap()'s behaviour, which includes
errata already.

Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm64/kernel/cpufeature.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index abda8e861865..6eb77ae99b79 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -1090,20 +1090,29 @@ static void __init setup_feature_capabilities(void)
  * Check if the current CPU has a given feature capability.
  * Should be called from non-preemptible context.
  */
-bool this_cpu_has_cap(unsigned int cap)
+static bool __this_cpu_has_cap(const struct arm64_cpu_capabilities *cap_array,
+			       unsigned int cap)
 {
 	const struct arm64_cpu_capabilities *caps;
 
 	if (WARN_ON(preemptible()))
 		return false;
 
-	for (caps = arm64_features; caps->desc; caps++)
+	for (caps = cap_array; caps->desc; caps++)
 		if (caps->capability == cap && caps->matches)
 			return caps->matches(caps, SCOPE_LOCAL_CPU);
 
 	return false;
 }
 
+extern const struct arm64_cpu_capabilities arm64_errata[];
+
+bool this_cpu_has_cap(unsigned int cap)
+{
+	return (__this_cpu_has_cap(arm64_features, cap) ||
+		__this_cpu_has_cap(arm64_errata, cap));
+}
+
 void __init setup_cpu_features(void)
 {
 	u32 cwg;
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 59+ messages in thread

* [PATCH v3 02/18] arm64: Add CNTVCT_EL0 trap handler
  2017-04-04 17:18 ` Marc Zyngier
@ 2017-04-04 17:18   ` Marc Zyngier
  -1 siblings, 0 replies; 59+ messages in thread
From: Marc Zyngier @ 2017-04-04 17:18 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel
  Cc: Mark Rutland, Catalin Marinas, Daniel Lezcano, Will Deacon,
	Scott Wood, Hanjun Guo, Ding Tianhong, dann frazier,
	Thomas Gleixner

Since people seem to make a point in breaking the userspace visible
counter, we have no choice but to trap the access. Add the required
handler.

Acked-by: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm64/include/asm/esr.h |  2 ++
 arch/arm64/kernel/traps.c    | 14 ++++++++++++++
 2 files changed, 16 insertions(+)

diff --git a/arch/arm64/include/asm/esr.h b/arch/arm64/include/asm/esr.h
index d14c478976d0..ad42e79a5d4d 100644
--- a/arch/arm64/include/asm/esr.h
+++ b/arch/arm64/include/asm/esr.h
@@ -175,6 +175,8 @@
 #define ESR_ELx_SYS64_ISS_SYS_CTR_READ	(ESR_ELx_SYS64_ISS_SYS_CTR | \
 					 ESR_ELx_SYS64_ISS_DIR_READ)
 
+#define ESR_ELx_SYS64_ISS_SYS_CNTVCT	(ESR_ELx_SYS64_ISS_SYS_VAL(3, 3, 2, 14, 0) | \
+					 ESR_ELx_SYS64_ISS_DIR_READ)
 #ifndef __ASSEMBLY__
 #include <asm/types.h>
 
diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
index e52be6aa44ee..1de444e6c669 100644
--- a/arch/arm64/kernel/traps.c
+++ b/arch/arm64/kernel/traps.c
@@ -505,6 +505,14 @@ static void ctr_read_handler(unsigned int esr, struct pt_regs *regs)
 	regs->pc += 4;
 }
 
+static void cntvct_read_handler(unsigned int esr, struct pt_regs *regs)
+{
+	int rt = (esr & ESR_ELx_SYS64_ISS_RT_MASK) >> ESR_ELx_SYS64_ISS_RT_SHIFT;
+
+	pt_regs_write_reg(regs, rt, arch_counter_get_cntvct());
+	regs->pc += 4;
+}
+
 struct sys64_hook {
 	unsigned int esr_mask;
 	unsigned int esr_val;
@@ -523,6 +531,12 @@ static struct sys64_hook sys64_hooks[] = {
 		.esr_val = ESR_ELx_SYS64_ISS_SYS_CTR_READ,
 		.handler = ctr_read_handler,
 	},
+	{
+		/* Trap read access to CNTVCT_EL0 */
+		.esr_mask = ESR_ELx_SYS64_ISS_SYS_OP_MASK,
+		.esr_val = ESR_ELx_SYS64_ISS_SYS_CNTVCT,
+		.handler = cntvct_read_handler,
+	},
 	{},
 };
 
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 59+ messages in thread

* [PATCH v3 02/18] arm64: Add CNTVCT_EL0 trap handler
@ 2017-04-04 17:18   ` Marc Zyngier
  0 siblings, 0 replies; 59+ messages in thread
From: Marc Zyngier @ 2017-04-04 17:18 UTC (permalink / raw)
  To: linux-arm-kernel

Since people seem to make a point in breaking the userspace visible
counter, we have no choice but to trap the access. Add the required
handler.

Acked-by: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm64/include/asm/esr.h |  2 ++
 arch/arm64/kernel/traps.c    | 14 ++++++++++++++
 2 files changed, 16 insertions(+)

diff --git a/arch/arm64/include/asm/esr.h b/arch/arm64/include/asm/esr.h
index d14c478976d0..ad42e79a5d4d 100644
--- a/arch/arm64/include/asm/esr.h
+++ b/arch/arm64/include/asm/esr.h
@@ -175,6 +175,8 @@
 #define ESR_ELx_SYS64_ISS_SYS_CTR_READ	(ESR_ELx_SYS64_ISS_SYS_CTR | \
 					 ESR_ELx_SYS64_ISS_DIR_READ)
 
+#define ESR_ELx_SYS64_ISS_SYS_CNTVCT	(ESR_ELx_SYS64_ISS_SYS_VAL(3, 3, 2, 14, 0) | \
+					 ESR_ELx_SYS64_ISS_DIR_READ)
 #ifndef __ASSEMBLY__
 #include <asm/types.h>
 
diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
index e52be6aa44ee..1de444e6c669 100644
--- a/arch/arm64/kernel/traps.c
+++ b/arch/arm64/kernel/traps.c
@@ -505,6 +505,14 @@ static void ctr_read_handler(unsigned int esr, struct pt_regs *regs)
 	regs->pc += 4;
 }
 
+static void cntvct_read_handler(unsigned int esr, struct pt_regs *regs)
+{
+	int rt = (esr & ESR_ELx_SYS64_ISS_RT_MASK) >> ESR_ELx_SYS64_ISS_RT_SHIFT;
+
+	pt_regs_write_reg(regs, rt, arch_counter_get_cntvct());
+	regs->pc += 4;
+}
+
 struct sys64_hook {
 	unsigned int esr_mask;
 	unsigned int esr_val;
@@ -523,6 +531,12 @@ static struct sys64_hook sys64_hooks[] = {
 		.esr_val = ESR_ELx_SYS64_ISS_SYS_CTR_READ,
 		.handler = ctr_read_handler,
 	},
+	{
+		/* Trap read access to CNTVCT_EL0 */
+		.esr_mask = ESR_ELx_SYS64_ISS_SYS_OP_MASK,
+		.esr_val = ESR_ELx_SYS64_ISS_SYS_CNTVCT,
+		.handler = cntvct_read_handler,
+	},
 	{},
 };
 
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 59+ messages in thread

* [PATCH v3 03/18] arm64: Define Cortex-A73 MIDR
  2017-04-04 17:18 ` Marc Zyngier
@ 2017-04-04 17:18   ` Marc Zyngier
  -1 siblings, 0 replies; 59+ messages in thread
From: Marc Zyngier @ 2017-04-04 17:18 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel
  Cc: Mark Rutland, Catalin Marinas, Daniel Lezcano, Will Deacon,
	Scott Wood, Hanjun Guo, Ding Tianhong, dann frazier,
	Thomas Gleixner

As we're about to introduce a new workaround that is specific to
Cortex-A73, let's define the coresponding MIDR.

Acked-by: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm64/include/asm/cputype.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm64/include/asm/cputype.h b/arch/arm64/include/asm/cputype.h
index fc502713ab37..0984d1b3a8f2 100644
--- a/arch/arm64/include/asm/cputype.h
+++ b/arch/arm64/include/asm/cputype.h
@@ -80,6 +80,7 @@
 #define ARM_CPU_PART_FOUNDATION		0xD00
 #define ARM_CPU_PART_CORTEX_A57		0xD07
 #define ARM_CPU_PART_CORTEX_A53		0xD03
+#define ARM_CPU_PART_CORTEX_A73		0xD09
 
 #define APM_CPU_PART_POTENZA		0x000
 
@@ -92,6 +93,7 @@
 
 #define MIDR_CORTEX_A53 MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_A53)
 #define MIDR_CORTEX_A57 MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_A57)
+#define MIDR_CORTEX_A73 MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_A73)
 #define MIDR_THUNDERX	MIDR_CPU_MODEL(ARM_CPU_IMP_CAVIUM, CAVIUM_CPU_PART_THUNDERX)
 #define MIDR_THUNDERX_81XX MIDR_CPU_MODEL(ARM_CPU_IMP_CAVIUM, CAVIUM_CPU_PART_THUNDERX_81XX)
 #define MIDR_QCOM_FALKOR_V1 MIDR_CPU_MODEL(ARM_CPU_IMP_QCOM, QCOM_CPU_PART_FALKOR_V1)
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 59+ messages in thread

* [PATCH v3 03/18] arm64: Define Cortex-A73 MIDR
@ 2017-04-04 17:18   ` Marc Zyngier
  0 siblings, 0 replies; 59+ messages in thread
From: Marc Zyngier @ 2017-04-04 17:18 UTC (permalink / raw)
  To: linux-arm-kernel

As we're about to introduce a new workaround that is specific to
Cortex-A73, let's define the coresponding MIDR.

Acked-by: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm64/include/asm/cputype.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm64/include/asm/cputype.h b/arch/arm64/include/asm/cputype.h
index fc502713ab37..0984d1b3a8f2 100644
--- a/arch/arm64/include/asm/cputype.h
+++ b/arch/arm64/include/asm/cputype.h
@@ -80,6 +80,7 @@
 #define ARM_CPU_PART_FOUNDATION		0xD00
 #define ARM_CPU_PART_CORTEX_A57		0xD07
 #define ARM_CPU_PART_CORTEX_A53		0xD03
+#define ARM_CPU_PART_CORTEX_A73		0xD09
 
 #define APM_CPU_PART_POTENZA		0x000
 
@@ -92,6 +93,7 @@
 
 #define MIDR_CORTEX_A53 MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_A53)
 #define MIDR_CORTEX_A57 MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_A57)
+#define MIDR_CORTEX_A73 MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_A73)
 #define MIDR_THUNDERX	MIDR_CPU_MODEL(ARM_CPU_IMP_CAVIUM, CAVIUM_CPU_PART_THUNDERX)
 #define MIDR_THUNDERX_81XX MIDR_CPU_MODEL(ARM_CPU_IMP_CAVIUM, CAVIUM_CPU_PART_THUNDERX_81XX)
 #define MIDR_QCOM_FALKOR_V1 MIDR_CPU_MODEL(ARM_CPU_IMP_QCOM, QCOM_CPU_PART_FALKOR_V1)
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 59+ messages in thread

* [PATCH v3 04/18] arm64: cpu_errata: Allow an erratum to be match for all revisions of a core
  2017-04-04 17:18 ` Marc Zyngier
@ 2017-04-04 17:18   ` Marc Zyngier
  -1 siblings, 0 replies; 59+ messages in thread
From: Marc Zyngier @ 2017-04-04 17:18 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel
  Cc: Mark Rutland, Catalin Marinas, Daniel Lezcano, Will Deacon,
	Scott Wood, Hanjun Guo, Ding Tianhong, dann frazier,
	Thomas Gleixner

Some minor erratum may not be fixed in further revisions of a core,
leading to a situation where the workaround needs to be updated each
time an updated core is released.

Introduce a MIDR_ALL_VERSIONS match helper that will work for all
versions of that MIDR, once and for all.

Acked-by: Mark Rutland <mark.rutland@arm.com>
Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm64/kernel/cpu_errata.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
index f6cc67e7626e..2be1d1c05303 100644
--- a/arch/arm64/kernel/cpu_errata.c
+++ b/arch/arm64/kernel/cpu_errata.c
@@ -53,6 +53,13 @@ static int cpu_enable_trap_ctr_access(void *__unused)
 	.midr_range_min = min, \
 	.midr_range_max = max
 
+#define MIDR_ALL_VERSIONS(model) \
+	.def_scope = SCOPE_LOCAL_CPU, \
+	.matches = is_affected_midr_range, \
+	.midr_model = model, \
+	.midr_range_min = 0, \
+	.midr_range_max = (MIDR_VARIANT_MASK | MIDR_REVISION_MASK)
+
 const struct arm64_cpu_capabilities arm64_errata[] = {
 #if	defined(CONFIG_ARM64_ERRATUM_826319) || \
 	defined(CONFIG_ARM64_ERRATUM_827319) || \
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 59+ messages in thread

* [PATCH v3 04/18] arm64: cpu_errata: Allow an erratum to be match for all revisions of a core
@ 2017-04-04 17:18   ` Marc Zyngier
  0 siblings, 0 replies; 59+ messages in thread
From: Marc Zyngier @ 2017-04-04 17:18 UTC (permalink / raw)
  To: linux-arm-kernel

Some minor erratum may not be fixed in further revisions of a core,
leading to a situation where the workaround needs to be updated each
time an updated core is released.

Introduce a MIDR_ALL_VERSIONS match helper that will work for all
versions of that MIDR, once and for all.

Acked-by: Mark Rutland <mark.rutland@arm.com>
Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm64/kernel/cpu_errata.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
index f6cc67e7626e..2be1d1c05303 100644
--- a/arch/arm64/kernel/cpu_errata.c
+++ b/arch/arm64/kernel/cpu_errata.c
@@ -53,6 +53,13 @@ static int cpu_enable_trap_ctr_access(void *__unused)
 	.midr_range_min = min, \
 	.midr_range_max = max
 
+#define MIDR_ALL_VERSIONS(model) \
+	.def_scope = SCOPE_LOCAL_CPU, \
+	.matches = is_affected_midr_range, \
+	.midr_model = model, \
+	.midr_range_min = 0, \
+	.midr_range_max = (MIDR_VARIANT_MASK | MIDR_REVISION_MASK)
+
 const struct arm64_cpu_capabilities arm64_errata[] = {
 #if	defined(CONFIG_ARM64_ERRATUM_826319) || \
 	defined(CONFIG_ARM64_ERRATUM_827319) || \
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 59+ messages in thread

* [PATCH v3 05/18] arm64: cpu_errata: Add capability to advertise Cortex-A73 erratum 858921
  2017-04-04 17:18 ` Marc Zyngier
@ 2017-04-04 17:18   ` Marc Zyngier
  -1 siblings, 0 replies; 59+ messages in thread
From: Marc Zyngier @ 2017-04-04 17:18 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel
  Cc: Mark Rutland, Catalin Marinas, Daniel Lezcano, Will Deacon,
	Scott Wood, Hanjun Guo, Ding Tianhong, dann frazier,
	Thomas Gleixner

In order to work around Cortex-A73 erratum 858921 in a subsequent
patch, add the required capability that advertise the erratum.

As the configuration option it depends on is not present yet,
this has no immediate effect.

Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 Documentation/arm64/silicon-errata.txt | 1 +
 arch/arm64/include/asm/cpucaps.h       | 3 ++-
 arch/arm64/kernel/cpu_errata.c         | 8 ++++++++
 3 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/Documentation/arm64/silicon-errata.txt b/Documentation/arm64/silicon-errata.txt
index 2f66683500b8..10f2dddbf449 100644
--- a/Documentation/arm64/silicon-errata.txt
+++ b/Documentation/arm64/silicon-errata.txt
@@ -54,6 +54,7 @@ stable kernels.
 | ARM            | Cortex-A57      | #852523         | N/A                         |
 | ARM            | Cortex-A57      | #834220         | ARM64_ERRATUM_834220        |
 | ARM            | Cortex-A72      | #853709         | N/A                         |
+| ARM            | Cortex-A73      | #858921         | ARM64_ERRATUM_858921        |
 | ARM            | MMU-500         | #841119,#826419 | N/A                         |
 |                |                 |                 |                             |
 | Cavium         | ThunderX ITS    | #22375, #24313  | CAVIUM_ERRATUM_22375        |
diff --git a/arch/arm64/include/asm/cpucaps.h b/arch/arm64/include/asm/cpucaps.h
index fb78a5d3b60b..b3aab8a17868 100644
--- a/arch/arm64/include/asm/cpucaps.h
+++ b/arch/arm64/include/asm/cpucaps.h
@@ -37,7 +37,8 @@
 #define ARM64_HAS_NO_FPSIMD			16
 #define ARM64_WORKAROUND_REPEAT_TLBI		17
 #define ARM64_WORKAROUND_QCOM_FALKOR_E1003	18
+#define ARM64_WORKAROUND_858921			19
 
-#define ARM64_NCAPS				19
+#define ARM64_NCAPS				20
 
 #endif /* __ASM_CPUCAPS_H */
diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
index 2be1d1c05303..2ed2a7657711 100644
--- a/arch/arm64/kernel/cpu_errata.c
+++ b/arch/arm64/kernel/cpu_errata.c
@@ -158,6 +158,14 @@ const struct arm64_cpu_capabilities arm64_errata[] = {
 			   MIDR_CPU_VAR_REV(0, 0)),
 	},
 #endif
+#ifdef CONFIG_ARM64_ERRATUM_858921
+	{
+	/* Cortex-A73 all versions */
+		.desc = "ARM erratum 858921",
+		.capability = ARM64_WORKAROUND_858921,
+		MIDR_ALL_VERSIONS(MIDR_CORTEX_A73),
+	},
+#endif
 	{
 	}
 };
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 59+ messages in thread

* [PATCH v3 05/18] arm64: cpu_errata: Add capability to advertise Cortex-A73 erratum 858921
@ 2017-04-04 17:18   ` Marc Zyngier
  0 siblings, 0 replies; 59+ messages in thread
From: Marc Zyngier @ 2017-04-04 17:18 UTC (permalink / raw)
  To: linux-arm-kernel

In order to work around Cortex-A73 erratum 858921 in a subsequent
patch, add the required capability that advertise the erratum.

As the configuration option it depends on is not present yet,
this has no immediate effect.

Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 Documentation/arm64/silicon-errata.txt | 1 +
 arch/arm64/include/asm/cpucaps.h       | 3 ++-
 arch/arm64/kernel/cpu_errata.c         | 8 ++++++++
 3 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/Documentation/arm64/silicon-errata.txt b/Documentation/arm64/silicon-errata.txt
index 2f66683500b8..10f2dddbf449 100644
--- a/Documentation/arm64/silicon-errata.txt
+++ b/Documentation/arm64/silicon-errata.txt
@@ -54,6 +54,7 @@ stable kernels.
 | ARM            | Cortex-A57      | #852523         | N/A                         |
 | ARM            | Cortex-A57      | #834220         | ARM64_ERRATUM_834220        |
 | ARM            | Cortex-A72      | #853709         | N/A                         |
+| ARM            | Cortex-A73      | #858921         | ARM64_ERRATUM_858921        |
 | ARM            | MMU-500         | #841119,#826419 | N/A                         |
 |                |                 |                 |                             |
 | Cavium         | ThunderX ITS    | #22375, #24313  | CAVIUM_ERRATUM_22375        |
diff --git a/arch/arm64/include/asm/cpucaps.h b/arch/arm64/include/asm/cpucaps.h
index fb78a5d3b60b..b3aab8a17868 100644
--- a/arch/arm64/include/asm/cpucaps.h
+++ b/arch/arm64/include/asm/cpucaps.h
@@ -37,7 +37,8 @@
 #define ARM64_HAS_NO_FPSIMD			16
 #define ARM64_WORKAROUND_REPEAT_TLBI		17
 #define ARM64_WORKAROUND_QCOM_FALKOR_E1003	18
+#define ARM64_WORKAROUND_858921			19
 
-#define ARM64_NCAPS				19
+#define ARM64_NCAPS				20
 
 #endif /* __ASM_CPUCAPS_H */
diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
index 2be1d1c05303..2ed2a7657711 100644
--- a/arch/arm64/kernel/cpu_errata.c
+++ b/arch/arm64/kernel/cpu_errata.c
@@ -158,6 +158,14 @@ const struct arm64_cpu_capabilities arm64_errata[] = {
 			   MIDR_CPU_VAR_REV(0, 0)),
 	},
 #endif
+#ifdef CONFIG_ARM64_ERRATUM_858921
+	{
+	/* Cortex-A73 all versions */
+		.desc = "ARM erratum 858921",
+		.capability = ARM64_WORKAROUND_858921,
+		MIDR_ALL_VERSIONS(MIDR_CORTEX_A73),
+	},
+#endif
 	{
 	}
 };
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 59+ messages in thread

* [PATCH v3 06/18] arm64: arch_timer: Add infrastructure for multiple erratum detection methods
  2017-04-04 17:18 ` Marc Zyngier
@ 2017-04-04 17:18   ` Marc Zyngier
  -1 siblings, 0 replies; 59+ messages in thread
From: Marc Zyngier @ 2017-04-04 17:18 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel
  Cc: Mark Rutland, Catalin Marinas, Daniel Lezcano, Will Deacon,
	Scott Wood, Hanjun Guo, Ding Tianhong, dann frazier,
	Thomas Gleixner

We're currently stuck with DT when it comes to handling errata, which
is pretty restrictive. In order to make things more flexible, let's
introduce an infrastructure that could support alternative discovery
methods. No change in functionality.

Reviewed-by: Hanjun Guo <hanjun.guo@linaro.org>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm64/include/asm/arch_timer.h  |  7 ++-
 drivers/clocksource/arm_arch_timer.c | 83 +++++++++++++++++++++++++++++++-----
 2 files changed, 78 insertions(+), 12 deletions(-)

diff --git a/arch/arm64/include/asm/arch_timer.h b/arch/arm64/include/asm/arch_timer.h
index b4b34004a21e..5cd964e90d11 100644
--- a/arch/arm64/include/asm/arch_timer.h
+++ b/arch/arm64/include/asm/arch_timer.h
@@ -37,9 +37,14 @@ extern struct static_key_false arch_timer_read_ool_enabled;
 #define needs_unstable_timer_counter_workaround()  false
 #endif
 
+enum arch_timer_erratum_match_type {
+	ate_match_dt,
+};
 
 struct arch_timer_erratum_workaround {
-	const char *id;		/* Indicate the Erratum ID */
+	enum arch_timer_erratum_match_type match_type;
+	const void *id;
+	const char *desc;
 	u32 (*read_cntp_tval_el0)(void);
 	u32 (*read_cntv_tval_el0)(void);
 	u64 (*read_cntvct_el0)(void);
diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index 7a8a4117f123..5c5c2af74ad9 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -182,7 +182,9 @@ EXPORT_SYMBOL_GPL(arch_timer_read_ool_enabled);
 static const struct arch_timer_erratum_workaround ool_workarounds[] = {
 #ifdef CONFIG_FSL_ERRATUM_A008585
 	{
+		.match_type = ate_match_dt,
 		.id = "fsl,erratum-a008585",
+		.desc = "Freescale erratum a005858",
 		.read_cntp_tval_el0 = fsl_a008585_read_cntp_tval_el0,
 		.read_cntv_tval_el0 = fsl_a008585_read_cntv_tval_el0,
 		.read_cntvct_el0 = fsl_a008585_read_cntvct_el0,
@@ -190,13 +192,81 @@ static const struct arch_timer_erratum_workaround ool_workarounds[] = {
 #endif
 #ifdef CONFIG_HISILICON_ERRATUM_161010101
 	{
+		.match_type = ate_match_dt,
 		.id = "hisilicon,erratum-161010101",
+		.desc = "HiSilicon erratum 161010101",
 		.read_cntp_tval_el0 = hisi_161010101_read_cntp_tval_el0,
 		.read_cntv_tval_el0 = hisi_161010101_read_cntv_tval_el0,
 		.read_cntvct_el0 = hisi_161010101_read_cntvct_el0,
 	},
 #endif
 };
+
+typedef bool (*ate_match_fn_t)(const struct arch_timer_erratum_workaround *,
+			       const void *);
+
+static
+bool arch_timer_check_dt_erratum(const struct arch_timer_erratum_workaround *wa,
+				 const void *arg)
+{
+	const struct device_node *np = arg;
+
+	return of_property_read_bool(np, wa->id);
+}
+
+static const struct arch_timer_erratum_workaround *
+arch_timer_iterate_errata(enum arch_timer_erratum_match_type type,
+			  ate_match_fn_t match_fn,
+			  void *arg)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(ool_workarounds); i++) {
+		if (ool_workarounds[i].match_type != type)
+			continue;
+
+		if (match_fn(&ool_workarounds[i], arg))
+			return &ool_workarounds[i];
+	}
+
+	return NULL;
+}
+
+static
+void arch_timer_enable_workaround(const struct arch_timer_erratum_workaround *wa)
+{
+	timer_unstable_counter_workaround = wa;
+	static_branch_enable(&arch_timer_read_ool_enabled);
+}
+
+static void arch_timer_check_ool_workaround(enum arch_timer_erratum_match_type type,
+					    void *arg)
+{
+	const struct arch_timer_erratum_workaround *wa;
+	ate_match_fn_t match_fn = NULL;
+
+	if (static_branch_unlikely(&arch_timer_read_ool_enabled))
+		return;
+
+	switch (type) {
+	case ate_match_dt:
+		match_fn = arch_timer_check_dt_erratum;
+		break;
+	default:
+		WARN_ON(1);
+		return;
+	}
+
+	wa = arch_timer_iterate_errata(type, match_fn, arg);
+	if (!wa)
+		return;
+
+	arch_timer_enable_workaround(wa);
+	pr_info("Enabling global workaround for %s\n", wa->desc);
+}
+
+#else
+#define arch_timer_check_ool_workaround(t,a)		do { } while(0)
 #endif /* CONFIG_ARM_ARCH_TIMER_OOL_WORKAROUND */
 
 static __always_inline
@@ -960,17 +1030,8 @@ static int __init arch_timer_of_init(struct device_node *np)
 
 	arch_timer_c3stop = !of_property_read_bool(np, "always-on");
 
-#ifdef CONFIG_ARM_ARCH_TIMER_OOL_WORKAROUND
-	for (i = 0; i < ARRAY_SIZE(ool_workarounds); i++) {
-		if (of_property_read_bool(np, ool_workarounds[i].id)) {
-			timer_unstable_counter_workaround = &ool_workarounds[i];
-			static_branch_enable(&arch_timer_read_ool_enabled);
-			pr_info("arch_timer: Enabling workaround for %s\n",
-				timer_unstable_counter_workaround->id);
-			break;
-		}
-	}
-#endif
+	/* Check for globally applicable workarounds */
+	arch_timer_check_ool_workaround(ate_match_dt, np);
 
 	/*
 	 * If we cannot rely on firmware initializing the timer registers then
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 59+ messages in thread

* [PATCH v3 06/18] arm64: arch_timer: Add infrastructure for multiple erratum detection methods
@ 2017-04-04 17:18   ` Marc Zyngier
  0 siblings, 0 replies; 59+ messages in thread
From: Marc Zyngier @ 2017-04-04 17:18 UTC (permalink / raw)
  To: linux-arm-kernel

We're currently stuck with DT when it comes to handling errata, which
is pretty restrictive. In order to make things more flexible, let's
introduce an infrastructure that could support alternative discovery
methods. No change in functionality.

Reviewed-by: Hanjun Guo <hanjun.guo@linaro.org>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm64/include/asm/arch_timer.h  |  7 ++-
 drivers/clocksource/arm_arch_timer.c | 83 +++++++++++++++++++++++++++++++-----
 2 files changed, 78 insertions(+), 12 deletions(-)

diff --git a/arch/arm64/include/asm/arch_timer.h b/arch/arm64/include/asm/arch_timer.h
index b4b34004a21e..5cd964e90d11 100644
--- a/arch/arm64/include/asm/arch_timer.h
+++ b/arch/arm64/include/asm/arch_timer.h
@@ -37,9 +37,14 @@ extern struct static_key_false arch_timer_read_ool_enabled;
 #define needs_unstable_timer_counter_workaround()  false
 #endif
 
+enum arch_timer_erratum_match_type {
+	ate_match_dt,
+};
 
 struct arch_timer_erratum_workaround {
-	const char *id;		/* Indicate the Erratum ID */
+	enum arch_timer_erratum_match_type match_type;
+	const void *id;
+	const char *desc;
 	u32 (*read_cntp_tval_el0)(void);
 	u32 (*read_cntv_tval_el0)(void);
 	u64 (*read_cntvct_el0)(void);
diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index 7a8a4117f123..5c5c2af74ad9 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -182,7 +182,9 @@ EXPORT_SYMBOL_GPL(arch_timer_read_ool_enabled);
 static const struct arch_timer_erratum_workaround ool_workarounds[] = {
 #ifdef CONFIG_FSL_ERRATUM_A008585
 	{
+		.match_type = ate_match_dt,
 		.id = "fsl,erratum-a008585",
+		.desc = "Freescale erratum a005858",
 		.read_cntp_tval_el0 = fsl_a008585_read_cntp_tval_el0,
 		.read_cntv_tval_el0 = fsl_a008585_read_cntv_tval_el0,
 		.read_cntvct_el0 = fsl_a008585_read_cntvct_el0,
@@ -190,13 +192,81 @@ static const struct arch_timer_erratum_workaround ool_workarounds[] = {
 #endif
 #ifdef CONFIG_HISILICON_ERRATUM_161010101
 	{
+		.match_type = ate_match_dt,
 		.id = "hisilicon,erratum-161010101",
+		.desc = "HiSilicon erratum 161010101",
 		.read_cntp_tval_el0 = hisi_161010101_read_cntp_tval_el0,
 		.read_cntv_tval_el0 = hisi_161010101_read_cntv_tval_el0,
 		.read_cntvct_el0 = hisi_161010101_read_cntvct_el0,
 	},
 #endif
 };
+
+typedef bool (*ate_match_fn_t)(const struct arch_timer_erratum_workaround *,
+			       const void *);
+
+static
+bool arch_timer_check_dt_erratum(const struct arch_timer_erratum_workaround *wa,
+				 const void *arg)
+{
+	const struct device_node *np = arg;
+
+	return of_property_read_bool(np, wa->id);
+}
+
+static const struct arch_timer_erratum_workaround *
+arch_timer_iterate_errata(enum arch_timer_erratum_match_type type,
+			  ate_match_fn_t match_fn,
+			  void *arg)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(ool_workarounds); i++) {
+		if (ool_workarounds[i].match_type != type)
+			continue;
+
+		if (match_fn(&ool_workarounds[i], arg))
+			return &ool_workarounds[i];
+	}
+
+	return NULL;
+}
+
+static
+void arch_timer_enable_workaround(const struct arch_timer_erratum_workaround *wa)
+{
+	timer_unstable_counter_workaround = wa;
+	static_branch_enable(&arch_timer_read_ool_enabled);
+}
+
+static void arch_timer_check_ool_workaround(enum arch_timer_erratum_match_type type,
+					    void *arg)
+{
+	const struct arch_timer_erratum_workaround *wa;
+	ate_match_fn_t match_fn = NULL;
+
+	if (static_branch_unlikely(&arch_timer_read_ool_enabled))
+		return;
+
+	switch (type) {
+	case ate_match_dt:
+		match_fn = arch_timer_check_dt_erratum;
+		break;
+	default:
+		WARN_ON(1);
+		return;
+	}
+
+	wa = arch_timer_iterate_errata(type, match_fn, arg);
+	if (!wa)
+		return;
+
+	arch_timer_enable_workaround(wa);
+	pr_info("Enabling global workaround for %s\n", wa->desc);
+}
+
+#else
+#define arch_timer_check_ool_workaround(t,a)		do { } while(0)
 #endif /* CONFIG_ARM_ARCH_TIMER_OOL_WORKAROUND */
 
 static __always_inline
@@ -960,17 +1030,8 @@ static int __init arch_timer_of_init(struct device_node *np)
 
 	arch_timer_c3stop = !of_property_read_bool(np, "always-on");
 
-#ifdef CONFIG_ARM_ARCH_TIMER_OOL_WORKAROUND
-	for (i = 0; i < ARRAY_SIZE(ool_workarounds); i++) {
-		if (of_property_read_bool(np, ool_workarounds[i].id)) {
-			timer_unstable_counter_workaround = &ool_workarounds[i];
-			static_branch_enable(&arch_timer_read_ool_enabled);
-			pr_info("arch_timer: Enabling workaround for %s\n",
-				timer_unstable_counter_workaround->id);
-			break;
-		}
-	}
-#endif
+	/* Check for globally applicable workarounds */
+	arch_timer_check_ool_workaround(ate_match_dt, np);
 
 	/*
 	 * If we cannot rely on firmware initializing the timer registers then
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 59+ messages in thread

* [PATCH v3 07/18] arm64: arch_timer: Add erratum handler for CPU-specific capability
  2017-04-04 17:18 ` Marc Zyngier
@ 2017-04-04 17:18   ` Marc Zyngier
  -1 siblings, 0 replies; 59+ messages in thread
From: Marc Zyngier @ 2017-04-04 17:18 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel
  Cc: Mark Rutland, Catalin Marinas, Daniel Lezcano, Will Deacon,
	Scott Wood, Hanjun Guo, Ding Tianhong, dann frazier,
	Thomas Gleixner

Should we ever have a workaround for an erratum that is detected using
a capability and affecting a particular CPU, it'd be nice to have
a way to probe them directly.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm64/include/asm/arch_timer.h  |  1 +
 drivers/clocksource/arm_arch_timer.c | 28 ++++++++++++++++++++++++----
 2 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/include/asm/arch_timer.h b/arch/arm64/include/asm/arch_timer.h
index 5cd964e90d11..1b0d7e994e0c 100644
--- a/arch/arm64/include/asm/arch_timer.h
+++ b/arch/arm64/include/asm/arch_timer.h
@@ -39,6 +39,7 @@ extern struct static_key_false arch_timer_read_ool_enabled;
 
 enum arch_timer_erratum_match_type {
 	ate_match_dt,
+	ate_match_local_cap_id,
 };
 
 struct arch_timer_erratum_workaround {
diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index 5c5c2af74ad9..532e47fa43b3 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -214,6 +214,13 @@ bool arch_timer_check_dt_erratum(const struct arch_timer_erratum_workaround *wa,
 	return of_property_read_bool(np, wa->id);
 }
 
+static
+bool arch_timer_check_local_cap_erratum(const struct arch_timer_erratum_workaround *wa,
+					const void *arg)
+{
+	return this_cpu_has_cap((uintptr_t)wa->id);
+}
+
 static const struct arch_timer_erratum_workaround *
 arch_timer_iterate_errata(enum arch_timer_erratum_match_type type,
 			  ate_match_fn_t match_fn,
@@ -244,14 +251,16 @@ static void arch_timer_check_ool_workaround(enum arch_timer_erratum_match_type t
 {
 	const struct arch_timer_erratum_workaround *wa;
 	ate_match_fn_t match_fn = NULL;
-
-	if (static_branch_unlikely(&arch_timer_read_ool_enabled))
-		return;
+	bool local = false;
 
 	switch (type) {
 	case ate_match_dt:
 		match_fn = arch_timer_check_dt_erratum;
 		break;
+	case ate_match_local_cap_id:
+		match_fn = arch_timer_check_local_cap_erratum;
+		local = true;
+		break;
 	default:
 		WARN_ON(1);
 		return;
@@ -261,8 +270,17 @@ static void arch_timer_check_ool_workaround(enum arch_timer_erratum_match_type t
 	if (!wa)
 		return;
 
+	if (needs_unstable_timer_counter_workaround()) {
+		if (wa != timer_unstable_counter_workaround)
+			pr_warn("Can't enable workaround for %s (clashes with %s\n)",
+				wa->desc,
+				timer_unstable_counter_workaround->desc);
+		return;
+	}
+
 	arch_timer_enable_workaround(wa);
-	pr_info("Enabling global workaround for %s\n", wa->desc);
+	pr_info("Enabling %s workaround for %s\n",
+		local ? "local" : "global", wa->desc);
 }
 
 #else
@@ -522,6 +540,8 @@ static void __arch_timer_setup(unsigned type,
 			BUG();
 		}
 
+		arch_timer_check_ool_workaround(ate_match_local_cap_id, NULL);
+
 		erratum_workaround_set_sne(clk);
 	} else {
 		clk->features |= CLOCK_EVT_FEAT_DYNIRQ;
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 59+ messages in thread

* [PATCH v3 07/18] arm64: arch_timer: Add erratum handler for CPU-specific capability
@ 2017-04-04 17:18   ` Marc Zyngier
  0 siblings, 0 replies; 59+ messages in thread
From: Marc Zyngier @ 2017-04-04 17:18 UTC (permalink / raw)
  To: linux-arm-kernel

Should we ever have a workaround for an erratum that is detected using
a capability and affecting a particular CPU, it'd be nice to have
a way to probe them directly.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm64/include/asm/arch_timer.h  |  1 +
 drivers/clocksource/arm_arch_timer.c | 28 ++++++++++++++++++++++++----
 2 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/include/asm/arch_timer.h b/arch/arm64/include/asm/arch_timer.h
index 5cd964e90d11..1b0d7e994e0c 100644
--- a/arch/arm64/include/asm/arch_timer.h
+++ b/arch/arm64/include/asm/arch_timer.h
@@ -39,6 +39,7 @@ extern struct static_key_false arch_timer_read_ool_enabled;
 
 enum arch_timer_erratum_match_type {
 	ate_match_dt,
+	ate_match_local_cap_id,
 };
 
 struct arch_timer_erratum_workaround {
diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index 5c5c2af74ad9..532e47fa43b3 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -214,6 +214,13 @@ bool arch_timer_check_dt_erratum(const struct arch_timer_erratum_workaround *wa,
 	return of_property_read_bool(np, wa->id);
 }
 
+static
+bool arch_timer_check_local_cap_erratum(const struct arch_timer_erratum_workaround *wa,
+					const void *arg)
+{
+	return this_cpu_has_cap((uintptr_t)wa->id);
+}
+
 static const struct arch_timer_erratum_workaround *
 arch_timer_iterate_errata(enum arch_timer_erratum_match_type type,
 			  ate_match_fn_t match_fn,
@@ -244,14 +251,16 @@ static void arch_timer_check_ool_workaround(enum arch_timer_erratum_match_type t
 {
 	const struct arch_timer_erratum_workaround *wa;
 	ate_match_fn_t match_fn = NULL;
-
-	if (static_branch_unlikely(&arch_timer_read_ool_enabled))
-		return;
+	bool local = false;
 
 	switch (type) {
 	case ate_match_dt:
 		match_fn = arch_timer_check_dt_erratum;
 		break;
+	case ate_match_local_cap_id:
+		match_fn = arch_timer_check_local_cap_erratum;
+		local = true;
+		break;
 	default:
 		WARN_ON(1);
 		return;
@@ -261,8 +270,17 @@ static void arch_timer_check_ool_workaround(enum arch_timer_erratum_match_type t
 	if (!wa)
 		return;
 
+	if (needs_unstable_timer_counter_workaround()) {
+		if (wa != timer_unstable_counter_workaround)
+			pr_warn("Can't enable workaround for %s (clashes with %s\n)",
+				wa->desc,
+				timer_unstable_counter_workaround->desc);
+		return;
+	}
+
 	arch_timer_enable_workaround(wa);
-	pr_info("Enabling global workaround for %s\n", wa->desc);
+	pr_info("Enabling %s workaround for %s\n",
+		local ? "local" : "global", wa->desc);
 }
 
 #else
@@ -522,6 +540,8 @@ static void __arch_timer_setup(unsigned type,
 			BUG();
 		}
 
+		arch_timer_check_ool_workaround(ate_match_local_cap_id, NULL);
+
 		erratum_workaround_set_sne(clk);
 	} else {
 		clk->features |= CLOCK_EVT_FEAT_DYNIRQ;
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 59+ messages in thread

* [PATCH v3 08/18] arm64: arch_timer: Move arch_timer_reg_read/write around
  2017-04-04 17:18 ` Marc Zyngier
@ 2017-04-04 17:18   ` Marc Zyngier
  -1 siblings, 0 replies; 59+ messages in thread
From: Marc Zyngier @ 2017-04-04 17:18 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel
  Cc: Mark Rutland, Catalin Marinas, Daniel Lezcano, Will Deacon,
	Scott Wood, Hanjun Guo, Ding Tianhong, dann frazier,
	Thomas Gleixner

As we're about to move things around, let's start with the low
level read/write functions. This allows us to use these functions
in the errata handling code without having to use forward declaration
of static functions.

Acked-by: Mark Rutland <mark.rutland@arm.com>
Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 drivers/clocksource/arm_arch_timer.c | 124 +++++++++++++++++------------------
 1 file changed, 62 insertions(+), 62 deletions(-)

diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index 532e47fa43b3..b5c83cc2a67a 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -96,6 +96,68 @@ early_param("clocksource.arm_arch_timer.evtstrm", early_evtstrm_cfg);
  * Architected system timer support.
  */
 
+static __always_inline
+void arch_timer_reg_write(int access, enum arch_timer_reg reg, u32 val,
+			  struct clock_event_device *clk)
+{
+	if (access == ARCH_TIMER_MEM_PHYS_ACCESS) {
+		struct arch_timer *timer = to_arch_timer(clk);
+		switch (reg) {
+		case ARCH_TIMER_REG_CTRL:
+			writel_relaxed(val, timer->base + CNTP_CTL);
+			break;
+		case ARCH_TIMER_REG_TVAL:
+			writel_relaxed(val, timer->base + CNTP_TVAL);
+			break;
+		}
+	} else if (access == ARCH_TIMER_MEM_VIRT_ACCESS) {
+		struct arch_timer *timer = to_arch_timer(clk);
+		switch (reg) {
+		case ARCH_TIMER_REG_CTRL:
+			writel_relaxed(val, timer->base + CNTV_CTL);
+			break;
+		case ARCH_TIMER_REG_TVAL:
+			writel_relaxed(val, timer->base + CNTV_TVAL);
+			break;
+		}
+	} else {
+		arch_timer_reg_write_cp15(access, reg, val);
+	}
+}
+
+static __always_inline
+u32 arch_timer_reg_read(int access, enum arch_timer_reg reg,
+			struct clock_event_device *clk)
+{
+	u32 val;
+
+	if (access == ARCH_TIMER_MEM_PHYS_ACCESS) {
+		struct arch_timer *timer = to_arch_timer(clk);
+		switch (reg) {
+		case ARCH_TIMER_REG_CTRL:
+			val = readl_relaxed(timer->base + CNTP_CTL);
+			break;
+		case ARCH_TIMER_REG_TVAL:
+			val = readl_relaxed(timer->base + CNTP_TVAL);
+			break;
+		}
+	} else if (access == ARCH_TIMER_MEM_VIRT_ACCESS) {
+		struct arch_timer *timer = to_arch_timer(clk);
+		switch (reg) {
+		case ARCH_TIMER_REG_CTRL:
+			val = readl_relaxed(timer->base + CNTV_CTL);
+			break;
+		case ARCH_TIMER_REG_TVAL:
+			val = readl_relaxed(timer->base + CNTV_TVAL);
+			break;
+		}
+	} else {
+		val = arch_timer_reg_read_cp15(access, reg);
+	}
+
+	return val;
+}
+
 #ifdef CONFIG_FSL_ERRATUM_A008585
 /*
  * The number of retries is an arbitrary value well beyond the highest number
@@ -287,68 +349,6 @@ static void arch_timer_check_ool_workaround(enum arch_timer_erratum_match_type t
 #define arch_timer_check_ool_workaround(t,a)		do { } while(0)
 #endif /* CONFIG_ARM_ARCH_TIMER_OOL_WORKAROUND */
 
-static __always_inline
-void arch_timer_reg_write(int access, enum arch_timer_reg reg, u32 val,
-			  struct clock_event_device *clk)
-{
-	if (access == ARCH_TIMER_MEM_PHYS_ACCESS) {
-		struct arch_timer *timer = to_arch_timer(clk);
-		switch (reg) {
-		case ARCH_TIMER_REG_CTRL:
-			writel_relaxed(val, timer->base + CNTP_CTL);
-			break;
-		case ARCH_TIMER_REG_TVAL:
-			writel_relaxed(val, timer->base + CNTP_TVAL);
-			break;
-		}
-	} else if (access == ARCH_TIMER_MEM_VIRT_ACCESS) {
-		struct arch_timer *timer = to_arch_timer(clk);
-		switch (reg) {
-		case ARCH_TIMER_REG_CTRL:
-			writel_relaxed(val, timer->base + CNTV_CTL);
-			break;
-		case ARCH_TIMER_REG_TVAL:
-			writel_relaxed(val, timer->base + CNTV_TVAL);
-			break;
-		}
-	} else {
-		arch_timer_reg_write_cp15(access, reg, val);
-	}
-}
-
-static __always_inline
-u32 arch_timer_reg_read(int access, enum arch_timer_reg reg,
-			struct clock_event_device *clk)
-{
-	u32 val;
-
-	if (access == ARCH_TIMER_MEM_PHYS_ACCESS) {
-		struct arch_timer *timer = to_arch_timer(clk);
-		switch (reg) {
-		case ARCH_TIMER_REG_CTRL:
-			val = readl_relaxed(timer->base + CNTP_CTL);
-			break;
-		case ARCH_TIMER_REG_TVAL:
-			val = readl_relaxed(timer->base + CNTP_TVAL);
-			break;
-		}
-	} else if (access == ARCH_TIMER_MEM_VIRT_ACCESS) {
-		struct arch_timer *timer = to_arch_timer(clk);
-		switch (reg) {
-		case ARCH_TIMER_REG_CTRL:
-			val = readl_relaxed(timer->base + CNTV_CTL);
-			break;
-		case ARCH_TIMER_REG_TVAL:
-			val = readl_relaxed(timer->base + CNTV_TVAL);
-			break;
-		}
-	} else {
-		val = arch_timer_reg_read_cp15(access, reg);
-	}
-
-	return val;
-}
-
 static __always_inline irqreturn_t timer_handler(const int access,
 					struct clock_event_device *evt)
 {
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 59+ messages in thread

* [PATCH v3 08/18] arm64: arch_timer: Move arch_timer_reg_read/write around
@ 2017-04-04 17:18   ` Marc Zyngier
  0 siblings, 0 replies; 59+ messages in thread
From: Marc Zyngier @ 2017-04-04 17:18 UTC (permalink / raw)
  To: linux-arm-kernel

As we're about to move things around, let's start with the low
level read/write functions. This allows us to use these functions
in the errata handling code without having to use forward declaration
of static functions.

Acked-by: Mark Rutland <mark.rutland@arm.com>
Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 drivers/clocksource/arm_arch_timer.c | 124 +++++++++++++++++------------------
 1 file changed, 62 insertions(+), 62 deletions(-)

diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index 532e47fa43b3..b5c83cc2a67a 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -96,6 +96,68 @@ early_param("clocksource.arm_arch_timer.evtstrm", early_evtstrm_cfg);
  * Architected system timer support.
  */
 
+static __always_inline
+void arch_timer_reg_write(int access, enum arch_timer_reg reg, u32 val,
+			  struct clock_event_device *clk)
+{
+	if (access == ARCH_TIMER_MEM_PHYS_ACCESS) {
+		struct arch_timer *timer = to_arch_timer(clk);
+		switch (reg) {
+		case ARCH_TIMER_REG_CTRL:
+			writel_relaxed(val, timer->base + CNTP_CTL);
+			break;
+		case ARCH_TIMER_REG_TVAL:
+			writel_relaxed(val, timer->base + CNTP_TVAL);
+			break;
+		}
+	} else if (access == ARCH_TIMER_MEM_VIRT_ACCESS) {
+		struct arch_timer *timer = to_arch_timer(clk);
+		switch (reg) {
+		case ARCH_TIMER_REG_CTRL:
+			writel_relaxed(val, timer->base + CNTV_CTL);
+			break;
+		case ARCH_TIMER_REG_TVAL:
+			writel_relaxed(val, timer->base + CNTV_TVAL);
+			break;
+		}
+	} else {
+		arch_timer_reg_write_cp15(access, reg, val);
+	}
+}
+
+static __always_inline
+u32 arch_timer_reg_read(int access, enum arch_timer_reg reg,
+			struct clock_event_device *clk)
+{
+	u32 val;
+
+	if (access == ARCH_TIMER_MEM_PHYS_ACCESS) {
+		struct arch_timer *timer = to_arch_timer(clk);
+		switch (reg) {
+		case ARCH_TIMER_REG_CTRL:
+			val = readl_relaxed(timer->base + CNTP_CTL);
+			break;
+		case ARCH_TIMER_REG_TVAL:
+			val = readl_relaxed(timer->base + CNTP_TVAL);
+			break;
+		}
+	} else if (access == ARCH_TIMER_MEM_VIRT_ACCESS) {
+		struct arch_timer *timer = to_arch_timer(clk);
+		switch (reg) {
+		case ARCH_TIMER_REG_CTRL:
+			val = readl_relaxed(timer->base + CNTV_CTL);
+			break;
+		case ARCH_TIMER_REG_TVAL:
+			val = readl_relaxed(timer->base + CNTV_TVAL);
+			break;
+		}
+	} else {
+		val = arch_timer_reg_read_cp15(access, reg);
+	}
+
+	return val;
+}
+
 #ifdef CONFIG_FSL_ERRATUM_A008585
 /*
  * The number of retries is an arbitrary value well beyond the highest number
@@ -287,68 +349,6 @@ static void arch_timer_check_ool_workaround(enum arch_timer_erratum_match_type t
 #define arch_timer_check_ool_workaround(t,a)		do { } while(0)
 #endif /* CONFIG_ARM_ARCH_TIMER_OOL_WORKAROUND */
 
-static __always_inline
-void arch_timer_reg_write(int access, enum arch_timer_reg reg, u32 val,
-			  struct clock_event_device *clk)
-{
-	if (access == ARCH_TIMER_MEM_PHYS_ACCESS) {
-		struct arch_timer *timer = to_arch_timer(clk);
-		switch (reg) {
-		case ARCH_TIMER_REG_CTRL:
-			writel_relaxed(val, timer->base + CNTP_CTL);
-			break;
-		case ARCH_TIMER_REG_TVAL:
-			writel_relaxed(val, timer->base + CNTP_TVAL);
-			break;
-		}
-	} else if (access == ARCH_TIMER_MEM_VIRT_ACCESS) {
-		struct arch_timer *timer = to_arch_timer(clk);
-		switch (reg) {
-		case ARCH_TIMER_REG_CTRL:
-			writel_relaxed(val, timer->base + CNTV_CTL);
-			break;
-		case ARCH_TIMER_REG_TVAL:
-			writel_relaxed(val, timer->base + CNTV_TVAL);
-			break;
-		}
-	} else {
-		arch_timer_reg_write_cp15(access, reg, val);
-	}
-}
-
-static __always_inline
-u32 arch_timer_reg_read(int access, enum arch_timer_reg reg,
-			struct clock_event_device *clk)
-{
-	u32 val;
-
-	if (access == ARCH_TIMER_MEM_PHYS_ACCESS) {
-		struct arch_timer *timer = to_arch_timer(clk);
-		switch (reg) {
-		case ARCH_TIMER_REG_CTRL:
-			val = readl_relaxed(timer->base + CNTP_CTL);
-			break;
-		case ARCH_TIMER_REG_TVAL:
-			val = readl_relaxed(timer->base + CNTP_TVAL);
-			break;
-		}
-	} else if (access == ARCH_TIMER_MEM_VIRT_ACCESS) {
-		struct arch_timer *timer = to_arch_timer(clk);
-		switch (reg) {
-		case ARCH_TIMER_REG_CTRL:
-			val = readl_relaxed(timer->base + CNTV_CTL);
-			break;
-		case ARCH_TIMER_REG_TVAL:
-			val = readl_relaxed(timer->base + CNTV_TVAL);
-			break;
-		}
-	} else {
-		val = arch_timer_reg_read_cp15(access, reg);
-	}
-
-	return val;
-}
-
 static __always_inline irqreturn_t timer_handler(const int access,
 					struct clock_event_device *evt)
 {
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 59+ messages in thread

* [PATCH v3 09/18] arm64: arch_timer: Get rid of erratum_workaround_set_sne
  2017-04-04 17:18 ` Marc Zyngier
@ 2017-04-04 17:18   ` Marc Zyngier
  -1 siblings, 0 replies; 59+ messages in thread
From: Marc Zyngier @ 2017-04-04 17:18 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel
  Cc: Mark Rutland, Catalin Marinas, Daniel Lezcano, Will Deacon,
	Scott Wood, Hanjun Guo, Ding Tianhong, dann frazier,
	Thomas Gleixner

Let's move the handling of workarounds affecting set_next_event
to the affected function, instead of overriding the pointers
as an afterthough. Yes, this is an extra indirection on the
erratum handling path, but the HW is busted anyway.

This will allow for some more flexibility later.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 drivers/clocksource/arm_arch_timer.c | 90 ++++++++++++++++--------------------
 1 file changed, 41 insertions(+), 49 deletions(-)

diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index b5c83cc2a67a..2c02e25d1475 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -241,6 +241,38 @@ EXPORT_SYMBOL_GPL(timer_unstable_counter_workaround);
 DEFINE_STATIC_KEY_FALSE(arch_timer_read_ool_enabled);
 EXPORT_SYMBOL_GPL(arch_timer_read_ool_enabled);
 
+static void erratum_set_next_event_tval_generic(const int access, unsigned long evt,
+						struct clock_event_device *clk)
+{
+	unsigned long ctrl;
+	u64 cval = evt + arch_counter_get_cntvct();
+
+	ctrl = arch_timer_reg_read(access, ARCH_TIMER_REG_CTRL, clk);
+	ctrl |= ARCH_TIMER_CTRL_ENABLE;
+	ctrl &= ~ARCH_TIMER_CTRL_IT_MASK;
+
+	if (access == ARCH_TIMER_PHYS_ACCESS)
+		write_sysreg(cval, cntp_cval_el0);
+	else
+		write_sysreg(cval, cntv_cval_el0);
+
+	arch_timer_reg_write(access, ARCH_TIMER_REG_CTRL, ctrl, clk);
+}
+
+static int erratum_set_next_event_tval_virt(unsigned long evt,
+					    struct clock_event_device *clk)
+{
+	erratum_set_next_event_tval_generic(ARCH_TIMER_VIRT_ACCESS, evt, clk);
+	return 0;
+}
+
+static int erratum_set_next_event_tval_phys(unsigned long evt,
+					    struct clock_event_device *clk)
+{
+	erratum_set_next_event_tval_generic(ARCH_TIMER_PHYS_ACCESS, evt, clk);
+	return 0;
+}
+
 static const struct arch_timer_erratum_workaround ool_workarounds[] = {
 #ifdef CONFIG_FSL_ERRATUM_A008585
 	{
@@ -347,6 +379,9 @@ static void arch_timer_check_ool_workaround(enum arch_timer_erratum_match_type t
 
 #else
 #define arch_timer_check_ool_workaround(t,a)		do { } while(0)
+#define erratum_set_next_event_tval_virt(...)		({BUG(); 0;})
+#define erratum_set_next_event_tval_phys(...)		({BUG(); 0;})
+#define needs_unstable_timer_counter_workaround()	({false;})
 #endif /* CONFIG_ARM_ARCH_TIMER_OOL_WORKAROUND */
 
 static __always_inline irqreturn_t timer_handler(const int access,
@@ -436,43 +471,12 @@ static __always_inline void set_next_event(const int access, unsigned long evt,
 	arch_timer_reg_write(access, ARCH_TIMER_REG_CTRL, ctrl, clk);
 }
 
-#ifdef CONFIG_ARM_ARCH_TIMER_OOL_WORKAROUND
-static __always_inline void erratum_set_next_event_generic(const int access,
-		unsigned long evt, struct clock_event_device *clk)
-{
-	unsigned long ctrl;
-	u64 cval = evt + arch_counter_get_cntvct();
-
-	ctrl = arch_timer_reg_read(access, ARCH_TIMER_REG_CTRL, clk);
-	ctrl |= ARCH_TIMER_CTRL_ENABLE;
-	ctrl &= ~ARCH_TIMER_CTRL_IT_MASK;
-
-	if (access == ARCH_TIMER_PHYS_ACCESS)
-		write_sysreg(cval, cntp_cval_el0);
-	else if (access == ARCH_TIMER_VIRT_ACCESS)
-		write_sysreg(cval, cntv_cval_el0);
-
-	arch_timer_reg_write(access, ARCH_TIMER_REG_CTRL, ctrl, clk);
-}
-
-static int erratum_set_next_event_virt(unsigned long evt,
-					   struct clock_event_device *clk)
-{
-	erratum_set_next_event_generic(ARCH_TIMER_VIRT_ACCESS, evt, clk);
-	return 0;
-}
-
-static int erratum_set_next_event_phys(unsigned long evt,
-					   struct clock_event_device *clk)
-{
-	erratum_set_next_event_generic(ARCH_TIMER_PHYS_ACCESS, evt, clk);
-	return 0;
-}
-#endif /* CONFIG_ARM_ARCH_TIMER_OOL_WORKAROUND */
-
 static int arch_timer_set_next_event_virt(unsigned long evt,
 					  struct clock_event_device *clk)
 {
+	if (needs_unstable_timer_counter_workaround())
+		return erratum_set_next_event_tval_virt(evt, clk);
+
 	set_next_event(ARCH_TIMER_VIRT_ACCESS, evt, clk);
 	return 0;
 }
@@ -480,6 +484,9 @@ static int arch_timer_set_next_event_virt(unsigned long evt,
 static int arch_timer_set_next_event_phys(unsigned long evt,
 					  struct clock_event_device *clk)
 {
+	if (needs_unstable_timer_counter_workaround())
+		return erratum_set_next_event_tval_phys(evt, clk);
+
 	set_next_event(ARCH_TIMER_PHYS_ACCESS, evt, clk);
 	return 0;
 }
@@ -498,19 +505,6 @@ static int arch_timer_set_next_event_phys_mem(unsigned long evt,
 	return 0;
 }
 
-static void erratum_workaround_set_sne(struct clock_event_device *clk)
-{
-#ifdef CONFIG_ARM_ARCH_TIMER_OOL_WORKAROUND
-	if (!static_branch_unlikely(&arch_timer_read_ool_enabled))
-		return;
-
-	if (arch_timer_uses_ppi == VIRT_PPI)
-		clk->set_next_event = erratum_set_next_event_virt;
-	else
-		clk->set_next_event = erratum_set_next_event_phys;
-#endif
-}
-
 static void __arch_timer_setup(unsigned type,
 			       struct clock_event_device *clk)
 {
@@ -541,8 +535,6 @@ static void __arch_timer_setup(unsigned type,
 		}
 
 		arch_timer_check_ool_workaround(ate_match_local_cap_id, NULL);
-
-		erratum_workaround_set_sne(clk);
 	} else {
 		clk->features |= CLOCK_EVT_FEAT_DYNIRQ;
 		clk->name = "arch_mem_timer";
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 59+ messages in thread

* [PATCH v3 09/18] arm64: arch_timer: Get rid of erratum_workaround_set_sne
@ 2017-04-04 17:18   ` Marc Zyngier
  0 siblings, 0 replies; 59+ messages in thread
From: Marc Zyngier @ 2017-04-04 17:18 UTC (permalink / raw)
  To: linux-arm-kernel

Let's move the handling of workarounds affecting set_next_event
to the affected function, instead of overriding the pointers
as an afterthough. Yes, this is an extra indirection on the
erratum handling path, but the HW is busted anyway.

This will allow for some more flexibility later.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 drivers/clocksource/arm_arch_timer.c | 90 ++++++++++++++++--------------------
 1 file changed, 41 insertions(+), 49 deletions(-)

diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index b5c83cc2a67a..2c02e25d1475 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -241,6 +241,38 @@ EXPORT_SYMBOL_GPL(timer_unstable_counter_workaround);
 DEFINE_STATIC_KEY_FALSE(arch_timer_read_ool_enabled);
 EXPORT_SYMBOL_GPL(arch_timer_read_ool_enabled);
 
+static void erratum_set_next_event_tval_generic(const int access, unsigned long evt,
+						struct clock_event_device *clk)
+{
+	unsigned long ctrl;
+	u64 cval = evt + arch_counter_get_cntvct();
+
+	ctrl = arch_timer_reg_read(access, ARCH_TIMER_REG_CTRL, clk);
+	ctrl |= ARCH_TIMER_CTRL_ENABLE;
+	ctrl &= ~ARCH_TIMER_CTRL_IT_MASK;
+
+	if (access == ARCH_TIMER_PHYS_ACCESS)
+		write_sysreg(cval, cntp_cval_el0);
+	else
+		write_sysreg(cval, cntv_cval_el0);
+
+	arch_timer_reg_write(access, ARCH_TIMER_REG_CTRL, ctrl, clk);
+}
+
+static int erratum_set_next_event_tval_virt(unsigned long evt,
+					    struct clock_event_device *clk)
+{
+	erratum_set_next_event_tval_generic(ARCH_TIMER_VIRT_ACCESS, evt, clk);
+	return 0;
+}
+
+static int erratum_set_next_event_tval_phys(unsigned long evt,
+					    struct clock_event_device *clk)
+{
+	erratum_set_next_event_tval_generic(ARCH_TIMER_PHYS_ACCESS, evt, clk);
+	return 0;
+}
+
 static const struct arch_timer_erratum_workaround ool_workarounds[] = {
 #ifdef CONFIG_FSL_ERRATUM_A008585
 	{
@@ -347,6 +379,9 @@ static void arch_timer_check_ool_workaround(enum arch_timer_erratum_match_type t
 
 #else
 #define arch_timer_check_ool_workaround(t,a)		do { } while(0)
+#define erratum_set_next_event_tval_virt(...)		({BUG(); 0;})
+#define erratum_set_next_event_tval_phys(...)		({BUG(); 0;})
+#define needs_unstable_timer_counter_workaround()	({false;})
 #endif /* CONFIG_ARM_ARCH_TIMER_OOL_WORKAROUND */
 
 static __always_inline irqreturn_t timer_handler(const int access,
@@ -436,43 +471,12 @@ static __always_inline void set_next_event(const int access, unsigned long evt,
 	arch_timer_reg_write(access, ARCH_TIMER_REG_CTRL, ctrl, clk);
 }
 
-#ifdef CONFIG_ARM_ARCH_TIMER_OOL_WORKAROUND
-static __always_inline void erratum_set_next_event_generic(const int access,
-		unsigned long evt, struct clock_event_device *clk)
-{
-	unsigned long ctrl;
-	u64 cval = evt + arch_counter_get_cntvct();
-
-	ctrl = arch_timer_reg_read(access, ARCH_TIMER_REG_CTRL, clk);
-	ctrl |= ARCH_TIMER_CTRL_ENABLE;
-	ctrl &= ~ARCH_TIMER_CTRL_IT_MASK;
-
-	if (access == ARCH_TIMER_PHYS_ACCESS)
-		write_sysreg(cval, cntp_cval_el0);
-	else if (access == ARCH_TIMER_VIRT_ACCESS)
-		write_sysreg(cval, cntv_cval_el0);
-
-	arch_timer_reg_write(access, ARCH_TIMER_REG_CTRL, ctrl, clk);
-}
-
-static int erratum_set_next_event_virt(unsigned long evt,
-					   struct clock_event_device *clk)
-{
-	erratum_set_next_event_generic(ARCH_TIMER_VIRT_ACCESS, evt, clk);
-	return 0;
-}
-
-static int erratum_set_next_event_phys(unsigned long evt,
-					   struct clock_event_device *clk)
-{
-	erratum_set_next_event_generic(ARCH_TIMER_PHYS_ACCESS, evt, clk);
-	return 0;
-}
-#endif /* CONFIG_ARM_ARCH_TIMER_OOL_WORKAROUND */
-
 static int arch_timer_set_next_event_virt(unsigned long evt,
 					  struct clock_event_device *clk)
 {
+	if (needs_unstable_timer_counter_workaround())
+		return erratum_set_next_event_tval_virt(evt, clk);
+
 	set_next_event(ARCH_TIMER_VIRT_ACCESS, evt, clk);
 	return 0;
 }
@@ -480,6 +484,9 @@ static int arch_timer_set_next_event_virt(unsigned long evt,
 static int arch_timer_set_next_event_phys(unsigned long evt,
 					  struct clock_event_device *clk)
 {
+	if (needs_unstable_timer_counter_workaround())
+		return erratum_set_next_event_tval_phys(evt, clk);
+
 	set_next_event(ARCH_TIMER_PHYS_ACCESS, evt, clk);
 	return 0;
 }
@@ -498,19 +505,6 @@ static int arch_timer_set_next_event_phys_mem(unsigned long evt,
 	return 0;
 }
 
-static void erratum_workaround_set_sne(struct clock_event_device *clk)
-{
-#ifdef CONFIG_ARM_ARCH_TIMER_OOL_WORKAROUND
-	if (!static_branch_unlikely(&arch_timer_read_ool_enabled))
-		return;
-
-	if (arch_timer_uses_ppi == VIRT_PPI)
-		clk->set_next_event = erratum_set_next_event_virt;
-	else
-		clk->set_next_event = erratum_set_next_event_phys;
-#endif
-}
-
 static void __arch_timer_setup(unsigned type,
 			       struct clock_event_device *clk)
 {
@@ -541,8 +535,6 @@ static void __arch_timer_setup(unsigned type,
 		}
 
 		arch_timer_check_ool_workaround(ate_match_local_cap_id, NULL);
-
-		erratum_workaround_set_sne(clk);
 	} else {
 		clk->features |= CLOCK_EVT_FEAT_DYNIRQ;
 		clk->name = "arch_mem_timer";
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 59+ messages in thread

* [PATCH v3 10/18] arm64: arch_timer: Rework the set_next_event workarounds
  2017-04-04 17:18 ` Marc Zyngier
@ 2017-04-04 17:18   ` Marc Zyngier
  -1 siblings, 0 replies; 59+ messages in thread
From: Marc Zyngier @ 2017-04-04 17:18 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel
  Cc: Mark Rutland, Catalin Marinas, Daniel Lezcano, Will Deacon,
	Scott Wood, Hanjun Guo, Ding Tianhong, dann frazier,
	Thomas Gleixner

The way we work around errata affecting set_next_event is not very
nice, at it imposes this workaround on errata that do not need it.

Add new workaround hooks and let the existing workarounds use them.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm64/include/asm/arch_timer.h  |  4 ++++
 drivers/clocksource/arm_arch_timer.c | 31 ++++++++++++++++++++++++++-----
 2 files changed, 30 insertions(+), 5 deletions(-)

diff --git a/arch/arm64/include/asm/arch_timer.h b/arch/arm64/include/asm/arch_timer.h
index 1b0d7e994e0c..cc1e08127fb4 100644
--- a/arch/arm64/include/asm/arch_timer.h
+++ b/arch/arm64/include/asm/arch_timer.h
@@ -42,6 +42,8 @@ enum arch_timer_erratum_match_type {
 	ate_match_local_cap_id,
 };
 
+struct clock_event_device;
+
 struct arch_timer_erratum_workaround {
 	enum arch_timer_erratum_match_type match_type;
 	const void *id;
@@ -49,6 +51,8 @@ struct arch_timer_erratum_workaround {
 	u32 (*read_cntp_tval_el0)(void);
 	u32 (*read_cntv_tval_el0)(void);
 	u64 (*read_cntvct_el0)(void);
+	int (*set_next_event_phys)(unsigned long, struct clock_event_device *);
+	int (*set_next_event_virt)(unsigned long, struct clock_event_device *);
 };
 
 extern const struct arch_timer_erratum_workaround *timer_unstable_counter_workaround;
diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index 2c02e25d1475..a0c9ee80147e 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -282,6 +282,8 @@ static const struct arch_timer_erratum_workaround ool_workarounds[] = {
 		.read_cntp_tval_el0 = fsl_a008585_read_cntp_tval_el0,
 		.read_cntv_tval_el0 = fsl_a008585_read_cntv_tval_el0,
 		.read_cntvct_el0 = fsl_a008585_read_cntvct_el0,
+		.set_next_event_phys = erratum_set_next_event_tval_phys,
+		.set_next_event_virt = erratum_set_next_event_tval_virt,
 	},
 #endif
 #ifdef CONFIG_HISILICON_ERRATUM_161010101
@@ -292,6 +294,8 @@ static const struct arch_timer_erratum_workaround ool_workarounds[] = {
 		.read_cntp_tval_el0 = hisi_161010101_read_cntp_tval_el0,
 		.read_cntv_tval_el0 = hisi_161010101_read_cntv_tval_el0,
 		.read_cntvct_el0 = hisi_161010101_read_cntvct_el0,
+		.set_next_event_phys = erratum_set_next_event_tval_phys,
+		.set_next_event_virt = erratum_set_next_event_tval_virt,
 	},
 #endif
 };
@@ -377,11 +381,24 @@ static void arch_timer_check_ool_workaround(enum arch_timer_erratum_match_type t
 		local ? "local" : "global", wa->desc);
 }
 
+#define erratum_handler(fn, r, ...)					\
+({									\
+	bool __val;							\
+	if (needs_unstable_timer_counter_workaround() &&		\
+	    timer_unstable_counter_workaround->fn) {			\
+		r = timer_unstable_counter_workaround->fn(__VA_ARGS__);	\
+		__val = true;						\
+	} else {							\
+		__val = false;						\
+	}								\
+	__val;								\
+})
+
 #else
 #define arch_timer_check_ool_workaround(t,a)		do { } while(0)
 #define erratum_set_next_event_tval_virt(...)		({BUG(); 0;})
 #define erratum_set_next_event_tval_phys(...)		({BUG(); 0;})
-#define needs_unstable_timer_counter_workaround()	({false;})
+#define erratum_handler(fn, r, ...)			({false;})
 #endif /* CONFIG_ARM_ARCH_TIMER_OOL_WORKAROUND */
 
 static __always_inline irqreturn_t timer_handler(const int access,
@@ -474,8 +491,10 @@ static __always_inline void set_next_event(const int access, unsigned long evt,
 static int arch_timer_set_next_event_virt(unsigned long evt,
 					  struct clock_event_device *clk)
 {
-	if (needs_unstable_timer_counter_workaround())
-		return erratum_set_next_event_tval_virt(evt, clk);
+	int ret;
+
+	if (erratum_handler(set_next_event_virt, ret, evt, clk))
+		return ret;
 
 	set_next_event(ARCH_TIMER_VIRT_ACCESS, evt, clk);
 	return 0;
@@ -484,8 +503,10 @@ static int arch_timer_set_next_event_virt(unsigned long evt,
 static int arch_timer_set_next_event_phys(unsigned long evt,
 					  struct clock_event_device *clk)
 {
-	if (needs_unstable_timer_counter_workaround())
-		return erratum_set_next_event_tval_phys(evt, clk);
+	int ret;
+
+	if (erratum_handler(set_next_event_phys, ret, evt, clk))
+		return ret;
 
 	set_next_event(ARCH_TIMER_PHYS_ACCESS, evt, clk);
 	return 0;
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 59+ messages in thread

* [PATCH v3 10/18] arm64: arch_timer: Rework the set_next_event workarounds
@ 2017-04-04 17:18   ` Marc Zyngier
  0 siblings, 0 replies; 59+ messages in thread
From: Marc Zyngier @ 2017-04-04 17:18 UTC (permalink / raw)
  To: linux-arm-kernel

The way we work around errata affecting set_next_event is not very
nice, at it imposes this workaround on errata that do not need it.

Add new workaround hooks and let the existing workarounds use them.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm64/include/asm/arch_timer.h  |  4 ++++
 drivers/clocksource/arm_arch_timer.c | 31 ++++++++++++++++++++++++++-----
 2 files changed, 30 insertions(+), 5 deletions(-)

diff --git a/arch/arm64/include/asm/arch_timer.h b/arch/arm64/include/asm/arch_timer.h
index 1b0d7e994e0c..cc1e08127fb4 100644
--- a/arch/arm64/include/asm/arch_timer.h
+++ b/arch/arm64/include/asm/arch_timer.h
@@ -42,6 +42,8 @@ enum arch_timer_erratum_match_type {
 	ate_match_local_cap_id,
 };
 
+struct clock_event_device;
+
 struct arch_timer_erratum_workaround {
 	enum arch_timer_erratum_match_type match_type;
 	const void *id;
@@ -49,6 +51,8 @@ struct arch_timer_erratum_workaround {
 	u32 (*read_cntp_tval_el0)(void);
 	u32 (*read_cntv_tval_el0)(void);
 	u64 (*read_cntvct_el0)(void);
+	int (*set_next_event_phys)(unsigned long, struct clock_event_device *);
+	int (*set_next_event_virt)(unsigned long, struct clock_event_device *);
 };
 
 extern const struct arch_timer_erratum_workaround *timer_unstable_counter_workaround;
diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index 2c02e25d1475..a0c9ee80147e 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -282,6 +282,8 @@ static const struct arch_timer_erratum_workaround ool_workarounds[] = {
 		.read_cntp_tval_el0 = fsl_a008585_read_cntp_tval_el0,
 		.read_cntv_tval_el0 = fsl_a008585_read_cntv_tval_el0,
 		.read_cntvct_el0 = fsl_a008585_read_cntvct_el0,
+		.set_next_event_phys = erratum_set_next_event_tval_phys,
+		.set_next_event_virt = erratum_set_next_event_tval_virt,
 	},
 #endif
 #ifdef CONFIG_HISILICON_ERRATUM_161010101
@@ -292,6 +294,8 @@ static const struct arch_timer_erratum_workaround ool_workarounds[] = {
 		.read_cntp_tval_el0 = hisi_161010101_read_cntp_tval_el0,
 		.read_cntv_tval_el0 = hisi_161010101_read_cntv_tval_el0,
 		.read_cntvct_el0 = hisi_161010101_read_cntvct_el0,
+		.set_next_event_phys = erratum_set_next_event_tval_phys,
+		.set_next_event_virt = erratum_set_next_event_tval_virt,
 	},
 #endif
 };
@@ -377,11 +381,24 @@ static void arch_timer_check_ool_workaround(enum arch_timer_erratum_match_type t
 		local ? "local" : "global", wa->desc);
 }
 
+#define erratum_handler(fn, r, ...)					\
+({									\
+	bool __val;							\
+	if (needs_unstable_timer_counter_workaround() &&		\
+	    timer_unstable_counter_workaround->fn) {			\
+		r = timer_unstable_counter_workaround->fn(__VA_ARGS__);	\
+		__val = true;						\
+	} else {							\
+		__val = false;						\
+	}								\
+	__val;								\
+})
+
 #else
 #define arch_timer_check_ool_workaround(t,a)		do { } while(0)
 #define erratum_set_next_event_tval_virt(...)		({BUG(); 0;})
 #define erratum_set_next_event_tval_phys(...)		({BUG(); 0;})
-#define needs_unstable_timer_counter_workaround()	({false;})
+#define erratum_handler(fn, r, ...)			({false;})
 #endif /* CONFIG_ARM_ARCH_TIMER_OOL_WORKAROUND */
 
 static __always_inline irqreturn_t timer_handler(const int access,
@@ -474,8 +491,10 @@ static __always_inline void set_next_event(const int access, unsigned long evt,
 static int arch_timer_set_next_event_virt(unsigned long evt,
 					  struct clock_event_device *clk)
 {
-	if (needs_unstable_timer_counter_workaround())
-		return erratum_set_next_event_tval_virt(evt, clk);
+	int ret;
+
+	if (erratum_handler(set_next_event_virt, ret, evt, clk))
+		return ret;
 
 	set_next_event(ARCH_TIMER_VIRT_ACCESS, evt, clk);
 	return 0;
@@ -484,8 +503,10 @@ static int arch_timer_set_next_event_virt(unsigned long evt,
 static int arch_timer_set_next_event_phys(unsigned long evt,
 					  struct clock_event_device *clk)
 {
-	if (needs_unstable_timer_counter_workaround())
-		return erratum_set_next_event_tval_phys(evt, clk);
+	int ret;
+
+	if (erratum_handler(set_next_event_phys, ret, evt, clk))
+		return ret;
 
 	set_next_event(ARCH_TIMER_PHYS_ACCESS, evt, clk);
 	return 0;
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 59+ messages in thread

* [PATCH v3 11/18] arm64: arch_timer: Make workaround methods optional
  2017-04-04 17:18 ` Marc Zyngier
@ 2017-04-04 17:18   ` Marc Zyngier
  -1 siblings, 0 replies; 59+ messages in thread
From: Marc Zyngier @ 2017-04-04 17:18 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel
  Cc: Mark Rutland, Catalin Marinas, Daniel Lezcano, Will Deacon,
	Scott Wood, Hanjun Guo, Ding Tianhong, dann frazier,
	Thomas Gleixner

Not all errata need to workaround all access types. Allow them to
be optional.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm64/include/asm/arch_timer.h | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/include/asm/arch_timer.h b/arch/arm64/include/asm/arch_timer.h
index cc1e08127fb4..01917b4c65ca 100644
--- a/arch/arm64/include/asm/arch_timer.h
+++ b/arch/arm64/include/asm/arch_timer.h
@@ -60,8 +60,9 @@ extern const struct arch_timer_erratum_workaround *timer_unstable_counter_workar
 #define arch_timer_reg_read_stable(reg) 		\
 ({							\
 	u64 _val;					\
-	if (needs_unstable_timer_counter_workaround())		\
-		_val = timer_unstable_counter_workaround->read_##reg();\
+	if (needs_unstable_timer_counter_workaround() &&		\
+	    timer_unstable_counter_workaround->read_##reg)		\
+		_val = timer_unstable_counter_workaround->read_##reg();	\
 	else						\
 		_val = read_sysreg(reg);		\
 	_val;						\
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 59+ messages in thread

* [PATCH v3 11/18] arm64: arch_timer: Make workaround methods optional
@ 2017-04-04 17:18   ` Marc Zyngier
  0 siblings, 0 replies; 59+ messages in thread
From: Marc Zyngier @ 2017-04-04 17:18 UTC (permalink / raw)
  To: linux-arm-kernel

Not all errata need to workaround all access types. Allow them to
be optional.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm64/include/asm/arch_timer.h | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/include/asm/arch_timer.h b/arch/arm64/include/asm/arch_timer.h
index cc1e08127fb4..01917b4c65ca 100644
--- a/arch/arm64/include/asm/arch_timer.h
+++ b/arch/arm64/include/asm/arch_timer.h
@@ -60,8 +60,9 @@ extern const struct arch_timer_erratum_workaround *timer_unstable_counter_workar
 #define arch_timer_reg_read_stable(reg) 		\
 ({							\
 	u64 _val;					\
-	if (needs_unstable_timer_counter_workaround())		\
-		_val = timer_unstable_counter_workaround->read_##reg();\
+	if (needs_unstable_timer_counter_workaround() &&		\
+	    timer_unstable_counter_workaround->read_##reg)		\
+		_val = timer_unstable_counter_workaround->read_##reg();	\
 	else						\
 		_val = read_sysreg(reg);		\
 	_val;						\
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 59+ messages in thread

* [PATCH v3 12/18] arm64: arch_timer: Allows a CPU-specific erratum to only affect a subset of CPUs
  2017-04-04 17:18 ` Marc Zyngier
@ 2017-04-04 17:18   ` Marc Zyngier
  -1 siblings, 0 replies; 59+ messages in thread
From: Marc Zyngier @ 2017-04-04 17:18 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel
  Cc: Mark Rutland, Catalin Marinas, Daniel Lezcano, Will Deacon,
	Scott Wood, Hanjun Guo, Ding Tianhong, dann frazier,
	Thomas Gleixner

Instead of applying a CPU-specific workaround to all CPUs in the system,
allow it to only affect a subset of them (typical big-little case).

This is done by turning the erratum pointer into a per-CPU variable.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm64/include/asm/arch_timer.h  | 31 ++++++++++++++++----------
 drivers/clocksource/arm_arch_timer.c | 42 +++++++++++++++++++++++++-----------
 2 files changed, 50 insertions(+), 23 deletions(-)

diff --git a/arch/arm64/include/asm/arch_timer.h b/arch/arm64/include/asm/arch_timer.h
index 01917b4c65ca..6bd1a9a1573a 100644
--- a/arch/arm64/include/asm/arch_timer.h
+++ b/arch/arm64/include/asm/arch_timer.h
@@ -25,6 +25,7 @@
 #include <linux/bug.h>
 #include <linux/init.h>
 #include <linux/jump_label.h>
+#include <linux/smp.h>
 #include <linux/types.h>
 
 #include <clocksource/arm_arch_timer.h>
@@ -55,17 +56,25 @@ struct arch_timer_erratum_workaround {
 	int (*set_next_event_virt)(unsigned long, struct clock_event_device *);
 };
 
-extern const struct arch_timer_erratum_workaround *timer_unstable_counter_workaround;
-
-#define arch_timer_reg_read_stable(reg) 		\
-({							\
-	u64 _val;					\
-	if (needs_unstable_timer_counter_workaround() &&		\
-	    timer_unstable_counter_workaround->read_##reg)		\
-		_val = timer_unstable_counter_workaround->read_##reg();	\
-	else						\
-		_val = read_sysreg(reg);		\
-	_val;						\
+DECLARE_PER_CPU(const struct arch_timer_erratum_workaround *,
+		timer_unstable_counter_workaround);
+
+#define arch_timer_reg_read_stable(reg)					\
+({									\
+	u64 _val;							\
+	if (needs_unstable_timer_counter_workaround()) {		\
+		const struct arch_timer_erratum_workaround *wa;		\
+		preempt_disable();					\
+		wa = __this_cpu_read(timer_unstable_counter_workaround); \
+		if (wa && wa->read_##reg)				\
+			_val = wa->read_##reg();			\
+		else							\
+			_val = read_sysreg(reg);			\
+		preempt_enable();					\
+	} else {							\
+		_val = read_sysreg(reg);				\
+	}								\
+	_val;								\
 })
 
 /*
diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index a0c9ee80147e..4551587bcb44 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -235,7 +235,8 @@ static u64 notrace hisi_161010101_read_cntvct_el0(void)
 #endif
 
 #ifdef CONFIG_ARM_ARCH_TIMER_OOL_WORKAROUND
-const struct arch_timer_erratum_workaround *timer_unstable_counter_workaround = NULL;
+DEFINE_PER_CPU(const struct arch_timer_erratum_workaround *,
+	       timer_unstable_counter_workaround);
 EXPORT_SYMBOL_GPL(timer_unstable_counter_workaround);
 
 DEFINE_STATIC_KEY_FALSE(arch_timer_read_ool_enabled);
@@ -338,9 +339,18 @@ arch_timer_iterate_errata(enum arch_timer_erratum_match_type type,
 }
 
 static
-void arch_timer_enable_workaround(const struct arch_timer_erratum_workaround *wa)
+void arch_timer_enable_workaround(const struct arch_timer_erratum_workaround *wa,
+				  bool local)
 {
-	timer_unstable_counter_workaround = wa;
+	int i;
+
+	if (local) {
+		__this_cpu_write(timer_unstable_counter_workaround, wa);
+	} else {
+		for_each_possible_cpu(i)
+			per_cpu(timer_unstable_counter_workaround, i) = wa;
+	}
+
 	static_branch_enable(&arch_timer_read_ool_enabled);
 }
 
@@ -369,14 +379,17 @@ static void arch_timer_check_ool_workaround(enum arch_timer_erratum_match_type t
 		return;
 
 	if (needs_unstable_timer_counter_workaround()) {
-		if (wa != timer_unstable_counter_workaround)
+		const struct arch_timer_erratum_workaround *__wa;
+		__wa = __this_cpu_read(timer_unstable_counter_workaround);
+		if (__wa && wa != __wa)
 			pr_warn("Can't enable workaround for %s (clashes with %s\n)",
-				wa->desc,
-				timer_unstable_counter_workaround->desc);
-		return;
+				wa->desc, __wa->desc);
+
+		if (__wa)
+			return;
 	}
 
-	arch_timer_enable_workaround(wa);
+	arch_timer_enable_workaround(wa, local);
 	pr_info("Enabling %s workaround for %s\n",
 		local ? "local" : "global", wa->desc);
 }
@@ -384,10 +397,15 @@ static void arch_timer_check_ool_workaround(enum arch_timer_erratum_match_type t
 #define erratum_handler(fn, r, ...)					\
 ({									\
 	bool __val;							\
-	if (needs_unstable_timer_counter_workaround() &&		\
-	    timer_unstable_counter_workaround->fn) {			\
-		r = timer_unstable_counter_workaround->fn(__VA_ARGS__);	\
-		__val = true;						\
+	if (needs_unstable_timer_counter_workaround()) {		\
+		const struct arch_timer_erratum_workaround *__wa;	\
+		__wa = __this_cpu_read(timer_unstable_counter_workaround); \
+		if (__wa && __wa->fn) {					\
+			r = __wa->fn(__VA_ARGS__);			\
+			__val = true;					\
+		} else {						\
+			__val = false;					\
+		}							\
 	} else {							\
 		__val = false;						\
 	}								\
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 59+ messages in thread

* [PATCH v3 12/18] arm64: arch_timer: Allows a CPU-specific erratum to only affect a subset of CPUs
@ 2017-04-04 17:18   ` Marc Zyngier
  0 siblings, 0 replies; 59+ messages in thread
From: Marc Zyngier @ 2017-04-04 17:18 UTC (permalink / raw)
  To: linux-arm-kernel

Instead of applying a CPU-specific workaround to all CPUs in the system,
allow it to only affect a subset of them (typical big-little case).

This is done by turning the erratum pointer into a per-CPU variable.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm64/include/asm/arch_timer.h  | 31 ++++++++++++++++----------
 drivers/clocksource/arm_arch_timer.c | 42 +++++++++++++++++++++++++-----------
 2 files changed, 50 insertions(+), 23 deletions(-)

diff --git a/arch/arm64/include/asm/arch_timer.h b/arch/arm64/include/asm/arch_timer.h
index 01917b4c65ca..6bd1a9a1573a 100644
--- a/arch/arm64/include/asm/arch_timer.h
+++ b/arch/arm64/include/asm/arch_timer.h
@@ -25,6 +25,7 @@
 #include <linux/bug.h>
 #include <linux/init.h>
 #include <linux/jump_label.h>
+#include <linux/smp.h>
 #include <linux/types.h>
 
 #include <clocksource/arm_arch_timer.h>
@@ -55,17 +56,25 @@ struct arch_timer_erratum_workaround {
 	int (*set_next_event_virt)(unsigned long, struct clock_event_device *);
 };
 
-extern const struct arch_timer_erratum_workaround *timer_unstable_counter_workaround;
-
-#define arch_timer_reg_read_stable(reg) 		\
-({							\
-	u64 _val;					\
-	if (needs_unstable_timer_counter_workaround() &&		\
-	    timer_unstable_counter_workaround->read_##reg)		\
-		_val = timer_unstable_counter_workaround->read_##reg();	\
-	else						\
-		_val = read_sysreg(reg);		\
-	_val;						\
+DECLARE_PER_CPU(const struct arch_timer_erratum_workaround *,
+		timer_unstable_counter_workaround);
+
+#define arch_timer_reg_read_stable(reg)					\
+({									\
+	u64 _val;							\
+	if (needs_unstable_timer_counter_workaround()) {		\
+		const struct arch_timer_erratum_workaround *wa;		\
+		preempt_disable();					\
+		wa = __this_cpu_read(timer_unstable_counter_workaround); \
+		if (wa && wa->read_##reg)				\
+			_val = wa->read_##reg();			\
+		else							\
+			_val = read_sysreg(reg);			\
+		preempt_enable();					\
+	} else {							\
+		_val = read_sysreg(reg);				\
+	}								\
+	_val;								\
 })
 
 /*
diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index a0c9ee80147e..4551587bcb44 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -235,7 +235,8 @@ static u64 notrace hisi_161010101_read_cntvct_el0(void)
 #endif
 
 #ifdef CONFIG_ARM_ARCH_TIMER_OOL_WORKAROUND
-const struct arch_timer_erratum_workaround *timer_unstable_counter_workaround = NULL;
+DEFINE_PER_CPU(const struct arch_timer_erratum_workaround *,
+	       timer_unstable_counter_workaround);
 EXPORT_SYMBOL_GPL(timer_unstable_counter_workaround);
 
 DEFINE_STATIC_KEY_FALSE(arch_timer_read_ool_enabled);
@@ -338,9 +339,18 @@ arch_timer_iterate_errata(enum arch_timer_erratum_match_type type,
 }
 
 static
-void arch_timer_enable_workaround(const struct arch_timer_erratum_workaround *wa)
+void arch_timer_enable_workaround(const struct arch_timer_erratum_workaround *wa,
+				  bool local)
 {
-	timer_unstable_counter_workaround = wa;
+	int i;
+
+	if (local) {
+		__this_cpu_write(timer_unstable_counter_workaround, wa);
+	} else {
+		for_each_possible_cpu(i)
+			per_cpu(timer_unstable_counter_workaround, i) = wa;
+	}
+
 	static_branch_enable(&arch_timer_read_ool_enabled);
 }
 
@@ -369,14 +379,17 @@ static void arch_timer_check_ool_workaround(enum arch_timer_erratum_match_type t
 		return;
 
 	if (needs_unstable_timer_counter_workaround()) {
-		if (wa != timer_unstable_counter_workaround)
+		const struct arch_timer_erratum_workaround *__wa;
+		__wa = __this_cpu_read(timer_unstable_counter_workaround);
+		if (__wa && wa != __wa)
 			pr_warn("Can't enable workaround for %s (clashes with %s\n)",
-				wa->desc,
-				timer_unstable_counter_workaround->desc);
-		return;
+				wa->desc, __wa->desc);
+
+		if (__wa)
+			return;
 	}
 
-	arch_timer_enable_workaround(wa);
+	arch_timer_enable_workaround(wa, local);
 	pr_info("Enabling %s workaround for %s\n",
 		local ? "local" : "global", wa->desc);
 }
@@ -384,10 +397,15 @@ static void arch_timer_check_ool_workaround(enum arch_timer_erratum_match_type t
 #define erratum_handler(fn, r, ...)					\
 ({									\
 	bool __val;							\
-	if (needs_unstable_timer_counter_workaround() &&		\
-	    timer_unstable_counter_workaround->fn) {			\
-		r = timer_unstable_counter_workaround->fn(__VA_ARGS__);	\
-		__val = true;						\
+	if (needs_unstable_timer_counter_workaround()) {		\
+		const struct arch_timer_erratum_workaround *__wa;	\
+		__wa = __this_cpu_read(timer_unstable_counter_workaround); \
+		if (__wa && __wa->fn) {					\
+			r = __wa->fn(__VA_ARGS__);			\
+			__val = true;					\
+		} else {						\
+			__val = false;					\
+		}							\
 	} else {							\
 		__val = false;						\
 	}								\
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 59+ messages in thread

* [PATCH v3 13/18] arm64: arch_timer: Move clocksource_counter and co around
  2017-04-04 17:18 ` Marc Zyngier
@ 2017-04-04 17:18   ` Marc Zyngier
  -1 siblings, 0 replies; 59+ messages in thread
From: Marc Zyngier @ 2017-04-04 17:18 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel
  Cc: Mark Rutland, Catalin Marinas, Daniel Lezcano, Will Deacon,
	Scott Wood, Hanjun Guo, Ding Tianhong, dann frazier,
	Thomas Gleixner

In order to access clocksource_counter from the errata handling code,
move it (together with the related structures and functions) towards
the top of the file.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 drivers/clocksource/arm_arch_timer.c | 62 ++++++++++++++++++------------------
 1 file changed, 31 insertions(+), 31 deletions(-)

diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index 4551587bcb44..395f5d95a737 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -158,6 +158,37 @@ u32 arch_timer_reg_read(int access, enum arch_timer_reg reg,
 	return val;
 }
 
+/*
+ * Default to cp15 based access because arm64 uses this function for
+ * sched_clock() before DT is probed and the cp15 method is guaranteed
+ * to exist on arm64. arm doesn't use this before DT is probed so even
+ * if we don't have the cp15 accessors we won't have a problem.
+ */
+u64 (*arch_timer_read_counter)(void) = arch_counter_get_cntvct;
+
+static u64 arch_counter_read(struct clocksource *cs)
+{
+	return arch_timer_read_counter();
+}
+
+static u64 arch_counter_read_cc(const struct cyclecounter *cc)
+{
+	return arch_timer_read_counter();
+}
+
+static struct clocksource clocksource_counter = {
+	.name	= "arch_sys_counter",
+	.rating	= 400,
+	.read	= arch_counter_read,
+	.mask	= CLOCKSOURCE_MASK(56),
+	.flags	= CLOCK_SOURCE_IS_CONTINUOUS,
+};
+
+static struct cyclecounter cyclecounter __ro_after_init = {
+	.read	= arch_counter_read_cc,
+	.mask	= CLOCKSOURCE_MASK(56),
+};
+
 #ifdef CONFIG_FSL_ERRATUM_A008585
 /*
  * The number of retries is an arbitrary value well beyond the highest number
@@ -742,37 +773,6 @@ static u64 arch_counter_get_cntvct_mem(void)
 	return ((u64) vct_hi << 32) | vct_lo;
 }
 
-/*
- * Default to cp15 based access because arm64 uses this function for
- * sched_clock() before DT is probed and the cp15 method is guaranteed
- * to exist on arm64. arm doesn't use this before DT is probed so even
- * if we don't have the cp15 accessors we won't have a problem.
- */
-u64 (*arch_timer_read_counter)(void) = arch_counter_get_cntvct;
-
-static u64 arch_counter_read(struct clocksource *cs)
-{
-	return arch_timer_read_counter();
-}
-
-static u64 arch_counter_read_cc(const struct cyclecounter *cc)
-{
-	return arch_timer_read_counter();
-}
-
-static struct clocksource clocksource_counter = {
-	.name	= "arch_sys_counter",
-	.rating	= 400,
-	.read	= arch_counter_read,
-	.mask	= CLOCKSOURCE_MASK(56),
-	.flags	= CLOCK_SOURCE_IS_CONTINUOUS,
-};
-
-static struct cyclecounter cyclecounter __ro_after_init = {
-	.read	= arch_counter_read_cc,
-	.mask	= CLOCKSOURCE_MASK(56),
-};
-
 static struct arch_timer_kvm_info arch_timer_kvm_info;
 
 struct arch_timer_kvm_info *arch_timer_get_kvm_info(void)
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 59+ messages in thread

* [PATCH v3 13/18] arm64: arch_timer: Move clocksource_counter and co around
@ 2017-04-04 17:18   ` Marc Zyngier
  0 siblings, 0 replies; 59+ messages in thread
From: Marc Zyngier @ 2017-04-04 17:18 UTC (permalink / raw)
  To: linux-arm-kernel

In order to access clocksource_counter from the errata handling code,
move it (together with the related structures and functions) towards
the top of the file.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 drivers/clocksource/arm_arch_timer.c | 62 ++++++++++++++++++------------------
 1 file changed, 31 insertions(+), 31 deletions(-)

diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index 4551587bcb44..395f5d95a737 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -158,6 +158,37 @@ u32 arch_timer_reg_read(int access, enum arch_timer_reg reg,
 	return val;
 }
 
+/*
+ * Default to cp15 based access because arm64 uses this function for
+ * sched_clock() before DT is probed and the cp15 method is guaranteed
+ * to exist on arm64. arm doesn't use this before DT is probed so even
+ * if we don't have the cp15 accessors we won't have a problem.
+ */
+u64 (*arch_timer_read_counter)(void) = arch_counter_get_cntvct;
+
+static u64 arch_counter_read(struct clocksource *cs)
+{
+	return arch_timer_read_counter();
+}
+
+static u64 arch_counter_read_cc(const struct cyclecounter *cc)
+{
+	return arch_timer_read_counter();
+}
+
+static struct clocksource clocksource_counter = {
+	.name	= "arch_sys_counter",
+	.rating	= 400,
+	.read	= arch_counter_read,
+	.mask	= CLOCKSOURCE_MASK(56),
+	.flags	= CLOCK_SOURCE_IS_CONTINUOUS,
+};
+
+static struct cyclecounter cyclecounter __ro_after_init = {
+	.read	= arch_counter_read_cc,
+	.mask	= CLOCKSOURCE_MASK(56),
+};
+
 #ifdef CONFIG_FSL_ERRATUM_A008585
 /*
  * The number of retries is an arbitrary value well beyond the highest number
@@ -742,37 +773,6 @@ static u64 arch_counter_get_cntvct_mem(void)
 	return ((u64) vct_hi << 32) | vct_lo;
 }
 
-/*
- * Default to cp15 based access because arm64 uses this function for
- * sched_clock() before DT is probed and the cp15 method is guaranteed
- * to exist on arm64. arm doesn't use this before DT is probed so even
- * if we don't have the cp15 accessors we won't have a problem.
- */
-u64 (*arch_timer_read_counter)(void) = arch_counter_get_cntvct;
-
-static u64 arch_counter_read(struct clocksource *cs)
-{
-	return arch_timer_read_counter();
-}
-
-static u64 arch_counter_read_cc(const struct cyclecounter *cc)
-{
-	return arch_timer_read_counter();
-}
-
-static struct clocksource clocksource_counter = {
-	.name	= "arch_sys_counter",
-	.rating	= 400,
-	.read	= arch_counter_read,
-	.mask	= CLOCKSOURCE_MASK(56),
-	.flags	= CLOCK_SOURCE_IS_CONTINUOUS,
-};
-
-static struct cyclecounter cyclecounter __ro_after_init = {
-	.read	= arch_counter_read_cc,
-	.mask	= CLOCKSOURCE_MASK(56),
-};
-
 static struct arch_timer_kvm_info arch_timer_kvm_info;
 
 struct arch_timer_kvm_info *arch_timer_get_kvm_info(void)
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 59+ messages in thread

* [PATCH v3 14/18] arm64: arch_timer: Save cntkctl_el1 as a per-cpu variable
  2017-04-04 17:18 ` Marc Zyngier
@ 2017-04-04 17:18   ` Marc Zyngier
  -1 siblings, 0 replies; 59+ messages in thread
From: Marc Zyngier @ 2017-04-04 17:18 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel
  Cc: Mark Rutland, Catalin Marinas, Daniel Lezcano, Will Deacon,
	Scott Wood, Hanjun Guo, Ding Tianhong, dann frazier,
	Thomas Gleixner

As we're about to allow per CPU cntkctl_el1 configuration, we cannot
rely on the register value to be common when performing power
management.

Let's turn saved_cntkctl into a per-cpu variable.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 drivers/clocksource/arm_arch_timer.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index 395f5d95a737..5c114da0ed71 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -839,14 +839,14 @@ static int arch_timer_dying_cpu(unsigned int cpu)
 }
 
 #ifdef CONFIG_CPU_PM
-static unsigned int saved_cntkctl;
+static DEFINE_PER_CPU(unsigned long, saved_cntkctl);
 static int arch_timer_cpu_pm_notify(struct notifier_block *self,
 				    unsigned long action, void *hcpu)
 {
 	if (action == CPU_PM_ENTER)
-		saved_cntkctl = arch_timer_get_cntkctl();
+		__this_cpu_write(saved_cntkctl, arch_timer_get_cntkctl());
 	else if (action == CPU_PM_ENTER_FAILED || action == CPU_PM_EXIT)
-		arch_timer_set_cntkctl(saved_cntkctl);
+		arch_timer_set_cntkctl(__this_cpu_read(saved_cntkctl));
 	return NOTIFY_OK;
 }
 
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 59+ messages in thread

* [PATCH v3 14/18] arm64: arch_timer: Save cntkctl_el1 as a per-cpu variable
@ 2017-04-04 17:18   ` Marc Zyngier
  0 siblings, 0 replies; 59+ messages in thread
From: Marc Zyngier @ 2017-04-04 17:18 UTC (permalink / raw)
  To: linux-arm-kernel

As we're about to allow per CPU cntkctl_el1 configuration, we cannot
rely on the register value to be common when performing power
management.

Let's turn saved_cntkctl into a per-cpu variable.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 drivers/clocksource/arm_arch_timer.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index 395f5d95a737..5c114da0ed71 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -839,14 +839,14 @@ static int arch_timer_dying_cpu(unsigned int cpu)
 }
 
 #ifdef CONFIG_CPU_PM
-static unsigned int saved_cntkctl;
+static DEFINE_PER_CPU(unsigned long, saved_cntkctl);
 static int arch_timer_cpu_pm_notify(struct notifier_block *self,
 				    unsigned long action, void *hcpu)
 {
 	if (action == CPU_PM_ENTER)
-		saved_cntkctl = arch_timer_get_cntkctl();
+		__this_cpu_write(saved_cntkctl, arch_timer_get_cntkctl());
 	else if (action == CPU_PM_ENTER_FAILED || action == CPU_PM_EXIT)
-		arch_timer_set_cntkctl(saved_cntkctl);
+		arch_timer_set_cntkctl(__this_cpu_read(saved_cntkctl));
 	return NOTIFY_OK;
 }
 
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 59+ messages in thread

* [PATCH v3 15/18] arm64: arch_timer: Enable CNTVCT_EL0 trap if workaround is enabled
  2017-04-04 17:18 ` Marc Zyngier
@ 2017-04-04 17:18   ` Marc Zyngier
  -1 siblings, 0 replies; 59+ messages in thread
From: Marc Zyngier @ 2017-04-04 17:18 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel
  Cc: Mark Rutland, Catalin Marinas, Daniel Lezcano, Will Deacon,
	Scott Wood, Hanjun Guo, Ding Tianhong, dann frazier,
	Thomas Gleixner

Userspace being allowed to use read CNTVCT_EL0 anytime (and not
only in the VDSO), we need to enable trapping whenever a cntvct
workaround is enabled on a given CPU.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 drivers/clocksource/arm_arch_timer.c | 45 +++++++++++++++++++++++++-----------
 1 file changed, 32 insertions(+), 13 deletions(-)

diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index 5c114da0ed71..f8adea258cf0 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -83,6 +83,7 @@ static enum ppi_nr arch_timer_uses_ppi = VIRT_PPI;
 static bool arch_timer_c3stop;
 static bool arch_timer_mem_use_virtual;
 static bool arch_counter_suspend_stop;
+static bool vdso_default = true;
 
 static bool evtstrm_enable = IS_ENABLED(CONFIG_ARM_ARCH_TIMER_EVTSTREAM);
 
@@ -383,6 +384,17 @@ void arch_timer_enable_workaround(const struct arch_timer_erratum_workaround *wa
 	}
 
 	static_branch_enable(&arch_timer_read_ool_enabled);
+
+	/*
+	 * Don't use the vdso fastpath if errata require using the
+	 * out-of-line counter accessor. We may change our mind pretty
+	 * late in the game (with a per-CPU erratum, for example), so
+	 * change both the default value and the vdso itself.
+	 */
+	if (wa->read_cntvct_el0) {
+		clocksource_counter.archdata.vdso_direct = false;
+		vdso_default = false;
+	}
 }
 
 static void arch_timer_check_ool_workaround(enum arch_timer_erratum_match_type type,
@@ -443,11 +455,19 @@ static void arch_timer_check_ool_workaround(enum arch_timer_erratum_match_type t
 	__val;								\
 })
 
+static bool arch_timer_this_cpu_has_cntvct_wa(void)
+{
+	const struct arch_timer_erratum_workaround *wa;
+
+	wa = __this_cpu_read(timer_unstable_counter_workaround);
+	return wa && wa->read_cntvct_el0;
+}
 #else
 #define arch_timer_check_ool_workaround(t,a)		do { } while(0)
 #define erratum_set_next_event_tval_virt(...)		({BUG(); 0;})
 #define erratum_set_next_event_tval_phys(...)		({BUG(); 0;})
 #define erratum_handler(fn, r, ...)			({false;})
+#define arch_timer_this_cpu_has_cntvct_wa()		({false;})
 #endif /* CONFIG_ARM_ARCH_TIMER_OOL_WORKAROUND */
 
 static __always_inline irqreturn_t timer_handler(const int access,
@@ -660,15 +680,23 @@ static void arch_counter_set_user_access(void)
 {
 	u32 cntkctl = arch_timer_get_cntkctl();
 
-	/* Disable user access to the timers and the physical counter */
+	/* Disable user access to the timers and both counters */
 	/* Also disable virtual event stream */
 	cntkctl &= ~(ARCH_TIMER_USR_PT_ACCESS_EN
 			| ARCH_TIMER_USR_VT_ACCESS_EN
+		        | ARCH_TIMER_USR_VCT_ACCESS_EN
 			| ARCH_TIMER_VIRT_EVT_EN
 			| ARCH_TIMER_USR_PCT_ACCESS_EN);
 
-	/* Enable user access to the virtual counter */
-	cntkctl |= ARCH_TIMER_USR_VCT_ACCESS_EN;
+	/*
+	 * Enable user access to the virtual counter if it doesn't
+	 * need to be workaround. The vdso may have been already
+	 * disabled though.
+	 */
+	if (arch_timer_this_cpu_has_cntvct_wa())
+		pr_info("CPU%d: Trapping CNTVCT access\n", smp_processor_id());
+	else
+		cntkctl |= ARCH_TIMER_USR_VCT_ACCESS_EN;
 
 	arch_timer_set_cntkctl(cntkctl);
 }
@@ -791,16 +819,7 @@ static void __init arch_counter_register(unsigned type)
 		else
 			arch_timer_read_counter = arch_counter_get_cntpct;
 
-		clocksource_counter.archdata.vdso_direct = true;
-
-#ifdef CONFIG_ARM_ARCH_TIMER_OOL_WORKAROUND
-		/*
-		 * Don't use the vdso fastpath if errata require using
-		 * the out-of-line counter accessor.
-		 */
-		if (static_branch_unlikely(&arch_timer_read_ool_enabled))
-			clocksource_counter.archdata.vdso_direct = false;
-#endif
+		clocksource_counter.archdata.vdso_direct = vdso_default;
 	} else {
 		arch_timer_read_counter = arch_counter_get_cntvct_mem;
 	}
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 59+ messages in thread

* [PATCH v3 15/18] arm64: arch_timer: Enable CNTVCT_EL0 trap if workaround is enabled
@ 2017-04-04 17:18   ` Marc Zyngier
  0 siblings, 0 replies; 59+ messages in thread
From: Marc Zyngier @ 2017-04-04 17:18 UTC (permalink / raw)
  To: linux-arm-kernel

Userspace being allowed to use read CNTVCT_EL0 anytime (and not
only in the VDSO), we need to enable trapping whenever a cntvct
workaround is enabled on a given CPU.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 drivers/clocksource/arm_arch_timer.c | 45 +++++++++++++++++++++++++-----------
 1 file changed, 32 insertions(+), 13 deletions(-)

diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index 5c114da0ed71..f8adea258cf0 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -83,6 +83,7 @@ static enum ppi_nr arch_timer_uses_ppi = VIRT_PPI;
 static bool arch_timer_c3stop;
 static bool arch_timer_mem_use_virtual;
 static bool arch_counter_suspend_stop;
+static bool vdso_default = true;
 
 static bool evtstrm_enable = IS_ENABLED(CONFIG_ARM_ARCH_TIMER_EVTSTREAM);
 
@@ -383,6 +384,17 @@ void arch_timer_enable_workaround(const struct arch_timer_erratum_workaround *wa
 	}
 
 	static_branch_enable(&arch_timer_read_ool_enabled);
+
+	/*
+	 * Don't use the vdso fastpath if errata require using the
+	 * out-of-line counter accessor. We may change our mind pretty
+	 * late in the game (with a per-CPU erratum, for example), so
+	 * change both the default value and the vdso itself.
+	 */
+	if (wa->read_cntvct_el0) {
+		clocksource_counter.archdata.vdso_direct = false;
+		vdso_default = false;
+	}
 }
 
 static void arch_timer_check_ool_workaround(enum arch_timer_erratum_match_type type,
@@ -443,11 +455,19 @@ static void arch_timer_check_ool_workaround(enum arch_timer_erratum_match_type t
 	__val;								\
 })
 
+static bool arch_timer_this_cpu_has_cntvct_wa(void)
+{
+	const struct arch_timer_erratum_workaround *wa;
+
+	wa = __this_cpu_read(timer_unstable_counter_workaround);
+	return wa && wa->read_cntvct_el0;
+}
 #else
 #define arch_timer_check_ool_workaround(t,a)		do { } while(0)
 #define erratum_set_next_event_tval_virt(...)		({BUG(); 0;})
 #define erratum_set_next_event_tval_phys(...)		({BUG(); 0;})
 #define erratum_handler(fn, r, ...)			({false;})
+#define arch_timer_this_cpu_has_cntvct_wa()		({false;})
 #endif /* CONFIG_ARM_ARCH_TIMER_OOL_WORKAROUND */
 
 static __always_inline irqreturn_t timer_handler(const int access,
@@ -660,15 +680,23 @@ static void arch_counter_set_user_access(void)
 {
 	u32 cntkctl = arch_timer_get_cntkctl();
 
-	/* Disable user access to the timers and the physical counter */
+	/* Disable user access to the timers and both counters */
 	/* Also disable virtual event stream */
 	cntkctl &= ~(ARCH_TIMER_USR_PT_ACCESS_EN
 			| ARCH_TIMER_USR_VT_ACCESS_EN
+		        | ARCH_TIMER_USR_VCT_ACCESS_EN
 			| ARCH_TIMER_VIRT_EVT_EN
 			| ARCH_TIMER_USR_PCT_ACCESS_EN);
 
-	/* Enable user access to the virtual counter */
-	cntkctl |= ARCH_TIMER_USR_VCT_ACCESS_EN;
+	/*
+	 * Enable user access to the virtual counter if it doesn't
+	 * need to be workaround. The vdso may have been already
+	 * disabled though.
+	 */
+	if (arch_timer_this_cpu_has_cntvct_wa())
+		pr_info("CPU%d: Trapping CNTVCT access\n", smp_processor_id());
+	else
+		cntkctl |= ARCH_TIMER_USR_VCT_ACCESS_EN;
 
 	arch_timer_set_cntkctl(cntkctl);
 }
@@ -791,16 +819,7 @@ static void __init arch_counter_register(unsigned type)
 		else
 			arch_timer_read_counter = arch_counter_get_cntpct;
 
-		clocksource_counter.archdata.vdso_direct = true;
-
-#ifdef CONFIG_ARM_ARCH_TIMER_OOL_WORKAROUND
-		/*
-		 * Don't use the vdso fastpath if errata require using
-		 * the out-of-line counter accessor.
-		 */
-		if (static_branch_unlikely(&arch_timer_read_ool_enabled))
-			clocksource_counter.archdata.vdso_direct = false;
-#endif
+		clocksource_counter.archdata.vdso_direct = vdso_default;
 	} else {
 		arch_timer_read_counter = arch_counter_get_cntvct_mem;
 	}
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 59+ messages in thread

* [PATCH v3 16/18] arm64: arch_timer: Workaround for Cortex-A73 erratum 858921
  2017-04-04 17:18 ` Marc Zyngier
@ 2017-04-04 17:18   ` Marc Zyngier
  -1 siblings, 0 replies; 59+ messages in thread
From: Marc Zyngier @ 2017-04-04 17:18 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel
  Cc: Mark Rutland, Catalin Marinas, Daniel Lezcano, Will Deacon,
	Scott Wood, Hanjun Guo, Ding Tianhong, dann frazier,
	Thomas Gleixner

Cortex-A73 (all versions) counter read can return a wrong value
when the counter crosses a 32bit boundary.

The workaround involves performing the read twice, and to return
one or the other depending on whether a transition has taken place.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 drivers/clocksource/Kconfig          | 11 +++++++++++
 drivers/clocksource/arm_arch_timer.c | 19 +++++++++++++++++++
 2 files changed, 30 insertions(+)

diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
index 3356ab821624..af80a7c44faf 100644
--- a/drivers/clocksource/Kconfig
+++ b/drivers/clocksource/Kconfig
@@ -366,6 +366,17 @@ config HISILICON_ERRATUM_161010101
 	  161010101. The workaround will be active if the hisilicon,erratum-161010101
 	  property is found in the timer node.
 
+config ARM64_ERRATUM_858921
+	bool "Workaround for Cortex-A73 erratum 858921"
+	default y
+	select ARM_ARCH_TIMER_OOL_WORKAROUND
+	depends on ARM_ARCH_TIMER && ARM64
+	help
+	  This option enables a workaround applicable to Cortex-A73
+	  (all versions), whose counter may return incorrect values.
+	  The workaround will be dynamically enabled when an affected
+	  core is detected.
+
 config ARM_GLOBAL_TIMER
 	bool "Support for the ARM global timer" if COMPILE_TEST
 	select CLKSRC_OF if OF
diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index f8adea258cf0..8459d19a84f5 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -266,6 +266,17 @@ static u64 notrace hisi_161010101_read_cntvct_el0(void)
 }
 #endif
 
+#ifdef CONFIG_ARM64_ERRATUM_858921
+static u64 notrace arm64_858921_read_cntvct_el0(void)
+{
+	u64 old, new;
+
+	old = read_sysreg(cntvct_el0);
+	new = read_sysreg(cntvct_el0);
+	return (((old ^ new) >> 32) & 1) ? old : new;
+}
+#endif
+
 #ifdef CONFIG_ARM_ARCH_TIMER_OOL_WORKAROUND
 DEFINE_PER_CPU(const struct arch_timer_erratum_workaround *,
 	       timer_unstable_counter_workaround);
@@ -331,6 +342,14 @@ static const struct arch_timer_erratum_workaround ool_workarounds[] = {
 		.set_next_event_virt = erratum_set_next_event_tval_virt,
 	},
 #endif
+#ifdef CONFIG_ARM64_ERRATUM_858921
+	{
+		.match_type = ate_match_local_cap_id,
+		.id = (void *)ARM64_WORKAROUND_858921,
+		.desc = "ARM erratum 858921",
+		.read_cntvct_el0 = arm64_858921_read_cntvct_el0,
+	},
+#endif
 };
 
 typedef bool (*ate_match_fn_t)(const struct arch_timer_erratum_workaround *,
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 59+ messages in thread

* [PATCH v3 16/18] arm64: arch_timer: Workaround for Cortex-A73 erratum 858921
@ 2017-04-04 17:18   ` Marc Zyngier
  0 siblings, 0 replies; 59+ messages in thread
From: Marc Zyngier @ 2017-04-04 17:18 UTC (permalink / raw)
  To: linux-arm-kernel

Cortex-A73 (all versions) counter read can return a wrong value
when the counter crosses a 32bit boundary.

The workaround involves performing the read twice, and to return
one or the other depending on whether a transition has taken place.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 drivers/clocksource/Kconfig          | 11 +++++++++++
 drivers/clocksource/arm_arch_timer.c | 19 +++++++++++++++++++
 2 files changed, 30 insertions(+)

diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
index 3356ab821624..af80a7c44faf 100644
--- a/drivers/clocksource/Kconfig
+++ b/drivers/clocksource/Kconfig
@@ -366,6 +366,17 @@ config HISILICON_ERRATUM_161010101
 	  161010101. The workaround will be active if the hisilicon,erratum-161010101
 	  property is found in the timer node.
 
+config ARM64_ERRATUM_858921
+	bool "Workaround for Cortex-A73 erratum 858921"
+	default y
+	select ARM_ARCH_TIMER_OOL_WORKAROUND
+	depends on ARM_ARCH_TIMER && ARM64
+	help
+	  This option enables a workaround applicable to Cortex-A73
+	  (all versions), whose counter may return incorrect values.
+	  The workaround will be dynamically enabled when an affected
+	  core is detected.
+
 config ARM_GLOBAL_TIMER
 	bool "Support for the ARM global timer" if COMPILE_TEST
 	select CLKSRC_OF if OF
diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index f8adea258cf0..8459d19a84f5 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -266,6 +266,17 @@ static u64 notrace hisi_161010101_read_cntvct_el0(void)
 }
 #endif
 
+#ifdef CONFIG_ARM64_ERRATUM_858921
+static u64 notrace arm64_858921_read_cntvct_el0(void)
+{
+	u64 old, new;
+
+	old = read_sysreg(cntvct_el0);
+	new = read_sysreg(cntvct_el0);
+	return (((old ^ new) >> 32) & 1) ? old : new;
+}
+#endif
+
 #ifdef CONFIG_ARM_ARCH_TIMER_OOL_WORKAROUND
 DEFINE_PER_CPU(const struct arch_timer_erratum_workaround *,
 	       timer_unstable_counter_workaround);
@@ -331,6 +342,14 @@ static const struct arch_timer_erratum_workaround ool_workarounds[] = {
 		.set_next_event_virt = erratum_set_next_event_tval_virt,
 	},
 #endif
+#ifdef CONFIG_ARM64_ERRATUM_858921
+	{
+		.match_type = ate_match_local_cap_id,
+		.id = (void *)ARM64_WORKAROUND_858921,
+		.desc = "ARM erratum 858921",
+		.read_cntvct_el0 = arm64_858921_read_cntvct_el0,
+	},
+#endif
 };
 
 typedef bool (*ate_match_fn_t)(const struct arch_timer_erratum_workaround *,
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 59+ messages in thread

* [PATCH v3 17/18] arm64: arch_timer: Allow erratum matching with ACPI OEM information
  2017-04-04 17:18 ` Marc Zyngier
@ 2017-04-04 17:18   ` Marc Zyngier
  -1 siblings, 0 replies; 59+ messages in thread
From: Marc Zyngier @ 2017-04-04 17:18 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel
  Cc: Mark Rutland, Catalin Marinas, Daniel Lezcano, Will Deacon,
	Scott Wood, Hanjun Guo, Ding Tianhong, dann frazier,
	Thomas Gleixner

Just as we're able to identify a broken platform using some DT
information, let's enable a way to spot the offenders with ACPI.

The difference is that we can only match on some OEM info instead
of implementation-specific properties. So in order to avoid the
insane multiplication of errata structures, we allow an array
of OEM descriptions to be attached to an erratum structure.

Tested-by: dann frazier <dann.frazier@canonical.com>
Tested-by: Hanjun Guo <hanjun.guo@linaro.org>
Reviewed-by: Hanjun Guo <hanjun.guo@linaro.org>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm64/include/asm/arch_timer.h  |  1 +
 drivers/clocksource/arm_arch_timer.c | 34 ++++++++++++++++++++++++++++++++++
 2 files changed, 35 insertions(+)

diff --git a/arch/arm64/include/asm/arch_timer.h b/arch/arm64/include/asm/arch_timer.h
index 6bd1a9a1573a..74d08e44a651 100644
--- a/arch/arm64/include/asm/arch_timer.h
+++ b/arch/arm64/include/asm/arch_timer.h
@@ -41,6 +41,7 @@ extern struct static_key_false arch_timer_read_ool_enabled;
 enum arch_timer_erratum_match_type {
 	ate_match_dt,
 	ate_match_local_cap_id,
+	ate_match_acpi_oem_info,
 };
 
 struct clock_event_device;
diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index 8459d19a84f5..887f6d00a71d 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -190,6 +190,12 @@ static struct cyclecounter cyclecounter __ro_after_init = {
 	.mask	= CLOCKSOURCE_MASK(56),
 };
 
+struct ate_acpi_oem_info {
+	char oem_id[ACPI_OEM_ID_SIZE + 1];
+	char oem_table_id[ACPI_OEM_TABLE_ID_SIZE + 1];
+	u32 oem_revision;
+};
+
 #ifdef CONFIG_FSL_ERRATUM_A008585
 /*
  * The number of retries is an arbitrary value well beyond the highest number
@@ -371,6 +377,28 @@ bool arch_timer_check_local_cap_erratum(const struct arch_timer_erratum_workarou
 	return this_cpu_has_cap((uintptr_t)wa->id);
 }
 
+
+static
+bool arch_timer_check_acpi_oem_erratum(const struct arch_timer_erratum_workaround *wa,
+				       const void *arg)
+{
+	static const struct ate_acpi_oem_info empty_oem_info = {};
+	const struct ate_acpi_oem_info *info = wa->id;
+	const struct acpi_table_header *table = arg;
+
+	/* Iterate over the ACPI OEM info array, looking for a match */
+	while (memcmp(info, &empty_oem_info, sizeof(*info))) {
+		if (!memcmp(info->oem_id, table->oem_id, ACPI_OEM_ID_SIZE) &&
+		    !memcmp(info->oem_table_id, table->oem_table_id, ACPI_OEM_TABLE_ID_SIZE) &&
+		    info->oem_revision == table->oem_revision)
+			return true;
+
+		info++;
+	}
+
+	return false;
+}
+
 static const struct arch_timer_erratum_workaround *
 arch_timer_iterate_errata(enum arch_timer_erratum_match_type type,
 			  ate_match_fn_t match_fn,
@@ -431,6 +459,9 @@ static void arch_timer_check_ool_workaround(enum arch_timer_erratum_match_type t
 		match_fn = arch_timer_check_local_cap_erratum;
 		local = true;
 		break;
+	case ate_match_acpi_oem_info:
+		match_fn = arch_timer_check_acpi_oem_erratum;
+		break;
 	default:
 		WARN_ON(1);
 		return;
@@ -1277,6 +1308,9 @@ static int __init arch_timer_acpi_init(struct acpi_table_header *table)
 	/* Always-on capability */
 	arch_timer_c3stop = !(gtdt->non_secure_el1_flags & ACPI_GTDT_ALWAYS_ON);
 
+	/* Check for globally applicable workarounds */
+	arch_timer_check_ool_workaround(ate_match_acpi_oem_info, table);
+
 	arch_timer_init();
 	return 0;
 }
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 59+ messages in thread

* [PATCH v3 17/18] arm64: arch_timer: Allow erratum matching with ACPI OEM information
@ 2017-04-04 17:18   ` Marc Zyngier
  0 siblings, 0 replies; 59+ messages in thread
From: Marc Zyngier @ 2017-04-04 17:18 UTC (permalink / raw)
  To: linux-arm-kernel

Just as we're able to identify a broken platform using some DT
information, let's enable a way to spot the offenders with ACPI.

The difference is that we can only match on some OEM info instead
of implementation-specific properties. So in order to avoid the
insane multiplication of errata structures, we allow an array
of OEM descriptions to be attached to an erratum structure.

Tested-by: dann frazier <dann.frazier@canonical.com>
Tested-by: Hanjun Guo <hanjun.guo@linaro.org>
Reviewed-by: Hanjun Guo <hanjun.guo@linaro.org>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm64/include/asm/arch_timer.h  |  1 +
 drivers/clocksource/arm_arch_timer.c | 34 ++++++++++++++++++++++++++++++++++
 2 files changed, 35 insertions(+)

diff --git a/arch/arm64/include/asm/arch_timer.h b/arch/arm64/include/asm/arch_timer.h
index 6bd1a9a1573a..74d08e44a651 100644
--- a/arch/arm64/include/asm/arch_timer.h
+++ b/arch/arm64/include/asm/arch_timer.h
@@ -41,6 +41,7 @@ extern struct static_key_false arch_timer_read_ool_enabled;
 enum arch_timer_erratum_match_type {
 	ate_match_dt,
 	ate_match_local_cap_id,
+	ate_match_acpi_oem_info,
 };
 
 struct clock_event_device;
diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index 8459d19a84f5..887f6d00a71d 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -190,6 +190,12 @@ static struct cyclecounter cyclecounter __ro_after_init = {
 	.mask	= CLOCKSOURCE_MASK(56),
 };
 
+struct ate_acpi_oem_info {
+	char oem_id[ACPI_OEM_ID_SIZE + 1];
+	char oem_table_id[ACPI_OEM_TABLE_ID_SIZE + 1];
+	u32 oem_revision;
+};
+
 #ifdef CONFIG_FSL_ERRATUM_A008585
 /*
  * The number of retries is an arbitrary value well beyond the highest number
@@ -371,6 +377,28 @@ bool arch_timer_check_local_cap_erratum(const struct arch_timer_erratum_workarou
 	return this_cpu_has_cap((uintptr_t)wa->id);
 }
 
+
+static
+bool arch_timer_check_acpi_oem_erratum(const struct arch_timer_erratum_workaround *wa,
+				       const void *arg)
+{
+	static const struct ate_acpi_oem_info empty_oem_info = {};
+	const struct ate_acpi_oem_info *info = wa->id;
+	const struct acpi_table_header *table = arg;
+
+	/* Iterate over the ACPI OEM info array, looking for a match */
+	while (memcmp(info, &empty_oem_info, sizeof(*info))) {
+		if (!memcmp(info->oem_id, table->oem_id, ACPI_OEM_ID_SIZE) &&
+		    !memcmp(info->oem_table_id, table->oem_table_id, ACPI_OEM_TABLE_ID_SIZE) &&
+		    info->oem_revision == table->oem_revision)
+			return true;
+
+		info++;
+	}
+
+	return false;
+}
+
 static const struct arch_timer_erratum_workaround *
 arch_timer_iterate_errata(enum arch_timer_erratum_match_type type,
 			  ate_match_fn_t match_fn,
@@ -431,6 +459,9 @@ static void arch_timer_check_ool_workaround(enum arch_timer_erratum_match_type t
 		match_fn = arch_timer_check_local_cap_erratum;
 		local = true;
 		break;
+	case ate_match_acpi_oem_info:
+		match_fn = arch_timer_check_acpi_oem_erratum;
+		break;
 	default:
 		WARN_ON(1);
 		return;
@@ -1277,6 +1308,9 @@ static int __init arch_timer_acpi_init(struct acpi_table_header *table)
 	/* Always-on capability */
 	arch_timer_c3stop = !(gtdt->non_secure_el1_flags & ACPI_GTDT_ALWAYS_ON);
 
+	/* Check for globally applicable workarounds */
+	arch_timer_check_ool_workaround(ate_match_acpi_oem_info, table);
+
 	arch_timer_init();
 	return 0;
 }
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 59+ messages in thread

* [PATCH v3 18/18] arm64: arch_timer: Add HISILICON_ERRATUM_161010101 ACPI matching data
  2017-04-04 17:18 ` Marc Zyngier
@ 2017-04-04 17:18   ` Marc Zyngier
  -1 siblings, 0 replies; 59+ messages in thread
From: Marc Zyngier @ 2017-04-04 17:18 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel
  Cc: Mark Rutland, Catalin Marinas, Daniel Lezcano, Will Deacon,
	Scott Wood, Hanjun Guo, Ding Tianhong, dann frazier,
	Thomas Gleixner

In order to deal with ACPI enabled platforms suffering from the
HISILICON_ERRATUM_161010101, let's add the required OEM data that
allow the workaround to be enabled.

Tested-by: dann frazier <dann.frazier@canonical.com>
Tested-by: Hanjun Guo <hanjun.guo@linaro.org>
Reviewed-by: Hanjun Guo <hanjun.guo@linaro.org>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 drivers/clocksource/arm_arch_timer.c | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index 887f6d00a71d..bf9e9d76375e 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -270,6 +270,29 @@ static u64 notrace hisi_161010101_read_cntvct_el0(void)
 {
 	return __hisi_161010101_read_reg(cntvct_el0);
 }
+
+static struct ate_acpi_oem_info hisi_161010101_oem_info[] = {
+	/*
+	 * Note that trailing spaces are required to properly match
+	 * the OEM table information.
+	 */
+	{
+		.oem_id		= "HISI  ",
+		.oem_table_id	= "HIP05   ",
+		.oem_revision	= 0,
+	},
+	{
+		.oem_id		= "HISI  ",
+		.oem_table_id	= "HIP06   ",
+		.oem_revision	= 0,
+	},
+	{
+		.oem_id		= "HISI  ",
+		.oem_table_id	= "HIP07   ",
+		.oem_revision	= 0,
+	},
+	{ /* Sentinel indicating the end of the OEM array */ },
+};
 #endif
 
 #ifdef CONFIG_ARM64_ERRATUM_858921
@@ -347,6 +370,16 @@ static const struct arch_timer_erratum_workaround ool_workarounds[] = {
 		.set_next_event_phys = erratum_set_next_event_tval_phys,
 		.set_next_event_virt = erratum_set_next_event_tval_virt,
 	},
+	{
+		.match_type = ate_match_acpi_oem_info,
+		.id = hisi_161010101_oem_info,
+		.desc = "HiSilicon erratum 161010101",
+		.read_cntp_tval_el0 = hisi_161010101_read_cntp_tval_el0,
+		.read_cntv_tval_el0 = hisi_161010101_read_cntv_tval_el0,
+		.read_cntvct_el0 = hisi_161010101_read_cntvct_el0,
+		.set_next_event_phys = erratum_set_next_event_tval_phys,
+		.set_next_event_virt = erratum_set_next_event_tval_virt,
+	},
 #endif
 #ifdef CONFIG_ARM64_ERRATUM_858921
 	{
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 59+ messages in thread

* [PATCH v3 18/18] arm64: arch_timer: Add HISILICON_ERRATUM_161010101 ACPI matching data
@ 2017-04-04 17:18   ` Marc Zyngier
  0 siblings, 0 replies; 59+ messages in thread
From: Marc Zyngier @ 2017-04-04 17:18 UTC (permalink / raw)
  To: linux-arm-kernel

In order to deal with ACPI enabled platforms suffering from the
HISILICON_ERRATUM_161010101, let's add the required OEM data that
allow the workaround to be enabled.

Tested-by: dann frazier <dann.frazier@canonical.com>
Tested-by: Hanjun Guo <hanjun.guo@linaro.org>
Reviewed-by: Hanjun Guo <hanjun.guo@linaro.org>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 drivers/clocksource/arm_arch_timer.c | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index 887f6d00a71d..bf9e9d76375e 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -270,6 +270,29 @@ static u64 notrace hisi_161010101_read_cntvct_el0(void)
 {
 	return __hisi_161010101_read_reg(cntvct_el0);
 }
+
+static struct ate_acpi_oem_info hisi_161010101_oem_info[] = {
+	/*
+	 * Note that trailing spaces are required to properly match
+	 * the OEM table information.
+	 */
+	{
+		.oem_id		= "HISI  ",
+		.oem_table_id	= "HIP05   ",
+		.oem_revision	= 0,
+	},
+	{
+		.oem_id		= "HISI  ",
+		.oem_table_id	= "HIP06   ",
+		.oem_revision	= 0,
+	},
+	{
+		.oem_id		= "HISI  ",
+		.oem_table_id	= "HIP07   ",
+		.oem_revision	= 0,
+	},
+	{ /* Sentinel indicating the end of the OEM array */ },
+};
 #endif
 
 #ifdef CONFIG_ARM64_ERRATUM_858921
@@ -347,6 +370,16 @@ static const struct arch_timer_erratum_workaround ool_workarounds[] = {
 		.set_next_event_phys = erratum_set_next_event_tval_phys,
 		.set_next_event_virt = erratum_set_next_event_tval_virt,
 	},
+	{
+		.match_type = ate_match_acpi_oem_info,
+		.id = hisi_161010101_oem_info,
+		.desc = "HiSilicon erratum 161010101",
+		.read_cntp_tval_el0 = hisi_161010101_read_cntp_tval_el0,
+		.read_cntv_tval_el0 = hisi_161010101_read_cntv_tval_el0,
+		.read_cntvct_el0 = hisi_161010101_read_cntvct_el0,
+		.set_next_event_phys = erratum_set_next_event_tval_phys,
+		.set_next_event_virt = erratum_set_next_event_tval_virt,
+	},
 #endif
 #ifdef CONFIG_ARM64_ERRATUM_858921
 	{
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 59+ messages in thread

* Re: [PATCH v3 00/18] clocksource/arch_timer: Errata workaround infrastructure rework
  2017-04-04 17:18 ` Marc Zyngier
@ 2017-04-06 16:27   ` Catalin Marinas
  -1 siblings, 0 replies; 59+ messages in thread
From: Catalin Marinas @ 2017-04-06 16:27 UTC (permalink / raw)
  To: Daniel Lezcano
  Cc: Marc Zyngier, linux-arm-kernel, linux-kernel, Mark Rutland,
	Will Deacon, Scott Wood, Hanjun Guo, Ding Tianhong, dann frazier,
	Thomas Gleixner

Hi Daniel,

On Tue, Apr 04, 2017 at 06:18:08PM +0100, Marc Zyngier wrote:
> Marc Zyngier (18):
>   arm64: Allow checking of a CPU-local erratum
>   arm64: Add CNTVCT_EL0 trap handler
>   arm64: Define Cortex-A73 MIDR
>   arm64: cpu_errata: Allow an erratum to be match for all revisions of a
>     core
>   arm64: cpu_errata: Add capability to advertise Cortex-A73 erratum
>     858921
>   arm64: arch_timer: Add infrastructure for multiple erratum detection
>     methods
>   arm64: arch_timer: Add erratum handler for CPU-specific capability
>   arm64: arch_timer: Move arch_timer_reg_read/write around
>   arm64: arch_timer: Get rid of erratum_workaround_set_sne
>   arm64: arch_timer: Rework the set_next_event workarounds
>   arm64: arch_timer: Make workaround methods optional
>   arm64: arch_timer: Allows a CPU-specific erratum to only affect a
>     subset of CPUs
>   arm64: arch_timer: Move clocksource_counter and co around
>   arm64: arch_timer: Save cntkctl_el1 as a per-cpu variable
>   arm64: arch_timer: Enable CNTVCT_EL0 trap if workaround is enabled
>   arm64: arch_timer: Workaround for Cortex-A73 erratum 858921
>   arm64: arch_timer: Allow erratum matching with ACPI OEM information
>   arm64: arch_timer: Add HISILICON_ERRATUM_161010101 ACPI matching data

Are you happy to take the arch_timer patches above? If yes, Marc can
create a common branch with the first 5 patches which I will merge in
the arm64 tree. You can then pull the whole series into your tree.

Thanks.

-- 
Catalin

^ permalink raw reply	[flat|nested] 59+ messages in thread

* [PATCH v3 00/18] clocksource/arch_timer: Errata workaround infrastructure rework
@ 2017-04-06 16:27   ` Catalin Marinas
  0 siblings, 0 replies; 59+ messages in thread
From: Catalin Marinas @ 2017-04-06 16:27 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Daniel,

On Tue, Apr 04, 2017 at 06:18:08PM +0100, Marc Zyngier wrote:
> Marc Zyngier (18):
>   arm64: Allow checking of a CPU-local erratum
>   arm64: Add CNTVCT_EL0 trap handler
>   arm64: Define Cortex-A73 MIDR
>   arm64: cpu_errata: Allow an erratum to be match for all revisions of a
>     core
>   arm64: cpu_errata: Add capability to advertise Cortex-A73 erratum
>     858921
>   arm64: arch_timer: Add infrastructure for multiple erratum detection
>     methods
>   arm64: arch_timer: Add erratum handler for CPU-specific capability
>   arm64: arch_timer: Move arch_timer_reg_read/write around
>   arm64: arch_timer: Get rid of erratum_workaround_set_sne
>   arm64: arch_timer: Rework the set_next_event workarounds
>   arm64: arch_timer: Make workaround methods optional
>   arm64: arch_timer: Allows a CPU-specific erratum to only affect a
>     subset of CPUs
>   arm64: arch_timer: Move clocksource_counter and co around
>   arm64: arch_timer: Save cntkctl_el1 as a per-cpu variable
>   arm64: arch_timer: Enable CNTVCT_EL0 trap if workaround is enabled
>   arm64: arch_timer: Workaround for Cortex-A73 erratum 858921
>   arm64: arch_timer: Allow erratum matching with ACPI OEM information
>   arm64: arch_timer: Add HISILICON_ERRATUM_161010101 ACPI matching data

Are you happy to take the arch_timer patches above? If yes, Marc can
create a common branch with the first 5 patches which I will merge in
the arm64 tree. You can then pull the whole series into your tree.

Thanks.

-- 
Catalin

^ permalink raw reply	[flat|nested] 59+ messages in thread

* [PATCH v3 00/18] clocksource/arch_timer: Errata workaround infrastructure rework
  2017-04-06 16:27   ` Catalin Marinas
  (?)
@ 2017-04-06 21:15   ` Daniel Lezcano
  2017-04-07  8:15       ` Thomas Gleixner
  -1 siblings, 1 reply; 59+ messages in thread
From: Daniel Lezcano @ 2017-04-06 21:15 UTC (permalink / raw)
  To: linux-arm-kernel

On 06/04/2017 18:27, Catalin Marinas wrote:
> Hi Daniel,
> 
> On Tue, Apr 04, 2017 at 06:18:08PM +0100, Marc Zyngier wrote:
>> Marc Zyngier (18):
>>   arm64: Allow checking of a CPU-local erratum
>>   arm64: Add CNTVCT_EL0 trap handler
>>   arm64: Define Cortex-A73 MIDR
>>   arm64: cpu_errata: Allow an erratum to be match for all revisions of a
>>     core
>>   arm64: cpu_errata: Add capability to advertise Cortex-A73 erratum
>>     858921
>>   arm64: arch_timer: Add infrastructure for multiple erratum detection
>>     methods
>>   arm64: arch_timer: Add erratum handler for CPU-specific capability
>>   arm64: arch_timer: Move arch_timer_reg_read/write around
>>   arm64: arch_timer: Get rid of erratum_workaround_set_sne
>>   arm64: arch_timer: Rework the set_next_event workarounds
>>   arm64: arch_timer: Make workaround methods optional
>>   arm64: arch_timer: Allows a CPU-specific erratum to only affect a
>>     subset of CPUs
>>   arm64: arch_timer: Move clocksource_counter and co around
>>   arm64: arch_timer: Save cntkctl_el1 as a per-cpu variable
>>   arm64: arch_timer: Enable CNTVCT_EL0 trap if workaround is enabled
>>   arm64: arch_timer: Workaround for Cortex-A73 erratum 858921
>>   arm64: arch_timer: Allow erratum matching with ACPI OEM information
>>   arm64: arch_timer: Add HISILICON_ERRATUM_161010101 ACPI matching data
> 
> Are you happy to take the arch_timer patches above? If yes, Marc can
> create a common branch with the first 5 patches which I will merge in
> the arm64 tree. You can then pull the whole series into your tree.

Hi Catalin,

I'm fine with the merge process you are proposing. But before going
forward, I would like to wait a bit in order to give the opportunity to
Thomas to review the patchset.

Thanks.

  -- Daniel


-- 
 <http://www.linaro.org/> Linaro.org ? Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

^ permalink raw reply	[flat|nested] 59+ messages in thread

* Re: [PATCH v3 00/18] clocksource/arch_timer: Errata workaround infrastructure rework
  2017-04-06 21:15   ` Daniel Lezcano
@ 2017-04-07  8:15       ` Thomas Gleixner
  0 siblings, 0 replies; 59+ messages in thread
From: Thomas Gleixner @ 2017-04-07  8:15 UTC (permalink / raw)
  To: Daniel Lezcano
  Cc: Catalin Marinas, Marc Zyngier, linux-arm-kernel, linux-kernel,
	Mark Rutland, Will Deacon, Scott Wood, Hanjun Guo, Ding Tianhong,
	dann frazier

On Thu, 6 Apr 2017, Daniel Lezcano wrote:
> On 06/04/2017 18:27, Catalin Marinas wrote:
> > Are you happy to take the arch_timer patches above? If yes, Marc can
> > create a common branch with the first 5 patches which I will merge in
> > the arm64 tree. You can then pull the whole series into your tree.
> 
> Hi Catalin,
> 
> I'm fine with the merge process you are proposing. But before going
> forward, I would like to wait a bit in order to give the opportunity to
> Thomas to review the patchset.

I'm fine with that. It's sad that we need to have a workaround framework
for dealing with this, but silicon tinkerers seem to compete on providing
the most broken implementation of an _architected_ timer.

Acked-by: Thomas Gleixner <tglx@linutronix.de>

^ permalink raw reply	[flat|nested] 59+ messages in thread

* [PATCH v3 00/18] clocksource/arch_timer: Errata workaround infrastructure rework
@ 2017-04-07  8:15       ` Thomas Gleixner
  0 siblings, 0 replies; 59+ messages in thread
From: Thomas Gleixner @ 2017-04-07  8:15 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, 6 Apr 2017, Daniel Lezcano wrote:
> On 06/04/2017 18:27, Catalin Marinas wrote:
> > Are you happy to take the arch_timer patches above? If yes, Marc can
> > create a common branch with the first 5 patches which I will merge in
> > the arm64 tree. You can then pull the whole series into your tree.
> 
> Hi Catalin,
> 
> I'm fine with the merge process you are proposing. But before going
> forward, I would like to wait a bit in order to give the opportunity to
> Thomas to review the patchset.

I'm fine with that. It's sad that we need to have a workaround framework
for dealing with this, but silicon tinkerers seem to compete on providing
the most broken implementation of an _architected_ timer.

Acked-by: Thomas Gleixner <tglx@linutronix.de>

^ permalink raw reply	[flat|nested] 59+ messages in thread

* Re: [PATCH v3 15/18] arm64: arch_timer: Enable CNTVCT_EL0 trap if workaround is enabled
       [not found]   ` <58FDB05B.6020108@huawei.com>
@ 2017-04-24  8:13       ` Marc Zyngier
  0 siblings, 0 replies; 59+ messages in thread
From: Marc Zyngier @ 2017-04-24  8:13 UTC (permalink / raw)
  To: Hanjun Guo, linux-arm-kernel, linux-kernel
  Cc: Mark Rutland, Catalin Marinas, Daniel Lezcano, Will Deacon,
	Scott Wood, Hanjun Guo, Ding Tianhong, dann frazier,
	Thomas Gleixner, lixiaoping3

On 24/04/17 08:59, Hanjun Guo wrote:
> Hi Marc,
> 
> On 2017/4/5 1:18, Marc Zyngier wrote:
>> Userspace being allowed to use read CNTVCT_EL0 anytime (and not
>> only in the VDSO), we need to enable trapping whenever a cntvct
>> workaround is enabled on a given CPU.
>>
>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
>> ---
>>  drivers/clocksource/arm_arch_timer.c | 45 +++++++++++++++++++++++++-----------
>>  1 file changed, 32 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
> [...]
>>  #else
>>  #define arch_timer_check_ool_workaround(t,a)		do { } while(0)
>>  #define erratum_set_next_event_tval_virt(...)		({BUG(); 0;})
>>  #define erratum_set_next_event_tval_phys(...)		({BUG(); 0;})
>>  #define erratum_handler(fn, r, ...)			({false;})
>> +#define arch_timer_this_cpu_has_cntvct_wa()		({false;})
>>  #endif /* CONFIG_ARM_ARCH_TIMER_OOL_WORKAROUND */
>>  
>>  static __always_inline irqreturn_t timer_handler(const int access,
>> @@ -660,15 +680,23 @@ static void arch_counter_set_user_access(void)
>>  {
>>  	u32 cntkctl = arch_timer_get_cntkctl();
>>  
>> -	/* Disable user access to the timers and the physical counter */
>> +	/* Disable user access to the timers and both counters */
>>  	/* Also disable virtual event stream */
>>  	cntkctl &= ~(ARCH_TIMER_USR_PT_ACCESS_EN
>>  			| ARCH_TIMER_USR_VT_ACCESS_EN
>> +		        | ARCH_TIMER_USR_VCT_ACCESS_EN
>>  			| ARCH_TIMER_VIRT_EVT_EN
>>  			| ARCH_TIMER_USR_PCT_ACCESS_EN);
>>  
>> -	/* Enable user access to the virtual counter */
>> -	cntkctl |= ARCH_TIMER_USR_VCT_ACCESS_EN;
>> +	/*
>> +	 * Enable user access to the virtual counter if it doesn't
>> +	 * need to be workaround. The vdso may have been already
>> +	 * disabled though.
>> +	 */
>> +	if (arch_timer_this_cpu_has_cntvct_wa())
>> +		pr_info("CPU%d: Trapping CNTVCT access\n", smp_processor_id());
>> +	else
>> +		cntkctl |= ARCH_TIMER_USR_VCT_ACCESS_EN;
> 
> Since CNTVCT_EL0 and CNTFRQ_EL0 share the same control register,then we
> need to trap CNTFRQ_EL0 as well.
> 
> We hit an "undefined instruction" when running Open MPI, which
> read CNTFRQ_EL0 in the user space:
> 
> https://github.com/open-mpi/ompi/blob/5b40fd267f9ddc6463051e402382a988637c3bb3/opal/include/opal/sys/arm64/timer.h
> 
> static inline opal_timer_t
> opal_sys_timer_freq(void) { opal_timer_t freq; __asm__ __volatile__
> ("mrs %0, CNTFRQ_EL0" : "=r" (freq)); return (opal_timer_t)(freq); }
> Please take look :) Thanks Hanjun

Ah, nice :-/ ... Can you please give the patch below a shot?

Thanks,

	M.

>From 3f13e53878356057bde3e023614789d6c6b73628 Mon Sep 17 00:00:00 2001
From: Marc Zyngier <marc.zyngier@arm.com>
Date: Mon, 24 Apr 2017 09:04:03 +0100
Subject: [PATCH] arm64: Add CNTFRQ_EL0 trap handler

We now trap accesses to CNTVCT_EL0 when the counter is broken
enough to require the kernel to mediate the access. But it
turns out that some existing userspace (such as OpenMPI) do
probe for the counter frequency, leading to an UNDEF exception
as CNTVCT_EL0 and CNTFRQ_EL0 share the same control bit.

The fix is to handle the exception the same way we do for CNTVCT_EL0.

Fixes: a86bd139f2ae ("arm64: arch_timer: Enable CNTVCT_EL0 trap if workaround is enabled")
Reported-by: Hanjun Guo <guohanjun@huawei.com>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm64/include/asm/esr.h |  4 ++++
 arch/arm64/kernel/traps.c    | 14 ++++++++++++++
 2 files changed, 18 insertions(+)

diff --git a/arch/arm64/include/asm/esr.h b/arch/arm64/include/asm/esr.h
index ad42e79a5d4d..8ea134f88fda 100644
--- a/arch/arm64/include/asm/esr.h
+++ b/arch/arm64/include/asm/esr.h
@@ -177,6 +177,10 @@
 
 #define ESR_ELx_SYS64_ISS_SYS_CNTVCT	(ESR_ELx_SYS64_ISS_SYS_VAL(3, 3, 2, 14, 0) | \
 					 ESR_ELx_SYS64_ISS_DIR_READ)
+
+#define ESR_ELx_SYS64_ISS_SYS_CNTFRQ	(ESR_ELx_SYS64_ISS_SYS_VAL(3, 3, 14, 0, 0) | \
+					 ESR_ELx_SYS64_ISS_DIR_READ)
+
 #ifndef __ASSEMBLY__
 #include <asm/types.h>
 
diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
index 1de444e6c669..d4d6ae02cd55 100644
--- a/arch/arm64/kernel/traps.c
+++ b/arch/arm64/kernel/traps.c
@@ -513,6 +513,14 @@ static void cntvct_read_handler(unsigned int esr, struct pt_regs *regs)
 	regs->pc += 4;
 }
 
+static void cntfrq_read_handler(unsigned int esr, struct pt_regs *regs)
+{
+	int rt = (esr & ESR_ELx_SYS64_ISS_RT_MASK) >> ESR_ELx_SYS64_ISS_RT_SHIFT;
+
+	pt_regs_write_reg(regs, rt, read_sysreg(cntfrq_el0));
+	regs->pc += 4;
+}
+
 struct sys64_hook {
 	unsigned int esr_mask;
 	unsigned int esr_val;
@@ -537,6 +545,12 @@ static struct sys64_hook sys64_hooks[] = {
 		.esr_val = ESR_ELx_SYS64_ISS_SYS_CNTVCT,
 		.handler = cntvct_read_handler,
 	},
+	{
+		/* Trap read access to CNTFRQ_EL0 */
+		.esr_mask = ESR_ELx_SYS64_ISS_SYS_OP_MASK,
+		.esr_val = ESR_ELx_SYS64_ISS_SYS_CNTFRQ,
+		.handler = cntfrq_read_handler,
+	},
 	{},
 };
 
-- 
2.11.0

-- 
Jazz is not dead. It just smells funny...

^ permalink raw reply related	[flat|nested] 59+ messages in thread

* [PATCH v3 15/18] arm64: arch_timer: Enable CNTVCT_EL0 trap if workaround is enabled
@ 2017-04-24  8:13       ` Marc Zyngier
  0 siblings, 0 replies; 59+ messages in thread
From: Marc Zyngier @ 2017-04-24  8:13 UTC (permalink / raw)
  To: linux-arm-kernel

On 24/04/17 08:59, Hanjun Guo wrote:
> Hi Marc,
> 
> On 2017/4/5 1:18, Marc Zyngier wrote:
>> Userspace being allowed to use read CNTVCT_EL0 anytime (and not
>> only in the VDSO), we need to enable trapping whenever a cntvct
>> workaround is enabled on a given CPU.
>>
>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
>> ---
>>  drivers/clocksource/arm_arch_timer.c | 45 +++++++++++++++++++++++++-----------
>>  1 file changed, 32 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
> [...]
>>  #else
>>  #define arch_timer_check_ool_workaround(t,a)		do { } while(0)
>>  #define erratum_set_next_event_tval_virt(...)		({BUG(); 0;})
>>  #define erratum_set_next_event_tval_phys(...)		({BUG(); 0;})
>>  #define erratum_handler(fn, r, ...)			({false;})
>> +#define arch_timer_this_cpu_has_cntvct_wa()		({false;})
>>  #endif /* CONFIG_ARM_ARCH_TIMER_OOL_WORKAROUND */
>>  
>>  static __always_inline irqreturn_t timer_handler(const int access,
>> @@ -660,15 +680,23 @@ static void arch_counter_set_user_access(void)
>>  {
>>  	u32 cntkctl = arch_timer_get_cntkctl();
>>  
>> -	/* Disable user access to the timers and the physical counter */
>> +	/* Disable user access to the timers and both counters */
>>  	/* Also disable virtual event stream */
>>  	cntkctl &= ~(ARCH_TIMER_USR_PT_ACCESS_EN
>>  			| ARCH_TIMER_USR_VT_ACCESS_EN
>> +		        | ARCH_TIMER_USR_VCT_ACCESS_EN
>>  			| ARCH_TIMER_VIRT_EVT_EN
>>  			| ARCH_TIMER_USR_PCT_ACCESS_EN);
>>  
>> -	/* Enable user access to the virtual counter */
>> -	cntkctl |= ARCH_TIMER_USR_VCT_ACCESS_EN;
>> +	/*
>> +	 * Enable user access to the virtual counter if it doesn't
>> +	 * need to be workaround. The vdso may have been already
>> +	 * disabled though.
>> +	 */
>> +	if (arch_timer_this_cpu_has_cntvct_wa())
>> +		pr_info("CPU%d: Trapping CNTVCT access\n", smp_processor_id());
>> +	else
>> +		cntkctl |= ARCH_TIMER_USR_VCT_ACCESS_EN;
> 
> Since CNTVCT_EL0 and CNTFRQ_EL0 share the same control register,then we
> need to trap CNTFRQ_EL0 as well.
> 
> We hit an "undefined instruction" when running Open MPI, which
> read CNTFRQ_EL0 in the user space:
> 
> https://github.com/open-mpi/ompi/blob/5b40fd267f9ddc6463051e402382a988637c3bb3/opal/include/opal/sys/arm64/timer.h
> 
> static inline opal_timer_t
> opal_sys_timer_freq(void) { opal_timer_t freq; __asm__ __volatile__
> ("mrs %0, CNTFRQ_EL0" : "=r" (freq)); return (opal_timer_t)(freq); }
> Please take look :) Thanks Hanjun

Ah, nice :-/ ... Can you please give the patch below a shot?

Thanks,

	M.

>From 3f13e53878356057bde3e023614789d6c6b73628 Mon Sep 17 00:00:00 2001
From: Marc Zyngier <marc.zyngier@arm.com>
Date: Mon, 24 Apr 2017 09:04:03 +0100
Subject: [PATCH] arm64: Add CNTFRQ_EL0 trap handler

We now trap accesses to CNTVCT_EL0 when the counter is broken
enough to require the kernel to mediate the access. But it
turns out that some existing userspace (such as OpenMPI) do
probe for the counter frequency, leading to an UNDEF exception
as CNTVCT_EL0 and CNTFRQ_EL0 share the same control bit.

The fix is to handle the exception the same way we do for CNTVCT_EL0.

Fixes: a86bd139f2ae ("arm64: arch_timer: Enable CNTVCT_EL0 trap if workaround is enabled")
Reported-by: Hanjun Guo <guohanjun@huawei.com>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm64/include/asm/esr.h |  4 ++++
 arch/arm64/kernel/traps.c    | 14 ++++++++++++++
 2 files changed, 18 insertions(+)

diff --git a/arch/arm64/include/asm/esr.h b/arch/arm64/include/asm/esr.h
index ad42e79a5d4d..8ea134f88fda 100644
--- a/arch/arm64/include/asm/esr.h
+++ b/arch/arm64/include/asm/esr.h
@@ -177,6 +177,10 @@
 
 #define ESR_ELx_SYS64_ISS_SYS_CNTVCT	(ESR_ELx_SYS64_ISS_SYS_VAL(3, 3, 2, 14, 0) | \
 					 ESR_ELx_SYS64_ISS_DIR_READ)
+
+#define ESR_ELx_SYS64_ISS_SYS_CNTFRQ	(ESR_ELx_SYS64_ISS_SYS_VAL(3, 3, 14, 0, 0) | \
+					 ESR_ELx_SYS64_ISS_DIR_READ)
+
 #ifndef __ASSEMBLY__
 #include <asm/types.h>
 
diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
index 1de444e6c669..d4d6ae02cd55 100644
--- a/arch/arm64/kernel/traps.c
+++ b/arch/arm64/kernel/traps.c
@@ -513,6 +513,14 @@ static void cntvct_read_handler(unsigned int esr, struct pt_regs *regs)
 	regs->pc += 4;
 }
 
+static void cntfrq_read_handler(unsigned int esr, struct pt_regs *regs)
+{
+	int rt = (esr & ESR_ELx_SYS64_ISS_RT_MASK) >> ESR_ELx_SYS64_ISS_RT_SHIFT;
+
+	pt_regs_write_reg(regs, rt, read_sysreg(cntfrq_el0));
+	regs->pc += 4;
+}
+
 struct sys64_hook {
 	unsigned int esr_mask;
 	unsigned int esr_val;
@@ -537,6 +545,12 @@ static struct sys64_hook sys64_hooks[] = {
 		.esr_val = ESR_ELx_SYS64_ISS_SYS_CNTVCT,
 		.handler = cntvct_read_handler,
 	},
+	{
+		/* Trap read access to CNTFRQ_EL0 */
+		.esr_mask = ESR_ELx_SYS64_ISS_SYS_OP_MASK,
+		.esr_val = ESR_ELx_SYS64_ISS_SYS_CNTFRQ,
+		.handler = cntfrq_read_handler,
+	},
 	{},
 };
 
-- 
2.11.0

-- 
Jazz is not dead. It just smells funny...

^ permalink raw reply related	[flat|nested] 59+ messages in thread

* RE: [PATCH v3 15/18] arm64: arch_timer: Enable CNTVCT_EL0 trap if workaround is enabled
  2017-04-24  8:13       ` Marc Zyngier
@ 2017-04-24  8:18         ` Lixiaoping (Timmy)
  -1 siblings, 0 replies; 59+ messages in thread
From: Lixiaoping (Timmy) @ 2017-04-24  8:18 UTC (permalink / raw)
  To: Marc Zyngier, Guohanjun (Hanjun Guo), linux-arm-kernel, linux-kernel
  Cc: Mark Rutland, Catalin Marinas, Daniel Lezcano, Will Deacon,
	Scott Wood, Hanjun Guo, Dingtianhong, dann frazier,
	Thomas Gleixner

Hi Marc,

+#define ESR_ELx_SYS64_ISS_SYS_CNTFRQ	(ESR_ELx_SYS64_ISS_SYS_VAL(3, 3, 14, 0, 0) | \
+					 ESR_ELx_SYS64_ISS_DIR_READ)

I think (3, 3, 14, 0, 0) should be (3, 3, 0, 14, 0)?

-----------------------------------------------------------------------------
Best Regards,

Timmy Li (Lixiaoping)
Turing Architecture and Design Dept, Hisilicon

-----------------------------------------------------------------------------
This e-mail and its attachments contain confidential information from HUAWEI, which is intended only for the person or entity whose address is listed above. Any use of the information contained herein in any way (including, but not limited to, total or partial disclosure, reproduction, or dissemination) by persons other than the intended recipient(s) is prohibited. If you receive this e-mail in error, please notify the sender by phone or email immediately and delete it!

-----Original Message-----
From: Marc Zyngier [mailto:marc.zyngier@arm.com] 
Sent: Monday, April 24, 2017 4:13 PM
To: Guohanjun (Hanjun Guo) <guohanjun@huawei.com>; linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org
Cc: Mark Rutland <mark.rutland@arm.com>; Catalin Marinas <catalin.marinas@arm.com>; Daniel Lezcano <daniel.lezcano@linaro.org>; Will Deacon <will.deacon@arm.com>; Scott Wood <oss@buserror.net>; Hanjun Guo <hanjun.guo@linaro.org>; Dingtianhong <dingtianhong@huawei.com>; dann frazier <dann.frazier@canonical.com>; Thomas Gleixner <tglx@linutronix.de>; Lixiaoping (Timmy) <lixiaoping3@huawei.com>
Subject: Re: [PATCH v3 15/18] arm64: arch_timer: Enable CNTVCT_EL0 trap if workaround is enabled

On 24/04/17 08:59, Hanjun Guo wrote:
> Hi Marc,
> 
> On 2017/4/5 1:18, Marc Zyngier wrote:
>> Userspace being allowed to use read CNTVCT_EL0 anytime (and not
>> only in the VDSO), we need to enable trapping whenever a cntvct
>> workaround is enabled on a given CPU.
>>
>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
>> ---
>>  drivers/clocksource/arm_arch_timer.c | 45 +++++++++++++++++++++++++-----------
>>  1 file changed, 32 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
> [...]
>>  #else
>>  #define arch_timer_check_ool_workaround(t,a)		do { } while(0)
>>  #define erratum_set_next_event_tval_virt(...)		({BUG(); 0;})
>>  #define erratum_set_next_event_tval_phys(...)		({BUG(); 0;})
>>  #define erratum_handler(fn, r, ...)			({false;})
>> +#define arch_timer_this_cpu_has_cntvct_wa()		({false;})
>>  #endif /* CONFIG_ARM_ARCH_TIMER_OOL_WORKAROUND */
>>  
>>  static __always_inline irqreturn_t timer_handler(const int access,
>> @@ -660,15 +680,23 @@ static void arch_counter_set_user_access(void)
>>  {
>>  	u32 cntkctl = arch_timer_get_cntkctl();
>>  
>> -	/* Disable user access to the timers and the physical counter */
>> +	/* Disable user access to the timers and both counters */
>>  	/* Also disable virtual event stream */
>>  	cntkctl &= ~(ARCH_TIMER_USR_PT_ACCESS_EN
>>  			| ARCH_TIMER_USR_VT_ACCESS_EN
>> +		        | ARCH_TIMER_USR_VCT_ACCESS_EN
>>  			| ARCH_TIMER_VIRT_EVT_EN
>>  			| ARCH_TIMER_USR_PCT_ACCESS_EN);
>>  
>> -	/* Enable user access to the virtual counter */
>> -	cntkctl |= ARCH_TIMER_USR_VCT_ACCESS_EN;
>> +	/*
>> +	 * Enable user access to the virtual counter if it doesn't
>> +	 * need to be workaround. The vdso may have been already
>> +	 * disabled though.
>> +	 */
>> +	if (arch_timer_this_cpu_has_cntvct_wa())
>> +		pr_info("CPU%d: Trapping CNTVCT access\n", smp_processor_id());
>> +	else
>> +		cntkctl |= ARCH_TIMER_USR_VCT_ACCESS_EN;
> 
> Since CNTVCT_EL0 and CNTFRQ_EL0 share the same control register,then we
> need to trap CNTFRQ_EL0 as well.
> 
> We hit an "undefined instruction" when running Open MPI, which
> read CNTFRQ_EL0 in the user space:
> 
> https://github.com/open-mpi/ompi/blob/5b40fd267f9ddc6463051e402382a988637c3bb3/opal/include/opal/sys/arm64/timer.h
> 
> static inline opal_timer_t
> opal_sys_timer_freq(void) { opal_timer_t freq; __asm__ __volatile__
> ("mrs %0, CNTFRQ_EL0" : "=r" (freq)); return (opal_timer_t)(freq); }
> Please take look :) Thanks Hanjun

Ah, nice :-/ ... Can you please give the patch below a shot?

Thanks,

	M.

>From 3f13e53878356057bde3e023614789d6c6b73628 Mon Sep 17 00:00:00 2001
From: Marc Zyngier <marc.zyngier@arm.com>
Date: Mon, 24 Apr 2017 09:04:03 +0100
Subject: [PATCH] arm64: Add CNTFRQ_EL0 trap handler

We now trap accesses to CNTVCT_EL0 when the counter is broken
enough to require the kernel to mediate the access. But it
turns out that some existing userspace (such as OpenMPI) do
probe for the counter frequency, leading to an UNDEF exception
as CNTVCT_EL0 and CNTFRQ_EL0 share the same control bit.

The fix is to handle the exception the same way we do for CNTVCT_EL0.

Fixes: a86bd139f2ae ("arm64: arch_timer: Enable CNTVCT_EL0 trap if workaround is enabled")
Reported-by: Hanjun Guo <guohanjun@huawei.com>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm64/include/asm/esr.h |  4 ++++
 arch/arm64/kernel/traps.c    | 14 ++++++++++++++
 2 files changed, 18 insertions(+)

diff --git a/arch/arm64/include/asm/esr.h b/arch/arm64/include/asm/esr.h
index ad42e79a5d4d..8ea134f88fda 100644
--- a/arch/arm64/include/asm/esr.h
+++ b/arch/arm64/include/asm/esr.h
@@ -177,6 +177,10 @@
 
 #define ESR_ELx_SYS64_ISS_SYS_CNTVCT	(ESR_ELx_SYS64_ISS_SYS_VAL(3, 3, 2, 14, 0) | \
 					 ESR_ELx_SYS64_ISS_DIR_READ)
+
+#define ESR_ELx_SYS64_ISS_SYS_CNTFRQ	(ESR_ELx_SYS64_ISS_SYS_VAL(3, 3, 14, 0, 0) | \
+					 ESR_ELx_SYS64_ISS_DIR_READ)
+
 #ifndef __ASSEMBLY__
 #include <asm/types.h>
 
diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
index 1de444e6c669..d4d6ae02cd55 100644
--- a/arch/arm64/kernel/traps.c
+++ b/arch/arm64/kernel/traps.c
@@ -513,6 +513,14 @@ static void cntvct_read_handler(unsigned int esr, struct pt_regs *regs)
 	regs->pc += 4;
 }
 
+static void cntfrq_read_handler(unsigned int esr, struct pt_regs *regs)
+{
+	int rt = (esr & ESR_ELx_SYS64_ISS_RT_MASK) >> ESR_ELx_SYS64_ISS_RT_SHIFT;
+
+	pt_regs_write_reg(regs, rt, read_sysreg(cntfrq_el0));
+	regs->pc += 4;
+}
+
 struct sys64_hook {
 	unsigned int esr_mask;
 	unsigned int esr_val;
@@ -537,6 +545,12 @@ static struct sys64_hook sys64_hooks[] = {
 		.esr_val = ESR_ELx_SYS64_ISS_SYS_CNTVCT,
 		.handler = cntvct_read_handler,
 	},
+	{
+		/* Trap read access to CNTFRQ_EL0 */
+		.esr_mask = ESR_ELx_SYS64_ISS_SYS_OP_MASK,
+		.esr_val = ESR_ELx_SYS64_ISS_SYS_CNTFRQ,
+		.handler = cntfrq_read_handler,
+	},
 	{},
 };
 
-- 
2.11.0

-- 
Jazz is not dead. It just smells funny...

^ permalink raw reply related	[flat|nested] 59+ messages in thread

* [PATCH v3 15/18] arm64: arch_timer: Enable CNTVCT_EL0 trap if workaround is enabled
@ 2017-04-24  8:18         ` Lixiaoping (Timmy)
  0 siblings, 0 replies; 59+ messages in thread
From: Lixiaoping (Timmy) @ 2017-04-24  8:18 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Marc,

+#define ESR_ELx_SYS64_ISS_SYS_CNTFRQ	(ESR_ELx_SYS64_ISS_SYS_VAL(3, 3, 14, 0, 0) | \
+					 ESR_ELx_SYS64_ISS_DIR_READ)

I think (3, 3, 14, 0, 0) should be (3, 3, 0, 14, 0)?

-----------------------------------------------------------------------------
Best Regards,

Timmy Li (Lixiaoping)
Turing Architecture and Design Dept, Hisilicon

-----------------------------------------------------------------------------
This e-mail and its attachments contain confidential information from HUAWEI, which is intended only for the person or entity whose address is listed above. Any use of the information contained herein in any way (including, but not limited to, total or partial disclosure, reproduction, or dissemination) by persons other than the intended recipient(s) is prohibited. If you receive this e-mail in error, please notify the sender by phone or email immediately and delete it!

-----Original Message-----
From: Marc Zyngier [mailto:marc.zyngier at arm.com] 
Sent: Monday, April 24, 2017 4:13 PM
To: Guohanjun (Hanjun Guo) <guohanjun@huawei.com>; linux-arm-kernel at lists.infradead.org; linux-kernel at vger.kernel.org
Cc: Mark Rutland <mark.rutland@arm.com>; Catalin Marinas <catalin.marinas@arm.com>; Daniel Lezcano <daniel.lezcano@linaro.org>; Will Deacon <will.deacon@arm.com>; Scott Wood <oss@buserror.net>; Hanjun Guo <hanjun.guo@linaro.org>; Dingtianhong <dingtianhong@huawei.com>; dann frazier <dann.frazier@canonical.com>; Thomas Gleixner <tglx@linutronix.de>; Lixiaoping (Timmy) <lixiaoping3@huawei.com>
Subject: Re: [PATCH v3 15/18] arm64: arch_timer: Enable CNTVCT_EL0 trap if workaround is enabled

On 24/04/17 08:59, Hanjun Guo wrote:
> Hi Marc,
> 
> On 2017/4/5 1:18, Marc Zyngier wrote:
>> Userspace being allowed to use read CNTVCT_EL0 anytime (and not
>> only in the VDSO), we need to enable trapping whenever a cntvct
>> workaround is enabled on a given CPU.
>>
>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
>> ---
>>  drivers/clocksource/arm_arch_timer.c | 45 +++++++++++++++++++++++++-----------
>>  1 file changed, 32 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
> [...]
>>  #else
>>  #define arch_timer_check_ool_workaround(t,a)		do { } while(0)
>>  #define erratum_set_next_event_tval_virt(...)		({BUG(); 0;})
>>  #define erratum_set_next_event_tval_phys(...)		({BUG(); 0;})
>>  #define erratum_handler(fn, r, ...)			({false;})
>> +#define arch_timer_this_cpu_has_cntvct_wa()		({false;})
>>  #endif /* CONFIG_ARM_ARCH_TIMER_OOL_WORKAROUND */
>>  
>>  static __always_inline irqreturn_t timer_handler(const int access,
>> @@ -660,15 +680,23 @@ static void arch_counter_set_user_access(void)
>>  {
>>  	u32 cntkctl = arch_timer_get_cntkctl();
>>  
>> -	/* Disable user access to the timers and the physical counter */
>> +	/* Disable user access to the timers and both counters */
>>  	/* Also disable virtual event stream */
>>  	cntkctl &= ~(ARCH_TIMER_USR_PT_ACCESS_EN
>>  			| ARCH_TIMER_USR_VT_ACCESS_EN
>> +		        | ARCH_TIMER_USR_VCT_ACCESS_EN
>>  			| ARCH_TIMER_VIRT_EVT_EN
>>  			| ARCH_TIMER_USR_PCT_ACCESS_EN);
>>  
>> -	/* Enable user access to the virtual counter */
>> -	cntkctl |= ARCH_TIMER_USR_VCT_ACCESS_EN;
>> +	/*
>> +	 * Enable user access to the virtual counter if it doesn't
>> +	 * need to be workaround. The vdso may have been already
>> +	 * disabled though.
>> +	 */
>> +	if (arch_timer_this_cpu_has_cntvct_wa())
>> +		pr_info("CPU%d: Trapping CNTVCT access\n", smp_processor_id());
>> +	else
>> +		cntkctl |= ARCH_TIMER_USR_VCT_ACCESS_EN;
> 
> Since CNTVCT_EL0 and CNTFRQ_EL0 share the same control register,then we
> need to trap CNTFRQ_EL0 as well.
> 
> We hit an "undefined instruction" when running Open MPI, which
> read CNTFRQ_EL0 in the user space:
> 
> https://github.com/open-mpi/ompi/blob/5b40fd267f9ddc6463051e402382a988637c3bb3/opal/include/opal/sys/arm64/timer.h
> 
> static inline opal_timer_t
> opal_sys_timer_freq(void) { opal_timer_t freq; __asm__ __volatile__
> ("mrs %0, CNTFRQ_EL0" : "=r" (freq)); return (opal_timer_t)(freq); }
> Please take look :) Thanks Hanjun

Ah, nice :-/ ... Can you please give the patch below a shot?

Thanks,

	M.

>From 3f13e53878356057bde3e023614789d6c6b73628 Mon Sep 17 00:00:00 2001
From: Marc Zyngier <marc.zyngier@arm.com>
Date: Mon, 24 Apr 2017 09:04:03 +0100
Subject: [PATCH] arm64: Add CNTFRQ_EL0 trap handler

We now trap accesses to CNTVCT_EL0 when the counter is broken
enough to require the kernel to mediate the access. But it
turns out that some existing userspace (such as OpenMPI) do
probe for the counter frequency, leading to an UNDEF exception
as CNTVCT_EL0 and CNTFRQ_EL0 share the same control bit.

The fix is to handle the exception the same way we do for CNTVCT_EL0.

Fixes: a86bd139f2ae ("arm64: arch_timer: Enable CNTVCT_EL0 trap if workaround is enabled")
Reported-by: Hanjun Guo <guohanjun@huawei.com>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm64/include/asm/esr.h |  4 ++++
 arch/arm64/kernel/traps.c    | 14 ++++++++++++++
 2 files changed, 18 insertions(+)

diff --git a/arch/arm64/include/asm/esr.h b/arch/arm64/include/asm/esr.h
index ad42e79a5d4d..8ea134f88fda 100644
--- a/arch/arm64/include/asm/esr.h
+++ b/arch/arm64/include/asm/esr.h
@@ -177,6 +177,10 @@
 
 #define ESR_ELx_SYS64_ISS_SYS_CNTVCT	(ESR_ELx_SYS64_ISS_SYS_VAL(3, 3, 2, 14, 0) | \
 					 ESR_ELx_SYS64_ISS_DIR_READ)
+
+#define ESR_ELx_SYS64_ISS_SYS_CNTFRQ	(ESR_ELx_SYS64_ISS_SYS_VAL(3, 3, 14, 0, 0) | \
+					 ESR_ELx_SYS64_ISS_DIR_READ)
+
 #ifndef __ASSEMBLY__
 #include <asm/types.h>
 
diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
index 1de444e6c669..d4d6ae02cd55 100644
--- a/arch/arm64/kernel/traps.c
+++ b/arch/arm64/kernel/traps.c
@@ -513,6 +513,14 @@ static void cntvct_read_handler(unsigned int esr, struct pt_regs *regs)
 	regs->pc += 4;
 }
 
+static void cntfrq_read_handler(unsigned int esr, struct pt_regs *regs)
+{
+	int rt = (esr & ESR_ELx_SYS64_ISS_RT_MASK) >> ESR_ELx_SYS64_ISS_RT_SHIFT;
+
+	pt_regs_write_reg(regs, rt, read_sysreg(cntfrq_el0));
+	regs->pc += 4;
+}
+
 struct sys64_hook {
 	unsigned int esr_mask;
 	unsigned int esr_val;
@@ -537,6 +545,12 @@ static struct sys64_hook sys64_hooks[] = {
 		.esr_val = ESR_ELx_SYS64_ISS_SYS_CNTVCT,
 		.handler = cntvct_read_handler,
 	},
+	{
+		/* Trap read access to CNTFRQ_EL0 */
+		.esr_mask = ESR_ELx_SYS64_ISS_SYS_OP_MASK,
+		.esr_val = ESR_ELx_SYS64_ISS_SYS_CNTFRQ,
+		.handler = cntfrq_read_handler,
+	},
 	{},
 };
 
-- 
2.11.0

-- 
Jazz is not dead. It just smells funny...

^ permalink raw reply related	[flat|nested] 59+ messages in thread

* RE: [PATCH v3 15/18] arm64: arch_timer: Enable CNTVCT_EL0 trap if workaround is enabled
  2017-04-24  8:13       ` Marc Zyngier
@ 2017-04-24  8:25         ` Lixiaoping (Timmy)
  -1 siblings, 0 replies; 59+ messages in thread
From: Lixiaoping (Timmy) @ 2017-04-24  8:25 UTC (permalink / raw)
  To: Marc Zyngier, Guohanjun (Hanjun Guo), linux-arm-kernel, linux-kernel
  Cc: Mark Rutland, Catalin Marinas, Daniel Lezcano, Will Deacon,
	Scott Wood, Hanjun Guo, Dingtianhong, dann frazier,
	Thomas Gleixner

Hi Marc,

Sorry about previous email's confidential info. Please forget it.

+#define ESR_ELx_SYS64_ISS_SYS_CNTFRQ	(ESR_ELx_SYS64_ISS_SYS_VAL(3, 3, 14, 0, 0) | \
+					 ESR_ELx_SYS64_ISS_DIR_READ)

I think (3, 3, 14, 0, 0) should be (3, 3, 0, 14, 0)?

-----------------------------------------------------------------------------
Best Regards,

Timmy Li (Lixiaoping)

-----Original Message-----
From: Marc Zyngier [mailto:marc.zyngier@arm.com] 
Sent: Monday, April 24, 2017 4:13 PM
To: Guohanjun (Hanjun Guo) <guohanjun@huawei.com>; linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org
Cc: Mark Rutland <mark.rutland@arm.com>; Catalin Marinas <catalin.marinas@arm.com>; Daniel Lezcano <daniel.lezcano@linaro.org>; Will Deacon <will.deacon@arm.com>; Scott Wood <oss@buserror.net>; Hanjun Guo <hanjun.guo@linaro.org>; Dingtianhong <dingtianhong@huawei.com>; dann frazier <dann.frazier@canonical.com>; Thomas Gleixner <tglx@linutronix.de>; Lixiaoping (Timmy) <lixiaoping3@huawei.com>
Subject: Re: [PATCH v3 15/18] arm64: arch_timer: Enable CNTVCT_EL0 trap if workaround is enabled

On 24/04/17 08:59, Hanjun Guo wrote:
> Hi Marc,
> 
> On 2017/4/5 1:18, Marc Zyngier wrote:
>> Userspace being allowed to use read CNTVCT_EL0 anytime (and not
>> only in the VDSO), we need to enable trapping whenever a cntvct
>> workaround is enabled on a given CPU.
>>
>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
>> ---
>>  drivers/clocksource/arm_arch_timer.c | 45 +++++++++++++++++++++++++-----------
>>  1 file changed, 32 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
> [...]
>>  #else
>>  #define arch_timer_check_ool_workaround(t,a)		do { } while(0)
>>  #define erratum_set_next_event_tval_virt(...)		({BUG(); 0;})
>>  #define erratum_set_next_event_tval_phys(...)		({BUG(); 0;})
>>  #define erratum_handler(fn, r, ...)			({false;})
>> +#define arch_timer_this_cpu_has_cntvct_wa()		({false;})
>>  #endif /* CONFIG_ARM_ARCH_TIMER_OOL_WORKAROUND */
>>  
>>  static __always_inline irqreturn_t timer_handler(const int access,
>> @@ -660,15 +680,23 @@ static void arch_counter_set_user_access(void)
>>  {
>>  	u32 cntkctl = arch_timer_get_cntkctl();
>>  
>> -	/* Disable user access to the timers and the physical counter */
>> +	/* Disable user access to the timers and both counters */
>>  	/* Also disable virtual event stream */
>>  	cntkctl &= ~(ARCH_TIMER_USR_PT_ACCESS_EN
>>  			| ARCH_TIMER_USR_VT_ACCESS_EN
>> +		        | ARCH_TIMER_USR_VCT_ACCESS_EN
>>  			| ARCH_TIMER_VIRT_EVT_EN
>>  			| ARCH_TIMER_USR_PCT_ACCESS_EN);
>>  
>> -	/* Enable user access to the virtual counter */
>> -	cntkctl |= ARCH_TIMER_USR_VCT_ACCESS_EN;
>> +	/*
>> +	 * Enable user access to the virtual counter if it doesn't
>> +	 * need to be workaround. The vdso may have been already
>> +	 * disabled though.
>> +	 */
>> +	if (arch_timer_this_cpu_has_cntvct_wa())
>> +		pr_info("CPU%d: Trapping CNTVCT access\n", smp_processor_id());
>> +	else
>> +		cntkctl |= ARCH_TIMER_USR_VCT_ACCESS_EN;
> 
> Since CNTVCT_EL0 and CNTFRQ_EL0 share the same control register,then we
> need to trap CNTFRQ_EL0 as well.
> 
> We hit an "undefined instruction" when running Open MPI, which
> read CNTFRQ_EL0 in the user space:
> 
> https://github.com/open-mpi/ompi/blob/5b40fd267f9ddc6463051e402382a988637c3bb3/opal/include/opal/sys/arm64/timer.h
> 
> static inline opal_timer_t
> opal_sys_timer_freq(void) { opal_timer_t freq; __asm__ __volatile__
> ("mrs %0, CNTFRQ_EL0" : "=r" (freq)); return (opal_timer_t)(freq); }
> Please take look :) Thanks Hanjun

Ah, nice :-/ ... Can you please give the patch below a shot?

Thanks,

	M.

>From 3f13e53878356057bde3e023614789d6c6b73628 Mon Sep 17 00:00:00 2001
From: Marc Zyngier <marc.zyngier@arm.com>
Date: Mon, 24 Apr 2017 09:04:03 +0100
Subject: [PATCH] arm64: Add CNTFRQ_EL0 trap handler

We now trap accesses to CNTVCT_EL0 when the counter is broken
enough to require the kernel to mediate the access. But it
turns out that some existing userspace (such as OpenMPI) do
probe for the counter frequency, leading to an UNDEF exception
as CNTVCT_EL0 and CNTFRQ_EL0 share the same control bit.

The fix is to handle the exception the same way we do for CNTVCT_EL0.

Fixes: a86bd139f2ae ("arm64: arch_timer: Enable CNTVCT_EL0 trap if workaround is enabled")
Reported-by: Hanjun Guo <guohanjun@huawei.com>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm64/include/asm/esr.h |  4 ++++
 arch/arm64/kernel/traps.c    | 14 ++++++++++++++
 2 files changed, 18 insertions(+)

diff --git a/arch/arm64/include/asm/esr.h b/arch/arm64/include/asm/esr.h
index ad42e79a5d4d..8ea134f88fda 100644
--- a/arch/arm64/include/asm/esr.h
+++ b/arch/arm64/include/asm/esr.h
@@ -177,6 +177,10 @@
 
 #define ESR_ELx_SYS64_ISS_SYS_CNTVCT	(ESR_ELx_SYS64_ISS_SYS_VAL(3, 3, 2, 14, 0) | \
 					 ESR_ELx_SYS64_ISS_DIR_READ)
+
+#define ESR_ELx_SYS64_ISS_SYS_CNTFRQ	(ESR_ELx_SYS64_ISS_SYS_VAL(3, 3, 14, 0, 0) | \
+					 ESR_ELx_SYS64_ISS_DIR_READ)
+
 #ifndef __ASSEMBLY__
 #include <asm/types.h>
 
diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
index 1de444e6c669..d4d6ae02cd55 100644
--- a/arch/arm64/kernel/traps.c
+++ b/arch/arm64/kernel/traps.c
@@ -513,6 +513,14 @@ static void cntvct_read_handler(unsigned int esr, struct pt_regs *regs)
 	regs->pc += 4;
 }
 
+static void cntfrq_read_handler(unsigned int esr, struct pt_regs *regs)
+{
+	int rt = (esr & ESR_ELx_SYS64_ISS_RT_MASK) >> ESR_ELx_SYS64_ISS_RT_SHIFT;
+
+	pt_regs_write_reg(regs, rt, read_sysreg(cntfrq_el0));
+	regs->pc += 4;
+}
+
 struct sys64_hook {
 	unsigned int esr_mask;
 	unsigned int esr_val;
@@ -537,6 +545,12 @@ static struct sys64_hook sys64_hooks[] = {
 		.esr_val = ESR_ELx_SYS64_ISS_SYS_CNTVCT,
 		.handler = cntvct_read_handler,
 	},
+	{
+		/* Trap read access to CNTFRQ_EL0 */
+		.esr_mask = ESR_ELx_SYS64_ISS_SYS_OP_MASK,
+		.esr_val = ESR_ELx_SYS64_ISS_SYS_CNTFRQ,
+		.handler = cntfrq_read_handler,
+	},
 	{},
 };
 
-- 
2.11.0

-- 
Jazz is not dead. It just smells funny...

^ permalink raw reply related	[flat|nested] 59+ messages in thread

* [PATCH v3 15/18] arm64: arch_timer: Enable CNTVCT_EL0 trap if workaround is enabled
@ 2017-04-24  8:25         ` Lixiaoping (Timmy)
  0 siblings, 0 replies; 59+ messages in thread
From: Lixiaoping (Timmy) @ 2017-04-24  8:25 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Marc,

Sorry about previous email's confidential info. Please forget it.

+#define ESR_ELx_SYS64_ISS_SYS_CNTFRQ	(ESR_ELx_SYS64_ISS_SYS_VAL(3, 3, 14, 0, 0) | \
+					 ESR_ELx_SYS64_ISS_DIR_READ)

I think (3, 3, 14, 0, 0) should be (3, 3, 0, 14, 0)?

-----------------------------------------------------------------------------
Best Regards,

Timmy Li (Lixiaoping)

-----Original Message-----
From: Marc Zyngier [mailto:marc.zyngier at arm.com] 
Sent: Monday, April 24, 2017 4:13 PM
To: Guohanjun (Hanjun Guo) <guohanjun@huawei.com>; linux-arm-kernel at lists.infradead.org; linux-kernel at vger.kernel.org
Cc: Mark Rutland <mark.rutland@arm.com>; Catalin Marinas <catalin.marinas@arm.com>; Daniel Lezcano <daniel.lezcano@linaro.org>; Will Deacon <will.deacon@arm.com>; Scott Wood <oss@buserror.net>; Hanjun Guo <hanjun.guo@linaro.org>; Dingtianhong <dingtianhong@huawei.com>; dann frazier <dann.frazier@canonical.com>; Thomas Gleixner <tglx@linutronix.de>; Lixiaoping (Timmy) <lixiaoping3@huawei.com>
Subject: Re: [PATCH v3 15/18] arm64: arch_timer: Enable CNTVCT_EL0 trap if workaround is enabled

On 24/04/17 08:59, Hanjun Guo wrote:
> Hi Marc,
> 
> On 2017/4/5 1:18, Marc Zyngier wrote:
>> Userspace being allowed to use read CNTVCT_EL0 anytime (and not
>> only in the VDSO), we need to enable trapping whenever a cntvct
>> workaround is enabled on a given CPU.
>>
>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
>> ---
>>  drivers/clocksource/arm_arch_timer.c | 45 +++++++++++++++++++++++++-----------
>>  1 file changed, 32 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
> [...]
>>  #else
>>  #define arch_timer_check_ool_workaround(t,a)		do { } while(0)
>>  #define erratum_set_next_event_tval_virt(...)		({BUG(); 0;})
>>  #define erratum_set_next_event_tval_phys(...)		({BUG(); 0;})
>>  #define erratum_handler(fn, r, ...)			({false;})
>> +#define arch_timer_this_cpu_has_cntvct_wa()		({false;})
>>  #endif /* CONFIG_ARM_ARCH_TIMER_OOL_WORKAROUND */
>>  
>>  static __always_inline irqreturn_t timer_handler(const int access,
>> @@ -660,15 +680,23 @@ static void arch_counter_set_user_access(void)
>>  {
>>  	u32 cntkctl = arch_timer_get_cntkctl();
>>  
>> -	/* Disable user access to the timers and the physical counter */
>> +	/* Disable user access to the timers and both counters */
>>  	/* Also disable virtual event stream */
>>  	cntkctl &= ~(ARCH_TIMER_USR_PT_ACCESS_EN
>>  			| ARCH_TIMER_USR_VT_ACCESS_EN
>> +		        | ARCH_TIMER_USR_VCT_ACCESS_EN
>>  			| ARCH_TIMER_VIRT_EVT_EN
>>  			| ARCH_TIMER_USR_PCT_ACCESS_EN);
>>  
>> -	/* Enable user access to the virtual counter */
>> -	cntkctl |= ARCH_TIMER_USR_VCT_ACCESS_EN;
>> +	/*
>> +	 * Enable user access to the virtual counter if it doesn't
>> +	 * need to be workaround. The vdso may have been already
>> +	 * disabled though.
>> +	 */
>> +	if (arch_timer_this_cpu_has_cntvct_wa())
>> +		pr_info("CPU%d: Trapping CNTVCT access\n", smp_processor_id());
>> +	else
>> +		cntkctl |= ARCH_TIMER_USR_VCT_ACCESS_EN;
> 
> Since CNTVCT_EL0 and CNTFRQ_EL0 share the same control register,then we
> need to trap CNTFRQ_EL0 as well.
> 
> We hit an "undefined instruction" when running Open MPI, which
> read CNTFRQ_EL0 in the user space:
> 
> https://github.com/open-mpi/ompi/blob/5b40fd267f9ddc6463051e402382a988637c3bb3/opal/include/opal/sys/arm64/timer.h
> 
> static inline opal_timer_t
> opal_sys_timer_freq(void) { opal_timer_t freq; __asm__ __volatile__
> ("mrs %0, CNTFRQ_EL0" : "=r" (freq)); return (opal_timer_t)(freq); }
> Please take look :) Thanks Hanjun

Ah, nice :-/ ... Can you please give the patch below a shot?

Thanks,

	M.

>From 3f13e53878356057bde3e023614789d6c6b73628 Mon Sep 17 00:00:00 2001
From: Marc Zyngier <marc.zyngier@arm.com>
Date: Mon, 24 Apr 2017 09:04:03 +0100
Subject: [PATCH] arm64: Add CNTFRQ_EL0 trap handler

We now trap accesses to CNTVCT_EL0 when the counter is broken
enough to require the kernel to mediate the access. But it
turns out that some existing userspace (such as OpenMPI) do
probe for the counter frequency, leading to an UNDEF exception
as CNTVCT_EL0 and CNTFRQ_EL0 share the same control bit.

The fix is to handle the exception the same way we do for CNTVCT_EL0.

Fixes: a86bd139f2ae ("arm64: arch_timer: Enable CNTVCT_EL0 trap if workaround is enabled")
Reported-by: Hanjun Guo <guohanjun@huawei.com>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm64/include/asm/esr.h |  4 ++++
 arch/arm64/kernel/traps.c    | 14 ++++++++++++++
 2 files changed, 18 insertions(+)

diff --git a/arch/arm64/include/asm/esr.h b/arch/arm64/include/asm/esr.h
index ad42e79a5d4d..8ea134f88fda 100644
--- a/arch/arm64/include/asm/esr.h
+++ b/arch/arm64/include/asm/esr.h
@@ -177,6 +177,10 @@
 
 #define ESR_ELx_SYS64_ISS_SYS_CNTVCT	(ESR_ELx_SYS64_ISS_SYS_VAL(3, 3, 2, 14, 0) | \
 					 ESR_ELx_SYS64_ISS_DIR_READ)
+
+#define ESR_ELx_SYS64_ISS_SYS_CNTFRQ	(ESR_ELx_SYS64_ISS_SYS_VAL(3, 3, 14, 0, 0) | \
+					 ESR_ELx_SYS64_ISS_DIR_READ)
+
 #ifndef __ASSEMBLY__
 #include <asm/types.h>
 
diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
index 1de444e6c669..d4d6ae02cd55 100644
--- a/arch/arm64/kernel/traps.c
+++ b/arch/arm64/kernel/traps.c
@@ -513,6 +513,14 @@ static void cntvct_read_handler(unsigned int esr, struct pt_regs *regs)
 	regs->pc += 4;
 }
 
+static void cntfrq_read_handler(unsigned int esr, struct pt_regs *regs)
+{
+	int rt = (esr & ESR_ELx_SYS64_ISS_RT_MASK) >> ESR_ELx_SYS64_ISS_RT_SHIFT;
+
+	pt_regs_write_reg(regs, rt, read_sysreg(cntfrq_el0));
+	regs->pc += 4;
+}
+
 struct sys64_hook {
 	unsigned int esr_mask;
 	unsigned int esr_val;
@@ -537,6 +545,12 @@ static struct sys64_hook sys64_hooks[] = {
 		.esr_val = ESR_ELx_SYS64_ISS_SYS_CNTVCT,
 		.handler = cntvct_read_handler,
 	},
+	{
+		/* Trap read access to CNTFRQ_EL0 */
+		.esr_mask = ESR_ELx_SYS64_ISS_SYS_OP_MASK,
+		.esr_val = ESR_ELx_SYS64_ISS_SYS_CNTFRQ,
+		.handler = cntfrq_read_handler,
+	},
 	{},
 };
 
-- 
2.11.0

-- 
Jazz is not dead. It just smells funny...

^ permalink raw reply related	[flat|nested] 59+ messages in thread

* Re: [PATCH v3 15/18] arm64: arch_timer: Enable CNTVCT_EL0 trap if workaround is enabled
  2017-04-24  8:25         ` Lixiaoping (Timmy)
@ 2017-04-24  8:40           ` Marc Zyngier
  -1 siblings, 0 replies; 59+ messages in thread
From: Marc Zyngier @ 2017-04-24  8:40 UTC (permalink / raw)
  To: Lixiaoping (Timmy), Guohanjun (Hanjun Guo),
	linux-arm-kernel, linux-kernel
  Cc: Mark Rutland, Catalin Marinas, Daniel Lezcano, Will Deacon,
	Scott Wood, Hanjun Guo, Dingtianhong, dann frazier,
	Thomas Gleixner

On 24/04/17 09:25, Lixiaoping (Timmy) wrote:
> Hi Marc,
> 
> Sorry about previous email's confidential info. Please forget it.
> 
> +#define ESR_ELx_SYS64_ISS_SYS_CNTFRQ	(ESR_ELx_SYS64_ISS_SYS_VAL(3, 3, 14, 0, 0) | \
> +					 ESR_ELx_SYS64_ISS_DIR_READ)
> 
> I think (3, 3, 14, 0, 0) should be (3, 3, 0, 14, 0)?

Thanks for spotting this. I assumed that the sys_reg() and
SR_ELx_SYS64_ISS_SYS_VAL() macros took their arguments in the same 
order. That would have been too easy... ;-)

Amended patch below, please let me know if it works for you.

Thanks,

	M.

>From 4444c86a97c1a487e12c319fdc197c88631d72b5 Mon Sep 17 00:00:00 2001
From: Marc Zyngier <marc.zyngier@arm.com>
Date: Mon, 24 Apr 2017 09:04:03 +0100
Subject: [PATCH] arm64: Add CNTFRQ_EL0 trap handler

We now trap accesses to CNTVCT_EL0 when the counter is broken
enough to require the kernel to mediate the access. But it
turns out that some existing userspace (such as OpenMPI) do
probe for the counter frequency, leading to an UNDEF exception
as CNTVCT_EL0 and CNTFRQ_EL0 share the same control bit.

The fix is to handle the exception the same way we do for CNTVCT_EL0.

Fixes: a86bd139f2ae ("arm64: arch_timer: Enable CNTVCT_EL0 trap if workaround is enabled")
Reported-by: Hanjun Guo <guohanjun@huawei.com>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm64/include/asm/esr.h |  4 ++++
 arch/arm64/kernel/traps.c    | 14 ++++++++++++++
 2 files changed, 18 insertions(+)

diff --git a/arch/arm64/include/asm/esr.h b/arch/arm64/include/asm/esr.h
index ad42e79a5d4d..85997c0e5443 100644
--- a/arch/arm64/include/asm/esr.h
+++ b/arch/arm64/include/asm/esr.h
@@ -177,6 +177,10 @@
 
 #define ESR_ELx_SYS64_ISS_SYS_CNTVCT	(ESR_ELx_SYS64_ISS_SYS_VAL(3, 3, 2, 14, 0) | \
 					 ESR_ELx_SYS64_ISS_DIR_READ)
+
+#define ESR_ELx_SYS64_ISS_SYS_CNTFRQ	(ESR_ELx_SYS64_ISS_SYS_VAL(3, 3, 0, 14, 0) | \
+					 ESR_ELx_SYS64_ISS_DIR_READ)
+
 #ifndef __ASSEMBLY__
 #include <asm/types.h>
 
diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
index 1de444e6c669..d4d6ae02cd55 100644
--- a/arch/arm64/kernel/traps.c
+++ b/arch/arm64/kernel/traps.c
@@ -513,6 +513,14 @@ static void cntvct_read_handler(unsigned int esr, struct pt_regs *regs)
 	regs->pc += 4;
 }
 
+static void cntfrq_read_handler(unsigned int esr, struct pt_regs *regs)
+{
+	int rt = (esr & ESR_ELx_SYS64_ISS_RT_MASK) >> ESR_ELx_SYS64_ISS_RT_SHIFT;
+
+	pt_regs_write_reg(regs, rt, read_sysreg(cntfrq_el0));
+	regs->pc += 4;
+}
+
 struct sys64_hook {
 	unsigned int esr_mask;
 	unsigned int esr_val;
@@ -537,6 +545,12 @@ static struct sys64_hook sys64_hooks[] = {
 		.esr_val = ESR_ELx_SYS64_ISS_SYS_CNTVCT,
 		.handler = cntvct_read_handler,
 	},
+	{
+		/* Trap read access to CNTFRQ_EL0 */
+		.esr_mask = ESR_ELx_SYS64_ISS_SYS_OP_MASK,
+		.esr_val = ESR_ELx_SYS64_ISS_SYS_CNTFRQ,
+		.handler = cntfrq_read_handler,
+	},
 	{},
 };
 
-- 
2.11.0


-- 
Jazz is not dead. It just smells funny...

^ permalink raw reply related	[flat|nested] 59+ messages in thread

* [PATCH v3 15/18] arm64: arch_timer: Enable CNTVCT_EL0 trap if workaround is enabled
@ 2017-04-24  8:40           ` Marc Zyngier
  0 siblings, 0 replies; 59+ messages in thread
From: Marc Zyngier @ 2017-04-24  8:40 UTC (permalink / raw)
  To: linux-arm-kernel

On 24/04/17 09:25, Lixiaoping (Timmy) wrote:
> Hi Marc,
> 
> Sorry about previous email's confidential info. Please forget it.
> 
> +#define ESR_ELx_SYS64_ISS_SYS_CNTFRQ	(ESR_ELx_SYS64_ISS_SYS_VAL(3, 3, 14, 0, 0) | \
> +					 ESR_ELx_SYS64_ISS_DIR_READ)
> 
> I think (3, 3, 14, 0, 0) should be (3, 3, 0, 14, 0)?

Thanks for spotting this. I assumed that the sys_reg() and
SR_ELx_SYS64_ISS_SYS_VAL() macros took their arguments in the same 
order. That would have been too easy... ;-)

Amended patch below, please let me know if it works for you.

Thanks,

	M.

>From 4444c86a97c1a487e12c319fdc197c88631d72b5 Mon Sep 17 00:00:00 2001
From: Marc Zyngier <marc.zyngier@arm.com>
Date: Mon, 24 Apr 2017 09:04:03 +0100
Subject: [PATCH] arm64: Add CNTFRQ_EL0 trap handler

We now trap accesses to CNTVCT_EL0 when the counter is broken
enough to require the kernel to mediate the access. But it
turns out that some existing userspace (such as OpenMPI) do
probe for the counter frequency, leading to an UNDEF exception
as CNTVCT_EL0 and CNTFRQ_EL0 share the same control bit.

The fix is to handle the exception the same way we do for CNTVCT_EL0.

Fixes: a86bd139f2ae ("arm64: arch_timer: Enable CNTVCT_EL0 trap if workaround is enabled")
Reported-by: Hanjun Guo <guohanjun@huawei.com>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm64/include/asm/esr.h |  4 ++++
 arch/arm64/kernel/traps.c    | 14 ++++++++++++++
 2 files changed, 18 insertions(+)

diff --git a/arch/arm64/include/asm/esr.h b/arch/arm64/include/asm/esr.h
index ad42e79a5d4d..85997c0e5443 100644
--- a/arch/arm64/include/asm/esr.h
+++ b/arch/arm64/include/asm/esr.h
@@ -177,6 +177,10 @@
 
 #define ESR_ELx_SYS64_ISS_SYS_CNTVCT	(ESR_ELx_SYS64_ISS_SYS_VAL(3, 3, 2, 14, 0) | \
 					 ESR_ELx_SYS64_ISS_DIR_READ)
+
+#define ESR_ELx_SYS64_ISS_SYS_CNTFRQ	(ESR_ELx_SYS64_ISS_SYS_VAL(3, 3, 0, 14, 0) | \
+					 ESR_ELx_SYS64_ISS_DIR_READ)
+
 #ifndef __ASSEMBLY__
 #include <asm/types.h>
 
diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
index 1de444e6c669..d4d6ae02cd55 100644
--- a/arch/arm64/kernel/traps.c
+++ b/arch/arm64/kernel/traps.c
@@ -513,6 +513,14 @@ static void cntvct_read_handler(unsigned int esr, struct pt_regs *regs)
 	regs->pc += 4;
 }
 
+static void cntfrq_read_handler(unsigned int esr, struct pt_regs *regs)
+{
+	int rt = (esr & ESR_ELx_SYS64_ISS_RT_MASK) >> ESR_ELx_SYS64_ISS_RT_SHIFT;
+
+	pt_regs_write_reg(regs, rt, read_sysreg(cntfrq_el0));
+	regs->pc += 4;
+}
+
 struct sys64_hook {
 	unsigned int esr_mask;
 	unsigned int esr_val;
@@ -537,6 +545,12 @@ static struct sys64_hook sys64_hooks[] = {
 		.esr_val = ESR_ELx_SYS64_ISS_SYS_CNTVCT,
 		.handler = cntvct_read_handler,
 	},
+	{
+		/* Trap read access to CNTFRQ_EL0 */
+		.esr_mask = ESR_ELx_SYS64_ISS_SYS_OP_MASK,
+		.esr_val = ESR_ELx_SYS64_ISS_SYS_CNTFRQ,
+		.handler = cntfrq_read_handler,
+	},
 	{},
 };
 
-- 
2.11.0


-- 
Jazz is not dead. It just smells funny...

^ permalink raw reply related	[flat|nested] 59+ messages in thread

* Re: [PATCH v3 15/18] arm64: arch_timer: Enable CNTVCT_EL0 trap if workaround is enabled
  2017-04-24  8:40           ` Marc Zyngier
@ 2017-04-24  9:14             ` Hanjun Guo
  -1 siblings, 0 replies; 59+ messages in thread
From: Hanjun Guo @ 2017-04-24  9:14 UTC (permalink / raw)
  To: Marc Zyngier, Lixiaoping (Timmy), linux-arm-kernel, linux-kernel
  Cc: Mark Rutland, Catalin Marinas, Daniel Lezcano, Will Deacon,
	Scott Wood, Hanjun Guo, Dingtianhong, dann frazier,
	Thomas Gleixner

On 2017/4/24 16:40, Marc Zyngier wrote:
> On 24/04/17 09:25, Lixiaoping (Timmy) wrote:
>> Hi Marc,
>>
>> Sorry about previous email's confidential info. Please forget it.
>>
>> +#define ESR_ELx_SYS64_ISS_SYS_CNTFRQ	(ESR_ELx_SYS64_ISS_SYS_VAL(3, 3, 14, 0, 0) | \
>> +					 ESR_ELx_SYS64_ISS_DIR_READ)
>>
>> I think (3, 3, 14, 0, 0) should be (3, 3, 0, 14, 0)?
> Thanks for spotting this. I assumed that the sys_reg() and
> SR_ELx_SYS64_ISS_SYS_VAL() macros took their arguments in the same 
> order. That would have been too easy... ;-)
>
> Amended patch below, please let me know if it works for you.
>
> Thanks,
>
> 	M.
>
> >From 4444c86a97c1a487e12c319fdc197c88631d72b5 Mon Sep 17 00:00:00 2001
> From: Marc Zyngier <marc.zyngier@arm.com>
> Date: Mon, 24 Apr 2017 09:04:03 +0100
> Subject: [PATCH] arm64: Add CNTFRQ_EL0 trap handler
>
> We now trap accesses to CNTVCT_EL0 when the counter is broken
> enough to require the kernel to mediate the access. But it
> turns out that some existing userspace (such as OpenMPI) do
> probe for the counter frequency, leading to an UNDEF exception
> as CNTVCT_EL0 and CNTFRQ_EL0 share the same control bit.
>
> The fix is to handle the exception the same way we do for CNTVCT_EL0.
>
> Fixes: a86bd139f2ae ("arm64: arch_timer: Enable CNTVCT_EL0 trap if workaround is enabled")
> Reported-by: Hanjun Guo <guohanjun@huawei.com>
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---

I tested this patch and the undefined instruction error is gone, I can
get the FREQ in the user space now, thank you very much for the quick
response.

Tested-by: Hanjun Guo <guohanjun@huawei.com>
Reviewed-by: Hanjun Guo <guohanjun@huawei.com>

Thanks
Hanjun

^ permalink raw reply	[flat|nested] 59+ messages in thread

* [PATCH v3 15/18] arm64: arch_timer: Enable CNTVCT_EL0 trap if workaround is enabled
@ 2017-04-24  9:14             ` Hanjun Guo
  0 siblings, 0 replies; 59+ messages in thread
From: Hanjun Guo @ 2017-04-24  9:14 UTC (permalink / raw)
  To: linux-arm-kernel

On 2017/4/24 16:40, Marc Zyngier wrote:
> On 24/04/17 09:25, Lixiaoping (Timmy) wrote:
>> Hi Marc,
>>
>> Sorry about previous email's confidential info. Please forget it.
>>
>> +#define ESR_ELx_SYS64_ISS_SYS_CNTFRQ	(ESR_ELx_SYS64_ISS_SYS_VAL(3, 3, 14, 0, 0) | \
>> +					 ESR_ELx_SYS64_ISS_DIR_READ)
>>
>> I think (3, 3, 14, 0, 0) should be (3, 3, 0, 14, 0)?
> Thanks for spotting this. I assumed that the sys_reg() and
> SR_ELx_SYS64_ISS_SYS_VAL() macros took their arguments in the same 
> order. That would have been too easy... ;-)
>
> Amended patch below, please let me know if it works for you.
>
> Thanks,
>
> 	M.
>
> >From 4444c86a97c1a487e12c319fdc197c88631d72b5 Mon Sep 17 00:00:00 2001
> From: Marc Zyngier <marc.zyngier@arm.com>
> Date: Mon, 24 Apr 2017 09:04:03 +0100
> Subject: [PATCH] arm64: Add CNTFRQ_EL0 trap handler
>
> We now trap accesses to CNTVCT_EL0 when the counter is broken
> enough to require the kernel to mediate the access. But it
> turns out that some existing userspace (such as OpenMPI) do
> probe for the counter frequency, leading to an UNDEF exception
> as CNTVCT_EL0 and CNTFRQ_EL0 share the same control bit.
>
> The fix is to handle the exception the same way we do for CNTVCT_EL0.
>
> Fixes: a86bd139f2ae ("arm64: arch_timer: Enable CNTVCT_EL0 trap if workaround is enabled")
> Reported-by: Hanjun Guo <guohanjun@huawei.com>
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---

I tested this patch and the undefined instruction error is gone, I can
get the FREQ in the user space now, thank you very much for the quick
response.

Tested-by: Hanjun Guo <guohanjun@huawei.com>
Reviewed-by: Hanjun Guo <guohanjun@huawei.com>

Thanks
Hanjun

^ permalink raw reply	[flat|nested] 59+ messages in thread

* Re: [PATCH v3 15/18] arm64: arch_timer: Enable CNTVCT_EL0 trap if workaround is enabled
  2017-04-24  9:14             ` Hanjun Guo
@ 2017-04-24  9:18               ` Daniel Lezcano
  -1 siblings, 0 replies; 59+ messages in thread
From: Daniel Lezcano @ 2017-04-24  9:18 UTC (permalink / raw)
  To: Hanjun Guo
  Cc: Marc Zyngier, Lixiaoping (Timmy),
	linux-arm-kernel, linux-kernel, Mark Rutland, Catalin Marinas,
	Will Deacon, Scott Wood, Hanjun Guo, Dingtianhong, dann frazier,
	Thomas Gleixner

On Mon, Apr 24, 2017 at 05:14:29PM +0800, Hanjun Guo wrote:
> On 2017/4/24 16:40, Marc Zyngier wrote:
> > On 24/04/17 09:25, Lixiaoping (Timmy) wrote:
> >> Hi Marc,
> >>
> >> Sorry about previous email's confidential info. Please forget it.
> >>
> >> +#define ESR_ELx_SYS64_ISS_SYS_CNTFRQ	(ESR_ELx_SYS64_ISS_SYS_VAL(3, 3, 14, 0, 0) | \
> >> +					 ESR_ELx_SYS64_ISS_DIR_READ)
> >>
> >> I think (3, 3, 14, 0, 0) should be (3, 3, 0, 14, 0)?
> > Thanks for spotting this. I assumed that the sys_reg() and
> > SR_ELx_SYS64_ISS_SYS_VAL() macros took their arguments in the same 
> > order. That would have been too easy... ;-)
> >
> > Amended patch below, please let me know if it works for you.
> >
> > Thanks,
> >
> > 	M.
> >
> > >From 4444c86a97c1a487e12c319fdc197c88631d72b5 Mon Sep 17 00:00:00 2001
> > From: Marc Zyngier <marc.zyngier@arm.com>
> > Date: Mon, 24 Apr 2017 09:04:03 +0100
> > Subject: [PATCH] arm64: Add CNTFRQ_EL0 trap handler
> >
> > We now trap accesses to CNTVCT_EL0 when the counter is broken
> > enough to require the kernel to mediate the access. But it
> > turns out that some existing userspace (such as OpenMPI) do
> > probe for the counter frequency, leading to an UNDEF exception
> > as CNTVCT_EL0 and CNTFRQ_EL0 share the same control bit.
> >
> > The fix is to handle the exception the same way we do for CNTVCT_EL0.
> >
> > Fixes: a86bd139f2ae ("arm64: arch_timer: Enable CNTVCT_EL0 trap if workaround is enabled")
> > Reported-by: Hanjun Guo <guohanjun@huawei.com>
> > Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> > ---
> 
> I tested this patch and the undefined instruction error is gone, I can
> get the FREQ in the user space now, thank you very much for the quick
> response.
> 
> Tested-by: Hanjun Guo <guohanjun@huawei.com>
> Reviewed-by: Hanjun Guo <guohanjun@huawei.com>

Thanks for providing so quickly a fix and test it.

  -- Daniel

-- 

 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

^ permalink raw reply	[flat|nested] 59+ messages in thread

* [PATCH v3 15/18] arm64: arch_timer: Enable CNTVCT_EL0 trap if workaround is enabled
@ 2017-04-24  9:18               ` Daniel Lezcano
  0 siblings, 0 replies; 59+ messages in thread
From: Daniel Lezcano @ 2017-04-24  9:18 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Apr 24, 2017 at 05:14:29PM +0800, Hanjun Guo wrote:
> On 2017/4/24 16:40, Marc Zyngier wrote:
> > On 24/04/17 09:25, Lixiaoping (Timmy) wrote:
> >> Hi Marc,
> >>
> >> Sorry about previous email's confidential info. Please forget it.
> >>
> >> +#define ESR_ELx_SYS64_ISS_SYS_CNTFRQ	(ESR_ELx_SYS64_ISS_SYS_VAL(3, 3, 14, 0, 0) | \
> >> +					 ESR_ELx_SYS64_ISS_DIR_READ)
> >>
> >> I think (3, 3, 14, 0, 0) should be (3, 3, 0, 14, 0)?
> > Thanks for spotting this. I assumed that the sys_reg() and
> > SR_ELx_SYS64_ISS_SYS_VAL() macros took their arguments in the same 
> > order. That would have been too easy... ;-)
> >
> > Amended patch below, please let me know if it works for you.
> >
> > Thanks,
> >
> > 	M.
> >
> > >From 4444c86a97c1a487e12c319fdc197c88631d72b5 Mon Sep 17 00:00:00 2001
> > From: Marc Zyngier <marc.zyngier@arm.com>
> > Date: Mon, 24 Apr 2017 09:04:03 +0100
> > Subject: [PATCH] arm64: Add CNTFRQ_EL0 trap handler
> >
> > We now trap accesses to CNTVCT_EL0 when the counter is broken
> > enough to require the kernel to mediate the access. But it
> > turns out that some existing userspace (such as OpenMPI) do
> > probe for the counter frequency, leading to an UNDEF exception
> > as CNTVCT_EL0 and CNTFRQ_EL0 share the same control bit.
> >
> > The fix is to handle the exception the same way we do for CNTVCT_EL0.
> >
> > Fixes: a86bd139f2ae ("arm64: arch_timer: Enable CNTVCT_EL0 trap if workaround is enabled")
> > Reported-by: Hanjun Guo <guohanjun@huawei.com>
> > Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> > ---
> 
> I tested this patch and the undefined instruction error is gone, I can
> get the FREQ in the user space now, thank you very much for the quick
> response.
> 
> Tested-by: Hanjun Guo <guohanjun@huawei.com>
> Reviewed-by: Hanjun Guo <guohanjun@huawei.com>

Thanks for providing so quickly a fix and test it.

  -- Daniel

-- 

 <http://www.linaro.org/> Linaro.org ? Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

^ permalink raw reply	[flat|nested] 59+ messages in thread

* Re: [PATCH v3 15/18] arm64: arch_timer: Enable CNTVCT_EL0 trap if workaround is enabled
  2017-04-24  9:14             ` Hanjun Guo
@ 2017-04-24  9:33               ` Marc Zyngier
  -1 siblings, 0 replies; 59+ messages in thread
From: Marc Zyngier @ 2017-04-24  9:33 UTC (permalink / raw)
  To: Hanjun Guo, Lixiaoping (Timmy),
	linux-arm-kernel, linux-kernel, Catalin Marinas
  Cc: Mark Rutland, Daniel Lezcano, Will Deacon, Scott Wood,
	Hanjun Guo, Dingtianhong, dann frazier, Thomas Gleixner

On 24/04/17 10:14, Hanjun Guo wrote:
> On 2017/4/24 16:40, Marc Zyngier wrote:
>> On 24/04/17 09:25, Lixiaoping (Timmy) wrote:
>>> Hi Marc,
>>>
>>> Sorry about previous email's confidential info. Please forget it.
>>>
>>> +#define ESR_ELx_SYS64_ISS_SYS_CNTFRQ	(ESR_ELx_SYS64_ISS_SYS_VAL(3, 3, 14, 0, 0) | \
>>> +					 ESR_ELx_SYS64_ISS_DIR_READ)
>>>
>>> I think (3, 3, 14, 0, 0) should be (3, 3, 0, 14, 0)?
>> Thanks for spotting this. I assumed that the sys_reg() and
>> SR_ELx_SYS64_ISS_SYS_VAL() macros took their arguments in the same 
>> order. That would have been too easy... ;-)
>>
>> Amended patch below, please let me know if it works for you.
>>
>> Thanks,
>>
>> 	M.
>>
>> >From 4444c86a97c1a487e12c319fdc197c88631d72b5 Mon Sep 17 00:00:00 2001
>> From: Marc Zyngier <marc.zyngier@arm.com>
>> Date: Mon, 24 Apr 2017 09:04:03 +0100
>> Subject: [PATCH] arm64: Add CNTFRQ_EL0 trap handler
>>
>> We now trap accesses to CNTVCT_EL0 when the counter is broken
>> enough to require the kernel to mediate the access. But it
>> turns out that some existing userspace (such as OpenMPI) do
>> probe for the counter frequency, leading to an UNDEF exception
>> as CNTVCT_EL0 and CNTFRQ_EL0 share the same control bit.
>>
>> The fix is to handle the exception the same way we do for CNTVCT_EL0.
>>
>> Fixes: a86bd139f2ae ("arm64: arch_timer: Enable CNTVCT_EL0 trap if workaround is enabled")
>> Reported-by: Hanjun Guo <guohanjun@huawei.com>
>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
>> ---
> 
> I tested this patch and the undefined instruction error is gone, I can
> get the FREQ in the user space now, thank you very much for the quick
> response.
> 
> Tested-by: Hanjun Guo <guohanjun@huawei.com>
> Reviewed-by: Hanjun Guo <guohanjun@huawei.com>

Thanks for giving it a go. Catalin, can you queue this via the arm64 tree?

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

^ permalink raw reply	[flat|nested] 59+ messages in thread

* [PATCH v3 15/18] arm64: arch_timer: Enable CNTVCT_EL0 trap if workaround is enabled
@ 2017-04-24  9:33               ` Marc Zyngier
  0 siblings, 0 replies; 59+ messages in thread
From: Marc Zyngier @ 2017-04-24  9:33 UTC (permalink / raw)
  To: linux-arm-kernel

On 24/04/17 10:14, Hanjun Guo wrote:
> On 2017/4/24 16:40, Marc Zyngier wrote:
>> On 24/04/17 09:25, Lixiaoping (Timmy) wrote:
>>> Hi Marc,
>>>
>>> Sorry about previous email's confidential info. Please forget it.
>>>
>>> +#define ESR_ELx_SYS64_ISS_SYS_CNTFRQ	(ESR_ELx_SYS64_ISS_SYS_VAL(3, 3, 14, 0, 0) | \
>>> +					 ESR_ELx_SYS64_ISS_DIR_READ)
>>>
>>> I think (3, 3, 14, 0, 0) should be (3, 3, 0, 14, 0)?
>> Thanks for spotting this. I assumed that the sys_reg() and
>> SR_ELx_SYS64_ISS_SYS_VAL() macros took their arguments in the same 
>> order. That would have been too easy... ;-)
>>
>> Amended patch below, please let me know if it works for you.
>>
>> Thanks,
>>
>> 	M.
>>
>> >From 4444c86a97c1a487e12c319fdc197c88631d72b5 Mon Sep 17 00:00:00 2001
>> From: Marc Zyngier <marc.zyngier@arm.com>
>> Date: Mon, 24 Apr 2017 09:04:03 +0100
>> Subject: [PATCH] arm64: Add CNTFRQ_EL0 trap handler
>>
>> We now trap accesses to CNTVCT_EL0 when the counter is broken
>> enough to require the kernel to mediate the access. But it
>> turns out that some existing userspace (such as OpenMPI) do
>> probe for the counter frequency, leading to an UNDEF exception
>> as CNTVCT_EL0 and CNTFRQ_EL0 share the same control bit.
>>
>> The fix is to handle the exception the same way we do for CNTVCT_EL0.
>>
>> Fixes: a86bd139f2ae ("arm64: arch_timer: Enable CNTVCT_EL0 trap if workaround is enabled")
>> Reported-by: Hanjun Guo <guohanjun@huawei.com>
>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
>> ---
> 
> I tested this patch and the undefined instruction error is gone, I can
> get the FREQ in the user space now, thank you very much for the quick
> response.
> 
> Tested-by: Hanjun Guo <guohanjun@huawei.com>
> Reviewed-by: Hanjun Guo <guohanjun@huawei.com>

Thanks for giving it a go. Catalin, can you queue this via the arm64 tree?

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

^ permalink raw reply	[flat|nested] 59+ messages in thread

* Re: [PATCH v3 15/18] arm64: arch_timer: Enable CNTVCT_EL0 trap if workaround is enabled
  2017-04-24  9:33               ` Marc Zyngier
@ 2017-04-24 11:26                 ` Catalin Marinas
  -1 siblings, 0 replies; 59+ messages in thread
From: Catalin Marinas @ 2017-04-24 11:26 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: Hanjun Guo, Lixiaoping (Timmy),
	linux-arm-kernel, linux-kernel, Mark Rutland, dann frazier,
	Daniel Lezcano, Will Deacon, Scott Wood, Hanjun Guo,
	Dingtianhong, Thomas Gleixner

On Mon, Apr 24, 2017 at 10:33:29AM +0100, Marc Zyngier wrote:
> On 24/04/17 10:14, Hanjun Guo wrote:
> > On 2017/4/24 16:40, Marc Zyngier wrote:
> >> On 24/04/17 09:25, Lixiaoping (Timmy) wrote:
> >>> Sorry about previous email's confidential info. Please forget it.
> >>>
> >>> +#define ESR_ELx_SYS64_ISS_SYS_CNTFRQ	(ESR_ELx_SYS64_ISS_SYS_VAL(3, 3, 14, 0, 0) | \
> >>> +					 ESR_ELx_SYS64_ISS_DIR_READ)
> >>>
> >>> I think (3, 3, 14, 0, 0) should be (3, 3, 0, 14, 0)?
> >> Thanks for spotting this. I assumed that the sys_reg() and
> >> SR_ELx_SYS64_ISS_SYS_VAL() macros took their arguments in the same 
> >> order. That would have been too easy... ;-)
> >>
> >> Amended patch below, please let me know if it works for you.
> >>
> >> Thanks,
> >>
> >> 	M.
> >>
> >> >From 4444c86a97c1a487e12c319fdc197c88631d72b5 Mon Sep 17 00:00:00 2001
> >> From: Marc Zyngier <marc.zyngier@arm.com>
> >> Date: Mon, 24 Apr 2017 09:04:03 +0100
> >> Subject: [PATCH] arm64: Add CNTFRQ_EL0 trap handler
> >>
> >> We now trap accesses to CNTVCT_EL0 when the counter is broken
> >> enough to require the kernel to mediate the access. But it
> >> turns out that some existing userspace (such as OpenMPI) do
> >> probe for the counter frequency, leading to an UNDEF exception
> >> as CNTVCT_EL0 and CNTFRQ_EL0 share the same control bit.
> >>
> >> The fix is to handle the exception the same way we do for CNTVCT_EL0.
> >>
> >> Fixes: a86bd139f2ae ("arm64: arch_timer: Enable CNTVCT_EL0 trap if workaround is enabled")
> >> Reported-by: Hanjun Guo <guohanjun@huawei.com>
> >> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> >> ---
> > 
> > I tested this patch and the undefined instruction error is gone, I can
> > get the FREQ in the user space now, thank you very much for the quick
> > response.
> > 
> > Tested-by: Hanjun Guo <guohanjun@huawei.com>
> > Reviewed-by: Hanjun Guo <guohanjun@huawei.com>
> 
> Thanks for giving it a go. Catalin, can you queue this via the arm64 tree?

Done. Thanks for the quick fix.

-- 
Catalin

^ permalink raw reply	[flat|nested] 59+ messages in thread

* [PATCH v3 15/18] arm64: arch_timer: Enable CNTVCT_EL0 trap if workaround is enabled
@ 2017-04-24 11:26                 ` Catalin Marinas
  0 siblings, 0 replies; 59+ messages in thread
From: Catalin Marinas @ 2017-04-24 11:26 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Apr 24, 2017 at 10:33:29AM +0100, Marc Zyngier wrote:
> On 24/04/17 10:14, Hanjun Guo wrote:
> > On 2017/4/24 16:40, Marc Zyngier wrote:
> >> On 24/04/17 09:25, Lixiaoping (Timmy) wrote:
> >>> Sorry about previous email's confidential info. Please forget it.
> >>>
> >>> +#define ESR_ELx_SYS64_ISS_SYS_CNTFRQ	(ESR_ELx_SYS64_ISS_SYS_VAL(3, 3, 14, 0, 0) | \
> >>> +					 ESR_ELx_SYS64_ISS_DIR_READ)
> >>>
> >>> I think (3, 3, 14, 0, 0) should be (3, 3, 0, 14, 0)?
> >> Thanks for spotting this. I assumed that the sys_reg() and
> >> SR_ELx_SYS64_ISS_SYS_VAL() macros took their arguments in the same 
> >> order. That would have been too easy... ;-)
> >>
> >> Amended patch below, please let me know if it works for you.
> >>
> >> Thanks,
> >>
> >> 	M.
> >>
> >> >From 4444c86a97c1a487e12c319fdc197c88631d72b5 Mon Sep 17 00:00:00 2001
> >> From: Marc Zyngier <marc.zyngier@arm.com>
> >> Date: Mon, 24 Apr 2017 09:04:03 +0100
> >> Subject: [PATCH] arm64: Add CNTFRQ_EL0 trap handler
> >>
> >> We now trap accesses to CNTVCT_EL0 when the counter is broken
> >> enough to require the kernel to mediate the access. But it
> >> turns out that some existing userspace (such as OpenMPI) do
> >> probe for the counter frequency, leading to an UNDEF exception
> >> as CNTVCT_EL0 and CNTFRQ_EL0 share the same control bit.
> >>
> >> The fix is to handle the exception the same way we do for CNTVCT_EL0.
> >>
> >> Fixes: a86bd139f2ae ("arm64: arch_timer: Enable CNTVCT_EL0 trap if workaround is enabled")
> >> Reported-by: Hanjun Guo <guohanjun@huawei.com>
> >> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> >> ---
> > 
> > I tested this patch and the undefined instruction error is gone, I can
> > get the FREQ in the user space now, thank you very much for the quick
> > response.
> > 
> > Tested-by: Hanjun Guo <guohanjun@huawei.com>
> > Reviewed-by: Hanjun Guo <guohanjun@huawei.com>
> 
> Thanks for giving it a go. Catalin, can you queue this via the arm64 tree?

Done. Thanks for the quick fix.

-- 
Catalin

^ permalink raw reply	[flat|nested] 59+ messages in thread

end of thread, other threads:[~2017-04-24 11:26 UTC | newest]

Thread overview: 59+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-04 17:18 [PATCH v3 00/18] clocksource/arch_timer: Errata workaround infrastructure rework Marc Zyngier
2017-04-04 17:18 ` Marc Zyngier
2017-04-04 17:18 ` [PATCH v3 01/18] arm64: Allow checking of a CPU-local erratum Marc Zyngier
2017-04-04 17:18   ` Marc Zyngier
2017-04-04 17:18 ` [PATCH v3 02/18] arm64: Add CNTVCT_EL0 trap handler Marc Zyngier
2017-04-04 17:18   ` Marc Zyngier
2017-04-04 17:18 ` [PATCH v3 03/18] arm64: Define Cortex-A73 MIDR Marc Zyngier
2017-04-04 17:18   ` Marc Zyngier
2017-04-04 17:18 ` [PATCH v3 04/18] arm64: cpu_errata: Allow an erratum to be match for all revisions of a core Marc Zyngier
2017-04-04 17:18   ` Marc Zyngier
2017-04-04 17:18 ` [PATCH v3 05/18] arm64: cpu_errata: Add capability to advertise Cortex-A73 erratum 858921 Marc Zyngier
2017-04-04 17:18   ` Marc Zyngier
2017-04-04 17:18 ` [PATCH v3 06/18] arm64: arch_timer: Add infrastructure for multiple erratum detection methods Marc Zyngier
2017-04-04 17:18   ` Marc Zyngier
2017-04-04 17:18 ` [PATCH v3 07/18] arm64: arch_timer: Add erratum handler for CPU-specific capability Marc Zyngier
2017-04-04 17:18   ` Marc Zyngier
2017-04-04 17:18 ` [PATCH v3 08/18] arm64: arch_timer: Move arch_timer_reg_read/write around Marc Zyngier
2017-04-04 17:18   ` Marc Zyngier
2017-04-04 17:18 ` [PATCH v3 09/18] arm64: arch_timer: Get rid of erratum_workaround_set_sne Marc Zyngier
2017-04-04 17:18   ` Marc Zyngier
2017-04-04 17:18 ` [PATCH v3 10/18] arm64: arch_timer: Rework the set_next_event workarounds Marc Zyngier
2017-04-04 17:18   ` Marc Zyngier
2017-04-04 17:18 ` [PATCH v3 11/18] arm64: arch_timer: Make workaround methods optional Marc Zyngier
2017-04-04 17:18   ` Marc Zyngier
2017-04-04 17:18 ` [PATCH v3 12/18] arm64: arch_timer: Allows a CPU-specific erratum to only affect a subset of CPUs Marc Zyngier
2017-04-04 17:18   ` Marc Zyngier
2017-04-04 17:18 ` [PATCH v3 13/18] arm64: arch_timer: Move clocksource_counter and co around Marc Zyngier
2017-04-04 17:18   ` Marc Zyngier
2017-04-04 17:18 ` [PATCH v3 14/18] arm64: arch_timer: Save cntkctl_el1 as a per-cpu variable Marc Zyngier
2017-04-04 17:18   ` Marc Zyngier
2017-04-04 17:18 ` [PATCH v3 15/18] arm64: arch_timer: Enable CNTVCT_EL0 trap if workaround is enabled Marc Zyngier
2017-04-04 17:18   ` Marc Zyngier
     [not found]   ` <58FDB05B.6020108@huawei.com>
2017-04-24  8:13     ` Marc Zyngier
2017-04-24  8:13       ` Marc Zyngier
2017-04-24  8:18       ` Lixiaoping (Timmy)
2017-04-24  8:18         ` Lixiaoping (Timmy)
2017-04-24  8:25       ` Lixiaoping (Timmy)
2017-04-24  8:25         ` Lixiaoping (Timmy)
2017-04-24  8:40         ` Marc Zyngier
2017-04-24  8:40           ` Marc Zyngier
2017-04-24  9:14           ` Hanjun Guo
2017-04-24  9:14             ` Hanjun Guo
2017-04-24  9:18             ` Daniel Lezcano
2017-04-24  9:18               ` Daniel Lezcano
2017-04-24  9:33             ` Marc Zyngier
2017-04-24  9:33               ` Marc Zyngier
2017-04-24 11:26               ` Catalin Marinas
2017-04-24 11:26                 ` Catalin Marinas
2017-04-04 17:18 ` [PATCH v3 16/18] arm64: arch_timer: Workaround for Cortex-A73 erratum 858921 Marc Zyngier
2017-04-04 17:18   ` Marc Zyngier
2017-04-04 17:18 ` [PATCH v3 17/18] arm64: arch_timer: Allow erratum matching with ACPI OEM information Marc Zyngier
2017-04-04 17:18   ` Marc Zyngier
2017-04-04 17:18 ` [PATCH v3 18/18] arm64: arch_timer: Add HISILICON_ERRATUM_161010101 ACPI matching data Marc Zyngier
2017-04-04 17:18   ` Marc Zyngier
2017-04-06 16:27 ` [PATCH v3 00/18] clocksource/arch_timer: Errata workaround infrastructure rework Catalin Marinas
2017-04-06 16:27   ` Catalin Marinas
2017-04-06 21:15   ` Daniel Lezcano
2017-04-07  8:15     ` Thomas Gleixner
2017-04-07  8:15       ` Thomas Gleixner

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.