When re-scanning features, forced caps are taken into account but unforced
such as this are not.  This causes BTC_NO to go missing, and for the system to
appear to have lost features.

The practical consequence of this observation is that all after-the-fact
adjustments to CPUID must be forced.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
---
 xen/arch/x86/cpu/amd.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/xen/arch/x86/cpu/amd.c b/xen/arch/x86/cpu/amd.c
index 9b02e1ba675c..8f468aaf0921 100644
--- a/xen/arch/x86/cpu/amd.c
+++ b/xen/arch/x86/cpu/amd.c
@@ -1225,8 +1225,9 @@ static void cf_check init_amd(struct cpuinfo_x86 *c)
 		 * Branch Type Confusion, but predate the allocation of the
 		 * BTC_NO bit.  Fill it back in if we're not virtualised.
 		 */
-		if (!cpu_has_hypervisor && !cpu_has(c, X86_FEATURE_BTC_NO))
-			__set_bit(X86_FEATURE_BTC_NO, c->x86_capability);
+		if (c == &boot_cpu_data && !cpu_has_hypervisor &&
+		    !cpu_has(c, X86_FEATURE_BTC_NO))
+			setup_force_cpu_cap(X86_FEATURE_BTC_NO);
 		break;
 	}
 
-- 
2.39.5

On 26.11.2025 14:22, Andrew Cooper wrote:
> When re-scanning features,

What exactly do you mean with this, outside of XenServer (i.e. upstream)? The
only thing I can think of is recheck_cpu_features(), which calls identify_cpu()
and hence init_amd(). Thus ...

> forced caps are taken into account but unforced
> such as this are not.  This causes BTC_NO to go missing, and for the system to
> appear to have lost features.

... I don't really follow where features might be lost.

Jan

On 26/11/2025 2:19 pm, Jan Beulich wrote:
> On 26.11.2025 14:22, Andrew Cooper wrote:
>> When re-scanning features,
> What exactly do you mean with this, outside of XenServer (i.e. upstream)? The
> only thing I can think of is recheck_cpu_features(), which calls identify_cpu()
> and hence init_amd(). Thus ...
>
>> forced caps are taken into account but unforced
>> such as this are not.  This causes BTC_NO to go missing, and for the system to
>> appear to have lost features.
> ... I don't really follow where features might be lost.

Well - it's a feature that we started upstreaming and I still hope to
finish in some copious free time.

Already upstream, we rescan the Raw CPU policy after microcode load. 
That has had fixes such as dis-engaging CPUID Masking/Overriding so the
Raw policy comes out accurate.

The next step (not upstream yet) is to regenerate the Host and Guest
policies.  I recently fixed a bug in XenServer's testing and noticed
that the underlying logic had bit-rotted quite a bit, hence this series.

The purpose is to be able to activate new features added by a late
microcode load, such as new speculative defences, or simply provide new
FOO_NO bits.

~Andrew

On 26.11.2025 16:12, Andrew Cooper wrote:
> On 26/11/2025 2:19 pm, Jan Beulich wrote:
>> On 26.11.2025 14:22, Andrew Cooper wrote:
>>> When re-scanning features,
>> What exactly do you mean with this, outside of XenServer (i.e. upstream)? The
>> only thing I can think of is recheck_cpu_features(), which calls identify_cpu()
>> and hence init_amd(). Thus ...
>>
>>> forced caps are taken into account but unforced
>>> such as this are not.  This causes BTC_NO to go missing, and for the system to
>>> appear to have lost features.
>> ... I don't really follow where features might be lost.
> 
> Well - it's a feature that we started upstreaming and I still hope to
> finish in some copious free time.
> 
> Already upstream, we rescan the Raw CPU policy after microcode load. 
> That has had fixes such as dis-engaging CPUID Masking/Overriding so the
> Raw policy comes out accurate.

Yet that doesn't take forced features into account afaics. So at the very
least this needs to come with a description which more accurately describes
what (if anything) is actually being fixed / altered upstream.

> The next step (not upstream yet) is to regenerate the Host and Guest
> policies.  I recently fixed a bug in XenServer's testing and noticed
> that the underlying logic had bit-rotted quite a bit, hence this series.
> 
> The purpose is to be able to activate new features added by a late
> microcode load, such as new speculative defences, or simply provide new
> FOO_NO bits.

Yes, that's a good goal.

Jan

On 26/11/2025 3:25 pm, Jan Beulich wrote:
> On 26.11.2025 16:12, Andrew Cooper wrote:
>> On 26/11/2025 2:19 pm, Jan Beulich wrote:
>>> On 26.11.2025 14:22, Andrew Cooper wrote:
>>>> When re-scanning features,
>>> What exactly do you mean with this, outside of XenServer (i.e. upstream)? The
>>> only thing I can think of is recheck_cpu_features(), which calls identify_cpu()
>>> and hence init_amd(). Thus ...
>>>
>>>> forced caps are taken into account but unforced
>>>> such as this are not.  This causes BTC_NO to go missing, and for the system to
>>>> appear to have lost features.
>>> ... I don't really follow where features might be lost.
>> Well - it's a feature that we started upstreaming and I still hope to
>> finish in some copious free time.
>>
>> Already upstream, we rescan the Raw CPU policy after microcode load. 
>> That has had fixes such as dis-engaging CPUID Masking/Overriding so the
>> Raw policy comes out accurate.
> Yet that doesn't take forced features into account afaics. So at the very
> least this needs to come with a description which more accurately describes
> what (if anything) is actually being fixed / altered upstream.

I don't know what more you want me to say.  It's not a problem per say
in upstream, but it does come about because BTC_NO is handled
inconsistently to the other FOO_NO bits.

recheck_cpu_features() papers over the issue by re-invoking
identify_cpu().  It's necessary for S3 resume because all of
init_$VENDOR() really is needed, but it looks bogus in
smp_store_cpu_info() because it's repeating work done immediately prior
in start_secondary().

~Andrew

On 26.11.2025 17:33, Andrew Cooper wrote:
> On 26/11/2025 3:25 pm, Jan Beulich wrote:
>> On 26.11.2025 16:12, Andrew Cooper wrote:
>>> On 26/11/2025 2:19 pm, Jan Beulich wrote:
>>>> On 26.11.2025 14:22, Andrew Cooper wrote:
>>>>> When re-scanning features,
>>>> What exactly do you mean with this, outside of XenServer (i.e. upstream)? The
>>>> only thing I can think of is recheck_cpu_features(), which calls identify_cpu()
>>>> and hence init_amd(). Thus ...
>>>>
>>>>> forced caps are taken into account but unforced
>>>>> such as this are not.  This causes BTC_NO to go missing, and for the system to
>>>>> appear to have lost features.
>>>> ... I don't really follow where features might be lost.
>>> Well - it's a feature that we started upstreaming and I still hope to
>>> finish in some copious free time.
>>>
>>> Already upstream, we rescan the Raw CPU policy after microcode load. 
>>> That has had fixes such as dis-engaging CPUID Masking/Overriding so the
>>> Raw policy comes out accurate.
>> Yet that doesn't take forced features into account afaics. So at the very
>> least this needs to come with a description which more accurately describes
>> what (if anything) is actually being fixed / altered upstream.
> 
> I don't know what more you want me to say.  It's not a problem per say
> in upstream, but it does come about because BTC_NO is handled
> inconsistently to the other FOO_NO bits.

First, it still is unclear to me how "When re-scanning features, forced caps
are taken into account" applies to upstream. As a a result, "This causes
BTC_NO to go missing" is unclear as well. Both aspects need to be clear from
the description alone.

Further, while indeed BTC_NO is the only *_NO one set by means of __set_bit(),
an (apparently) similar issue exists for SRSO_US_NO, in it being cleared by
__clear_bit(). How that goes together with the feature elsewhere being forced
on I can't immediately tell.

> recheck_cpu_features() papers over the issue by re-invoking
> identify_cpu().

This isn't papering over anything. We do the normal identification, to then
compare its results with the ones obtained during boot.

>  It's necessary for S3 resume because all of
> init_$VENDOR() really is needed, but it looks bogus in
> smp_store_cpu_info() because it's repeating work done immediately prior
> in start_secondary().

What is it that is done there "immediately prior"? I see smp_callin()
calling smp_store_cpu_info() alone, which in turn calls either
identify_cpu() or recheck_cpu_features(). Nothing being repeated afaics.
There is an earlier call to cpu_init(), but that doesn't collect any
feature info. (Perhaps we have a latent ordering problem there, as in
principle feature dependent setup would better come after determining what
features are available. The only feature dependent operations there look to
be PKRU clearing and skinit_enable_intr(), so hopefully not an active issue;
certainly not as long as the system is symmetric.)

Jan

A XenServer feature in the process of being upstreamed is to be able to
re-caculate the guest CPU Policies at runtime, e.g. after a microcode load
and/or livepatch to expose new functionality.  Right now, upstream Xen only
rescans the Raw CPU Policy on microcode load.

One complication with recalculating the guest policies is that BTC_NO is
handled differently to other $FOO_NO bits, by using __set_bit() rather than
setup_force_cpu_cap().

For consistency, switch it to using setup_force_cpu_cap().  This doesn't
matter for upstream Xen right now, but it will ease upstream the feature.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>

v2:
 * Rewrite the commit message.
---
 xen/arch/x86/cpu/amd.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/xen/arch/x86/cpu/amd.c b/xen/arch/x86/cpu/amd.c
index 805a8189e6cd..b3e12b084c56 100644
--- a/xen/arch/x86/cpu/amd.c
+++ b/xen/arch/x86/cpu/amd.c
@@ -1115,8 +1115,9 @@ static void cf_check init_amd(struct cpuinfo_x86 *c)
 		 * Branch Type Confusion, but predate the allocation of the
 		 * BTC_NO bit.  Fill it back in if we're not virtualised.
 		 */
-		if (!cpu_has_hypervisor && !cpu_has(c, X86_FEATURE_BTC_NO))
-			__set_bit(X86_FEATURE_BTC_NO, c->x86_capability);
+		if (c == &boot_cpu_data && !cpu_has_hypervisor &&
+		    !cpu_has(c, X86_FEATURE_BTC_NO))
+			setup_force_cpu_cap(X86_FEATURE_BTC_NO);
 		break;
 	}
 
-- 
2.39.5

On 01.12.2025 17:05, Andrew Cooper wrote:
> A XenServer feature in the process of being upstreamed is to be able to
> re-caculate the guest CPU Policies at runtime, e.g. after a microcode load
> and/or livepatch to expose new functionality.  Right now, upstream Xen only
> rescans the Raw CPU Policy on microcode load.
> 
> One complication with recalculating the guest policies is that BTC_NO is
> handled differently to other $FOO_NO bits, by using __set_bit() rather than
> setup_force_cpu_cap().
> 
> For consistency, switch it to using setup_force_cpu_cap().  This doesn't
> matter for upstream Xen right now, but it will ease upstream the feature.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Acked-by: Jan Beulich <jbeulich@suse.com>