Page MenuHomeFreeBSD

amd64: handle MXCSR from XSAVEOPT when x87 state was optimized
ClosedPublic

Authored by kib on Mar 27 2024, 11:17 AM.
Tags
None
Referenced Files
F102835073: D44522.diff
Sun, Nov 17, 7:08 PM
Unknown Object (File)
Thu, Nov 7, 4:14 PM
Unknown Object (File)
Sat, Nov 2, 2:29 PM
Unknown Object (File)
Sat, Nov 2, 2:29 PM
Unknown Object (File)
Tue, Oct 22, 7:40 AM
Unknown Object (File)
Tue, Oct 22, 7:40 AM
Unknown Object (File)
Tue, Oct 22, 7:40 AM
Unknown Object (File)
Tue, Oct 22, 7:40 AM
Subscribers

Details

Summary

PR: 275322
Reported by: Cheyenne Wills <cheyenne.wills@gmail.com>

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

kib requested review of this revision.Mar 27 2024, 11:17 AM

I'm not fully familiar with the xsave details - I'm curious why this was AMD-specific (and/or, what implies that the x87 area was not saved, in the new block)?

AMD-specific thing there is that X87 state was not saved, as reported by xstate_bv.bit0 == 0 (I believe Intel always saves X87). But regardless of that, MXCSR state is always saved even if X87 is not. By overwriting X87 non-saved area with the default content, we also override the potentially changed MXCSR, which was reported in the PR.

Presumably i386 needs the same fixup as well?

What an odd bug.

sys/amd64/amd64/fpu.c
888

AVX512? Realistically I suspect anything using AVX512 will also be dirtying AVX state (and really AVX should always imply SSE so you could maybe even get by with only checking SSE here?)

892
This revision is now accepted and ready to land.Mar 27 2024, 2:55 PM
kib marked 2 inline comments as done.Mar 27 2024, 3:25 PM
kib added inline comments.
sys/amd64/amd64/fpu.c
888

Literally, Intel SDM states

IF RFBM[1] = 1 or RFBM[2] = 1
   THEN store MXCSR and MXCSR_MASK into legacy region of XSAVE area;
FI;

But now I think that the mask must be saved as well.

kib marked an inline comment as done.

Handle i386

This revision now requires review to proceed.Mar 27 2024, 3:25 PM
sys/amd64/amd64/fpu.c
905–908

we write this each time through the loop?

kib marked an inline comment as done.Mar 27 2024, 3:42 PM
kib added inline comments.
sys/amd64/amd64/fpu.c
905–908

Indeed. This is not incorrect but excessive of course.

kib marked an inline comment as done.

Initialize do_mxcsr on each iteration.

olce added inline comments.
sys/amd64/amd64/fpu.c
888

The Intel SDM also states that, when XSAVEC is used, only RFBM[1] set causes MXCSR and MXCSR_MASK to be stored. So shouldn't the newly introduced if be changed to deal with this situation, by testing only for RFBM[1], or RFBM[1] and RFBM[2], depending on whether XCOMP_BV[63] is set (flag for the compact format)?

kib marked an inline comment as done.Mar 27 2024, 4:43 PM
kib added inline comments.
sys/amd64/amd64/fpu.c
888

We do not use XSAVEC. This code needs to be completely revamped to accomodate XSAVEC.

This revision is now accepted and ready to land.Mar 27 2024, 4:54 PM
This revision was automatically updated to reflect the committed changes.
kib marked an inline comment as done.