[PATCH v2 for-4.22] x86/cpu-policy: set up host policy earlier

Jan Beulich posted 1 patch 1 week, 3 days ago
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/b329e8d1-5f6b-445d-acbd-d60a2d644c1a@suse.com
There is a newer version of this series
[PATCH v2 for-4.22] x86/cpu-policy: set up host policy earlier
Posted by Jan Beulich 1 week, 3 days ago
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(). Then make sure .x86_capability[]
and host policy remain in sync when setup_{force,clear}_cpu_cap() are
used.

Rename the function now it's no longer static.

Fixes: 894bd7617924 ("x86/Intel: use host CPU policy for ARAT checking")
Reported-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
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.
---
v2: Call calculate_host_cpu_policy() from setup_{force,clear}_cpu_cap()
    instead of from init_guest_cpu_policies().

--- a/xen/arch/x86/cpu/common.c
+++ b/xen/arch/x86/cpu/common.c
@@ -66,8 +66,10 @@ void __init setup_clear_cpu_cap(unsigned
 	__clear_bit(cap, boot_cpu_data.x86_capability);
 	dfs = x86_cpu_policy_lookup_deep_deps(cap);
 
-	if (!dfs)
+	if (!dfs) {
+		calculate_host_cpu_policy();
 		return;
+	}
 
 	for (i = 0; i < FSCAPINTS; ++i) {
 		cleared_caps[i] |= dfs[i];
@@ -78,6 +80,8 @@ void __init setup_clear_cpu_cap(unsigned
 		       __builtin_return_address(0),
 		       i, forced_caps[i] & dfs[i]);
 	}
+
+	calculate_host_cpu_policy();
 }
 
 void __init setup_force_cpu_cap(unsigned int cap)
@@ -92,6 +96,8 @@ void __init setup_force_cpu_cap(unsigned
 	}
 
 	__set_bit(cap, boot_cpu_data.x86_capability);
+
+	calculate_host_cpu_policy();
 }
 
 bool __init is_forced_cpu_cap(unsigned int cap)
@@ -586,6 +590,8 @@ void identify_cpu(struct cpuinfo_x86 *c)
 	}
 
 	/* Now the feature flags better reflect actual CPU features! */
+	if (c == &boot_cpu_data)
+		calculate_host_cpu_policy();
 
 	xstate_init(c);
 
--- a/xen/arch/x86/cpu-policy.c
+++ b/xen/arch/x86/cpu-policy.c
@@ -359,11 +359,18 @@ void calculate_raw_cpu_policy(void)
     /* Was already added by probe_cpuid_faulting() */
 }
 
-static void __init calculate_host_policy(void)
+void __init calculate_host_cpu_policy(void)
 {
     struct cpu_policy *p = &host_cpu_policy;
     unsigned int max_extd_leaf;
 
+    /*
+     * Bail if the raw policy wasn't set up yet. At least recalculate_xstate()
+     * can't be used yet in that case.
+     */
+    if ( !raw_cpu_policy.basic.max_leaf )
+        return;
+
     *p = raw_cpu_policy;
 
     p->basic.max_leaf =
@@ -904,8 +911,6 @@ static void __init calculate_hvm_def_pol
 
 void __init init_guest_cpu_policies(void)
 {
-    calculate_host_policy();
-
     if ( IS_ENABLED(CONFIG_PV) )
     {
         calculate_pv_max_policy();
--- a/xen/arch/x86/include/asm/cpu-policy.h
+++ b/xen/arch/x86/include/asm/cpu-policy.h
@@ -30,4 +30,10 @@ void recalculate_cpuid_policy(struct dom
  */
 void calculate_raw_cpu_policy(void);
 
+/*
+ * Collect the host CPU policy.  Called after collecting enough CPUID output,
+ * and again after all feature overrides have been put in place.
+ */
+void calculate_host_cpu_policy(void);
+
 #endif /* X86_CPU_POLICY_H */

Re: [PATCH v2 for-4.22] x86/cpu-policy: set up host policy earlier
Posted by Jan Beulich 6 days, 2 hours ago
On 03.07.2026 08:26, Jan Beulich wrote:
> --- a/xen/arch/x86/cpu/common.c
> +++ b/xen/arch/x86/cpu/common.c
> @@ -66,8 +66,10 @@ void __init setup_clear_cpu_cap(unsigned
>  	__clear_bit(cap, boot_cpu_data.x86_capability);
>  	dfs = x86_cpu_policy_lookup_deep_deps(cap);
>  
> -	if (!dfs)
> +	if (!dfs) {
> +		calculate_host_cpu_policy();
>  		return;
> +	}
>  
>  	for (i = 0; i < FSCAPINTS; ++i) {
>  		cleared_caps[i] |= dfs[i];
> @@ -78,6 +80,8 @@ void __init setup_clear_cpu_cap(unsigned
>  		       __builtin_return_address(0),
>  		       i, forced_caps[i] & dfs[i]);
>  	}
> +
> +	calculate_host_cpu_policy();
>  }
>  
>  void __init setup_force_cpu_cap(unsigned int cap)
> @@ -92,6 +96,8 @@ void __init setup_force_cpu_cap(unsigned
>  	}
>  
>  	__set_bit(cap, boot_cpu_data.x86_capability);
> +
> +	calculate_host_cpu_policy();
>  }

During the call yesterday you raised concern over the overhead these calls
may cause. How about we make this call dependent upon cap < FSCAPINTS * 32?
Synthetic features and bug flags don't have dependencies, as they don't
have representation in the policies in the first place.

Whether something similar would make sense in setup_clear_cpu_cap() I'm
inclined to question. We typically "force" synthetic features and bug
flags, while we typically "clear" bits having representation in policy.
Plus if at all such a check would then likely better live ahead of the
call to x86_cpu_policy_lookup_deep_deps() anyway.

Jan
Re: [PATCH v2 for-4.22] x86/cpu-policy: set up host policy earlier
Posted by Teddy Astie 1 week, 2 days ago
Le 03/07/2026 à 08:28, Jan Beulich a écrit :
> 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(). Then make sure .x86_capability[]
> and host policy remain in sync when setup_{force,clear}_cpu_cap() are
> used.
> 
> Rename the function now it's no longer static.
> 
> Fixes: 894bd7617924 ("x86/Intel: use host CPU policy for ARAT checking")
> Reported-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> ---
> 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.
> ---
> v2: Call calculate_host_cpu_policy() from setup_{force,clear}_cpu_cap()
>      instead of from init_guest_cpu_policies().
> 
> --- a/xen/arch/x86/cpu/common.c
> +++ b/xen/arch/x86/cpu/common.c
> @@ -66,8 +66,10 @@ void __init setup_clear_cpu_cap(unsigned
>   	__clear_bit(cap, boot_cpu_data.x86_capability);
>   	dfs = x86_cpu_policy_lookup_deep_deps(cap);
>   
> -	if (!dfs)
> +	if (!dfs) {
> +		calculate_host_cpu_policy();
>   		return;
> +	}
>   
>   	for (i = 0; i < FSCAPINTS; ++i) {
>   		cleared_caps[i] |= dfs[i];
> @@ -78,6 +80,8 @@ void __init setup_clear_cpu_cap(unsigned
>   		       __builtin_return_address(0),
>   		       i, forced_caps[i] & dfs[i]);
>   	}
> +
> +	calculate_host_cpu_policy();
>   }
>   
>   void __init setup_force_cpu_cap(unsigned int cap)
> @@ -92,6 +96,8 @@ void __init setup_force_cpu_cap(unsigned
>   	}
>   
>   	__set_bit(cap, boot_cpu_data.x86_capability);
> +
> +	calculate_host_cpu_policy();
>   }
>   
>   bool __init is_forced_cpu_cap(unsigned int cap)
> @@ -586,6 +590,8 @@ void identify_cpu(struct cpuinfo_x86 *c)
>   	}
>   
>   	/* Now the feature flags better reflect actual CPU features! */
> +	if (c == &boot_cpu_data)
> +		calculate_host_cpu_policy();
>   
>   	xstate_init(c);
>   
> --- a/xen/arch/x86/cpu-policy.c
> +++ b/xen/arch/x86/cpu-policy.c
> @@ -359,11 +359,18 @@ void calculate_raw_cpu_policy(void)
>       /* Was already added by probe_cpuid_faulting() */
>   }
>   
> -static void __init calculate_host_policy(void)
> +void __init calculate_host_cpu_policy(void)
>   {
>       struct cpu_policy *p = &host_cpu_policy;
>       unsigned int max_extd_leaf;
>   
> +    /*
> +     * Bail if the raw policy wasn't set up yet. At least recalculate_xstate()
> +     * can't be used yet in that case.
> +     */
> +    if ( !raw_cpu_policy.basic.max_leaf )
> +        return;
> +
>       *p = raw_cpu_policy;
>   
>       p->basic.max_leaf =
> @@ -904,8 +911,6 @@ static void __init calculate_hvm_def_pol
>   
>   void __init init_guest_cpu_policies(void)
>   {
> -    calculate_host_policy();
> -
>       if ( IS_ENABLED(CONFIG_PV) )
>       {
>           calculate_pv_max_policy();
> --- a/xen/arch/x86/include/asm/cpu-policy.h
> +++ b/xen/arch/x86/include/asm/cpu-policy.h
> @@ -30,4 +30,10 @@ void recalculate_cpuid_policy(struct dom
>    */
>   void calculate_raw_cpu_policy(void);
>   
> +/*
> + * Collect the host CPU policy.  Called after collecting enough CPUID output,
> + * and again after all feature overrides have been put in place.
> + */
> +void calculate_host_cpu_policy(void);
> +
>   #endif /* X86_CPU_POLICY_H */
> 

That looks quite confusing.

As I understand it, (ideally) host CPU policy (which I suppose, contains 
"non synthetic" cpu featureset) is about the CPUID info that Xen is 
allowed to see and rely on, while "synthetic CPU capabilities" is more 
about Xen decisions.

If "cpu featureset info" and "host cpu policy" can get out of sync, then 
recalculating host cpu policy each time we make a modification of the 
cpu featureset sounds like a bad idea, and we probably should try to not 
end up in this situation in the first place by not having host CPU 
policy and featureset competing on what hardware feature Xen should rely on.

Teddy
Re: [PATCH v2 for-4.22] x86/cpu-policy: set up host policy earlier
Posted by Jan Beulich 1 week ago
On 03.07.2026 15:37, Teddy Astie wrote:
> Le 03/07/2026 à 08:28, Jan Beulich a écrit :
>> --- a/xen/arch/x86/include/asm/cpu-policy.h
>> +++ b/xen/arch/x86/include/asm/cpu-policy.h
>> @@ -30,4 +30,10 @@ void recalculate_cpuid_policy(struct dom
>>    */
>>   void calculate_raw_cpu_policy(void);
>>   
>> +/*
>> + * Collect the host CPU policy.  Called after collecting enough CPUID output,
>> + * and again after all feature overrides have been put in place.
>> + */
>> +void calculate_host_cpu_policy(void);
>> +
>>   #endif /* X86_CPU_POLICY_H */
>>
> 
> That looks quite confusing.
> 
> As I understand it, (ideally) host CPU policy (which I suppose, contains 
> "non synthetic" cpu featureset) is about the CPUID info that Xen is 
> allowed to see and rely on, while "synthetic CPU capabilities" is more 
> about Xen decisions.

I'm not sure what the long term plans are here. Likely synthetic bits need
to remain separate from the policy, yes. Yet I don't see how synthetic bits
matter here - x86_cpu_featureset_to_policy() doesn't consume any of the
synthetic parts (those covered by X86_NR_SYNTH and X86_NR_BUG).

> If "cpu featureset info" and "host cpu policy" can get out of sync, then 
> recalculating host cpu policy each time we make a modification of the 
> cpu featureset sounds like a bad idea, and we probably should try to not 
> end up in this situation in the first place by not having host CPU 
> policy and featureset competing on what hardware feature Xen should rely on.

Well, that'll require a lot of work, as effectively to avoid this the
duplication needs to go away. Hence adjusting the policy when the featureset
changes is (imo) the least bad option for the time being.

Jan

Re: [PATCH v2 for-4.22] x86/cpu-policy: set up host policy earlier
Posted by Oleksii Kurochko 1 week, 3 days ago

On 7/3/26 8:26 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(). Then make sure .x86_capability[]
> and host policy remain in sync when setup_{force,clear}_cpu_cap() are
> used.
> 
> Rename the function now it's no longer static.
> 
> Fixes: 894bd7617924 ("x86/Intel: use host CPU policy for ARAT checking")
> Reported-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Release-Acked-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>

~ Oleksii