On Wed, Mar 11, 2020 at 03:28:32PM +0200, Jarkko Sakkinen wrote:
On Thu, Mar 05, 2020 at 01:33:28PM +0200, Jarkko Sakkinen wrote:
On Wed, 2020-03-04 at 14:27 -0500, Nathaniel McCallum wrote:
+xsave_area:
.fill 1, 4, 0x037F # FCW.fill 5, 4, 0.fill 1, 4, 0x1F80 # MXCSR.fill 1, 4, 0xFFFF # MXCSR_MASK.fill 123, 4, 0.fill 1, 4, 0x80000000 # XCOMP_BV[63] = 1, compaction mode.fill 12, 4, 0I find this much more readable:
And I always aim to get things more readable. Thank you.
xsave_area: # Legacy .fill 1, 4, 0x037F # FCW .fill 5, 4, 0 .fill 1, 4, 0x1F80 # MXCSR .fill 1, 4, 0xFFFF # MXCSR_MASK .fill 60, 8, 0
# Header .fill 1, 8, 0 # XSTATE_BV .fill 1, 8, 1 << 63 # XCOMP_BV (compaction mode) .fill 6, 8, 0Also, since people are likely to copy this code for their own enclaves, it would be helpful to document which flags are set in FCW and MXCSR.
It was meant as a test program but I'd guess what you say is true because it also might be the only alternative user space to Intel's :-) And a great starting point if you want to do things from scratch.
Because I meant it as a smoke test program for SGX, not everything is too well documented but given the multipurpose use for that code I'll make the improvements that you are suggesting.
For FPU Control World (FCW), I think 0x037F is not the right value even if section 18.5 in the x86 SDM says that it is the initial value for it.
I took that value from that section.
The reason I think that there is an error in the SDM is that if you look at the section 8.1.5, you'll see that bit 6 is a reserved bit. Thus, does not make to set it on.
I think the legit value ought to be 0x33F i.e. unset bit 6.
Bit 6 is reserved, but it's forced to '1' by the CPU.
Regardless, IMO it'd be better to drop this code entirely, it's all kinds of wonky. The label says "xsave_area" and implies XSAVE state is being loaded, but the code uses FXRSTOR, which will only load x86/MMX/XMM state, i.e. the first 512 bytes of the so called xsave_area.
The test enclave doesn't touch state managed by XSAVE, let alone put secrets into said state. I see no reason to bother purging anything.
In any case check:
https://raw.githubusercontent.com/jsakkine-intel/linux-sgx/master/tools/test...
I.e. both have now a reference:
- To the section that describes the default.
- To the section that describes what the bits mean.
/Jarkko