[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20250515235210.ooq7ogymcdvbtakd@desk>
Date: Thu, 15 May 2025 16:52:10 -0700
From: Pawan Gupta <pawan.kumar.gupta@...ux.intel.com>
To: David Kaplan <david.kaplan@....com>
Cc: Thomas Gleixner <tglx@...utronix.de>, Borislav Petkov <bp@...en8.de>,
Peter Zijlstra <peterz@...radead.org>,
Josh Poimboeuf <jpoimboe@...nel.org>,
Ingo Molnar <mingo@...hat.com>,
Dave Hansen <dave.hansen@...ux.intel.com>, x86@...nel.org,
"H. Peter Anvin" <hpa@...or.com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] x86/bugs: Restructure ITS mitigation
On Thu, May 15, 2025 at 08:47:56AM -0500, David Kaplan wrote:
> Restructure the ITS mitigation to use select/update/apply functions like
> the other mitigations.
>
> There is a particularly complex interaction between ITS and Retbleed as CDT
> (Call Depth Tracking) is a mitigation for both, and either its=stuff or
> retbleed=stuff will attempt to enable CDT.
>
> retbleed_update_mitigation() runs first and will check the necessary
> pre-conditions for CDT if either ITS or Retbleed stuffing is selected. If
> checks pass and ITS stuffing is selected, it will select stuffing for
> Retbleed as well.
>
> its_update_mitigation() runs after and will either select stuffing if
> retbleed stuffing was enabled, or fall back to the default (aligned thunks)
> if stuffing could not be enabled.
>
> Enablement of CDT is done exclusively in retbleed_apply_mitigation().
> its_apply_mitigation() is only used to enable aligned thunks.
>
> Signed-off-by: David Kaplan <david.kaplan@....com>
> ---
> arch/x86/kernel/cpu/bugs.c | 167 ++++++++++++++++++++-----------------
> 1 file changed, 90 insertions(+), 77 deletions(-)
>
> diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
> index dd8b50b4ceaa..db26fb5a0a13 100644
> --- a/arch/x86/kernel/cpu/bugs.c
> +++ b/arch/x86/kernel/cpu/bugs.c
> @@ -92,6 +92,8 @@ static void __init bhi_select_mitigation(void);
> static void __init bhi_update_mitigation(void);
> static void __init bhi_apply_mitigation(void);
> static void __init its_select_mitigation(void);
> +static void __init its_update_mitigation(void);
> +static void __init its_apply_mitigation(void);
>
> /* The base value of the SPEC_CTRL MSR without task-specific bits set */
> u64 x86_spec_ctrl_base;
> @@ -235,6 +237,11 @@ void __init cpu_select_mitigations(void)
> * spectre_v2=ibrs.
> */
> retbleed_update_mitigation();
> + /*
> + * its_update_mitigation() depends on spectre_v2_update_mitigation()
> + * and retbleed_update_mitigation().
> + */
> + its_update_mitigation();
>
> /*
> * spectre_v2_user_update_mitigation() depends on
> @@ -263,6 +270,7 @@ void __init cpu_select_mitigations(void)
> srbds_apply_mitigation();
> srso_apply_mitigation();
> gds_apply_mitigation();
> + its_apply_mitigation();
> bhi_apply_mitigation();
> }
>
> @@ -1125,6 +1133,14 @@ enum retbleed_mitigation {
> RETBLEED_MITIGATION_STUFF,
> };
>
> +enum its_mitigation {
> + ITS_MITIGATION_OFF,
> + ITS_MITIGATION_AUTO,
> + ITS_MITIGATION_VMEXIT_ONLY,
> + ITS_MITIGATION_ALIGNED_THUNKS,
> + ITS_MITIGATION_RETPOLINE_STUFF,
> +};
This is in between retbleed declarations, I would suggest to move this
before retbleed mitigation starts.
enum its_mitigation {
ITS_MITIGATION_OFF,
ITS_MITIGATION_AUTO,
ITS_MITIGATION_VMEXIT_ONLY,
ITS_MITIGATION_ALIGNED_THUNKS,
ITS_MITIGATION_RETPOLINE_STUFF,
};
static enum its_mitigation its_mitigation __ro_after_init =
IS_ENABLED(CONFIG_MITIGATION_ITS) ? ITS_MITIGATION_AUTO : ITS_MITIGATION_OFF;
#undef pr_fmt
#define pr_fmt(fmt) "RETBleed: " fmt
enum retbleed_mitigation {
> static const char * const retbleed_strings[] = {
> [RETBLEED_MITIGATION_NONE] = "Vulnerable",
> [RETBLEED_MITIGATION_UNRET] = "Mitigation: untrained return thunk",
> @@ -1137,6 +1153,9 @@ static const char * const retbleed_strings[] = {
> static enum retbleed_mitigation retbleed_mitigation __ro_after_init =
> IS_ENABLED(CONFIG_MITIGATION_RETBLEED) ? RETBLEED_MITIGATION_AUTO : RETBLEED_MITIGATION_NONE;
>
> +static enum its_mitigation its_mitigation __ro_after_init =
> + IS_ENABLED(CONFIG_MITIGATION_ITS) ? ITS_MITIGATION_AUTO : ITS_MITIGATION_OFF;
Ditto.
> static int __ro_after_init retbleed_nosmt = false;
>
> static int __init retbleed_parse_cmdline(char *str)
> @@ -1242,11 +1261,19 @@ static void __init retbleed_update_mitigation(void)
> /*
> * retbleed=stuff is only allowed on Intel. If stuffing can't be used
> * then a different mitigation will be selected below.
> + *
> + * its=stuff will also attempt to enable stuffing.
> */
> - if (retbleed_mitigation == RETBLEED_MITIGATION_STUFF) {
> + if (retbleed_mitigation == RETBLEED_MITIGATION_STUFF ||
> + its_mitigation == ITS_MITIGATION_RETPOLINE_STUFF) {
> if (spectre_v2_enabled != SPECTRE_V2_RETPOLINE) {
SPECTRE_V2_EIBRS_RETPOLINE also enables retpoline.
> pr_err("WARNING: retbleed=stuff depends on spectre_v2=retpoline\n");
This can be updated to:
pr_err("WARNING: retbleed=stuff depends on retpoline\n");
> retbleed_mitigation = RETBLEED_MITIGATION_AUTO;
> + } else {
> + if (retbleed_mitigation != RETBLEED_MITIGATION_STUFF)
> + pr_info("Retbleed mitigation updated to stuffing\n");
> +
> + retbleed_mitigation = RETBLEED_MITIGATION_STUFF;
> }
> }
> /*
> @@ -1338,20 +1365,6 @@ static void __init retbleed_apply_mitigation(void)
> #undef pr_fmt
> #define pr_fmt(fmt) "ITS: " fmt
>
> -enum its_mitigation_cmd {
> - ITS_CMD_OFF,
> - ITS_CMD_ON,
> - ITS_CMD_VMEXIT,
> - ITS_CMD_RSB_STUFF,
> -};
> -
> -enum its_mitigation {
> - ITS_MITIGATION_OFF,
> - ITS_MITIGATION_VMEXIT_ONLY,
> - ITS_MITIGATION_ALIGNED_THUNKS,
> - ITS_MITIGATION_RETPOLINE_STUFF,
> -};
> -
> static const char * const its_strings[] = {
> [ITS_MITIGATION_OFF] = "Vulnerable",
> [ITS_MITIGATION_VMEXIT_ONLY] = "Mitigation: Vulnerable, KVM: Not affected",
> @@ -1359,11 +1372,6 @@ static const char * const its_strings[] = {
> [ITS_MITIGATION_RETPOLINE_STUFF] = "Mitigation: Retpolines, Stuffing RSB",
> };
>
> -static enum its_mitigation its_mitigation __ro_after_init = ITS_MITIGATION_ALIGNED_THUNKS;
> -
> -static enum its_mitigation_cmd its_cmd __ro_after_init =
> - IS_ENABLED(CONFIG_MITIGATION_ITS) ? ITS_CMD_ON : ITS_CMD_OFF;
> -
> static int __init its_parse_cmdline(char *str)
> {
> if (!str)
> @@ -1375,16 +1383,16 @@ static int __init its_parse_cmdline(char *str)
> }
>
> if (!strcmp(str, "off")) {
> - its_cmd = ITS_CMD_OFF;
> + its_mitigation = ITS_MITIGATION_OFF;
> } else if (!strcmp(str, "on")) {
> - its_cmd = ITS_CMD_ON;
> + its_mitigation = ITS_MITIGATION_ALIGNED_THUNKS;
> } else if (!strcmp(str, "force")) {
> - its_cmd = ITS_CMD_ON;
> + its_mitigation = ITS_MITIGATION_ALIGNED_THUNKS;
> setup_force_cpu_bug(X86_BUG_ITS);
> } else if (!strcmp(str, "vmexit")) {
> - its_cmd = ITS_CMD_VMEXIT;
> + its_mitigation = ITS_MITIGATION_VMEXIT_ONLY;
> } else if (!strcmp(str, "stuff")) {
> - its_cmd = ITS_CMD_RSB_STUFF;
> + its_mitigation = ITS_MITIGATION_RETPOLINE_STUFF;
> } else {
> pr_err("Ignoring unknown indirect_target_selection option (%s).", str);
> }
> @@ -1395,85 +1403,90 @@ early_param("indirect_target_selection", its_parse_cmdline);
>
> static void __init its_select_mitigation(void)
> {
> - enum its_mitigation_cmd cmd = its_cmd;
> -
> if (!boot_cpu_has_bug(X86_BUG_ITS) || cpu_mitigations_off()) {
> its_mitigation = ITS_MITIGATION_OFF;
> return;
> }
>
> - /* Retpoline+CDT mitigates ITS, bail out */
> - if (boot_cpu_has(X86_FEATURE_RETPOLINE) &&
> - boot_cpu_has(X86_FEATURE_CALL_DEPTH)) {
> - its_mitigation = ITS_MITIGATION_RETPOLINE_STUFF;
> - goto out;
> - }
> + if (its_mitigation == ITS_MITIGATION_AUTO)
> + its_mitigation = ITS_MITIGATION_ALIGNED_THUNKS;
> +
> + if (its_mitigation == ITS_MITIGATION_OFF)
> + return;
>
> - /* Exit early to avoid irrelevant warnings */
> - if (cmd == ITS_CMD_OFF) {
> - its_mitigation = ITS_MITIGATION_OFF;
> - goto out;
> - }
> - if (spectre_v2_enabled == SPECTRE_V2_NONE) {
> - pr_err("WARNING: Spectre-v2 mitigation is off, disabling ITS\n");
> - its_mitigation = ITS_MITIGATION_OFF;
> - goto out;
> - }
> if (!IS_ENABLED(CONFIG_MITIGATION_RETPOLINE) ||
> !IS_ENABLED(CONFIG_MITIGATION_RETHUNK)) {
> pr_err("WARNING: ITS mitigation depends on retpoline and rethunk support\n");
> its_mitigation = ITS_MITIGATION_OFF;
> - goto out;
> + return;
> }
> +
> if (IS_ENABLED(CONFIG_DEBUG_FORCE_FUNCTION_ALIGN_64B)) {
> pr_err("WARNING: ITS mitigation is not compatible with CONFIG_DEBUG_FORCE_FUNCTION_ALIGN_64B\n");
> its_mitigation = ITS_MITIGATION_OFF;
> - goto out;
> - }
> - if (boot_cpu_has(X86_FEATURE_RETPOLINE_LFENCE)) {
> - pr_err("WARNING: ITS mitigation is not compatible with lfence mitigation\n");
> - its_mitigation = ITS_MITIGATION_OFF;
> - goto out;
> + return;
> }
>
> - if (cmd == ITS_CMD_RSB_STUFF &&
> - (!boot_cpu_has(X86_FEATURE_RETPOLINE) || !IS_ENABLED(CONFIG_MITIGATION_CALL_DEPTH_TRACKING))) {
> + if (its_mitigation == ITS_MITIGATION_RETPOLINE_STUFF &&
> + !IS_ENABLED(CONFIG_MITIGATION_CALL_DEPTH_TRACKING)) {
> pr_err("RSB stuff mitigation not supported, using default\n");
> - cmd = ITS_CMD_ON;
> + its_mitigation = ITS_MITIGATION_ALIGNED_THUNKS;
This and ...
> }
>
> - switch (cmd) {
> - case ITS_CMD_OFF:
> + if (its_mitigation == ITS_MITIGATION_VMEXIT_ONLY &&
> + !boot_cpu_has_bug(X86_BUG_ITS_NATIVE_ONLY))
> + its_mitigation = ITS_MITIGATION_ALIGNED_THUNKS;
... this are essentially resetting the mitigation to default. This will be
more clear if you could change the mitigation to ITS_MITIGATION_AUTO here,
and at the end have:
if (its_mitigation == ITS_MITIGATION_AUTO)
its_mitigation = ITS_MITIGATION_ALIGNED_THUNKS;
> +
> +}
> +
> +static void __init its_update_mitigation(void)
> +{
> + if (!boot_cpu_has_bug(X86_BUG_ITS) || cpu_mitigations_off())
> + return;
> +
> + switch (spectre_v2_enabled) {
> + case SPECTRE_V2_NONE:
> + pr_err("WARNING: Spectre-v2 mitigation is off, disabling ITS\n");
> its_mitigation = ITS_MITIGATION_OFF;
> break;
> - case ITS_CMD_VMEXIT:
> - if (boot_cpu_has_bug(X86_BUG_ITS_NATIVE_ONLY)) {
> - its_mitigation = ITS_MITIGATION_VMEXIT_ONLY;
> - goto out;
> - }
> - fallthrough;
> - case ITS_CMD_ON:
> - its_mitigation = ITS_MITIGATION_ALIGNED_THUNKS;
> - if (!boot_cpu_has(X86_FEATURE_RETPOLINE))
> - setup_force_cpu_cap(X86_FEATURE_INDIRECT_THUNK_ITS);
> - setup_force_cpu_cap(X86_FEATURE_RETHUNK);
> - set_return_thunk(its_return_thunk);
> + case SPECTRE_V2_RETPOLINE:
Also SPECTRE_V2_EIBRS_RETPOLINE.
> + /* Retpoline+CDT mitigates ITS */
> + if (retbleed_mitigation == RETBLEED_MITIGATION_STUFF)
> + its_mitigation = ITS_MITIGATION_RETPOLINE_STUFF;
> break;
> - case ITS_CMD_RSB_STUFF:
> - its_mitigation = ITS_MITIGATION_RETPOLINE_STUFF;
> - setup_force_cpu_cap(X86_FEATURE_RETHUNK);
> - setup_force_cpu_cap(X86_FEATURE_CALL_DEPTH);
> - set_return_thunk(call_depth_return_thunk);
> - if (retbleed_mitigation == RETBLEED_MITIGATION_NONE) {
> - retbleed_mitigation = RETBLEED_MITIGATION_STUFF;
> - pr_info("Retbleed mitigation updated to stuffing\n");
> - }
> + case SPECTRE_V2_LFENCE:
> + case SPECTRE_V2_EIBRS_LFENCE:
> + pr_err("WARNING: ITS mitigation is not compatible with lfence mitigation\n");
> + its_mitigation = ITS_MITIGATION_OFF;
> + break;
> + default:
> break;
> }
> -out:
> +
> + /*
> + * retbleed_update_mitigation() will try to do stuffing if its=stuff.
> + * If it can't, such as if spectre_v2!=retpoline, then fall back to
> + * aligned thunks.
> + */
> + if (its_mitigation == ITS_MITIGATION_RETPOLINE_STUFF &&
> + retbleed_mitigation != RETBLEED_MITIGATION_STUFF)
> + its_mitigation = ITS_MITIGATION_ALIGNED_THUNKS;
The =stuff mitigation depends on retpoline, not really on retbleed.
Shouldn't this be handled in the switch (spectre_v2_enabled) above?
> pr_info("%s\n", its_strings[its_mitigation]);
> }
>
> +static void __init its_apply_mitigation(void)
> +{
> + /* its=stuff forces retbleed stuffing and is enabled there. */
Oh, this is why you are depending on retbleed_mitigation above, this part is
a bit confusing.
Will think about it more later, trying to have a couple of days off.
> + if (its_mitigation != ITS_MITIGATION_ALIGNED_THUNKS)
> + return;
> +
> + if (!boot_cpu_has(X86_FEATURE_RETPOLINE))
> + setup_force_cpu_cap(X86_FEATURE_INDIRECT_THUNK_ITS);
> +
> + setup_force_cpu_cap(X86_FEATURE_RETHUNK);
> + set_return_thunk(its_return_thunk);
> +}
Powered by blists - more mailing lists