2023-08-13 11:02:30

by Borislav Petkov

[permalink] [raw]
Subject: [PATCH] x86/srso: Disable the mitigation on unaffected configurations

From: "Borislav Petkov (AMD)" <[email protected]>

Skip the srso cmd line parsing which is not needed on Zen1/2 with SMT
disabled and with the proper microcode applied (latter should be the
case anyway) as those are not affected.

Fixes: 5a15d8348881 ("x86/srso: Tie SBPB bit setting to microcode patch detection")
Signed-off-by: Borislav Petkov (AMD) <[email protected]>
---
arch/x86/kernel/cpu/bugs.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
index d02f73c5339d..8959a1b9fb80 100644
--- a/arch/x86/kernel/cpu/bugs.c
+++ b/arch/x86/kernel/cpu/bugs.c
@@ -2418,8 +2418,10 @@ static void __init srso_select_mitigation(void)
* IBPB microcode has been applied.
*/
if ((boot_cpu_data.x86 < 0x19) &&
- (!cpu_smt_possible() || (cpu_smt_control == CPU_SMT_DISABLED)))
+ (!cpu_smt_possible() || (cpu_smt_control == CPU_SMT_DISABLED))) {
setup_force_cpu_cap(X86_FEATURE_SRSO_NO);
+ goto pred_cmd;
+ }
}

if (retbleed_mitigation == RETBLEED_MITIGATION_IBPB) {
@@ -2696,6 +2698,9 @@ static ssize_t retbleed_show_state(char *buf)

static ssize_t srso_show_state(char *buf)
{
+ if (boot_cpu_has(X86_FEATURE_SRSO_NO))
+ return sysfs_emit(buf, "Not affected\n");
+
return sysfs_emit(buf, "%s%s\n",
srso_strings[srso_mitigation],
(cpu_has_ibpb_brtype_microcode() ? "" : ", no microcode"));
--
2.42.0.rc0.25.ga82fb66fed25



2023-08-14 07:13:34

by Nikolay Borisov

[permalink] [raw]
Subject: Re: [PATCH] x86/srso: Disable the mitigation on unaffected configurations



On 13.08.23 г. 13:45 ч., Borislav Petkov wrote:
> From: "Borislav Petkov (AMD)" <[email protected]>
>
> Skip the srso cmd line parsing which is not needed on Zen1/2 with SMT
> disabled and with the proper microcode applied (latter should be the
> case anyway) as those are not affected.
>
> Fixes: 5a15d8348881 ("x86/srso: Tie SBPB bit setting to microcode patch detection")
> Signed-off-by: Borislav Petkov (AMD) <[email protected]>
> ---
> arch/x86/kernel/cpu/bugs.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
> index d02f73c5339d..8959a1b9fb80 100644
> --- a/arch/x86/kernel/cpu/bugs.c
> +++ b/arch/x86/kernel/cpu/bugs.c
> @@ -2418,8 +2418,10 @@ static void __init srso_select_mitigation(void)
> * IBPB microcode has been applied.
> */
> if ((boot_cpu_data.x86 < 0x19) &&
> - (!cpu_smt_possible() || (cpu_smt_control == CPU_SMT_DISABLED)))
> + (!cpu_smt_possible() || (cpu_smt_control == CPU_SMT_DISABLED))) {
> setup_force_cpu_cap(X86_FEATURE_SRSO_NO);
> + goto pred_cmd;

Actually can't you simply return, given that zen1/2 never have the SBPB
flag set? Only zen3/4 with the updated microcode?


> + }
> }
>
> if (retbleed_mitigation == RETBLEED_MITIGATION_IBPB) {
> @@ -2696,6 +2698,9 @@ static ssize_t retbleed_show_state(char *buf)
>
> static ssize_t srso_show_state(char *buf)
> {
> + if (boot_cpu_has(X86_FEATURE_SRSO_NO))
> + return sysfs_emit(buf, "Not affected\n");
> +
> return sysfs_emit(buf, "%s%s\n",
> srso_strings[srso_mitigation],
> (cpu_has_ibpb_brtype_microcode() ? "" : ", no microcode"));

Subject: [tip: x86/urgent] x86/srso: Disable the mitigation on unaffected configurations

The following commit has been merged into the x86/urgent branch of tip:

Commit-ID: e9fbc47b818b964ddff5df5b2d5c0f5f32f4a147
Gitweb: https://git.kernel.org/tip/e9fbc47b818b964ddff5df5b2d5c0f5f32f4a147
Author: Borislav Petkov (AMD) <[email protected]>
AuthorDate: Sun, 13 Aug 2023 12:39:34 +02:00
Committer: Borislav Petkov (AMD) <[email protected]>
CommitterDate: Mon, 14 Aug 2023 11:28:51 +02:00

x86/srso: Disable the mitigation on unaffected configurations

Skip the srso cmd line parsing which is not needed on Zen1/2 with SMT
disabled and with the proper microcode applied (latter should be the
case anyway) as those are not affected.

Fixes: 5a15d8348881 ("x86/srso: Tie SBPB bit setting to microcode patch detection")
Signed-off-by: Borislav Petkov (AMD) <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
arch/x86/kernel/cpu/bugs.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
index d02f73c..6c04aef 100644
--- a/arch/x86/kernel/cpu/bugs.c
+++ b/arch/x86/kernel/cpu/bugs.c
@@ -2418,8 +2418,10 @@ static void __init srso_select_mitigation(void)
* IBPB microcode has been applied.
*/
if ((boot_cpu_data.x86 < 0x19) &&
- (!cpu_smt_possible() || (cpu_smt_control == CPU_SMT_DISABLED)))
+ (!cpu_smt_possible() || (cpu_smt_control == CPU_SMT_DISABLED))) {
setup_force_cpu_cap(X86_FEATURE_SRSO_NO);
+ return;
+ }
}

if (retbleed_mitigation == RETBLEED_MITIGATION_IBPB) {
@@ -2696,6 +2698,9 @@ static ssize_t retbleed_show_state(char *buf)

static ssize_t srso_show_state(char *buf)
{
+ if (boot_cpu_has(X86_FEATURE_SRSO_NO))
+ return sysfs_emit(buf, "Not affected\n");
+
return sysfs_emit(buf, "%s%s\n",
srso_strings[srso_mitigation],
(cpu_has_ibpb_brtype_microcode() ? "" : ", no microcode"));

2023-08-14 20:35:44

by Josh Poimboeuf

[permalink] [raw]
Subject: Re: [PATCH] x86/srso: Disable the mitigation on unaffected configurations

On Mon, Aug 14, 2023 at 09:39:11AM +0300, Nikolay Borisov wrote:
>
>
> On 13.08.23 г. 13:45 ч., Borislav Petkov wrote:
> > From: "Borislav Petkov (AMD)" <[email protected]>
> >
> > Skip the srso cmd line parsing which is not needed on Zen1/2 with SMT
> > disabled and with the proper microcode applied (latter should be the
> > case anyway) as those are not affected.
> >
> > Fixes: 5a15d8348881 ("x86/srso: Tie SBPB bit setting to microcode patch detection")
> > Signed-off-by: Borislav Petkov (AMD) <[email protected]>
> > ---
> > arch/x86/kernel/cpu/bugs.c | 7 ++++++-
> > 1 file changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
> > index d02f73c5339d..8959a1b9fb80 100644
> > --- a/arch/x86/kernel/cpu/bugs.c
> > +++ b/arch/x86/kernel/cpu/bugs.c
> > @@ -2418,8 +2418,10 @@ static void __init srso_select_mitigation(void)
> > * IBPB microcode has been applied.
> > */
> > if ((boot_cpu_data.x86 < 0x19) &&
> > - (!cpu_smt_possible() || (cpu_smt_control == CPU_SMT_DISABLED)))
> > + (!cpu_smt_possible() || (cpu_smt_control == CPU_SMT_DISABLED))) {
> > setup_force_cpu_cap(X86_FEATURE_SRSO_NO);
> > + goto pred_cmd;
>
> Actually can't you simply return, given that zen1/2 never have the SBPB flag
> set? Only zen3/4 with the updated microcode?

Tangentially, the 'cpu_smt_control == CPU_SMT_DISABLED' check is wrong,
as SMT could still get enabled at runtime and SRSO would be exposed.

Also is there a reason to re-use the hardware SRSO_NO bit rather than
clear the bug bit? That seems cleaner, then you wouldn't need this
hack:

> > + if (boot_cpu_has(X86_FEATURE_SRSO_NO))
> > + return sysfs_emit(buf, "Not affected\n");
> > +

--
Josh

2023-08-14 21:24:45

by Josh Poimboeuf

[permalink] [raw]
Subject: Re: [PATCH] x86/srso: Disable the mitigation on unaffected configurations

On Mon, Aug 14, 2023 at 10:25:45PM +0200, Borislav Petkov wrote:
> On Mon, Aug 14, 2023 at 01:08:13PM -0700, Josh Poimboeuf wrote:
> > Tangentially, the 'cpu_smt_control == CPU_SMT_DISABLED' check is wrong,
> > as SMT could still get enabled at runtime and SRSO would be exposed.
>
> Well, even if it gets exposed, I don't think we can safely enable the
> mitigation at runtime as alternatives have run already.

Right, I wasn't suggesting to enable the mitigation at runtime. Rather,
enable the mitigation at boot time, if SMT is even possible. That's
what we've done for other mitigations. Better safe than sorry.

> I guess I could use CPU_SMT_FORCE_DISABLED here.

cpu_smt_possible() already does that.

> > Also is there a reason to re-use the hardware SRSO_NO bit
>
> Not a hardware bit - this is set by software - it is only allocated in
> the CPUID leaf for easier interaction with guests.

2. ENUMERATION OF NEW CAPABILITIES
AMD is defining three new CPUID bits to assist with the enumeration of capabilities related to SRSO:
CPUID Fn8000_0021_EAX[29] (SRSO_NO) – If this bit is 1, it indicates the CPU is not subject to the SRSO
vulnerability.
CPUID Fn8000_0021_EAX[28] (IBPB_BRTYPE) – If this bit is 1, it indicates that MSR 49h (PRED_CMD) bit 0
(IBPB) flushes all branch type predictions from the CPU branch predictor.
CPUID Fn8000_0021_EAX[27] (SBPB)

> > rather than clear the bug bit?
>
> We don't clear the X86_BUGs. Ever. The logic is that if the CPU matches
> an affected CPU, that flag remains to show that it is potentially
> affected.

Hm, ok. I thought that was the point of the vulnerabilities file.

> /sys/devices/system/cpu/vulnerabilities/ tells you what the actual state
> is.

Since technically the CPU is affected, I'm thinking it should say
"Mitigation: SMT disabled" or such, instead of "Not affected".

> > That seems cleaner, then you wouldn't need this hack:
>
> Not a hack. This is just like the other "not affected" feature flags.

Hm? You mean the *_NO ones that determine whether the BUG bits get set
in the first place? How do they print "Not affected"?

--
Josh