linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 0/2] Implement a software workaround for Falkor erratum 1041
@ 2017-11-27 23:17 Shanker Donthineni
  2017-11-27 23:17 ` [PATCH v4 1/2] arm64: Define cputype macros for Falkor CPU Shanker Donthineni
  2017-11-27 23:18 ` [PATCH v4 2/2] arm64: Add software workaround for Falkor erratum 1041 Shanker Donthineni
  0 siblings, 2 replies; 6+ messages in thread
From: Shanker Donthineni @ 2017-11-27 23:17 UTC (permalink / raw)
  To: Will Deacon, Marc Zyngier, linux-arm-kernel
  Cc: Catalin Marinas, James Morse, Robin Murphy, Ard Biesheuvel,
	Matt Fleming, Christoffer Dall, linux-kernel, linux-efi, kvmarm,
	Shanker Donthineni

On Falkor CPU, we’ve discovered a hardware issue which might lead to a
kernel crash or the unexpected behavior. The Falkor core may errantly
access memory locations on speculative instruction fetches. This may
happen whenever MMU translation state, SCTLR_ELn[M] bit is being changed
from enabled to disabled for the currently running exception level. To
prevent the errant hardware behavior, software must execute an ISB
immediately prior to executing the MSR that changes SCTLR_ELn[M] from a
value of 1 to 0.

These v4 patches are based on 4.15-rc1 and tested on QDF2400 platform.

Patch2 from V1 series got dropped to accommodate review comments. Apply
the workaround where it's required.

Posted wrong the patches in v2.

Shanker Donthineni (2):
  arm64: Define cputype macros for Falkor CPU
  arm64: Add software workaround for Falkor erratum 1041

 Documentation/arm64/silicon-errata.txt |  1 +
 arch/arm64/Kconfig                     | 12 +++++++++++-
 arch/arm64/include/asm/assembler.h     | 19 +++++++++++++++++++
 arch/arm64/include/asm/cpucaps.h       |  3 ++-
 arch/arm64/include/asm/cputype.h       |  2 ++
 arch/arm64/kernel/cpu-reset.S          |  1 +
 arch/arm64/kernel/cpu_errata.c         | 16 ++++++++++++++++
 arch/arm64/kernel/efi-entry.S          |  2 ++
 arch/arm64/kernel/head.S               |  1 +
 arch/arm64/kernel/relocate_kernel.S    |  1 +
 arch/arm64/kvm/hyp-init.S              |  1 +
 11 files changed, 57 insertions(+), 2 deletions(-)

-- 
Qualcomm Datacenter Technologies, Inc. on behalf of the Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.

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

* [PATCH v4 1/2] arm64: Define cputype macros for Falkor CPU
  2017-11-27 23:17 [PATCH v4 0/2] Implement a software workaround for Falkor erratum 1041 Shanker Donthineni
@ 2017-11-27 23:17 ` Shanker Donthineni
  2017-11-27 23:18 ` [PATCH v4 2/2] arm64: Add software workaround for Falkor erratum 1041 Shanker Donthineni
  1 sibling, 0 replies; 6+ messages in thread
From: Shanker Donthineni @ 2017-11-27 23:17 UTC (permalink / raw)
  To: Will Deacon, Marc Zyngier, linux-arm-kernel
  Cc: Catalin Marinas, James Morse, Robin Murphy, Ard Biesheuvel,
	Matt Fleming, Christoffer Dall, linux-kernel, linux-efi, kvmarm,
	Shanker Donthineni

Add cputype definition macros for Qualcomm Datacenter Technologies
Falkor CPU in cputype.h. It's unfortunate that the first revision
of the Falkor CPU used the wrong part number 0x800, got fixed in v2
chip with part number 0xC00, and would be used the same value for
future revisions.

Signed-off-by: Shanker Donthineni <shankerd@codeaurora.org>
---
 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 235e77d..cbf08d7 100644
--- a/arch/arm64/include/asm/cputype.h
+++ b/arch/arm64/include/asm/cputype.h
@@ -91,6 +91,7 @@
 #define BRCM_CPU_PART_VULCAN		0x516
 
 #define QCOM_CPU_PART_FALKOR_V1		0x800
+#define QCOM_CPU_PART_FALKOR		0xC00
 
 #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)
@@ -99,6 +100,7 @@
 #define MIDR_THUNDERX_81XX MIDR_CPU_MODEL(ARM_CPU_IMP_CAVIUM, CAVIUM_CPU_PART_THUNDERX_81XX)
 #define MIDR_THUNDERX_83XX MIDR_CPU_MODEL(ARM_CPU_IMP_CAVIUM, CAVIUM_CPU_PART_THUNDERX_83XX)
 #define MIDR_QCOM_FALKOR_V1 MIDR_CPU_MODEL(ARM_CPU_IMP_QCOM, QCOM_CPU_PART_FALKOR_V1)
+#define MIDR_QCOM_FALKOR MIDR_CPU_MODEL(ARM_CPU_IMP_QCOM, QCOM_CPU_PART_FALKOR)
 
 #ifndef __ASSEMBLY__
 
-- 
Qualcomm Datacenter Technologies, Inc. on behalf of the Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.

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

* [PATCH v4 2/2] arm64: Add software workaround for Falkor erratum 1041
  2017-11-27 23:17 [PATCH v4 0/2] Implement a software workaround for Falkor erratum 1041 Shanker Donthineni
  2017-11-27 23:17 ` [PATCH v4 1/2] arm64: Define cputype macros for Falkor CPU Shanker Donthineni
@ 2017-11-27 23:18 ` Shanker Donthineni
  2017-12-01 11:24   ` Will Deacon
  1 sibling, 1 reply; 6+ messages in thread
From: Shanker Donthineni @ 2017-11-27 23:18 UTC (permalink / raw)
  To: Will Deacon, Marc Zyngier, linux-arm-kernel
  Cc: Catalin Marinas, James Morse, Robin Murphy, Ard Biesheuvel,
	Matt Fleming, Christoffer Dall, linux-kernel, linux-efi, kvmarm,
	Shanker Donthineni

The ARM architecture defines the memory locations that are permitted
to be accessed as the result of a speculative instruction fetch from
an exception level for which all stages of translation are disabled.
Specifically, the core is permitted to speculatively fetch from the
4KB region containing the current program counter 4K and next 4K.

When translation is changed from enabled to disabled for the running
exception level (SCTLR_ELn[M] changed from a value of 1 to 0), the
Falkor core may errantly speculatively access memory locations outside
of the 4KB region permitted by the architecture. The errant memory
access may lead to one of the following unexpected behaviors.

1) A System Error Interrupt (SEI) being raised by the Falkor core due
   to the errant memory access attempting to access a region of memory
   that is protected by a slave-side memory protection unit.
2) Unpredictable device behavior due to a speculative read from device
   memory. This behavior may only occur if the instruction cache is
   disabled prior to or coincident with translation being changed from
   enabled to disabled.

The conditions leading to this erratum will not occur when either of the
following occur:
 1) A higher exception level disables translation of a lower exception level
   (e.g. EL2 changing SCTLR_EL1[M] from a value of 1 to 0).
 2) An exception level disabling its stage-1 translation if its stage-2
    translation is enabled (e.g. EL1 changing SCTLR_EL1[M] from a value of 1
    to 0 when HCR_EL2[VM] has a value of 1).

To avoid the errant behavior, software must execute an ISB immediately
prior to executing the MSR that will change SCTLR_ELn[M] from 1 to 0.

Signed-off-by: Shanker Donthineni <shankerd@codeaurora.org>
---
Changes since v3:
  Rebased to kernel v4.15-rc1.
Changes since v2:
  Repost the corrected patches.
Changes since v1:
  Apply the workaround where it's required.

 Documentation/arm64/silicon-errata.txt |  1 +
 arch/arm64/Kconfig                     | 12 +++++++++++-
 arch/arm64/include/asm/assembler.h     | 19 +++++++++++++++++++
 arch/arm64/include/asm/cpucaps.h       |  3 ++-
 arch/arm64/kernel/cpu-reset.S          |  1 +
 arch/arm64/kernel/cpu_errata.c         | 16 ++++++++++++++++
 arch/arm64/kernel/efi-entry.S          |  2 ++
 arch/arm64/kernel/head.S               |  1 +
 arch/arm64/kernel/relocate_kernel.S    |  1 +
 arch/arm64/kvm/hyp-init.S              |  1 +
 10 files changed, 55 insertions(+), 2 deletions(-)

diff --git a/Documentation/arm64/silicon-errata.txt b/Documentation/arm64/silicon-errata.txt
index 304bf22..fc1c884 100644
--- a/Documentation/arm64/silicon-errata.txt
+++ b/Documentation/arm64/silicon-errata.txt
@@ -75,3 +75,4 @@ stable kernels.
 | Qualcomm Tech. | Falkor v1       | E1003           | QCOM_FALKOR_ERRATUM_1003    |
 | Qualcomm Tech. | Falkor v1       | E1009           | QCOM_FALKOR_ERRATUM_1009    |
 | Qualcomm Tech. | QDF2400 ITS     | E0065           | QCOM_QDF2400_ERRATUM_0065   |
+| Qualcomm Tech. | Falkor v{1,2}   | E1041           | QCOM_FALKOR_ERRATUM_1041    |
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index a93339f..c9a7e9e 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -557,7 +557,6 @@ config QCOM_QDF2400_ERRATUM_0065
 
 	  If unsure, say Y.
 
-
 config SOCIONEXT_SYNQUACER_PREITS
 	bool "Socionext Synquacer: Workaround for GICv3 pre-ITS"
 	default y
@@ -576,6 +575,17 @@ config HISILICON_ERRATUM_161600802
 	  a 128kB offset to be applied to the target address in this commands.
 
 	  If unsure, say Y.
+
+config QCOM_FALKOR_ERRATUM_E1041
+	bool "Falkor E1041: Speculative instruction fetches might cause errant memory access"
+	default y
+	help
+	  Falkor CPU may speculatively fetch instructions from an improper
+	  memory location when MMU translation is changed from SCTLR_ELn[M]=1
+	  to SCTLR_ELn[M]=0. Prefix an ISB instruction to fix the problem.
+
+	  If unsure, say Y.
+
 endmenu
 
 
diff --git a/arch/arm64/include/asm/assembler.h b/arch/arm64/include/asm/assembler.h
index aef72d8..c77742a 100644
--- a/arch/arm64/include/asm/assembler.h
+++ b/arch/arm64/include/asm/assembler.h
@@ -31,6 +31,7 @@
 #include <asm/pgtable-hwdef.h>
 #include <asm/ptrace.h>
 #include <asm/thread_info.h>
+#include <asm/alternative.h>
 
 	.macro save_and_disable_daif, flags
 	mrs	\flags, daif
@@ -512,4 +513,22 @@
 #endif
 	.endm
 
+/**
+ * Errata workaround prior to disable MMU. Insert an ISB immediately prior
+ * to executing the MSR that will change SCTLR_ELn[M] from a value of 1 to 0.
+ */
+	.macro pre_disable_mmu_workaround
+#ifdef CONFIG_QCOM_FALKOR_ERRATUM_E1041
+alternative_if ARM64_WORKAROUND_QCOM_FALKOR_E1041
+	isb
+alternative_else_nop_endif
+#endif
+	.endm
+
+	.macro pre_disable_mmu_early_workaround
+#ifdef CONFIG_QCOM_FALKOR_ERRATUM_E1041
+	isb
+#endif
+	.endm
+
 #endif	/* __ASM_ASSEMBLER_H */
diff --git a/arch/arm64/include/asm/cpucaps.h b/arch/arm64/include/asm/cpucaps.h
index 2ff7c5e..b884268 100644
--- a/arch/arm64/include/asm/cpucaps.h
+++ b/arch/arm64/include/asm/cpucaps.h
@@ -41,7 +41,8 @@
 #define ARM64_WORKAROUND_CAVIUM_30115		20
 #define ARM64_HAS_DCPOP				21
 #define ARM64_SVE				22
+#define ARM64_WORKAROUND_QCOM_FALKOR_E1041	23
 
-#define ARM64_NCAPS				23
+#define ARM64_NCAPS				24
 
 #endif /* __ASM_CPUCAPS_H */
diff --git a/arch/arm64/kernel/cpu-reset.S b/arch/arm64/kernel/cpu-reset.S
index 65f42d2..2a752cb 100644
--- a/arch/arm64/kernel/cpu-reset.S
+++ b/arch/arm64/kernel/cpu-reset.S
@@ -37,6 +37,7 @@ ENTRY(__cpu_soft_restart)
 	mrs	x12, sctlr_el1
 	ldr	x13, =SCTLR_ELx_FLAGS
 	bic	x12, x12, x13
+	pre_disable_mmu_workaround
 	msr	sctlr_el1, x12
 	isb
 
diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
index 0e27f86..2fd1938 100644
--- a/arch/arm64/kernel/cpu_errata.c
+++ b/arch/arm64/kernel/cpu_errata.c
@@ -179,6 +179,22 @@ static int cpu_enable_trap_ctr_access(void *__unused)
 			   MIDR_CPU_VAR_REV(0, 0)),
 	},
 #endif
+#ifdef CONFIG_QCOM_FALKOR_ERRATUM_E1041
+	{
+		.desc = "Qualcomm Technologies Falkor erratum 1041",
+		.capability = ARM64_WORKAROUND_QCOM_FALKOR_E1041,
+		MIDR_RANGE(MIDR_QCOM_FALKOR_V1,
+			MIDR_CPU_VAR_REV(0, 0),
+			MIDR_CPU_VAR_REV(0, 0)),
+	},
+	{
+		.desc = "Qualcomm Technologies Falkor erratum 1041",
+		.capability = ARM64_WORKAROUND_QCOM_FALKOR_E1041,
+		MIDR_RANGE(MIDR_QCOM_FALKOR,
+			MIDR_CPU_VAR_REV(0, 1),
+			MIDR_CPU_VAR_REV(0, 2)),
+	},
+#endif
 #ifdef CONFIG_ARM64_ERRATUM_858921
 	{
 	/* Cortex-A73 all versions */
diff --git a/arch/arm64/kernel/efi-entry.S b/arch/arm64/kernel/efi-entry.S
index 4e6ad35..dc675ba 100644
--- a/arch/arm64/kernel/efi-entry.S
+++ b/arch/arm64/kernel/efi-entry.S
@@ -96,6 +96,7 @@ ENTRY(entry)
 	mrs	x0, sctlr_el2
 	bic	x0, x0, #1 << 0	// clear SCTLR.M
 	bic	x0, x0, #1 << 2	// clear SCTLR.C
+	pre_disable_mmu_early_workaround
 	msr	sctlr_el2, x0
 	isb
 	b	2f
@@ -103,6 +104,7 @@ ENTRY(entry)
 	mrs	x0, sctlr_el1
 	bic	x0, x0, #1 << 0	// clear SCTLR.M
 	bic	x0, x0, #1 << 2	// clear SCTLR.C
+	pre_disable_mmu_early_workaround
 	msr	sctlr_el1, x0
 	isb
 2:
diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
index 67e86a0..a0d737d 100644
--- a/arch/arm64/kernel/head.S
+++ b/arch/arm64/kernel/head.S
@@ -750,6 +750,7 @@ __primary_switch:
 	 * to take into account by discarding the current kernel mapping and
 	 * creating a new one.
 	 */
+	pre_disable_mmu_early_workaround
 	msr	sctlr_el1, x20			// disable the MMU
 	isb
 	bl	__create_page_tables		// recreate kernel mapping
diff --git a/arch/arm64/kernel/relocate_kernel.S b/arch/arm64/kernel/relocate_kernel.S
index ce704a4..f407e42 100644
--- a/arch/arm64/kernel/relocate_kernel.S
+++ b/arch/arm64/kernel/relocate_kernel.S
@@ -45,6 +45,7 @@ ENTRY(arm64_relocate_new_kernel)
 	mrs	x0, sctlr_el2
 	ldr	x1, =SCTLR_ELx_FLAGS
 	bic	x0, x0, x1
+	pre_disable_mmu_workaround
 	msr	sctlr_el2, x0
 	isb
 1:
diff --git a/arch/arm64/kvm/hyp-init.S b/arch/arm64/kvm/hyp-init.S
index 3f96155..870828c 100644
--- a/arch/arm64/kvm/hyp-init.S
+++ b/arch/arm64/kvm/hyp-init.S
@@ -151,6 +151,7 @@ reset:
 	mrs	x5, sctlr_el2
 	ldr	x6, =SCTLR_ELx_FLAGS
 	bic	x5, x5, x6		// Clear SCTL_M and etc
+	pre_disable_mmu_workaround
 	msr	sctlr_el2, x5
 	isb
 
-- 
Qualcomm Datacenter Technologies, Inc. on behalf of the Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.

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

* Re: [PATCH v4 2/2] arm64: Add software workaround for Falkor erratum 1041
  2017-11-27 23:18 ` [PATCH v4 2/2] arm64: Add software workaround for Falkor erratum 1041 Shanker Donthineni
@ 2017-12-01 11:24   ` Will Deacon
  2017-12-03 13:35     ` Shanker Donthineni
  0 siblings, 1 reply; 6+ messages in thread
From: Will Deacon @ 2017-12-01 11:24 UTC (permalink / raw)
  To: Shanker Donthineni
  Cc: Marc Zyngier, linux-arm-kernel, Catalin Marinas, James Morse,
	Robin Murphy, Ard Biesheuvel, Matt Fleming, Christoffer Dall,
	linux-kernel, linux-efi, kvmarm

On Mon, Nov 27, 2017 at 05:18:00PM -0600, Shanker Donthineni wrote:
> The ARM architecture defines the memory locations that are permitted
> to be accessed as the result of a speculative instruction fetch from
> an exception level for which all stages of translation are disabled.
> Specifically, the core is permitted to speculatively fetch from the
> 4KB region containing the current program counter 4K and next 4K.
> 
> When translation is changed from enabled to disabled for the running
> exception level (SCTLR_ELn[M] changed from a value of 1 to 0), the
> Falkor core may errantly speculatively access memory locations outside
> of the 4KB region permitted by the architecture. The errant memory
> access may lead to one of the following unexpected behaviors.
> 
> 1) A System Error Interrupt (SEI) being raised by the Falkor core due
>    to the errant memory access attempting to access a region of memory
>    that is protected by a slave-side memory protection unit.
> 2) Unpredictable device behavior due to a speculative read from device
>    memory. This behavior may only occur if the instruction cache is
>    disabled prior to or coincident with translation being changed from
>    enabled to disabled.
> 
> The conditions leading to this erratum will not occur when either of the
> following occur:
>  1) A higher exception level disables translation of a lower exception level
>    (e.g. EL2 changing SCTLR_EL1[M] from a value of 1 to 0).
>  2) An exception level disabling its stage-1 translation if its stage-2
>     translation is enabled (e.g. EL1 changing SCTLR_EL1[M] from a value of 1
>     to 0 when HCR_EL2[VM] has a value of 1).
> 
> To avoid the errant behavior, software must execute an ISB immediately
> prior to executing the MSR that will change SCTLR_ELn[M] from 1 to 0.
> 
> Signed-off-by: Shanker Donthineni <shankerd@codeaurora.org>
> ---
> Changes since v3:
>   Rebased to kernel v4.15-rc1.
> Changes since v2:
>   Repost the corrected patches.
> Changes since v1:
>   Apply the workaround where it's required.
> 
>  Documentation/arm64/silicon-errata.txt |  1 +
>  arch/arm64/Kconfig                     | 12 +++++++++++-
>  arch/arm64/include/asm/assembler.h     | 19 +++++++++++++++++++
>  arch/arm64/include/asm/cpucaps.h       |  3 ++-
>  arch/arm64/kernel/cpu-reset.S          |  1 +
>  arch/arm64/kernel/cpu_errata.c         | 16 ++++++++++++++++
>  arch/arm64/kernel/efi-entry.S          |  2 ++
>  arch/arm64/kernel/head.S               |  1 +
>  arch/arm64/kernel/relocate_kernel.S    |  1 +
>  arch/arm64/kvm/hyp-init.S              |  1 +

This is an awful lot of code just to add an ISB instruction prior to
disabling the MMU. Why do you need to go through the alternatives framework
for this? Just do it with an #ifdef; this isn't a fastpath.

Will

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

* Re: [PATCH v4 2/2] arm64: Add software workaround for Falkor erratum 1041
  2017-12-01 11:24   ` Will Deacon
@ 2017-12-03 13:35     ` Shanker Donthineni
  2017-12-04 20:09       ` Shanker Donthineni
  0 siblings, 1 reply; 6+ messages in thread
From: Shanker Donthineni @ 2017-12-03 13:35 UTC (permalink / raw)
  To: Will Deacon
  Cc: linux-efi, Ard Biesheuvel, Marc Zyngier, Catalin Marinas,
	linux-kernel, Matt Fleming, James Morse, Christoffer Dall,
	Robin Murphy, kvmarm, linux-arm-kernel

Hi Will, thanks for your review comments.

On 12/01/2017 05:24 AM, Will Deacon wrote:
> On Mon, Nov 27, 2017 at 05:18:00PM -0600, Shanker Donthineni wrote:
>> The ARM architecture defines the memory locations that are permitted
>> to be accessed as the result of a speculative instruction fetch from
>> an exception level for which all stages of translation are disabled.
>> Specifically, the core is permitted to speculatively fetch from the
>> 4KB region containing the current program counter 4K and next 4K.
>>
>> When translation is changed from enabled to disabled for the running
>> exception level (SCTLR_ELn[M] changed from a value of 1 to 0), the
>> Falkor core may errantly speculatively access memory locations outside
>> of the 4KB region permitted by the architecture. The errant memory
>> access may lead to one of the following unexpected behaviors.
>>
>> 1) A System Error Interrupt (SEI) being raised by the Falkor core due
>>    to the errant memory access attempting to access a region of memory
>>    that is protected by a slave-side memory protection unit.
>> 2) Unpredictable device behavior due to a speculative read from device
>>    memory. This behavior may only occur if the instruction cache is
>>    disabled prior to or coincident with translation being changed from
>>    enabled to disabled.
>>
>> The conditions leading to this erratum will not occur when either of the
>> following occur:
>>  1) A higher exception level disables translation of a lower exception level
>>    (e.g. EL2 changing SCTLR_EL1[M] from a value of 1 to 0).
>>  2) An exception level disabling its stage-1 translation if its stage-2
>>     translation is enabled (e.g. EL1 changing SCTLR_EL1[M] from a value of 1
>>     to 0 when HCR_EL2[VM] has a value of 1).
>>
>> To avoid the errant behavior, software must execute an ISB immediately
>> prior to executing the MSR that will change SCTLR_ELn[M] from 1 to 0.
>>
>> Signed-off-by: Shanker Donthineni <shankerd@codeaurora.org>
>> ---
>> Changes since v3:
>>   Rebased to kernel v4.15-rc1.
>> Changes since v2:
>>   Repost the corrected patches.
>> Changes since v1:
>>   Apply the workaround where it's required.
>>
>>  Documentation/arm64/silicon-errata.txt |  1 +
>>  arch/arm64/Kconfig                     | 12 +++++++++++-
>>  arch/arm64/include/asm/assembler.h     | 19 +++++++++++++++++++
>>  arch/arm64/include/asm/cpucaps.h       |  3 ++-
>>  arch/arm64/kernel/cpu-reset.S          |  1 +
>>  arch/arm64/kernel/cpu_errata.c         | 16 ++++++++++++++++
>>  arch/arm64/kernel/efi-entry.S          |  2 ++
>>  arch/arm64/kernel/head.S               |  1 +
>>  arch/arm64/kernel/relocate_kernel.S    |  1 +
>>  arch/arm64/kvm/hyp-init.S              |  1 +
> 
> This is an awful lot of code just to add an ISB instruction prior to
> disabling the MMU. Why do you need to go through the alternatives framework
> for this? Just do it with an #ifdef; this isn't a fastpath.
> 

We can avoid changes to only two files cpu_errata.c and cpucaps.h without using
the alternatives framework. Even though it's in slow path, cpu-errata.c changes 
provides a nice debug message which indicates the erratum E1041 is applied. 

Erratum log information would be very useful to conform our customers using the
right kernel with E1014 patch by looking at dmesg. Other than that I don't have
any other strong opinion to avoid alternatives and handle using #idef.

Should I go head and post v5 patch without alternatives?

> Will
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

-- 
Shanker Donthineni
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.

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

* Re: [PATCH v4 2/2] arm64: Add software workaround for Falkor erratum 1041
  2017-12-03 13:35     ` Shanker Donthineni
@ 2017-12-04 20:09       ` Shanker Donthineni
  0 siblings, 0 replies; 6+ messages in thread
From: Shanker Donthineni @ 2017-12-04 20:09 UTC (permalink / raw)
  To: Will Deacon
  Cc: linux-efi, Ard Biesheuvel, Marc Zyngier, Catalin Marinas,
	linux-kernel, Matt Fleming, linux-arm-kernel, Robin Murphy,
	kvmarm

Hi Will,

On 12/03/2017 07:35 AM, Shanker Donthineni wrote:
> Hi Will, thanks for your review comments.
> 
> On 12/01/2017 05:24 AM, Will Deacon wrote:
>> On Mon, Nov 27, 2017 at 05:18:00PM -0600, Shanker Donthineni wrote:
>>> The ARM architecture defines the memory locations that are permitted
>>> to be accessed as the result of a speculative instruction fetch from
>>> an exception level for which all stages of translation are disabled.
>>> Specifically, the core is permitted to speculatively fetch from the
>>> 4KB region containing the current program counter 4K and next 4K.
>>>
>>> When translation is changed from enabled to disabled for the running
>>> exception level (SCTLR_ELn[M] changed from a value of 1 to 0), the
>>> Falkor core may errantly speculatively access memory locations outside
>>> of the 4KB region permitted by the architecture. The errant memory
>>> access may lead to one of the following unexpected behaviors.
>>>
>>> 1) A System Error Interrupt (SEI) being raised by the Falkor core due
>>>    to the errant memory access attempting to access a region of memory
>>>    that is protected by a slave-side memory protection unit.
>>> 2) Unpredictable device behavior due to a speculative read from device
>>>    memory. This behavior may only occur if the instruction cache is
>>>    disabled prior to or coincident with translation being changed from
>>>    enabled to disabled.
>>>
>>> The conditions leading to this erratum will not occur when either of the
>>> following occur:
>>>  1) A higher exception level disables translation of a lower exception level
>>>    (e.g. EL2 changing SCTLR_EL1[M] from a value of 1 to 0).
>>>  2) An exception level disabling its stage-1 translation if its stage-2
>>>     translation is enabled (e.g. EL1 changing SCTLR_EL1[M] from a value of 1
>>>     to 0 when HCR_EL2[VM] has a value of 1).
>>>
>>> To avoid the errant behavior, software must execute an ISB immediately
>>> prior to executing the MSR that will change SCTLR_ELn[M] from 1 to 0.
>>>
>>> Signed-off-by: Shanker Donthineni <shankerd@codeaurora.org>
>>> ---
>>> Changes since v3:
>>>   Rebased to kernel v4.15-rc1.
>>> Changes since v2:
>>>   Repost the corrected patches.
>>> Changes since v1:
>>>   Apply the workaround where it's required.
>>>
>>>  Documentation/arm64/silicon-errata.txt |  1 +
>>>  arch/arm64/Kconfig                     | 12 +++++++++++-
>>>  arch/arm64/include/asm/assembler.h     | 19 +++++++++++++++++++
>>>  arch/arm64/include/asm/cpucaps.h       |  3 ++-
>>>  arch/arm64/kernel/cpu-reset.S          |  1 +
>>>  arch/arm64/kernel/cpu_errata.c         | 16 ++++++++++++++++
>>>  arch/arm64/kernel/efi-entry.S          |  2 ++
>>>  arch/arm64/kernel/head.S               |  1 +
>>>  arch/arm64/kernel/relocate_kernel.S    |  1 +
>>>  arch/arm64/kvm/hyp-init.S              |  1 +
>>
>> This is an awful lot of code just to add an ISB instruction prior to
>> disabling the MMU. Why do you need to go through the alternatives framework
>> for this? Just do it with an #ifdef; this isn't a fastpath.
>>
> 
> We can avoid changes to only two files cpu_errata.c and cpucaps.h without using
> the alternatives framework. Even though it's in slow path, cpu-errata.c changes 
> provides a nice debug message which indicates the erratum E1041 is applied. 
> 
> Erratum log information would be very useful to conform our customers using the
> right kernel with E1014 patch by looking at dmesg. Other than that I don't have
> any other strong opinion to avoid alternatives and handle using #idef.
> 
> Should I go ahead and post v5 patch without alternatives?
> 


Please provide your thoughts on next step. We would like to merge this erratum
to v4.15 kernel.

>> Will
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>>
> 

-- 
Shanker Donthineni
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.

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

end of thread, other threads:[~2017-12-04 20:09 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-27 23:17 [PATCH v4 0/2] Implement a software workaround for Falkor erratum 1041 Shanker Donthineni
2017-11-27 23:17 ` [PATCH v4 1/2] arm64: Define cputype macros for Falkor CPU Shanker Donthineni
2017-11-27 23:18 ` [PATCH v4 2/2] arm64: Add software workaround for Falkor erratum 1041 Shanker Donthineni
2017-12-01 11:24   ` Will Deacon
2017-12-03 13:35     ` Shanker Donthineni
2017-12-04 20:09       ` Shanker Donthineni

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).