Re: [PATCH v3 2/2] x86/fpu: Split fpu_setup_fpu() in three

["Alejandro Vallejo" <alejandro.vallejo@cloud.com>]

On Tue Aug 13, 2024 at 3:32 PM BST, Jan Beulich wrote:
> On 13.08.2024 16:21, Alejandro Vallejo wrote:
> > It was trying to do too many things at once and there was no clear way of
> > defining what it was meant to do. This commit splits the function in three.
> > 
> >   1. A function to return the FPU to power-on reset values.
> >   2. A function to return the FPU to default values.
> >   3. A x87/SSE state loader (equivalent to the old function when it took a data
> >      pointer).
> > 
> > While at it, make sure the abridged tag is consistent with the manuals and
> > start as 0xFF.
> > 
> > Signed-off-by: Alejandro Vallejo <alejandro.vallejo@cloud.com>
>
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
>
> > ---
> > v3:
> >   * Adjust commit message, as the split is now in 3.
> >   * Remove bulky comment, as the rationale for it turned out to be
> >     unsubstantiated. I can't find proof in xen-devel of the stream
> >     operating the way I claimed, and at that point having the comment
> >     at all is pointless
>
> So you deliberately removed the comment altogether, not just point 3 of it?
>
> Jan

Yes. The other two cases can be deduced pretty trivially from the conditional,
I reckon. I commented them more heavily in order to properly introduce (3), but
seeing how it was all a midsummer dream might as well reduce clutter.

I got as far as the original implementation of XSAVE in Xen and it seems to
have been tested against many combinations of src and dst, none of which was
that ficticious "xsave enabled + xsave context missing". I suspect the
xsave_enabled(v) was merely avoiding writing to the XSAVE buffer just for
efficiency (however minor effect it might have had). I just reverse engineering
it wrong.

Which reminds me. Thanks for mentioning that, because it was really just
guesswork on my part.

Cheers,
Alejandro