Re: [PATCH] x86/cpu-policy: Extend the guest max policy max leaf/subleaves

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Tue, Oct 29, 2024 at 05:55:05PM +0000, Andrew Cooper wrote:
> We already have one migration case opencoded (feat.max_subleaf).  A more
> recent discovery is that we advertise x2APIC to guests without ensuring that
> we provide max_leaf >= 0xb.
> 
> In general, any leaf known to Xen can be safely configured by the toolstack if
> it doesn't violate other constraints.
> 
> Therefore, introduce guest_common_{max,default}_leaves() to generalise the
> special case we currently have for feat.max_subleaf, in preparation to be able
> to provide x2APIC topology in leaf 0xb even on older hardware.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> ---
> CC: Jan Beulich <JBeulich@xxxxxxxx>
> CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> CC: Alejandro Vallejo <alejandro.vallejo@xxxxxxxxx>
> 
> On a KabyLake I have to hand, here's the delta in what xen-cpuid -p reports:
> 
>   git diff --no-index xen-cpuid-p-{before,after}.log
>   diff --git a/xen-cpuid-p-before.log b/xen-cpuid-p-after.log
>   index 5a76d05..24e22be 100644
>   --- a/xen-cpuid-p-before.log
>   +++ b/xen-cpuid-p-after.log
>   @@ -61,7 +61,7 @@ Host policy: 33 leaves, 2 MSRs
>      index    -> value
>      000000ce -> 0000000080000000
>      0000010a -> 000000000e000c04
>   -PV Max policy: 33 leaves, 2 MSRs
>   +PV Max policy: 58 leaves, 2 MSRs
>     CPUID:
>      leaf     subleaf  -> eax      ebx      ecx      edx
>      00000000:ffffffff -> 0000000d:756e6547:6c65746e:49656e69
>   @@ -75,7 +75,7 @@ PV Max policy: 33 leaves, 2 MSRs
>      0000000d:00000000 -> 00000007:00000000:00000340:00000000
>      0000000d:00000001 -> 00000007:00000000:00000000:00000000
>      0000000d:00000002 -> 00000100:00000240:00000000:00000000
>   -  80000000:ffffffff -> 80000008:00000000:00000000:00000000
>   +  80000000:ffffffff -> 80000021:00000000:00000000:00000000
>      80000001:ffffffff -> 00000000:00000000:00000123:28100800
>      80000002:ffffffff -> 65746e49:2952286c:6f655820:2952286e
>      80000003:ffffffff -> 55504320:2d334520:30333231:20367620
>   @@ -87,7 +87,7 @@ PV Max policy: 33 leaves, 2 MSRs
>      index    -> value
>      000000ce -> 0000000080000000
>      0000010a -> 000000001c020004
>   -HVM Max policy: 35 leaves, 2 MSRs
>   +HVM Max policy: 60 leaves, 2 MSRs
>     CPUID:
>      leaf     subleaf  -> eax      ebx      ecx      edx
>      00000000:ffffffff -> 0000000d:756e6547:6c65746e:49656e69
>   @@ -103,7 +103,7 @@ HVM Max policy: 35 leaves, 2 MSRs
>      0000000d:00000002 -> 00000100:00000240:00000000:00000000
>      0000000d:00000003 -> 00000040:000003c0:00000000:00000000
>      0000000d:00000004 -> 00000040:00000400:00000000:00000000
>   -  80000000:ffffffff -> 80000008:00000000:00000000:00000000
>   +  80000000:ffffffff -> 80000021:00000000:00000000:00000000
>      80000001:ffffffff -> 00000000:00000000:00000123:2c100800
>      80000002:ffffffff -> 65746e49:2952286c:6f655820:2952286e
>      80000003:ffffffff -> 55504320:2d334520:30333231:20367620
> ---
>  xen/arch/x86/cpu-policy.c | 39 +++++++++++++++++++++++++--------------
>  1 file changed, 25 insertions(+), 14 deletions(-)
> 
> diff --git a/xen/arch/x86/cpu-policy.c b/xen/arch/x86/cpu-policy.c
> index b6d9fad56773..78bc9872b09a 100644
> --- a/xen/arch/x86/cpu-policy.c
> +++ b/xen/arch/x86/cpu-policy.c
> @@ -391,6 +391,27 @@ static void __init calculate_host_policy(void)
>      p->platform_info.cpuid_faulting = cpu_has_cpuid_faulting;
>  }
>  
> +/*
> + * Guest max policies can have any max leaf/subleaf within bounds.
> + *
> + * - Some incoming VMs have a larger-than-necessary feat max_subleaf.
> + * - Some VMs we'd like to synthesise leaves not present on the host.
> + */
> +static void __init guest_common_max_leaves(struct cpu_policy *p)
> +{
> +    p->basic.max_leaf       = ARRAY_SIZE(p->basic.raw) - 1;
> +    p->feat.max_subleaf     = ARRAY_SIZE(p->feat.raw) - 1;
> +    p->extd.max_leaf        = 0x80000000U + ARRAY_SIZE(p->extd.raw) - 1;
> +}
> +
> +/* Guest default policies inherit the host max leaf/subleaf settings. */
> +static void __init guest_common_default_leaves(struct cpu_policy *p)
> +{
> +    p->basic.max_leaf       = host_cpu_policy.basic.max_leaf;
> +    p->feat.max_subleaf     = host_cpu_policy.feat.max_subleaf;
> +    p->extd.max_leaf        = host_cpu_policy.extd.max_leaf;
> +}

I think this what I'm going to ask is future work.  After the
modifications done to the host policy by max functions
(calculate_{hvm,pv}_max_policy()) won't the max {sub,}leaf adjustments
better be done taking into account the contents of the policy, rather
than capping to the host values?

(note this comment is strictly for guest_common_default_leaves(), the
max version is fine using ARRAY_SIZE).

Thanks, Roger.