public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH RESEND v2 0/3] x86/cpu: Add facility to force-enable CPU caps and bugs
@ 2025-01-29 15:35 Brendan Jackman
  2025-01-29 15:35 ` [PATCH RESEND v2 1/3] x86/cpu: Create helper to parse clearcpuid param Brendan Jackman
                   ` (2 more replies)
  0 siblings, 3 replies; 13+ messages in thread
From: Brendan Jackman @ 2025-01-29 15:35 UTC (permalink / raw)
  To: Jonathan Corbet, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	Dave Hansen, x86, H. Peter Anvin, Peter Zijlstra
  Cc: linux-doc, linux-kernel, Brendan Jackman

For testing, development, and experimentation, add the ability to force
the kernel to behave as if the CPU has a bug, even if it doesn't, using
a command-line param.

Also do this in general for CPU flags, since:

 - The infrastructure is the same so there is almost no extra
   implementation complexity.

 - While setting random CPU flags is certain to break the kernel in
   mysterious and horrifying ways, this is not dramatically worse than
   setting CPU bugs. Although CPU bug mitigations don't have any very
   obvious ways to break the system if run on the wrong hardware, it's
   still very much an unsupported configuration, even beyond the
   security concern implied breaking mitigation logic.

   Since a taint and scary docs are necessary regardless, supporting
   arbitrary CPU flags doesn't add significant maintenance/support
   burden either.

Signed-off-by: Brendan Jackman <jackmanb@google.com>
---
Changes in v2:
- Switched from a bugs-only force_cpu_bug= to a more general setcpuid=.
- Made it taint the kernel.
- Made docs sound scarier.
- Spellchecked and avoided new usage of personal pronouns.
- Link to v1: https://lore.kernel.org/r/20241119-force-cpu-bug-v1-1-2aa31c6c1ccf@google.com

---
Brendan Jackman (3):
      x86/cpu: Create helper to parse clearcpuid param
      x86/cpu: Add setcpuid cmdline param
      x86/cpu: Enable modifying bug flags with {clear,set}puid

 arch/x86/include/asm/cpufeature.h |   1 +
 arch/x86/kernel/cpu/common.c      | 139 +++++++++++++++++++++++---------------
 2 files changed, 87 insertions(+), 53 deletions(-)
---
base-commit: eabcdba3ad4098460a376538df2ae36500223c1e
change-id: 20241119-force-cpu-bug-94a08ab0239f

Best regards,
-- 
Brendan Jackman <jackmanb@google.com>


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

* [PATCH RESEND v2 1/3] x86/cpu: Create helper to parse clearcpuid param
  2025-01-29 15:35 [PATCH RESEND v2 0/3] x86/cpu: Add facility to force-enable CPU caps and bugs Brendan Jackman
@ 2025-01-29 15:35 ` Brendan Jackman
  2025-02-17 10:59   ` Borislav Petkov
  2025-01-29 15:35 ` [PATCH RESEND v2 2/3] x86/cpu: Add setcpuid cmdline param Brendan Jackman
  2025-01-29 15:35 ` [PATCH RESEND v2 3/3] x86/cpu: Enable modifying bug flags with {clear,set}puid Brendan Jackman
  2 siblings, 1 reply; 13+ messages in thread
From: Brendan Jackman @ 2025-01-29 15:35 UTC (permalink / raw)
  To: Jonathan Corbet, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	Dave Hansen, x86, H. Peter Anvin, Peter Zijlstra
  Cc: linux-doc, linux-kernel, Brendan Jackman

This is in preparation for a later commit that will reuse this code, to
make review convenient.

Factor out a helper function which does the full handling for this arg
including printing info to the console.

No functional change intended.

Signed-off-by: Brendan Jackman <jackmanb@google.com>
---
 arch/x86/kernel/cpu/common.c | 96 ++++++++++++++++++++++++--------------------
 1 file changed, 52 insertions(+), 44 deletions(-)

diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 3e9037690814b331b3433a4abdecc25368c2a662..87ea1a6f7835592e560aae3442bbea881123ac64 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -1477,56 +1477,18 @@ static void detect_nopl(void)
 #endif
 }
 
-/*
- * We parse cpu parameters early because fpu__init_system() is executed
- * before parse_early_param().
- */
-static void __init cpu_parse_early_param(void)
+static inline void parse_clearcpuid(char *arg)
 {
-	char arg[128];
-	char *argptr = arg, *opt;
-	int arglen, taint = 0;
-
-#ifdef CONFIG_X86_32
-	if (cmdline_find_option_bool(boot_command_line, "no387"))
-#ifdef CONFIG_MATH_EMULATION
-		setup_clear_cpu_cap(X86_FEATURE_FPU);
-#else
-		pr_err("Option 'no387' required CONFIG_MATH_EMULATION enabled.\n");
-#endif
-
-	if (cmdline_find_option_bool(boot_command_line, "nofxsr"))
-		setup_clear_cpu_cap(X86_FEATURE_FXSR);
-#endif
-
-	if (cmdline_find_option_bool(boot_command_line, "noxsave"))
-		setup_clear_cpu_cap(X86_FEATURE_XSAVE);
-
-	if (cmdline_find_option_bool(boot_command_line, "noxsaveopt"))
-		setup_clear_cpu_cap(X86_FEATURE_XSAVEOPT);
-
-	if (cmdline_find_option_bool(boot_command_line, "noxsaves"))
-		setup_clear_cpu_cap(X86_FEATURE_XSAVES);
-
-	if (cmdline_find_option_bool(boot_command_line, "nousershstk"))
-		setup_clear_cpu_cap(X86_FEATURE_USER_SHSTK);
-
-	/* Minimize the gap between FRED is available and available but disabled. */
-	arglen = cmdline_find_option(boot_command_line, "fred", arg, sizeof(arg));
-	if (arglen != 2 || strncmp(arg, "on", 2))
-		setup_clear_cpu_cap(X86_FEATURE_FRED);
-
-	arglen = cmdline_find_option(boot_command_line, "clearcpuid", arg, sizeof(arg));
-	if (arglen <= 0)
-		return;
+	char *opt;
+	int taint = 0;
 
 	pr_info("Clearing CPUID bits:");
 
-	while (argptr) {
+	while (arg) {
 		bool found __maybe_unused = false;
 		unsigned int bit;
 
-		opt = strsep(&argptr, ",");
+		opt = strsep(&arg, ",");
 
 		/*
 		 * Handle naked numbers first for feature flags which don't
@@ -1568,10 +1530,56 @@ static void __init cpu_parse_early_param(void)
 		if (!found)
 			pr_cont(" (unknown: %s)", opt);
 	}
-	pr_cont("\n");
 
 	if (taint)
 		add_taint(TAINT_CPU_OUT_OF_SPEC, LOCKDEP_STILL_OK);
+
+	pr_cont("\n");
+}
+
+
+/*
+ * We parse cpu parameters early because fpu__init_system() is executed
+ * before parse_early_param().
+ */
+static void __init cpu_parse_early_param(void)
+{
+	char arg[128];
+	int arglen;
+
+#ifdef CONFIG_X86_32
+	if (cmdline_find_option_bool(boot_command_line, "no387"))
+#ifdef CONFIG_MATH_EMULATION
+		setup_clear_cpu_cap(X86_FEATURE_FPU);
+#else
+		pr_err("Option 'no387' required CONFIG_MATH_EMULATION enabled.\n");
+#endif
+
+	if (cmdline_find_option_bool(boot_command_line, "nofxsr"))
+		setup_clear_cpu_cap(X86_FEATURE_FXSR);
+#endif
+
+	if (cmdline_find_option_bool(boot_command_line, "noxsave"))
+		setup_clear_cpu_cap(X86_FEATURE_XSAVE);
+
+	if (cmdline_find_option_bool(boot_command_line, "noxsaveopt"))
+		setup_clear_cpu_cap(X86_FEATURE_XSAVEOPT);
+
+	if (cmdline_find_option_bool(boot_command_line, "noxsaves"))
+		setup_clear_cpu_cap(X86_FEATURE_XSAVES);
+
+	if (cmdline_find_option_bool(boot_command_line, "nousershstk"))
+		setup_clear_cpu_cap(X86_FEATURE_USER_SHSTK);
+
+	/* Minimize the gap between FRED is available and available but disabled. */
+	arglen = cmdline_find_option(boot_command_line, "fred", arg, sizeof(arg));
+	if (arglen != 2 || strncmp(arg, "on", 2))
+		setup_clear_cpu_cap(X86_FEATURE_FRED);
+
+	arglen = cmdline_find_option(boot_command_line, "clearcpuid", arg, sizeof(arg));
+	if (arglen <= 0)
+		return;
+	parse_clearcpuid(arg);
 }
 
 /*

-- 
2.48.1.262.g85cc9f2d1e-goog


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

* [PATCH RESEND v2 2/3] x86/cpu: Add setcpuid cmdline param
  2025-01-29 15:35 [PATCH RESEND v2 0/3] x86/cpu: Add facility to force-enable CPU caps and bugs Brendan Jackman
  2025-01-29 15:35 ` [PATCH RESEND v2 1/3] x86/cpu: Create helper to parse clearcpuid param Brendan Jackman
@ 2025-01-29 15:35 ` Brendan Jackman
  2025-02-17 11:03   ` Borislav Petkov
  2025-01-29 15:35 ` [PATCH RESEND v2 3/3] x86/cpu: Enable modifying bug flags with {clear,set}puid Brendan Jackman
  2 siblings, 1 reply; 13+ messages in thread
From: Brendan Jackman @ 2025-01-29 15:35 UTC (permalink / raw)
  To: Jonathan Corbet, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	Dave Hansen, x86, H. Peter Anvin, Peter Zijlstra
  Cc: linux-doc, linux-kernel, Brendan Jackman

In preparation for adding support to fake out CPU bugs, add a general
facility to force enablement of CPU flags.

The flag taints the kernel and the documentation attempts to be clear
that this is highly unsuitable for uses outside of kernel development
and platform experimentation.

The new arg is parsed just like clearcpuid, but instead of leading to
setup_clear_cpu_cap() it leads to setup_force_cpu_cap().

I've tested this by booting a nested QEMU guest on an Intel host, which
with setcpuid=svm will claim that it supports AMD virtualization.

Signed-off-by: Brendan Jackman <jackmanb@google.com>
---
 arch/x86/kernel/cpu/common.c | 35 ++++++++++++++++++++++++++---------
 1 file changed, 26 insertions(+), 9 deletions(-)

diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 87ea1a6f7835592e560aae3442bbea881123ac64..e26cf8789f0e1a27ad126f531e05afee0fdebbb8 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -1477,12 +1477,12 @@ static void detect_nopl(void)
 #endif
 }
 
-static inline void parse_clearcpuid(char *arg)
+static inline void parse_set_clear_cpuid(char *arg, bool set)
 {
 	char *opt;
 	int taint = 0;
 
-	pr_info("Clearing CPUID bits:");
+	pr_info("%s CPUID bits:", set ? "Force-enabling" : "Clearing");
 
 	while (arg) {
 		bool found __maybe_unused = false;
@@ -1503,7 +1503,10 @@ static inline void parse_clearcpuid(char *arg)
 				else
 					pr_cont(" " X86_CAP_FMT, x86_cap_flag(bit));
 
-				setup_clear_cpu_cap(bit);
+				if (set)
+					setup_force_cpu_cap(bit);
+				else
+					setup_clear_cpu_cap(bit);
 				taint++;
 			}
 			/*
@@ -1521,7 +1524,10 @@ static inline void parse_clearcpuid(char *arg)
 				continue;
 
 			pr_cont(" %s", opt);
-			setup_clear_cpu_cap(bit);
+			if (set)
+				setup_force_cpu_cap(bit);
+			else
+				setup_clear_cpu_cap(bit);
 			taint++;
 			found = true;
 			break;
@@ -1577,9 +1583,12 @@ static void __init cpu_parse_early_param(void)
 		setup_clear_cpu_cap(X86_FEATURE_FRED);
 
 	arglen = cmdline_find_option(boot_command_line, "clearcpuid", arg, sizeof(arg));
-	if (arglen <= 0)
-		return;
-	parse_clearcpuid(arg);
+	if (arglen > 0)
+		parse_set_clear_cpuid(arg, false);
+
+	arglen = cmdline_find_option(boot_command_line, "setcpuid", arg, sizeof(arg));
+	if (arglen > 0)
+		parse_set_clear_cpuid(arg, true);
 }
 
 /*
@@ -2011,15 +2020,23 @@ void print_cpu_info(struct cpuinfo_x86 *c)
 }
 
 /*
- * clearcpuid= was already parsed in cpu_parse_early_param().  This dummy
- * function prevents it from becoming an environment variable for init.
+ * clearcpuid= and setcpuid= were already parsed in cpu_parse_early_param().
+ * These dummy functions prevent them from becoming an environment variable for
+ * init.
  */
+
 static __init int setup_clearcpuid(char *arg)
 {
 	return 1;
 }
 __setup("clearcpuid=", setup_clearcpuid);
 
+static __init int setup_setcpuid(char *arg)
+{
+	return 1;
+}
+__setup("setcpuid=", setup_setcpuid);
+
 DEFINE_PER_CPU_ALIGNED(struct pcpu_hot, pcpu_hot) = {
 	.current_task	= &init_task,
 	.preempt_count	= INIT_PREEMPT_COUNT,

-- 
2.48.1.262.g85cc9f2d1e-goog


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

* [PATCH RESEND v2 3/3] x86/cpu: Enable modifying bug flags with {clear,set}puid
  2025-01-29 15:35 [PATCH RESEND v2 0/3] x86/cpu: Add facility to force-enable CPU caps and bugs Brendan Jackman
  2025-01-29 15:35 ` [PATCH RESEND v2 1/3] x86/cpu: Create helper to parse clearcpuid param Brendan Jackman
  2025-01-29 15:35 ` [PATCH RESEND v2 2/3] x86/cpu: Add setcpuid cmdline param Brendan Jackman
@ 2025-01-29 15:35 ` Brendan Jackman
  2025-02-17 11:10   ` Borislav Petkov
  2 siblings, 1 reply; 13+ messages in thread
From: Brendan Jackman @ 2025-01-29 15:35 UTC (permalink / raw)
  To: Jonathan Corbet, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	Dave Hansen, x86, H. Peter Anvin, Peter Zijlstra
  Cc: linux-doc, linux-kernel, Brendan Jackman

Sometimes it can be very useful to run CPU vulnerability mitigations on
systems where they aren't known to mitigate any real-world
vulnerabilities. This can be handy for mundane reasons like debugging
HW-agnostic logic on whatever machine is to hand, but also for research
reasons: while some mitigations are focused on individual vulns and
uarches, others are fairly general, and it's strategically useful to
have an idea how they'd perform on systems where they aren't currently
needed.

As evidence for this being useful, a flag specifically for Retbleed was
added in commit 5c9a92dec323 ("x86/bugs: Add retbleed=force").

Since CPU bugs are tracked using the same basic mechanism as features,
and there are already parameters for manipulating them by hand, extend
that mechanism to support bug as well as capabilities.

With this patch and setcpuid=srso, a QEMU guest running on an Intel host
will boot with Safe-RET enabled.

Signed-off-by: Brendan Jackman <jackmanb@google.com>
---
 arch/x86/include/asm/cpufeature.h |  1 +
 arch/x86/kernel/cpu/common.c      | 16 ++++++++++++----
 2 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/arch/x86/include/asm/cpufeature.h b/arch/x86/include/asm/cpufeature.h
index 0b9611da6c53f19ae6c45d85d1ee191118ad1895..6e17f47ab0521acadb7db38ce5934c4717d457ba 100644
--- a/arch/x86/include/asm/cpufeature.h
+++ b/arch/x86/include/asm/cpufeature.h
@@ -50,6 +50,7 @@ extern const char * const x86_power_flags[32];
  * X86_BUG_<name> - NCAPINTS*32.
  */
 extern const char * const x86_bug_flags[NBUGINTS*32];
+#define x86_bug_flag(flag) x86_bug_flags[flag]
 
 #define test_cpu_cap(c, bit)						\
 	 arch_test_bit(bit, (unsigned long *)((c)->x86_capability))
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index e26cf8789f0e1a27ad126f531e05afee0fdebbb8..d94d7ebff42dadae30f77af1ef675d1a83ba6c3f 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -1492,7 +1492,8 @@ static inline void parse_set_clear_cpuid(char *arg, bool set)
 
 		/*
 		 * Handle naked numbers first for feature flags which don't
-		 * have names.
+		 * have names. It doesn't make sense for a bug not to have a
+		 * name so don't handle bug flags here.
 		 */
 		if (!kstrtouint(opt, 10, &bit)) {
 			if (bit < NCAPINTS * 32) {
@@ -1516,11 +1517,18 @@ static inline void parse_set_clear_cpuid(char *arg, bool set)
 			continue;
 		}
 
-		for (bit = 0; bit < 32 * NCAPINTS; bit++) {
-			if (!x86_cap_flag(bit))
+		for (bit = 0; bit < 32 * (NCAPINTS + NBUGINTS); bit++) {
+			const char *flag;
+
+			if (bit < 32 * NCAPINTS)
+				flag = x86_cap_flag(bit);
+			else
+				flag = x86_bug_flag(bit - (32 * NCAPINTS));
+
+			if (!flag)
 				continue;
 
-			if (strcmp(x86_cap_flag(bit), opt))
+			if (strcmp(flag, opt))
 				continue;
 
 			pr_cont(" %s", opt);

-- 
2.48.1.262.g85cc9f2d1e-goog


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

* Re: [PATCH RESEND v2 1/3] x86/cpu: Create helper to parse clearcpuid param
  2025-01-29 15:35 ` [PATCH RESEND v2 1/3] x86/cpu: Create helper to parse clearcpuid param Brendan Jackman
@ 2025-02-17 10:59   ` Borislav Petkov
  0 siblings, 0 replies; 13+ messages in thread
From: Borislav Petkov @ 2025-02-17 10:59 UTC (permalink / raw)
  To: Brendan Jackman
  Cc: Jonathan Corbet, Thomas Gleixner, Ingo Molnar, Dave Hansen, x86,
	H. Peter Anvin, Peter Zijlstra, linux-doc, linux-kernel

On Wed, Jan 29, 2025 at 03:35:39PM +0000, Brendan Jackman wrote:
> +/*
> + * We parse cpu parameters early because fpu__init_system() is executed

While at it:

s/We parse cpu parameters/Parse CPU parameters/

> + * before parse_early_param().
> + */
> +static void __init cpu_parse_early_param(void)
> +{
> +	char arg[128];
> +	int arglen;
> +
> +#ifdef CONFIG_X86_32
> +	if (cmdline_find_option_bool(boot_command_line, "no387"))
> +#ifdef CONFIG_MATH_EMULATION
> +		setup_clear_cpu_cap(X86_FEATURE_FPU);
> +#else
> +		pr_err("Option 'no387' required CONFIG_MATH_EMULATION enabled.\n");
> +#endif
> +
> +	if (cmdline_find_option_bool(boot_command_line, "nofxsr"))
> +		setup_clear_cpu_cap(X86_FEATURE_FXSR);
> +#endif
> +
> +	if (cmdline_find_option_bool(boot_command_line, "noxsave"))
> +		setup_clear_cpu_cap(X86_FEATURE_XSAVE);
> +
> +	if (cmdline_find_option_bool(boot_command_line, "noxsaveopt"))
> +		setup_clear_cpu_cap(X86_FEATURE_XSAVEOPT);
> +
> +	if (cmdline_find_option_bool(boot_command_line, "noxsaves"))
> +		setup_clear_cpu_cap(X86_FEATURE_XSAVES);
> +
> +	if (cmdline_find_option_bool(boot_command_line, "nousershstk"))
> +		setup_clear_cpu_cap(X86_FEATURE_USER_SHSTK);
> +
> +	/* Minimize the gap between FRED is available and available but disabled. */
> +	arglen = cmdline_find_option(boot_command_line, "fred", arg, sizeof(arg));
> +	if (arglen != 2 || strncmp(arg, "on", 2))
> +		setup_clear_cpu_cap(X86_FEATURE_FRED);
> +
> +	arglen = cmdline_find_option(boot_command_line, "clearcpuid", arg, sizeof(arg));
> +	if (arglen <= 0)
> +		return;

Newline here.

> +	parse_clearcpuid(arg);
>  }
>  
>  /*
> 
> -- 
> 2.48.1.262.g85cc9f2d1e-goog
> 

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

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

* Re: [PATCH RESEND v2 2/3] x86/cpu: Add setcpuid cmdline param
  2025-01-29 15:35 ` [PATCH RESEND v2 2/3] x86/cpu: Add setcpuid cmdline param Brendan Jackman
@ 2025-02-17 11:03   ` Borislav Petkov
  0 siblings, 0 replies; 13+ messages in thread
From: Borislav Petkov @ 2025-02-17 11:03 UTC (permalink / raw)
  To: Brendan Jackman
  Cc: Jonathan Corbet, Thomas Gleixner, Ingo Molnar, Dave Hansen, x86,
	H. Peter Anvin, Peter Zijlstra, linux-doc, linux-kernel

On Wed, Jan 29, 2025 at 03:35:40PM +0000, Brendan Jackman wrote:
> In preparation for adding support to fake out CPU bugs, add a general
> facility to force enablement of CPU flags.

"... to force-enable ..."

> 
> The flag taints the kernel and the documentation attempts to be clear
> that this is highly unsuitable for uses outside of kernel development
> and platform experimentation.
> 
> The new arg is parsed just like clearcpuid, but instead of leading to
> setup_clear_cpu_cap() it leads to setup_force_cpu_cap().
> 
> I've tested this by booting a nested QEMU guest on an Intel host, which
> with setcpuid=svm will claim that it supports AMD virtualization.

Move this sentence...

> Signed-off-by: Brendan Jackman <jackmanb@google.com>
> ---

... here as it doesn't belong in a commit message.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

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

* Re: [PATCH RESEND v2 3/3] x86/cpu: Enable modifying bug flags with {clear,set}puid
  2025-01-29 15:35 ` [PATCH RESEND v2 3/3] x86/cpu: Enable modifying bug flags with {clear,set}puid Brendan Jackman
@ 2025-02-17 11:10   ` Borislav Petkov
  2025-02-17 11:20     ` Brendan Jackman
  2025-02-17 16:56     ` Brendan Jackman
  0 siblings, 2 replies; 13+ messages in thread
From: Borislav Petkov @ 2025-02-17 11:10 UTC (permalink / raw)
  To: Brendan Jackman
  Cc: Jonathan Corbet, Thomas Gleixner, Ingo Molnar, Dave Hansen, x86,
	H. Peter Anvin, Peter Zijlstra, linux-doc, linux-kernel

On Wed, Jan 29, 2025 at 03:35:41PM +0000, Brendan Jackman wrote:
> Sometimes it can be very useful to run CPU vulnerability mitigations on
> systems where they aren't known to mitigate any real-world
> vulnerabilities. This can be handy for mundane reasons like debugging
> HW-agnostic logic on whatever machine is to hand, but also for research
> reasons: while some mitigations are focused on individual vulns and
> uarches, others are fairly general, and it's strategically useful to
> have an idea how they'd perform on systems where they aren't currently
> needed.
> 
> As evidence for this being useful, a flag specifically for Retbleed was
> added in commit 5c9a92dec323 ("x86/bugs: Add retbleed=force").
> 
> Since CPU bugs are tracked using the same basic mechanism as features,
> and there are already parameters for manipulating them by hand, extend
> that mechanism to support bug as well as capabilities.
> 
> With this patch and setcpuid=srso, a QEMU guest running on an Intel host
> will boot with Safe-RET enabled.

As before. Move that sentence ...

> Signed-off-by: Brendan Jackman <jackmanb@google.com>
> ---

... here.

>  arch/x86/include/asm/cpufeature.h |  1 +
>  arch/x86/kernel/cpu/common.c      | 16 ++++++++++++----
>  2 files changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/x86/include/asm/cpufeature.h b/arch/x86/include/asm/cpufeature.h
> index 0b9611da6c53f19ae6c45d85d1ee191118ad1895..6e17f47ab0521acadb7db38ce5934c4717d457ba 100644
> --- a/arch/x86/include/asm/cpufeature.h
> +++ b/arch/x86/include/asm/cpufeature.h
> @@ -50,6 +50,7 @@ extern const char * const x86_power_flags[32];
>   * X86_BUG_<name> - NCAPINTS*32.
>   */
>  extern const char * const x86_bug_flags[NBUGINTS*32];
> +#define x86_bug_flag(flag) x86_bug_flags[flag]

Why?

>  #define test_cpu_cap(c, bit)						\
>  	 arch_test_bit(bit, (unsigned long *)((c)->x86_capability))
> diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
> index e26cf8789f0e1a27ad126f531e05afee0fdebbb8..d94d7ebff42dadae30f77af1ef675d1a83ba6c3f 100644
> --- a/arch/x86/kernel/cpu/common.c
> +++ b/arch/x86/kernel/cpu/common.c
> @@ -1492,7 +1492,8 @@ static inline void parse_set_clear_cpuid(char *arg, bool set)
>  
>  		/*
>  		 * Handle naked numbers first for feature flags which don't
> -		 * have names.
> +		 * have names. It doesn't make sense for a bug not to have a
> +		 * name so don't handle bug flags here.
>  		 */
>  		if (!kstrtouint(opt, 10, &bit)) {
>  			if (bit < NCAPINTS * 32) {

It did but after

  7583e8fbdc49 ("x86/cpu: Remove X86_FEATURE_NAMES")

this chunk can be whacked now. Please do that in a pre-patch.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

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

* Re: [PATCH RESEND v2 3/3] x86/cpu: Enable modifying bug flags with {clear,set}puid
  2025-02-17 11:10   ` Borislav Petkov
@ 2025-02-17 11:20     ` Brendan Jackman
  2025-02-17 11:53       ` Borislav Petkov
  2025-02-17 16:56     ` Brendan Jackman
  1 sibling, 1 reply; 13+ messages in thread
From: Brendan Jackman @ 2025-02-17 11:20 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: Jonathan Corbet, Thomas Gleixner, Ingo Molnar, Dave Hansen, x86,
	H. Peter Anvin, Peter Zijlstra, linux-doc, linux-kernel

On Mon, 17 Feb 2025 at 12:10, Borislav Petkov <bp@alien8.de> wrote:
>
> >  extern const char * const x86_bug_flags[NBUGINTS*32];
> > +#define x86_bug_flag(flag) x86_bug_flags[flag]
>
> Why?

That's just for consistency with x86_cap_flag().

I don't remember seeing any reason why that indirection exists. Maybe
it's vestigial. Shall I just remove it?

To everything else: ack, thanks for the review.

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

* Re: [PATCH RESEND v2 3/3] x86/cpu: Enable modifying bug flags with {clear,set}puid
  2025-02-17 11:20     ` Brendan Jackman
@ 2025-02-17 11:53       ` Borislav Petkov
  0 siblings, 0 replies; 13+ messages in thread
From: Borislav Petkov @ 2025-02-17 11:53 UTC (permalink / raw)
  To: Brendan Jackman
  Cc: Jonathan Corbet, Thomas Gleixner, Ingo Molnar, Dave Hansen, x86,
	H. Peter Anvin, Peter Zijlstra, linux-doc, linux-kernel

On Mon, Feb 17, 2025 at 12:20:56PM +0100, Brendan Jackman wrote:
> On Mon, 17 Feb 2025 at 12:10, Borislav Petkov <bp@alien8.de> wrote:
> >
> > >  extern const char * const x86_bug_flags[NBUGINTS*32];
> > > +#define x86_bug_flag(flag) x86_bug_flags[flag]
> >
> > Why?
> 
> That's just for consistency with x86_cap_flag().
> 
> I don't remember seeing any reason why that indirection exists. Maybe
> it's vestigial. Shall I just remove it?

Wondering the same. Both arrays are automatically generated into capflags.c as 

const char * const x86_cap_flags[NCAPINTS*32] = {
const char * const x86_bug_flags[NBUGINTS*32] = {

so it's not like something is going to change them at runtime so what's the
point of the wrapper I dunno...

/me greps some...

Oh, I know:

-#ifdef CONFIG_X86_FEATURE_NAMES
 extern const char * const x86_cap_flags[NCAPINTS*32];
 extern const char * const x86_power_flags[32];
 #define X86_CAP_FMT "%s"
 #define x86_cap_flag(flag) x86_cap_flags[flag]
-#else
-#define X86_CAP_FMT X86_CAP_FMT_NUM
-#define x86_cap_flag x86_cap_flag_num
-#endif

from 7583e8fbdc49a4dbd916d14863cf1deeddb982f9

So yeah, it is a useless leftover now. So please remove.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

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

* Re: [PATCH RESEND v2 3/3] x86/cpu: Enable modifying bug flags with {clear,set}puid
  2025-02-17 11:10   ` Borislav Petkov
  2025-02-17 11:20     ` Brendan Jackman
@ 2025-02-17 16:56     ` Brendan Jackman
  2025-02-17 17:08       ` Borislav Petkov
  1 sibling, 1 reply; 13+ messages in thread
From: Brendan Jackman @ 2025-02-17 16:56 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: Jonathan Corbet, Thomas Gleixner, Ingo Molnar, Dave Hansen, x86,
	H. Peter Anvin, Peter Zijlstra, linux-doc, linux-kernel

On Mon, 17 Feb 2025 at 12:10, Borislav Petkov <bp@alien8.de> wrote:
> > @@ -1492,7 +1492,8 @@ static inline void parse_set_clear_cpuid(char *arg, bool set)
> >
> >               /*
> >                * Handle naked numbers first for feature flags which don't
> > -              * have names.
> > +              * have names. It doesn't make sense for a bug not to have a
> > +              * name so don't handle bug flags here.
> >                */
> >               if (!kstrtouint(opt, 10, &bit)) {
> >                       if (bit < NCAPINTS * 32) {
>
> It did but after
>
>   7583e8fbdc49 ("x86/cpu: Remove X86_FEATURE_NAMES")
>
> this chunk can be whacked now. Please do that in a pre-patch.

Er, hold on, what chunk can be whacked? Do you mean delete the ability
to set clearcpuid by number? There are still features with no name.

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

* Re: [PATCH RESEND v2 3/3] x86/cpu: Enable modifying bug flags with {clear,set}puid
  2025-02-17 16:56     ` Brendan Jackman
@ 2025-02-17 17:08       ` Borislav Petkov
  2025-02-17 17:20         ` Brendan Jackman
  0 siblings, 1 reply; 13+ messages in thread
From: Borislav Petkov @ 2025-02-17 17:08 UTC (permalink / raw)
  To: Brendan Jackman
  Cc: Jonathan Corbet, Thomas Gleixner, Ingo Molnar, Dave Hansen, x86,
	H. Peter Anvin, Peter Zijlstra, linux-doc, linux-kernel

On Mon, Feb 17, 2025 at 05:56:32PM +0100, Brendan Jackman wrote:
> Er, hold on, what chunk can be whacked? Do you mean delete the ability
> to set clearcpuid by number? There are still features with no name.

Really, which ones?

Are you saying you want to turn off *arbitrary* features? Not only what gets
advertized in /proc/cpuinfo?

Why?

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

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

* Re: [PATCH RESEND v2 3/3] x86/cpu: Enable modifying bug flags with {clear,set}puid
  2025-02-17 17:08       ` Borislav Petkov
@ 2025-02-17 17:20         ` Brendan Jackman
  2025-02-17 19:22           ` Borislav Petkov
  0 siblings, 1 reply; 13+ messages in thread
From: Brendan Jackman @ 2025-02-17 17:20 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: Jonathan Corbet, Thomas Gleixner, Ingo Molnar, Dave Hansen, x86,
	H. Peter Anvin, Peter Zijlstra, linux-doc, linux-kernel

On Mon, 17 Feb 2025 at 18:08, Borislav Petkov <bp@alien8.de> wrote:
>
> On Mon, Feb 17, 2025 at 05:56:32PM +0100, Brendan Jackman wrote:
> > Er, hold on, what chunk can be whacked? Do you mean delete the ability
> > to set clearcpuid by number? There are still features with no name.
>
> Really, which ones?

I just mean the ones without a "name" in cpufeatures.h, e.g.

#define X86_FEATURE_MSR_SPEC_CTRL ( 7*32+16) /* MSR SPEC_CTRL is implemented */

> Are you saying you want to turn off *arbitrary* features? Not only what gets
> advertized in /proc/cpuinfo?

You can already clear arbitrary ones with clearcpuid.

But for bugs, they all have a name. I was thinking that this was
because they are defined by the kernel, that's what I meant by "It t
doesn't make sense for a bug not to have a name", although now I think
about it we could totally have a bug and not give it a user-visible
name if we wanted to.

Anyway, still think the current logic is what we want here:

- The new setcpuid should be consistent with the existing clearcpuid,
i.e. accept numbers for the same things clearcpuid does.

- There are currently no bugs without names so for those, require the
string for both setcpuid and clearcpuid. If we wanted to we could add
number support later.

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

* Re: [PATCH RESEND v2 3/3] x86/cpu: Enable modifying bug flags with {clear,set}puid
  2025-02-17 17:20         ` Brendan Jackman
@ 2025-02-17 19:22           ` Borislav Petkov
  0 siblings, 0 replies; 13+ messages in thread
From: Borislav Petkov @ 2025-02-17 19:22 UTC (permalink / raw)
  To: Brendan Jackman
  Cc: Jonathan Corbet, Thomas Gleixner, Ingo Molnar, Dave Hansen, x86,
	H. Peter Anvin, Peter Zijlstra, linux-doc, linux-kernel

On Mon, Feb 17, 2025 at 06:20:33PM +0100, Brendan Jackman wrote:
> But for bugs, they all have a name. I was thinking that this was
> because they are defined by the kernel, that's what I meant by "It t
> doesn't make sense for a bug not to have a name", although now I think
> about it we could totally have a bug and not give it a user-visible
> name if we wanted to.

Right.

> Anyway, still think the current logic is what we want here:
> 
> - The new setcpuid should be consistent with the existing clearcpuid,
> i.e. accept numbers for the same things clearcpuid does.
> 
> - There are currently no bugs without names so for those, require the
> string for both setcpuid and clearcpuid. If we wanted to we could add
> number support later.

Right, let's not make this more than it is - a hacky interface for hacks - not
to be used in production anyway. :-)

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

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

end of thread, other threads:[~2025-02-17 19:22 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-01-29 15:35 [PATCH RESEND v2 0/3] x86/cpu: Add facility to force-enable CPU caps and bugs Brendan Jackman
2025-01-29 15:35 ` [PATCH RESEND v2 1/3] x86/cpu: Create helper to parse clearcpuid param Brendan Jackman
2025-02-17 10:59   ` Borislav Petkov
2025-01-29 15:35 ` [PATCH RESEND v2 2/3] x86/cpu: Add setcpuid cmdline param Brendan Jackman
2025-02-17 11:03   ` Borislav Petkov
2025-01-29 15:35 ` [PATCH RESEND v2 3/3] x86/cpu: Enable modifying bug flags with {clear,set}puid Brendan Jackman
2025-02-17 11:10   ` Borislav Petkov
2025-02-17 11:20     ` Brendan Jackman
2025-02-17 11:53       ` Borislav Petkov
2025-02-17 16:56     ` Brendan Jackman
2025-02-17 17:08       ` Borislav Petkov
2025-02-17 17:20         ` Brendan Jackman
2025-02-17 19:22           ` Borislav Petkov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox