Re: [PATCH v1 8/8] x86: Cleanup cr0.TS flag handling

[Jan Beulich <jbeulich@xxxxxxxx>]

On 23.03.2026 15:14, Ross Lagerwall wrote:
> On 3/23/26 12:30 PM, Jan Beulich wrote:
>> On 19.03.2026 14:29, Ross Lagerwall wrote:
>>> --- a/xen/arch/x86/cpu/common.c
>>> +++ b/xen/arch/x86/cpu/common.c
>>> @@ -883,9 +883,6 @@ void cpu_init(void)
>>>     /* Install correct page table. */
>>>     write_ptbase(current);
>>>   
>>> -   /* Ensure FPU gets initialised for each domain. */
>>> -   stts();
>>
>> I'm a little concerned by the removal of this and ...
>>
>>> @@ -193,31 +190,18 @@ void vcpu_restore_fpu(struct vcpu *v)
>>>    * On each context switch, save the necessary FPU info of VCPU being 
>>> switch
>>>    * out. It dispatches saving operation based on CPU's capability.
>>>    */
>>> -static bool _vcpu_save_fpu(struct vcpu *v)
>>> +void vcpu_save_fpu(struct vcpu *v)
>>>   {
>>>       ASSERT(!is_idle_vcpu(v));
>>>   
>>>       /* This can happen, if a paravirtualised guest OS has set its CR0.TS. 
>>> */
>>> -    clts();
>>> +    if ( is_pv_vcpu(v) )
>>> +        clts();
>>>   
>>>       if ( cpu_has_xsave )
>>>           fpu_xsave(v);
>>>       else
>>>           fpu_fxsave(v);
>>> -
>>> -    return true;
>>> -}
>>> -
>>> -void vcpu_save_fpu(struct vcpu *v)
>>> -{
>>> -    _vcpu_save_fpu(v);
>>> -    stts();
>>
>> ... this. At present it guards us against e.g. an idle CPU or context
>> switch code mistakenly using in particular XMM registers (but of course
>> also other extended state).
> 
> Given this concern and Andrew's comment, I could drop this patch for now.
> It can be revisited in future if needed.

We discussed this some on the x86 maintainers call earlier today. Andrew
(who wants to put together a more extensive reply) indicates that getting
rid of the stts() instances is a relevant goal of this work. If this can
be clearly stated for this patch, and if the implications are clear, then
I think this could still be okay to go in.

Jan