[PAL/Linux-SGX] Remove no-XSAVE code paths (dead code) #1402
Conversation
Commit "[PAL/Linux-SGX] Enforce AES-NI, XSAVE and RDRAND" enforced the XSAVE CPU feature. The Linux-SGX PAL still had some code that used no-XSAVE code paths, falling back to older FXSAVE. With that commit, such code became dead code. Also, this commit adds an additional enforcement: the OS must also enable the XSAVE feature, which is reflected in CPUID.1:ECX.OSXSAVE. Signed-off-by: Dmitrii Kuvaiskii <[email protected]>
dimakuv
force-pushed
the
dimakuv/rm-dead-code-always-xsave-eanbled
branch
from
a059331
to
80df8c3
Compare
There was a problem hiding this comment.
Reviewable status: 0 of 5 files reviewed, all discussions resolved, not enough approvals from maintainers (2 more required), not enough approvals from different teams (1 more required, approved so far: Intel)
pal/src/host/linux-sgx/host_main.c line 1161 at r1 (raw file):
FYI: This is a required check, otherwise we could have a situation:
- the CPU supports XSAVE feature (reported in
CPUID.1.ECX.XSAVE) - the OS does not support XSAVE feature (reported in
CPUID.1.ECX.OSXSAVE, which is set by the OS kernel)
This can happen e.g. when Linux kernel is started with noxsave.
In general, the flow to use XSAVE features is described in Intel SDM, Vol. 1, Chapter 13.3.
Here's a relevant snippet, for convenience of reviewers:
Thus, software can use the following algorithm to determine the support and enabling for the XSAVE feature set:
- Use CPUID to discover the value of CPUID.1:ECX.OSXSAVE.
— If the bit is 0, either the XSAVE feature set is not supported by the processor or has not been enabled by software. Either way, the XSAVE feature set is not available, nor are XSAVE-enabled features such as AVX.
— If the bit is 1, the processor supports the XSAVE feature set — including the XGETBV instruction — and it has been enabled by software. The XSAVE feature set can be used to manage x87 state (because XCR0[0] is always 1). Software requiring more detailed information can go on to the next step.- Execute XGETBV with ECX = 0 to discover the value of XCR0. If XCR0[1] = 1, the XSAVE feature set can be used to manage SSE state. If XCR0[2] = 1, the XSAVE feature set can be used to manage AVX state and software can execute Intel AVX instructions. If XCR0[4:3] is 11b, the XSAVE feature set can be used to manage MPX state and software can execute Intel MPX instructions. If XCR0[7:5] is 111b, the XSAVE feature set can be used to manage AVX-512 state and software can execute Intel AVX-512 instructions. If XCR0[9] = 1, the XSAVE feature set can be used to manage PKRU state.
There was a problem hiding this comment.
Reviewed 5 of 5 files at r1, all commit messages.
Reviewable status:complete! all files reviewed, all discussions resolved
Description of the changes
Commit "[PAL/Linux-SGX] Enforce AES-NI, XSAVE and RDRAND" enforced the XSAVE CPU feature. The Linux-SGX PAL still had some code that use no-XSAVE code paths, falling back to older FXSAVE. With that commit, such code became dead code.
Also, this commit adds an additional enforcement: the OS must also enable the XSAVE feature, which is reflected in CPUID.1:ECX.OSXSAVE.
How to test this PR?
CI should be enough. Also, testing on some big new Intel CPUs would be beneficial (to test e.g. AMX enablement).
This was detected while working on #955 (which we hit again).
This change is