Re: [PATCH 2/2] x86/fpu: Initialise FTW as well as FCW in xstate_alloc_save_area()

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 27/03/2026 10:17 am, Jan Beulich wrote:
> On 27.03.2026 11:04, Ross Lagerwall wrote:
>> On 3/26/26 7:04 PM, Andrew Cooper wrote:
>>> xstate_alloc_save_area() configures FCW and MXCSR to #RESET values but 
>>> misses
>>> FTW.  Fixing this means that the backing memory always has an 
>>> architecturally
>>> correct value.
>>>
>>> Adjust the comment to state that it's the #RESET values which we care about.
>>>
>>> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
>>> ---
>>> CC: Jan Beulich <JBeulich@xxxxxxxx>
>>> CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
>>> CC: Ross Lagerwall <ross.lagerwall@xxxxxxxxxx>
>>>
>>> I don't understand what the rest of the comment is trying to say, so have 
>>> left
>>> it alone.  There's still a lot of cleanup to be done to merge i387 and 
>>> xstate.
>>> ---
>>>   xen/arch/x86/xstate.c | 3 ++-
>>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/xen/arch/x86/xstate.c b/xen/arch/x86/xstate.c
>>> index e990abc9d18c..747df0b2e9a9 100644
>>> --- a/xen/arch/x86/xstate.c
>>> +++ b/xen/arch/x86/xstate.c
>>> @@ -550,11 +550,12 @@ int xstate_alloc_save_area(struct vcpu *v)
>>>           return -ENOMEM;
>>>   
>>>       /*
>>> -     * Set the memory image to default values, but don't force the context
>>> +     * Set the memory image to #RESET values, but don't force the context
>>>        * to be loaded from memory (i.e. keep save_area->xsave_hdr.xstate_bv
>>>        * clear).
>>>        */
>>>       save_area->fpu_sse.fcw = FCW_DEFAULT;
>>> +    save_area->fpu_sse.ftw = FXSAVE_FTW_RESET;
>>>       save_area->fpu_sse.mxcsr = MXCSR_DEFAULT;
>>>   
>>>       v->arch.xsave_area = save_area;
>> Is this comment correct given that it is initializing FCW to FCW_DEFAULT
>> which is different from FCW_RESET?
> Is the goal here to represent XSAVE init-state in memory, or do we truly mean
> #RESET state (in which case FCW_RESET would need using, and in which case
> leaving xstate_bv bit 0 clear would be wrong).

We're creating the vCPU, so conceptually I think #RESET state is what we
want.

But, I'd forgotten that #RESET and #INIT FCW are different.  Also, we
don't really want to be taking the overhead of keeping this properly at
the #RESET state until the guest executes an FNINIT/etc.

I think it's better to keep using DEFAULT here.  I'll rework the comment
and commit message.

This also suggests that we want to rethink vcpu_reset_fpu(), but we can
leave that for later.

~Andrew