All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/6] arm64: Extend Cortex-A53 errata workaround
@ 2016-05-09 16:49 ` Andre Przywara
  0 siblings, 0 replies; 30+ messages in thread
From: Andre Przywara @ 2016-05-09 16:49 UTC (permalink / raw)
  To: Will Deacon, Catalin Marinas; +Cc: linux-arm-kernel, linux-kernel

According to the errata documentation for the ARM errata 819472, 826319,
827319 and 824069, in addition to the already covered promotion of
"dc cvac" cache maintenance instructions to "dc civac"[1], we also need
to promote "dc cvau" operations.
Also as cache maintenance instructions on ARMv8 can be issued by EL0 as
well, we unfortunately have to promote them too, which is only possible
by means of trap-and-emulate.

These patches cover all in-kernel users of "dc cvau" and make sure
they are using "dc civac" if run on an affected core.
In addition if at least one core in the system has one of the above
mentioned erratas, we set the respective bit in SCTLR to trap cache
maintenance instructions from EL0 to EL1 on all CPUs, where we "emulate"
them by executing the potentially fixed instruction on behalf of userspace.

Apart from the actual patches 2/6 and 6/6, which do the main work, the
other patches are cleanups and do refactoring to make the promotion and
trapping of EL0 cache maintenance easier.

Tested on a Juno R0 with an userspace tool to issue various cache
maintenance instructions (including one with triggers a SIGSEGV) and
verified with some debugfs entries.
At least one LTP test also issues around 100 cache maintenance
instructions, which this code survived happily.

Cheers,
Andre.

[1] commit 301bcfac4289 ("arm64: add Cortex-A53 cache errata workaround")

Andre Przywara (6):
  arm64: alternatives: drop enable parameter from _else and _endif macro
  arm64: fix "dc cvau" cache operation on errata-affected core
  arm64: include alternative handling in dcache_by_line_op
  arm64: errata: Calling enable functions for CPU errata too
  arm64: consolidate signal injection on emulation errors
  arm64: trap userspace "dc cvau" cache operation on errata-affected
    core

 arch/arm64/include/asm/alternative.h |  28 +++++++--
 arch/arm64/include/asm/cpufeature.h  |   2 +
 arch/arm64/include/asm/processor.h   |   1 +
 arch/arm64/include/asm/sysreg.h      |   2 +-
 arch/arm64/include/asm/traps.h       |   3 +
 arch/arm64/kernel/armv8_deprecated.c |  13 ++--
 arch/arm64/kernel/cpu_errata.c       |   7 +++
 arch/arm64/kernel/cpufeature.c       |   4 +-
 arch/arm64/kernel/entry.S            |  12 +++-
 arch/arm64/kernel/traps.c            | 111 +++++++++++++++++++++++++++++++----
 arch/arm64/mm/cache.S                |   2 +-
 arch/arm64/mm/proc-macros.S          |   9 ++-
 12 files changed, 164 insertions(+), 30 deletions(-)

-- 
2.7.3

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

* [PATCH 0/6] arm64: Extend Cortex-A53 errata workaround
@ 2016-05-09 16:49 ` Andre Przywara
  0 siblings, 0 replies; 30+ messages in thread
From: Andre Przywara @ 2016-05-09 16:49 UTC (permalink / raw)
  To: linux-arm-kernel

According to the errata documentation for the ARM errata 819472, 826319,
827319 and 824069, in addition to the already covered promotion of
"dc cvac" cache maintenance instructions to "dc civac"[1], we also need
to promote "dc cvau" operations.
Also as cache maintenance instructions on ARMv8 can be issued by EL0 as
well, we unfortunately have to promote them too, which is only possible
by means of trap-and-emulate.

These patches cover all in-kernel users of "dc cvau" and make sure
they are using "dc civac" if run on an affected core.
In addition if at least one core in the system has one of the above
mentioned erratas, we set the respective bit in SCTLR to trap cache
maintenance instructions from EL0 to EL1 on all CPUs, where we "emulate"
them by executing the potentially fixed instruction on behalf of userspace.

Apart from the actual patches 2/6 and 6/6, which do the main work, the
other patches are cleanups and do refactoring to make the promotion and
trapping of EL0 cache maintenance easier.

Tested on a Juno R0 with an userspace tool to issue various cache
maintenance instructions (including one with triggers a SIGSEGV) and
verified with some debugfs entries.
At least one LTP test also issues around 100 cache maintenance
instructions, which this code survived happily.

Cheers,
Andre.

[1] commit 301bcfac4289 ("arm64: add Cortex-A53 cache errata workaround")

Andre Przywara (6):
  arm64: alternatives: drop enable parameter from _else and _endif macro
  arm64: fix "dc cvau" cache operation on errata-affected core
  arm64: include alternative handling in dcache_by_line_op
  arm64: errata: Calling enable functions for CPU errata too
  arm64: consolidate signal injection on emulation errors
  arm64: trap userspace "dc cvau" cache operation on errata-affected
    core

 arch/arm64/include/asm/alternative.h |  28 +++++++--
 arch/arm64/include/asm/cpufeature.h  |   2 +
 arch/arm64/include/asm/processor.h   |   1 +
 arch/arm64/include/asm/sysreg.h      |   2 +-
 arch/arm64/include/asm/traps.h       |   3 +
 arch/arm64/kernel/armv8_deprecated.c |  13 ++--
 arch/arm64/kernel/cpu_errata.c       |   7 +++
 arch/arm64/kernel/cpufeature.c       |   4 +-
 arch/arm64/kernel/entry.S            |  12 +++-
 arch/arm64/kernel/traps.c            | 111 +++++++++++++++++++++++++++++++----
 arch/arm64/mm/cache.S                |   2 +-
 arch/arm64/mm/proc-macros.S          |   9 ++-
 12 files changed, 164 insertions(+), 30 deletions(-)

-- 
2.7.3

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

* [PATCH 1/6] arm64: alternatives: drop enable parameter from _else and _endif macro
  2016-05-09 16:49 ` Andre Przywara
@ 2016-05-09 16:49   ` Andre Przywara
  -1 siblings, 0 replies; 30+ messages in thread
From: Andre Przywara @ 2016-05-09 16:49 UTC (permalink / raw)
  To: Will Deacon, Catalin Marinas; +Cc: linux-arm-kernel, linux-kernel

Commit 77ee306c0aea9 ("arm64: alternatives: add enable parameter to
conditional asm macros") extended the alternative assembly macros.
Unfortunately this does not really work as one would expect, as the
enable parameter in fact correctly protects the alternative section
magic, but not the actual code sequences.
So if enable is false, we will have the original instruction(s) _and_
the alternative ones in the file, which is just wrong.
To make this work one would need to additionally protect the
alternative sequence with extra .if directives, which makes
the intention of the enable parameter rather pointless.
Instead users should directly guard the whole "_else; insn; _endif"
sequence with .if directives.
Add a comment describing this usage and drop the enable parameter from
the alternative_else and alternative_endif macros.

This reverts parts of commit 77ee306c0aea ("arm64: alternatives: add
enable parameter to conditional asm macros").

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 arch/arm64/include/asm/alternative.h | 24 ++++++++++++++++++------
 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/arch/arm64/include/asm/alternative.h b/arch/arm64/include/asm/alternative.h
index beccbde..502c9ef 100644
--- a/arch/arm64/include/asm/alternative.h
+++ b/arch/arm64/include/asm/alternative.h
@@ -94,6 +94,8 @@ void apply_alternatives(void *start, size_t length);
  *
  * The code that follows this macro will be assembled and linked as
  * normal. There are no restrictions on this code.
+ * If you use the enable parameter, see the comments below for _else
+ * and _endif.
  */
 .macro alternative_if_not cap, enable = 1
 	.if \enable
@@ -117,23 +119,33 @@ void apply_alternatives(void *start, size_t length);
  * 2. Not contain a branch target that is used outside of the
  *    alternative sequence it is defined in (branches into an
  *    alternative sequence are not fixed up).
+ *
+ * If you used the optional enable parameter in the opening
+ * alternative_if_not macro above, please protect the whole _else
+ * branch with an .if directive:
+ *	alternative_if_not CAP_SOMETHING, condition
+ *		orig_insn
+ *	.if condition
+ *	alternative_else
+ *		repl_insn
+ *	alternative_endif
+ *	.endif
  */
-.macro alternative_else, enable = 1
-	.if \enable
+.macro alternative_else
 662:	.pushsection .altinstr_replacement, "ax"
 663:
-	.endif
 .endm
 
 /*
  * Complete an alternative code sequence.
+ *
+ * Please mind the comment at alternative_else above if you used the
+ * optional enable parameter with the opening alternative_if_not macro.
  */
-.macro alternative_endif, enable = 1
-	.if \enable
+.macro alternative_endif
 664:	.popsection
 	.org	. - (664b-663b) + (662b-661b)
 	.org	. - (662b-661b) + (664b-663b)
-	.endif
 .endm
 
 #define _ALTERNATIVE_CFG(insn1, insn2, cap, cfg, ...)	\
-- 
2.7.3

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

* [PATCH 1/6] arm64: alternatives: drop enable parameter from _else and _endif macro
@ 2016-05-09 16:49   ` Andre Przywara
  0 siblings, 0 replies; 30+ messages in thread
From: Andre Przywara @ 2016-05-09 16:49 UTC (permalink / raw)
  To: linux-arm-kernel

Commit 77ee306c0aea9 ("arm64: alternatives: add enable parameter to
conditional asm macros") extended the alternative assembly macros.
Unfortunately this does not really work as one would expect, as the
enable parameter in fact correctly protects the alternative section
magic, but not the actual code sequences.
So if enable is false, we will have the original instruction(s) _and_
the alternative ones in the file, which is just wrong.
To make this work one would need to additionally protect the
alternative sequence with extra .if directives, which makes
the intention of the enable parameter rather pointless.
Instead users should directly guard the whole "_else; insn; _endif"
sequence with .if directives.
Add a comment describing this usage and drop the enable parameter from
the alternative_else and alternative_endif macros.

This reverts parts of commit 77ee306c0aea ("arm64: alternatives: add
enable parameter to conditional asm macros").

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 arch/arm64/include/asm/alternative.h | 24 ++++++++++++++++++------
 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/arch/arm64/include/asm/alternative.h b/arch/arm64/include/asm/alternative.h
index beccbde..502c9ef 100644
--- a/arch/arm64/include/asm/alternative.h
+++ b/arch/arm64/include/asm/alternative.h
@@ -94,6 +94,8 @@ void apply_alternatives(void *start, size_t length);
  *
  * The code that follows this macro will be assembled and linked as
  * normal. There are no restrictions on this code.
+ * If you use the enable parameter, see the comments below for _else
+ * and _endif.
  */
 .macro alternative_if_not cap, enable = 1
 	.if \enable
@@ -117,23 +119,33 @@ void apply_alternatives(void *start, size_t length);
  * 2. Not contain a branch target that is used outside of the
  *    alternative sequence it is defined in (branches into an
  *    alternative sequence are not fixed up).
+ *
+ * If you used the optional enable parameter in the opening
+ * alternative_if_not macro above, please protect the whole _else
+ * branch with an .if directive:
+ *	alternative_if_not CAP_SOMETHING, condition
+ *		orig_insn
+ *	.if condition
+ *	alternative_else
+ *		repl_insn
+ *	alternative_endif
+ *	.endif
  */
-.macro alternative_else, enable = 1
-	.if \enable
+.macro alternative_else
 662:	.pushsection .altinstr_replacement, "ax"
 663:
-	.endif
 .endm
 
 /*
  * Complete an alternative code sequence.
+ *
+ * Please mind the comment at alternative_else above if you used the
+ * optional enable parameter with the opening alternative_if_not macro.
  */
-.macro alternative_endif, enable = 1
-	.if \enable
+.macro alternative_endif
 664:	.popsection
 	.org	. - (664b-663b) + (662b-661b)
 	.org	. - (662b-661b) + (664b-663b)
-	.endif
 .endm
 
 #define _ALTERNATIVE_CFG(insn1, insn2, cap, cfg, ...)	\
-- 
2.7.3

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

* [PATCH 2/6] arm64: fix "dc cvau" cache operation on errata-affected core
  2016-05-09 16:49 ` Andre Przywara
@ 2016-05-09 16:49   ` Andre Przywara
  -1 siblings, 0 replies; 30+ messages in thread
From: Andre Przywara @ 2016-05-09 16:49 UTC (permalink / raw)
  To: Will Deacon, Catalin Marinas; +Cc: linux-arm-kernel, linux-kernel

The ARM errata 819472, 826319, 827319 and 824069 for affected
Cortex-A53 cores demand to promote "dc cvau" instructions to
"dc civac" as well.
Attribute the usage of the instruction in __flush_cache_user_range
to also be covered by our alternative patching efforts.
For that we introduce an assembly macro which both deals with
alternatives while still tagging the instructions as USER.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 arch/arm64/include/asm/alternative.h | 4 ++++
 arch/arm64/mm/cache.S                | 2 +-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/include/asm/alternative.h b/arch/arm64/include/asm/alternative.h
index 502c9ef..d4a69a9 100644
--- a/arch/arm64/include/asm/alternative.h
+++ b/arch/arm64/include/asm/alternative.h
@@ -151,6 +151,10 @@ void apply_alternatives(void *start, size_t length);
 #define _ALTERNATIVE_CFG(insn1, insn2, cap, cfg, ...)	\
 	alternative_insn insn1, insn2, cap, IS_ENABLED(cfg)
 
+.macro user_alt, label, oldinstr, newinstr, cond
+9999:	alternative_insn "\oldinstr", "\newinstr", \cond
+	_ASM_EXTABLE 9999b, \label
+.endm
 
 /*
  * Generate the assembly for UAO alternatives with exception table entries.
diff --git a/arch/arm64/mm/cache.S b/arch/arm64/mm/cache.S
index 6df0706..82f40bd 100644
--- a/arch/arm64/mm/cache.S
+++ b/arch/arm64/mm/cache.S
@@ -54,7 +54,7 @@ ENTRY(__flush_cache_user_range)
 	sub	x3, x2, #1
 	bic	x4, x0, x3
 1:
-USER(9f, dc	cvau, x4	)		// clean D line to PoU
+user_alt 9f, "dc cvau, x4",  "dc civac, x4",  ARM64_WORKAROUND_CLEAN_CACHE
 	add	x4, x4, x2
 	cmp	x4, x1
 	b.lo	1b
-- 
2.7.3

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

* [PATCH 2/6] arm64: fix "dc cvau" cache operation on errata-affected core
@ 2016-05-09 16:49   ` Andre Przywara
  0 siblings, 0 replies; 30+ messages in thread
From: Andre Przywara @ 2016-05-09 16:49 UTC (permalink / raw)
  To: linux-arm-kernel

The ARM errata 819472, 826319, 827319 and 824069 for affected
Cortex-A53 cores demand to promote "dc cvau" instructions to
"dc civac" as well.
Attribute the usage of the instruction in __flush_cache_user_range
to also be covered by our alternative patching efforts.
For that we introduce an assembly macro which both deals with
alternatives while still tagging the instructions as USER.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 arch/arm64/include/asm/alternative.h | 4 ++++
 arch/arm64/mm/cache.S                | 2 +-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/include/asm/alternative.h b/arch/arm64/include/asm/alternative.h
index 502c9ef..d4a69a9 100644
--- a/arch/arm64/include/asm/alternative.h
+++ b/arch/arm64/include/asm/alternative.h
@@ -151,6 +151,10 @@ void apply_alternatives(void *start, size_t length);
 #define _ALTERNATIVE_CFG(insn1, insn2, cap, cfg, ...)	\
 	alternative_insn insn1, insn2, cap, IS_ENABLED(cfg)
 
+.macro user_alt, label, oldinstr, newinstr, cond
+9999:	alternative_insn "\oldinstr", "\newinstr", \cond
+	_ASM_EXTABLE 9999b, \label
+.endm
 
 /*
  * Generate the assembly for UAO alternatives with exception table entries.
diff --git a/arch/arm64/mm/cache.S b/arch/arm64/mm/cache.S
index 6df0706..82f40bd 100644
--- a/arch/arm64/mm/cache.S
+++ b/arch/arm64/mm/cache.S
@@ -54,7 +54,7 @@ ENTRY(__flush_cache_user_range)
 	sub	x3, x2, #1
 	bic	x4, x0, x3
 1:
-USER(9f, dc	cvau, x4	)		// clean D line to PoU
+user_alt 9f, "dc cvau, x4",  "dc civac, x4",  ARM64_WORKAROUND_CLEAN_CACHE
 	add	x4, x4, x2
 	cmp	x4, x1
 	b.lo	1b
-- 
2.7.3

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

* [PATCH 3/6] arm64: include alternative handling in dcache_by_line_op
  2016-05-09 16:49 ` Andre Przywara
@ 2016-05-09 16:49   ` Andre Przywara
  -1 siblings, 0 replies; 30+ messages in thread
From: Andre Przywara @ 2016-05-09 16:49 UTC (permalink / raw)
  To: Will Deacon, Catalin Marinas; +Cc: linux-arm-kernel, linux-kernel

The newly introduced dcache_by_line_op macro is used at least in
one occassion at the moment to issue a "dc cvau" instruction,
which is affected by ARM errata 819472, 826319, 827319 and 824069.
Change the macro to allow for alternative patching in there to
protect affected Cortex-A53 cores.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 arch/arm64/mm/proc-macros.S | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/mm/proc-macros.S b/arch/arm64/mm/proc-macros.S
index e6a30e1..5786017 100644
--- a/arch/arm64/mm/proc-macros.S
+++ b/arch/arm64/mm/proc-macros.S
@@ -78,7 +78,14 @@
 	add	\size, \kaddr, \size
 	sub	\tmp2, \tmp1, #1
 	bic	\kaddr, \kaddr, \tmp2
-9998:	dc	\op, \kaddr
+9998:
+	alternative_if_not ARM64_WORKAROUND_CLEAN_CACHE, (\op == cvau || \op == cvac)
+	dc	\op, \kaddr
+	.if	(\op == cvau || \op == cvac)
+	alternative_else
+	dc	civac, \kaddr
+	alternative_endif
+	.endif
 	add	\kaddr, \kaddr, \tmp1
 	cmp	\kaddr, \size
 	b.lo	9998b
-- 
2.7.3

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

* [PATCH 3/6] arm64: include alternative handling in dcache_by_line_op
@ 2016-05-09 16:49   ` Andre Przywara
  0 siblings, 0 replies; 30+ messages in thread
From: Andre Przywara @ 2016-05-09 16:49 UTC (permalink / raw)
  To: linux-arm-kernel

The newly introduced dcache_by_line_op macro is used at least in
one occassion at the moment to issue a "dc cvau" instruction,
which is affected by ARM errata 819472, 826319, 827319 and 824069.
Change the macro to allow for alternative patching in there to
protect affected Cortex-A53 cores.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 arch/arm64/mm/proc-macros.S | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/mm/proc-macros.S b/arch/arm64/mm/proc-macros.S
index e6a30e1..5786017 100644
--- a/arch/arm64/mm/proc-macros.S
+++ b/arch/arm64/mm/proc-macros.S
@@ -78,7 +78,14 @@
 	add	\size, \kaddr, \size
 	sub	\tmp2, \tmp1, #1
 	bic	\kaddr, \kaddr, \tmp2
-9998:	dc	\op, \kaddr
+9998:
+	alternative_if_not ARM64_WORKAROUND_CLEAN_CACHE, (\op == cvau || \op == cvac)
+	dc	\op, \kaddr
+	.if	(\op == cvau || \op == cvac)
+	alternative_else
+	dc	civac, \kaddr
+	alternative_endif
+	.endif
 	add	\kaddr, \kaddr, \tmp1
 	cmp	\kaddr, \size
 	b.lo	9998b
-- 
2.7.3

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

* [PATCH 4/6] arm64: errata: Calling enable functions for CPU errata too
  2016-05-09 16:49 ` Andre Przywara
@ 2016-05-09 16:49   ` Andre Przywara
  -1 siblings, 0 replies; 30+ messages in thread
From: Andre Przywara @ 2016-05-09 16:49 UTC (permalink / raw)
  To: Will Deacon, Catalin Marinas; +Cc: linux-arm-kernel, linux-kernel

Currently we call the (optional) enable function for CPU _features_
only. As CPU _errata_ descriptions share the same data structure and
having an enable function is useful for errata as well (for instance
to set bits in SCTLR), lets call it when enumerating erratas too.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 arch/arm64/include/asm/cpufeature.h | 2 ++
 arch/arm64/kernel/cpu_errata.c      | 5 +++++
 arch/arm64/kernel/cpufeature.c      | 4 ++--
 3 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
index b9b6494..cb8fb2c 100644
--- a/arch/arm64/include/asm/cpufeature.h
+++ b/arch/arm64/include/asm/cpufeature.h
@@ -174,7 +174,9 @@ void __init setup_cpu_features(void);
 
 void update_cpu_capabilities(const struct arm64_cpu_capabilities *caps,
 			    const char *info);
+void enable_cpu_capabilities(const struct arm64_cpu_capabilities *caps);
 void check_local_cpu_errata(void);
+void __init enable_cpu_errata(void);
 
 void verify_local_cpu_capabilities(void);
 
diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
index 06afd04..64cb71d 100644
--- a/arch/arm64/kernel/cpu_errata.c
+++ b/arch/arm64/kernel/cpu_errata.c
@@ -105,3 +105,8 @@ void check_local_cpu_errata(void)
 {
 	update_cpu_capabilities(arm64_errata, "enabling workaround for");
 }
+
+void __init enable_cpu_errata(void)
+{
+	enable_cpu_capabilities(arm64_errata);
+}
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index 943f514..04a324f 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -835,8 +835,7 @@ void update_cpu_capabilities(const struct arm64_cpu_capabilities *caps,
  * Run through the enabled capabilities and enable() it on all active
  * CPUs
  */
-static void __init
-enable_cpu_capabilities(const struct arm64_cpu_capabilities *caps)
+void __init enable_cpu_capabilities(const struct arm64_cpu_capabilities *caps)
 {
 	int i;
 
@@ -974,6 +973,7 @@ void __init setup_cpu_features(void)
 
 	/* Set the CPU feature capabilies */
 	setup_feature_capabilities();
+	enable_cpu_errata();
 	setup_cpu_hwcaps();
 
 	/* Advertise that we have computed the system capabilities */
-- 
2.7.3

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

* [PATCH 4/6] arm64: errata: Calling enable functions for CPU errata too
@ 2016-05-09 16:49   ` Andre Przywara
  0 siblings, 0 replies; 30+ messages in thread
From: Andre Przywara @ 2016-05-09 16:49 UTC (permalink / raw)
  To: linux-arm-kernel

Currently we call the (optional) enable function for CPU _features_
only. As CPU _errata_ descriptions share the same data structure and
having an enable function is useful for errata as well (for instance
to set bits in SCTLR), lets call it when enumerating erratas too.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 arch/arm64/include/asm/cpufeature.h | 2 ++
 arch/arm64/kernel/cpu_errata.c      | 5 +++++
 arch/arm64/kernel/cpufeature.c      | 4 ++--
 3 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
index b9b6494..cb8fb2c 100644
--- a/arch/arm64/include/asm/cpufeature.h
+++ b/arch/arm64/include/asm/cpufeature.h
@@ -174,7 +174,9 @@ void __init setup_cpu_features(void);
 
 void update_cpu_capabilities(const struct arm64_cpu_capabilities *caps,
 			    const char *info);
+void enable_cpu_capabilities(const struct arm64_cpu_capabilities *caps);
 void check_local_cpu_errata(void);
+void __init enable_cpu_errata(void);
 
 void verify_local_cpu_capabilities(void);
 
diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
index 06afd04..64cb71d 100644
--- a/arch/arm64/kernel/cpu_errata.c
+++ b/arch/arm64/kernel/cpu_errata.c
@@ -105,3 +105,8 @@ void check_local_cpu_errata(void)
 {
 	update_cpu_capabilities(arm64_errata, "enabling workaround for");
 }
+
+void __init enable_cpu_errata(void)
+{
+	enable_cpu_capabilities(arm64_errata);
+}
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index 943f514..04a324f 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -835,8 +835,7 @@ void update_cpu_capabilities(const struct arm64_cpu_capabilities *caps,
  * Run through the enabled capabilities and enable() it on all active
  * CPUs
  */
-static void __init
-enable_cpu_capabilities(const struct arm64_cpu_capabilities *caps)
+void __init enable_cpu_capabilities(const struct arm64_cpu_capabilities *caps)
 {
 	int i;
 
@@ -974,6 +973,7 @@ void __init setup_cpu_features(void)
 
 	/* Set the CPU feature capabilies */
 	setup_feature_capabilities();
+	enable_cpu_errata();
 	setup_cpu_hwcaps();
 
 	/* Advertise that we have computed the system capabilities */
-- 
2.7.3

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

* [PATCH 5/6] arm64: consolidate signal injection on emulation errors
  2016-05-09 16:49 ` Andre Przywara
@ 2016-05-09 16:49   ` Andre Przywara
  -1 siblings, 0 replies; 30+ messages in thread
From: Andre Przywara @ 2016-05-09 16:49 UTC (permalink / raw)
  To: Will Deacon, Catalin Marinas; +Cc: linux-arm-kernel, linux-kernel

The code for injecting a signal into userland if a trapped instruction
fails emulation due to a _userland_ error (like an illegal address)
will be used more often with the next patch.
Factor out the core functionality into a separate function and use
that both for the existing trap handler and for the deprecated
instructions emulation.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 arch/arm64/include/asm/traps.h       |  3 +++
 arch/arm64/kernel/armv8_deprecated.c | 13 ++++------
 arch/arm64/kernel/traps.c            | 46 ++++++++++++++++++++++++++----------
 3 files changed, 41 insertions(+), 21 deletions(-)

diff --git a/arch/arm64/include/asm/traps.h b/arch/arm64/include/asm/traps.h
index 0cc2f29..b20ce2f 100644
--- a/arch/arm64/include/asm/traps.h
+++ b/arch/arm64/include/asm/traps.h
@@ -34,6 +34,9 @@ struct undef_hook {
 void register_undef_hook(struct undef_hook *hook);
 void unregister_undef_hook(struct undef_hook *hook);
 
+void force_signal_inject(int signal, int code, struct pt_regs *regs,
+			 unsigned long address);
+
 #ifdef CONFIG_FUNCTION_GRAPH_TRACER
 static inline int __in_irqentry_text(unsigned long ptr)
 {
diff --git a/arch/arm64/kernel/armv8_deprecated.c b/arch/arm64/kernel/armv8_deprecated.c
index c37202c..4003843 100644
--- a/arch/arm64/kernel/armv8_deprecated.c
+++ b/arch/arm64/kernel/armv8_deprecated.c
@@ -321,21 +321,18 @@ static void __init register_insn_emulation_sysctl(struct ctl_table *table)
  */
 static void set_segfault(struct pt_regs *regs, unsigned long addr)
 {
-	siginfo_t info;
+	int code;
 
 	down_read(&current->mm->mmap_sem);
 	if (find_vma(current->mm, addr) == NULL)
-		info.si_code = SEGV_MAPERR;
+		code = SEGV_MAPERR;
 	else
-		info.si_code = SEGV_ACCERR;
+		code = SEGV_ACCERR;
 	up_read(&current->mm->mmap_sem);
 
-	info.si_signo = SIGSEGV;
-	info.si_errno = 0;
-	info.si_addr  = (void *) instruction_pointer(regs);
-
 	pr_debug("SWP{B} emulation: access caused memory abort!\n");
-	arm64_notify_die("Illegal memory access", regs, &info, 0);
+
+	force_signal_inject(SIGSEGV, code, regs, addr);
 }
 
 static int emulate_swpX(unsigned int address, unsigned int *data,
diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
index c539208..03755a4 100644
--- a/arch/arm64/kernel/traps.c
+++ b/arch/arm64/kernel/traps.c
@@ -373,30 +373,50 @@ exit:
 	return fn ? fn(regs, instr) : 1;
 }
 
-asmlinkage void __exception do_undefinstr(struct pt_regs *regs)
+void force_signal_inject(int signal, int code, struct pt_regs *regs,
+			 unsigned long address)
 {
 	siginfo_t info;
 	void __user *pc = (void __user *)instruction_pointer(regs);
+	const char *desc;
 
-	/* check for AArch32 breakpoint instructions */
-	if (!aarch32_break_handler(regs))
-		return;
-
-	if (call_undef_hook(regs) == 0)
-		return;
+	switch (signal) {
+	case SIGILL:
+		desc = "undefined instruction";
+		break;
+	case SIGSEGV:
+		desc = "illegal memory access";
+		break;
+	default:
+		desc = "bad mode";
+		break;
+	}
 
-	if (unhandled_signal(current, SIGILL) && show_unhandled_signals_ratelimited()) {
-		pr_info("%s[%d]: undefined instruction: pc=%p\n",
-			current->comm, task_pid_nr(current), pc);
+	if (unhandled_signal(current, signal) &&
+	    show_unhandled_signals_ratelimited()) {
+		pr_info("%s[%d]: %s: pc=%p\n",
+			current->comm, task_pid_nr(current), desc, pc);
 		dump_instr(KERN_INFO, regs);
 	}
 
-	info.si_signo = SIGILL;
+	info.si_signo = signal;
 	info.si_errno = 0;
-	info.si_code  = ILL_ILLOPC;
+	info.si_code  = code;
 	info.si_addr  = pc;
 
-	arm64_notify_die("Oops - undefined instruction", regs, &info, 0);
+	arm64_notify_die(desc, regs, &info, 0);
+}
+
+asmlinkage void __exception do_undefinstr(struct pt_regs *regs)
+{
+	/* check for AArch32 breakpoint instructions */
+	if (!aarch32_break_handler(regs))
+		return;
+
+	if (call_undef_hook(regs) == 0)
+		return;
+
+	force_signal_inject(SIGILL, ILL_ILLOPC, regs, 0);
 }
 
 long compat_arm_syscall(struct pt_regs *regs);
-- 
2.7.3

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

* [PATCH 5/6] arm64: consolidate signal injection on emulation errors
@ 2016-05-09 16:49   ` Andre Przywara
  0 siblings, 0 replies; 30+ messages in thread
From: Andre Przywara @ 2016-05-09 16:49 UTC (permalink / raw)
  To: linux-arm-kernel

The code for injecting a signal into userland if a trapped instruction
fails emulation due to a _userland_ error (like an illegal address)
will be used more often with the next patch.
Factor out the core functionality into a separate function and use
that both for the existing trap handler and for the deprecated
instructions emulation.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 arch/arm64/include/asm/traps.h       |  3 +++
 arch/arm64/kernel/armv8_deprecated.c | 13 ++++------
 arch/arm64/kernel/traps.c            | 46 ++++++++++++++++++++++++++----------
 3 files changed, 41 insertions(+), 21 deletions(-)

diff --git a/arch/arm64/include/asm/traps.h b/arch/arm64/include/asm/traps.h
index 0cc2f29..b20ce2f 100644
--- a/arch/arm64/include/asm/traps.h
+++ b/arch/arm64/include/asm/traps.h
@@ -34,6 +34,9 @@ struct undef_hook {
 void register_undef_hook(struct undef_hook *hook);
 void unregister_undef_hook(struct undef_hook *hook);
 
+void force_signal_inject(int signal, int code, struct pt_regs *regs,
+			 unsigned long address);
+
 #ifdef CONFIG_FUNCTION_GRAPH_TRACER
 static inline int __in_irqentry_text(unsigned long ptr)
 {
diff --git a/arch/arm64/kernel/armv8_deprecated.c b/arch/arm64/kernel/armv8_deprecated.c
index c37202c..4003843 100644
--- a/arch/arm64/kernel/armv8_deprecated.c
+++ b/arch/arm64/kernel/armv8_deprecated.c
@@ -321,21 +321,18 @@ static void __init register_insn_emulation_sysctl(struct ctl_table *table)
  */
 static void set_segfault(struct pt_regs *regs, unsigned long addr)
 {
-	siginfo_t info;
+	int code;
 
 	down_read(&current->mm->mmap_sem);
 	if (find_vma(current->mm, addr) == NULL)
-		info.si_code = SEGV_MAPERR;
+		code = SEGV_MAPERR;
 	else
-		info.si_code = SEGV_ACCERR;
+		code = SEGV_ACCERR;
 	up_read(&current->mm->mmap_sem);
 
-	info.si_signo = SIGSEGV;
-	info.si_errno = 0;
-	info.si_addr  = (void *) instruction_pointer(regs);
-
 	pr_debug("SWP{B} emulation: access caused memory abort!\n");
-	arm64_notify_die("Illegal memory access", regs, &info, 0);
+
+	force_signal_inject(SIGSEGV, code, regs, addr);
 }
 
 static int emulate_swpX(unsigned int address, unsigned int *data,
diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
index c539208..03755a4 100644
--- a/arch/arm64/kernel/traps.c
+++ b/arch/arm64/kernel/traps.c
@@ -373,30 +373,50 @@ exit:
 	return fn ? fn(regs, instr) : 1;
 }
 
-asmlinkage void __exception do_undefinstr(struct pt_regs *regs)
+void force_signal_inject(int signal, int code, struct pt_regs *regs,
+			 unsigned long address)
 {
 	siginfo_t info;
 	void __user *pc = (void __user *)instruction_pointer(regs);
+	const char *desc;
 
-	/* check for AArch32 breakpoint instructions */
-	if (!aarch32_break_handler(regs))
-		return;
-
-	if (call_undef_hook(regs) == 0)
-		return;
+	switch (signal) {
+	case SIGILL:
+		desc = "undefined instruction";
+		break;
+	case SIGSEGV:
+		desc = "illegal memory access";
+		break;
+	default:
+		desc = "bad mode";
+		break;
+	}
 
-	if (unhandled_signal(current, SIGILL) && show_unhandled_signals_ratelimited()) {
-		pr_info("%s[%d]: undefined instruction: pc=%p\n",
-			current->comm, task_pid_nr(current), pc);
+	if (unhandled_signal(current, signal) &&
+	    show_unhandled_signals_ratelimited()) {
+		pr_info("%s[%d]: %s: pc=%p\n",
+			current->comm, task_pid_nr(current), desc, pc);
 		dump_instr(KERN_INFO, regs);
 	}
 
-	info.si_signo = SIGILL;
+	info.si_signo = signal;
 	info.si_errno = 0;
-	info.si_code  = ILL_ILLOPC;
+	info.si_code  = code;
 	info.si_addr  = pc;
 
-	arm64_notify_die("Oops - undefined instruction", regs, &info, 0);
+	arm64_notify_die(desc, regs, &info, 0);
+}
+
+asmlinkage void __exception do_undefinstr(struct pt_regs *regs)
+{
+	/* check for AArch32 breakpoint instructions */
+	if (!aarch32_break_handler(regs))
+		return;
+
+	if (call_undef_hook(regs) == 0)
+		return;
+
+	force_signal_inject(SIGILL, ILL_ILLOPC, regs, 0);
 }
 
 long compat_arm_syscall(struct pt_regs *regs);
-- 
2.7.3

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

* [PATCH 6/6] arm64: trap userspace "dc cvau" cache operation on errata-affected core
  2016-05-09 16:49 ` Andre Przywara
@ 2016-05-09 16:49   ` Andre Przywara
  -1 siblings, 0 replies; 30+ messages in thread
From: Andre Przywara @ 2016-05-09 16:49 UTC (permalink / raw)
  To: Will Deacon, Catalin Marinas; +Cc: linux-arm-kernel, linux-kernel

The ARM errata 819472, 826319, 827319 and 824069 for affected
Cortex-A53 cores demand to promote "dc cvau" instructions to
"dc civac". Since we allow userspace to also emit those instructions,
we should make sure that "dc cvau" gets promoted there too.
So lets grasp the nettle here and actually trap every userland cache
maintenance instruction once we detect at least one affected core in
the system.
We then emulate the instruction by executing it on behalf of userland,
promoting "dc cvau" to "dc civac" on the way and injecting access
fault back into userspace.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 arch/arm64/include/asm/processor.h |  1 +
 arch/arm64/include/asm/sysreg.h    |  2 +-
 arch/arm64/kernel/cpu_errata.c     |  2 ++
 arch/arm64/kernel/entry.S          | 12 ++++++-
 arch/arm64/kernel/traps.c          | 71 ++++++++++++++++++++++++++++++++++++++
 5 files changed, 86 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/include/asm/processor.h b/arch/arm64/include/asm/processor.h
index cef1cf3..ace0a96 100644
--- a/arch/arm64/include/asm/processor.h
+++ b/arch/arm64/include/asm/processor.h
@@ -192,5 +192,6 @@ static inline void spin_lock_prefetch(const void *ptr)
 
 void cpu_enable_pan(void *__unused);
 void cpu_enable_uao(void *__unused);
+void cpu_enable_cache_maint_trap(void *__unused);
 
 #endif /* __ASM_PROCESSOR_H */
diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
index 1287416..7e0b097 100644
--- a/arch/arm64/include/asm/sysreg.h
+++ b/arch/arm64/include/asm/sysreg.h
@@ -90,7 +90,7 @@
 #define SCTLR_EL1_CP15BEN	(0x1 << 5)
 #define SCTLR_EL1_SED		(0x1 << 8)
 #define SCTLR_EL1_SPAN		(0x1 << 23)
-
+#define SCTLR_EL1_UCI		(0x1 << 26)
 
 /* id_aa64isar0 */
 #define ID_AA64ISAR0_RDM_SHIFT		28
diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
index 64cb71d..3f54d23 100644
--- a/arch/arm64/kernel/cpu_errata.c
+++ b/arch/arm64/kernel/cpu_errata.c
@@ -44,6 +44,7 @@ const struct arm64_cpu_capabilities arm64_errata[] = {
 		.desc = "ARM errata 826319, 827319, 824069",
 		.capability = ARM64_WORKAROUND_CLEAN_CACHE,
 		MIDR_RANGE(MIDR_CORTEX_A53, 0x00, 0x02),
+		.enable = cpu_enable_cache_maint_trap,
 	},
 #endif
 #ifdef CONFIG_ARM64_ERRATUM_819472
@@ -52,6 +53,7 @@ const struct arm64_cpu_capabilities arm64_errata[] = {
 		.desc = "ARM errata 819472",
 		.capability = ARM64_WORKAROUND_CLEAN_CACHE,
 		MIDR_RANGE(MIDR_CORTEX_A53, 0x00, 0x01),
+		.enable = cpu_enable_cache_maint_trap,
 	},
 #endif
 #ifdef CONFIG_ARM64_ERRATUM_832075
diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
index 12e8d2b..86480b4 100644
--- a/arch/arm64/kernel/entry.S
+++ b/arch/arm64/kernel/entry.S
@@ -451,7 +451,7 @@ el0_sync:
 	cmp	x24, #ESR_ELx_EC_FP_EXC64	// FP/ASIMD exception
 	b.eq	el0_fpsimd_exc
 	cmp	x24, #ESR_ELx_EC_SYS64		// configurable trap
-	b.eq	el0_undef
+	b.eq	el0_sys
 	cmp	x24, #ESR_ELx_EC_SP_ALIGN	// stack alignment exception
 	b.eq	el0_sp_pc
 	cmp	x24, #ESR_ELx_EC_PC_ALIGN	// pc alignment exception
@@ -579,6 +579,16 @@ el0_undef:
 	mov	x0, sp
 	bl	do_undefinstr
 	b	ret_to_user
+el0_sys:
+	/*
+	 * System instructions, for trapped cache maintenance instructions
+	 */
+	enable_dbg_and_irq
+	ct_user_exit
+	mov	x0, x25
+	mov	x1, sp
+	bl	do_sysinstr
+	b	ret_to_user
 el0_dbg:
 	/*
 	 * Debug exception handling
diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
index 03755a4..c2a158e 100644
--- a/arch/arm64/kernel/traps.c
+++ b/arch/arm64/kernel/traps.c
@@ -41,6 +41,7 @@
 #include <asm/stacktrace.h>
 #include <asm/exception.h>
 #include <asm/system_misc.h>
+#include <asm/sysreg.h>
 
 static const char *handler[]= {
 	"Synchronous Abort",
@@ -419,6 +420,76 @@ asmlinkage void __exception do_undefinstr(struct pt_regs *regs)
 	force_signal_inject(SIGILL, ILL_ILLOPC, regs, 0);
 }
 
+void cpu_enable_cache_maint_trap(void *__unused)
+{
+	config_sctlr_el1(SCTLR_EL1_UCI, 0);
+}
+
+#define __user_cache_maint(insn, address, res)			\
+	asm volatile (						\
+		"1:	" insn ", %1\n"				\
+		"	mov	%w0, #0\n"			\
+		"2:\n"						\
+		"	.pushsection .fixup,\"ax\"\n"		\
+		"	.align	2\n"				\
+		"3:	mov	%w0, %w2\n"			\
+		"	b	2b\n"				\
+		"	.popsection\n"				\
+		_ASM_EXTABLE(1b, 3b)				\
+		: "=r" (res)					\
+		: "r" (address), "i" (-EFAULT)			\
+		: "memory")
+
+asmlinkage void __exception do_sysinstr(unsigned int esr, struct pt_regs *regs)
+{
+	unsigned long address;
+	int ret;
+
+	/* if this is a write with: Op0=1, Op2=1, Op1=3, CRn=7 */
+	if ((esr & 0x01fffc01) == 0x0012dc00) {
+		int rt = (esr >> 5) & 0x1f;
+		int crm = (esr >> 1) & 0x0f;
+
+		address = regs->regs[rt];
+
+		switch (crm) {
+		case 11:		/* DC CVAU, gets promoted */
+			__user_cache_maint("dc civac", address, ret);
+			break;
+		case 10:		/* DC CVAC, gets promoted */
+			__user_cache_maint("dc civac", address, ret);
+			break;
+		case 14:		/* DC CIVAC */
+			__user_cache_maint("dc civac", address, ret);
+			break;
+		case 5:			/* IC IVAU */
+			__user_cache_maint("ic ivau", address, ret);
+			break;
+		default:
+			force_signal_inject(SIGILL, ILL_ILLOPC, regs, 0);
+			return;
+		}
+	} else {
+		force_signal_inject(SIGILL, ILL_ILLOPC, regs, 0);
+		return;
+	}
+
+	if (ret) {
+		int sig_code;
+
+		down_read(&current->mm->mmap_sem);
+		if (find_vma(current->mm, address) == NULL)
+			sig_code = SEGV_MAPERR;
+		else
+			sig_code = SEGV_ACCERR;
+		up_read(&current->mm->mmap_sem);
+
+		force_signal_inject(SIGSEGV, sig_code, regs, address);
+	} else {
+		regs->pc += 4;
+	}
+}
+
 long compat_arm_syscall(struct pt_regs *regs);
 
 asmlinkage long do_ni_syscall(struct pt_regs *regs)
-- 
2.7.3

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

* [PATCH 6/6] arm64: trap userspace "dc cvau" cache operation on errata-affected core
@ 2016-05-09 16:49   ` Andre Przywara
  0 siblings, 0 replies; 30+ messages in thread
From: Andre Przywara @ 2016-05-09 16:49 UTC (permalink / raw)
  To: linux-arm-kernel

The ARM errata 819472, 826319, 827319 and 824069 for affected
Cortex-A53 cores demand to promote "dc cvau" instructions to
"dc civac". Since we allow userspace to also emit those instructions,
we should make sure that "dc cvau" gets promoted there too.
So lets grasp the nettle here and actually trap every userland cache
maintenance instruction once we detect at least one affected core in
the system.
We then emulate the instruction by executing it on behalf of userland,
promoting "dc cvau" to "dc civac" on the way and injecting access
fault back into userspace.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 arch/arm64/include/asm/processor.h |  1 +
 arch/arm64/include/asm/sysreg.h    |  2 +-
 arch/arm64/kernel/cpu_errata.c     |  2 ++
 arch/arm64/kernel/entry.S          | 12 ++++++-
 arch/arm64/kernel/traps.c          | 71 ++++++++++++++++++++++++++++++++++++++
 5 files changed, 86 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/include/asm/processor.h b/arch/arm64/include/asm/processor.h
index cef1cf3..ace0a96 100644
--- a/arch/arm64/include/asm/processor.h
+++ b/arch/arm64/include/asm/processor.h
@@ -192,5 +192,6 @@ static inline void spin_lock_prefetch(const void *ptr)
 
 void cpu_enable_pan(void *__unused);
 void cpu_enable_uao(void *__unused);
+void cpu_enable_cache_maint_trap(void *__unused);
 
 #endif /* __ASM_PROCESSOR_H */
diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
index 1287416..7e0b097 100644
--- a/arch/arm64/include/asm/sysreg.h
+++ b/arch/arm64/include/asm/sysreg.h
@@ -90,7 +90,7 @@
 #define SCTLR_EL1_CP15BEN	(0x1 << 5)
 #define SCTLR_EL1_SED		(0x1 << 8)
 #define SCTLR_EL1_SPAN		(0x1 << 23)
-
+#define SCTLR_EL1_UCI		(0x1 << 26)
 
 /* id_aa64isar0 */
 #define ID_AA64ISAR0_RDM_SHIFT		28
diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
index 64cb71d..3f54d23 100644
--- a/arch/arm64/kernel/cpu_errata.c
+++ b/arch/arm64/kernel/cpu_errata.c
@@ -44,6 +44,7 @@ const struct arm64_cpu_capabilities arm64_errata[] = {
 		.desc = "ARM errata 826319, 827319, 824069",
 		.capability = ARM64_WORKAROUND_CLEAN_CACHE,
 		MIDR_RANGE(MIDR_CORTEX_A53, 0x00, 0x02),
+		.enable = cpu_enable_cache_maint_trap,
 	},
 #endif
 #ifdef CONFIG_ARM64_ERRATUM_819472
@@ -52,6 +53,7 @@ const struct arm64_cpu_capabilities arm64_errata[] = {
 		.desc = "ARM errata 819472",
 		.capability = ARM64_WORKAROUND_CLEAN_CACHE,
 		MIDR_RANGE(MIDR_CORTEX_A53, 0x00, 0x01),
+		.enable = cpu_enable_cache_maint_trap,
 	},
 #endif
 #ifdef CONFIG_ARM64_ERRATUM_832075
diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
index 12e8d2b..86480b4 100644
--- a/arch/arm64/kernel/entry.S
+++ b/arch/arm64/kernel/entry.S
@@ -451,7 +451,7 @@ el0_sync:
 	cmp	x24, #ESR_ELx_EC_FP_EXC64	// FP/ASIMD exception
 	b.eq	el0_fpsimd_exc
 	cmp	x24, #ESR_ELx_EC_SYS64		// configurable trap
-	b.eq	el0_undef
+	b.eq	el0_sys
 	cmp	x24, #ESR_ELx_EC_SP_ALIGN	// stack alignment exception
 	b.eq	el0_sp_pc
 	cmp	x24, #ESR_ELx_EC_PC_ALIGN	// pc alignment exception
@@ -579,6 +579,16 @@ el0_undef:
 	mov	x0, sp
 	bl	do_undefinstr
 	b	ret_to_user
+el0_sys:
+	/*
+	 * System instructions, for trapped cache maintenance instructions
+	 */
+	enable_dbg_and_irq
+	ct_user_exit
+	mov	x0, x25
+	mov	x1, sp
+	bl	do_sysinstr
+	b	ret_to_user
 el0_dbg:
 	/*
 	 * Debug exception handling
diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
index 03755a4..c2a158e 100644
--- a/arch/arm64/kernel/traps.c
+++ b/arch/arm64/kernel/traps.c
@@ -41,6 +41,7 @@
 #include <asm/stacktrace.h>
 #include <asm/exception.h>
 #include <asm/system_misc.h>
+#include <asm/sysreg.h>
 
 static const char *handler[]= {
 	"Synchronous Abort",
@@ -419,6 +420,76 @@ asmlinkage void __exception do_undefinstr(struct pt_regs *regs)
 	force_signal_inject(SIGILL, ILL_ILLOPC, regs, 0);
 }
 
+void cpu_enable_cache_maint_trap(void *__unused)
+{
+	config_sctlr_el1(SCTLR_EL1_UCI, 0);
+}
+
+#define __user_cache_maint(insn, address, res)			\
+	asm volatile (						\
+		"1:	" insn ", %1\n"				\
+		"	mov	%w0, #0\n"			\
+		"2:\n"						\
+		"	.pushsection .fixup,\"ax\"\n"		\
+		"	.align	2\n"				\
+		"3:	mov	%w0, %w2\n"			\
+		"	b	2b\n"				\
+		"	.popsection\n"				\
+		_ASM_EXTABLE(1b, 3b)				\
+		: "=r" (res)					\
+		: "r" (address), "i" (-EFAULT)			\
+		: "memory")
+
+asmlinkage void __exception do_sysinstr(unsigned int esr, struct pt_regs *regs)
+{
+	unsigned long address;
+	int ret;
+
+	/* if this is a write with: Op0=1, Op2=1, Op1=3, CRn=7 */
+	if ((esr & 0x01fffc01) == 0x0012dc00) {
+		int rt = (esr >> 5) & 0x1f;
+		int crm = (esr >> 1) & 0x0f;
+
+		address = regs->regs[rt];
+
+		switch (crm) {
+		case 11:		/* DC CVAU, gets promoted */
+			__user_cache_maint("dc civac", address, ret);
+			break;
+		case 10:		/* DC CVAC, gets promoted */
+			__user_cache_maint("dc civac", address, ret);
+			break;
+		case 14:		/* DC CIVAC */
+			__user_cache_maint("dc civac", address, ret);
+			break;
+		case 5:			/* IC IVAU */
+			__user_cache_maint("ic ivau", address, ret);
+			break;
+		default:
+			force_signal_inject(SIGILL, ILL_ILLOPC, regs, 0);
+			return;
+		}
+	} else {
+		force_signal_inject(SIGILL, ILL_ILLOPC, regs, 0);
+		return;
+	}
+
+	if (ret) {
+		int sig_code;
+
+		down_read(&current->mm->mmap_sem);
+		if (find_vma(current->mm, address) == NULL)
+			sig_code = SEGV_MAPERR;
+		else
+			sig_code = SEGV_ACCERR;
+		up_read(&current->mm->mmap_sem);
+
+		force_signal_inject(SIGSEGV, sig_code, regs, address);
+	} else {
+		regs->pc += 4;
+	}
+}
+
 long compat_arm_syscall(struct pt_regs *regs);
 
 asmlinkage long do_ni_syscall(struct pt_regs *regs)
-- 
2.7.3

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

* Re: [PATCH 4/6] arm64: errata: Calling enable functions for CPU errata too
  2016-05-09 16:49   ` Andre Przywara
@ 2016-06-10 15:31     ` Suzuki K Poulose
  -1 siblings, 0 replies; 30+ messages in thread
From: Suzuki K Poulose @ 2016-06-10 15:31 UTC (permalink / raw)
  To: Andre Przywara, Will Deacon, Catalin Marinas
  Cc: linux-arm-kernel, linux-kernel

On 09/05/16 17:49, Andre Przywara wrote:
> Currently we call the (optional) enable function for CPU _features_
> only. As CPU _errata_ descriptions share the same data structure and
> having an enable function is useful for errata as well (for instance
> to set bits in SCTLR), lets call it when enumerating erratas too.
>
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>

These changes look good to me.

Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>


Suzuki

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

* [PATCH 4/6] arm64: errata: Calling enable functions for CPU errata too
@ 2016-06-10 15:31     ` Suzuki K Poulose
  0 siblings, 0 replies; 30+ messages in thread
From: Suzuki K Poulose @ 2016-06-10 15:31 UTC (permalink / raw)
  To: linux-arm-kernel

On 09/05/16 17:49, Andre Przywara wrote:
> Currently we call the (optional) enable function for CPU _features_
> only. As CPU _errata_ descriptions share the same data structure and
> having an enable function is useful for errata as well (for instance
> to set bits in SCTLR), lets call it when enumerating erratas too.
>
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>

These changes look good to me.

Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>


Suzuki

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

* Re: [PATCH 6/6] arm64: trap userspace "dc cvau" cache operation on errata-affected core
  2016-05-09 16:49   ` Andre Przywara
@ 2016-06-14 16:16     ` Suzuki K Poulose
  -1 siblings, 0 replies; 30+ messages in thread
From: Suzuki K Poulose @ 2016-06-14 16:16 UTC (permalink / raw)
  To: Andre Przywara, Will Deacon, Catalin Marinas
  Cc: linux-arm-kernel, linux-kernel

On 09/05/16 17:49, Andre Przywara wrote:
> The ARM errata 819472, 826319, 827319 and 824069 for affected
> Cortex-A53 cores demand to promote "dc cvau" instructions to
> "dc civac". Since we allow userspace to also emit those instructions,
> we should make sure that "dc cvau" gets promoted there too.
> So lets grasp the nettle here and actually trap every userland cache
> maintenance instruction once we detect at least one affected core in
> the system.
> We then emulate the instruction by executing it on behalf of userland,
> promoting "dc cvau" to "dc civac" on the way and injecting access
> fault back into userspace.
>
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>


> +
> +asmlinkage void __exception do_sysinstr(unsigned int esr, struct pt_regs *regs)
> +{
> +	unsigned long address;
> +	int ret;
> +
> +	/* if this is a write with: Op0=1, Op2=1, Op1=3, CRn=7 */
> +	if ((esr & 0x01fffc01) == 0x0012dc00) {
> +		int rt = (esr >> 5) & 0x1f;
> +		int crm = (esr >> 1) & 0x0f;
> +
> +		address = regs->regs[rt];
> +
> +		switch (crm) {
> +		case 11:		/* DC CVAU, gets promoted */
> +			__user_cache_maint("dc civac", address, ret);
> +			break;
> +		case 10:		/* DC CVAC, gets promoted */
> +			__user_cache_maint("dc civac", address, ret);
> +			break;
> +		case 14:		/* DC CIVAC */
> +			__user_cache_maint("dc civac", address, ret);
> +			break;
> +		case 5:			/* IC IVAU */
> +			__user_cache_maint("ic ivau", address, ret);
> +			break;
> +		default:
> +			force_signal_inject(SIGILL, ILL_ILLOPC, regs, 0);
> +			return;
> +		}
> +	} else {
> +		force_signal_inject(SIGILL, ILL_ILLOPC, regs, 0);
> +		return;

Correct me if I am wrong, I think we should handle DC ZVA and emulate the same ?
Thats the only EL0 accessible instruction we don't handle above.

Suzuki

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

* [PATCH 6/6] arm64: trap userspace "dc cvau" cache operation on errata-affected core
@ 2016-06-14 16:16     ` Suzuki K Poulose
  0 siblings, 0 replies; 30+ messages in thread
From: Suzuki K Poulose @ 2016-06-14 16:16 UTC (permalink / raw)
  To: linux-arm-kernel

On 09/05/16 17:49, Andre Przywara wrote:
> The ARM errata 819472, 826319, 827319 and 824069 for affected
> Cortex-A53 cores demand to promote "dc cvau" instructions to
> "dc civac". Since we allow userspace to also emit those instructions,
> we should make sure that "dc cvau" gets promoted there too.
> So lets grasp the nettle here and actually trap every userland cache
> maintenance instruction once we detect at least one affected core in
> the system.
> We then emulate the instruction by executing it on behalf of userland,
> promoting "dc cvau" to "dc civac" on the way and injecting access
> fault back into userspace.
>
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>


> +
> +asmlinkage void __exception do_sysinstr(unsigned int esr, struct pt_regs *regs)
> +{
> +	unsigned long address;
> +	int ret;
> +
> +	/* if this is a write with: Op0=1, Op2=1, Op1=3, CRn=7 */
> +	if ((esr & 0x01fffc01) == 0x0012dc00) {
> +		int rt = (esr >> 5) & 0x1f;
> +		int crm = (esr >> 1) & 0x0f;
> +
> +		address = regs->regs[rt];
> +
> +		switch (crm) {
> +		case 11:		/* DC CVAU, gets promoted */
> +			__user_cache_maint("dc civac", address, ret);
> +			break;
> +		case 10:		/* DC CVAC, gets promoted */
> +			__user_cache_maint("dc civac", address, ret);
> +			break;
> +		case 14:		/* DC CIVAC */
> +			__user_cache_maint("dc civac", address, ret);
> +			break;
> +		case 5:			/* IC IVAU */
> +			__user_cache_maint("ic ivau", address, ret);
> +			break;
> +		default:
> +			force_signal_inject(SIGILL, ILL_ILLOPC, regs, 0);
> +			return;
> +		}
> +	} else {
> +		force_signal_inject(SIGILL, ILL_ILLOPC, regs, 0);
> +		return;

Correct me if I am wrong, I think we should handle DC ZVA and emulate the same ?
Thats the only EL0 accessible instruction we don't handle above.

Suzuki

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

* Re: [PATCH 6/6] arm64: trap userspace "dc cvau" cache operation on errata-affected core
  2016-06-14 16:16     ` Suzuki K Poulose
@ 2016-06-17 17:20       ` Andre Przywara
  -1 siblings, 0 replies; 30+ messages in thread
From: Andre Przywara @ 2016-06-17 17:20 UTC (permalink / raw)
  To: Suzuki K Poulose, Will Deacon, Catalin Marinas
  Cc: linux-arm-kernel, linux-kernel

Hi Suzuki,

thanks for having a look!

On 14/06/16 17:16, Suzuki K Poulose wrote:
> On 09/05/16 17:49, Andre Przywara wrote:
>> The ARM errata 819472, 826319, 827319 and 824069 for affected
>> Cortex-A53 cores demand to promote "dc cvau" instructions to
>> "dc civac". Since we allow userspace to also emit those instructions,
>> we should make sure that "dc cvau" gets promoted there too.
>> So lets grasp the nettle here and actually trap every userland cache
>> maintenance instruction once we detect at least one affected core in
>> the system.
>> We then emulate the instruction by executing it on behalf of userland,
>> promoting "dc cvau" to "dc civac" on the way and injecting access
>> fault back into userspace.
>>
>> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> 
> 
>> +
>> +asmlinkage void __exception do_sysinstr(unsigned int esr, struct
>> pt_regs *regs)
>> +{
>> +    unsigned long address;
>> +    int ret;
>> +
>> +    /* if this is a write with: Op0=1, Op2=1, Op1=3, CRn=7 */
>> +    if ((esr & 0x01fffc01) == 0x0012dc00) {
>> +        int rt = (esr >> 5) & 0x1f;
>> +        int crm = (esr >> 1) & 0x0f;
>> +
>> +        address = regs->regs[rt];
>> +
>> +        switch (crm) {
>> +        case 11:        /* DC CVAU, gets promoted */
>> +            __user_cache_maint("dc civac", address, ret);
>> +            break;
>> +        case 10:        /* DC CVAC, gets promoted */
>> +            __user_cache_maint("dc civac", address, ret);
>> +            break;
>> +        case 14:        /* DC CIVAC */
>> +            __user_cache_maint("dc civac", address, ret);
>> +            break;
>> +        case 5:            /* IC IVAU */
>> +            __user_cache_maint("ic ivau", address, ret);
>> +            break;
>> +        default:
>> +            force_signal_inject(SIGILL, ILL_ILLOPC, regs, 0);
>> +            return;
>> +        }
>> +    } else {
>> +        force_signal_inject(SIGILL, ILL_ILLOPC, regs, 0);
>> +        return;
> 
> Correct me if I am wrong, I think we should handle DC ZVA and emulate
> the same ?
> Thats the only EL0 accessible instruction we don't handle above.

Mmmh, but why should we care?
1) DC ZVA is not trapped by setting SCTLR.UCI - instead it has its own
bit (SCTLR.DZE).
2) The SDEN document does not speak about DC ZVA, so it's not affected
by that mentioned errata.
3) A fault caused by this instruction will not trigger this SIGILL fault
path, AFAICT. We get a synchronous data abort on a NULL pointer
dereference, for instance, so it's a SIGSEGV.

I tested it with issuing valid and invalid DC ZVA instructions and it
worked fine on both an affected and unaffected system.
I saw SIGSEGVs due to PC=0 with *some* unaligned addresses, though, but
that behaviour was reproducible on a non-affected core without the
patches as well, so I don't think it's related (need to investigate).

Yes, a DC ZVA shares the encoding masking above (Op0=1, Op2=1, Op1=3,
CRn=7), but unless the kernel actually sets SCTLR.DZE, we should be
safe. So is it that potential case that you are after or do I miss
something else here?

Cheers,
Andre.

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

* [PATCH 6/6] arm64: trap userspace "dc cvau" cache operation on errata-affected core
@ 2016-06-17 17:20       ` Andre Przywara
  0 siblings, 0 replies; 30+ messages in thread
From: Andre Przywara @ 2016-06-17 17:20 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Suzuki,

thanks for having a look!

On 14/06/16 17:16, Suzuki K Poulose wrote:
> On 09/05/16 17:49, Andre Przywara wrote:
>> The ARM errata 819472, 826319, 827319 and 824069 for affected
>> Cortex-A53 cores demand to promote "dc cvau" instructions to
>> "dc civac". Since we allow userspace to also emit those instructions,
>> we should make sure that "dc cvau" gets promoted there too.
>> So lets grasp the nettle here and actually trap every userland cache
>> maintenance instruction once we detect at least one affected core in
>> the system.
>> We then emulate the instruction by executing it on behalf of userland,
>> promoting "dc cvau" to "dc civac" on the way and injecting access
>> fault back into userspace.
>>
>> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> 
> 
>> +
>> +asmlinkage void __exception do_sysinstr(unsigned int esr, struct
>> pt_regs *regs)
>> +{
>> +    unsigned long address;
>> +    int ret;
>> +
>> +    /* if this is a write with: Op0=1, Op2=1, Op1=3, CRn=7 */
>> +    if ((esr & 0x01fffc01) == 0x0012dc00) {
>> +        int rt = (esr >> 5) & 0x1f;
>> +        int crm = (esr >> 1) & 0x0f;
>> +
>> +        address = regs->regs[rt];
>> +
>> +        switch (crm) {
>> +        case 11:        /* DC CVAU, gets promoted */
>> +            __user_cache_maint("dc civac", address, ret);
>> +            break;
>> +        case 10:        /* DC CVAC, gets promoted */
>> +            __user_cache_maint("dc civac", address, ret);
>> +            break;
>> +        case 14:        /* DC CIVAC */
>> +            __user_cache_maint("dc civac", address, ret);
>> +            break;
>> +        case 5:            /* IC IVAU */
>> +            __user_cache_maint("ic ivau", address, ret);
>> +            break;
>> +        default:
>> +            force_signal_inject(SIGILL, ILL_ILLOPC, regs, 0);
>> +            return;
>> +        }
>> +    } else {
>> +        force_signal_inject(SIGILL, ILL_ILLOPC, regs, 0);
>> +        return;
> 
> Correct me if I am wrong, I think we should handle DC ZVA and emulate
> the same ?
> Thats the only EL0 accessible instruction we don't handle above.

Mmmh, but why should we care?
1) DC ZVA is not trapped by setting SCTLR.UCI - instead it has its own
bit (SCTLR.DZE).
2) The SDEN document does not speak about DC ZVA, so it's not affected
by that mentioned errata.
3) A fault caused by this instruction will not trigger this SIGILL fault
path, AFAICT. We get a synchronous data abort on a NULL pointer
dereference, for instance, so it's a SIGSEGV.

I tested it with issuing valid and invalid DC ZVA instructions and it
worked fine on both an affected and unaffected system.
I saw SIGSEGVs due to PC=0 with *some* unaligned addresses, though, but
that behaviour was reproducible on a non-affected core without the
patches as well, so I don't think it's related (need to investigate).

Yes, a DC ZVA shares the encoding masking above (Op0=1, Op2=1, Op1=3,
CRn=7), but unless the kernel actually sets SCTLR.DZE, we should be
safe. So is it that potential case that you are after or do I miss
something else here?

Cheers,
Andre.

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

* Re: [PATCH 6/6] arm64: trap userspace "dc cvau" cache operation on errata-affected core
  2016-06-17 17:20       ` Andre Przywara
@ 2016-06-17 17:25         ` Suzuki K Poulose
  -1 siblings, 0 replies; 30+ messages in thread
From: Suzuki K Poulose @ 2016-06-17 17:25 UTC (permalink / raw)
  To: Andre Przywara, Will Deacon, Catalin Marinas
  Cc: linux-arm-kernel, linux-kernel

On 17/06/16 18:20, Andre Przywara wrote:
> Hi Suzuki,
>
> thanks for having a look!
>
> On 14/06/16 17:16, Suzuki K Poulose wrote:
>> On 09/05/16 17:49, Andre Przywara wrote:
>>> The ARM errata 819472, 826319, 827319 and 824069 for affected
>>> Cortex-A53 cores demand to promote "dc cvau" instructions to
>>> "dc civac". Since we allow userspace to also emit those instructions,
>>> we should make sure that "dc cvau" gets promoted there too.
>>> So lets grasp the nettle here and actually trap every userland cache
>>> maintenance instruction once we detect at least one affected core in
>>> the system.
            __user_cache_maint("dc civac", address, ret);
>>> +            break;
>>> +        case 10:        /* DC CVAC, gets promoted */
>>> +            __user_cache_maint("dc civac", address, ret);
>>> +            break;
>>> +        case 14:        /* DC CIVAC */
>>> +            __user_cache_maint("dc civac", address, ret);
>>> +            break;
>>> +        case 5:            /* IC IVAU */
>>> +            __user_cache_maint("ic ivau", address, ret);
>>> +            break;
>>> +        default:
>>> +            force_signal_inject(SIGILL, ILL_ILLOPC, regs, 0);
>>> +            return;
>>> +        }
>>> +    } else {
>>> +        force_signal_inject(SIGILL, ILL_ILLOPC, regs, 0);
>>> +        return;
>>
>> Correct me if I am wrong, I think we should handle DC ZVA and emulate
>> the same ?
>> Thats the only EL0 accessible instruction we don't handle above.
>
> Mmmh, but why should we care?
> 1) DC ZVA is not trapped by setting SCTLR.UCI - instead it has its own
> bit (SCTLR.DZE).

You are right. I was thinking that UCI traps all DC operations. It only
traps  DC CVAU, DC CIVAC, DC CVAC, and IC IVAU.






Cheers
Suzuki

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

* [PATCH 6/6] arm64: trap userspace "dc cvau" cache operation on errata-affected core
@ 2016-06-17 17:25         ` Suzuki K Poulose
  0 siblings, 0 replies; 30+ messages in thread
From: Suzuki K Poulose @ 2016-06-17 17:25 UTC (permalink / raw)
  To: linux-arm-kernel

On 17/06/16 18:20, Andre Przywara wrote:
> Hi Suzuki,
>
> thanks for having a look!
>
> On 14/06/16 17:16, Suzuki K Poulose wrote:
>> On 09/05/16 17:49, Andre Przywara wrote:
>>> The ARM errata 819472, 826319, 827319 and 824069 for affected
>>> Cortex-A53 cores demand to promote "dc cvau" instructions to
>>> "dc civac". Since we allow userspace to also emit those instructions,
>>> we should make sure that "dc cvau" gets promoted there too.
>>> So lets grasp the nettle here and actually trap every userland cache
>>> maintenance instruction once we detect at least one affected core in
>>> the system.
            __user_cache_maint("dc civac", address, ret);
>>> +            break;
>>> +        case 10:        /* DC CVAC, gets promoted */
>>> +            __user_cache_maint("dc civac", address, ret);
>>> +            break;
>>> +        case 14:        /* DC CIVAC */
>>> +            __user_cache_maint("dc civac", address, ret);
>>> +            break;
>>> +        case 5:            /* IC IVAU */
>>> +            __user_cache_maint("ic ivau", address, ret);
>>> +            break;
>>> +        default:
>>> +            force_signal_inject(SIGILL, ILL_ILLOPC, regs, 0);
>>> +            return;
>>> +        }
>>> +    } else {
>>> +        force_signal_inject(SIGILL, ILL_ILLOPC, regs, 0);
>>> +        return;
>>
>> Correct me if I am wrong, I think we should handle DC ZVA and emulate
>> the same ?
>> Thats the only EL0 accessible instruction we don't handle above.
>
> Mmmh, but why should we care?
> 1) DC ZVA is not trapped by setting SCTLR.UCI - instead it has its own
> bit (SCTLR.DZE).

You are right. I was thinking that UCI traps all DC operations. It only
traps  DC CVAU, DC CIVAC, DC CVAC, and IC IVAU.






Cheers
Suzuki

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

* Re: [PATCH 1/6] arm64: alternatives: drop enable parameter from _else and _endif macro
  2016-05-09 16:49   ` Andre Przywara
@ 2016-06-23 17:17     ` Catalin Marinas
  -1 siblings, 0 replies; 30+ messages in thread
From: Catalin Marinas @ 2016-06-23 17:17 UTC (permalink / raw)
  To: Andre Przywara; +Cc: Will Deacon, linux-kernel, linux-arm-kernel

On Mon, May 09, 2016 at 05:49:45PM +0100, Andre Przywara wrote:
> Commit 77ee306c0aea9 ("arm64: alternatives: add enable parameter to
> conditional asm macros") extended the alternative assembly macros.
> Unfortunately this does not really work as one would expect, as the
> enable parameter in fact correctly protects the alternative section
> magic, but not the actual code sequences.

I don't remember how we ended up with this code because it's not used
anywhere.

> So if enable is false, we will have the original instruction(s) _and_
> the alternative ones in the file, which is just wrong.
> To make this work one would need to additionally protect the
> alternative sequence with extra .if directives, which makes
> the intention of the enable parameter rather pointless.
> Instead users should directly guard the whole "_else; insn; _endif"
> sequence with .if directives.
> Add a comment describing this usage and drop the enable parameter from
> the alternative_else and alternative_endif macros.
> 
> This reverts parts of commit 77ee306c0aea ("arm64: alternatives: add
> enable parameter to conditional asm macros").
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
>  arch/arm64/include/asm/alternative.h | 24 ++++++++++++++++++------
>  1 file changed, 18 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/alternative.h b/arch/arm64/include/asm/alternative.h
> index beccbde..502c9ef 100644
> --- a/arch/arm64/include/asm/alternative.h
> +++ b/arch/arm64/include/asm/alternative.h
> @@ -94,6 +94,8 @@ void apply_alternatives(void *start, size_t length);
>   *
>   * The code that follows this macro will be assembled and linked as
>   * normal. There are no restrictions on this code.
> + * If you use the enable parameter, see the comments below for _else
> + * and _endif.
>   */
>  .macro alternative_if_not cap, enable = 1
>  	.if \enable
> @@ -117,23 +119,33 @@ void apply_alternatives(void *start, size_t length);
>   * 2. Not contain a branch target that is used outside of the
>   *    alternative sequence it is defined in (branches into an
>   *    alternative sequence are not fixed up).
> + *
> + * If you used the optional enable parameter in the opening
> + * alternative_if_not macro above, please protect the whole _else
> + * branch with an .if directive:
> + *	alternative_if_not CAP_SOMETHING, condition
> + *		orig_insn
> + *	.if condition
> + *	alternative_else
> + *		repl_insn
> + *	alternative_endif
> + *	.endif

I think the intention was to that in the !condition case, both
alternatives are dropped. That includes the original code.

I propose we revert the original commit, I don't think it is strictly
needed for your patches (I'll try to fix them up and see how it goes).

-- 
Catalin

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

* [PATCH 1/6] arm64: alternatives: drop enable parameter from _else and _endif macro
@ 2016-06-23 17:17     ` Catalin Marinas
  0 siblings, 0 replies; 30+ messages in thread
From: Catalin Marinas @ 2016-06-23 17:17 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, May 09, 2016 at 05:49:45PM +0100, Andre Przywara wrote:
> Commit 77ee306c0aea9 ("arm64: alternatives: add enable parameter to
> conditional asm macros") extended the alternative assembly macros.
> Unfortunately this does not really work as one would expect, as the
> enable parameter in fact correctly protects the alternative section
> magic, but not the actual code sequences.

I don't remember how we ended up with this code because it's not used
anywhere.

> So if enable is false, we will have the original instruction(s) _and_
> the alternative ones in the file, which is just wrong.
> To make this work one would need to additionally protect the
> alternative sequence with extra .if directives, which makes
> the intention of the enable parameter rather pointless.
> Instead users should directly guard the whole "_else; insn; _endif"
> sequence with .if directives.
> Add a comment describing this usage and drop the enable parameter from
> the alternative_else and alternative_endif macros.
> 
> This reverts parts of commit 77ee306c0aea ("arm64: alternatives: add
> enable parameter to conditional asm macros").
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
>  arch/arm64/include/asm/alternative.h | 24 ++++++++++++++++++------
>  1 file changed, 18 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/alternative.h b/arch/arm64/include/asm/alternative.h
> index beccbde..502c9ef 100644
> --- a/arch/arm64/include/asm/alternative.h
> +++ b/arch/arm64/include/asm/alternative.h
> @@ -94,6 +94,8 @@ void apply_alternatives(void *start, size_t length);
>   *
>   * The code that follows this macro will be assembled and linked as
>   * normal. There are no restrictions on this code.
> + * If you use the enable parameter, see the comments below for _else
> + * and _endif.
>   */
>  .macro alternative_if_not cap, enable = 1
>  	.if \enable
> @@ -117,23 +119,33 @@ void apply_alternatives(void *start, size_t length);
>   * 2. Not contain a branch target that is used outside of the
>   *    alternative sequence it is defined in (branches into an
>   *    alternative sequence are not fixed up).
> + *
> + * If you used the optional enable parameter in the opening
> + * alternative_if_not macro above, please protect the whole _else
> + * branch with an .if directive:
> + *	alternative_if_not CAP_SOMETHING, condition
> + *		orig_insn
> + *	.if condition
> + *	alternative_else
> + *		repl_insn
> + *	alternative_endif
> + *	.endif

I think the intention was to that in the !condition case, both
alternatives are dropped. That includes the original code.

I propose we revert the original commit, I don't think it is strictly
needed for your patches (I'll try to fix them up and see how it goes).

-- 
Catalin

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

* Re: [PATCH 3/6] arm64: include alternative handling in dcache_by_line_op
  2016-05-09 16:49   ` Andre Przywara
@ 2016-06-24 15:32     ` Catalin Marinas
  -1 siblings, 0 replies; 30+ messages in thread
From: Catalin Marinas @ 2016-06-24 15:32 UTC (permalink / raw)
  To: Andre Przywara; +Cc: Will Deacon, linux-kernel, linux-arm-kernel

On Mon, May 09, 2016 at 05:49:47PM +0100, Andre Przywara wrote:
> The newly introduced dcache_by_line_op macro is used at least in
> one occassion at the moment to issue a "dc cvau" instruction,
> which is affected by ARM errata 819472, 826319, 827319 and 824069.
> Change the macro to allow for alternative patching in there to
> protect affected Cortex-A53 cores.
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
>  arch/arm64/mm/proc-macros.S | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/mm/proc-macros.S b/arch/arm64/mm/proc-macros.S
> index e6a30e1..5786017 100644
> --- a/arch/arm64/mm/proc-macros.S
> +++ b/arch/arm64/mm/proc-macros.S
> @@ -78,7 +78,14 @@
>  	add	\size, \kaddr, \size
>  	sub	\tmp2, \tmp1, #1
>  	bic	\kaddr, \kaddr, \tmp2
> -9998:	dc	\op, \kaddr
> +9998:
> +	alternative_if_not ARM64_WORKAROUND_CLEAN_CACHE, (\op == cvau || \op == cvac)
> +	dc	\op, \kaddr
> +	.if	(\op == cvau || \op == cvac)
> +	alternative_else
> +	dc	civac, \kaddr
> +	alternative_endif
> +	.endif

We can revert commit 77ee306c0aea ("arm64: alternatives: add enable
parameter to conditional asm macros"), drop the first patch in this
series and move the .if outside the alternative block (with a
duplication of the "dc \op, \kaddr" line.

-- 
Catalin

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

* [PATCH 3/6] arm64: include alternative handling in dcache_by_line_op
@ 2016-06-24 15:32     ` Catalin Marinas
  0 siblings, 0 replies; 30+ messages in thread
From: Catalin Marinas @ 2016-06-24 15:32 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, May 09, 2016 at 05:49:47PM +0100, Andre Przywara wrote:
> The newly introduced dcache_by_line_op macro is used at least in
> one occassion at the moment to issue a "dc cvau" instruction,
> which is affected by ARM errata 819472, 826319, 827319 and 824069.
> Change the macro to allow for alternative patching in there to
> protect affected Cortex-A53 cores.
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
>  arch/arm64/mm/proc-macros.S | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/mm/proc-macros.S b/arch/arm64/mm/proc-macros.S
> index e6a30e1..5786017 100644
> --- a/arch/arm64/mm/proc-macros.S
> +++ b/arch/arm64/mm/proc-macros.S
> @@ -78,7 +78,14 @@
>  	add	\size, \kaddr, \size
>  	sub	\tmp2, \tmp1, #1
>  	bic	\kaddr, \kaddr, \tmp2
> -9998:	dc	\op, \kaddr
> +9998:
> +	alternative_if_not ARM64_WORKAROUND_CLEAN_CACHE, (\op == cvau || \op == cvac)
> +	dc	\op, \kaddr
> +	.if	(\op == cvau || \op == cvac)
> +	alternative_else
> +	dc	civac, \kaddr
> +	alternative_endif
> +	.endif

We can revert commit 77ee306c0aea ("arm64: alternatives: add enable
parameter to conditional asm macros"), drop the first patch in this
series and move the .if outside the alternative block (with a
duplication of the "dc \op, \kaddr" line.

-- 
Catalin

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

* Re: [PATCH 4/6] arm64: errata: Calling enable functions for CPU errata too
  2016-05-09 16:49   ` Andre Przywara
@ 2016-06-24 15:34     ` Catalin Marinas
  -1 siblings, 0 replies; 30+ messages in thread
From: Catalin Marinas @ 2016-06-24 15:34 UTC (permalink / raw)
  To: Andre Przywara; +Cc: Will Deacon, linux-kernel, linux-arm-kernel

On Mon, May 09, 2016 at 05:49:48PM +0100, Andre Przywara wrote:
> Currently we call the (optional) enable function for CPU _features_
> only. As CPU _errata_ descriptions share the same data structure and
> having an enable function is useful for errata as well (for instance
> to set bits in SCTLR), lets call it when enumerating erratas too.
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
>  arch/arm64/include/asm/cpufeature.h | 2 ++
>  arch/arm64/kernel/cpu_errata.c      | 5 +++++
>  arch/arm64/kernel/cpufeature.c      | 4 ++--
>  3 files changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
> index b9b6494..cb8fb2c 100644
> --- a/arch/arm64/include/asm/cpufeature.h
> +++ b/arch/arm64/include/asm/cpufeature.h
> @@ -174,7 +174,9 @@ void __init setup_cpu_features(void);
>  
>  void update_cpu_capabilities(const struct arm64_cpu_capabilities *caps,
>  			    const char *info);
> +void enable_cpu_capabilities(const struct arm64_cpu_capabilities *caps);
>  void check_local_cpu_errata(void);
> +void __init enable_cpu_errata(void);

Nitpick: I would rather call this enable_errata_workarounds since errata
are already enabled by in hardware ;).

-- 
Catalin

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

* [PATCH 4/6] arm64: errata: Calling enable functions for CPU errata too
@ 2016-06-24 15:34     ` Catalin Marinas
  0 siblings, 0 replies; 30+ messages in thread
From: Catalin Marinas @ 2016-06-24 15:34 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, May 09, 2016 at 05:49:48PM +0100, Andre Przywara wrote:
> Currently we call the (optional) enable function for CPU _features_
> only. As CPU _errata_ descriptions share the same data structure and
> having an enable function is useful for errata as well (for instance
> to set bits in SCTLR), lets call it when enumerating erratas too.
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
>  arch/arm64/include/asm/cpufeature.h | 2 ++
>  arch/arm64/kernel/cpu_errata.c      | 5 +++++
>  arch/arm64/kernel/cpufeature.c      | 4 ++--
>  3 files changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
> index b9b6494..cb8fb2c 100644
> --- a/arch/arm64/include/asm/cpufeature.h
> +++ b/arch/arm64/include/asm/cpufeature.h
> @@ -174,7 +174,9 @@ void __init setup_cpu_features(void);
>  
>  void update_cpu_capabilities(const struct arm64_cpu_capabilities *caps,
>  			    const char *info);
> +void enable_cpu_capabilities(const struct arm64_cpu_capabilities *caps);
>  void check_local_cpu_errata(void);
> +void __init enable_cpu_errata(void);

Nitpick: I would rather call this enable_errata_workarounds since errata
are already enabled by in hardware ;).

-- 
Catalin

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

* Re: [PATCH 6/6] arm64: trap userspace "dc cvau" cache operation on errata-affected core
  2016-05-09 16:49   ` Andre Przywara
@ 2016-06-24 16:25     ` Catalin Marinas
  -1 siblings, 0 replies; 30+ messages in thread
From: Catalin Marinas @ 2016-06-24 16:25 UTC (permalink / raw)
  To: Andre Przywara; +Cc: Will Deacon, linux-kernel, linux-arm-kernel

On Mon, May 09, 2016 at 05:49:50PM +0100, Andre Przywara wrote:
> +#define __user_cache_maint(insn, address, res)			\
> +	asm volatile (						\
> +		"1:	" insn ", %1\n"				\
> +		"	mov	%w0, #0\n"			\
> +		"2:\n"						\
> +		"	.pushsection .fixup,\"ax\"\n"		\
> +		"	.align	2\n"				\
> +		"3:	mov	%w0, %w2\n"			\
> +		"	b	2b\n"				\
> +		"	.popsection\n"				\
> +		_ASM_EXTABLE(1b, 3b)				\
> +		: "=r" (res)					\
> +		: "r" (address), "i" (-EFAULT)			\
> +		: "memory")

I don't think we need the "memory" clobber here. It's not really
accessing memory that the compiler controls.

> +asmlinkage void __exception do_sysinstr(unsigned int esr, struct pt_regs *regs)
> +{
> +	unsigned long address;
> +	int ret;
> +
> +	/* if this is a write with: Op0=1, Op2=1, Op1=3, CRn=7 */
> +	if ((esr & 0x01fffc01) == 0x0012dc00) {
> +		int rt = (esr >> 5) & 0x1f;
> +		int crm = (esr >> 1) & 0x0f;
> +
> +		address = regs->regs[rt];
> +
> +		switch (crm) {
> +		case 11:		/* DC CVAU, gets promoted */
> +			__user_cache_maint("dc civac", address, ret);
> +			break;
> +		case 10:		/* DC CVAC, gets promoted */
> +			__user_cache_maint("dc civac", address, ret);
> +			break;
> +		case 14:		/* DC CIVAC */
> +			__user_cache_maint("dc civac", address, ret);
> +			break;
> +		case 5:			/* IC IVAU */
> +			__user_cache_maint("ic ivau", address, ret);
> +			break;
> +		default:
> +			force_signal_inject(SIGILL, ILL_ILLOPC, regs, 0);
> +			return;
> +		}
> +	} else {
> +		force_signal_inject(SIGILL, ILL_ILLOPC, regs, 0);
> +		return;
> +	}
> +
> +	if (ret) {
> +		int sig_code;
> +
> +		down_read(&current->mm->mmap_sem);
> +		if (find_vma(current->mm, address) == NULL)
> +			sig_code = SEGV_MAPERR;
> +		else
> +			sig_code = SEGV_ACCERR;
> +		up_read(&current->mm->mmap_sem);
> +
> +		force_signal_inject(SIGSEGV, sig_code, regs, address);

BTW, there is some duplication with set_segfault() in
armv8_deprecated.c, could you make this a common function in trap.c?

-- 
Catalin

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

* [PATCH 6/6] arm64: trap userspace "dc cvau" cache operation on errata-affected core
@ 2016-06-24 16:25     ` Catalin Marinas
  0 siblings, 0 replies; 30+ messages in thread
From: Catalin Marinas @ 2016-06-24 16:25 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, May 09, 2016 at 05:49:50PM +0100, Andre Przywara wrote:
> +#define __user_cache_maint(insn, address, res)			\
> +	asm volatile (						\
> +		"1:	" insn ", %1\n"				\
> +		"	mov	%w0, #0\n"			\
> +		"2:\n"						\
> +		"	.pushsection .fixup,\"ax\"\n"		\
> +		"	.align	2\n"				\
> +		"3:	mov	%w0, %w2\n"			\
> +		"	b	2b\n"				\
> +		"	.popsection\n"				\
> +		_ASM_EXTABLE(1b, 3b)				\
> +		: "=r" (res)					\
> +		: "r" (address), "i" (-EFAULT)			\
> +		: "memory")

I don't think we need the "memory" clobber here. It's not really
accessing memory that the compiler controls.

> +asmlinkage void __exception do_sysinstr(unsigned int esr, struct pt_regs *regs)
> +{
> +	unsigned long address;
> +	int ret;
> +
> +	/* if this is a write with: Op0=1, Op2=1, Op1=3, CRn=7 */
> +	if ((esr & 0x01fffc01) == 0x0012dc00) {
> +		int rt = (esr >> 5) & 0x1f;
> +		int crm = (esr >> 1) & 0x0f;
> +
> +		address = regs->regs[rt];
> +
> +		switch (crm) {
> +		case 11:		/* DC CVAU, gets promoted */
> +			__user_cache_maint("dc civac", address, ret);
> +			break;
> +		case 10:		/* DC CVAC, gets promoted */
> +			__user_cache_maint("dc civac", address, ret);
> +			break;
> +		case 14:		/* DC CIVAC */
> +			__user_cache_maint("dc civac", address, ret);
> +			break;
> +		case 5:			/* IC IVAU */
> +			__user_cache_maint("ic ivau", address, ret);
> +			break;
> +		default:
> +			force_signal_inject(SIGILL, ILL_ILLOPC, regs, 0);
> +			return;
> +		}
> +	} else {
> +		force_signal_inject(SIGILL, ILL_ILLOPC, regs, 0);
> +		return;
> +	}
> +
> +	if (ret) {
> +		int sig_code;
> +
> +		down_read(&current->mm->mmap_sem);
> +		if (find_vma(current->mm, address) == NULL)
> +			sig_code = SEGV_MAPERR;
> +		else
> +			sig_code = SEGV_ACCERR;
> +		up_read(&current->mm->mmap_sem);
> +
> +		force_signal_inject(SIGSEGV, sig_code, regs, address);

BTW, there is some duplication with set_segfault() in
armv8_deprecated.c, could you make this a common function in trap.c?

-- 
Catalin

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

end of thread, other threads:[~2016-06-24 16:25 UTC | newest]

Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-05-09 16:49 [PATCH 0/6] arm64: Extend Cortex-A53 errata workaround Andre Przywara
2016-05-09 16:49 ` Andre Przywara
2016-05-09 16:49 ` [PATCH 1/6] arm64: alternatives: drop enable parameter from _else and _endif macro Andre Przywara
2016-05-09 16:49   ` Andre Przywara
2016-06-23 17:17   ` Catalin Marinas
2016-06-23 17:17     ` Catalin Marinas
2016-05-09 16:49 ` [PATCH 2/6] arm64: fix "dc cvau" cache operation on errata-affected core Andre Przywara
2016-05-09 16:49   ` Andre Przywara
2016-05-09 16:49 ` [PATCH 3/6] arm64: include alternative handling in dcache_by_line_op Andre Przywara
2016-05-09 16:49   ` Andre Przywara
2016-06-24 15:32   ` Catalin Marinas
2016-06-24 15:32     ` Catalin Marinas
2016-05-09 16:49 ` [PATCH 4/6] arm64: errata: Calling enable functions for CPU errata too Andre Przywara
2016-05-09 16:49   ` Andre Przywara
2016-06-10 15:31   ` Suzuki K Poulose
2016-06-10 15:31     ` Suzuki K Poulose
2016-06-24 15:34   ` Catalin Marinas
2016-06-24 15:34     ` Catalin Marinas
2016-05-09 16:49 ` [PATCH 5/6] arm64: consolidate signal injection on emulation errors Andre Przywara
2016-05-09 16:49   ` Andre Przywara
2016-05-09 16:49 ` [PATCH 6/6] arm64: trap userspace "dc cvau" cache operation on errata-affected core Andre Przywara
2016-05-09 16:49   ` Andre Przywara
2016-06-14 16:16   ` Suzuki K Poulose
2016-06-14 16:16     ` Suzuki K Poulose
2016-06-17 17:20     ` Andre Przywara
2016-06-17 17:20       ` Andre Przywara
2016-06-17 17:25       ` Suzuki K Poulose
2016-06-17 17:25         ` Suzuki K Poulose
2016-06-24 16:25   ` Catalin Marinas
2016-06-24 16:25     ` Catalin Marinas

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.