LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH 0/5] arm64: Survival kit for SCR_EL3.HCE==0 conditions
@ 2021-08-12 19:02 Marc Zyngier
  2021-08-12 19:02 ` [PATCH 1/5] arm64: Directly expand __init_el2_nvhe_prepare_eret where needed Marc Zyngier
                   ` (6 more replies)
  0 siblings, 7 replies; 20+ messages in thread
From: Marc Zyngier @ 2021-08-12 19:02 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel
  Cc: Rafał Miłecki, Will Deacon, Catalin Marinas,
	Mark Rutland, Ard Biesheuvel, Florian Fainelli,
	bcm-kernel-feedback-list, kernel-team

Anyone vaguely familiar with the ARMv8 architecture would quickly
understand that entering the kernel at EL2 without enabling the HVC
instruction is... living dangerously. But as it turns out [0], there
is a whole range of (*cough*) "high quality" (*cough*) Broadcom
systems out there configured exactly like that.

If you are speechless, I'm right with you.

These machines have stopped being able to boot an upstream kernel
since 5.12, where we changed the way we switch from nVHE to VHE, as
this relies on the HVC instruction being usable... It is also worth
noting that these systems have never been able to use KVM. Or kexec.

This small series addresses the issue by detecting an UNDEFing HVC in
a fairly controlled environment, and in this case pretend that we have
booted at EL1. It also documents the requirement for SCR_EL3.HCE to be
set to *1* if the kernel is entered at EL2. Turns out that we really
have to state the obvious.

This has been tested on a FVP model with a hacked-up boot-wrapper.

Note that I really don't think any of this is -stable material, except
maybe for the documentation. It isn't 5.14 material either. Best case,
this is 5.15, or maybe even later. If ever.

	M. (drink required)

[0] https://lore.kernel.org/r/53f3a2d2-22f8-edee-2507-d41a4090dad7@gmail.com

Marc Zyngier (5):
  arm64: Directly expand __init_el2_nvhe_prepare_eret where needed
  arm64: Handle UNDEF in the EL2 stub vectors
  arm64: Detect disabled HVC early
  arm64: Warn on booting at EL2 with HVC disabled
  arm64: Document the requirement for SCR_EL3.HCE

 Documentation/arm64/booting.rst    |  5 +++++
 arch/arm64/include/asm/el2_setup.h |  6 ------
 arch/arm64/include/asm/virt.h      | 10 +++++++++
 arch/arm64/kernel/head.S           | 34 ++++++++++++++++++++++++++++++
 arch/arm64/kernel/hyp-stub.S       | 19 ++++++++++++++++-
 arch/arm64/kernel/smp.c            |  3 +++
 6 files changed, 70 insertions(+), 7 deletions(-)

-- 
2.30.2


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

* [PATCH 1/5] arm64: Directly expand __init_el2_nvhe_prepare_eret where needed
  2021-08-12 19:02 [PATCH 0/5] arm64: Survival kit for SCR_EL3.HCE==0 conditions Marc Zyngier
@ 2021-08-12 19:02 ` Marc Zyngier
  2021-08-12 19:02 ` [PATCH 2/5] arm64: Handle UNDEF in the EL2 stub vectors Marc Zyngier
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 20+ messages in thread
From: Marc Zyngier @ 2021-08-12 19:02 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel
  Cc: Rafał Miłecki, Will Deacon, Catalin Marinas,
	Mark Rutland, Ard Biesheuvel, Florian Fainelli,
	bcm-kernel-feedback-list, kernel-team

As we are about to enable taking exceptions extremely early,
it becomes pointless to pre-populate SPSR_EL2 in init_el2_state.

We already have the stuck-in-VHE handling code that needs some
special casing, so let's bite the bullet and get rid of the macros
by expanding them where needed.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 arch/arm64/include/asm/el2_setup.h | 6 ------
 arch/arm64/kernel/head.S           | 2 ++
 2 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/arch/arm64/include/asm/el2_setup.h b/arch/arm64/include/asm/el2_setup.h
index 21fa330f498d..0b5c8033eaf2 100644
--- a/arch/arm64/include/asm/el2_setup.h
+++ b/arch/arm64/include/asm/el2_setup.h
@@ -164,11 +164,6 @@
 .Lskip_fgt_\@:
 .endm
 
-.macro __init_el2_nvhe_prepare_eret
-	mov	x0, #INIT_PSTATE_EL1
-	msr	spsr_el2, x0
-.endm
-
 /**
  * Initialize EL2 registers to sane values. This should be called early on all
  * cores that were booted in EL2. Note that everything gets initialised as
@@ -189,7 +184,6 @@
 	__init_el2_nvhe_cptr
 	__init_el2_nvhe_sve
 	__init_el2_fgt
-	__init_el2_nvhe_prepare_eret
 .endm
 
 #endif /* __ARM_KVM_INIT_H__ */
diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
index c5c994a73a64..9d5aa56a98cc 100644
--- a/arch/arm64/kernel/head.S
+++ b/arch/arm64/kernel/head.S
@@ -547,6 +547,8 @@ SYM_INNER_LABEL(init_el2, SYM_L_LOCAL)
 	mov_q	x0, INIT_SCTLR_EL1_MMU_OFF
 	msr	sctlr_el1, x0
 
+	mov_q	x0, INIT_PSTATE_EL1
+	msr	spsr_el2, x0
 	msr	elr_el2, lr
 	mov	w0, #BOOT_CPU_MODE_EL2
 	eret
-- 
2.30.2


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

* [PATCH 2/5] arm64: Handle UNDEF in the EL2 stub vectors
  2021-08-12 19:02 [PATCH 0/5] arm64: Survival kit for SCR_EL3.HCE==0 conditions Marc Zyngier
  2021-08-12 19:02 ` [PATCH 1/5] arm64: Directly expand __init_el2_nvhe_prepare_eret where needed Marc Zyngier
@ 2021-08-12 19:02 ` Marc Zyngier
  2021-08-13 13:08   ` Robin Murphy
  2021-08-12 19:02 ` [PATCH 3/5] arm64: Detect disabled HVC early Marc Zyngier
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 20+ messages in thread
From: Marc Zyngier @ 2021-08-12 19:02 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel
  Cc: Rafał Miłecki, Will Deacon, Catalin Marinas,
	Mark Rutland, Ard Biesheuvel, Florian Fainelli,
	bcm-kernel-feedback-list, kernel-team

As we want to handle the silly case where HVC has been disabled
from EL3, augment our ability to handle exception at EL2.

Check for unknown exceptions (usually UNDEF) coming from EL2,
and treat them as a failing HVC call into the stubs. While
this isn't great and obviously doesn't catter for the gigantic
range of possible exceptions, it isn't any worse than what we
have today.

Just don't try and use it for anything else.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 arch/arm64/kernel/hyp-stub.S | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/kernel/hyp-stub.S b/arch/arm64/kernel/hyp-stub.S
index 43d212618834..026a34515b21 100644
--- a/arch/arm64/kernel/hyp-stub.S
+++ b/arch/arm64/kernel/hyp-stub.S
@@ -46,7 +46,16 @@ SYM_CODE_END(__hyp_stub_vectors)
 	.align 11
 
 SYM_CODE_START_LOCAL(elx_sync)
-	cmp	x0, #HVC_SET_VECTORS
+	mrs	x4, spsr_el2
+	and	x4, x4, #PSR_MODE_MASK
+	orr	x4, x4, #1
+	cmp	x4, #PSR_MODE_EL2h
+	b.ne	0f
+	mrs	x4, esr_el2
+	eor	x4, x4, #ESR_ELx_IL
+	cbz	x4, el2_undef
+
+0:	cmp	x0, #HVC_SET_VECTORS
 	b.ne	1f
 	msr	vbar_el2, x1
 	b	9f
@@ -71,6 +80,14 @@ SYM_CODE_START_LOCAL(elx_sync)
 
 9:	mov	x0, xzr
 	eret
+
+el2_undef:
+	// Assumes this was a HVC that went really wrong...
+	mrs	x0, elr_el2
+	add	x0, x0, #4
+	msr	elr_el2, x0
+	mov_q	x0, HVC_STUB_ERR
+	eret
 SYM_CODE_END(elx_sync)
 
 // nVHE? No way! Give me the real thing!
-- 
2.30.2


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

* [PATCH 3/5] arm64: Detect disabled HVC early
  2021-08-12 19:02 [PATCH 0/5] arm64: Survival kit for SCR_EL3.HCE==0 conditions Marc Zyngier
  2021-08-12 19:02 ` [PATCH 1/5] arm64: Directly expand __init_el2_nvhe_prepare_eret where needed Marc Zyngier
  2021-08-12 19:02 ` [PATCH 2/5] arm64: Handle UNDEF in the EL2 stub vectors Marc Zyngier
@ 2021-08-12 19:02 ` Marc Zyngier
  2021-08-12 19:47   ` Rafał Miłecki
  2021-08-13  9:05   ` Will Deacon
  2021-08-12 19:02 ` [PATCH 4/5] arm64: Warn on booting at EL2 with HVC disabled Marc Zyngier
                   ` (3 subsequent siblings)
  6 siblings, 2 replies; 20+ messages in thread
From: Marc Zyngier @ 2021-08-12 19:02 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel
  Cc: Rafał Miłecki, Will Deacon, Catalin Marinas,
	Mark Rutland, Ard Biesheuvel, Florian Fainelli,
	bcm-kernel-feedback-list, kernel-team

Having HVC disabled from EL3 while the kernel is entered at EL2
is a complete nightmare.

We end-up taking an UNDEF at the worse possible moment (checking
for VHE) and even if we didn't, having KVM enabled would signify
the premature end of the kernel.

Instead, try and detect this stupid case by issuing a HVC
for HVC_RESET_VECTORS, which does nothing when the stubs
are live. If we get HVC_STUB_ERR back, that's because the
UNDEF handler has kicked in.

In this situation, close your eyes, block your nose, and gracefully
pretend we have booted at EL1.

Reported-by: Rafał Miłecki <zajec5@gmail.com>
Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 arch/arm64/kernel/head.S | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
index 9d5aa56a98cc..d6b2b05f5d3a 100644
--- a/arch/arm64/kernel/head.S
+++ b/arch/arm64/kernel/head.S
@@ -519,6 +519,28 @@ SYM_INNER_LABEL(init_el2, SYM_L_LOCAL)
 	msr	vbar_el2, x0
 	isb
 
+	// Check that HVC actually works...
+	mov	x0, #HVC_RESET_VECTORS
+	hvc	#0
+
+	mov_q	x1, HVC_STUB_ERR
+	cmp	x0, x1
+	b.ne	0f
+
+	/*
+	 * HVC is unusable, so pretend we actually booted at EL1.
+	 * Once we have left EL2, there will be no going back.
+	 */
+	mov_q	x0, INIT_SCTLR_EL1_MMU_OFF
+	msr	sctlr_el1, x0
+
+	mov_q	x0, INIT_PSTATE_EL1
+	msr	spsr_el2, x0
+	msr	elr_el2, lr
+	mov_q	w0, BOOT_CPU_MODE_EL1
+	eret
+
+0:
 	/*
 	 * Fruity CPUs seem to have HCR_EL2.E2H set to RES1,
 	 * making it impossible to start in nVHE mode. Is that
-- 
2.30.2


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

* [PATCH 4/5] arm64: Warn on booting at EL2 with HVC disabled
  2021-08-12 19:02 [PATCH 0/5] arm64: Survival kit for SCR_EL3.HCE==0 conditions Marc Zyngier
                   ` (2 preceding siblings ...)
  2021-08-12 19:02 ` [PATCH 3/5] arm64: Detect disabled HVC early Marc Zyngier
@ 2021-08-12 19:02 ` Marc Zyngier
  2021-08-12 19:58   ` Rafał Miłecki
  2021-08-12 19:02 ` [PATCH 5/5] arm64: Document the requirement for SCR_EL3.HCE Marc Zyngier
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 20+ messages in thread
From: Marc Zyngier @ 2021-08-12 19:02 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel
  Cc: Rafał Miłecki, Will Deacon, Catalin Marinas,
	Mark Rutland, Ard Biesheuvel, Florian Fainelli,
	bcm-kernel-feedback-list, kernel-team

Now that we are able to paper over the gigantic stupidity that
booting at EL2 with SCR_EL3.HCE==0 is, let's taint WARN_TAINT()
when detecting this situation.

Yes, this is *LOUD*.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 arch/arm64/include/asm/virt.h | 10 ++++++++++
 arch/arm64/kernel/head.S      | 10 ++++++++++
 arch/arm64/kernel/smp.c       |  3 +++
 3 files changed, 23 insertions(+)

diff --git a/arch/arm64/include/asm/virt.h b/arch/arm64/include/asm/virt.h
index 7379f35ae2c6..89bf5ae522da 100644
--- a/arch/arm64/include/asm/virt.h
+++ b/arch/arm64/include/asm/virt.h
@@ -49,6 +49,9 @@
 #define BOOT_CPU_MODE_EL1	(0xe11)
 #define BOOT_CPU_MODE_EL2	(0xe12)
 
+/* Flags associated to the boot mode */
+#define BOOT_CPU_MODE_DOWNGRADED (1 << 0)
+
 #ifndef __ASSEMBLY__
 
 #include <asm/ptrace.h>
@@ -67,6 +70,13 @@
  */
 extern u32 __boot_cpu_mode[2];
 
+/*
+ * __boot_cpu_mode_flags records events that are associated with CPUs
+ * coming online. A CPU having been downgraded from EL2 to EL1 because
+ * of HVC not being enabled will have BOOT_CPU_MODE_DOWNGRADED set.
+ */
+extern u32 __boot_cpu_mode_flags[1];
+
 void __hyp_set_vectors(phys_addr_t phys_vector_base);
 void __hyp_reset_vectors(void);
 
diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
index d6b2b05f5d3a..fdad6805868b 100644
--- a/arch/arm64/kernel/head.S
+++ b/arch/arm64/kernel/head.S
@@ -530,7 +530,13 @@ SYM_INNER_LABEL(init_el2, SYM_L_LOCAL)
 	/*
 	 * HVC is unusable, so pretend we actually booted at EL1.
 	 * Once we have left EL2, there will be no going back.
+	 * set_cpu_boot_mode_flag will do the necessary CMOs for us.
 	 */
+	adr_l	x1, __boot_cpu_mode_flags
+	ldr	w0, [x1]
+	orr	w0, w0, BOOT_CPU_MODE_DOWNGRADED
+	str	w0, [x1]
+
 	mov_q	x0, INIT_SCTLR_EL1_MMU_OFF
 	msr	sctlr_el1, x0
 
@@ -623,6 +629,10 @@ SYM_DATA_START(__early_cpu_boot_status)
 	.quad 	0
 SYM_DATA_END(__early_cpu_boot_status)
 
+SYM_DATA_START(__boot_cpu_mode_flags)
+	.long	0
+SYM_DATA_END(__boot_cpu_mode_flags)
+
 	.popsection
 
 	/*
diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
index 6f6ff072acbd..43fad7ca9110 100644
--- a/arch/arm64/kernel/smp.c
+++ b/arch/arm64/kernel/smp.c
@@ -425,6 +425,9 @@ static void __init hyp_mode_check(void)
 	else if (is_hyp_mode_mismatched())
 		WARN_TAINT(1, TAINT_CPU_OUT_OF_SPEC,
 			   "CPU: CPUs started in inconsistent modes");
+	else if (__boot_cpu_mode_flags[0] & BOOT_CPU_MODE_DOWNGRADED)
+		WARN_TAINT(1, TAINT_CPU_OUT_OF_SPEC,
+			   "CPU: CPUs downgraded to EL1, HVC disabled");
 	else
 		pr_info("CPU: All CPU(s) started at EL1\n");
 	if (IS_ENABLED(CONFIG_KVM) && !is_kernel_in_hyp_mode()) {
-- 
2.30.2


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

* [PATCH 5/5] arm64: Document the requirement for SCR_EL3.HCE
  2021-08-12 19:02 [PATCH 0/5] arm64: Survival kit for SCR_EL3.HCE==0 conditions Marc Zyngier
                   ` (3 preceding siblings ...)
  2021-08-12 19:02 ` [PATCH 4/5] arm64: Warn on booting at EL2 with HVC disabled Marc Zyngier
@ 2021-08-12 19:02 ` Marc Zyngier
  2021-08-24 10:49   ` Catalin Marinas
  2021-08-15  7:28 ` [PATCH 0/5] arm64: Survival kit for SCR_EL3.HCE==0 conditions Florian Fainelli
  2021-08-24 16:19 ` (subset) " Catalin Marinas
  6 siblings, 1 reply; 20+ messages in thread
From: Marc Zyngier @ 2021-08-12 19:02 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel
  Cc: Rafał Miłecki, Will Deacon, Catalin Marinas,
	Mark Rutland, Ard Biesheuvel, Florian Fainelli,
	bcm-kernel-feedback-list, kernel-team

It is amazing that we never documented this absolutely basic
requirement: if you boot the kernel at EL2, you'd better
enable the HVC instruction from EL3.

Really, just do it.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 Documentation/arm64/booting.rst | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/arm64/booting.rst b/Documentation/arm64/booting.rst
index a9192e7a231b..6c729d0c4bc2 100644
--- a/Documentation/arm64/booting.rst
+++ b/Documentation/arm64/booting.rst
@@ -212,6 +212,11 @@ Before jumping into the kernel, the following conditions must be met:
   - The value of SCR_EL3.FIQ must be the same as the one present at boot
     time whenever the kernel is executing.
 
+  For all systems:
+  - If EL3 is present and the kernel is entered at EL2:
+
+    - SCR_EL3.HCE (bit 8) must be initialised to 0b1.
+
   For systems with a GICv3 interrupt controller to be used in v3 mode:
   - If EL3 is present:
 
-- 
2.30.2


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

* Re: [PATCH 3/5] arm64: Detect disabled HVC early
  2021-08-12 19:02 ` [PATCH 3/5] arm64: Detect disabled HVC early Marc Zyngier
@ 2021-08-12 19:47   ` Rafał Miłecki
  2021-08-13  9:05   ` Will Deacon
  1 sibling, 0 replies; 20+ messages in thread
From: Rafał Miłecki @ 2021-08-12 19:47 UTC (permalink / raw)
  To: Marc Zyngier, linux-arm-kernel, linux-kernel
  Cc: Will Deacon, Catalin Marinas, Mark Rutland, Ard Biesheuvel,
	Florian Fainelli, bcm-kernel-feedback-list, kernel-team

On 12.08.2021 21:02, Marc Zyngier wrote:
> Having HVC disabled from EL3 while the kernel is entered at EL2
> is a complete nightmare.
> 
> We end-up taking an UNDEF at the worse possible moment (checking
> for VHE) and even if we didn't, having KVM enabled would signify
> the premature end of the kernel.
> 
> Instead, try and detect this stupid case by issuing a HVC
> for HVC_RESET_VECTORS, which does nothing when the stubs
> are live. If we get HVC_STUB_ERR back, that's because the
> UNDEF handler has kicked in.
> 
> In this situation, close your eyes, block your nose, and gracefully
> pretend we have booted at EL1.
> 
> Reported-by: Rafał Miłecki <zajec5@gmail.com>
> Signed-off-by: Marc Zyngier <maz@kernel.org>

This does the trick.

Tested-by: Rafał Miłecki <rafal@milecki.pl>

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

* Re: [PATCH 4/5] arm64: Warn on booting at EL2 with HVC disabled
  2021-08-12 19:02 ` [PATCH 4/5] arm64: Warn on booting at EL2 with HVC disabled Marc Zyngier
@ 2021-08-12 19:58   ` Rafał Miłecki
  0 siblings, 0 replies; 20+ messages in thread
From: Rafał Miłecki @ 2021-08-12 19:58 UTC (permalink / raw)
  To: Marc Zyngier, linux-arm-kernel, linux-kernel
  Cc: Will Deacon, Catalin Marinas, Mark Rutland, Ard Biesheuvel,
	Florian Fainelli, bcm-kernel-feedback-list, kernel-team

On 12.08.2021 21:02, Marc Zyngier wrote:
> Now that we are able to paper over the gigantic stupidity that
> booting at EL2 with SCR_EL3.HCE==0 is, let's taint WARN_TAINT()
> when detecting this situation.
> 
> Yes, this is *LOUD*.
> 
> Signed-off-by: Marc Zyngier <maz@kernel.org>

Tested-by: Rafał Miłecki <rafal@milecki.pl>

This replaces:
CPU: All CPU(s) started at EL1

with:
------------[ cut here ]------------
CPU: CPUs downgraded to EL1, HVC disabled
WARNING: CPU: 0 PID: 1 at arch/arm64/kernel/smp.c:429 smp_cpus_done+0xac/0xe8
CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.14.0-rc5-g86fc10657896 #41
Hardware name: Asus GT-AC5300 (DT)
pstate: 60000005 (nZCv daif -PAN -UAO -TCO BTYPE=--)
pc : smp_cpus_done+0xac/0xe8
lr : smp_cpus_done+0xac/0xe8
sp : ffffffc01002be00
x29: ffffffc01002be00 x28: 0000000000000000 x27: 0000000000000000
x26: 0000000000000000 x25: 0000000000000000 x24: 0000000000000000
x23: ffffffc010ab4000 x22: 0000000000000000 x21: 0000000000000000
x20: ffffffc0107b7e74 x19: ffffffc010a78000 x18: 0000000000000001
x17: ffffffc010a9ee40 x16: 0000000000000000 x15: 000042496b0a18f2
x14: fffffffffffc0f87 x13: 0000000000000039 x12: ffffff80010b03b0
x11: 00000000ffffffea x10: ffffffc010a5eb50 x9 : 0000000000000001
x8 : 0000000000000001 x7 : 0000000000017fe8 x6 : c0000000ffffefff
x5 : 0000000000057fa8 x4 : 0000000000000000 x3 : 0000000000000000
x2 : 00000000ffffffff x1 : a8d68d1fd96fdc00 x0 : 0000000000000000
Call trace:
  smp_cpus_done+0xac/0xe8
  smp_init+0x68/0x78
  kernel_init_freeable+0xd0/0x214
  kernel_init+0x24/0x120
  ret_from_fork+0x10/0x18
---[ end trace a7d4af835e9d6e6b ]---


BEFORE:

smp: Bringing up secondary CPUs ...
Detected VIPT I-cache on CPU1
CPU1: Booted secondary processor 0x0000000001 [0x420f1000]
Detected VIPT I-cache on CPU2
CPU2: Booted secondary processor 0x0000000002 [0x420f1000]
Detected VIPT I-cache on CPU3
CPU3: Booted secondary processor 0x0000000003 [0x420f1000]
smp: Brought up 1 node, 4 CPUs
SMP: Total of 4 processors activated.
CPU features: detected: 32-bit EL0 Support
CPU features: detected: 32-bit EL1 Support
CPU features: detected: CRC32 instructions
CPU: All CPU(s) started at EL1


AFTER:

smp: Bringing up secondary CPUs ...
Detected VIPT I-cache on CPU1
CPU1: Booted secondary processor 0x0000000001 [0x420f1000]
Detected VIPT I-cache on CPU2
CPU2: Booted secondary processor 0x0000000002 [0x420f1000]
Detected VIPT I-cache on CPU3
CPU3: Booted secondary processor 0x0000000003 [0x420f1000]
smp: Brought up 1 node, 4 CPUs
SMP: Total of 4 processors activated.
CPU features: detected: 32-bit EL0 Support
CPU features: detected: 32-bit EL1 Support
CPU features: detected: CRC32 instructions
------------[ cut here ]------------
CPU: CPUs downgraded to EL1, HVC disabled
WARNING: CPU: 0 PID: 1 at arch/arm64/kernel/smp.c:429 smp_cpus_done+0xac/0xe8
CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.14.0-rc5-g86fc10657896 #41
Hardware name: Asus GT-AC5300 (DT)
pstate: 60000005 (nZCv daif -PAN -UAO -TCO BTYPE=--)
pc : smp_cpus_done+0xac/0xe8
lr : smp_cpus_done+0xac/0xe8
sp : ffffffc01002be00
x29: ffffffc01002be00 x28: 0000000000000000 x27: 0000000000000000
x26: 0000000000000000 x25: 0000000000000000 x24: 0000000000000000
x23: ffffffc010ab4000 x22: 0000000000000000 x21: 0000000000000000
x20: ffffffc0107b7e74 x19: ffffffc010a78000 x18: 0000000000000001
x17: ffffffc010a9ee40 x16: 0000000000000000 x15: 000042496b0a18f2
x14: fffffffffffc0f87 x13: 0000000000000039 x12: ffffff80010b03b0
x11: 00000000ffffffea x10: ffffffc010a5eb50 x9 : 0000000000000001
x8 : 0000000000000001 x7 : 0000000000017fe8 x6 : c0000000ffffefff
x5 : 0000000000057fa8 x4 : 0000000000000000 x3 : 0000000000000000
x2 : 00000000ffffffff x1 : a8d68d1fd96fdc00 x0 : 0000000000000000
Call trace:
  smp_cpus_done+0xac/0xe8
  smp_init+0x68/0x78
  kernel_init_freeable+0xd0/0x214
  kernel_init+0x24/0x120
  ret_from_fork+0x10/0x18
---[ end trace a7d4af835e9d6e6b ]---

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

* Re: [PATCH 3/5] arm64: Detect disabled HVC early
  2021-08-12 19:02 ` [PATCH 3/5] arm64: Detect disabled HVC early Marc Zyngier
  2021-08-12 19:47   ` Rafał Miłecki
@ 2021-08-13  9:05   ` Will Deacon
  2021-08-13 17:33     ` Marc Zyngier
  1 sibling, 1 reply; 20+ messages in thread
From: Will Deacon @ 2021-08-13  9:05 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: linux-arm-kernel, linux-kernel, Rafał Miłecki,
	Catalin Marinas, Mark Rutland, Ard Biesheuvel, Florian Fainelli,
	bcm-kernel-feedback-list, kernel-team

On Thu, Aug 12, 2021 at 08:02:11PM +0100, Marc Zyngier wrote:
> Having HVC disabled from EL3 while the kernel is entered at EL2
> is a complete nightmare.
> 
> We end-up taking an UNDEF at the worse possible moment (checking
> for VHE) and even if we didn't, having KVM enabled would signify
> the premature end of the kernel.
> 
> Instead, try and detect this stupid case by issuing a HVC
> for HVC_RESET_VECTORS, which does nothing when the stubs
> are live. If we get HVC_STUB_ERR back, that's because the
> UNDEF handler has kicked in.
> 
> In this situation, close your eyes, block your nose, and gracefully
> pretend we have booted at EL1.
> 
> Reported-by: Rafał Miłecki <zajec5@gmail.com>
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
>  arch/arm64/kernel/head.S | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
> 
> diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
> index 9d5aa56a98cc..d6b2b05f5d3a 100644
> --- a/arch/arm64/kernel/head.S
> +++ b/arch/arm64/kernel/head.S
> @@ -519,6 +519,28 @@ SYM_INNER_LABEL(init_el2, SYM_L_LOCAL)
>  	msr	vbar_el2, x0
>  	isb
>  
> +	// Check that HVC actually works...
> +	mov	x0, #HVC_RESET_VECTORS
> +	hvc	#0
> +
> +	mov_q	x1, HVC_STUB_ERR
> +	cmp	x0, x1
> +	b.ne	0f
> +
> +	/*
> +	 * HVC is unusable, so pretend we actually booted at EL1.
> +	 * Once we have left EL2, there will be no going back.
> +	 */

This comment got me thinking...

	.macro	host_hvc0
	mrs	xzr, actlr_el1
	.endm

then set HCR_EL2.TACR=1 while we still can and match the ISS against a
constant in the handler. Too awful?

Will

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

* Re: [PATCH 2/5] arm64: Handle UNDEF in the EL2 stub vectors
  2021-08-12 19:02 ` [PATCH 2/5] arm64: Handle UNDEF in the EL2 stub vectors Marc Zyngier
@ 2021-08-13 13:08   ` Robin Murphy
  2021-08-13 17:41     ` Marc Zyngier
  0 siblings, 1 reply; 20+ messages in thread
From: Robin Murphy @ 2021-08-13 13:08 UTC (permalink / raw)
  To: Marc Zyngier, linux-arm-kernel, linux-kernel
  Cc: Rafał Miłecki, Will Deacon, Catalin Marinas,
	Mark Rutland, Ard Biesheuvel, Florian Fainelli,
	bcm-kernel-feedback-list, kernel-team

On 2021-08-12 20:02, Marc Zyngier wrote:
> As we want to handle the silly case where HVC has been disabled
> from EL3, augment our ability to handle exception at EL2.
> 
> Check for unknown exceptions (usually UNDEF) coming from EL2,
> and treat them as a failing HVC call into the stubs. While
> this isn't great and obviously doesn't catter for the gigantic
> range of possible exceptions, it isn't any worse than what we
> have today.
> 
> Just don't try and use it for anything else.
> 
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
>   arch/arm64/kernel/hyp-stub.S | 19 ++++++++++++++++++-
>   1 file changed, 18 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/kernel/hyp-stub.S b/arch/arm64/kernel/hyp-stub.S
> index 43d212618834..026a34515b21 100644
> --- a/arch/arm64/kernel/hyp-stub.S
> +++ b/arch/arm64/kernel/hyp-stub.S
> @@ -46,7 +46,16 @@ SYM_CODE_END(__hyp_stub_vectors)
>   	.align 11
>   
>   SYM_CODE_START_LOCAL(elx_sync)
> -	cmp	x0, #HVC_SET_VECTORS
> +	mrs	x4, spsr_el2
> +	and	x4, x4, #PSR_MODE_MASK
> +	orr	x4, x4, #1
> +	cmp	x4, #PSR_MODE_EL2h
> +	b.ne	0f
> +	mrs	x4, esr_el2
> +	eor	x4, x4, #ESR_ELx_IL
> +	cbz	x4, el2_undef

Hmm, might it be neater to check ESR_EL2.ISS to see if we landed here 
for any reason *other* than a successfully-executed HVC?

Robin.

> +
> +0:	cmp	x0, #HVC_SET_VECTORS
>   	b.ne	1f
>   	msr	vbar_el2, x1
>   	b	9f
> @@ -71,6 +80,14 @@ SYM_CODE_START_LOCAL(elx_sync)
>   
>   9:	mov	x0, xzr
>   	eret
> +
> +el2_undef:
> +	// Assumes this was a HVC that went really wrong...
> +	mrs	x0, elr_el2
> +	add	x0, x0, #4
> +	msr	elr_el2, x0
> +	mov_q	x0, HVC_STUB_ERR
> +	eret
>   SYM_CODE_END(elx_sync)
>   
>   // nVHE? No way! Give me the real thing!
> 

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

* Re: [PATCH 3/5] arm64: Detect disabled HVC early
  2021-08-13  9:05   ` Will Deacon
@ 2021-08-13 17:33     ` Marc Zyngier
  0 siblings, 0 replies; 20+ messages in thread
From: Marc Zyngier @ 2021-08-13 17:33 UTC (permalink / raw)
  To: Will Deacon
  Cc: linux-arm-kernel, linux-kernel, Rafał Miłecki,
	Catalin Marinas, Mark Rutland, Ard Biesheuvel, Florian Fainelli,
	bcm-kernel-feedback-list, kernel-team

On Fri, 13 Aug 2021 10:05:40 +0100,
Will Deacon <will@kernel.org> wrote:
> 
> On Thu, Aug 12, 2021 at 08:02:11PM +0100, Marc Zyngier wrote:
> > Having HVC disabled from EL3 while the kernel is entered at EL2
> > is a complete nightmare.
> > 
> > We end-up taking an UNDEF at the worse possible moment (checking
> > for VHE) and even if we didn't, having KVM enabled would signify
> > the premature end of the kernel.
> > 
> > Instead, try and detect this stupid case by issuing a HVC
> > for HVC_RESET_VECTORS, which does nothing when the stubs
> > are live. If we get HVC_STUB_ERR back, that's because the
> > UNDEF handler has kicked in.
> > 
> > In this situation, close your eyes, block your nose, and gracefully
> > pretend we have booted at EL1.
> > 
> > Reported-by: Rafał Miłecki <zajec5@gmail.com>
> > Signed-off-by: Marc Zyngier <maz@kernel.org>
> > ---
> >  arch/arm64/kernel/head.S | 22 ++++++++++++++++++++++
> >  1 file changed, 22 insertions(+)
> > 
> > diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
> > index 9d5aa56a98cc..d6b2b05f5d3a 100644
> > --- a/arch/arm64/kernel/head.S
> > +++ b/arch/arm64/kernel/head.S
> > @@ -519,6 +519,28 @@ SYM_INNER_LABEL(init_el2, SYM_L_LOCAL)
> >  	msr	vbar_el2, x0
> >  	isb
> >  
> > +	// Check that HVC actually works...
> > +	mov	x0, #HVC_RESET_VECTORS
> > +	hvc	#0
> > +
> > +	mov_q	x1, HVC_STUB_ERR
> > +	cmp	x0, x1
> > +	b.ne	0f
> > +
> > +	/*
> > +	 * HVC is unusable, so pretend we actually booted at EL1.
> > +	 * Once we have left EL2, there will be no going back.
> > +	 */
> 
> This comment got me thinking...
> 
> 	.macro	host_hvc0
> 	mrs	xzr, actlr_el1
> 	.endm
> 
> then set HCR_EL2.TACR=1 while we still can and match the ISS against a
> constant in the handler. Too awful?

I had a similar idea, using an ID register instead (though ACTLR_EL1
is much neater). It would indeed allow the kernel to go back to EL2.

But for what purpose? Yes, we could enable KVM that way. However, the
guest would need to know about it as well, as even simple things like
PSCI wouldn't work. That's would be defining a new guest ABI, and I'm
not overly keen on it.

Unless you are thinking of something else?

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.

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

* Re: [PATCH 2/5] arm64: Handle UNDEF in the EL2 stub vectors
  2021-08-13 13:08   ` Robin Murphy
@ 2021-08-13 17:41     ` Marc Zyngier
  2021-08-13 18:17       ` Robin Murphy
  0 siblings, 1 reply; 20+ messages in thread
From: Marc Zyngier @ 2021-08-13 17:41 UTC (permalink / raw)
  To: Robin Murphy
  Cc: linux-arm-kernel, linux-kernel, Rafał Miłecki,
	Will Deacon, Catalin Marinas, Mark Rutland, Ard Biesheuvel,
	Florian Fainelli, bcm-kernel-feedback-list, kernel-team

On Fri, 13 Aug 2021 14:08:23 +0100,
Robin Murphy <robin.murphy@arm.com> wrote:
> 
> On 2021-08-12 20:02, Marc Zyngier wrote:
> > As we want to handle the silly case where HVC has been disabled
> > from EL3, augment our ability to handle exception at EL2.
> > 
> > Check for unknown exceptions (usually UNDEF) coming from EL2,
> > and treat them as a failing HVC call into the stubs. While
> > this isn't great and obviously doesn't catter for the gigantic
> > range of possible exceptions, it isn't any worse than what we
> > have today.
> > 
> > Just don't try and use it for anything else.
> > 
> > Signed-off-by: Marc Zyngier <maz@kernel.org>
> > ---
> >   arch/arm64/kernel/hyp-stub.S | 19 ++++++++++++++++++-
> >   1 file changed, 18 insertions(+), 1 deletion(-)
> > 
> > diff --git a/arch/arm64/kernel/hyp-stub.S b/arch/arm64/kernel/hyp-stub.S
> > index 43d212618834..026a34515b21 100644
> > --- a/arch/arm64/kernel/hyp-stub.S
> > +++ b/arch/arm64/kernel/hyp-stub.S
> > @@ -46,7 +46,16 @@ SYM_CODE_END(__hyp_stub_vectors)
> >   	.align 11
> >     SYM_CODE_START_LOCAL(elx_sync)
> > -	cmp	x0, #HVC_SET_VECTORS
> > +	mrs	x4, spsr_el2
> > +	and	x4, x4, #PSR_MODE_MASK
> > +	orr	x4, x4, #1
> > +	cmp	x4, #PSR_MODE_EL2h
> > +	b.ne	0f
> > +	mrs	x4, esr_el2
> > +	eor	x4, x4, #ESR_ELx_IL
> > +	cbz	x4, el2_undef
> 
> Hmm, might it be neater to check ESR_EL2.ISS to see if we landed here
> for any reason *other* than a successfully-executed HVC?

We absolutely could. However, the sixpence question (yes, that's the
Brexit effect for you) is "what do you do with exceptions that are
neither UNDEF now HVC?".

We are taking a leap of faith by assuming that the only thing that
will UNDEF at EL2 while the stubs are installed is HVC. If anything
else occurs, I have no idea what to do with it. I guess we could always
ignore it instead of treating it as a HVC (as it is done at the
moment).

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.

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

* Re: [PATCH 2/5] arm64: Handle UNDEF in the EL2 stub vectors
  2021-08-13 17:41     ` Marc Zyngier
@ 2021-08-13 18:17       ` Robin Murphy
  2021-08-14  9:38         ` Marc Zyngier
  0 siblings, 1 reply; 20+ messages in thread
From: Robin Murphy @ 2021-08-13 18:17 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: linux-arm-kernel, linux-kernel, Rafał Miłecki,
	Will Deacon, Catalin Marinas, Mark Rutland, Ard Biesheuvel,
	Florian Fainelli, bcm-kernel-feedback-list, kernel-team

On 2021-08-13 18:41, Marc Zyngier wrote:
> On Fri, 13 Aug 2021 14:08:23 +0100,
> Robin Murphy <robin.murphy@arm.com> wrote:
>>
>> On 2021-08-12 20:02, Marc Zyngier wrote:
>>> As we want to handle the silly case where HVC has been disabled
>>> from EL3, augment our ability to handle exception at EL2.
>>>
>>> Check for unknown exceptions (usually UNDEF) coming from EL2,
>>> and treat them as a failing HVC call into the stubs. While
>>> this isn't great and obviously doesn't catter for the gigantic
>>> range of possible exceptions, it isn't any worse than what we
>>> have today.
>>>
>>> Just don't try and use it for anything else.
>>>
>>> Signed-off-by: Marc Zyngier <maz@kernel.org>
>>> ---
>>>    arch/arm64/kernel/hyp-stub.S | 19 ++++++++++++++++++-
>>>    1 file changed, 18 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/arch/arm64/kernel/hyp-stub.S b/arch/arm64/kernel/hyp-stub.S
>>> index 43d212618834..026a34515b21 100644
>>> --- a/arch/arm64/kernel/hyp-stub.S
>>> +++ b/arch/arm64/kernel/hyp-stub.S
>>> @@ -46,7 +46,16 @@ SYM_CODE_END(__hyp_stub_vectors)
>>>    	.align 11
>>>      SYM_CODE_START_LOCAL(elx_sync)
>>> -	cmp	x0, #HVC_SET_VECTORS
>>> +	mrs	x4, spsr_el2
>>> +	and	x4, x4, #PSR_MODE_MASK
>>> +	orr	x4, x4, #1
>>> +	cmp	x4, #PSR_MODE_EL2h
>>> +	b.ne	0f
>>> +	mrs	x4, esr_el2
>>> +	eor	x4, x4, #ESR_ELx_IL
>>> +	cbz	x4, el2_undef
>>
>> Hmm, might it be neater to check ESR_EL2.ISS to see if we landed here
>> for any reason *other* than a successfully-executed HVC?
> 
> We absolutely could. However, the sixpence question (yes, that's the
> Brexit effect for you) is "what do you do with exceptions that are
> neither UNDEF now HVC?".
> 
> We are taking a leap of faith by assuming that the only thing that
> will UNDEF at EL2 while the stubs are installed is HVC. If anything
> else occurs, I have no idea what to do with it. I guess we could always
> ignore it instead of treating it as a HVC (as it is done at the
> moment).

Right, I think that concern applies pretty much equally whichever way 
you slice it. "Any exception other than an unknown from EL2 must imply 
HVC" doesn't seem any less sketchy than "Any exception other than HVC 
implies something is horribly wrong and abandoning EL2 might be wise" to 
me, but it was primarily that the latter avoids having to faff with the 
SPSR as well. No big deal either way, just one of my "I reckon this 
could be shorter..." musings; it's been particularly Friday today :)

Robin.

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

* Re: [PATCH 2/5] arm64: Handle UNDEF in the EL2 stub vectors
  2021-08-13 18:17       ` Robin Murphy
@ 2021-08-14  9:38         ` Marc Zyngier
  0 siblings, 0 replies; 20+ messages in thread
From: Marc Zyngier @ 2021-08-14  9:38 UTC (permalink / raw)
  To: Robin Murphy
  Cc: linux-arm-kernel, linux-kernel, Rafał Miłecki,
	Will Deacon, Catalin Marinas, Mark Rutland, Ard Biesheuvel,
	Florian Fainelli, bcm-kernel-feedback-list, kernel-team

On Fri, 13 Aug 2021 19:17:56 +0100,
Robin Murphy <robin.murphy@arm.com> wrote:
> 
> On 2021-08-13 18:41, Marc Zyngier wrote:
> > On Fri, 13 Aug 2021 14:08:23 +0100,
> > Robin Murphy <robin.murphy@arm.com> wrote:
> >> 
> >> On 2021-08-12 20:02, Marc Zyngier wrote:
> >>> As we want to handle the silly case where HVC has been disabled
> >>> from EL3, augment our ability to handle exception at EL2.
> >>> 
> >>> Check for unknown exceptions (usually UNDEF) coming from EL2,
> >>> and treat them as a failing HVC call into the stubs. While
> >>> this isn't great and obviously doesn't catter for the gigantic
> >>> range of possible exceptions, it isn't any worse than what we
> >>> have today.
> >>> 
> >>> Just don't try and use it for anything else.
> >>> 
> >>> Signed-off-by: Marc Zyngier <maz@kernel.org>
> >>> ---
> >>>    arch/arm64/kernel/hyp-stub.S | 19 ++++++++++++++++++-
> >>>    1 file changed, 18 insertions(+), 1 deletion(-)
> >>> 
> >>> diff --git a/arch/arm64/kernel/hyp-stub.S b/arch/arm64/kernel/hyp-stub.S
> >>> index 43d212618834..026a34515b21 100644
> >>> --- a/arch/arm64/kernel/hyp-stub.S
> >>> +++ b/arch/arm64/kernel/hyp-stub.S
> >>> @@ -46,7 +46,16 @@ SYM_CODE_END(__hyp_stub_vectors)
> >>>    	.align 11
> >>>      SYM_CODE_START_LOCAL(elx_sync)
> >>> -	cmp	x0, #HVC_SET_VECTORS
> >>> +	mrs	x4, spsr_el2
> >>> +	and	x4, x4, #PSR_MODE_MASK
> >>> +	orr	x4, x4, #1
> >>> +	cmp	x4, #PSR_MODE_EL2h
> >>> +	b.ne	0f
> >>> +	mrs	x4, esr_el2
> >>> +	eor	x4, x4, #ESR_ELx_IL
> >>> +	cbz	x4, el2_undef
> >> 
> >> Hmm, might it be neater to check ESR_EL2.ISS to see if we landed here
> >> for any reason *other* than a successfully-executed HVC?
> > 
> > We absolutely could. However, the sixpence question (yes, that's the
> > Brexit effect for you) is "what do you do with exceptions that are
> > neither UNDEF now HVC?".
> > 
> > We are taking a leap of faith by assuming that the only thing that
> > will UNDEF at EL2 while the stubs are installed is HVC. If anything
> > else occurs, I have no idea what to do with it. I guess we could always
> > ignore it instead of treating it as a HVC (as it is done at the
> > moment).
> 
> Right, I think that concern applies pretty much equally whichever way
> you slice it. "Any exception other than an unknown from EL2 must imply
> HVC" doesn't seem any less sketchy than "Any exception other than HVC
> implies something is horribly wrong and abandoning EL2 might be wise"
> to me, but it was primarily that the latter avoids having to faff with
> the SPSR as well.

Actually, that's not a bad idea at all. Here's my take on the theme,
completely untested:

diff --git a/arch/arm64/kernel/hyp-stub.S b/arch/arm64/kernel/hyp-stub.S
index 43d212618834..5783dbab529f 100644
--- a/arch/arm64/kernel/hyp-stub.S
+++ b/arch/arm64/kernel/hyp-stub.S
@@ -46,6 +46,23 @@ SYM_CODE_END(__hyp_stub_vectors)
 	.align 11
 
 SYM_CODE_START_LOCAL(elx_sync)
+	// tpidr_el2 isn't used for anything while the stubs are
+	// installed, so use it to save x0 while we guess the
+	// exception type. No, we don't have a stack...
+	msr	tpidr_el2, x0
+	mrs	x0, esr_el2
+	ubfx	x0, x0, #26, #6
+	cmp	x0, #ESR_ELx_EC_HVC64
+	b.eq	elx_hvc
+	cbz	x0, elx_unknown
+
+	// For anything else, we have no reasonable way to handle
+	// the exception. Go back to the faulting instruction...
+	mrs	x0, tpidr_el2
+	eret
+
+elx_hvc:
+	mrs	x0, tpidr_el2
 	cmp	x0, #HVC_SET_VECTORS
 	b.ne	1f
 	msr	vbar_el2, x1
@@ -71,6 +88,14 @@ SYM_CODE_START_LOCAL(elx_sync)
 
 9:	mov	x0, xzr
 	eret
+
+elx_unknown:
+	// Assumes this was a HVC that went really wrong...
+	mrs	x0, elr_el2
+	add	x0, x0, #4
+	msr	elr_el2, x0
+	mov_q	x0, HVC_STUB_ERR
+	eret
 SYM_CODE_END(elx_sync)
 
 // nVHE? No way! Give me the real thing!


> No big deal either way, just one of my "I reckon this could be
> shorter..." musings; it's been particularly Friday today :)

Well, I just made it a lot longer! :D Let me know what you think.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.

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

* Re: [PATCH 0/5] arm64: Survival kit for SCR_EL3.HCE==0 conditions
  2021-08-12 19:02 [PATCH 0/5] arm64: Survival kit for SCR_EL3.HCE==0 conditions Marc Zyngier
                   ` (4 preceding siblings ...)
  2021-08-12 19:02 ` [PATCH 5/5] arm64: Document the requirement for SCR_EL3.HCE Marc Zyngier
@ 2021-08-15  7:28 ` Florian Fainelli
  2021-08-15  9:27   ` Marc Zyngier
  2021-08-24 16:19 ` (subset) " Catalin Marinas
  6 siblings, 1 reply; 20+ messages in thread
From: Florian Fainelli @ 2021-08-15  7:28 UTC (permalink / raw)
  To: Marc Zyngier, linux-arm-kernel, linux-kernel
  Cc: Rafał Miłecki, Will Deacon, Catalin Marinas,
	Mark Rutland, Ard Biesheuvel, Florian Fainelli,
	bcm-kernel-feedback-list, kernel-team



On 8/12/2021 9:02 PM, Marc Zyngier wrote:
> Anyone vaguely familiar with the ARMv8 architecture would quickly
> understand that entering the kernel at EL2 without enabling the HVC
> instruction is... living dangerously. But as it turns out [0], there
> is a whole range of (*cough*) "high quality" (*cough*) Broadcom
> systems out there configured exactly like that.

Some Broadcom systems, namely the 4908 and all of those using CFE, they 
later switched to u-boot and ATF and got it right.

> 
> If you are speechless, I'm right with you.
> 
> These machines have stopped being able to boot an upstream kernel
> since 5.12, where we changed the way we switch from nVHE to VHE, as
> this relies on the HVC instruction being usable... It is also worth
> noting that these systems have never been able to use KVM. Or kexec.
> 
> This small series addresses the issue by detecting an UNDEFing HVC in
> a fairly controlled environment, and in this case pretend that we have
> booted at EL1. It also documents the requirement for SCR_EL3.HCE to be
> set to *1* if the kernel is entered at EL2. Turns out that we really
> have to state the obvious.
> 
> This has been tested on a FVP model with a hacked-up boot-wrapper.
> 
> Note that I really don't think any of this is -stable material, except
> maybe for the documentation. It isn't 5.14 material either. Best case,
> this is 5.15, or maybe even later. If ever.

While I am very appreciative of the work you have done here to try to 
get the dysfunctional systems to warn and continue to boot, I would 
rather we try to load a minimal shim at EL3 capable of fixing up any 
incorrect EL3 register setting ahead of loading the kernel provided this 
is possible at all on a commercially available system. Rafal, is this 
something that CFE allows you to do (as I could not get a straight 
answer from that team), if so have you tried it?
-- 
Florian

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

* Re: [PATCH 0/5] arm64: Survival kit for SCR_EL3.HCE==0 conditions
  2021-08-15  7:28 ` [PATCH 0/5] arm64: Survival kit for SCR_EL3.HCE==0 conditions Florian Fainelli
@ 2021-08-15  9:27   ` Marc Zyngier
  2021-08-22 11:31     ` Florian Fainelli
  0 siblings, 1 reply; 20+ messages in thread
From: Marc Zyngier @ 2021-08-15  9:27 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: linux-arm-kernel, linux-kernel, Rafał Miłecki,
	Will Deacon, Catalin Marinas, Mark Rutland, Ard Biesheuvel,
	bcm-kernel-feedback-list, kernel-team

On Sun, 15 Aug 2021 08:28:47 +0100,
Florian Fainelli <f.fainelli@gmail.com> wrote:
> 
> 
> 
> On 8/12/2021 9:02 PM, Marc Zyngier wrote:
> > Anyone vaguely familiar with the ARMv8 architecture would quickly
> > understand that entering the kernel at EL2 without enabling the HVC
> > instruction is... living dangerously. But as it turns out [0], there
> > is a whole range of (*cough*) "high quality" (*cough*) Broadcom
> > systems out there configured exactly like that.
> 
> Some Broadcom systems, namely the 4908 and all of those using CFE,
> they later switched to u-boot and ATF and got it right.

Do we have a list of the affected systems?

> 
> > 
> > If you are speechless, I'm right with you.
> > 
> > These machines have stopped being able to boot an upstream kernel
> > since 5.12, where we changed the way we switch from nVHE to VHE, as
> > this relies on the HVC instruction being usable... It is also worth
> > noting that these systems have never been able to use KVM. Or kexec.
> > 
> > This small series addresses the issue by detecting an UNDEFing HVC in
> > a fairly controlled environment, and in this case pretend that we have
> > booted at EL1. It also documents the requirement for SCR_EL3.HCE to be
> > set to *1* if the kernel is entered at EL2. Turns out that we really
> > have to state the obvious.
> > 
> > This has been tested on a FVP model with a hacked-up boot-wrapper.
> > 
> > Note that I really don't think any of this is -stable material, except
> > maybe for the documentation. It isn't 5.14 material either. Best case,
> > this is 5.15, or maybe even later. If ever.
> 
> While I am very appreciative of the work you have done here to try to
> get the dysfunctional systems to warn and continue to boot, I would
> rather we try to load a minimal shim at EL3 capable of fixing up any
> incorrect EL3 register setting ahead of loading the kernel provided
> this is possible at all on a commercially available system.

That would be the best thing to do, and would make the machine fully
usable. I still think we need to have something in the kernel to at
least let the user know that their system is misconfigured though.

If CFE allows a payload to be loaded at EL3 and executed on all CPUs,
that would be absolutely awesome. It would even allow switching over
to ATF...

Thanks,

	M.

> Rafal, is this something that CFE allows you to do (as I could not
> get a straight answer from that team), if so have you tried it?
> -- 
> Florian
> 

-- 
Without deviation from the norm, progress is not possible.

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

* Re: [PATCH 0/5] arm64: Survival kit for SCR_EL3.HCE==0 conditions
  2021-08-15  9:27   ` Marc Zyngier
@ 2021-08-22 11:31     ` Florian Fainelli
  0 siblings, 0 replies; 20+ messages in thread
From: Florian Fainelli @ 2021-08-22 11:31 UTC (permalink / raw)
  To: Marc Zyngier, Florian Fainelli
  Cc: linux-arm-kernel, linux-kernel, Rafał Miłecki,
	Will Deacon, Catalin Marinas, Mark Rutland, Ard Biesheuvel,
	bcm-kernel-feedback-list, kernel-team



On 8/15/2021 11:27 AM, Marc Zyngier wrote:
> On Sun, 15 Aug 2021 08:28:47 +0100,
> Florian Fainelli <f.fainelli@gmail.com> wrote:
>>
>>
>>
>> On 8/12/2021 9:02 PM, Marc Zyngier wrote:
>>> Anyone vaguely familiar with the ARMv8 architecture would quickly
>>> understand that entering the kernel at EL2 without enabling the HVC
>>> instruction is... living dangerously. But as it turns out [0], there
>>> is a whole range of (*cough*) "high quality" (*cough*) Broadcom
>>> systems out there configured exactly like that.
>>
>> Some Broadcom systems, namely the 4908 and all of those using CFE,
>> they later switched to u-boot and ATF and got it right.
> 
> Do we have a list of the affected systems?

That is going to be hard to come up with given that OEMs such as Asus, 
TP-Link, or Netgear and multiple devices are affected. I would start 
with this list which is far from exhaustive: Netgear R8000P, TP-Link 
Archer C2300, Asus GT AC5300.

> 
>>
>>>
>>> If you are speechless, I'm right with you.
>>>
>>> These machines have stopped being able to boot an upstream kernel
>>> since 5.12, where we changed the way we switch from nVHE to VHE, as
>>> this relies on the HVC instruction being usable... It is also worth
>>> noting that these systems have never been able to use KVM. Or kexec.
>>>
>>> This small series addresses the issue by detecting an UNDEFing HVC in
>>> a fairly controlled environment, and in this case pretend that we have
>>> booted at EL1. It also documents the requirement for SCR_EL3.HCE to be
>>> set to *1* if the kernel is entered at EL2. Turns out that we really
>>> have to state the obvious.
>>>
>>> This has been tested on a FVP model with a hacked-up boot-wrapper.
>>>
>>> Note that I really don't think any of this is -stable material, except
>>> maybe for the documentation. It isn't 5.14 material either. Best case,
>>> this is 5.15, or maybe even later. If ever.
>>
>> While I am very appreciative of the work you have done here to try to
>> get the dysfunctional systems to warn and continue to boot, I would
>> rather we try to load a minimal shim at EL3 capable of fixing up any
>> incorrect EL3 register setting ahead of loading the kernel provided
>> this is possible at all on a commercially available system.
> 
> That would be the best thing to do, and would make the machine fully
> usable. I still think we need to have something in the kernel to at
> least let the user know that their system is misconfigured though.
> 
> If CFE allows a payload to be loaded at EL3 and executed on all CPUs,
> that would be absolutely awesome. It would even allow switching over
> to ATF...

Supposedly there are GPL tarballs containing an u-boot that is suitable 
for the 4908, however I have no idea where to find those. In premise you 
could chainload those to CFE and a get seemingly functional system.
-- 
Florian

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

* Re: [PATCH 5/5] arm64: Document the requirement for SCR_EL3.HCE
  2021-08-12 19:02 ` [PATCH 5/5] arm64: Document the requirement for SCR_EL3.HCE Marc Zyngier
@ 2021-08-24 10:49   ` Catalin Marinas
  2021-08-24 10:52     ` Mark Rutland
  0 siblings, 1 reply; 20+ messages in thread
From: Catalin Marinas @ 2021-08-24 10:49 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: linux-arm-kernel, linux-kernel, Rafał Miłecki,
	Will Deacon, Mark Rutland, Ard Biesheuvel, Florian Fainelli,
	bcm-kernel-feedback-list, kernel-team

On Thu, Aug 12, 2021 at 08:02:13PM +0100, Marc Zyngier wrote:
> It is amazing that we never documented this absolutely basic
> requirement: if you boot the kernel at EL2, you'd better
> enable the HVC instruction from EL3.
> 
> Really, just do it.
> 
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
>  Documentation/arm64/booting.rst | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/Documentation/arm64/booting.rst b/Documentation/arm64/booting.rst
> index a9192e7a231b..6c729d0c4bc2 100644
> --- a/Documentation/arm64/booting.rst
> +++ b/Documentation/arm64/booting.rst
> @@ -212,6 +212,11 @@ Before jumping into the kernel, the following conditions must be met:
>    - The value of SCR_EL3.FIQ must be the same as the one present at boot
>      time whenever the kernel is executing.
>  
> +  For all systems:
> +  - If EL3 is present and the kernel is entered at EL2:
> +
> +    - SCR_EL3.HCE (bit 8) must be initialised to 0b1.
> +
>    For systems with a GICv3 interrupt controller to be used in v3 mode:
>    - If EL3 is present:

I'll queue this patch only for now.

A nitpick, I think we should move "For all systems" and "If EL3 is
present..." above the lines describing the SCR_EL3.FIQ requirement (I
can make the change locally).

-- 
Catalin

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

* Re: [PATCH 5/5] arm64: Document the requirement for SCR_EL3.HCE
  2021-08-24 10:49   ` Catalin Marinas
@ 2021-08-24 10:52     ` Mark Rutland
  0 siblings, 0 replies; 20+ messages in thread
From: Mark Rutland @ 2021-08-24 10:52 UTC (permalink / raw)
  To: Catalin Marinas
  Cc: Marc Zyngier, linux-arm-kernel, linux-kernel,
	Rafał Miłecki, Will Deacon, Ard Biesheuvel,
	Florian Fainelli, bcm-kernel-feedback-list, kernel-team

On Tue, Aug 24, 2021 at 11:49:01AM +0100, Catalin Marinas wrote:
> On Thu, Aug 12, 2021 at 08:02:13PM +0100, Marc Zyngier wrote:
> > It is amazing that we never documented this absolutely basic
> > requirement: if you boot the kernel at EL2, you'd better
> > enable the HVC instruction from EL3.
> > 
> > Really, just do it.
> > 
> > Signed-off-by: Marc Zyngier <maz@kernel.org>
> > ---
> >  Documentation/arm64/booting.rst | 5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/Documentation/arm64/booting.rst b/Documentation/arm64/booting.rst
> > index a9192e7a231b..6c729d0c4bc2 100644
> > --- a/Documentation/arm64/booting.rst
> > +++ b/Documentation/arm64/booting.rst
> > @@ -212,6 +212,11 @@ Before jumping into the kernel, the following conditions must be met:
> >    - The value of SCR_EL3.FIQ must be the same as the one present at boot
> >      time whenever the kernel is executing.
> >  
> > +  For all systems:
> > +  - If EL3 is present and the kernel is entered at EL2:
> > +
> > +    - SCR_EL3.HCE (bit 8) must be initialised to 0b1.
> > +
> >    For systems with a GICv3 interrupt controller to be used in v3 mode:
> >    - If EL3 is present:
> 
> I'll queue this patch only for now.
> 
> A nitpick, I think we should move "For all systems" and "If EL3 is
> present..." above the lines describing the SCR_EL3.FIQ requirement (I
> can make the change locally).


FWIW, with that:

Acked-by: Mark Rutland <mark.rutland@arm.com>

Mark.

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

* Re: (subset) [PATCH 0/5] arm64: Survival kit for SCR_EL3.HCE==0 conditions
  2021-08-12 19:02 [PATCH 0/5] arm64: Survival kit for SCR_EL3.HCE==0 conditions Marc Zyngier
                   ` (5 preceding siblings ...)
  2021-08-15  7:28 ` [PATCH 0/5] arm64: Survival kit for SCR_EL3.HCE==0 conditions Florian Fainelli
@ 2021-08-24 16:19 ` Catalin Marinas
  6 siblings, 0 replies; 20+ messages in thread
From: Catalin Marinas @ 2021-08-24 16:19 UTC (permalink / raw)
  To: linux-kernel, linux-arm-kernel, Marc Zyngier
  Cc: Will Deacon, Ard Biesheuvel, Rafał Miłecki,
	Mark Rutland, kernel-team, bcm-kernel-feedback-list,
	Florian Fainelli

On Thu, 12 Aug 2021 20:02:08 +0100, Marc Zyngier wrote:
> Anyone vaguely familiar with the ARMv8 architecture would quickly
> understand that entering the kernel at EL2 without enabling the HVC
> instruction is... living dangerously. But as it turns out [0], there
> is a whole range of (*cough*) "high quality" (*cough*) Broadcom
> systems out there configured exactly like that.
> 
> If you are speechless, I'm right with you.
> 
> [...]

Applied to arm64 (for-next/misc), thanks!

[5/5] arm64: Document the requirement for SCR_EL3.HCE
      https://git.kernel.org/arm64/c/e3849765037b

-- 
Catalin


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

end of thread, other threads:[~2021-08-24 16:19 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-12 19:02 [PATCH 0/5] arm64: Survival kit for SCR_EL3.HCE==0 conditions Marc Zyngier
2021-08-12 19:02 ` [PATCH 1/5] arm64: Directly expand __init_el2_nvhe_prepare_eret where needed Marc Zyngier
2021-08-12 19:02 ` [PATCH 2/5] arm64: Handle UNDEF in the EL2 stub vectors Marc Zyngier
2021-08-13 13:08   ` Robin Murphy
2021-08-13 17:41     ` Marc Zyngier
2021-08-13 18:17       ` Robin Murphy
2021-08-14  9:38         ` Marc Zyngier
2021-08-12 19:02 ` [PATCH 3/5] arm64: Detect disabled HVC early Marc Zyngier
2021-08-12 19:47   ` Rafał Miłecki
2021-08-13  9:05   ` Will Deacon
2021-08-13 17:33     ` Marc Zyngier
2021-08-12 19:02 ` [PATCH 4/5] arm64: Warn on booting at EL2 with HVC disabled Marc Zyngier
2021-08-12 19:58   ` Rafał Miłecki
2021-08-12 19:02 ` [PATCH 5/5] arm64: Document the requirement for SCR_EL3.HCE Marc Zyngier
2021-08-24 10:49   ` Catalin Marinas
2021-08-24 10:52     ` Mark Rutland
2021-08-15  7:28 ` [PATCH 0/5] arm64: Survival kit for SCR_EL3.HCE==0 conditions Florian Fainelli
2021-08-15  9:27   ` Marc Zyngier
2021-08-22 11:31     ` Florian Fainelli
2021-08-24 16:19 ` (subset) " Catalin Marinas

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).