1. Patchew
  2. linux
  3. [PATCH RESEND v2 0/3] x86/cpu: Add facility to force-enable CPU caps and bugs
  4. View patch

[PATCH RESEND v2 3/3] x86/cpu: Enable modifying bug flags with {clear,set}puid

Brendan Jackman posted 3 patches 1 year ago
There is a newer version of this series
[PATCH RESEND v2 3/3] x86/cpu: Enable modifying bug flags with {clear,set}puid
Posted by Brendan Jackman 1 year ago 
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
Re: [PATCH RESEND v2 3/3] x86/cpu: Enable modifying bug flags with {clear,set}puid
Posted by Borislav Petkov 11 months, 3 weeks ago 
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
Re: [PATCH RESEND v2 3/3] x86/cpu: Enable modifying bug flags with {clear,set}puid
Posted by Brendan Jackman 11 months, 3 weeks ago 
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.
Re: [PATCH RESEND v2 3/3] x86/cpu: Enable modifying bug flags with {clear,set}puid
Posted by Borislav Petkov 11 months, 3 weeks ago 
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
Re: [PATCH RESEND v2 3/3] x86/cpu: Enable modifying bug flags with {clear,set}puid
Posted by Brendan Jackman 11 months, 3 weeks ago 
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.
Re: [PATCH RESEND v2 3/3] x86/cpu: Enable modifying bug flags with {clear,set}puid
Posted by Borislav Petkov 11 months, 3 weeks ago 
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
Re: [PATCH RESEND v2 3/3] x86/cpu: Enable modifying bug flags with {clear,set}puid
Posted by Brendan Jackman 11 months, 3 weeks ago 
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.
Re: [PATCH RESEND v2 3/3] x86/cpu: Enable modifying bug flags with {clear,set}puid
Posted by Borislav Petkov 11 months, 3 weeks ago 
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

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.