arm64: Early CPU feature override, and applications to VHE, BTI and PAuth

If someone happens to write the following code:

	b	1f
	init_el2_state	vhe
1:
	[...]

they will be in for a long debugging session, as the label "1f" will be resolved *inside* the init_el2_state macro instead of after it. Not really what one expects.

Instead, rewite the EL2 setup macros to use unambiguous labels, thanks to the usual macro counter trick.

Acked-by Catalin Marinas <[email protected]>

Signed-off-by Marc Zyngier <[email protected]>

---
 arch/arm64/include/asm/el2_setup.h | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

The arm64 kernel has long be able to use more than 39bit VAs. Since day one, actually. Let's rewrite the offending comment.

Signed-off-by Marc Zyngier <[email protected]>

---
 arch/arm64/mm/proc.S | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

arch/arm64/mm/proc.S +0 -0

index 1f7ee8c8b7b8..ece785477bdc 100644

Turning the MMU on is a popular sport in the arm64 kernel, and we do it more than once, or even twice. As we are about to add even more, let's turn it into a macro.

No expected functional change.

Signed-off-by Marc Zyngier <[email protected]>

As we are about to change the way a VHE system boots, let's provide the core helper, in the form of a stub hypercall that enables VHE and replicates the full EL1 context at EL2, thanks to EL1 and VHE-EL2 being extremely similar.

On exception return, the kernel carries on at EL2. Fancy!

Nothing calls this new hypercall yet, so no functional change.

Signed-off-by Marc Zyngier <[email protected]>

---
 arch/arm64/include/asm/virt.h |  7 +++-
 arch/arm64/kernel/hyp-stub.S  | 67 +++++++++++++++++++++++++++++++++--
 2 files changed, 71 insertions(+), 3 deletions(-)

arch/arm64/include/asm/virt.h +6 -1

index ee6a48df89d9..7379f35ae2c6 100644
@@ -35,8 +35,13 @@
  */
 #define HVC_RESET_VECTORS 2
 
+/*
+ * HVC_VHE_RESTART - Upgrade the CPU from EL1 to EL2, if possible
+ */
+#define HVC_VHE_RESTART	3
+
 /* Max number of HYP stub hypercalls */
-#define HVC_STUB_HCALL_NR 3
+#define HVC_STUB_HCALL_NR 4
 
 /* Error returned when an invalid stub number is passed into x0 */
 #define HVC_STUB_ERR	0xbadca11

arch/arm64/kernel/hyp-stub.S +5 -2

index 160f5881a0b7..fb12398b5c28 100644
@@ -8,9 +8,9 @@
 
 #include <linux/init.h>
 #include <linux/linkage.h>
-#include <linux/irqchip/arm-gic-v3.h>
 
 #include <asm/assembler.h>
+#include <asm/el2_setup.h>
 #include <asm/kvm_arm.h>
 #include <asm/kvm_asm.h>
 #include <asm/ptrace.h>

@@ -47,10 +47,13 @@ SYM_CODE_END(__hyp_stub_vectors)
 
 SYM_CODE_START_LOCAL(el1_sync)
 	cmp	x0, #HVC_SET_VECTORS
-	b.ne	2f
+	b.ne	1f
 	msr	vbar_el2, x1
 	b	9f
 
+1:	cmp	x0, #HVC_VHE_RESTART
+	b.eq	mutate_to_vhe
+
 2:	cmp	x0, #HVC_SOFT_RESTART
 	b.ne	3f
 	mov	x0, x2

@@ -70,6 +73,66 @@ SYM_CODE_START_LOCAL(el1_sync)
 	eret
 SYM_CODE_END(el1_sync)
 

arch/arm64/kernel/hyp-stub.S +50 -0

+
+	// Engage the VHE magic!
+	mov_q	x0, HCR_HOST_VHE_FLAGS
+	msr	hcr_el2, x0
+	isb
+
+	// Doesn't do much on VHE, but still, worth a shot
+	init_el2_state vhe
+
+	// Use the EL1 allocated stack, per-cpu offset
+	mrs	x0, sp_el1
+	mov	sp, x0
+	mrs	x0, tpidr_el1
+	msr	tpidr_el2, x0
+
+	// FP configuration, vectors
+	mrs_s	x0, SYS_CPACR_EL12
+	msr	cpacr_el1, x0
+	mrs_s	x0, SYS_VBAR_EL12
+	msr	vbar_el1, x0
+
+	// Transfer the MM state from EL1 to EL2
+	mrs_s	x0, SYS_TCR_EL12
+	msr	tcr_el1, x0
+	mrs_s	x0, SYS_TTBR0_EL12
+	msr	ttbr0_el1, x0
+	mrs_s	x0, SYS_TTBR1_EL12
+	msr	ttbr1_el1, x0
+	mrs_s	x0, SYS_MAIR_EL12
+	msr	mair_el1, x0
+	isb
+
+	// Invalidate TLBs before enabling the MMU
+	tlbi	vmalle1
+	dsb	nsh
+
+	// Enable the EL2 S1 MMU, as set up from EL1
+	mrs_s	x0, SYS_SCTLR_EL12
+	set_sctlr_el1	x0
+
+	// Hack the exception return to stay at EL2
+	mrs	x0, spsr_el1
+	and	x0, x0, #~PSR_MODE_MASK
+	mov	x1, #PSR_MODE_EL2h
+	orr	x0, x0, x1
+	msr	spsr_el1, x0
+
+1:	eret
+SYM_CODE_END(mutate_to_vhe)
+
 .macro invalid_vector	label
 SYM_CODE_START_LOCAL(\label)
 	b \label

Signature · 1 line

2.29.2

Quoted 68 lines · Marc Zyngier

As we are about to change the way a VHE system boots, let's provide the core helper, in the form of a stub hypercall that enables VHE and replicates the full EL1 context at EL2, thanks to EL1 and VHE-EL2 being extremely similar.

On exception return, the kernel carries on at EL2. Fancy!

Nothing calls this new hypercall yet, so no functional change.

Signed-off-by Marc Zyngier <[email protected]>

---
 arch/arm64/include/asm/virt.h |  7 +++-
 arch/arm64/kernel/hyp-stub.S  | 67 +++++++++++++++++++++++++++++++++--
 2 files changed, 71 insertions(+), 3 deletions(-)

arch/arm64/include/asm/virt.h +6 -1

index ee6a48df89d9..7379f35ae2c6 100644
@@ -35,8 +35,13 @@
  */
 #define HVC_RESET_VECTORS 2
 
+/*
+ * HVC_VHE_RESTART - Upgrade the CPU from EL1 to EL2, if possible
+ */
+#define HVC_VHE_RESTART	3
+
 /* Max number of HYP stub hypercalls */
-#define HVC_STUB_HCALL_NR 3
+#define HVC_STUB_HCALL_NR 4
 
 /* Error returned when an invalid stub number is passed into x0 */
 #define HVC_STUB_ERR	0xbadca11

arch/arm64/kernel/hyp-stub.S +5 -2

index 160f5881a0b7..fb12398b5c28 100644
@@ -8,9 +8,9 @@
 
 #include <linux/init.h>
 #include <linux/linkage.h>
-#include <linux/irqchip/arm-gic-v3.h>
 
 #include <asm/assembler.h>
+#include <asm/el2_setup.h>
 #include <asm/kvm_arm.h>
 #include <asm/kvm_asm.h>
 #include <asm/ptrace.h>

@@ -47,10 +47,13 @@ SYM_CODE_END(__hyp_stub_vectors)
 
 SYM_CODE_START_LOCAL(el1_sync)
 	cmp	x0, #HVC_SET_VECTORS
-	b.ne	2f
+	b.ne	1f
 	msr	vbar_el2, x1
 	b	9f
 
+1:	cmp	x0, #HVC_VHE_RESTART
+	b.eq	mutate_to_vhe
+
 2:	cmp	x0, #HVC_SOFT_RESTART
 	b.ne	3f
 	mov	x0, x2

@@ -70,6 +73,66 @@ SYM_CODE_START_LOCAL(el1_sync)
 	eret
 SYM_CODE_END(el1_sync)
 

arch/arm64/kernel/hyp-stub.S SYM_CODE_START_LOCAL(el1_sync) · line 85

Quoted 10 lines

arch/arm64/kernel/hyp-stub.S +10 -0

+// nVHE? No way! Give me the real thing!
+SYM_CODE_START_LOCAL(mutate_to_vhe)
+	// Sanity check: MMU *must* be off
+	mrs	x0, sctlr_el2
+	tbnz	x0, #0, 1f
+
+	// Needs to be VHE capable, obviously
+	mrs	x0, id_aa64mmfr1_el1
+	ubfx	x0, x0, #ID_AA64MMFR1_VHE_SHIFT, #4
+	cbz	x0, 1f

nit: There is a HVC_STUB_ERR that you could return if these sanity checks fail. The documentation also states that it should be returned on error.

We can now move the initial SCTLR_EL1 setup to be used for both EL1 and EL2 setup.

Signed-off-by Marc Zyngier <[email protected]>

arch/arm64/include/asm/cpufeature.h +2 -0

index 9a555809b89c..465d2cb63bfc 100644
@@ -75,6 +75,8 @@ struct arm64_ftr_reg {
 	u64				sys_val;
 	u64				user_val;
 	const struct arm64_ftr_bits	*ftr_bits;
+	u64				*override_val;
+	u64				*override_mask;
 };
 
 extern struct arm64_ftr_reg arm64_ftr_reg_ctrel0;

arch/arm64/kernel/cpufeature.c +15 -5

index e99eddec0a46..aaa075c6f029 100644
@@ -544,13 +544,17 @@ static const struct arm64_ftr_bits ftr_raz[] = {
 	ARM64_FTR_END,
 };
 
-#define ARM64_FTR_REG(id, table) {		\
-	.sys_id = id,				\
-	.reg = 	&(struct arm64_ftr_reg){	\
-		.name = #id,			\
-		.ftr_bits = &((table)[0]),	\
+#define ARM64_FTR_REG_OVERRIDE(id, table, v, m) {		\
+		.sys_id = id,					\
+		.reg = 	&(struct arm64_ftr_reg){		\
+			.name = #id,				\
+			.ftr_bits = &((table)[0]),		\
+			.override_val = v,			\
+			.override_mask = m,			\
 	}}
 
+#define ARM64_FTR_REG(id, table) ARM64_FTR_REG_OVERRIDE(id, table, NULL, NULL)
+
 static const struct __ftr_reg_entry {
 	u32			sys_id;
 	struct arm64_ftr_reg 	*reg;

@@ -760,6 +764,7 @@ static void __init init_cpu_ftr_reg(u32 sys_reg, u64 new)
 	u64 strict_mask = ~0x0ULL;
 	u64 user_mask = 0;
 	u64 valid_mask = 0;
+	u64 override_val = 0, override_mask = 0;
 
 	const struct arm64_ftr_bits *ftrp;
 	struct arm64_ftr_reg *reg = get_arm64_ftr_reg(sys_reg);

@@ -767,9 +772,38 @@ static void __init init_cpu_ftr_reg(u32 sys_reg, u64 new)
 	if (!reg)
 		return;
 
+	if (reg->override_mask && reg->override_val) {
+		override_mask = *reg->override_mask;
+		override_val = *reg->override_val;
+	}
+
 	for (ftrp = reg->ftr_bits; ftrp->width; ftrp++) {
 		u64 ftr_mask = arm64_ftr_mask(ftrp);
 		s64 ftr_new = arm64_ftr_value(ftrp, new);

arch/arm64/kernel/cpufeature.c +6 -0

+				tmp = ftr_ovr;
+				str = "ignoring override";
+			} else if (ftr_new != tmp) {
+				/* Override was valid */
+				ftr_new = tmp;
+				str = "forced";

Add a facility to globally override a feature, no matter what the HW says. Yes, this sounds dangerous, but we do respect the "safe" value for a given feature. This doesn't mean the user doesn't need to know what they are doing.

Nothing uses this yet, so we are pretty safe. For now.

Signed-off-by Marc Zyngier <[email protected]>

__read_sysreg_by_encoding() is used by a bunch of cpufeature helpers, which should take the feature override into account. Let's do that.

For a good measure (and because we are likely to need to further down the line), make this helper available to the rest of the non-modular kernel.

Code that needs to know the *real* features of a CPU can still use read_sysreg_s(), and find the bare, ugly truth.

Signed-off-by Marc Zyngier <[email protected]>

---
 arch/arm64/include/asm/cpufeature.h |  1 +
 arch/arm64/kernel/cpufeature.c      | 15 +++++++++++++--
 2 files changed, 14 insertions(+), 2 deletions(-)

arch/arm64/include/asm/cpufeature.h +1 -0

index 465d2cb63bfc..fe0130d6c0ff 100644
@@ -602,6 +602,7 @@ void __init setup_cpu_features(void);
 void check_local_cpu_capabilities(void);
 
 u64 read_sanitised_ftr_reg(u32 id);
+u64 __read_sysreg_by_encoding(u32 sys_id);
 
 static inline bool cpu_supports_mixed_endian_el0(void)
 {

arch/arm64/kernel/cpufeature.c +5 -2

index aaa075c6f029..48a011935d8c 100644
@@ -1149,14 +1149,17 @@ u64 read_sanitised_ftr_reg(u32 id)
 EXPORT_SYMBOL_GPL(read_sanitised_ftr_reg);
 
 #define read_sysreg_case(r)	\
-	case r:		return read_sysreg_s(r)
+	case r:		val = read_sysreg_s(r); break;
 
 /*
  * __read_sysreg_by_encoding() - Used by a STARTING cpu before cpuinfo is populated.
  * Read the system register on the current CPU
  */
-static u64 __read_sysreg_by_encoding(u32 sys_id)
+u64 __read_sysreg_by_encoding(u32 sys_id)
 {
+	struct arm64_ftr_reg *regp;
+	u64 val;
+
 	switch (sys_id) {
 	read_sysreg_case(SYS_ID_PFR0_EL1);
 	read_sysreg_case(SYS_ID_PFR1_EL1);

@@ -1199,6 +1202,14 @@ static u64 __read_sysreg_by_encoding(u32 sys_id)
 		BUG();

Quoted 38 lines · Marc Zyngier

__read_sysreg_by_encoding() is used by a bunch of cpufeature helpers, which should take the feature override into account. Let's do that.

For a good measure (and because we are likely to need to further down the line), make this helper available to the rest of the non-modular kernel.

Code that needs to know the *real* features of a CPU can still use read_sysreg_s(), and find the bare, ugly truth.

Signed-off-by Marc Zyngier <[email protected]>

arch/arm64/kernel/cpufeature.c +5 -2

index aaa075c6f029..48a011935d8c 100644
@@ -1149,14 +1149,17 @@ u64 read_sanitised_ftr_reg(u32 id)
  EXPORT_SYMBOL_GPL(read_sanitised_ftr_reg);
 
  #define read_sysreg_case(r)	\
-	case r:		return read_sysreg_s(r)
+	case r:		val = read_sysreg_s(r); break;
 
  /*
   * __read_sysreg_by_encoding() - Used by a STARTING cpu before cpuinfo is populated.
   * Read the system register on the current CPU
   */
-static u64 __read_sysreg_by_encoding(u32 sys_id)
+u64 __read_sysreg_by_encoding(u32 sys_id)
  {
+	struct arm64_ftr_reg *regp;
+	u64 val;
+
  	switch (sys_id) {
  	read_sysreg_case(SYS_ID_PFR0_EL1);
  	read_sysreg_case(SYS_ID_PFR1_EL1);

@@ -1199,6 +1202,14 @@ static u64 __read_sysreg_by_encoding(u32 sys_id)
  		BUG();

arch/arm64/kernel/cpufeature.c static u64 __read_sysreg_by_encoding(u32 sys_id) · line 1212

Quoted 10 lines

arch/arm64/kernel/cpufeature.c +8 -0

  		return 0;
  	}
+
+	regp  = get_arm64_ftr_reg(sys_id);
+	if (regp && regp->override_mask && regp->override_val) {
+		val &= ~*regp->override_mask;
+		val |= (*regp->override_val & *regp->override_mask);
+	}
+
+	return val;

Ah, now the previous patch makes more sense. I don't particularly like this but I can't tell how to work around it. I was hoping that the overriding feature behaves more like a secondary CPU that limits all the overridden features. However, this approach would fail for FTR_EXACT cases (like PAC, though I wonder whether it fails already with your previous patch since the boot CPU value won't match the override, hence dropping to the safe one).

As we want to parse more options very early in the kernel lifetime, let's always map the FDT early. This is achieved by moving that code out of kaslr_early_init().

No functionnal change expected.

Signed-off-by Marc Zyngier <[email protected]>

In order to be able to override CPU features at boot time, let's add a command line parser that matches options of the form "cpureg.feature=value", and store the corresponding value into the override val/mask pair.

No features are currently defined, so no expected change in functionality.

Signed-off-by Marc Zyngier <[email protected]>

---
 arch/arm64/kernel/Makefile         |   2 +-
 arch/arm64/kernel/head.S           |   1 +
 arch/arm64/kernel/idreg-override.c | 119 +++++++++++++++++++++++++++++
 3 files changed, 121 insertions(+), 1 deletion(-)
 create mode 100644 arch/arm64/kernel/idreg-override.c

arch/arm64/kernel/Makefile +1 -1

index 86364ab6f13f..2262f0392857 100644
@@ -17,7 +17,7 @@ obj-y			:= debug-monitors.o entry.o irq.o fpsimd.o		\
 			   return_address.o cpuinfo.o cpu_errata.o		\
 			   cpufeature.o alternative.o cacheinfo.o		\
 			   smp.o smp_spin_table.o topology.o smccc-call.o	\
-			   syscall.o proton-pack.o
+			   syscall.o proton-pack.o idreg-override.o
 
 targets			+= efi-entry.o
 

arch/arm64/kernel/head.S +1 -0

index d74e5f84042e..b3c4dd04f74b 100644
@@ -435,6 +435,7 @@ SYM_FUNC_START_LOCAL(__primary_switched)
 
 	mov	x0, x21				// pass FDT address in x0
 	bl	early_fdt_map			// Try mapping the FDT early
+	bl	init_shadow_regs
 	bl	switch_to_vhe
 #if defined(CONFIG_KASAN_GENERIC) || defined(CONFIG_KASAN_SW_TAGS)
 	bl	kasan_early_init

arch/arm64/kernel/idreg-override.c new file +11 -0

new file mode 100644
index 000000000000..392f93b67103
@@ -0,0 +1,119 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Early cpufeature override framework
+ *
+ * Copyright (C) 2020 Google LLC
+ * Author: Marc Zyngier <[email protected]>
+ */
+
+#include <linux/kernel.h>
+#include <linux/libfdt.h>
+

arch/arm64/kernel/idreg-override.c new file +98 -0

+	} 			fields[];
+};
+
+static const struct reg_desc * const regs[] __initdata = {
+};
+
+static int __init find_field(const char *cmdline, const struct reg_desc *reg,
+			     int f, u64 *v)
+{
+	char buf[256], *str;
+	size_t len;
+
+	snprintf(buf, ARRAY_SIZE(buf), "%s.%s=", reg->name, reg->fields[f].name);
+
+	str = strstr(cmdline, buf);
+	if (!(str == cmdline || (str > cmdline && *(str - 1) == ' ')))
+		return -1;
+
+	str += strlen(buf);
+	len = strcspn(str, " ");
+	len = min(len, ARRAY_SIZE(buf) - 1);
+	strncpy(buf, str, len);
+	buf[len] = 0;
+
+	return kstrtou64(buf, 0, v);
+}
+
+static void __init match_options(const char *cmdline)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(regs); i++) {
+		int f;
+
+		if (!regs[i]->val || !regs[i]->mask)
+			continue;
+
+		for (f = 0; regs[i]->fields[f].name; f++) {
+			u64 v;
+
+			if (find_field(cmdline, regs[i], f, &v))
+				continue;
+
+			*regs[i]->val  |= (v & 0xf) << regs[i]->fields[f].shift;
+			*regs[i]->mask |= 0xfUL << regs[i]->fields[f].shift;
+		}
+	}
+}
+
+static __init void parse_cmdline(void)
+{
+	if (!IS_ENABLED(CONFIG_CMDLINE_FORCE)) {
+		const u8 *prop;
+		void *fdt;
+		int node;
+
+		fdt = get_early_fdt_ptr();
+		if (!fdt)
+			goto out;
+
+		node = fdt_path_offset(fdt, "/chosen");
+		if (node < 0)
+			goto out;
+
+		prop = fdt_getprop(fdt, node, "bootargs", NULL);
+		if (!prop)
+			goto out;
+
+		match_options(prop);
+
+		if (!IS_ENABLED(CONFIG_CMDLINE_EXTEND))
+			return;
+	}
+
+out:
+	match_options(CONFIG_CMDLINE);
+}
+
+void __init init_shadow_regs(void)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(regs); i++) {
+		if (regs[i]->val)
+			*regs[i]->val  = 0;
+		if (regs[i]->mask)
+			*regs[i]->mask = 0;
+	}
+
+	parse_cmdline();
+
+	for (i = 0; i < ARRAY_SIZE(regs); i++) {
+		if (regs[i]->val)
+			__flush_dcache_area(regs[i]->val, sizeof(*regs[i]->val));
+		if (regs[i]->mask)
+			__flush_dcache_area(regs[i]->mask, sizeof(*regs[i]->mask));
+	}
+}

Signature · 1 line

2.29.2

arch/arm64/kernel/head.S +1 -0

index d74e5f84042e..b3c4dd04f74b 100644
@@ -435,6 +435,7 @@ SYM_FUNC_START_LOCAL(__primary_switched)
 
 	mov	x0, x21				// pass FDT address in x0
 	bl	early_fdt_map			// Try mapping the FDT early
+	bl	init_shadow_regs
 	bl	switch_to_vhe
 #if defined(CONFIG_KASAN_GENERIC) || defined(CONFIG_KASAN_SW_TAGS)
 	bl	kasan_early_init

+void __init init_shadow_regs(void)

+static const struct reg_desc * const regs[] __initdata = {

arch/arm64/kernel/idreg-override.c line 23

Quoted 9 lines · Marc Zyngier

arch/arm64/kernel/idreg-override.c new file +9 -0

+struct reg_desc {
+	const char * const	name;
+	u64 * const		val;
+	u64 * const		mask;
+	struct {
+		const char * const	name;
+		u8			 shift;
+	} 			fields[];
+};

Sorry, I didn't see this earlier. Do we need to add all these consts here? So you want the pointers to be const but why is 'shift' special and not a const then? Is it modified later?

Would this not work:

struct reg_desc {
	const char	*name;
	u64		*val;
	u64		*mask;
	struct {
		const char	*name;
		u8		shift;
	} fields[];
};

Quoted 1 line

+static const struct reg_desc * const regs[] __initdata = {

as we already declare the whole struct reg_desc pointers here as const. I may have confused myself...

arch/arm64/include/asm/cpufeature.h +3 -0

index fe0130d6c0ff..80a5f423444e 100644
@@ -814,6 +814,9 @@ static inline unsigned int get_vmid_bits(u64 mmfr1)
 	return 8;
 }
 
+extern u64 id_aa64mmfr1_val;
+extern u64 id_aa64mmfr1_mask;
+
 u32 get_kvm_ipa_limit(void);
 void dump_cpu_features(void);
 

arch/arm64/kernel/cpufeature.c +5 -1

index 48a011935d8c..5b9343d2e9f0 100644
@@ -555,6 +555,9 @@ static const struct arm64_ftr_bits ftr_raz[] = {
 
 #define ARM64_FTR_REG(id, table) ARM64_FTR_REG_OVERRIDE(id, table, NULL, NULL)
 
+u64 id_aa64mmfr1_val;
+u64 id_aa64mmfr1_mask;
+
 static const struct __ftr_reg_entry {
 	u32			sys_id;
 	struct arm64_ftr_reg 	*reg;

@@ -602,7 +605,8 @@ static const struct __ftr_reg_entry {
 
 	/* Op1 = 0, CRn = 0, CRm = 7 */
 	ARM64_FTR_REG(SYS_ID_AA64MMFR0_EL1, ftr_id_aa64mmfr0),
-	ARM64_FTR_REG(SYS_ID_AA64MMFR1_EL1, ftr_id_aa64mmfr1),
+	ARM64_FTR_REG_OVERRIDE(SYS_ID_AA64MMFR1_EL1, ftr_id_aa64mmfr1,
+			       &id_aa64mmfr1_val, &id_aa64mmfr1_mask),
 	ARM64_FTR_REG(SYS_ID_AA64MMFR2_EL1, ftr_id_aa64mmfr2),
 
 	/* Op1 = 0, CRn = 1, CRm = 2 */

arch/arm64/kernel/idreg-override.c +4 -0

index 392f93b67103..75d9845f489b 100644
@@ -10,6 +10,7 @@
 #include <linux/libfdt.h>
 
 #include <asm/cacheflush.h>
+#include <asm/cpufeature.h>
 #include <asm/setup.h>
 
 struct reg_desc {

@@ -22,7 +23,18 @@ struct reg_desc {
 	} 			fields[];
 };
 
+static const struct reg_desc mmfr1 __initdata = {
+	.name		= "id_aa64mmfr1",
+	.val		= &id_aa64mmfr1_val,

Finally we can check whether VHE is disabled on the command line, and not enable it if that's the user's wish.

Signed-off-by Marc Zyngier <[email protected]>

---
 arch/arm64/kernel/hyp-stub.S | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

arch/arm64/kernel/hyp-stub.S +1 -1

index 59820f9b8522..bbab2148a2a2 100644
@@ -77,13 +77,24 @@ SYM_CODE_END(el1_sync)
 SYM_CODE_START_LOCAL(mutate_to_vhe)
 	// Sanity check: MMU *must* be off
 	mrs	x0, sctlr_el2
-	tbnz	x0, #0, 1f
+	tbnz	x0, #0, 2f
 
 	// Needs to be VHE capable, obviously

arch/arm64/kernel/hyp-stub.S +7 -1

+	ubfx	x0, x0, #ID_AA64MMFR1_VHE_SHIFT, #4
+	ubfx	x1, x1, #ID_AA64MMFR1_VHE_SHIFT, #4
+	cbz	x1, 1f
+	and	x0, x0, x1
+	cbz	x0, 2f
+1:
 	// Engage the VHE magic!
 	mov_q	x0, HCR_HOST_VHE_FLAGS
 	msr	hcr_el2, x0

@@ -152,7 +163,7 @@ skip_spe:
 	orr	x0, x0, x1
 	msr	spsr_el1, x0
 
-1:	eret
+2:	eret
 SYM_CODE_END(mutate_to_vhe)
 
 .macro invalid_vector	label

Signature · 1 line

2.29.2

arch/arm64/kernel/hyp-stub.S +3 -2

index 59820f9b8522..bbab2148a2a2 100644
@@ -77,13 +77,24 @@ SYM_CODE_END(el1_sync)
 SYM_CODE_START_LOCAL(mutate_to_vhe)
 	// Sanity check: MMU *must* be off
 	mrs	x0, sctlr_el2
-	tbnz	x0, #0, 1f
+	tbnz	x0, #0, 2f
 
 	// Needs to be VHE capable, obviously
 	mrs	x0, id_aa64mmfr1_el1
 	ubfx	x0, x0, #ID_AA64MMFR1_VHE_SHIFT, #4
-	cbz	x0, 1f
+	cbz	x0, 2f
 
+	// Check whether VHE is disabled from the command line

Quoted 18 lines · Marc Zyngier

arch/arm64/kernel/hyp-stub.S +3 -2

index 59820f9b8522..bbab2148a2a2 100644
@@ -77,13 +77,24 @@ SYM_CODE_END(el1_sync)
 SYM_CODE_START_LOCAL(mutate_to_vhe)
 	// Sanity check: MMU *must* be off
 	mrs	x0, sctlr_el2
-	tbnz	x0, #0, 1f
+	tbnz	x0, #0, 2f
 
 	// Needs to be VHE capable, obviously
 	mrs	x0, id_aa64mmfr1_el1
 	ubfx	x0, x0, #ID_AA64MMFR1_VHE_SHIFT, #4
-	cbz	x0, 1f
+	cbz	x0, 2f
 
+	// Check whether VHE is disabled from the command line

arch/arm64/kernel/hyp-stub.S SYM_CODE_END(el1_sync) · line 97

Quoted 10 lines

arch/arm64/kernel/hyp-stub.S +10 -0

+	adr_l	x1, id_aa64mmfr1_val
+	ldr	x0, [x1]
+	adr_l	x1, id_aa64mmfr1_mask
+	ldr	x1, [x1]
+	ubfx	x0, x0, #ID_AA64MMFR1_VHE_SHIFT, #4
+	ubfx	x1, x1, #ID_AA64MMFR1_VHE_SHIFT, #4
+	cbz	x1, 1f
+	and	x0, x0, x1
+	cbz	x0, 2f
+1:

I can see the advantage here in separate id_aa64mmfr1_val/mask but we could use some asm offsets here and keep the pointer indirection simpler in C code. You'd just need something like 'adr_l mmfr1_ovrd + VAL_OFFSET'.

Anyway, if you have a strong preference for the current approach, leave it as is.

In order to map the override of idregs to options that a user can easily understand, let's introduce yet another option array, which maps an option to the corresponding idreg options.

Signed-off-by Marc Zyngier <[email protected]>

---
 arch/arm64/kernel/idreg-override.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

arch/arm64/kernel/idreg-override.c +6 -0

index 75d9845f489b..16bc8b3b93ae 100644
@@ -37,6 +37,12 @@ static const struct reg_desc * const regs[] __initdata = {
 	&mmfr1,
 };
 
+static const struct {
+	const char * const	alias;
+	const char * const	feature;
+} aliases[] __initdata = {
+};
+
 static int __init find_field(const char *cmdline, const struct reg_desc *reg,
 			     int f, u64 *v)
 {

@@ -80,6 +86,18 @@ static void __init match_options(const char *cmdline)
 	}

arch/arm64/kernel/idreg-override.c +6 -0

+			match_options(aliases[i].feature);
+	}
+}
+
 static __init void parse_cmdline(void)
 {
 	if (!IS_ENABLED(CONFIG_CMDLINE_FORCE)) {

@@ -100,6 +118,7 @@ static __init void parse_cmdline(void)
 			goto out;
 
 		match_options(prop);
+		match_aliases(prop);
 
 		if (!IS_ENABLED(CONFIG_CMDLINE_EXTEND))
 			return;

@@ -107,6 +126,7 @@ static __init void parse_cmdline(void)
 
 out:
 	match_options(CONFIG_CMDLINE);
+	match_aliases(CONFIG_CMDLINE);
 }
 
 void __init init_shadow_regs(void)

Signature · 1 line

2.29.2

Quoted 5 lines · Marc Zyngier

In order to map the override of idregs to options that a user can easily understand, let's introduce yet another option array, which maps an option to the corresponding idreg options.

Signed-off-by Marc Zyngier <[email protected]>

Acked-by David Brazdil <[email protected]>

Quoted 25 lines

---
 arch/arm64/kernel/idreg-override.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

arch/arm64/kernel/idreg-override.c +6 -0

index 75d9845f489b..16bc8b3b93ae 100644
@@ -37,6 +37,12 @@ static const struct reg_desc * const regs[] __initdata = {
 	&mmfr1,
 };
 
+static const struct {
+	const char * const	alias;
+	const char * const	feature;
+} aliases[] __initdata = {
+};
+
 static int __init find_field(const char *cmdline, const struct reg_desc *reg,
 			     int f, u64 *v)
 {

@@ -80,6 +86,18 @@ static void __init match_options(const char *cmdline)
 	}
 }
 

arch/arm64/kernel/idreg-override.c static void __init match_options(const char *cmdline) · line 96

Quoted 8 lines

arch/arm64/kernel/idreg-override.c +8 -0

+static __init void match_aliases(const char *cmdline)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(aliases); i++) {
+		char *str = strstr(cmdline, aliases[i].alias);
+
+		if ((str == cmdline || (str > cmdline && *(str - 1) == ' ')))

nit: Extract to a 'cmdline_contains' helper? Took me a good few seconds to parse this in the previous patch. Giving it a name would help, and now it's also shared.

Admitedly, passing id_aa64mmfr1.vh=0 on the command-line isn't that easy to understand, and it is likely that users would much prefer write "kvm-arm.mode=nvhe", or "...=protected".

So here you go. This has the added advantage that we can now always honor the "kvm-arm.mode=protected" option, even when booting on a VHE system.

Signed-off-by Marc Zyngier <[email protected]>

For completeness, let's document the HVC_VHE_RESTART stub.

Signed-off-by Marc Zyngier <[email protected]>

---
 Documentation/virt/kvm/arm/hyp-abi.rst | 9 +++++++++
 1 file changed, 9 insertions(+)

Documentation/virt/kvm/arm/hyp-abi.rst +0 -0

index 83cadd8186fa..1ba628baf11b 100644

Documentation/virt/kvm/arm/hyp-abi.rst +2 -0

+  for example).
+
 Any other value of r0/x0 triggers a hypervisor-specific handling,
 which is not documented here.
 

Signature · 1 line

2.29.2

Given that the early cpufeature infrastructure has borrowed quite a lot of code from the kaslr implementation, let's reimplement the matching of the "nokaslr" option with it.

Signed-off-by Marc Zyngier <[email protected]>

---
 arch/arm64/kernel/idreg-override.c | 17 ++++++++++++++
 arch/arm64/kernel/kaslr.c          | 37 +++---------------------------
 2 files changed, 20 insertions(+), 34 deletions(-)

arch/arm64/kernel/idreg-override.c +17 -0

index 1db54878b2c4..143fe7b8e3ce 100644
@@ -33,8 +33,24 @@ static const struct reg_desc mmfr1 __initdata = {
 	},
 };
 
+extern u64 kaslr_feature_val;
+extern u64 kaslr_feature_mask;
+
+static const struct reg_desc kaslr __initdata = {
+	.name		= "kaslr",
+#ifdef CONFIG_RANDOMIZE_BASE
+	.val		= &kaslr_feature_val,
+	.mask		= &kaslr_feature_mask,
+#endif
+	.fields		= {
+		{ "disabled", 0 },
+		{}
+	},
+};
+
 static const struct reg_desc * const regs[] __initdata = {
 	&mmfr1,
+	&kaslr,
 };
 
 static const struct {

@@ -43,6 +59,7 @@ static const struct {
 } aliases[] __initdata = {
 	{ "kvm-arm.mode=nvhe",		"id_aa64mmfr1.vh=0" },
 	{ "kvm-arm.mode=protected",	"id_aa64mmfr1.vh=0" },
+	{ "nokaslr",			"kaslr.disabled=1" },
 };
 
 static int __init find_field(const char *cmdline, const struct reg_desc *reg,

arch/arm64/kernel/kaslr.c +2 -33

index 5fc86e7d01a1..dcc4a5aadbe2 100644
@@ -51,39 +51,8 @@ static __init u64 get_kaslr_seed(void *fdt)
 	return ret;
 }
 
-static __init bool cmdline_contains_nokaslr(const u8 *cmdline)
-{
-	const u8 *str;
-
-	str = strstr(cmdline, "nokaslr");
-	return str == cmdline || (str > cmdline && *(str - 1) == ' ');
-}
-
-static __init bool is_kaslr_disabled_cmdline(void *fdt)
-{
-	if (!IS_ENABLED(CONFIG_CMDLINE_FORCE)) {
-		int node;
-		const u8 *prop;
-
-		node = fdt_path_offset(fdt, "/chosen");
-		if (node < 0)
-			goto out;
-
-		prop = fdt_getprop(fdt, node, "bootargs", NULL);
-		if (!prop)
-			goto out;
-
-		if (cmdline_contains_nokaslr(prop))
-			return true;
-
-		if (IS_ENABLED(CONFIG_CMDLINE_EXTEND))
-			goto out;
-
-		return false;
-	}
-out:
-	return cmdline_contains_nokaslr(CONFIG_CMDLINE);
-}
+u64 kaslr_feature_val __initdata;
+u64 kaslr_feature_mask __initdata;
 
 /*
  * This routine will be executed with the kernel mapped at its default virtual

arch/arm64/kernel/kaslr.c +0 -0

 		kaslr_status = KASLR_DISABLED_CMDLINE;
 		return 0;
 	}

Signature · 1 line

2.29.2

Quoted 5 lines · Marc Zyngier

Given that the early cpufeature infrastructure has borrowed quite a lot of code from the kaslr implementation, let's reimplement the matching of the "nokaslr" option with it.

Signed-off-by Marc Zyngier <[email protected]>

Acked-by David Brazdil <[email protected]>

arch/arm64/kernel/kaslr.c u64 __init kaslr_early_init(void) · line 98

Quoted 6 lines

arch/arm64/kernel/kaslr.c +1 -1

@@ -126,7 +95,7 @@ u64 __init kaslr_early_init(void)
 	 * Check if 'nokaslr' appears on the command line, and
 	 * return 0 if that is the case.
 	 */
-	if (is_kaslr_disabled_cmdline(fdt)) {
+	if (kaslr_feature_val & kaslr_feature_mask & 0xf) {

nit: Isn't the 0xf redundant here? You don't re-mask for VH either.

Given that the early cpufeature infrastructure has borrowed quite a lot of code from the kaslr implementation, let's reimplement the matching of the "nokaslr" option with it.

Signed-off-by Marc Zyngier <[email protected]>

---
 arch/arm64/kernel/idreg-override.c | 17 ++++++++++++++
 arch/arm64/kernel/kaslr.c          | 37 +++---------------------------
 2 files changed, 20 insertions(+), 34 deletions(-)

arch/arm64/kernel/idreg-override.c +0 -0

index 1db54878b2c4..143fe7b8e3ce 100644

In order to be able to disable BTI at runtime, whether it is for testing purposes, or to work around HW issues, let's add support for overriding the ID_AA64PFR1_EL1.BTI field.

This is further mapped on the arm64.nobti command-line alias.

Signed-off-by Marc Zyngier <[email protected]>

Quoted 7 lines · Marc Zyngier

In order to be able to disable BTI at runtime, whether it is for testing purposes, or to work around HW issues, let's add support for overriding the ID_AA64PFR1_EL1.BTI field.

This is further mapped on the arm64.nobti command-line alias.

Signed-off-by Marc Zyngier <[email protected]>

Reviewed-by Catalin Marinas <[email protected]>

From: Srinivas Ramana <[email protected]>

Defer enabling pointer authentication on boot core until after its required to be enabled by cpufeature framework. This will help in controlling the feature dynamically with a boot parameter.

Signed-off-by Ajay Patil <[email protected]>

Signed-off-by Prasad Sodagudi <[email protected]>

Signed-off-by Srinivas Ramana <[email protected]>

Signed-off-by Marc Zyngier <[email protected]>

Link https://lore.kernel.org/r/[email protected] open here

In order to be able to disable Pointer Authentication at runtime, whether it is for testing purposes, or to work around HW issues, let's add support for overriding the ID_AA64ISAR1_EL1.{GPI,GPA,API,APA} fields.

This is further mapped on the arm64.nopauth command-line alias.

Signed-off-by Marc Zyngier <[email protected]>

Quoted 8 lines · Marc Zyngier

In order to be able to disable Pointer Authentication at runtime, whether it is for testing purposes, or to work around HW issues, let's add support for overriding the ID_AA64ISAR1_EL1.{GPI,GPA,API,APA} fields.

This is further mapped on the arm64.nopauth command-line alias.

Signed-off-by Marc Zyngier <[email protected]>

Reviewed-by Catalin Marinas <[email protected]>

It recently came to light that there is a need to be able to override some CPU features very early on, before the kernel is fully up and running. The reasons for this range from specific feature support (such as using Protected KVM on VHE HW, which is the main motivation for this work) to errata workaround (a feature is broken on a CPU and needs to be turned off, or rather not enabled).

This series tries to offer a limited framework for this kind of problems, by allowing a set of options to be passed on the command-line and altering the feature set that the cpufeature subsystem exposes to the rest of the kernel. Note that this doesn't change anything for code that directly uses the CPU ID registers.

The series completely changes the way a VHE-capable system boots, by *always* booting non-VHE first, and then upgrading to VHE when deemed capable. Although it sounds scary, this is actually simple to implement (and I wish I had done that five years ago). The "upgrade to VHE" path is then conditioned on the VHE feature not being disabled from the command-line.

Said command-line parsing borrows a lot from the kaslr code, and subsequently allows the "nokaslr" option to be moved to the new infrastructure (though it all looks a bit... odd).

Further patches now add support for disabling BTI and PAuth, the latter being based on an initial series by Srinivas Ramana[0]. There is some ongoing discussions about being able to disable MTE, but no clear resolution on that subject yet.

This has been tested on multiple VHE and non-VHE systems.

* From v3 [3]:
- Fixed the VHE_RESTART stub (duh!)
- Switched to using arm64_ftr_safe_value() instead of the user provided value
- Per-feature override warning

* From v2 [2]:
- Simplify the VHE_RESTART stub
- Fixed a number of spelling mistakes, and hopefully introduced a few more
- Override features in __read_sysreg_by_encoding()
- Allow both BTI and PAuth to be overridden on the command line
- Rebased on -rc3

* From v1 [1]:
- Fix SPE init on VHE when EL2 doesn't own SPE
- Fix re-init when KASLR is used
- Handle the resume path
- Rebased to 5.11-rc2

[0] https://lore.kernel.org/r/[email protected]
[1] https://lore.kernel.org/r/[email protected]
[2] https://lore.kernel.org/r/[email protected]
[3] https://lore.kernel.org/r/[email protected]