On Wed, Feb 26, 2025 at 06:45:40PM +0000, Patrick Bellasi wrote: > > On Tue, Feb 18, 2025 at 02:42:57PM +0000, Patrick Bellasi wrote: > > > Maybe a small improvement we could add on top is to have a separate and > > > dedicated cmdline option? > > > > > > Indeed, with `X86_FEATURE_SRSO_USER_KERNEL_NO` we are not effectively using an > > > IBPB on VM-Exit anymore. Something like the diff down below? > > > > Except that I don't see the point of this yet one more cmdline option. Our > > mitigations options space is a nightmare. Why do we want to add another one? > > The changelog of the following patch provides the motivations. > > Do you think something like the following self contained change can be added on > top of your change? > > Best, > Patrick > > --- > From 62bd6151cdb5f8e3322d8c91166cf89b6ed9f5b6 Mon Sep 17 00:00:00 2001 > From: Patrick Bellasi <derkling@xxxxxxxxxx> > Date: Mon, 24 Feb 2025 17:41:30 +0000 > Subject: [PATCH] x86/bugs: Add explicit BP_SPEC_REDUCE cmdline > > Some AMD CPUs are vulnerable to SRSO only limited to the Guest->Host > attack vector. When no command line options are specified, the default > SRSO mitigation on these CPUs is "safe-ret", which is optimized to use > "ibpb-vmexit". A further optimization, introduced in [1], replaces IBPB > on VM-Exits with the more efficient BP_SPEC_REDUCE mitigation when the > CPU reports X86_FEATURE_SRSO_BP_SPEC_REDUCE support. > > The current implementation in bugs.c automatically selects the best > mitigation for a CPU when no command line options are provided. However, > it lacks the ability to explicitly choose between IBPB and > BP_SPEC_REDUCE. > In some scenarios it could be interesting to mitigate SRSO only when the > low overhead of BP_SPEC_REDUCE is available, without overwise falling > back to an IBPB at each VM-Exit. To add more details about this, we are using ASI as our main mitigation for SRSO. However, it's likely that bp-spec-reduce is cheaper, so we basically want to always use bp-spec-reduce if available, if not, we don't want the ibpb-on-vmexit or safe-ret as they are a lot more expensive than ASI. So we want the cmdline option to basically say only use bp-spec-reduce if it's available, but don't fallback if it isn't. On the other hand we are enlighting ASI to skip mitigating SRSO if X86_FEATURE_SRSO_BP_SPEC_REDUCE is enabled > More in general, an explicit control is valuable for testing, > benchmarking, and comparing the behavior and overhead of IBPB versus > BP_SPEC_REDUCE. > > Add a new kernel cmdline option to explicitly select BP_SPEC_REDUCE. > Do that with a minimal change that does not affect the current SafeRET > "fallback logic". Do warn when reduced speculation is required but the > support not available and properly report the vulnerability reason. > > [1] https://lore.kernel.org/lkml/20250218111306.GFZ7RrQh3RD4JKj1lu@fat_crate.local/ > > Signed-off-by: Patrick Bellasi <derkling@xxxxxxxxxx> > --- > arch/x86/kernel/cpu/bugs.c | 24 ++++++++++++++++++------ > 1 file changed, 18 insertions(+), 6 deletions(-) > > diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c > index 7fafd98368b91..2d785b2ca4e2e 100644 > --- a/arch/x86/kernel/cpu/bugs.c > +++ b/arch/x86/kernel/cpu/bugs.c > @@ -2523,6 +2523,7 @@ enum srso_mitigation { > SRSO_MITIGATION_IBPB, > SRSO_MITIGATION_IBPB_ON_VMEXIT, > SRSO_MITIGATION_BP_SPEC_REDUCE, > + SRSO_MITIGATION_BP_SPEC_REDUCE_NA, > }; > > enum srso_mitigation_cmd { > @@ -2531,6 +2532,7 @@ enum srso_mitigation_cmd { > SRSO_CMD_SAFE_RET, > SRSO_CMD_IBPB, > SRSO_CMD_IBPB_ON_VMEXIT, > + SRSO_CMD_BP_SPEC_REDUCE, > }; > > static const char * const srso_strings[] = { > @@ -2542,6 +2544,7 @@ static const char * const srso_strings[] = { > [SRSO_MITIGATION_IBPB] = "Mitigation: IBPB", > [SRSO_MITIGATION_IBPB_ON_VMEXIT] = "Mitigation: IBPB on VMEXIT only", > [SRSO_MITIGATION_BP_SPEC_REDUCE] = "Mitigation: Reduced Speculation", > + [SRSO_MITIGATION_BP_SPEC_REDUCE_NA] = "Vulnerable: Reduced Speculation, not available", > }; > > static enum srso_mitigation srso_mitigation __ro_after_init = SRSO_MITIGATION_NONE; > @@ -2562,6 +2565,8 @@ static int __init srso_parse_cmdline(char *str) > srso_cmd = SRSO_CMD_IBPB; > else if (!strcmp(str, "ibpb-vmexit")) > srso_cmd = SRSO_CMD_IBPB_ON_VMEXIT; > + else if (!strcmp(str, "bp-spec-reduce")) > + srso_cmd = SRSO_CMD_BP_SPEC_REDUCE; > else > pr_err("Ignoring unknown SRSO option (%s).", str); > > @@ -2672,12 +2677,8 @@ static void __init srso_select_mitigation(void) > > ibpb_on_vmexit: > case SRSO_CMD_IBPB_ON_VMEXIT: > - if (boot_cpu_has(X86_FEATURE_SRSO_BP_SPEC_REDUCE)) { > - pr_notice("Reducing speculation to address VM/HV SRSO attack vector.\n"); > - srso_mitigation = SRSO_MITIGATION_BP_SPEC_REDUCE; > - break; > - } > - > + if (boot_cpu_has(X86_FEATURE_SRSO_BP_SPEC_REDUCE)) > + goto bp_spec_reduce; > if (IS_ENABLED(CONFIG_MITIGATION_IBPB_ENTRY)) { > if (has_microcode) { > setup_force_cpu_cap(X86_FEATURE_IBPB_ON_VMEXIT); > @@ -2694,6 +2695,17 @@ static void __init srso_select_mitigation(void) > pr_err("WARNING: kernel not compiled with MITIGATION_IBPB_ENTRY.\n"); > } > break; > + > + case SRSO_CMD_BP_SPEC_REDUCE: > + if (boot_cpu_has(X86_FEATURE_SRSO_BP_SPEC_REDUCE)) { > +bp_spec_reduce: We can put this label above 'case SRSO_CMD_BP_SPEC_REDUCE' for consistency with the ibpb_on_vmexit label. The X86_FEATURE_SRSO_BP_SPEC_REDUCE check will be repeated but it shouldn't matter in practice and the compiler will probably optimize it anyway. > + pr_notice("Reducing speculation to address VM/HV SRSO attack vector.\n"); > + srso_mitigation = SRSO_MITIGATION_BP_SPEC_REDUCE; > + break; > + } else { > + srso_mitigation = SRSO_MITIGATION_BP_SPEC_REDUCE_NA; Why do we need SRSO_MITIGATION_BP_SPEC_REDUCE_NA? It seems like in other cases, if some requirements are not met, we just leave srso_mitigation set SRSO_MITIGATION_NONE. > + pr_warn("BP_SPEC_REDUCE not supported!\n"); > + } > default: > break; > } > -- > 2.48.1.711.g2feabab25a-goog
