Re: [PATCH] x86/cpu-policy: set up host policy earlier

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 07/04/2026 7:46 am, Jan Beulich wrote:
> In order to use cpu_has_* expanding to host_cpu_policy.*, the host policy
> needs setting up alongside boot_cpu_data.x86_capability[]. Arrange for
> that towards the end of identify_cpu().
>
> Fixes: 894bd7617924 ("x86/Intel: use host CPU policy for ARAT checking")
> Reported-by: Marek Marczykowski-Górecki <marmarek@xxxxxxxxxxxxxxxxxxxxxx>
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> ---
> In the longer run we may need to (also) update the host policy from
> setup_{force,clear}_cpu_cap().
>
> This really was part of one of the XSTATE cleanup patches at the head of
> the AMX series. This explains the specific placement ahead of the call to
> xstate_init()). Likely anything from there downwards shouldn't really
> live in identify_cpu() anyway.
>
> --- a/xen/arch/x86/cpu/common.c
> +++ b/xen/arch/x86/cpu/common.c
> @@ -586,6 +586,8 @@ void identify_cpu(struct cpuinfo_x86 *c)
>       }
>  
>       /* Now the feature flags better reflect actual CPU features! */
> +     if (c == &boot_cpu_data)
> +             calculate_host_policy();

calculate_host_cpu_policy() now it's not local to cpu-policy.c

>  
>       xstate_init(c);
>  
> --- a/xen/arch/x86/cpu-policy.c
> +++ b/xen/arch/x86/cpu-policy.c
> @@ -359,7 +359,7 @@ void calculate_raw_cpu_policy(void)
>      /* Was already added by probe_cpuid_faulting() */
>  }
>  
> -static void __init calculate_host_policy(void)
> +void __init calculate_host_policy(void)
>  {
>      struct cpu_policy *p = &host_cpu_policy;
>      unsigned int max_extd_leaf;
> @@ -904,6 +904,7 @@ static void __init calculate_hvm_def_pol
>  
>  void __init init_guest_cpu_policies(void)
>  {
> +    /* Do this a 2nd time to account for setup_{clear,force}_cpu_cap() uses. 
> */
>      calculate_host_policy();

I'm sorry, but no.  The cleanup work to CPUID handling is in an attempt
to purge this antipattern.

Double-initialising data structures *is* the entire reason we have
order-of-boot problems in the first place.  Having a structure which is
initialised but wrong is itself wrong.

Really, I should insist on a revert of switching cpu_has_* to use
host_policy.  It was not adequately tested and is not compatible with
how Xen starts up.

But as I expect you're going to object to that feedback, I'll tolerate
setup_*_cpu_cap() keeping the host policy correct as they're called. 
The easy option is to call x86_cpu_featureset_to_policy() each time. 
The lower overhead option would be to have a table similar to
cpu_user_regs_gpr_offsets[] and use that to locate the feature word in a
policy object as it's being edited, although it's yet another thing to
remember to keep in sync when adding new words.

~Andrew