Re: [PATCH v3 00/35] x86/bugs: Attack vector controls

[Borislav Petkov]

On Fri, Jan 10, 2025 at 12:36:27AM -0800, Pawan Gupta wrote:
> Below patch does some of the above for spectre_v1 mitigation. Please share
> your feedback if this is a good direction to take.

This has come up in the past. My problem with looping over an array of
function pointers and calling them is debuggability: it is not as easy as it
is currently with plain functions.

So we'd need a well-intergrated way to enable debugging of what gets called
when.

> diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
> index ad63b5678250..d719450f89c2 100644
> --- a/arch/x86/kernel/cpu/bugs.c
> +++ b/arch/x86/kernel/cpu/bugs.c
> @@ -53,8 +53,24 @@
>   * mitigation option.
>   */
>  
> -static void __init spectre_v1_select_mitigation(void);
> -static void __init spectre_v1_apply_mitigation(void);
> +struct cpu_mitigation {
> +	unsigned int x86_bug;			/* X86_BUG_* */
> +	int mitigation;				/* AUTO, FULL, NONE etc. */
> +	int mitigates;				/* Attack vectors to mitigate e.g. user->kernel */

This should be an enum.

Even better: you can group those arrays by attack vectors so you simply run
the respective array when you want to enable an attack vector. And then it is
obvious and self-documenting.

> +	char **strings;				/* sysfs status strings */
> +	void (*parse_cmdline)		(struct cpu_mitigation *m);
> +	void (*select_mitigation)	(struct cpu_mitigation *m);
> +	void (*update_mitigation)	(struct cpu_mitigation *m);
> +	void (*apply_mitigation)	(struct cpu_mitigation *m);
> +	void (*ap_init_mitigation)	(struct cpu_mitigation *m); /* Mitigation during secondary CPU init e.g. MSR writes */
> +	void (*smt_update_mitigation)	(struct cpu_mitigation *m); /* Mitigation update on SMT toggle */
> +	void (*sysfs_show_mitigation)	(char *buf);
> +	void (*s3_suspend)		(struct cpu_mitigation *m); /* Mitigation quirks on S3 suspend */
> +	void (*s3_resume)		(struct cpu_mitigation *m); /* Mitigation quirks on S3 resume */

Too many "mitigation" words in there. Perhaps drop "mitigation" from the
function ptr names.

And no side comments pls - all ontop.

Otherwise, once the dust settles here, this could be a nice cleanup.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette