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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 01.07.2026 17:21, Andrew Cooper wrote:
> On 07/04/2026 7:46 am, Jan Beulich wrote:
>> --- 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

Sure.

>> --- 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.

And it's not meant to stay here, once that cleanup work has further
progressed. We did discuss this on one of the recent calls. I'm happy
to ...

> 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.

... go further, just that I don't think what you suggest below will
work.

> 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.

You realize that there was no "switching cpu_has_* to use host_policy"?
There's no representation of leaf 6 in x86_capabilities[], and it
seemed awkward to introduce it when we're really moving to using the
policies.

> 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. 

Except that this isn't going to be enough. It may not need to be all
of what calculate_host_policy() currently does, but at the very least
recalculate_xstate() will imo need calling. Yet any cpu_has_* used
there and changing value would also have a knock-on effect. Overall it
then looks less error prone to me to call calculate_host_cpu_policy()
from there (then very obviously eliminating the need for the call from
init_guest_cpu_policies()), accepting the slight extra overhead.

> 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.

As per above multiple feature words may be affected by a single change.
Hence I think this isn't an option.

Jan