diff mbox series

[v4,12/21] arm64: cpufeature: Add an early command-line cpufeature override facility

Message ID 20210118094533.2874082-13-maz@kernel.org (mailing list archive)
State New, archived
Headers show
Series arm64: Early CPU feature override, and applications to VHE, BTI and PAuth | expand

Commit Message

Marc Zyngier Jan. 18, 2021, 9:45 a.m. UTC 
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 <maz@kernel.org>
---
 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

Comments

David Brazdil Jan. 18, 2021, 1:07 p.m. UTC | #1 
On Mon, Jan 18, 2021 at 09:45:24AM +0000, Marc Zyngier wrote:
> 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 <maz@kernel.org>

Acked-by: David Brazdil <dbrazdil@google.com>

> ---
>  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
> 
> diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
> index 86364ab6f13f..2262f0392857 100644
> --- a/arch/arm64/kernel/Makefile
> +++ b/arch/arm64/kernel/Makefile
> @@ -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
>  
> diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
> index d74e5f84042e..b3c4dd04f74b 100644
> --- a/arch/arm64/kernel/head.S
> +++ b/arch/arm64/kernel/head.S
> @@ -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
> diff --git a/arch/arm64/kernel/idreg-override.c b/arch/arm64/kernel/idreg-override.c
> new file mode 100644
> index 000000000000..392f93b67103
> --- /dev/null
> +++ b/arch/arm64/kernel/idreg-override.c
> @@ -0,0 +1,119 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Early cpufeature override framework
> + *
> + * Copyright (C) 2020 Google LLC
> + * Author: Marc Zyngier <maz@kernel.org>
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/libfdt.h>
> +
> +#include <asm/cacheflush.h>
> +#include <asm/setup.h>
> +
> +struct reg_desc {
> +	const char * const	name;
> +	u64 * const		val;
> +	u64 * const		mask;
> +	struct {
> +		const char * const	name;
> +		u8			 shift;
nit: There's an extra space before `shift`.

> +	} 			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));
> +	}
> +}
> -- 
> 2.29.2
>
Catalin Marinas Jan. 23, 2021, 1:23 p.m. UTC | #2 
On Mon, Jan 18, 2021 at 09:45:24AM +0000, Marc Zyngier wrote:
> diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
> index d74e5f84042e..b3c4dd04f74b 100644
> --- a/arch/arm64/kernel/head.S
> +++ b/arch/arm64/kernel/head.S
> @@ -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)

Do we need an asmlinkage here? And a declaration somewhere to silence
sparse (we did this for entry-common.c function even if the .S files
don't consume the prototypes).
Catalin Marinas Jan. 23, 2021, 1:43 p.m. UTC | #3 
On Mon, Jan 18, 2021 at 09:45:24AM +0000, Marc Zyngier wrote:
> +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[];
};

> +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...
Marc Zyngier Jan. 24, 2021, 4:21 p.m. UTC | #4 
On Sat, 23 Jan 2021 13:43:52 +0000,
Catalin Marinas <catalin.marinas@arm.com> wrote:
> 
> On Mon, Jan 18, 2021 at 09:45:24AM +0000, Marc Zyngier wrote:
> > +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[];
> };
> 
> > +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...

It definitely is better. Specially given that we throw all of this
away right after boot, there is no harm in keeping it simple.

I've also renamed "reg_desc" to "ftr_set_desc", because we don't
always describe a register (like for kaslr).

Thanks,

	M.
diff mbox series

Patch

diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
index 86364ab6f13f..2262f0392857 100644
--- a/arch/arm64/kernel/Makefile
+++ b/arch/arm64/kernel/Makefile
@@ -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
 
diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
index d74e5f84042e..b3c4dd04f74b 100644
--- a/arch/arm64/kernel/head.S
+++ b/arch/arm64/kernel/head.S
@@ -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
diff --git a/arch/arm64/kernel/idreg-override.c b/arch/arm64/kernel/idreg-override.c
new file mode 100644
index 000000000000..392f93b67103
--- /dev/null
+++ b/arch/arm64/kernel/idreg-override.c
@@ -0,0 +1,119 @@ 
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Early cpufeature override framework
+ *
+ * Copyright (C) 2020 Google LLC
+ * Author: Marc Zyngier <maz@kernel.org>
+ */
+
+#include <linux/kernel.h>
+#include <linux/libfdt.h>
+
+#include <asm/cacheflush.h>
+#include <asm/setup.h>
+
+struct reg_desc {
+	const char * const	name;
+	u64 * const		val;
+	u64 * const		mask;
+	struct {
+		const char * const	name;
+		u8			 shift;
+	} 			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));
+	}
+}