Re: [PATCH v6 12/21] arm64: cpufeature: Add an early command-line cpufeature override facility

From: Marc Zyngier
Date: Fri Feb 05 2021 - 15:37:16 EST


On 2021-02-05 16:35, Will Deacon wrote:
On Mon, Feb 01, 2021 at 11:56:28AM +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@xxxxxxxxxx>
Acked-by: David Brazdil <dbrazdil@xxxxxxxxxx>
Reviewed-by: Catalin Marinas <catalin.marinas@xxxxxxx>
---
arch/arm64/kernel/Makefile | 2 +-
arch/arm64/kernel/head.S | 1 +
arch/arm64/kernel/idreg-override.c | 164 +++++++++++++++++++++++++++++
3 files changed, 166 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..3243e3ae9bd8 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_feature_override
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..d8d0d3b25bc3
--- /dev/null
+++ b/arch/arm64/kernel/idreg-override.c
@@ -0,0 +1,164 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Early cpufeature override framework
+ *
+ * Copyright (C) 2020 Google LLC
+ * Author: Marc Zyngier <maz@xxxxxxxxxx>
+ */
+
+#include <linux/ctype.h>
+#include <linux/kernel.h>
+#include <linux/libfdt.h>
+
+#include <asm/cacheflush.h>
+#include <asm/setup.h>
+
+#define FTR_DESC_NAME_LEN 20
+#define FTR_DESC_FIELD_LEN 10
+
+struct ftr_set_desc {
+ char name[FTR_DESC_NAME_LEN];
+ struct arm64_ftr_override *override;
+ struct {
+ char name[FTR_DESC_FIELD_LEN];
+ u8 shift;
+ } fields[];
+};
+
+static const struct ftr_set_desc * const regs[] __initconst = {
+};
+
+static char *cmdline_contains_option(const char *cmdline, const char *option)
+{
+ char *str = strstr(cmdline, option);
+
+ if ((str == cmdline || (str > cmdline && isspace(*(str - 1)))))
+ return str;
+
+ return NULL;
+}
+
+static int __init find_field(const char *cmdline,
+ const struct ftr_set_desc *reg, int f, u64 *v)
+{
+ char opt[FTR_DESC_NAME_LEN + FTR_DESC_FIELD_LEN + 2], *str;
+ size_t len;
+
+ snprintf(opt, ARRAY_SIZE(opt), "%s.%s=", reg->name, reg->fields[f].name);
+
+ str = cmdline_contains_option(cmdline, opt);
+ if (!str)
+ return -1;
+
+ str += strlen(opt);
+ len = strcspn(str, " ");

I'm absolutely terrified of string parsing in C, but just wondering why you
only ignore literal spaces here. I _think_ the full-fat cmdline parsing uses
isspace() to delimit the options.

That's clearly an oversight, as I use a more complete set of characters
for the rest of the option splicing. I also think that with the way
things are now parsed (options being extracted early and trimmed),
we can drop this altogether.

Would it be possible to reuse any of the logic over in parse_args() to avoid
having to roll this ourselves?

Maybe. I need to have a look.

Thanks,

M.
--
Jazz is not dead. It just smells funny...