On 03.10.2024 15:54, Alejandro Vallejo wrote: > On Tue Aug 13, 2024 at 5:33 PM BST, Alejandro Vallejo wrote: >> 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 <[email protected]> >>> >>> Reviewed-by: Jan Beulich <[email protected]> >>> >>>> --- >>>> 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 > > Playing around with the FPU I noticed this patch wasn't committed, did it fall > under the cracks or is there a specific reason?
Well, it's patch 2 in a series with no statement that it's independent of patch 1, and patch 1 continues to lack an ack (based on earlier comments of mine you probably have inferred that I'm not intending to ack it in this shape, while at the same time - considering the arguments you gave - I also don't mean to stand in the way of it going in with someone else's ack). Jan
