* [PATCH v4 0/5] initialize SCTRL2_ELx
@ 2025-08-21 17:24 Yeoreum Yun
2025-08-21 17:24 ` [PATCH v4 1/5] arm64: make SCTLR2_EL1 accessible Yeoreum Yun
` (6 more replies)
0 siblings, 7 replies; 19+ messages in thread
From: Yeoreum Yun @ 2025-08-21 17:24 UTC (permalink / raw)
To: catalin.marinas, will, broonie, oliver.upton, anshuman.khandual,
robh, james.morse, mark.rutland, joey.gouly, Dave.Martin,
ahmed.genidi, kevin.brodsky, scott, mbenes, james.clark, frederic,
rafael, pavel, ryan.roberts, suzuki.poulose, maz
Cc: linux-arm-kernel, linux-kernel, linux-pm, kvmarm, Yeoreum Yun
This series introduces initial support for the SCTLR2_ELx registers in Linux.
The feature is optional starting from ARMv8.8/ARMv9.3,
and becomes mandatory from ARMv8.9/ARMv9.4.
Currently, Linux has no strict need to modify SCTLR2_ELx--
at least assuming that firmware initializes
these registers to reasonable defaults.
However, several upcoming architectural features will require configuring
control bits in these registers.
Notable examples include FEAT_PAuth_LR and FEAT_CPA2.
Patch History
==============
from v3 to v4:
- integrate set_sctlr2_elx() and __set_sctlr2_elx() to set_sctlr2_elx()
without isb()
- fix the wrong register setting in set_sctlr2_elx().
- add initialise SCTLR2_EL2 at HVC_SOFT_RESTART.
- https://lore.kernel.org/all/20250813120118.3953541-1-yeoreum.yun@arm.com/
from v2 to v3:
- rewrite commit messages.
- fix missing SCTLR2_EL2 synchonization at boot.
- merging the __kvm_host_psci_cpu_entry() changes into patch #1
- https://lore.kernel.org/all/20250811163340.1561893-1-yeoreum.yun@arm.com/
from v1 to v2:
- rebase to v6.17-rc1
- https://lore.kernel.org/all/20250804121724.3681531-1-yeoreum.yun@arm.com/
Yeoreum Yun (5):
arm64: make SCTLR2_EL1 accessible
arm64: initialise SCTLR2_ELx register at boot time
arm64: save/restore SCTLR2_EL1 when cpu_suspend()/resume()
arm64: initialise SCTLR2_EL1 at cpu_soft_restart()
arm64: make the per-task SCTLR2_EL1
arch/arm64/include/asm/assembler.h | 15 +++++++++++++++
arch/arm64/include/asm/el2_setup.h | 17 +++++++++++++++--
arch/arm64/include/asm/processor.h | 3 +++
arch/arm64/include/asm/suspend.h | 2 +-
arch/arm64/include/asm/sysreg.h | 5 +++++
arch/arm64/kernel/cpu-reset.S | 4 ++++
arch/arm64/kernel/head.S | 5 +++++
arch/arm64/kernel/hyp-stub.S | 10 ++++++++++
arch/arm64/kernel/process.c | 9 +++++++++
arch/arm64/kvm/hyp/nvhe/hyp-init.S | 3 +++
arch/arm64/kvm/hyp/nvhe/psci-relay.c | 3 +++
arch/arm64/mm/proc.S | 24 ++++++++++++++++--------
12 files changed, 89 insertions(+), 11 deletions(-)
base-commit: 8f5ae30d69d7543eee0d70083daf4de8fe15d585
--
LEVI:{C3F47F37-75D8-414A-A8BA-3980EC8A46D7}
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v4 1/5] arm64: make SCTLR2_EL1 accessible
2025-08-21 17:24 [PATCH v4 0/5] initialize SCTRL2_ELx Yeoreum Yun
@ 2025-08-21 17:24 ` Yeoreum Yun
2025-08-21 17:24 ` [PATCH v4 2/5] arm64: initialise SCTLR2_ELx register at boot time Yeoreum Yun
` (5 subsequent siblings)
6 siblings, 0 replies; 19+ messages in thread
From: Yeoreum Yun @ 2025-08-21 17:24 UTC (permalink / raw)
To: catalin.marinas, will, broonie, oliver.upton, anshuman.khandual,
robh, james.morse, mark.rutland, joey.gouly, Dave.Martin,
ahmed.genidi, kevin.brodsky, scott, mbenes, james.clark, frederic,
rafael, pavel, ryan.roberts, suzuki.poulose, maz
Cc: linux-arm-kernel, linux-kernel, linux-pm, kvmarm, Yeoreum Yun
When the kernel runs at EL1, and yet is booted at EL2,
HCRX_EL2.SCTLR2En must be set to avoid trapping SCTLR2_EL1 accesses
from EL1 to EL2.
Ensure this bit is set at the point of initialising EL2.
Signed-off-by: Yeoreum Yun <yeoreum.yun@arm.com>
Reviewed-by: Marc Zyngier <maz@kernel.org>
---
arch/arm64/include/asm/el2_setup.h | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/include/asm/el2_setup.h b/arch/arm64/include/asm/el2_setup.h
index 46033027510c..d9529dfc4783 100644
--- a/arch/arm64/include/asm/el2_setup.h
+++ b/arch/arm64/include/asm/el2_setup.h
@@ -57,10 +57,16 @@
/* Enable GCS if supported */
mrs_s x1, SYS_ID_AA64PFR1_EL1
ubfx x1, x1, #ID_AA64PFR1_EL1_GCS_SHIFT, #4
- cbz x1, .Lset_hcrx_\@
+ cbz x1, .Lskip_hcrx_GCSEn_\@
orr x0, x0, #HCRX_EL2_GCSEn
-.Lset_hcrx_\@:
+.Lskip_hcrx_GCSEn_\@:
+ mrs_s x1, SYS_ID_AA64MMFR3_EL1
+ ubfx x1, x1, #ID_AA64MMFR3_EL1_SCTLRX_SHIFT, #4
+ cbz x1, .Lskip_hcrx_SCTLR2En\@
+ orr x0, x0, #HCRX_EL2_SCTLR2En
+
+.Lskip_hcrx_SCTLR2En\@:
msr_s SYS_HCRX_EL2, x0
.Lskip_hcrx_\@:
.endm
--
LEVI:{C3F47F37-75D8-414A-A8BA-3980EC8A46D7}
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v4 2/5] arm64: initialise SCTLR2_ELx register at boot time
2025-08-21 17:24 [PATCH v4 0/5] initialize SCTRL2_ELx Yeoreum Yun
2025-08-21 17:24 ` [PATCH v4 1/5] arm64: make SCTLR2_EL1 accessible Yeoreum Yun
@ 2025-08-21 17:24 ` Yeoreum Yun
2025-09-01 15:13 ` Dave Martin
2025-08-21 17:24 ` [PATCH v4 3/5] arm64: save/restore SCTLR2_EL1 when cpu_suspend()/resume() Yeoreum Yun
` (4 subsequent siblings)
6 siblings, 1 reply; 19+ messages in thread
From: Yeoreum Yun @ 2025-08-21 17:24 UTC (permalink / raw)
To: catalin.marinas, will, broonie, oliver.upton, anshuman.khandual,
robh, james.morse, mark.rutland, joey.gouly, Dave.Martin,
ahmed.genidi, kevin.brodsky, scott, mbenes, james.clark, frederic,
rafael, pavel, ryan.roberts, suzuki.poulose, maz
Cc: linux-arm-kernel, linux-kernel, linux-pm, kvmarm, Yeoreum Yun
The value of the SCTLR2_ELx register is UNKNOWN after reset.
If the firmware initializes these registers properly, no additional
initialization is required.
However, in cases where they are not initialized correctly,
initialize the SCTLR2_ELx registers during CPU/vCPU boot
to prevent unexpected system behavior caused by invalid values.
Signed-off-by: Yeoreum Yun <yeoreum.yun@arm.com>
---
arch/arm64/include/asm/assembler.h | 15 +++++++++++++++
arch/arm64/include/asm/el2_setup.h | 7 +++++++
arch/arm64/include/asm/sysreg.h | 5 +++++
arch/arm64/kernel/head.S | 5 +++++
arch/arm64/kernel/hyp-stub.S | 10 ++++++++++
arch/arm64/kvm/hyp/nvhe/psci-relay.c | 3 +++
6 files changed, 45 insertions(+)
diff --git a/arch/arm64/include/asm/assembler.h b/arch/arm64/include/asm/assembler.h
index 23be85d93348..c25c2aed5125 100644
--- a/arch/arm64/include/asm/assembler.h
+++ b/arch/arm64/include/asm/assembler.h
@@ -738,6 +738,21 @@ alternative_endif
set_sctlr sctlr_el2, \reg
.endm
+/* Set SCTLR2_ELx to the @reg value. */
+.macro set_sctlr2_elx, el, reg, tmp
+ mrs_s \tmp, SYS_ID_AA64MMFR3_EL1
+ ubfx \tmp, \tmp, #ID_AA64MMFR3_EL1_SCTLRX_SHIFT, #4
+ cbz \tmp, .Lskip_sctlr2_\@
+ .if \el == 2
+ msr_s SYS_SCTLR2_EL2, \reg
+ .elseif \el == 12
+ msr_s SYS_SCTLR2_EL12, \reg
+ .else
+ msr_s SYS_SCTLR2_EL1, \reg
+ .endif
+.Lskip_sctlr2_\@:
+.endm
+
/*
* Check whether asm code should yield as soon as it is able. This is
* the case if we are currently running in task context, and the
diff --git a/arch/arm64/include/asm/el2_setup.h b/arch/arm64/include/asm/el2_setup.h
index d9529dfc4783..2addf7c096fc 100644
--- a/arch/arm64/include/asm/el2_setup.h
+++ b/arch/arm64/include/asm/el2_setup.h
@@ -48,6 +48,12 @@
isb
.endm
+.macro __init_sctlr2_el2
+ mov_q x0, INIT_SCTLR2_EL2
+ set_sctlr2_elx 2, x0, x1
+ isb
+.endm
+
.macro __init_el2_hcrx
mrs x0, id_aa64mmfr1_el1
ubfx x0, x0, #ID_AA64MMFR1_EL1_HCX_SHIFT, #4
@@ -411,6 +417,7 @@
*/
.macro init_el2_state
__init_el2_sctlr
+ __init_sctlr2_el2
__init_el2_hcrx
__init_el2_timers
__init_el2_debug
diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
index d5b5f2ae1afa..0431b357b87b 100644
--- a/arch/arm64/include/asm/sysreg.h
+++ b/arch/arm64/include/asm/sysreg.h
@@ -868,6 +868,8 @@
#define INIT_SCTLR_EL2_MMU_OFF \
(SCTLR_EL2_RES1 | ENDIAN_SET_EL2)
+#define INIT_SCTLR2_EL2 UL(0)
+
/* SCTLR_EL1 specific flags. */
#ifdef CONFIG_CPU_BIG_ENDIAN
#define ENDIAN_SET_EL1 (SCTLR_EL1_E0E | SCTLR_ELx_EE)
@@ -888,6 +890,8 @@
SCTLR_EL1_LSMAOE | SCTLR_EL1_nTLSMD | SCTLR_EL1_EIS | \
SCTLR_EL1_TSCXT | SCTLR_EL1_EOS)
+#define INIT_SCTLR2_EL1 UL(0)
+
/* MAIR_ELx memory attributes (used by Linux) */
#define MAIR_ATTR_DEVICE_nGnRnE UL(0x00)
#define MAIR_ATTR_DEVICE_nGnRE UL(0x04)
@@ -1164,6 +1168,7 @@
msr hcr_el2, \reg
#endif
.endm
+
#else
#include <linux/bitfield.h>
diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
index ca04b338cb0d..e42664246e15 100644
--- a/arch/arm64/kernel/head.S
+++ b/arch/arm64/kernel/head.S
@@ -276,6 +276,8 @@ SYM_INNER_LABEL(init_el1, SYM_L_LOCAL)
mov_q x0, INIT_SCTLR_EL1_MMU_OFF
pre_disable_mmu_workaround
msr sctlr_el1, x0
+ mov_q x0, INIT_SCTLR2_EL1
+ set_sctlr2_elx 1, x0, x1
isb
mov_q x0, INIT_PSTATE_EL1
msr spsr_el1, x0
@@ -308,6 +310,7 @@ SYM_INNER_LABEL(init_el2, SYM_L_LOCAL)
isb
mov_q x1, INIT_SCTLR_EL1_MMU_OFF
+ mov_q x2, INIT_SCTLR2_EL1
mrs x0, hcr_el2
and x0, x0, #HCR_E2H
@@ -315,11 +318,13 @@ SYM_INNER_LABEL(init_el2, SYM_L_LOCAL)
/* Set a sane SCTLR_EL1, the VHE way */
msr_s SYS_SCTLR_EL12, x1
+ set_sctlr2_elx 12, x2, x0
mov x2, #BOOT_CPU_FLAG_E2H
b 3f
2:
msr sctlr_el1, x1
+ set_sctlr2_elx 1, x2, x0
mov x2, xzr
3:
mov x0, #INIT_PSTATE_EL1
diff --git a/arch/arm64/kernel/hyp-stub.S b/arch/arm64/kernel/hyp-stub.S
index 36e2d26b54f5..ac12f1b4f8e2 100644
--- a/arch/arm64/kernel/hyp-stub.S
+++ b/arch/arm64/kernel/hyp-stub.S
@@ -144,7 +144,17 @@ SYM_CODE_START_LOCAL(__finalise_el2)
.Lskip_indirection:
.Lskip_tcr2:
+ mrs_s x1, SYS_ID_AA64MMFR3_EL1
+ ubfx x1, x1, #ID_AA64MMFR3_EL1_SCTLRX_SHIFT, #4
+ cbz x1, .Lskip_sctlr2
+ mrs_s x1, SYS_SCTLR2_EL12
+ msr_s SYS_SCTLR2_EL1, x1
+ // clean SCTLR2_EL1
+ mov_q x1, INIT_SCTLR2_EL1
+ msr_s SYS_SCTLR2_EL12, x1
+
+.Lskip_sctlr2:
isb
// Hack the exception return to stay at EL2
diff --git a/arch/arm64/kvm/hyp/nvhe/psci-relay.c b/arch/arm64/kvm/hyp/nvhe/psci-relay.c
index c3e196fb8b18..df1180cad7f8 100644
--- a/arch/arm64/kvm/hyp/nvhe/psci-relay.c
+++ b/arch/arm64/kvm/hyp/nvhe/psci-relay.c
@@ -4,6 +4,7 @@
* Author: David Brazdil <dbrazdil@google.com>
*/
+#include <asm/cpufeature.h>
#include <asm/kvm_asm.h>
#include <asm/kvm_hyp.h>
#include <asm/kvm_mmu.h>
@@ -219,6 +220,8 @@ asmlinkage void __noreturn __kvm_host_psci_cpu_entry(bool is_cpu_on)
release_boot_args(boot_args);
write_sysreg_el1(INIT_SCTLR_EL1_MMU_OFF, SYS_SCTLR);
+ if (cpus_have_final_cap(ARM64_HAS_SCTLR2))
+ write_sysreg_el1(INIT_SCTLR2_EL1, SYS_SCTLR2);
write_sysreg(INIT_PSTATE_EL1, SPSR_EL2);
__host_enter(host_ctxt);
--
LEVI:{C3F47F37-75D8-414A-A8BA-3980EC8A46D7}
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v4 3/5] arm64: save/restore SCTLR2_EL1 when cpu_suspend()/resume()
2025-08-21 17:24 [PATCH v4 0/5] initialize SCTRL2_ELx Yeoreum Yun
2025-08-21 17:24 ` [PATCH v4 1/5] arm64: make SCTLR2_EL1 accessible Yeoreum Yun
2025-08-21 17:24 ` [PATCH v4 2/5] arm64: initialise SCTLR2_ELx register at boot time Yeoreum Yun
@ 2025-08-21 17:24 ` Yeoreum Yun
2025-08-21 17:24 ` [PATCH v4 4/5] arm64: initialise SCTLR2_EL1 at cpu_soft_restart() Yeoreum Yun
` (3 subsequent siblings)
6 siblings, 0 replies; 19+ messages in thread
From: Yeoreum Yun @ 2025-08-21 17:24 UTC (permalink / raw)
To: catalin.marinas, will, broonie, oliver.upton, anshuman.khandual,
robh, james.morse, mark.rutland, joey.gouly, Dave.Martin,
ahmed.genidi, kevin.brodsky, scott, mbenes, james.clark, frederic,
rafael, pavel, ryan.roberts, suzuki.poulose, maz
Cc: linux-arm-kernel, linux-kernel, linux-pm, kvmarm, Yeoreum Yun
Save and restore the SCTLR2_EL1 value during cpu_suspend()/resume(),
ensuring that the configured value remains
consistent across suspend and resume.
Signed-off-by: Yeoreum Yun <yeoreum.yun@arm.com>
---
arch/arm64/include/asm/suspend.h | 2 +-
arch/arm64/mm/proc.S | 24 ++++++++++++++++--------
2 files changed, 17 insertions(+), 9 deletions(-)
diff --git a/arch/arm64/include/asm/suspend.h b/arch/arm64/include/asm/suspend.h
index 0cde2f473971..eb60c9735553 100644
--- a/arch/arm64/include/asm/suspend.h
+++ b/arch/arm64/include/asm/suspend.h
@@ -2,7 +2,7 @@
#ifndef __ASM_SUSPEND_H
#define __ASM_SUSPEND_H
-#define NR_CTX_REGS 13
+#define NR_CTX_REGS 14
#define NR_CALLEE_SAVED_REGS 12
/*
diff --git a/arch/arm64/mm/proc.S b/arch/arm64/mm/proc.S
index 8c75965afc9e..a330d828270f 100644
--- a/arch/arm64/mm/proc.S
+++ b/arch/arm64/mm/proc.S
@@ -87,8 +87,12 @@ SYM_FUNC_START(cpu_do_suspend)
mrs x9, mdscr_el1
mrs x10, oslsr_el1
mrs x11, sctlr_el1
- get_this_cpu_offset x12
- mrs x13, sp_el0
+alternative_if ARM64_HAS_SCTLR2
+ mrs_s x12, SYS_SCTLR2_EL1
+alternative_else_nop_endif
+ get_this_cpu_offset x13
+ mrs x14, sp_el0
+
stp x2, x3, [x0]
stp x4, x5, [x0, #16]
stp x6, x7, [x0, #32]
@@ -99,7 +103,7 @@ SYM_FUNC_START(cpu_do_suspend)
* Save x18 as it may be used as a platform register, e.g. by shadow
* call stack.
*/
- str x18, [x0, #96]
+ stp x14, x18, [x0, #96]
ret
SYM_FUNC_END(cpu_do_suspend)
@@ -120,8 +124,8 @@ SYM_FUNC_START(cpu_do_resume)
* the buffer to minimize the risk of exposure when used for shadow
* call stack.
*/
- ldr x18, [x0, #96]
- str xzr, [x0, #96]
+ ldp x15, x18, [x0, #96]
+ str xzr, [x0, #104]
msr tpidr_el0, x2
msr tpidrro_el0, x3
msr contextidr_el1, x4
@@ -136,8 +140,12 @@ SYM_FUNC_START(cpu_do_resume)
msr mdscr_el1, x10
msr sctlr_el1, x12
- set_this_cpu_offset x13
- msr sp_el0, x14
+alternative_if ARM64_HAS_SCTLR2
+ msr_s SYS_SCTLR2_EL1, x13
+alternative_else_nop_endif
+
+ set_this_cpu_offset x14
+ msr sp_el0, x15
/*
* Restore oslsr_el1 by writing oslar_el1
*/
@@ -151,7 +159,7 @@ alternative_if ARM64_HAS_RAS_EXTN
msr_s SYS_DISR_EL1, xzr
alternative_else_nop_endif
- ptrauth_keys_install_kernel_nosync x14, x1, x2, x3
+ ptrauth_keys_install_kernel_nosync x15, x1, x2, x3
isb
ret
SYM_FUNC_END(cpu_do_resume)
--
LEVI:{C3F47F37-75D8-414A-A8BA-3980EC8A46D7}
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v4 4/5] arm64: initialise SCTLR2_EL1 at cpu_soft_restart()
2025-08-21 17:24 [PATCH v4 0/5] initialize SCTRL2_ELx Yeoreum Yun
` (2 preceding siblings ...)
2025-08-21 17:24 ` [PATCH v4 3/5] arm64: save/restore SCTLR2_EL1 when cpu_suspend()/resume() Yeoreum Yun
@ 2025-08-21 17:24 ` Yeoreum Yun
2025-09-01 15:13 ` Dave Martin
2025-08-21 17:24 ` [PATCH v4 5/5] arm64: make the per-task SCTLR2_EL1 Yeoreum Yun
` (2 subsequent siblings)
6 siblings, 1 reply; 19+ messages in thread
From: Yeoreum Yun @ 2025-08-21 17:24 UTC (permalink / raw)
To: catalin.marinas, will, broonie, oliver.upton, anshuman.khandual,
robh, james.morse, mark.rutland, joey.gouly, Dave.Martin,
ahmed.genidi, kevin.brodsky, scott, mbenes, james.clark, frederic,
rafael, pavel, ryan.roberts, suzuki.poulose, maz
Cc: linux-arm-kernel, linux-kernel, linux-pm, kvmarm, Yeoreum Yun
Explicitly initialize the SCTLR2_ELx register before launching
a new kernel via kexec() to avoid leaving SCTLR2_ELx with an
arbitrary value when the new kernel runs.
Signed-off-by: Yeoreum Yun <yeoreum.yun@arm.com>
---
arch/arm64/kernel/cpu-reset.S | 4 ++++
arch/arm64/kvm/hyp/nvhe/hyp-init.S | 3 +++
2 files changed, 7 insertions(+)
diff --git a/arch/arm64/kernel/cpu-reset.S b/arch/arm64/kernel/cpu-reset.S
index c87445dde674..c8888891dc8d 100644
--- a/arch/arm64/kernel/cpu-reset.S
+++ b/arch/arm64/kernel/cpu-reset.S
@@ -37,6 +37,10 @@ SYM_TYPED_FUNC_START(cpu_soft_restart)
* regime if HCR_EL2.E2H == 1
*/
msr sctlr_el1, x12
+
+ mov_q x12, INIT_SCTLR2_EL1
+ set_sctlr2_elx 1, x12, x8
+
isb
cbz x0, 1f // el2_switch?
diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-init.S b/arch/arm64/kvm/hyp/nvhe/hyp-init.S
index aada42522e7b..cc569656fe35 100644
--- a/arch/arm64/kvm/hyp/nvhe/hyp-init.S
+++ b/arch/arm64/kvm/hyp/nvhe/hyp-init.S
@@ -255,6 +255,9 @@ SYM_CODE_START(__kvm_handle_stub_hvc)
mov x0, xzr
reset:
/* Reset kvm back to the hyp stub. */
+ mov_q x5, INIT_SCTLR2_EL2
+ set_sctlr2_elx 2, x5, x4
+
mov_q x5, INIT_SCTLR_EL2_MMU_OFF
pre_disable_mmu_workaround
msr sctlr_el2, x5
--
LEVI:{C3F47F37-75D8-414A-A8BA-3980EC8A46D7}
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v4 5/5] arm64: make the per-task SCTLR2_EL1
2025-08-21 17:24 [PATCH v4 0/5] initialize SCTRL2_ELx Yeoreum Yun
` (3 preceding siblings ...)
2025-08-21 17:24 ` [PATCH v4 4/5] arm64: initialise SCTLR2_EL1 at cpu_soft_restart() Yeoreum Yun
@ 2025-08-21 17:24 ` Yeoreum Yun
2025-09-01 10:08 ` [PATCH v4 0/5] initialize SCTRL2_ELx Yeoreum Yun
2025-09-01 15:18 ` Dave Martin
6 siblings, 0 replies; 19+ messages in thread
From: Yeoreum Yun @ 2025-08-21 17:24 UTC (permalink / raw)
To: catalin.marinas, will, broonie, oliver.upton, anshuman.khandual,
robh, james.morse, mark.rutland, joey.gouly, Dave.Martin,
ahmed.genidi, kevin.brodsky, scott, mbenes, james.clark, frederic,
rafael, pavel, ryan.roberts, suzuki.poulose, maz
Cc: linux-arm-kernel, linux-kernel, linux-pm, kvmarm, Yeoreum Yun
Some bits in SCTLR2_EL1 that control system behavior can be
configured on a per-task basis (e.g., fields related to FEAT_CPA2).
To support future use of these fields, SCTLR2_EL1 is maintained
per task.
On platforms without FEAT_SCTLR2 support, there is no functional
change and only minimal performance overhead.
Signed-off-by: Yeoreum Yun <yeoreum.yun@arm.com>
---
arch/arm64/include/asm/processor.h | 3 +++
arch/arm64/kernel/process.c | 9 +++++++++
2 files changed, 12 insertions(+)
diff --git a/arch/arm64/include/asm/processor.h b/arch/arm64/include/asm/processor.h
index 61d62bfd5a7b..e066116735c6 100644
--- a/arch/arm64/include/asm/processor.h
+++ b/arch/arm64/include/asm/processor.h
@@ -184,6 +184,7 @@ struct thread_struct {
u64 mte_ctrl;
#endif
u64 sctlr_user;
+ u64 sctlr2_user;
u64 svcr;
u64 tpidr2_el0;
u64 por_el0;
@@ -258,6 +259,8 @@ static inline void task_set_sve_vl_onexec(struct task_struct *task,
(SCTLR_ELx_ENIA | SCTLR_ELx_ENIB | SCTLR_ELx_ENDA | SCTLR_ELx_ENDB | \
SCTLR_EL1_TCF0_MASK)
+#define SCTLR2_USER_MASK (0)
+
static inline void arch_thread_struct_whitelist(unsigned long *offset,
unsigned long *size)
{
diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
index 96482a1412c6..e54f192c0629 100644
--- a/arch/arm64/kernel/process.c
+++ b/arch/arm64/kernel/process.c
@@ -698,6 +698,11 @@ void update_sctlr_el1(u64 sctlr)
isb();
}
+static void update_sctlr2_el1(u64 sctlr2)
+{
+ sysreg_clear_set_s(SYS_SCTLR2_EL1, SCTLR2_USER_MASK, sctlr2);
+}
+
/*
* Thread switching.
*/
@@ -737,6 +742,10 @@ struct task_struct *__switch_to(struct task_struct *prev,
if (prev->thread.sctlr_user != next->thread.sctlr_user)
update_sctlr_el1(next->thread.sctlr_user);
+ if (alternative_has_cap_unlikely(ARM64_HAS_SCTLR2) &&
+ prev->thread.sctlr2_user != next->thread.sctlr2_user)
+ update_sctlr2_el1(next->thread.sctlr2_user);
+
/* the actual thread switch */
last = cpu_switch_to(prev, next);
--
LEVI:{C3F47F37-75D8-414A-A8BA-3980EC8A46D7}
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH v4 0/5] initialize SCTRL2_ELx
2025-08-21 17:24 [PATCH v4 0/5] initialize SCTRL2_ELx Yeoreum Yun
` (4 preceding siblings ...)
2025-08-21 17:24 ` [PATCH v4 5/5] arm64: make the per-task SCTLR2_EL1 Yeoreum Yun
@ 2025-09-01 10:08 ` Yeoreum Yun
2025-09-01 15:18 ` Dave Martin
6 siblings, 0 replies; 19+ messages in thread
From: Yeoreum Yun @ 2025-09-01 10:08 UTC (permalink / raw)
To: catalin.marinas, will, broonie, oliver.upton, anshuman.khandual,
robh, james.morse, mark.rutland, joey.gouly, Dave.Martin,
ahmed.genidi, kevin.brodsky, scott, mbenes, james.clark, frederic,
rafael, pavel, ryan.roberts, suzuki.poulose, maz
Cc: linux-arm-kernel, linux-kernel, linux-pm, kvmarm
Gentle ping in case of forgotten.
On Thu, Aug 21, 2025 at 06:24:03PM +0100, Yeoreum Yun wrote:
> This series introduces initial support for the SCTLR2_ELx registers in Linux.
> The feature is optional starting from ARMv8.8/ARMv9.3,
> and becomes mandatory from ARMv8.9/ARMv9.4.
>
> Currently, Linux has no strict need to modify SCTLR2_ELx--
> at least assuming that firmware initializes
> these registers to reasonable defaults.
>
> However, several upcoming architectural features will require configuring
> control bits in these registers.
> Notable examples include FEAT_PAuth_LR and FEAT_CPA2.
>
> Patch History
> ==============
> from v3 to v4:
> - integrate set_sctlr2_elx() and __set_sctlr2_elx() to set_sctlr2_elx()
> without isb()
> - fix the wrong register setting in set_sctlr2_elx().
> - add initialise SCTLR2_EL2 at HVC_SOFT_RESTART.
> - https://lore.kernel.org/all/20250813120118.3953541-1-yeoreum.yun@arm.com/
>
> from v2 to v3:
> - rewrite commit messages.
> - fix missing SCTLR2_EL2 synchonization at boot.
> - merging the __kvm_host_psci_cpu_entry() changes into patch #1
> - https://lore.kernel.org/all/20250811163340.1561893-1-yeoreum.yun@arm.com/
>
> from v1 to v2:
> - rebase to v6.17-rc1
> - https://lore.kernel.org/all/20250804121724.3681531-1-yeoreum.yun@arm.com/
>
> Yeoreum Yun (5):
> arm64: make SCTLR2_EL1 accessible
> arm64: initialise SCTLR2_ELx register at boot time
> arm64: save/restore SCTLR2_EL1 when cpu_suspend()/resume()
> arm64: initialise SCTLR2_EL1 at cpu_soft_restart()
> arm64: make the per-task SCTLR2_EL1
>
> arch/arm64/include/asm/assembler.h | 15 +++++++++++++++
> arch/arm64/include/asm/el2_setup.h | 17 +++++++++++++++--
> arch/arm64/include/asm/processor.h | 3 +++
> arch/arm64/include/asm/suspend.h | 2 +-
> arch/arm64/include/asm/sysreg.h | 5 +++++
> arch/arm64/kernel/cpu-reset.S | 4 ++++
> arch/arm64/kernel/head.S | 5 +++++
> arch/arm64/kernel/hyp-stub.S | 10 ++++++++++
> arch/arm64/kernel/process.c | 9 +++++++++
> arch/arm64/kvm/hyp/nvhe/hyp-init.S | 3 +++
> arch/arm64/kvm/hyp/nvhe/psci-relay.c | 3 +++
> arch/arm64/mm/proc.S | 24 ++++++++++++++++--------
> 12 files changed, 89 insertions(+), 11 deletions(-)
>
>
> base-commit: 8f5ae30d69d7543eee0d70083daf4de8fe15d585
> --
> LEVI:{C3F47F37-75D8-414A-A8BA-3980EC8A46D7}
>
--
Sincerely,
Yeoreum Yun
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v4 2/5] arm64: initialise SCTLR2_ELx register at boot time
2025-08-21 17:24 ` [PATCH v4 2/5] arm64: initialise SCTLR2_ELx register at boot time Yeoreum Yun
@ 2025-09-01 15:13 ` Dave Martin
2025-09-01 18:29 ` Yeoreum Yun
0 siblings, 1 reply; 19+ messages in thread
From: Dave Martin @ 2025-09-01 15:13 UTC (permalink / raw)
To: Yeoreum Yun
Cc: catalin.marinas, will, broonie, oliver.upton, anshuman.khandual,
robh, james.morse, mark.rutland, joey.gouly, ahmed.genidi,
kevin.brodsky, scott, mbenes, james.clark, frederic, rafael,
pavel, ryan.roberts, suzuki.poulose, maz, linux-arm-kernel,
linux-kernel, linux-pm, kvmarm
Hi,
On Thu, Aug 21, 2025 at 06:24:05PM +0100, Yeoreum Yun wrote:
> The value of the SCTLR2_ELx register is UNKNOWN after reset.
> If the firmware initializes these registers properly, no additional
> initialization is required.
> However, in cases where they are not initialized correctly,
> initialize the SCTLR2_ELx registers during CPU/vCPU boot
> to prevent unexpected system behavior caused by invalid values.
>
> Signed-off-by: Yeoreum Yun <yeoreum.yun@arm.com>
> ---
[...]
> diff --git a/arch/arm64/include/asm/assembler.h b/arch/arm64/include/asm/assembler.h
> index 23be85d93348..c25c2aed5125 100644
> --- a/arch/arm64/include/asm/assembler.h
> +++ b/arch/arm64/include/asm/assembler.h
> @@ -738,6 +738,21 @@ alternative_endif
> set_sctlr sctlr_el2, \reg
> .endm
>
> +/* Set SCTLR2_ELx to the @reg value. */
> +.macro set_sctlr2_elx, el, reg, tmp
> + mrs_s \tmp, SYS_ID_AA64MMFR3_EL1
> + ubfx \tmp, \tmp, #ID_AA64MMFR3_EL1_SCTLRX_SHIFT, #4
> + cbz \tmp, .Lskip_sctlr2_\@
> + .if \el == 2
> + msr_s SYS_SCTLR2_EL2, \reg
> + .elseif \el == 12
> + msr_s SYS_SCTLR2_EL12, \reg
> + .else
> + msr_s SYS_SCTLR2_EL1, \reg
> + .endif
> +.Lskip_sctlr2_\@:
> +.endm
> +
You were right that just doing
msr_s SYS_SCTLR_\el, \reg
here doesn't work. It looks like we rely on the C preprocessor to
expand the SYS_FOO_REG names to numeric sysreg encodings. By the time
the assembler macros are expanded, it is too late to construct sysreg
names -- the C preprocessor only runs once, before the assembler.
So, your code here looks reasonable.
But, it will still silently do the wrong thing if \el is empty or
garbage, so it is probably worth adding an error check:
.else
.error "Bad EL \"\el\" in set_sctlr2_elx"
.endif
Also, looking at all this again, the "1", "2" and "12" suffixes are not
really numbers: SYS_REG_EL02 would definitely not be the same thing as
SYS_REG_EL2 (although there is no _EL02 version of this particular
register).
So, can you use .ifc to do a string comparison instead?
If might be a good idea to migrate other macros that use an "el"
argument to do something similar -- although that probably doesn't
belong in this series.
The assembler seems to have no ".elseifc" directive, so you'll need
separate nested .ifc blocks:
.ifc \el,2
msr_s SYS_SCTLR2_EL2, \reg
.else
.ifc \el,12
msr_s SYS_SCTLR2_EL12, \reg
.else
.ifc \el,1
msr_s SYS_SCTLR2_EL1, \reg
.else
.error "Bad EL \"\el\" in set_sctlr2_elx"
.endif
.endif
.endif
(Note, I've not tested this approach. If you can think of a better
way, please feel free to suggest.)
[...]
> diff --git a/arch/arm64/kernel/hyp-stub.S b/arch/arm64/kernel/hyp-stub.S
> index 36e2d26b54f5..ac12f1b4f8e2 100644
> --- a/arch/arm64/kernel/hyp-stub.S
> +++ b/arch/arm64/kernel/hyp-stub.S
> @@ -144,7 +144,17 @@ SYM_CODE_START_LOCAL(__finalise_el2)
>
> .Lskip_indirection:
> .Lskip_tcr2:
> + mrs_s x1, SYS_ID_AA64MMFR3_EL1
> + ubfx x1, x1, #ID_AA64MMFR3_EL1_SCTLRX_SHIFT, #4
> + cbz x1, .Lskip_sctlr2
> + mrs_s x1, SYS_SCTLR2_EL12
> + msr_s SYS_SCTLR2_EL1, x1
>
> + // clean SCTLR2_EL1
> + mov_q x1, INIT_SCTLR2_EL1
> + msr_s SYS_SCTLR2_EL12, x1
I'm still not sure why we need to do this. The code doesn't seem to
clean up by the EL1 value of any other register -- or have I missed
something?
We have already switched to EL2, via the HVC call that jumped to
__finalise_el2. We won't run at EL1 again unless KVM starts a guest --
but in that case, it's KVM's responsibility to set up the EL1 registers
before handing control to the guest.
In any case, is SCTLR2_EL1 ever set to anything except INIT_SCTLR2_EL1
before we get here?
[...]
Cheers
---Dave
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v4 4/5] arm64: initialise SCTLR2_EL1 at cpu_soft_restart()
2025-08-21 17:24 ` [PATCH v4 4/5] arm64: initialise SCTLR2_EL1 at cpu_soft_restart() Yeoreum Yun
@ 2025-09-01 15:13 ` Dave Martin
2025-09-01 18:33 ` Yeoreum Yun
0 siblings, 1 reply; 19+ messages in thread
From: Dave Martin @ 2025-09-01 15:13 UTC (permalink / raw)
To: Yeoreum Yun
Cc: catalin.marinas, will, broonie, oliver.upton, anshuman.khandual,
robh, james.morse, mark.rutland, joey.gouly, ahmed.genidi,
kevin.brodsky, scott, mbenes, james.clark, frederic, rafael,
pavel, ryan.roberts, suzuki.poulose, maz, linux-arm-kernel,
linux-kernel, linux-pm, kvmarm
Hi,
On Thu, Aug 21, 2025 at 06:24:07PM +0100, Yeoreum Yun wrote:
> Explicitly initialize the SCTLR2_ELx register before launching
> a new kernel via kexec() to avoid leaving SCTLR2_ELx with an
> arbitrary value when the new kernel runs.
>
> Signed-off-by: Yeoreum Yun <yeoreum.yun@arm.com>
> ---
> arch/arm64/kernel/cpu-reset.S | 4 ++++
> arch/arm64/kvm/hyp/nvhe/hyp-init.S | 3 +++
> 2 files changed, 7 insertions(+)
>
> diff --git a/arch/arm64/kernel/cpu-reset.S b/arch/arm64/kernel/cpu-reset.S
> index c87445dde674..c8888891dc8d 100644
> --- a/arch/arm64/kernel/cpu-reset.S
> +++ b/arch/arm64/kernel/cpu-reset.S
> @@ -37,6 +37,10 @@ SYM_TYPED_FUNC_START(cpu_soft_restart)
> * regime if HCR_EL2.E2H == 1
> */
> msr sctlr_el1, x12
> +
> + mov_q x12, INIT_SCTLR2_EL1
> + set_sctlr2_elx 1, x12, x8
> +
Nit: does it matter whether we reset SCTLR2 before SCTLR?
I can't find a convincing architectural reason why they need to be
reset in a particular order, but it looks a bit strange that the
cpu_soft_restart and __kvm_handle_stub_hvc versions of this reset the
registers in the opposite order...
> isb
>
> cbz x0, 1f // el2_switch?
> diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-init.S b/arch/arm64/kvm/hyp/nvhe/hyp-init.S
> index aada42522e7b..cc569656fe35 100644
> --- a/arch/arm64/kvm/hyp/nvhe/hyp-init.S
> +++ b/arch/arm64/kvm/hyp/nvhe/hyp-init.S
> @@ -255,6 +255,9 @@ SYM_CODE_START(__kvm_handle_stub_hvc)
> mov x0, xzr
> reset:
> /* Reset kvm back to the hyp stub. */
> + mov_q x5, INIT_SCTLR2_EL2
> + set_sctlr2_elx 2, x5, x4
> +
> mov_q x5, INIT_SCTLR_EL2_MMU_OFF
> pre_disable_mmu_workaround
> msr sctlr_el2, x5
Otherwise, I guess this is fine.
Cheers
---Dave
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v4 0/5] initialize SCTRL2_ELx
2025-08-21 17:24 [PATCH v4 0/5] initialize SCTRL2_ELx Yeoreum Yun
` (5 preceding siblings ...)
2025-09-01 10:08 ` [PATCH v4 0/5] initialize SCTRL2_ELx Yeoreum Yun
@ 2025-09-01 15:18 ` Dave Martin
2025-09-01 18:17 ` Yeoreum Yun
6 siblings, 1 reply; 19+ messages in thread
From: Dave Martin @ 2025-09-01 15:18 UTC (permalink / raw)
To: Yeoreum Yun
Cc: catalin.marinas, will, broonie, oliver.upton, anshuman.khandual,
robh, james.morse, mark.rutland, joey.gouly, ahmed.genidi,
kevin.brodsky, scott, mbenes, james.clark, frederic, rafael,
pavel, ryan.roberts, suzuki.poulose, maz, linux-arm-kernel,
linux-kernel, linux-pm, kvmarm
Hi,
On Thu, Aug 21, 2025 at 06:24:03PM +0100, Yeoreum Yun wrote:
> This series introduces initial support for the SCTLR2_ELx registers in Linux.
> The feature is optional starting from ARMv8.8/ARMv9.3,
> and becomes mandatory from ARMv8.9/ARMv9.4.
>
> Currently, Linux has no strict need to modify SCTLR2_ELx--
> at least assuming that firmware initializes
> these registers to reasonable defaults.
>
> However, several upcoming architectural features will require configuring
> control bits in these registers.
> Notable examples include FEAT_PAuth_LR and FEAT_CPA2.
This looks OK to me now, except for one or two minor issues (see
replies to the patches).
I think we will need a way of testing all the code paths before this
should be merged, though.
Have you tested all the code paths, or are there some things that have
not been tested?
Since this code is not useful by itself, it may make sense to delay
merging it until we have patches for a feature that depends on SCTLR2.
Cheers
---Dave
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v4 0/5] initialize SCTRL2_ELx
2025-09-01 15:18 ` Dave Martin
@ 2025-09-01 18:17 ` Yeoreum Yun
2025-09-03 10:52 ` Dave Martin
0 siblings, 1 reply; 19+ messages in thread
From: Yeoreum Yun @ 2025-09-01 18:17 UTC (permalink / raw)
To: Dave Martin
Cc: catalin.marinas, will, broonie, oliver.upton, anshuman.khandual,
robh, james.morse, mark.rutland, joey.gouly, ahmed.genidi,
kevin.brodsky, scott, mbenes, james.clark, frederic, rafael,
pavel, ryan.roberts, suzuki.poulose, maz, linux-arm-kernel,
linux-kernel, linux-pm, kvmarm
Hi Dave,
> On Thu, Aug 21, 2025 at 06:24:03PM +0100, Yeoreum Yun wrote:
> > This series introduces initial support for the SCTLR2_ELx registers in Linux.
> > The feature is optional starting from ARMv8.8/ARMv9.3,
> > and becomes mandatory from ARMv8.9/ARMv9.4.
> >
> > Currently, Linux has no strict need to modify SCTLR2_ELx--
> > at least assuming that firmware initializes
> > these registers to reasonable defaults.
> >
> > However, several upcoming architectural features will require configuring
> > control bits in these registers.
> > Notable examples include FEAT_PAuth_LR and FEAT_CPA2.
>
> This looks OK to me now, except for one or two minor issues (see
> replies to the patches).
>
> I think we will need a way of testing all the code paths before this
> should be merged, though.
>
> Have you tested all the code paths, or are there some things that have
> not been tested?
I've tested for pKVM, nested and nhve and crash path
(I do my best what can I do for modified path).
>
>
> Since this code is not useful by itself, it may make sense to delay
> merging it until we have patches for a feature that depends on SCTLR2.
Whatever I pass this detiermination for maintainer.
>
> Cheers
> ---Dave
--
Sincerely,
Yeoreum Yun
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v4 2/5] arm64: initialise SCTLR2_ELx register at boot time
2025-09-01 15:13 ` Dave Martin
@ 2025-09-01 18:29 ` Yeoreum Yun
2025-09-02 10:39 ` Dave Martin
0 siblings, 1 reply; 19+ messages in thread
From: Yeoreum Yun @ 2025-09-01 18:29 UTC (permalink / raw)
To: Dave Martin
Cc: catalin.marinas, will, broonie, oliver.upton, anshuman.khandual,
robh, james.morse, mark.rutland, joey.gouly, ahmed.genidi,
kevin.brodsky, scott, mbenes, james.clark, frederic, rafael,
pavel, ryan.roberts, suzuki.poulose, maz, linux-arm-kernel,
linux-kernel, linux-pm, kvmarm
Hi Dave,
> On Thu, Aug 21, 2025 at 06:24:05PM +0100, Yeoreum Yun wrote:
> > The value of the SCTLR2_ELx register is UNKNOWN after reset.
> > If the firmware initializes these registers properly, no additional
> > initialization is required.
> > However, in cases where they are not initialized correctly,
> > initialize the SCTLR2_ELx registers during CPU/vCPU boot
> > to prevent unexpected system behavior caused by invalid values.
> >
> > Signed-off-by: Yeoreum Yun <yeoreum.yun@arm.com>
> > ---
>
> [...]
>
> > diff --git a/arch/arm64/include/asm/assembler.h b/arch/arm64/include/asm/assembler.h
> > index 23be85d93348..c25c2aed5125 100644
> > --- a/arch/arm64/include/asm/assembler.h
> > +++ b/arch/arm64/include/asm/assembler.h
> > @@ -738,6 +738,21 @@ alternative_endif
> > set_sctlr sctlr_el2, \reg
> > .endm
> >
> > +/* Set SCTLR2_ELx to the @reg value. */
> > +.macro set_sctlr2_elx, el, reg, tmp
> > + mrs_s \tmp, SYS_ID_AA64MMFR3_EL1
> > + ubfx \tmp, \tmp, #ID_AA64MMFR3_EL1_SCTLRX_SHIFT, #4
> > + cbz \tmp, .Lskip_sctlr2_\@
> > + .if \el == 2
> > + msr_s SYS_SCTLR2_EL2, \reg
> > + .elseif \el == 12
> > + msr_s SYS_SCTLR2_EL12, \reg
> > + .else
> > + msr_s SYS_SCTLR2_EL1, \reg
> > + .endif
> > +.Lskip_sctlr2_\@:
> > +.endm
> > +
>
> You were right that just doing
>
> msr_s SYS_SCTLR_\el, \reg
>
> here doesn't work. It looks like we rely on the C preprocessor to
> expand the SYS_FOO_REG names to numeric sysreg encodings. By the time
> the assembler macros are expanded, it is too late to construct sysreg
> names -- the C preprocessor only runs once, before the assembler.
>
> So, your code here looks reasonable.
>
> But, it will still silently do the wrong thing if \el is empty or
> garbage, so it is probably worth adding an error check:
>
> .else
> .error "Bad EL \"\el\" in set_sctlr2_elx"
> .endif
>
>
> Also, looking at all this again, the "1", "2" and "12" suffixes are not
> really numbers: SYS_REG_EL02 would definitely not be the same thing as
> SYS_REG_EL2 (although there is no _EL02 version of this particular
> register).
>
> So, can you use .ifc to do a string comparison instead?
>
> If might be a good idea to migrate other macros that use an "el"
> argument to do something similar -- although that probably doesn't
> belong in this series.
>
> The assembler seems to have no ".elseifc" directive, so you'll need
> separate nested .ifc blocks:
>
> .ifc \el,2
> msr_s SYS_SCTLR2_EL2, \reg
> .else
> .ifc \el,12
> msr_s SYS_SCTLR2_EL12, \reg
> .else
> .ifc \el,1
> msr_s SYS_SCTLR2_EL1, \reg
> .else
> .error "Bad EL \"\el\" in set_sctlr2_elx"
> .endif
> .endif
> .endif
>
> (Note, I've not tested this approach. If you can think of a better
> way, please feel free to suggest.)
Thanks for your suggestion. Let me test and check about this idea could
be applied in other macro too :D
(But as you mention, I'll apply this for SCTLR2 in other patchset).
>
> [...]
>
> > diff --git a/arch/arm64/kernel/hyp-stub.S b/arch/arm64/kernel/hyp-stub.S
> > index 36e2d26b54f5..ac12f1b4f8e2 100644
> > --- a/arch/arm64/kernel/hyp-stub.S
> > +++ b/arch/arm64/kernel/hyp-stub.S
> > @@ -144,7 +144,17 @@ SYM_CODE_START_LOCAL(__finalise_el2)
> >
> > .Lskip_indirection:
> > .Lskip_tcr2:
> > + mrs_s x1, SYS_ID_AA64MMFR3_EL1
> > + ubfx x1, x1, #ID_AA64MMFR3_EL1_SCTLRX_SHIFT, #4
> > + cbz x1, .Lskip_sctlr2
> > + mrs_s x1, SYS_SCTLR2_EL12
> > + msr_s SYS_SCTLR2_EL1, x1
> >
> > + // clean SCTLR2_EL1
> > + mov_q x1, INIT_SCTLR2_EL1
> > + msr_s SYS_SCTLR2_EL12, x1
>
> I'm still not sure why we need to do this. The code doesn't seem to
> clean up by the EL1 value of any other register -- or have I missed
> something?
>
> We have already switched to EL2, via the HVC call that jumped to
> __finalise_el2. We won't run at EL1 again unless KVM starts a guest --
> but in that case, it's KVM's responsibility to set up the EL1 registers
> before handing control to the guest.
>
> In any case, is SCTLR2_EL1 ever set to anything except INIT_SCTLR2_EL1
> before we get here?
Regardless of VHE and nVHE, between init_kernel_el() and finalise_el2()
the kernel modifies SCTLR_EL1/SCTLR2_EL1 (since el level in this period
is EL1).
and at the end of finalise_el2(), kernel switches to el2 and
if VHE, it writes the SCTLR_EL1/SCTLR2_EL1 to SCTLR_EL2/SCTLR2_EL2 and
initialize the SCTLR_EL1/SCTLR2_EL1.
Although there is no code to modify SCTLR2_EL1 between this period,
as SCTLR1_ELx, I initialize the SCTLR2_EL1 as init value for the future
usage.
Thanks!
--
Sincerely,
Yeoreum Yun
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v4 4/5] arm64: initialise SCTLR2_EL1 at cpu_soft_restart()
2025-09-01 15:13 ` Dave Martin
@ 2025-09-01 18:33 ` Yeoreum Yun
0 siblings, 0 replies; 19+ messages in thread
From: Yeoreum Yun @ 2025-09-01 18:33 UTC (permalink / raw)
To: Dave Martin
Cc: catalin.marinas, will, broonie, oliver.upton, anshuman.khandual,
robh, james.morse, mark.rutland, joey.gouly, ahmed.genidi,
kevin.brodsky, scott, mbenes, james.clark, frederic, rafael,
pavel, ryan.roberts, suzuki.poulose, maz, linux-arm-kernel,
linux-kernel, linux-pm, kvmarm
Hi Dave,
> > Explicitly initialize the SCTLR2_ELx register before launching
> > a new kernel via kexec() to avoid leaving SCTLR2_ELx with an
> > arbitrary value when the new kernel runs.
> >
> > Signed-off-by: Yeoreum Yun <yeoreum.yun@arm.com>
> > ---
> > arch/arm64/kernel/cpu-reset.S | 4 ++++
> > arch/arm64/kvm/hyp/nvhe/hyp-init.S | 3 +++
> > 2 files changed, 7 insertions(+)
> >
> > diff --git a/arch/arm64/kernel/cpu-reset.S b/arch/arm64/kernel/cpu-reset.S
> > index c87445dde674..c8888891dc8d 100644
> > --- a/arch/arm64/kernel/cpu-reset.S
> > +++ b/arch/arm64/kernel/cpu-reset.S
> > @@ -37,6 +37,10 @@ SYM_TYPED_FUNC_START(cpu_soft_restart)
> > * regime if HCR_EL2.E2H == 1
> > */
> > msr sctlr_el1, x12
> > +
> > + mov_q x12, INIT_SCTLR2_EL1
> > + set_sctlr2_elx 1, x12, x8
> > +
>
> Nit: does it matter whether we reset SCTLR2 before SCTLR?
>
> I can't find a convincing architectural reason why they need to be
> reset in a particular order, but it looks a bit strange that the
> cpu_soft_restart and __kvm_handle_stub_hvc versions of this reset the
> registers in the opposite order...
TBH, I couldn't find the reason why SCTLR2_ELx should be initilized
before SCTLR_EL1. I don't think current bits on SCTLR2_ELx doesn't have
any effects from SCTLR_EL1 (MMU bit and etc) and vice versa.
But as other code, as you mention, it would be better to reorder this
initialization.
Thanks!
[...]
--
Sincerely,
Yeoreum Yun
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v4 2/5] arm64: initialise SCTLR2_ELx register at boot time
2025-09-01 18:29 ` Yeoreum Yun
@ 2025-09-02 10:39 ` Dave Martin
2025-09-02 11:05 ` Yeoreum Yun
0 siblings, 1 reply; 19+ messages in thread
From: Dave Martin @ 2025-09-02 10:39 UTC (permalink / raw)
To: Yeoreum Yun
Cc: catalin.marinas, will, broonie, oliver.upton, anshuman.khandual,
robh, james.morse, mark.rutland, joey.gouly, ahmed.genidi,
kevin.brodsky, scott, mbenes, james.clark, frederic, rafael,
pavel, ryan.roberts, suzuki.poulose, maz, linux-arm-kernel,
linux-kernel, linux-pm, kvmarm
Hi,
On Mon, Sep 01, 2025 at 07:29:28PM +0100, Yeoreum Yun wrote:
> Hi Dave,
>
> > On Thu, Aug 21, 2025 at 06:24:05PM +0100, Yeoreum Yun wrote:
> > > The value of the SCTLR2_ELx register is UNKNOWN after reset.
> > > If the firmware initializes these registers properly, no additional
> > > initialization is required.
> > > However, in cases where they are not initialized correctly,
> > > initialize the SCTLR2_ELx registers during CPU/vCPU boot
> > > to prevent unexpected system behavior caused by invalid values.
> > >
> > > Signed-off-by: Yeoreum Yun <yeoreum.yun@arm.com>
> > > ---
> >
> > [...]
> >
> > > diff --git a/arch/arm64/include/asm/assembler.h b/arch/arm64/include/asm/assembler.h
> > > index 23be85d93348..c25c2aed5125 100644
> > > --- a/arch/arm64/include/asm/assembler.h
> > > +++ b/arch/arm64/include/asm/assembler.h
> > > @@ -738,6 +738,21 @@ alternative_endif
[...]
> > > +/* Set SCTLR2_ELx to the @reg value. */
> > > +.macro set_sctlr2_elx, el, reg, tmp
> > > + mrs_s \tmp, SYS_ID_AA64MMFR3_EL1
> > > + ubfx \tmp, \tmp, #ID_AA64MMFR3_EL1_SCTLRX_SHIFT, #4
> > > + cbz \tmp, .Lskip_sctlr2_\@
> > > + .if \el == 2
> > > + msr_s SYS_SCTLR2_EL2, \reg
> > > + .elseif \el == 12
> > > + msr_s SYS_SCTLR2_EL12, \reg
> > > + .else
> > > + msr_s SYS_SCTLR2_EL1, \reg
> > > + .endif
> > > +.Lskip_sctlr2_\@:
> > > +.endm
[...]
> > If might be a good idea to migrate other macros that use an "el"
> > argument to do something similar -- although that probably doesn't
> > belong in this series.
> >
> > The assembler seems to have no ".elseifc" directive, so you'll need
> > separate nested .ifc blocks:
> >
> > .ifc \el,2
> > msr_s SYS_SCTLR2_EL2, \reg
> > .else
> > .ifc \el,12
> > msr_s SYS_SCTLR2_EL12, \reg
> > .else
> > .ifc \el,1
> > msr_s SYS_SCTLR2_EL1, \reg
> > .else
> > .error "Bad EL \"\el\" in set_sctlr2_elx"
> > .endif
> > .endif
> > .endif
> >
> > (Note, I've not tested this approach. If you can think of a better
> > way, please feel free to suggest.)
>
> Thanks for your suggestion. Let me test and check about this idea could
> be applied in other macro too :D
> (But as you mention, I'll apply this for SCTLR2 in other patchset).
Ack, let me know how it goes.
It is probably not worth doing this if the changes become complicated,
though.
[...]
> > > diff --git a/arch/arm64/kernel/hyp-stub.S b/arch/arm64/kernel/hyp-stub.S
> > > index 36e2d26b54f5..ac12f1b4f8e2 100644
> > > --- a/arch/arm64/kernel/hyp-stub.S
> > > +++ b/arch/arm64/kernel/hyp-stub.S
> > > @@ -144,7 +144,17 @@ SYM_CODE_START_LOCAL(__finalise_el2)
> > >
> > > .Lskip_indirection:
> > > .Lskip_tcr2:
> > > + mrs_s x1, SYS_ID_AA64MMFR3_EL1
> > > + ubfx x1, x1, #ID_AA64MMFR3_EL1_SCTLRX_SHIFT, #4
> > > + cbz x1, .Lskip_sctlr2
> > > + mrs_s x1, SYS_SCTLR2_EL12
> > > + msr_s SYS_SCTLR2_EL1, x1
> > >
> > > + // clean SCTLR2_EL1
> > > + mov_q x1, INIT_SCTLR2_EL1
> > > + msr_s SYS_SCTLR2_EL12, x1
> >
> > I'm still not sure why we need to do this. The code doesn't seem to
> > clean up by the EL1 value of any other register -- or have I missed
> > something?
> >
> > We have already switched to EL2, via the HVC call that jumped to
> > __finalise_el2. We won't run at EL1 again unless KVM starts a guest --
> > but in that case, it's KVM's responsibility to set up the EL1 registers
> > before handing control to the guest.
> >
> > In any case, is SCTLR2_EL1 ever set to anything except INIT_SCTLR2_EL1
> > before we get here?
>
> Regardless of VHE and nVHE, between init_kernel_el() and finalise_el2()
> the kernel modifies SCTLR_EL1/SCTLR2_EL1 (since el level in this period
> is EL1).
> and at the end of finalise_el2(), kernel switches to el2 and
> if VHE, it writes the SCTLR_EL1/SCTLR2_EL1 to SCTLR_EL2/SCTLR2_EL2 and
> initialize the SCTLR_EL1/SCTLR2_EL1.
>
> Although there is no code to modify SCTLR2_EL1 between this period,
> as SCTLR1_ELx, I initialize the SCTLR2_EL1 as init value for the future
> usage.
I think that we don't need to ensure that all sysregs are cleanly
initialised; only those that can affect execution in some way need to
be initialised.
Once we are running at EL2 with VHE, we don't switch back to EL1, so
the _EL1 control registers don't affect execution any more.
If we did have to clean up the _EL1 registers, then this code would
need to clean up all the other regs too, but I can't see clean-up for
anything other than SCTLR2_EL1 here. Is there some cleanup code
elsewhere that I have missed?
Cheers
---Dave
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v4 2/5] arm64: initialise SCTLR2_ELx register at boot time
2025-09-02 10:39 ` Dave Martin
@ 2025-09-02 11:05 ` Yeoreum Yun
2025-09-03 10:43 ` Dave Martin
0 siblings, 1 reply; 19+ messages in thread
From: Yeoreum Yun @ 2025-09-02 11:05 UTC (permalink / raw)
To: Dave Martin
Cc: catalin.marinas, will, broonie, oliver.upton, anshuman.khandual,
robh, james.morse, mark.rutland, joey.gouly, ahmed.genidi,
kevin.brodsky, scott, mbenes, james.clark, frederic, rafael,
pavel, ryan.roberts, suzuki.poulose, maz, linux-arm-kernel,
linux-kernel, linux-pm, kvmarm
Hi Dave,
[...]
> > > > diff --git a/arch/arm64/kernel/hyp-stub.S b/arch/arm64/kernel/hyp-stub.S
> > > > index 36e2d26b54f5..ac12f1b4f8e2 100644
> > > > --- a/arch/arm64/kernel/hyp-stub.S
> > > > +++ b/arch/arm64/kernel/hyp-stub.S
> > > > @@ -144,7 +144,17 @@ SYM_CODE_START_LOCAL(__finalise_el2)
> > > >
> > > > .Lskip_indirection:
> > > > .Lskip_tcr2:
> > > > + mrs_s x1, SYS_ID_AA64MMFR3_EL1
> > > > + ubfx x1, x1, #ID_AA64MMFR3_EL1_SCTLRX_SHIFT, #4
> > > > + cbz x1, .Lskip_sctlr2
> > > > + mrs_s x1, SYS_SCTLR2_EL12
> > > > + msr_s SYS_SCTLR2_EL1, x1
> > > >
> > > > + // clean SCTLR2_EL1
> > > > + mov_q x1, INIT_SCTLR2_EL1
> > > > + msr_s SYS_SCTLR2_EL12, x1
> > >
> > > I'm still not sure why we need to do this. The code doesn't seem to
> > > clean up by the EL1 value of any other register -- or have I missed
> > > something?
> > >
> > > We have already switched to EL2, via the HVC call that jumped to
> > > __finalise_el2. We won't run at EL1 again unless KVM starts a guest --
> > > but in that case, it's KVM's responsibility to set up the EL1 registers
> > > before handing control to the guest.
> > >
> > > In any case, is SCTLR2_EL1 ever set to anything except INIT_SCTLR2_EL1
> > > before we get here?
> >
> > Regardless of VHE and nVHE, between init_kernel_el() and finalise_el2()
> > the kernel modifies SCTLR_EL1/SCTLR2_EL1 (since el level in this period
> > is EL1).
> > and at the end of finalise_el2(), kernel switches to el2 and
> > if VHE, it writes the SCTLR_EL1/SCTLR2_EL1 to SCTLR_EL2/SCTLR2_EL2 and
> > initialize the SCTLR_EL1/SCTLR2_EL1.
> >
> > Although there is no code to modify SCTLR2_EL1 between this period,
> > as SCTLR1_ELx, I initialize the SCTLR2_EL1 as init value for the future
> > usage.
>
> I think that we don't need to ensure that all sysregs are cleanly
> initialised; only those that can affect execution in some way need to
> be initialised.
>
> Once we are running at EL2 with VHE, we don't switch back to EL1, so
> the _EL1 control registers don't affect execution any more.
>
> If we did have to clean up the _EL1 registers, then this code would
> need to clean up all the other regs too, but I can't see clean-up for
> anything other than SCTLR2_EL1 here. Is there some cleanup code
> elsewhere that I have missed?
>
> Cheers
> ---Dave
When I look at init_el2(), it returns to EL1 via:
mov x0, #INIT_PSTATE_EL1
msr spsr_el2, x0
...
eret
In other words, from init_kernel_el() through finalise_el2(),
all system-register accesses are made at EL1 (i.e., SYS_REG_EL1).
During this period, it appears that only SCTLR_EL1 is modified,
so the code only needs to care about the accessed register — SCTLR_EL1.
That’s why SCTLR_EL1 is reinitialised at the end of finalise_el2().
Otherwise, the MMU bit might remain enabled, which could cause errors later
when launching a VM under VHE.
However, the idea behind this patch is to initialise SCTLR2_ELx
the same way as SCTLR_ELx.
I’m not sure whether SCTLR2_ELx is modified during this period.
If it is (now or in the future),
it should be cleared/reinitialised just like SCTLR_EL1.
This patch is based on the assumption that there may be modifications to
SCTLR2_ELx during this period. So it isn’t about other system registers;
it’s about the register actually used during this period.
Am I missing anything?
Thanks!
--
Sincerely,
Yeoreum Yun
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v4 2/5] arm64: initialise SCTLR2_ELx register at boot time
2025-09-02 11:05 ` Yeoreum Yun
@ 2025-09-03 10:43 ` Dave Martin
2025-09-03 10:59 ` Yeoreum Yun
0 siblings, 1 reply; 19+ messages in thread
From: Dave Martin @ 2025-09-03 10:43 UTC (permalink / raw)
To: Yeoreum Yun
Cc: catalin.marinas, will, broonie, oliver.upton, anshuman.khandual,
robh, james.morse, mark.rutland, joey.gouly, ahmed.genidi,
kevin.brodsky, scott, mbenes, james.clark, frederic, rafael,
pavel, ryan.roberts, suzuki.poulose, maz, linux-arm-kernel,
linux-kernel, linux-pm, kvmarm
Hi,
On Tue, Sep 02, 2025 at 12:05:50PM +0100, Yeoreum Yun wrote:
> Hi Dave,
>
> [...]
>
> > > > > diff --git a/arch/arm64/kernel/hyp-stub.S b/arch/arm64/kernel/hyp-stub.S
> > > > > index 36e2d26b54f5..ac12f1b4f8e2 100644
> > > > > --- a/arch/arm64/kernel/hyp-stub.S
> > > > > +++ b/arch/arm64/kernel/hyp-stub.S
> > > > > @@ -144,7 +144,17 @@ SYM_CODE_START_LOCAL(__finalise_el2)
> > > > >
> > > > > .Lskip_indirection:
> > > > > .Lskip_tcr2:
> > > > > + mrs_s x1, SYS_ID_AA64MMFR3_EL1
> > > > > + ubfx x1, x1, #ID_AA64MMFR3_EL1_SCTLRX_SHIFT, #4
> > > > > + cbz x1, .Lskip_sctlr2
> > > > > + mrs_s x1, SYS_SCTLR2_EL12
> > > > > + msr_s SYS_SCTLR2_EL1, x1
> > > > >
> > > > > + // clean SCTLR2_EL1
> > > > > + mov_q x1, INIT_SCTLR2_EL1
> > > > > + msr_s SYS_SCTLR2_EL12, x1
> > > >
> > > > I'm still not sure why we need to do this. The code doesn't seem to
> > > > clean up by the EL1 value of any other register -- or have I missed
> > > > something?
> > > >
> > > > We have already switched to EL2, via the HVC call that jumped to
> > > > __finalise_el2. We won't run at EL1 again unless KVM starts a guest --
> > > > but in that case, it's KVM's responsibility to set up the EL1 registers
> > > > before handing control to the guest.
> > > >
> > > > In any case, is SCTLR2_EL1 ever set to anything except INIT_SCTLR2_EL1
> > > > before we get here?
[...]
> When I look at init_el2(), it returns to EL1 via:
>
> mov x0, #INIT_PSTATE_EL1
> msr spsr_el2, x0
> ...
> eret
>
> In other words, from init_kernel_el() through finalise_el2(),
> all system-register accesses are made at EL1 (i.e., SYS_REG_EL1).
> During this period, it appears that only SCTLR_EL1 is modified,
> so the code only needs to care about the accessed register — SCTLR_EL1.
>
> That’s why SCTLR_EL1 is reinitialised at the end of finalise_el2().
> Otherwise, the MMU bit might remain enabled, which could cause errors later
> when launching a VM under VHE.
>
> However, the idea behind this patch is to initialise SCTLR2_ELx
> the same way as SCTLR_ELx.
> I’m not sure whether SCTLR2_ELx is modified during this period.
> If it is (now or in the future),
> it should be cleared/reinitialised just like SCTLR_EL1.
>
> This patch is based on the assumption that there may be modifications to
> SCTLR2_ELx during this period. So it isn’t about other system registers;
> it’s about the register actually used during this period.
>
> Am I missing anything?
>
> Thanks!
>
> --
> Sincerely,
> Yeoreum Yun
I think I missed the SCTLR_EL1 reset in the idmap code after the
enter_vhe label.
Actually, I'm not sure whether there is any architectural reason for
setting SCTLR_EL1 to INIT_SCTLR_EL1_MMU_OFF here. "for good measure"
suggests that it felt like a good idea but there was no known reason
for it. The commit message for the original patch doesn't offer an
explanation -- maybe Marc can remember.
This might be a defence against speculative translation table walks
using the EL1&0 regime (but the architecture says [RNRJPP]: "If an
implementation is executing at EL3 or EL2, the PE is not permitted to
use the registers associated with the EL1&0 translation regime to
speculatively access memory or translation tables.") So it shouldn't
really matter, but in case buggy CPUs don't implement this rule
properly it may be a good idea to turn the stage1 MMU off just in case.
Since it's there, though, it probably does make sense to reinitialise
SCTLR2_EL1 at the same time -- but can you move this so that it is next
to the SCTLR_EL1 reinitialisation? Otherwise, the purpose of
reinitialising SCTLR2_EL1 is unclear. It really should come under the
same "for good measure" justification as the SCTLR_EL1 reset.
However, I don't think this has anything to do with putting things into
a clean state for VMs. KVM defines the reset state for all the _EL1
regs explicitly -- failing to do that would be a bug in KVM.
(See arch/arm64/kvm/sys_regs.c : sys_reg_descs[], kvm_reset_sys_regs().)
Cheers
---Dave
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v4 0/5] initialize SCTRL2_ELx
2025-09-01 18:17 ` Yeoreum Yun
@ 2025-09-03 10:52 ` Dave Martin
2025-09-03 12:08 ` Yeoreum Yun
0 siblings, 1 reply; 19+ messages in thread
From: Dave Martin @ 2025-09-03 10:52 UTC (permalink / raw)
To: Yeoreum Yun
Cc: catalin.marinas, will, broonie, oliver.upton, anshuman.khandual,
robh, james.morse, mark.rutland, joey.gouly, ahmed.genidi,
kevin.brodsky, scott, mbenes, james.clark, frederic, rafael,
pavel, ryan.roberts, suzuki.poulose, maz, linux-arm-kernel,
linux-kernel, linux-pm, kvmarm
Hi,
On Mon, Sep 01, 2025 at 07:17:56PM +0100, Yeoreum Yun wrote:
> Hi Dave,
>
> > On Thu, Aug 21, 2025 at 06:24:03PM +0100, Yeoreum Yun wrote:
> > > This series introduces initial support for the SCTLR2_ELx registers in Linux.
> > > The feature is optional starting from ARMv8.8/ARMv9.3,
> > > and becomes mandatory from ARMv8.9/ARMv9.4.
> > >
> > > Currently, Linux has no strict need to modify SCTLR2_ELx--
> > > at least assuming that firmware initializes
> > > these registers to reasonable defaults.
> > >
> > > However, several upcoming architectural features will require configuring
> > > control bits in these registers.
> > > Notable examples include FEAT_PAuth_LR and FEAT_CPA2.
> >
> > This looks OK to me now, except for one or two minor issues (see
> > replies to the patches).
> >
> > I think we will need a way of testing all the code paths before this
> > should be merged, though.
> >
> > Have you tested all the code paths, or are there some things that have
> > not been tested?
>
> I've tested for pKVM, nested and nhve and crash path
> (I do my best what can I do for modified path).
Was that just confirming that the kernel boots / does not crash?
What about CPU suspend/resume and hotplug?
My concern is that most of the defined SCTLR2_ELx bits won't affect
kernel execution unless the corresponding hardware features are
actively being used -- so while we know that the patches don't break
the kernel, this may not prove that SCTLR2_ELx is really being
initialised / saved / restored / reset correctly.
> > Since this code is not useful by itself, it may make sense to delay
> > merging it until we have patches for a feature that depends on SCTLR2.
>
> Whatever I pass this detiermination for maintainer.
Sure, that's just my opinion.
Either way, this doesn't stop anyone from building support for specific
features on top of this series before it gets merged.
Cheers
---Dave
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v4 2/5] arm64: initialise SCTLR2_ELx register at boot time
2025-09-03 10:43 ` Dave Martin
@ 2025-09-03 10:59 ` Yeoreum Yun
0 siblings, 0 replies; 19+ messages in thread
From: Yeoreum Yun @ 2025-09-03 10:59 UTC (permalink / raw)
To: Dave Martin
Cc: catalin.marinas, will, broonie, oliver.upton, anshuman.khandual,
robh, james.morse, mark.rutland, joey.gouly, ahmed.genidi,
kevin.brodsky, scott, mbenes, james.clark, frederic, rafael,
pavel, ryan.roberts, suzuki.poulose, maz, linux-arm-kernel,
linux-kernel, linux-pm, kvmarm
Hi Dave,
[...]
> > > > > > .Lskip_indirection:
> > > > > > .Lskip_tcr2:
> > > > > > + mrs_s x1, SYS_ID_AA64MMFR3_EL1
> > > > > > + ubfx x1, x1, #ID_AA64MMFR3_EL1_SCTLRX_SHIFT, #4
> > > > > > + cbz x1, .Lskip_sctlr2
> > > > > > + mrs_s x1, SYS_SCTLR2_EL12
> > > > > > + msr_s SYS_SCTLR2_EL1, x1
> > > > > >
> > > > > > + // clean SCTLR2_EL1
> > > > > > + mov_q x1, INIT_SCTLR2_EL1
> > > > > > + msr_s SYS_SCTLR2_EL12, x1
> > > > >
> > > > > I'm still not sure why we need to do this. The code doesn't seem to
> > > > > clean up by the EL1 value of any other register -- or have I missed
> > > > > something?
> > > > >
> > > > > We have already switched to EL2, via the HVC call that jumped to
> > > > > __finalise_el2. We won't run at EL1 again unless KVM starts a guest --
> > > > > but in that case, it's KVM's responsibility to set up the EL1 registers
> > > > > before handing control to the guest.
> > > > >
> > > > > In any case, is SCTLR2_EL1 ever set to anything except INIT_SCTLR2_EL1
> > > > > before we get here?
>
> [...]
>
> > When I look at init_el2(), it returns to EL1 via:
> >
> > mov x0, #INIT_PSTATE_EL1
> > msr spsr_el2, x0
> > ...
> > eret
> >
> > In other words, from init_kernel_el() through finalise_el2(),
> > all system-register accesses are made at EL1 (i.e., SYS_REG_EL1).
> > During this period, it appears that only SCTLR_EL1 is modified,
> > so the code only needs to care about the accessed register — SCTLR_EL1.
> >
> > That’s why SCTLR_EL1 is reinitialised at the end of finalise_el2().
> > Otherwise, the MMU bit might remain enabled, which could cause errors later
> > when launching a VM under VHE.
> >
> > However, the idea behind this patch is to initialise SCTLR2_ELx
> > the same way as SCTLR_ELx.
> > I’m not sure whether SCTLR2_ELx is modified during this period.
> > If it is (now or in the future),
> > it should be cleared/reinitialised just like SCTLR_EL1.
> >
> > This patch is based on the assumption that there may be modifications to
> > SCTLR2_ELx during this period. So it isn’t about other system registers;
> > it’s about the register actually used during this period.
> >
> > Am I missing anything?
> >
> > Thanks!
> >
> > --
> > Sincerely,
> > Yeoreum Yun
>
> I think I missed the SCTLR_EL1 reset in the idmap code after the
> enter_vhe label.
>
> Actually, I'm not sure whether there is any architectural reason for
> setting SCTLR_EL1 to INIT_SCTLR_EL1_MMU_OFF here. "for good measure"
> suggests that it felt like a good idea but there was no known reason
> for it. The commit message for the original patch doesn't offer an
> explanation -- maybe Marc can remember.
>
> This might be a defence against speculative translation table walks
> using the EL1&0 regime (but the architecture says [RNRJPP]: "If an
> implementation is executing at EL3 or EL2, the PE is not permitted to
> use the registers associated with the EL1&0 translation regime to
> speculatively access memory or translation tables.") So it shouldn't
> really matter, but in case buggy CPUs don't implement this rule
> properly it may be a good idea to turn the stage1 MMU off just in case.
>
Thanks for great deep insight :D.
> Since it's there, though, it probably does make sense to reinitialise
> SCTLR2_EL1 at the same time -- but can you move this so that it is next
> to the SCTLR_EL1 reinitialisation? Otherwise, the purpose of
> reinitialising SCTLR2_EL1 is unclear. It really should come under the
> same "for good measure" justification as the SCTLR_EL1 reset.
Okay.
>
> However, I don't think this has anything to do with putting things into
> a clean state for VMs. KVM defines the reset state for all the _EL1
> regs explicitly -- failing to do that would be a bug in KVM.
>
> (See arch/arm64/kvm/sys_regs.c : sys_reg_descs[], kvm_reset_sys_regs().)
Right. I've missed the reset sysregs when kvm is launching.
Thanks!
--
Sincerely,
Yeoreum Yun
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v4 0/5] initialize SCTRL2_ELx
2025-09-03 10:52 ` Dave Martin
@ 2025-09-03 12:08 ` Yeoreum Yun
0 siblings, 0 replies; 19+ messages in thread
From: Yeoreum Yun @ 2025-09-03 12:08 UTC (permalink / raw)
To: Dave Martin
Cc: catalin.marinas, will, broonie, oliver.upton, anshuman.khandual,
robh, james.morse, mark.rutland, joey.gouly, ahmed.genidi,
kevin.brodsky, scott, mbenes, james.clark, frederic, rafael,
pavel, ryan.roberts, suzuki.poulose, maz, linux-arm-kernel,
linux-kernel, linux-pm, kvmarm
Hi Dave,
[...]
> > > > This series introduces initial support for the SCTLR2_ELx registers in Linux.
> > > > The feature is optional starting from ARMv8.8/ARMv9.3,
> > > > and becomes mandatory from ARMv8.9/ARMv9.4.
> > > >
> > > > Currently, Linux has no strict need to modify SCTLR2_ELx--
> > > > at least assuming that firmware initializes
> > > > these registers to reasonable defaults.
> > > >
> > > > However, several upcoming architectural features will require configuring
> > > > control bits in these registers.
> > > > Notable examples include FEAT_PAuth_LR and FEAT_CPA2.
> > >
> > > This looks OK to me now, except for one or two minor issues (see
> > > replies to the patches).
> > >
> > > I think we will need a way of testing all the code paths before this
> > > should be merged, though.
> > >
> > > Have you tested all the code paths, or are there some things that have
> > > not been tested?
> >
> > I've tested for pKVM, nested and nhve and crash path
> > (I do my best what can I do for modified path).
>
> Was that just confirming that the kernel boots / does not crash?
Not only that, since the my last mistake, I've check it with debugger
too -- set the SCTLR2_ELx as I expected.
>
> What about CPU suspend/resume and hotplug?
Of course It's done both enter/exit idle and hotplug with related kselftest test.
>
> My concern is that most of the defined SCTLR2_ELx bits won't affect
> kernel execution unless the corresponding hardware features are
> actively being used -- so while we know that the patches don't break
> the kernel, this may not prove that SCTLR2_ELx is really being
> initialised / saved / restored / reset correctly.
That's why I've confirmed with debugger whether the SCTLR2_ELx value
sets as I expected... personally I've done as much as I can for
test related for SCTLR2_ELx.
>
> > > Since this code is not useful by itself, it may make sense to delay
> > > merging it until we have patches for a feature that depends on SCTLR2.
> >
> > Whatever I pass this detiermination for maintainer.
>
> Sure, that's just my opinion.
>
> Either way, this doesn't stop anyone from building support for specific
> features on top of this series before it gets merged.
Okay.
Thanks!
--
Sincerely,
Yeoreum Yun
^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2025-09-03 12:09 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-21 17:24 [PATCH v4 0/5] initialize SCTRL2_ELx Yeoreum Yun
2025-08-21 17:24 ` [PATCH v4 1/5] arm64: make SCTLR2_EL1 accessible Yeoreum Yun
2025-08-21 17:24 ` [PATCH v4 2/5] arm64: initialise SCTLR2_ELx register at boot time Yeoreum Yun
2025-09-01 15:13 ` Dave Martin
2025-09-01 18:29 ` Yeoreum Yun
2025-09-02 10:39 ` Dave Martin
2025-09-02 11:05 ` Yeoreum Yun
2025-09-03 10:43 ` Dave Martin
2025-09-03 10:59 ` Yeoreum Yun
2025-08-21 17:24 ` [PATCH v4 3/5] arm64: save/restore SCTLR2_EL1 when cpu_suspend()/resume() Yeoreum Yun
2025-08-21 17:24 ` [PATCH v4 4/5] arm64: initialise SCTLR2_EL1 at cpu_soft_restart() Yeoreum Yun
2025-09-01 15:13 ` Dave Martin
2025-09-01 18:33 ` Yeoreum Yun
2025-08-21 17:24 ` [PATCH v4 5/5] arm64: make the per-task SCTLR2_EL1 Yeoreum Yun
2025-09-01 10:08 ` [PATCH v4 0/5] initialize SCTRL2_ELx Yeoreum Yun
2025-09-01 15:18 ` Dave Martin
2025-09-01 18:17 ` Yeoreum Yun
2025-09-03 10:52 ` Dave Martin
2025-09-03 12:08 ` Yeoreum Yun
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).