Messages in this thread Patch in this message | | | Date | Thu, 31 Mar 2022 01:46:10 -0700 | From | Pawan Gupta <> | Subject | Re: [PATCH v2] x86/speculation/srbds: do not try to turn mitigation off when not supported |
| |
On Thu, Mar 31, 2022 at 09:48:14AM +0200, Ricardo Cañuelo wrote: >Borislav Petkov <bp@alien8.de> writes: >> So we could also do the below to denote what the situation is and >> therefore clear the bug flag for such CPUs. >> >> The thing is: I want this to be as clear as possible because bugs.c is >> already a nightmare and just slapping more logic to it without properly >> thinking it through is going to be a serious pain to deal with later... > >Thanks Boris, > >I agree that the more explicit the better, I'll give this a try. I saw >Pawan's suggestion as well but that one is similar to the originally >proposed patch in that the logic/checks are split between two functions, >this solution based on clearing the bug flag seems clearer considering >the comment just before the code block: > > /* > * Check to see if this is one of the MDS_NO systems supporting > * TSX that are only exposed to SRBDS when TSX is enabled. > */
If we clear the bug flag and not write to the MSR to disable the microcode mitigation, there will be unnecessary performance loss due to microcode mitigation. Specifically RDRAND/RDSEED/EGET_KEY will run slower.
Yes, the logic/checks between two functions is messy.
Does this simplify things a bit?
--- diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c index 6296e1ebed1d..0be6c0eabb71 100644 --- a/arch/x86/kernel/cpu/bugs.c +++ b/arch/x86/kernel/cpu/bugs.c @@ -431,34 +431,22 @@ static const char * const srbds_strings[] = { [SRBDS_MITIGATION_HYPERVISOR] = "Unknown: Dependent on hypervisor status", }; -static bool srbds_off; +static bool srbds_dis_ucode_mitigation; void update_srbds_msr(void) { u64 mcu_ctrl; - if (!boot_cpu_has_bug(X86_BUG_SRBDS)) - return; - - if (boot_cpu_has(X86_FEATURE_HYPERVISOR)) - return; - - if (srbds_mitigation == SRBDS_MITIGATION_UCODE_NEEDED) + if (!boot_cpu_has_bug(X86_BUG_SRBDS) || + !boot_cpu_has(X86_FEATURE_SRBDS_CTRL)) return; rdmsrl(MSR_IA32_MCU_OPT_CTRL, mcu_ctrl); - switch (srbds_mitigation) { - case SRBDS_MITIGATION_OFF: - case SRBDS_MITIGATION_TSX_OFF: + if (srbds_dis_ucode_mitigation) mcu_ctrl |= RNGDS_MITG_DIS; - break; - case SRBDS_MITIGATION_FULL: + else mcu_ctrl &= ~RNGDS_MITG_DIS; - break; - default: - break; - } wrmsrl(MSR_IA32_MCU_OPT_CTRL, mcu_ctrl); } @@ -481,9 +469,13 @@ static void __init srbds_select_mitigation(void) srbds_mitigation = SRBDS_MITIGATION_HYPERVISOR; else if (!boot_cpu_has(X86_FEATURE_SRBDS_CTRL)) srbds_mitigation = SRBDS_MITIGATION_UCODE_NEEDED; - else if (cpu_mitigations_off() || srbds_off) + else if (cpu_mitigations_off()) srbds_mitigation = SRBDS_MITIGATION_OFF; + if (srbds_mitigation == SRBDS_MITIGATION_OFF || + srbds_mitigation == SRBDS_MITIGATION_TSX_OFF) + srbds_dis_ucode_mitigation = true; + update_srbds_msr(); pr_info("%s\n", srbds_strings[srbds_mitigation]); } @@ -496,7 +488,9 @@ static int __init srbds_parse_cmdline(char *str) if (!boot_cpu_has_bug(X86_BUG_SRBDS)) return 0; - srbds_off = !strcmp(str, "off"); + if (!strcmp(str, "off")) + srbds_mitigation = SRBDS_MITIGATION_OFF; + return 0; } early_param("srbds", srbds_parse_cmdline);
| |