2022-11-04 21:52:11

by Kim Phillips

[permalink] [raw]
Subject: [PATCH 0/3] x86/speculation: Support Automatic IBRS

The AMD Zen4 core supports a new feature called Automatic IBRS.
(Indirect Branch Restricted Speculation).

Enable Automatic IBRS by default if the CPU feature is present.
It typically provides greater performance over the incumbent
generic retpolines mitigation.

Patch 1 adds support for the CPUID_8000_0021_EAX leaf
that has the bit that identifies X86_FEATURE_AUTOIBRS.

Patch 2 adds support for Auto IBRS.

Patch 3 makes the Auto IBRS feature available for VM guests.

Signed-off-by: Kim Phillips <[email protected]>
Cc: Borislav Petkov <[email protected]>
Cc: Boris Ostrovsky <[email protected]>
Cc: Dave Hansen <[email protected]>
Cc: "H. Peter Anvin" <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Joao Martins <[email protected]>
Cc: Jonathan Corbet <[email protected]>
Cc: Konrad Rzeszutek Wilk <[email protected]>
Cc: Paolo Bonzini <[email protected]>
Cc: Sean Christopherson <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: David Woodhouse <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: Juergen Gross <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Tony Luck <[email protected]>
Cc: Babu Moger <[email protected]>
Cc: Tom Lendacky <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]

Kim Phillips (3):
x86/cpufeatures: Add support for cpuid leaf 80000021/EAX
(FeatureExt2Eax)
x86/speculation: Support Automatic IBRS
x86/speculation: Support Automatic IBRS under virtualization

.../admin-guide/kernel-parameters.txt | 1 +
arch/x86/include/asm/cpufeature.h | 7 ++--
arch/x86/include/asm/cpufeatures.h | 5 ++-
arch/x86/include/asm/disabled-features.h | 3 +-
arch/x86/include/asm/msr-index.h | 2 ++
arch/x86/include/asm/nospec-branch.h | 1 +
arch/x86/include/asm/required-features.h | 3 +-
arch/x86/kernel/cpu/bugs.c | 34 +++++++++++++++++--
arch/x86/kernel/cpu/common.c | 3 ++
arch/x86/kvm/cpuid.c | 5 ++-
arch/x86/kvm/reverse_cpuid.h | 1 +
arch/x86/kvm/svm/svm.c | 3 ++
arch/x86/kvm/x86.c | 3 ++
13 files changed, 62 insertions(+), 9 deletions(-)

--
2.34.1



2022-11-04 21:53:16

by Kim Phillips

[permalink] [raw]
Subject: [PATCH 1/3] x86/cpufeatures: Add support for cpuid leaf 80000021/EAX (FeatureExt2Eax)

AMD Zen4 processors advertise features in this leaf.
Add the leaf and its Automatic IBRS feature bit.

Note: New whole leaf (vs a bit) due to propagation via KVM
later in this series.

Signed-off-by: Kim Phillips <[email protected]>
---
arch/x86/include/asm/cpufeature.h | 7 +++++--
arch/x86/include/asm/cpufeatures.h | 5 ++++-
arch/x86/include/asm/disabled-features.h | 3 ++-
arch/x86/include/asm/required-features.h | 3 ++-
arch/x86/kernel/cpu/common.c | 3 +++
5 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/arch/x86/include/asm/cpufeature.h b/arch/x86/include/asm/cpufeature.h
index 1a85e1fb0922..ce0c8f7d3218 100644
--- a/arch/x86/include/asm/cpufeature.h
+++ b/arch/x86/include/asm/cpufeature.h
@@ -32,6 +32,7 @@ enum cpuid_leafs
CPUID_8000_0007_EBX,
CPUID_7_EDX,
CPUID_8000_001F_EAX,
+ CPUID_8000_0021_EAX,
};

#define X86_CAP_FMT_NUM "%d:%d"
@@ -94,8 +95,9 @@ extern const char * const x86_bug_flags[NBUGINTS*32];
CHECK_BIT_IN_MASK_WORD(REQUIRED_MASK, 17, feature_bit) || \
CHECK_BIT_IN_MASK_WORD(REQUIRED_MASK, 18, feature_bit) || \
CHECK_BIT_IN_MASK_WORD(REQUIRED_MASK, 19, feature_bit) || \
+ CHECK_BIT_IN_MASK_WORD(REQUIRED_MASK, 20, feature_bit) || \
REQUIRED_MASK_CHECK || \
- BUILD_BUG_ON_ZERO(NCAPINTS != 20))
+ BUILD_BUG_ON_ZERO(NCAPINTS != 21))

#define DISABLED_MASK_BIT_SET(feature_bit) \
( CHECK_BIT_IN_MASK_WORD(DISABLED_MASK, 0, feature_bit) || \
@@ -118,8 +120,9 @@ extern const char * const x86_bug_flags[NBUGINTS*32];
CHECK_BIT_IN_MASK_WORD(DISABLED_MASK, 17, feature_bit) || \
CHECK_BIT_IN_MASK_WORD(DISABLED_MASK, 18, feature_bit) || \
CHECK_BIT_IN_MASK_WORD(DISABLED_MASK, 19, feature_bit) || \
+ CHECK_BIT_IN_MASK_WORD(DISABLED_MASK, 20, feature_bit) || \
DISABLED_MASK_CHECK || \
- BUILD_BUG_ON_ZERO(NCAPINTS != 20))
+ BUILD_BUG_ON_ZERO(NCAPINTS != 21))

#define cpu_has(c, bit) \
(__builtin_constant_p(bit) && REQUIRED_MASK_BIT_SET(bit) ? 1 : \
diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
index aefd0816a333..45ea992716b8 100644
--- a/arch/x86/include/asm/cpufeatures.h
+++ b/arch/x86/include/asm/cpufeatures.h
@@ -13,7 +13,7 @@
/*
* Defines x86 CPU feature bits
*/
-#define NCAPINTS 20 /* N 32-bit words worth of info */
+#define NCAPINTS 21 /* N 32-bit words worth of info */
#define NBUGINTS 1 /* N 32-bit bug flags */

/*
@@ -421,6 +421,9 @@
#define X86_FEATURE_V_TSC_AUX (19*32+ 9) /* "" Virtual TSC_AUX */
#define X86_FEATURE_SME_COHERENT (19*32+10) /* "" AMD hardware-enforced cache coherency */

+/* AMD-defined Extended Feature 2 EAX, CPUID level 0x80000021 (EAX), word 20 */
+#define X86_FEATURE_AUTOIBRS (20*32+ 8) /* AMD Automatic IBRS */
+
/*
* BUG word(s)
*/
diff --git a/arch/x86/include/asm/disabled-features.h b/arch/x86/include/asm/disabled-features.h
index bbb03b25263e..9be4e0b01b9c 100644
--- a/arch/x86/include/asm/disabled-features.h
+++ b/arch/x86/include/asm/disabled-features.h
@@ -118,6 +118,7 @@
#define DISABLED_MASK17 0
#define DISABLED_MASK18 0
#define DISABLED_MASK19 0
-#define DISABLED_MASK_CHECK BUILD_BUG_ON_ZERO(NCAPINTS != 20)
+#define DISABLED_MASK20 0
+#define DISABLED_MASK_CHECK BUILD_BUG_ON_ZERO(NCAPINTS != 21)

#endif /* _ASM_X86_DISABLED_FEATURES_H */
diff --git a/arch/x86/include/asm/required-features.h b/arch/x86/include/asm/required-features.h
index aff774775c67..7ba1726b71c7 100644
--- a/arch/x86/include/asm/required-features.h
+++ b/arch/x86/include/asm/required-features.h
@@ -98,6 +98,7 @@
#define REQUIRED_MASK17 0
#define REQUIRED_MASK18 0
#define REQUIRED_MASK19 0
-#define REQUIRED_MASK_CHECK BUILD_BUG_ON_ZERO(NCAPINTS != 20)
+#define REQUIRED_MASK20 0
+#define REQUIRED_MASK_CHECK BUILD_BUG_ON_ZERO(NCAPINTS != 21)

#endif /* _ASM_X86_REQUIRED_FEATURES_H */
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 2bec4b4b2c50..070350c2c514 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -1091,6 +1091,9 @@ void get_cpu_cap(struct cpuinfo_x86 *c)
if (c->extended_cpuid_level >= 0x8000001f)
c->x86_capability[CPUID_8000_001F_EAX] = cpuid_eax(0x8000001f);

+ if (c->extended_cpuid_level >= 0x80000021)
+ c->x86_capability[CPUID_8000_0021_EAX] = cpuid_eax(0x80000021);
+
init_scattered_cpuid_features(c);
init_speculation_control(c);

--
2.34.1


2022-11-04 21:57:18

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH 1/3] x86/cpufeatures: Add support for cpuid leaf 80000021/EAX (FeatureExt2Eax)

On Fri, Nov 04, 2022 at 04:36:49PM -0500, Kim Phillips wrote:
> AMD Zen4 processors advertise features in this leaf.
> Add the leaf and its Automatic IBRS feature bit.
>
> Note: New whole leaf (vs a bit) due to propagation via KVM
> later in this series.

No, not a separate leaf - use scattered.c

For an example what to do for KVM, see

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

Thx.

--
Regards/Gruss,
Boris.

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

2022-11-04 22:15:41

by Kim Phillips

[permalink] [raw]
Subject: [PATCH 2/3] x86/speculation: Support Automatic IBRS

The AMD Zen4 core supports a new feature called Automatic IBRS.

It is a "set-and-forget" feature that means that, unlike e.g.,
s/w-toggled SPEC_CTRL.IBRS, h/w manages its IBRS mitigation
resources automatically across CPL transitions.

The feature is advertised by CPUID_Fn80000021_EAX bit 8 and is
enabled by setting MSR C000_0080 (EFER) bit 21.

Enable Automatic IBRS by default if the CPU feature is present.
It typically provides greater performance over the incumbent
generic retpolines mitigation. In addition:

- Don't clear the RSB on VMEXIT when AutoIBRS is enabled:
The internal return address stack used for return address
predictions is automatically cleared on VMEXIT.

- Automatic IBRS removes the need for toggling IBRS during
firmware switches, so don't enable IBRS_FW when Automatic
IBRS is enabled.

- Allow for spectre_v2=autoibrs in the kernel command line,
reverting to auto-selection if the feature isn't available.

Signed-off-by: Kim Phillips <[email protected]>
---
.../admin-guide/kernel-parameters.txt | 1 +
arch/x86/include/asm/msr-index.h | 2 ++
arch/x86/include/asm/nospec-branch.h | 1 +
arch/x86/kernel/cpu/bugs.c | 34 +++++++++++++++++--
4 files changed, 35 insertions(+), 3 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index a465d5242774..5ac4422e16a6 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -5702,6 +5702,7 @@
eibrs,retpoline - enhanced IBRS + Retpolines
eibrs,lfence - enhanced IBRS + LFENCE
ibrs - use IBRS to protect kernel
+ autoibrs - AMD Automatic IBRS

Not specifying this option is equivalent to
spectre_v2=auto.
diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
index 10ac52705892..bd73e509cfa6 100644
--- a/arch/x86/include/asm/msr-index.h
+++ b/arch/x86/include/asm/msr-index.h
@@ -30,6 +30,7 @@
#define _EFER_SVME 12 /* Enable virtualization */
#define _EFER_LMSLE 13 /* Long Mode Segment Limit Enable */
#define _EFER_FFXSR 14 /* Enable Fast FXSAVE/FXRSTOR */
+#define _EFER_AUTOIBRS 21 /* Automatic IBRS Enable */

#define EFER_SCE (1<<_EFER_SCE)
#define EFER_LME (1<<_EFER_LME)
@@ -38,6 +39,7 @@
#define EFER_SVME (1<<_EFER_SVME)
#define EFER_LMSLE (1<<_EFER_LMSLE)
#define EFER_FFXSR (1<<_EFER_FFXSR)
+#define EFER_AUTOIBRS (1<<_EFER_AUTOIBRS)

/* Intel MSRs. Some also available on other CPUs */

diff --git a/arch/x86/include/asm/nospec-branch.h b/arch/x86/include/asm/nospec-branch.h
index 82580adbca4b..12b2b070caab 100644
--- a/arch/x86/include/asm/nospec-branch.h
+++ b/arch/x86/include/asm/nospec-branch.h
@@ -442,6 +442,7 @@ enum spectre_v2_mitigation {
SPECTRE_V2_EIBRS_RETPOLINE,
SPECTRE_V2_EIBRS_LFENCE,
SPECTRE_V2_IBRS,
+ SPECTRE_V2_AUTO_IBRS,
};

/* The indirect branch speculation control variants */
diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
index 66d7addf1784..31e5af78baa0 100644
--- a/arch/x86/kernel/cpu/bugs.c
+++ b/arch/x86/kernel/cpu/bugs.c
@@ -1050,6 +1050,7 @@ enum spectre_v2_mitigation_cmd {
SPECTRE_V2_CMD_EIBRS_RETPOLINE,
SPECTRE_V2_CMD_EIBRS_LFENCE,
SPECTRE_V2_CMD_IBRS,
+ SPECTRE_V2_CMD_AUTOIBRS,
};

enum spectre_v2_user_cmd {
@@ -1124,6 +1125,7 @@ spectre_v2_parse_user_cmdline(void)
return SPECTRE_V2_USER_CMD_AUTO;
}

+/* Checks for Intel IBRS versions */
static inline bool spectre_v2_in_ibrs_mode(enum spectre_v2_mitigation mode)
{
return mode == SPECTRE_V2_IBRS ||
@@ -1233,6 +1235,7 @@ static const char * const spectre_v2_strings[] = {
[SPECTRE_V2_EIBRS_LFENCE] = "Mitigation: Enhanced IBRS + LFENCE",
[SPECTRE_V2_EIBRS_RETPOLINE] = "Mitigation: Enhanced IBRS + Retpolines",
[SPECTRE_V2_IBRS] = "Mitigation: IBRS",
+ [SPECTRE_V2_AUTO_IBRS] = "Mitigation: Automatic IBRS",
};

static const struct {
@@ -1249,6 +1252,7 @@ static const struct {
{ "eibrs", SPECTRE_V2_CMD_EIBRS, false },
{ "eibrs,lfence", SPECTRE_V2_CMD_EIBRS_LFENCE, false },
{ "eibrs,retpoline", SPECTRE_V2_CMD_EIBRS_RETPOLINE, false },
+ { "autoibrs", SPECTRE_V2_CMD_AUTOIBRS, false },
{ "auto", SPECTRE_V2_CMD_AUTO, false },
{ "ibrs", SPECTRE_V2_CMD_IBRS, false },
};
@@ -1305,6 +1309,13 @@ static enum spectre_v2_mitigation_cmd __init spectre_v2_parse_cmdline(void)
return SPECTRE_V2_CMD_AUTO;
}

+ if (cmd == SPECTRE_V2_CMD_AUTOIBRS &&
+ !boot_cpu_has(X86_FEATURE_AUTOIBRS)) {
+ pr_err("%s selected but CPU doesn't have AMD Automatic IBRS. Switching to AUTO select\n",
+ mitigation_options[i].option);
+ return SPECTRE_V2_CMD_AUTO;
+ }
+
if ((cmd == SPECTRE_V2_CMD_RETPOLINE_LFENCE ||
cmd == SPECTRE_V2_CMD_EIBRS_LFENCE) &&
!boot_cpu_has(X86_FEATURE_LFENCE_RDTSC)) {
@@ -1392,6 +1403,7 @@ static void __init spectre_v2_determine_rsb_fill_type_at_vmexit(enum spectre_v2_
*/
switch (mode) {
case SPECTRE_V2_NONE:
+ case SPECTRE_V2_AUTO_IBRS:
return;

case SPECTRE_V2_EIBRS_LFENCE:
@@ -1419,6 +1431,7 @@ static void __init spectre_v2_select_mitigation(void)
{
enum spectre_v2_mitigation_cmd cmd = spectre_v2_parse_cmdline();
enum spectre_v2_mitigation mode = SPECTRE_V2_NONE;
+ uint64_t efer;

/*
* If the CPU is not affected and the command line mode is NONE or AUTO
@@ -1434,6 +1447,11 @@ static void __init spectre_v2_select_mitigation(void)

case SPECTRE_V2_CMD_FORCE:
case SPECTRE_V2_CMD_AUTO:
+ if (boot_cpu_has(X86_FEATURE_AUTOIBRS)) {
+ mode = SPECTRE_V2_AUTO_IBRS;
+ break;
+ }
+
if (boot_cpu_has(X86_FEATURE_IBRS_ENHANCED)) {
mode = SPECTRE_V2_EIBRS;
break;
@@ -1480,6 +1498,10 @@ static void __init spectre_v2_select_mitigation(void)
case SPECTRE_V2_CMD_EIBRS_RETPOLINE:
mode = SPECTRE_V2_EIBRS_RETPOLINE;
break;
+
+ case SPECTRE_V2_CMD_AUTOIBRS:
+ mode = SPECTRE_V2_AUTO_IBRS;
+ break;
}

if (mode == SPECTRE_V2_EIBRS && unprivileged_ebpf_enabled())
@@ -1495,6 +1517,11 @@ static void __init spectre_v2_select_mitigation(void)
case SPECTRE_V2_EIBRS:
break;

+ case SPECTRE_V2_AUTO_IBRS:
+ rdmsrl(MSR_EFER, efer);
+ wrmsrl(MSR_EFER, efer | EFER_AUTOIBRS);
+ break;
+
case SPECTRE_V2_IBRS:
setup_force_cpu_cap(X86_FEATURE_KERNEL_IBRS);
if (boot_cpu_has(X86_FEATURE_IBRS_ENHANCED))
@@ -1571,8 +1598,8 @@ static void __init spectre_v2_select_mitigation(void)
/*
* Retpoline protects the kernel, but doesn't protect firmware. IBRS
* and Enhanced IBRS protect firmware too, so enable IBRS around
- * firmware calls only when IBRS / Enhanced IBRS aren't otherwise
- * enabled.
+ * firmware calls only when IBRS / Enhanced / Automatic IBRS aren't
+ * otherwise enabled.
*
* Use "mode" to check Enhanced IBRS instead of boot_cpu_has(), because
* the user might select retpoline on the kernel command line and if
@@ -1589,7 +1616,8 @@ static void __init spectre_v2_select_mitigation(void)
pr_info("Enabling Speculation Barrier for firmware calls\n");
}

- } else if (boot_cpu_has(X86_FEATURE_IBRS) && !spectre_v2_in_ibrs_mode(mode)) {
+ } else if (boot_cpu_has(X86_FEATURE_IBRS) && !spectre_v2_in_ibrs_mode(mode) &&
+ mode != SPECTRE_V2_AUTO_IBRS) {
setup_force_cpu_cap(X86_FEATURE_USE_IBRS_FW);
pr_info("Enabling Restricted Speculation for firmware calls\n");
}
--
2.34.1


2022-11-04 22:20:12

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH 2/3] x86/speculation: Support Automatic IBRS

On Fri, Nov 04, 2022 at 04:36:50PM -0500, Kim Phillips wrote:
> - Allow for spectre_v2=autoibrs in the kernel command line,
> reverting to auto-selection if the feature isn't available.

Why?

What the whole logic here should do is enable autoibrs when detected
automatically, without the need for the user to even select it as it is
the superior mitigation.

--
Regards/Gruss,
Boris.

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

2022-11-04 22:40:45

by Dave Hansen

[permalink] [raw]
Subject: Re: [PATCH 0/3] x86/speculation: Support Automatic IBRS

On 11/4/22 14:36, Kim Phillips wrote:
> The AMD Zen4 core supports a new feature called Automatic IBRS.
> (Indirect Branch Restricted Speculation).
>
> Enable Automatic IBRS by default if the CPU feature is present.
> It typically provides greater performance over the incumbent
> generic retpolines mitigation.

Could you also share some information on how this differs from EIBRS and
why it needs to exist in parallel to EBIRS?

2022-11-05 11:26:58

by Peter Zijlstra

[permalink] [raw]
Subject: Re: [PATCH 2/3] x86/speculation: Support Automatic IBRS

On Fri, Nov 04, 2022 at 10:52:13PM +0100, Borislav Petkov wrote:
> On Fri, Nov 04, 2022 at 04:36:50PM -0500, Kim Phillips wrote:
> > - Allow for spectre_v2=autoibrs in the kernel command line,
> > reverting to auto-selection if the feature isn't available.
>
> Why?
>
> What the whole logic here should do is enable autoibrs when detected
> automatically, without the need for the user to even select it as it is
> the superior mitigation.

Well; perhaps the whole autoibrs thing should be mapped to the existing
eIBRS options. AFAICT this is the same thing under a new name, no need
to invent yet more options. bugs.c is quite insane enough already.

2022-11-05 11:57:26

by David Laight

[permalink] [raw]
Subject: RE: [PATCH 2/3] x86/speculation: Support Automatic IBRS

From: Borislav Petkov
> Sent: 04 November 2022 21:52
>
> On Fri, Nov 04, 2022 at 04:36:50PM -0500, Kim Phillips wrote:
> > - Allow for spectre_v2=autoibrs in the kernel command line,
> > reverting to auto-selection if the feature isn't available.
>
> Why?
>
> What the whole logic here should do is enable autoibrs when detected
> automatically, without the need for the user to even select it as it is
> the superior mitigation.

The only useful option is to allow a different option be
selected for code testing.
So maybe you want an option for completeness - for when
an 'even better' option is available.

David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

2022-11-07 22:46:10

by Kim Phillips

[permalink] [raw]
Subject: Re: [PATCH 2/3] x86/speculation: Support Automatic IBRS

On 11/5/22 6:39 AM, David Laight wrote:
> From: Borislav Petkov
>> Sent: 04 November 2022 21:52
>>
>> On Fri, Nov 04, 2022 at 04:36:50PM -0500, Kim Phillips wrote:
>>> - Allow for spectre_v2=autoibrs in the kernel command line,
>>> reverting to auto-selection if the feature isn't available.
>>
>> Why?
>>
>> What the whole logic here should do is enable autoibrs when detected
>> automatically, without the need for the user to even select it as it is
>> the superior mitigation.
>
> The only useful option is to allow a different option be
> selected for code testing.
> So maybe you want an option for completeness - for when
> an 'even better' option is available.

This is true.

Kim

2022-11-07 23:06:39

by Kim Phillips

[permalink] [raw]
Subject: Re: [PATCH 2/3] x86/speculation: Support Automatic IBRS

On 11/5/22 6:10 AM, Peter Zijlstra wrote:
> On Fri, Nov 04, 2022 at 10:52:13PM +0100, Borislav Petkov wrote:
>> On Fri, Nov 04, 2022 at 04:36:50PM -0500, Kim Phillips wrote:
>>> - Allow for spectre_v2=autoibrs in the kernel command line,
>>> reverting to auto-selection if the feature isn't available.
>>
>> Why?
>>
>> What the whole logic here should do is enable autoibrs when detected
>> automatically, without the need for the user to even select it as it is
>> the superior mitigation.
>
> Well; perhaps the whole autoibrs thing should be mapped to the existing
> eIBRS options. AFAICT this is the same thing under a new name, no need
> to invent yet more options. bugs.c is quite insane enough already.

I've started a version that has AUTOIBRS reuse SPECTRE_V2_EIBRS
spectre_v2_mitigation enum, but, so far, it's change to bugs.c
looks bigger: 58 lines changed vs. 34 (see below).

Let me know if you want me to send it as a part of a v2 submission
after I take care of the kvm CPUID review.

Thanks,

Kim

Autoibrs-as-eibrs diff:

diff --git a/arch/x86/include/asm/nospec-branch.h b/arch/x86/include/asm/nospec-branch.h
index 2e9dd8823244..3ab90f23e7f7 100644
--- a/arch/x86/include/asm/nospec-branch.h
+++ b/arch/x86/include/asm/nospec-branch.h
@@ -442,7 +442,6 @@ enum spectre_v2_mitigation {
SPECTRE_V2_EIBRS_RETPOLINE,
SPECTRE_V2_EIBRS_LFENCE,
SPECTRE_V2_IBRS,
- SPECTRE_V2_AUTO_IBRS,
};

/* The indirect branch speculation control variants */
diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
index 31e5af78baa0..ccfd8fb12095 100644
--- a/arch/x86/kernel/cpu/bugs.c
+++ b/arch/x86/kernel/cpu/bugs.c
@@ -1005,6 +1005,7 @@ static inline const char *spectre_v2_module_string(void) { return ""; }
#endif

#define SPECTRE_V2_LFENCE_MSG "WARNING: LFENCE mitigation is not recommended for this CPU, data leaks possible!\n"
+#define SPECTRE_V2_EIBRS_AMD_MSG "WARNING: AutoIBRS does not need additional RETPOLINE/LFENCE mitigations, not doing them\n"
#define SPECTRE_V2_EIBRS_EBPF_MSG "WARNING: Unprivileged eBPF is enabled with eIBRS on, data leaks possible via Spectre v2 BHB attacks!\n"
#define SPECTRE_V2_EIBRS_LFENCE_EBPF_SMT_MSG "WARNING: Unprivileged eBPF is enabled with eIBRS+LFENCE mitigation and SMT, data leaks possible via Spectre v2 BHB attacks!\n"
#define SPECTRE_V2_IBRS_PERF_MSG "WARNING: IBRS mitigation selected on Enhanced IBRS CPU, this may cause unnecessary performance loss\n"
@@ -1125,7 +1126,7 @@ spectre_v2_parse_user_cmdline(void)
return SPECTRE_V2_USER_CMD_AUTO;
}

-/* Checks for Intel IBRS versions */
+/* Checks for IBRS versions */
static inline bool spectre_v2_in_ibrs_mode(enum spectre_v2_mitigation mode)
{
return mode == SPECTRE_V2_IBRS ||
@@ -1201,7 +1202,8 @@ spectre_v2_user_select_mitigation(void)
*/
if (!boot_cpu_has(X86_FEATURE_STIBP) ||
!smt_possible ||
- spectre_v2_in_ibrs_mode(spectre_v2_enabled))
+ (spectre_v2_in_ibrs_mode(spectre_v2_enabled) &&
+ !boot_cpu_has(X86_FEATURE_AUTOIBRS)))
return;

/*
@@ -1231,11 +1233,10 @@ static const char * const spectre_v2_strings[] = {
[SPECTRE_V2_NONE] = "Vulnerable",
[SPECTRE_V2_RETPOLINE] = "Mitigation: Retpolines",
[SPECTRE_V2_LFENCE] = "Mitigation: LFENCE",
- [SPECTRE_V2_EIBRS] = "Mitigation: Enhanced IBRS",
+ [SPECTRE_V2_EIBRS] = "Mitigation: Enhanced / Automatic IBRS",
[SPECTRE_V2_EIBRS_LFENCE] = "Mitigation: Enhanced IBRS + LFENCE",
[SPECTRE_V2_EIBRS_RETPOLINE] = "Mitigation: Enhanced IBRS + Retpolines",
[SPECTRE_V2_IBRS] = "Mitigation: IBRS",
- [SPECTRE_V2_AUTO_IBRS] = "Mitigation: Automatic IBRS",
};

static const struct {
@@ -1250,9 +1251,9 @@ static const struct {
{ "retpoline,lfence", SPECTRE_V2_CMD_RETPOLINE_LFENCE, false },
{ "retpoline,generic", SPECTRE_V2_CMD_RETPOLINE_GENERIC, false },
{ "eibrs", SPECTRE_V2_CMD_EIBRS, false },
+ { "autoibrs", SPECTRE_V2_CMD_EIBRS, false },
{ "eibrs,lfence", SPECTRE_V2_CMD_EIBRS_LFENCE, false },
{ "eibrs,retpoline", SPECTRE_V2_CMD_EIBRS_RETPOLINE, false },
- { "autoibrs", SPECTRE_V2_CMD_AUTOIBRS, false },
{ "auto", SPECTRE_V2_CMD_AUTO, false },
{ "ibrs", SPECTRE_V2_CMD_IBRS, false },
};
@@ -1303,15 +1304,17 @@ static enum spectre_v2_mitigation_cmd __init spectre_v2_parse_cmdline(void)
if ((cmd == SPECTRE_V2_CMD_EIBRS ||
cmd == SPECTRE_V2_CMD_EIBRS_LFENCE ||
cmd == SPECTRE_V2_CMD_EIBRS_RETPOLINE) &&
- !boot_cpu_has(X86_FEATURE_IBRS_ENHANCED)) {
- pr_err("%s selected but CPU doesn't have eIBRS. Switching to AUTO select\n",
+ (!boot_cpu_has(X86_FEATURE_IBRS_ENHANCED) ||
+ !boot_cpu_has(X86_FEATURE_AUTOIBRS))) {
+ pr_err("%s selected but CPU doesn't have Enhanced or Automatic IBRS. Switching to AUTO select\n",
mitigation_options[i].option);
return SPECTRE_V2_CMD_AUTO;
}

- if (cmd == SPECTRE_V2_CMD_AUTOIBRS &&
- !boot_cpu_has(X86_FEATURE_AUTOIBRS)) {
- pr_err("%s selected but CPU doesn't have AMD Automatic IBRS. Switching to AUTO select\n",
+ if ((cmd == SPECTRE_V2_CMD_EIBRS_LFENCE ||
+ cmd == SPECTRE_V2_CMD_EIBRS_RETPOLINE) &&
+ boot_cpu_has(X86_FEATURE_AUTOIBRS)) {
+ pr_err("%s selected but AMD Automatic IBRS doesn't need extra retpoline mitigations. Switching to AUTO select\n",
mitigation_options[i].option);
return SPECTRE_V2_CMD_AUTO;
}
@@ -1403,7 +1406,6 @@ static void __init spectre_v2_determine_rsb_fill_type_at_vmexit(enum spectre_v2_
*/
switch (mode) {
case SPECTRE_V2_NONE:
- case SPECTRE_V2_AUTO_IBRS:
return;

case SPECTRE_V2_EIBRS_LFENCE:
@@ -1447,12 +1449,8 @@ static void __init spectre_v2_select_mitigation(void)

case SPECTRE_V2_CMD_FORCE:
case SPECTRE_V2_CMD_AUTO:
- if (boot_cpu_has(X86_FEATURE_AUTOIBRS)) {
- mode = SPECTRE_V2_AUTO_IBRS;
- break;
- }
-
- if (boot_cpu_has(X86_FEATURE_IBRS_ENHANCED)) {
+ if (boot_cpu_has(X86_FEATURE_IBRS_ENHANCED) ||
+ boot_cpu_has(X86_FEATURE_AUTOIBRS)) {
mode = SPECTRE_V2_EIBRS;
break;
}
@@ -1488,19 +1486,24 @@ static void __init spectre_v2_select_mitigation(void)
break;

case SPECTRE_V2_CMD_EIBRS:
+ case SPECTRE_V2_CMD_AUTOIBRS:
mode = SPECTRE_V2_EIBRS;
break;

case SPECTRE_V2_CMD_EIBRS_LFENCE:
- mode = SPECTRE_V2_EIBRS_LFENCE;
+ if (boot_cpu_has(X86_FEATURE_AUTOIBRS)) {
+ pr_err(SPECTRE_V2_EIBRS_AMD_MSG);
+ mode = SPECTRE_V2_EIBRS;
+ } else
+ mode = SPECTRE_V2_EIBRS_LFENCE;
break;

case SPECTRE_V2_CMD_EIBRS_RETPOLINE:
- mode = SPECTRE_V2_EIBRS_RETPOLINE;
- break;
-
- case SPECTRE_V2_CMD_AUTOIBRS:
- mode = SPECTRE_V2_AUTO_IBRS;
+ if (boot_cpu_has(X86_FEATURE_AUTOIBRS)) {
+ pr_err(SPECTRE_V2_EIBRS_AMD_MSG);
+ mode = SPECTRE_V2_EIBRS;
+ } else
+ mode = SPECTRE_V2_EIBRS_RETPOLINE;
break;
}

@@ -1508,8 +1511,13 @@ static void __init spectre_v2_select_mitigation(void)
pr_err(SPECTRE_V2_EIBRS_EBPF_MSG);

if (spectre_v2_in_ibrs_mode(mode)) {
- x86_spec_ctrl_base |= SPEC_CTRL_IBRS;
- write_spec_ctrl_current(x86_spec_ctrl_base, true);
+ if (boot_cpu_has(X86_FEATURE_AUTOIBRS)) {
+ rdmsrl(MSR_EFER, efer);
+ wrmsrl(MSR_EFER, efer | EFER_AUTOIBRS);
+ } else {
+ x86_spec_ctrl_base |= SPEC_CTRL_IBRS;
+ write_spec_ctrl_current(x86_spec_ctrl_base, true);
+ }
}

switch (mode) {
@@ -1517,11 +1525,6 @@ static void __init spectre_v2_select_mitigation(void)
case SPECTRE_V2_EIBRS:
break;

- case SPECTRE_V2_AUTO_IBRS:
- rdmsrl(MSR_EFER, efer);
- wrmsrl(MSR_EFER, efer | EFER_AUTOIBRS);
- break;
-
case SPECTRE_V2_IBRS:
setup_force_cpu_cap(X86_FEATURE_KERNEL_IBRS);
if (boot_cpu_has(X86_FEATURE_IBRS_ENHANCED))
@@ -1616,8 +1619,8 @@ static void __init spectre_v2_select_mitigation(void)
pr_info("Enabling Speculation Barrier for firmware calls\n");
}

- } else if (boot_cpu_has(X86_FEATURE_IBRS) && !spectre_v2_in_ibrs_mode(mode) &&
- mode != SPECTRE_V2_AUTO_IBRS) {
+ } else if ((boot_cpu_has(X86_FEATURE_IBRS) && !spectre_v2_in_ibrs_mode(mode)) ||
+ (boot_cpu_has(X86_FEATURE_AUTOIBRS) && !spectre_v2_in_ibrs_mode(mode))) {
setup_force_cpu_cap(X86_FEATURE_USE_IBRS_FW);
pr_info("Enabling Restricted Speculation for firmware calls\n");
}
@@ -2353,7 +2356,8 @@ static ssize_t mmio_stale_data_show_state(char *buf)

static char *stibp_state(void)
{
- if (spectre_v2_in_ibrs_mode(spectre_v2_enabled))
+ if (spectre_v2_in_ibrs_mode(spectre_v2_enabled) &&
+ !boot_cpu_has(X86_FEATURE_AUTOIBRS))
return "";

switch (spectre_v2_user_stibp) {

2022-11-07 23:23:13

by Kim Phillips

[permalink] [raw]
Subject: Re: [PATCH 0/3] x86/speculation: Support Automatic IBRS

On 11/4/22 5:06 PM, Dave Hansen wrote:
> On 11/4/22 14:36, Kim Phillips wrote:
>> The AMD Zen4 core supports a new feature called Automatic IBRS.
>> (Indirect Branch Restricted Speculation).
>>
>> Enable Automatic IBRS by default if the CPU feature is present.
>> It typically provides greater performance over the incumbent
>> generic retpolines mitigation.
>
> Could you also share some information on how this differs from EIBRS and

Some differences are:

1. Unlike EIBRS, AutoIBRS needs STIBP always-on
2. Unlike EIBRS, AutoIBRS doesn't need to clear the RSB on VMEXIT
3. Unlike EIBRS, no AutoIBRS processors are vulnerable to RETBLEED
4. Unlike EIBRS, AutoIBRS doesn't need
4. eIBRS also considers user/supervisor as well as host/guest modes when
determining its 'predictor mode', whereas AutoIBRS only affects CPL0.
5. eIBRS also groups CPL0/1/2 together, vs. just CPL0 for AutoIBRS.

> why it needs to exist in parallel to EBIRS?

If by 'in parallel' you mean why do these patches not tack AutoIBRS
onto the SPECTRE_V2_EIBRS enum, there's no big reason, other than
now that I've tried to implement it that way, the number of changes
to bugs.c goes from 34 lines changed to 58, mostly due to exceptions
caused by items such as #3 above, and ignoring EIBRS_RETPOLINE and
EIBRS_LFENCE.

I've posted the diff to the 2/3 patch thread, please take a look:

https://lkml.org/lkml/2022/11/7/1462
https://lore.kernel.org/lkml/[email protected]/T/#m78ef9bf6a38db8348e0adde3f5ac8b4953200b41

Thanks,

Kim

2022-11-07 23:47:51

by Dave Hansen

[permalink] [raw]
Subject: Re: [PATCH 2/3] x86/speculation: Support Automatic IBRS

On 11/7/22 14:39, Kim Phillips wrote:
> I've started a version that has AUTOIBRS reuse SPECTRE_V2_EIBRS
> spectre_v2_mitigation enum, but, so far, it's change to bugs.c
> looks bigger: 58 lines changed vs. 34 (see below).
>
> Let me know if you want me to send it as a part of a v2 submission
> after I take care of the kvm CPUID review.

Thanks for putting that together. I generally like how this looks.

I think it probably goes to a _bit_ too much trouble to turn off
"eibrs,lfence/retpoline". If someone goes to the trouble of specifying
those, a warning or pr_info() is probably enough. You don't need to
actively override it.

> - } else if (boot_cpu_has(X86_FEATURE_IBRS) && !spectre_v2_in_ibrs_mode(mode) &&
> - mode != SPECTRE_V2_AUTO_IBRS) {
> + } else if ((boot_cpu_has(X86_FEATURE_IBRS) && !spectre_v2_in_ibrs_mode(mode)) ||
> + (boot_cpu_has(X86_FEATURE_AUTOIBRS) && !spectre_v2_in_ibrs_mode(mode))) {
> setup_force_cpu_cap(X86_FEATURE_USE_IBRS_FW);
> pr_info("Enabling Restricted Speculation for firmware calls\n");

Did the "mode != SPECTRE_V2_AUTO_IBRS" check get dropped accidentally?
Or is it unnecessary now?

2022-11-08 08:28:02

by Peter Zijlstra

[permalink] [raw]
Subject: Re: [PATCH 2/3] x86/speculation: Support Automatic IBRS

On Mon, Nov 07, 2022 at 03:41:03PM -0800, Dave Hansen wrote:
> On 11/7/22 14:39, Kim Phillips wrote:
> > I've started a version that has AUTOIBRS reuse SPECTRE_V2_EIBRS
> > spectre_v2_mitigation enum, but, so far, it's change to bugs.c
> > looks bigger: 58 lines changed vs. 34 (see below).
> >
> > Let me know if you want me to send it as a part of a v2 submission
> > after I take care of the kvm CPUID review.
>
> Thanks for putting that together. I generally like how this looks.
>
> I think it probably goes to a _bit_ too much trouble to turn off
> "eibrs,lfence/retpoline". If someone goes to the trouble of specifying
> those, a warning or pr_info() is probably enough. You don't need to
> actively override it.

Not, even, just do it. User told you to, it's not technically
impossible, so just go.

2022-11-11 12:50:42

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH 2/3] x86/speculation: Support Automatic IBRS

On Mon, Nov 07, 2022 at 04:39:02PM -0600, Kim Phillips wrote:
> I've started a version that has AUTOIBRS reuse SPECTRE_V2_EIBRS
> spectre_v2_mitigation enum, but, so far, it's change to bugs.c
> looks bigger: 58 lines changed vs. 34 (see below).

It can be smaller. You simply do:

if (cpu_has(c, X86_FEATURE_AUTOIBRS))
setup_force_cpu_cap(X86_FEATURE_IBRS_ENHANCED);

and the rest should just work - see below.

And yes, as Peter says, when the user requests something, the user
should get it. No matter whether it makes sense or not.

Thx.

---
diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
index 66d7addf1784..2b77eaee9bd2 100644
--- a/arch/x86/kernel/cpu/bugs.c
+++ b/arch/x86/kernel/cpu/bugs.c
@@ -1005,6 +1005,7 @@ static inline const char *spectre_v2_module_string(void) { return ""; }
#endif

#define SPECTRE_V2_LFENCE_MSG "WARNING: LFENCE mitigation is not recommended for this CPU, data leaks possible!\n"
+#define SPECTRE_V2_EIBRS_AMD_MSG "WARNING: AutoIBRS does not need additional RETPOLINE/LFENCE mitigations, not doing them\n"
#define SPECTRE_V2_EIBRS_EBPF_MSG "WARNING: Unprivileged eBPF is enabled with eIBRS on, data leaks possible via Spectre v2 BHB attacks!\n"
#define SPECTRE_V2_EIBRS_LFENCE_EBPF_SMT_MSG "WARNING: Unprivileged eBPF is enabled with eIBRS+LFENCE mitigation and SMT, data leaks possible via Spectre v2 BHB attacks!\n"
#define SPECTRE_V2_IBRS_PERF_MSG "WARNING: IBRS mitigation selected on Enhanced IBRS CPU, this may cause unnecessary performance loss\n"
@@ -1124,6 +1125,7 @@ spectre_v2_parse_user_cmdline(void)
return SPECTRE_V2_USER_CMD_AUTO;
}

+/* Checks for IBRS versions */
static inline bool spectre_v2_in_ibrs_mode(enum spectre_v2_mitigation mode)
{
return mode == SPECTRE_V2_IBRS ||
@@ -1229,7 +1231,7 @@ static const char * const spectre_v2_strings[] = {
[SPECTRE_V2_NONE] = "Vulnerable",
[SPECTRE_V2_RETPOLINE] = "Mitigation: Retpolines",
[SPECTRE_V2_LFENCE] = "Mitigation: LFENCE",
- [SPECTRE_V2_EIBRS] = "Mitigation: Enhanced IBRS",
+ [SPECTRE_V2_EIBRS] = "Mitigation: Enhanced / Automatic IBRS",
[SPECTRE_V2_EIBRS_LFENCE] = "Mitigation: Enhanced IBRS + LFENCE",
[SPECTRE_V2_EIBRS_RETPOLINE] = "Mitigation: Enhanced IBRS + Retpolines",
[SPECTRE_V2_IBRS] = "Mitigation: IBRS",
@@ -1247,6 +1249,7 @@ static const struct {
{ "retpoline,lfence", SPECTRE_V2_CMD_RETPOLINE_LFENCE, false },
{ "retpoline,generic", SPECTRE_V2_CMD_RETPOLINE_GENERIC, false },
{ "eibrs", SPECTRE_V2_CMD_EIBRS, false },
+ { "autoibrs", SPECTRE_V2_CMD_EIBRS, false },
{ "eibrs,lfence", SPECTRE_V2_CMD_EIBRS_LFENCE, false },
{ "eibrs,retpoline", SPECTRE_V2_CMD_EIBRS_RETPOLINE, false },
{ "auto", SPECTRE_V2_CMD_AUTO, false },
@@ -1300,7 +1303,7 @@ static enum spectre_v2_mitigation_cmd __init spectre_v2_parse_cmdline(void)
cmd == SPECTRE_V2_CMD_EIBRS_LFENCE ||
cmd == SPECTRE_V2_CMD_EIBRS_RETPOLINE) &&
!boot_cpu_has(X86_FEATURE_IBRS_ENHANCED)) {
- pr_err("%s selected but CPU doesn't have eIBRS. Switching to AUTO select\n",
+ pr_err("%s selected but CPU doesn't have Enhanced or Automatic IBRS. Switching to AUTO select\n",
mitigation_options[i].option);
return SPECTRE_V2_CMD_AUTO;
}
@@ -1474,11 +1477,19 @@ static void __init spectre_v2_select_mitigation(void)
break;

case SPECTRE_V2_CMD_EIBRS_LFENCE:
- mode = SPECTRE_V2_EIBRS_LFENCE;
+ if (boot_cpu_has(X86_FEATURE_AUTOIBRS)) {
+ pr_err(SPECTRE_V2_EIBRS_AMD_MSG);
+ mode = SPECTRE_V2_EIBRS;
+ } else
+ mode = SPECTRE_V2_EIBRS_LFENCE;
break;

case SPECTRE_V2_CMD_EIBRS_RETPOLINE:
- mode = SPECTRE_V2_EIBRS_RETPOLINE;
+ if (boot_cpu_has(X86_FEATURE_AUTOIBRS)) {
+ pr_err(SPECTRE_V2_EIBRS_AMD_MSG);
+ mode = SPECTRE_V2_EIBRS;
+ } else
+ mode = SPECTRE_V2_EIBRS_RETPOLINE;
break;
}

@@ -1486,8 +1497,12 @@ static void __init spectre_v2_select_mitigation(void)
pr_err(SPECTRE_V2_EIBRS_EBPF_MSG);

if (spectre_v2_in_ibrs_mode(mode)) {
- x86_spec_ctrl_base |= SPEC_CTRL_IBRS;
- write_spec_ctrl_current(x86_spec_ctrl_base, true);
+ if (boot_cpu_has(X86_FEATURE_AUTOIBRS)) {
+ msr_set_bit(MSR_EFER, _EFER_AUTOIBRS);
+ } else {
+ x86_spec_ctrl_base |= SPEC_CTRL_IBRS;
+ write_spec_ctrl_current(x86_spec_ctrl_base, true);
+ }
}

switch (mode) {
@@ -1571,8 +1586,8 @@ static void __init spectre_v2_select_mitigation(void)
/*
* Retpoline protects the kernel, but doesn't protect firmware. IBRS
* and Enhanced IBRS protect firmware too, so enable IBRS around
- * firmware calls only when IBRS / Enhanced IBRS aren't otherwise
- * enabled.
+ * firmware calls only when IBRS / Enhanced / Automatic IBRS aren't
+ * otherwise enabled.
*
* Use "mode" to check Enhanced IBRS instead of boot_cpu_has(), because
* the user might select retpoline on the kernel command line and if
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 73cc546e024d..45e3670bdaaf 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -1341,6 +1344,10 @@ static void __init cpu_set_bug_bits(struct cpuinfo_x86 *c)
if (ia32_cap & ARCH_CAP_IBRS_ALL)
setup_force_cpu_cap(X86_FEATURE_IBRS_ENHANCED);

+ /* AMDs AutoIBRS is equivalent to Intel's eIBRS - use the Intel flag. */
+ if (cpu_has(c, X86_FEATURE_AUTOIBRS))
+ setup_force_cpu_cap(X86_FEATURE_IBRS_ENHANCED);
+
if (!cpu_matches(cpu_vuln_whitelist, NO_MDS) &&
!(ia32_cap & ARCH_CAP_MDS_NO)) {
setup_force_cpu_bug(X86_BUG_MDS);

--
Regards/Gruss,
Boris.

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

Subject: Re: [PATCH 2/3] x86/speculation: Support Automatic IBRS

On Fri, Nov 11, 2022 at 01:09:37PM +0100, Borislav Petkov wrote:
> On Mon, Nov 07, 2022 at 04:39:02PM -0600, Kim Phillips wrote:
> > I've started a version that has AUTOIBRS reuse SPECTRE_V2_EIBRS
> > spectre_v2_mitigation enum, but, so far, it's change to bugs.c
> > looks bigger: 58 lines changed vs. 34 (see below).
>
> It can be smaller. You simply do:
>
> if (cpu_has(c, X86_FEATURE_AUTOIBRS))
> setup_force_cpu_cap(X86_FEATURE_IBRS_ENHANCED);
>
> and the rest should just work - see below.
>
> And yes, as Peter says, when the user requests something, the user
> should get it. No matter whether it makes sense or not.
>
> Thx.
>
> ---
> diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
> index 66d7addf1784..2b77eaee9bd2 100644
> --- a/arch/x86/kernel/cpu/bugs.c
> +++ b/arch/x86/kernel/cpu/bugs.c
> @@ -1005,6 +1005,7 @@ static inline const char *spectre_v2_module_string(void) { return ""; }
> #endif
>
> #define SPECTRE_V2_LFENCE_MSG "WARNING: LFENCE mitigation is not recommended for this CPU, data leaks possible!\n"
> +#define SPECTRE_V2_EIBRS_AMD_MSG "WARNING: AutoIBRS does not need additional RETPOLINE/LFENCE mitigations, not doing them\n"
> #define SPECTRE_V2_EIBRS_EBPF_MSG "WARNING: Unprivileged eBPF is enabled with eIBRS on, data leaks possible via Spectre v2 BHB attacks!\n"
> #define SPECTRE_V2_EIBRS_LFENCE_EBPF_SMT_MSG "WARNING: Unprivileged eBPF is enabled with eIBRS+LFENCE mitigation and SMT, data leaks possible via Spectre v2 BHB attacks!\n"
> #define SPECTRE_V2_IBRS_PERF_MSG "WARNING: IBRS mitigation selected on Enhanced IBRS CPU, this may cause unnecessary performance loss\n"
> @@ -1124,6 +1125,7 @@ spectre_v2_parse_user_cmdline(void)
> return SPECTRE_V2_USER_CMD_AUTO;
> }
>
> +/* Checks for IBRS versions */
> static inline bool spectre_v2_in_ibrs_mode(enum spectre_v2_mitigation mode)
> {
> return mode == SPECTRE_V2_IBRS ||
> @@ -1229,7 +1231,7 @@ static const char * const spectre_v2_strings[] = {
> [SPECTRE_V2_NONE] = "Vulnerable",
> [SPECTRE_V2_RETPOLINE] = "Mitigation: Retpolines",
> [SPECTRE_V2_LFENCE] = "Mitigation: LFENCE",
> - [SPECTRE_V2_EIBRS] = "Mitigation: Enhanced IBRS",
> + [SPECTRE_V2_EIBRS] = "Mitigation: Enhanced / Automatic IBRS",
> [SPECTRE_V2_EIBRS_LFENCE] = "Mitigation: Enhanced IBRS + LFENCE",
> [SPECTRE_V2_EIBRS_RETPOLINE] = "Mitigation: Enhanced IBRS + Retpolines",
> [SPECTRE_V2_IBRS] = "Mitigation: IBRS",
> @@ -1247,6 +1249,7 @@ static const struct {
> { "retpoline,lfence", SPECTRE_V2_CMD_RETPOLINE_LFENCE, false },
> { "retpoline,generic", SPECTRE_V2_CMD_RETPOLINE_GENERIC, false },
> { "eibrs", SPECTRE_V2_CMD_EIBRS, false },
> + { "autoibrs", SPECTRE_V2_CMD_EIBRS, false },
> { "eibrs,lfence", SPECTRE_V2_CMD_EIBRS_LFENCE, false },
> { "eibrs,retpoline", SPECTRE_V2_CMD_EIBRS_RETPOLINE, false },
> { "auto", SPECTRE_V2_CMD_AUTO, false },
> @@ -1300,7 +1303,7 @@ static enum spectre_v2_mitigation_cmd __init spectre_v2_parse_cmdline(void)
> cmd == SPECTRE_V2_CMD_EIBRS_LFENCE ||
> cmd == SPECTRE_V2_CMD_EIBRS_RETPOLINE) &&
> !boot_cpu_has(X86_FEATURE_IBRS_ENHANCED)) {
> - pr_err("%s selected but CPU doesn't have eIBRS. Switching to AUTO select\n",
> + pr_err("%s selected but CPU doesn't have Enhanced or Automatic IBRS. Switching to AUTO select\n",
> mitigation_options[i].option);
> return SPECTRE_V2_CMD_AUTO;
> }
> @@ -1474,11 +1477,19 @@ static void __init spectre_v2_select_mitigation(void)
> break;
>
> case SPECTRE_V2_CMD_EIBRS_LFENCE:
> - mode = SPECTRE_V2_EIBRS_LFENCE;
> + if (boot_cpu_has(X86_FEATURE_AUTOIBRS)) {
> + pr_err(SPECTRE_V2_EIBRS_AMD_MSG);
> + mode = SPECTRE_V2_EIBRS;
> + } else
> + mode = SPECTRE_V2_EIBRS_LFENCE;
> break;
>
> case SPECTRE_V2_CMD_EIBRS_RETPOLINE:
> - mode = SPECTRE_V2_EIBRS_RETPOLINE;
> + if (boot_cpu_has(X86_FEATURE_AUTOIBRS)) {
> + pr_err(SPECTRE_V2_EIBRS_AMD_MSG);
> + mode = SPECTRE_V2_EIBRS;
> + } else
> + mode = SPECTRE_V2_EIBRS_RETPOLINE;
> break;
> }
>

I am confused here. Isn't the agreement that the user should get what they
asked for? That is, instead of warning and changing the mode to
SPECTRE_V2_EIBRS, the kernel should still use lfence or retpoline as requested?

The point of those options was to protect against Branch History Injection
attacks and Intra-Mode Branch Target Injection attacks. The first one might not
affect the CPUs that support AUTOIBRS, though we haven't heard that.

The second one (IMBTI) is very likely still possible with AUTOIBRS and
retpolines should still protect against those attacks. So users who want to be
paranoid should still be able to opt for "eibrs,retpoline" and have retpolines
enabled.

Cascardo.

> @@ -1486,8 +1497,12 @@ static void __init spectre_v2_select_mitigation(void)
> pr_err(SPECTRE_V2_EIBRS_EBPF_MSG);
>
> if (spectre_v2_in_ibrs_mode(mode)) {
> - x86_spec_ctrl_base |= SPEC_CTRL_IBRS;
> - write_spec_ctrl_current(x86_spec_ctrl_base, true);
> + if (boot_cpu_has(X86_FEATURE_AUTOIBRS)) {
> + msr_set_bit(MSR_EFER, _EFER_AUTOIBRS);
> + } else {
> + x86_spec_ctrl_base |= SPEC_CTRL_IBRS;
> + write_spec_ctrl_current(x86_spec_ctrl_base, true);
> + }
> }
>
> switch (mode) {
> @@ -1571,8 +1586,8 @@ static void __init spectre_v2_select_mitigation(void)
> /*
> * Retpoline protects the kernel, but doesn't protect firmware. IBRS
> * and Enhanced IBRS protect firmware too, so enable IBRS around
> - * firmware calls only when IBRS / Enhanced IBRS aren't otherwise
> - * enabled.
> + * firmware calls only when IBRS / Enhanced / Automatic IBRS aren't
> + * otherwise enabled.
> *
> * Use "mode" to check Enhanced IBRS instead of boot_cpu_has(), because
> * the user might select retpoline on the kernel command line and if
> diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
> index 73cc546e024d..45e3670bdaaf 100644
> --- a/arch/x86/kernel/cpu/common.c
> +++ b/arch/x86/kernel/cpu/common.c
> @@ -1341,6 +1344,10 @@ static void __init cpu_set_bug_bits(struct cpuinfo_x86 *c)
> if (ia32_cap & ARCH_CAP_IBRS_ALL)
> setup_force_cpu_cap(X86_FEATURE_IBRS_ENHANCED);
>
> + /* AMDs AutoIBRS is equivalent to Intel's eIBRS - use the Intel flag. */
> + if (cpu_has(c, X86_FEATURE_AUTOIBRS))
> + setup_force_cpu_cap(X86_FEATURE_IBRS_ENHANCED);
> +
> if (!cpu_matches(cpu_vuln_whitelist, NO_MDS) &&
> !(ia32_cap & ARCH_CAP_MDS_NO)) {
> setup_force_cpu_bug(X86_BUG_MDS);
>
> --
> Regards/Gruss,
> Boris.
>
> https://people.kernel.org/tglx/notes-about-netiquette

2022-11-12 01:20:59

by Kim Phillips

[permalink] [raw]
Subject: Re: [PATCH 2/3] x86/speculation: Support Automatic IBRS

On 11/11/22 6:40 AM, Thadeu Lima de Souza Cascardo wrote:
> On Fri, Nov 11, 2022 at 01:09:37PM +0100, Borislav Petkov wrote:
>> On Mon, Nov 07, 2022 at 04:39:02PM -0600, Kim Phillips wrote:
>>> I've started a version that has AUTOIBRS reuse SPECTRE_V2_EIBRS
>>> spectre_v2_mitigation enum, but, so far, it's change to bugs.c
>>> looks bigger: 58 lines changed vs. 34 (see below).
>>
>> It can be smaller. You simply do:
>>
>> if (cpu_has(c, X86_FEATURE_AUTOIBRS))
>> setup_force_cpu_cap(X86_FEATURE_IBRS_ENHANCED);
>>
>> and the rest should just work - see below.
>>
>> And yes, as Peter says, when the user requests something, the user
>> should get it. No matter whether it makes sense or not.

OK & thanks.

>> @@ -1474,11 +1477,19 @@ static void __init spectre_v2_select_mitigation(void)
>> break;
>>
>> case SPECTRE_V2_CMD_EIBRS_LFENCE:
>> - mode = SPECTRE_V2_EIBRS_LFENCE;
>> + if (boot_cpu_has(X86_FEATURE_AUTOIBRS)) {
>> + pr_err(SPECTRE_V2_EIBRS_AMD_MSG);
>> + mode = SPECTRE_V2_EIBRS;
>> + } else
>> + mode = SPECTRE_V2_EIBRS_LFENCE;
>> break;
>>
>> case SPECTRE_V2_CMD_EIBRS_RETPOLINE:
>> - mode = SPECTRE_V2_EIBRS_RETPOLINE;
>> + if (boot_cpu_has(X86_FEATURE_AUTOIBRS)) {
>> + pr_err(SPECTRE_V2_EIBRS_AMD_MSG);
>> + mode = SPECTRE_V2_EIBRS;
>> + } else
>> + mode = SPECTRE_V2_EIBRS_RETPOLINE;
>> break;
>> }
>>
>
> I am confused here. Isn't the agreement that the user should get what they
> asked for? That is, instead of warning and changing the mode to
> SPECTRE_V2_EIBRS, the kernel should still use lfence or retpoline as requested?
>
> The point of those options was to protect against Branch History Injection
> attacks and Intra-Mode Branch Target Injection attacks. The first one might not
> affect the CPUs that support AUTOIBRS, though we haven't heard that.
>
> The second one (IMBTI) is very likely still possible with AUTOIBRS and
> retpolines should still protect against those attacks. So users who want to be
> paranoid should still be able to opt for "eibrs,retpoline" and have retpolines
> enabled.

I've removed the above and have the complete diff below. It includes patch 1/3 and
drops 3/3 for now due to Jim Mattson's comments. After some more testing, I'll
resubmit.

Thanks,

Kim


diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index a465d5242774..b260a36dc3ef 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -5698,9 +5698,10 @@
retpoline,generic - Retpolines
retpoline,lfence - LFENCE; indirect branch
retpoline,amd - alias for retpoline,lfence
- eibrs - enhanced IBRS
- eibrs,retpoline - enhanced IBRS + Retpolines
- eibrs,lfence - enhanced IBRS + LFENCE
+ eibrs - Enhanced/Auto IBRS
+ autoibrs - Enhanced/Auto IBRS
+ eibrs,retpoline - Enhanced/Auto IBRS + Retpolines
+ eibrs,lfence - Enhanced/Auto IBRS + LFENCE
ibrs - use IBRS to protect kernel

Not specifying this option is equivalent to
diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
index 97669aaf1202..ec9a4eb8e7b9 100644
--- a/arch/x86/include/asm/cpufeatures.h
+++ b/arch/x86/include/asm/cpufeatures.h
@@ -84,7 +84,7 @@

/* CPU types for specific tunings: */
#define X86_FEATURE_K8 ( 3*32+ 4) /* "" Opteron, Athlon64 */
-/* FREE, was #define X86_FEATURE_K7 ( 3*32+ 5) "" Athlon */
+#define X86_FEATURE_AUTOIBRS ( 3*32+ 5) /* AMD Automatic IBRS */
#define X86_FEATURE_P3 ( 3*32+ 6) /* "" P3 */
#define X86_FEATURE_P4 ( 3*32+ 7) /* "" P4 */
#define X86_FEATURE_CONSTANT_TSC ( 3*32+ 8) /* TSC ticks at a constant rate */
diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
index a3eb4d3e70b8..56e4f3aab31c 100644
--- a/arch/x86/include/asm/msr-index.h
+++ b/arch/x86/include/asm/msr-index.h
@@ -30,6 +30,7 @@
#define _EFER_SVME 12 /* Enable virtualization */
#define _EFER_LMSLE 13 /* Long Mode Segment Limit Enable */
#define _EFER_FFXSR 14 /* Enable Fast FXSAVE/FXRSTOR */
+#define _EFER_AUTOIBRS 21 /* Enable Automatic IBRS */

#define EFER_SCE (1<<_EFER_SCE)
#define EFER_LME (1<<_EFER_LME)
@@ -38,6 +39,7 @@
#define EFER_SVME (1<<_EFER_SVME)
#define EFER_LMSLE (1<<_EFER_LMSLE)
#define EFER_FFXSR (1<<_EFER_FFXSR)
+#define EFER_AUTOIBRS (1<<_EFER_AUTOIBRS)

/* Intel MSRs. Some also available on other CPUs */

diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
index 66d7addf1784..4060ca8c2c60 100644
--- a/arch/x86/kernel/cpu/bugs.c
+++ b/arch/x86/kernel/cpu/bugs.c
@@ -1229,7 +1229,7 @@ static const char * const spectre_v2_strings[] = {
[SPECTRE_V2_NONE] = "Vulnerable",
[SPECTRE_V2_RETPOLINE] = "Mitigation: Retpolines",
[SPECTRE_V2_LFENCE] = "Mitigation: LFENCE",
- [SPECTRE_V2_EIBRS] = "Mitigation: Enhanced IBRS",
+ [SPECTRE_V2_EIBRS] = "Mitigation: Enhanced / Automatic IBRS",
[SPECTRE_V2_EIBRS_LFENCE] = "Mitigation: Enhanced IBRS + LFENCE",
[SPECTRE_V2_EIBRS_RETPOLINE] = "Mitigation: Enhanced IBRS + Retpolines",
[SPECTRE_V2_IBRS] = "Mitigation: IBRS",
@@ -1247,6 +1247,7 @@ static const struct {
{ "retpoline,lfence", SPECTRE_V2_CMD_RETPOLINE_LFENCE, false },
{ "retpoline,generic", SPECTRE_V2_CMD_RETPOLINE_GENERIC, false },
{ "eibrs", SPECTRE_V2_CMD_EIBRS, false },
+ { "autoibrs", SPECTRE_V2_CMD_EIBRS, false },
{ "eibrs,lfence", SPECTRE_V2_CMD_EIBRS_LFENCE, false },
{ "eibrs,retpoline", SPECTRE_V2_CMD_EIBRS_RETPOLINE, false },
{ "auto", SPECTRE_V2_CMD_AUTO, false },
@@ -1300,7 +1301,7 @@ static enum spectre_v2_mitigation_cmd __init spectre_v2_parse_cmdline(void)
cmd == SPECTRE_V2_CMD_EIBRS_LFENCE ||
cmd == SPECTRE_V2_CMD_EIBRS_RETPOLINE) &&
!boot_cpu_has(X86_FEATURE_IBRS_ENHANCED)) {
- pr_err("%s selected but CPU doesn't have eIBRS. Switching to AUTO select\n",
+ pr_err("%s selected but CPU doesn't have Enhanced or Automatic IBRS. Switching to AUTO select\n",
mitigation_options[i].option);
return SPECTRE_V2_CMD_AUTO;
}
@@ -1486,8 +1487,12 @@ static void __init spectre_v2_select_mitigation(void)
pr_err(SPECTRE_V2_EIBRS_EBPF_MSG);

if (spectre_v2_in_ibrs_mode(mode)) {
- x86_spec_ctrl_base |= SPEC_CTRL_IBRS;
- write_spec_ctrl_current(x86_spec_ctrl_base, true);
+ if (boot_cpu_has(X86_FEATURE_AUTOIBRS)) {
+ msr_set_bit(MSR_EFER, _EFER_AUTOIBRS);
+ } else {
+ x86_spec_ctrl_base |= SPEC_CTRL_IBRS;
+ write_spec_ctrl_current(x86_spec_ctrl_base, true);
+ }
}

switch (mode) {
@@ -1571,8 +1576,8 @@ static void __init spectre_v2_select_mitigation(void)
/*
* Retpoline protects the kernel, but doesn't protect firmware. IBRS
* and Enhanced IBRS protect firmware too, so enable IBRS around
- * firmware calls only when IBRS / Enhanced IBRS aren't otherwise
- * enabled.
+ * firmware calls only when IBRS / Enhanced / Automatic IBRS aren't
+ * otherwise enabled.
*
* Use "mode" to check Enhanced IBRS instead of boot_cpu_has(), because
* the user might select retpoline on the kernel command line and if
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 423a760fa9de..287b356ccf92 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -1340,6 +1340,10 @@ static void __init cpu_set_bug_bits(struct cpuinfo_x86 *c)
if (ia32_cap & ARCH_CAP_IBRS_ALL)
setup_force_cpu_cap(X86_FEATURE_IBRS_ENHANCED);

+ /* AMD's AutoIBRS is equivalent to Intel's eIBRS - use the Intel flag. */
+ if (cpu_has(c, X86_FEATURE_AUTOIBRS))
+ setup_force_cpu_cap(X86_FEATURE_IBRS_ENHANCED);
+
if (!cpu_matches(cpu_vuln_whitelist, NO_MDS) &&
!(ia32_cap & ARCH_CAP_MDS_NO)) {
setup_force_cpu_bug(X86_BUG_MDS);
diff --git a/arch/x86/kernel/cpu/scattered.c b/arch/x86/kernel/cpu/scattered.c
index f53944fb8f7f..cef8c3e688b4 100644
--- a/arch/x86/kernel/cpu/scattered.c
+++ b/arch/x86/kernel/cpu/scattered.c
@@ -45,8 +45,10 @@ static const struct cpuid_bit cpuid_bits[] = {
{ X86_FEATURE_CPB, CPUID_EDX, 9, 0x80000007, 0 },
{ X86_FEATURE_PROC_FEEDBACK, CPUID_EDX, 11, 0x80000007, 0 },
{ X86_FEATURE_MBA, CPUID_EBX, 6, 0x80000008, 0 },
+ { X86_FEATURE_AUTOIBRS, CPUID_EAX, 20, 0x80000021, 0 },
{ X86_FEATURE_PERFMON_V2, CPUID_EAX, 0, 0x80000022, 0 },
{ X86_FEATURE_AMD_LBR_V2, CPUID_EAX, 1, 0x80000022, 0 },
+
{ 0, 0, 0, 0, 0 }
};


2022-11-12 01:28:54

by Jim Mattson

[permalink] [raw]
Subject: Re: [PATCH 2/3] x86/speculation: Support Automatic IBRS

On Fri, Nov 11, 2022 at 4:46 PM Kim Phillips <[email protected]> wrote:
>
> On 11/11/22 6:40 AM, Thadeu Lima de Souza Cascardo wrote:
> > On Fri, Nov 11, 2022 at 01:09:37PM +0100, Borislav Petkov wrote:
> >> On Mon, Nov 07, 2022 at 04:39:02PM -0600, Kim Phillips wrote:
> >>> I've started a version that has AUTOIBRS reuse SPECTRE_V2_EIBRS
> >>> spectre_v2_mitigation enum, but, so far, it's change to bugs.c
> >>> looks bigger: 58 lines changed vs. 34 (see below).
> >>
> >> It can be smaller. You simply do:
> >>
> >> if (cpu_has(c, X86_FEATURE_AUTOIBRS))
> >> setup_force_cpu_cap(X86_FEATURE_IBRS_ENHANCED);
> >>
> >> and the rest should just work - see below.
> >>
> >> And yes, as Peter says, when the user requests something, the user
> >> should get it. No matter whether it makes sense or not.
>
> OK & thanks.
>
> >> @@ -1474,11 +1477,19 @@ static void __init spectre_v2_select_mitigation(void)
> >> break;
> >>
> >> case SPECTRE_V2_CMD_EIBRS_LFENCE:
> >> - mode = SPECTRE_V2_EIBRS_LFENCE;
> >> + if (boot_cpu_has(X86_FEATURE_AUTOIBRS)) {
> >> + pr_err(SPECTRE_V2_EIBRS_AMD_MSG);
> >> + mode = SPECTRE_V2_EIBRS;
> >> + } else
> >> + mode = SPECTRE_V2_EIBRS_LFENCE;
> >> break;
> >>
> >> case SPECTRE_V2_CMD_EIBRS_RETPOLINE:
> >> - mode = SPECTRE_V2_EIBRS_RETPOLINE;
> >> + if (boot_cpu_has(X86_FEATURE_AUTOIBRS)) {
> >> + pr_err(SPECTRE_V2_EIBRS_AMD_MSG);
> >> + mode = SPECTRE_V2_EIBRS;
> >> + } else
> >> + mode = SPECTRE_V2_EIBRS_RETPOLINE;
> >> break;
> >> }
> >>
> >
> > I am confused here. Isn't the agreement that the user should get what they
> > asked for? That is, instead of warning and changing the mode to
> > SPECTRE_V2_EIBRS, the kernel should still use lfence or retpoline as requested?
> >
> > The point of those options was to protect against Branch History Injection
> > attacks and Intra-Mode Branch Target Injection attacks. The first one might not
> > affect the CPUs that support AUTOIBRS, though we haven't heard that.
> >
> > The second one (IMBTI) is very likely still possible with AUTOIBRS and
> > retpolines should still protect against those attacks. So users who want to be
> > paranoid should still be able to opt for "eibrs,retpoline" and have retpolines
> > enabled.
>
> I've removed the above and have the complete diff below. It includes patch 1/3 and
> drops 3/3 for now due to Jim Mattson's comments. After some more testing, I'll
> resubmit.

I bought the argument that AutoIBRS => Same Mode IBRS, so L2 should
not be able to steer L1's indirect branches, even if they share a
predictor mode.

2022-11-15 23:32:22

by Kim Phillips

[permalink] [raw]
Subject: Re: [PATCH 1/3] x86/cpufeatures: Add support for cpuid leaf 80000021/EAX (FeatureExt2Eax)

On 11/4/22 4:48 PM, Borislav Petkov wrote:
> On Fri, Nov 04, 2022 at 04:36:49PM -0500, Kim Phillips wrote:
>> AMD Zen4 processors advertise features in this leaf.
>> Add the leaf and its Automatic IBRS feature bit.
>>
>> Note: New whole leaf (vs a bit) due to propagation via KVM
>> later in this series.
>
> No, not a separate leaf - use scattered.c
> > For an example what to do for KVM, see
>
> https://lore.kernel.org/r/[email protected]

That adds features that are mutually exclusive between
kvm and the host kernel, unlike AUTOIBRS.

When trying to wire up a scattered host AUTOIBRS version up to
kvm, I couldn't get past all the reverse_cpuid_check()
BUILD_BUGs demanding exclusivity between h/w and "Linux"
(s/w) FEATUREs.

Is there an example of a scattered feature that gets both its
boot_cpu_has() and guest_cpuid_has() satisfied in the same build?

If not, I'll resubmit like this original submission - AUTOIBRS in
a separate h/w leaf.

Thanks,

Kim

2022-11-16 12:21:04

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH 1/3] x86/cpufeatures: Add support for cpuid leaf 80000021/EAX (FeatureExt2Eax)

On Tue, Nov 15, 2022 at 05:10:50PM -0600, Kim Phillips wrote:
> When trying to wire up a scattered host AUTOIBRS version up to
> kvm, I couldn't get past all the reverse_cpuid_check()
> BUILD_BUGs demanding exclusivity between h/w and "Linux"
> (s/w) FEATUREs.

I guess something like below.

Sean, can you pls check the KVM bits whether I've done them all right?

In any case, it seems to work, guest has:

processor : 0
vendor_id : AuthenticAMD
cpu family : 25
model : 1
model name : AMD EPYC-Milan Processor
stepping : 1
flags : ... autoibrs ...

---
From: Borislav Petkov <[email protected]>
Date: Wed, 16 Nov 2022 12:50:08 +0100
Subject: [PATCH] x86/cpu, kvm: Add X86_FEATURE_AUTOIBRS

Add AMD AutoIBRS feature bit support. Use a synthetic bit as this is the
first bit from the 0x80000021 leaf.

Add the corresponding word to KVM's feature machinery so that the bit
gets advertized into the guest too.

Signed-off-by: Borislav Petkov <[email protected]>
---
arch/x86/include/asm/cpufeatures.h | 1 +
arch/x86/kernel/cpu/scattered.c | 1 +
arch/x86/kvm/cpuid.c | 2 ++
arch/x86/kvm/reverse_cpuid.h | 18 ++++++++++++------
4 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
index 2bc1557dc203..2cf102911241 100644
--- a/arch/x86/include/asm/cpufeatures.h
+++ b/arch/x86/include/asm/cpufeatures.h
@@ -306,6 +306,7 @@
#define X86_FEATURE_RSB_VMEXIT_LITE (11*32+17) /* "" Fill RSB on VM exit when EIBRS is enabled */
#define X86_FEATURE_SGX_EDECCSSA (11*32+18) /* "" SGX EDECCSSA user leaf function */
#define X86_FEATURE_CALL_DEPTH (11*32+19) /* "" Call depth tracking for RSB stuffing */
+#define X86_FEATURE_AUTOIBRS (11*32+20) /* AMD Automatic IBRS */

/* Intel-defined CPU features, CPUID level 0x00000007:1 (EAX), word 12 */
#define X86_FEATURE_AVX_VNNI (12*32+ 4) /* AVX VNNI instructions */
diff --git a/arch/x86/kernel/cpu/scattered.c b/arch/x86/kernel/cpu/scattered.c
index f53944fb8f7f..e20117658c5b 100644
--- a/arch/x86/kernel/cpu/scattered.c
+++ b/arch/x86/kernel/cpu/scattered.c
@@ -45,6 +45,7 @@ static const struct cpuid_bit cpuid_bits[] = {
{ X86_FEATURE_CPB, CPUID_EDX, 9, 0x80000007, 0 },
{ X86_FEATURE_PROC_FEEDBACK, CPUID_EDX, 11, 0x80000007, 0 },
{ X86_FEATURE_MBA, CPUID_EBX, 6, 0x80000008, 0 },
+ { X86_FEATURE_AUTOIBRS, CPUID_EAX, 8, 0x80000021, 0 },
{ X86_FEATURE_PERFMON_V2, CPUID_EAX, 0, 0x80000022, 0 },
{ X86_FEATURE_AMD_LBR_V2, CPUID_EAX, 1, 0x80000022, 0 },
{ 0, 0, 0, 0, 0 }
diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index c92c49a0b35b..050bca360731 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -750,6 +750,8 @@ void kvm_set_cpu_caps(void)
kvm_cpu_cap_clear(X86_FEATURE_RDTSCP);
kvm_cpu_cap_clear(X86_FEATURE_RDPID);
}
+
+ kvm_cpu_cap_init_scattered(CPUID_8000_0021_EAX, SF(AUTOIBRS));
}
EXPORT_SYMBOL_GPL(kvm_set_cpu_caps);

diff --git a/arch/x86/kvm/reverse_cpuid.h b/arch/x86/kvm/reverse_cpuid.h
index 4e5b8444f161..c4801ac84a4a 100644
--- a/arch/x86/kvm/reverse_cpuid.h
+++ b/arch/x86/kvm/reverse_cpuid.h
@@ -13,6 +13,7 @@
*/
enum kvm_only_cpuid_leafs {
CPUID_12_EAX = NCAPINTS,
+ CPUID_8000_0021_EAX,
NR_KVM_CPU_CAPS,

NKVMCAPINTS = NR_KVM_CPU_CAPS - NCAPINTS,
@@ -25,6 +26,9 @@ enum kvm_only_cpuid_leafs {
#define KVM_X86_FEATURE_SGX2 KVM_X86_FEATURE(CPUID_12_EAX, 1)
#define KVM_X86_FEATURE_SGX_EDECCSSA KVM_X86_FEATURE(CPUID_12_EAX, 11)

+/* AMD-defined Extended Feature 2 EAX, CPUID level 0x80000021 (EAX) */
+#define KVM_X86_FEATURE_AUTOIBRS KVM_X86_FEATURE(CPUID_8000_0021_EAX, 8)
+
struct cpuid_reg {
u32 function;
u32 index;
@@ -49,6 +53,7 @@ static const struct cpuid_reg reverse_cpuid[] = {
[CPUID_7_1_EAX] = { 7, 1, CPUID_EAX},
[CPUID_12_EAX] = {0x00000012, 0, CPUID_EAX},
[CPUID_8000_001F_EAX] = {0x8000001f, 0, CPUID_EAX},
+ [CPUID_8000_0021_EAX] = {0x80000021, 0, CPUID_EAX},
};

/*
@@ -75,12 +80,13 @@ static __always_inline void reverse_cpuid_check(unsigned int x86_leaf)
*/
static __always_inline u32 __feature_translate(int x86_feature)
{
- if (x86_feature == X86_FEATURE_SGX1)
- return KVM_X86_FEATURE_SGX1;
- else if (x86_feature == X86_FEATURE_SGX2)
- return KVM_X86_FEATURE_SGX2;
- else if (x86_feature == X86_FEATURE_SGX_EDECCSSA)
- return KVM_X86_FEATURE_SGX_EDECCSSA;
+ switch (x86_feature) {
+ case X86_FEATURE_SGX1: return KVM_X86_FEATURE_SGX1;
+ case X86_FEATURE_SGX2: return KVM_X86_FEATURE_SGX2;
+ case X86_FEATURE_SGX_EDECCSSA: return KVM_X86_FEATURE_SGX_EDECCSSA;
+ case X86_FEATURE_AUTOIBRS: return KVM_X86_FEATURE_AUTOIBRS;
+ default: break;
+ }

return x86_feature;
}
--
2.35.1

--
Regards/Gruss,
Boris.

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

2022-11-16 20:51:42

by Sean Christopherson

[permalink] [raw]
Subject: Re: [PATCH 1/3] x86/cpufeatures: Add support for cpuid leaf 80000021/EAX (FeatureExt2Eax)

On Wed, Nov 16, 2022, Borislav Petkov wrote:
> On Tue, Nov 15, 2022 at 05:10:50PM -0600, Kim Phillips wrote:
> > When trying to wire up a scattered host AUTOIBRS version up to
> > kvm, I couldn't get past all the reverse_cpuid_check()
> > BUILD_BUGs demanding exclusivity between h/w and "Linux"
> > (s/w) FEATUREs.

FWIW, it's not exclusivity per se, it's to ensure that any CPUID bits KVM wants
to advertise to userspace uses the architectural definition and not the kernel's
software defined info. This allows KVM to do things like

if (guest_cpuid_has(X86_FEATURE_AUTOIBRS))

and guarantee that the lookup on guest CPUID, which follows the architectural
layout, will look at the correct leaf+subleaf+reg+bit.

> I guess something like below.
>
> Sean, can you pls check the KVM bits whether I've done them all right?

Looks correct.

2022-11-16 21:37:59

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH 1/3] x86/cpufeatures: Add support for cpuid leaf 80000021/EAX (FeatureExt2Eax)

On Wed, Nov 16, 2022 at 08:22:23PM +0000, Sean Christopherson wrote:
> > Sean, can you pls check the KVM bits whether I've done them all right?
>
> Looks correct.

Thanks!

--
Regards/Gruss,
Boris.

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