Re: [PATCH] x86/bugs: Switch to "auto" when "ibrs" selected on Enhanced IBRS parts

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Jul 14, 2022 at 01:42:11PM +0200, Peter Zijlstra wrote:
On Thu, Jul 14, 2022 at 08:17:26AM -0300, Thadeu Lima de Souza Cascardo wrote:
On Wed, Jul 13, 2022 at 10:32:37PM -0700, Pawan Gupta wrote:
> Currently spectre_v2=ibrs forces write to MSR_IA32_SPEC_CTRL at every
> entry and exit. On Enhanced IBRS parts setting MSR_IA32_SPEC_CTRL[IBRS]
> only once at bootup is sufficient. MSR write at every kernel entry/exit
> incur unnecessary penalty that can be avoided.
>
> When Enhanced IBRS feature is present, switch from "ibrs" to "auto" mode
> so that appropriate mitigation is selected.
>
> Fixes: 7c693f54c873 ("x86/speculation: Add spectre_v2=ibrs option to support Kernel IBRS")
> Cc: stable@xxxxxxxxxxxxxxx # 5.10+
> Signed-off-by: Pawan Gupta <pawan.kumar.gupta@xxxxxxxxxxxxxxx>
> ---
>  arch/x86/kernel/cpu/bugs.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
> index 0dd04713434b..7d7ebfdfbeda 100644
> --- a/arch/x86/kernel/cpu/bugs.c
> +++ b/arch/x86/kernel/cpu/bugs.c
> @@ -1303,6 +1303,12 @@ static enum spectre_v2_mitigation_cmd __init spectre_v2_parse_cmdline(void)
>  		return SPECTRE_V2_CMD_AUTO;
>  	}
>
> +	if (cmd == SPECTRE_V2_CMD_IBRS && boot_cpu_has(X86_FEATURE_IBRS_ENHANCED)) {
> +		pr_err("%s selected but CPU supports Enhanced IBRS. Switching to AUTO select\n",
> +		       mitigation_options[i].option);
> +		return SPECTRE_V2_CMD_AUTO;
> +	}
> +
>  	spec_v2_print_cond(mitigation_options[i].option,
>  			   mitigation_options[i].secure);
>  	return cmd;
>
> base-commit: 72a8e05d4f66b5af7854df4490e3135168694b6b
> --
> 2.35.3
>
>

Shouldn't we just use the mitigation the user asked for if it is still
possible? We could add the warning advising the user that a different
mitigation could be used instead with less penalty, but if the user asked for
IBRS and that is available, it should be used.

One of the reasons for that is testing. I know it was useful enough for me and
it helped me find some bugs.

Yeah this; if the user asks for IBRS, we should give him IBRS. I hate
the 'I know better, let me change that for you' mentality.

If you want to do something, print a warning.

Fair enough, I will change that to a warning.

Thanks,
Pawan



[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux