Re: [RFC PATCH 27/34] x86/bugs: Add attack vector controls for spectre_v1

[Dave Hansen <dave.hansen@intel.com>]

On 9/12/24 12:08, David Kaplan wrote:
> @@ -1114,6 +1114,9 @@ static void __init spectre_v1_select_mitigation(void)
>  {
>  	if (!boot_cpu_has_bug(X86_BUG_SPECTRE_V1) || cpu_mitigations_off())
>  		spectre_v1_mitigation = SPECTRE_V1_MITIGATION_NONE;
> +
> +	if (!should_mitigate_vuln(SPECTRE_V1))
> +		spectre_v1_mitigation = SPECTRE_V1_MITIGATION_NONE;
>  }

Just a high-level comment on this: usually in a well-structured series
that has sufficient refactoring, if you start to look at the end of the
series, things start to fall into place.  The series (at some point)
stops adding complexity things get simpler.

I don't really see that inflection point here.

For instance, I would have expected cpu_mitigations_off() to be
consulted in should_mitigate_vuln() so that some of the individual sites
can go away.

There's also added complexity from having 'enum vulnerabilities' which
basically duplicate the X86_BUG_* space.  If the infrastructure was, for
instance, built around X86_BUG bits, it might have enabled this patch to
be something like:

-  	if (!boot_cpu_has_bug(X86_BUG_SPECTRE_V1) ||
-	    cpu_mitigations_off())
+	if (!should_mitigate_vuln(X86_BUG_SPECTRE_V1))
		spectre_v1_mitigation = SPECTRE_V1_MITIGATION_NONE;

I'm also not sure this series takes the right approach in representing
logic in data structures versus code.

For instance, this:

> +	case MDS:
> +	case TAA:
> +	case MMIO:
> +	case RFDS:
> +	case SRBDS:
> +	case GDS:
> +		return cpu_mitigate_attack_vector(CPU_MITIGATE_USER_KERNEL) ||
> +			cpu_mitigate_attack_vector(CPU_MITIGATE_GUEST_HOST) ||
> +			cpu_mitigate_attack_vector(CPU_MITIGATE_USER_USER) ||
> +			cpu_mitigate_attack_vector(CPU_MITIGATE_GUEST_GUEST);

We've _tended_ to represent these in data structure like cpu_vuln_whitelist.

struct whatever var[] =
   MACRO(MDS,  USER_KERNEL | GUEST_HOST | USER_USER | GUEST_GUEST)
   MACRO(MMIO, USER_KERNEL | GUEST_HOST | USER_USER | GUEST_GUEST)
   ...
};

But I do like the concept of users being focused on the attack vectors
in general.  That part is really nice.

As we talk about this at Plumbers, we probably need to be focused on
whether users want this new attack-vector-based selection mechanism or
the old style.  Because adding the attack-vector style is going to add
complexity any way we do it.