Re: [PATCH v6 01/11] lib/x86: Relax checks about policy compatibility

[Jan Beulich <jbeulich@xxxxxxxx>]

On 01.10.2024 14:37, Alejandro Vallejo wrote:
> --- a/xen/lib/x86/policy.c
> +++ b/xen/lib/x86/policy.c
> @@ -15,7 +15,16 @@ int x86_cpu_policies_are_compatible(const struct 
> cpu_policy *host,
>  #define FAIL_MSR(m) \
>      do { e.msr = (m); goto out; } while ( 0 )
>  
> -    if ( guest->basic.max_leaf > host->basic.max_leaf )
> +    /*
> +     * Old AMD hardware doesn't expose topology information in leaf 0xb. We
> +     * want to emulate that leaf with credible information because it must be
> +     * present on systems in which we emulate the x2APIC.
> +     *
> +     * For that reason, allow the max basic guest leaf to be larger than the
> +     * hosts' up until 0xb.
> +     */
> +    if ( guest->basic.max_leaf > 0xb &&
> +         guest->basic.max_leaf > host->basic.max_leaf )
>          FAIL_CPUID(0, NA);
>  
>      if ( guest->feat.max_subleaf > host->feat.max_subleaf )

I'm concerned by this in multiple ways:

1) It's pretty ad hoc, and hence doesn't make clear how to deal with similar
situations in the future.

2) Why would we permit going up to leaf 0xb when x2APIC is off in the respective
leaf?

3) We similarly force a higher extended leaf in order to accommodate the LFENCE-
is-dispatch-serializing bit. Yet there's no similar extra logic there in the
function here.

4) While there the guest vs host check won't matter, the situation with AMX and
AVX10 leaves imo still wants considering here right away. IOW (taken together
with at least 3) above) I think we need to first settle on a model for
collectively all max (sub)leaf handling. That in particular needs to properly
spell out who's responsible for what (tool stack vs Xen).

Jan