| Message ID | 20250409165510.23066-1-vdronov@redhat.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | selftests/sgx: Fix an enclave built with extended instructions | expand |
On 4/9/25 09:55, Vladis Dronov wrote: ... > Fix this by adding "-mno-avx" to ENCL_CFLAGS in Makefile. Add some comments > about this to code locations where enclave's xfrm field is set. > > Suggested-by: Dave Hansen <dave.hansen@linux.intel.com> > Signed-off-by: Vladis Dronov <vdronov@redhat.com> First of all, this looks fine to me: Acked-by: Dave Hansen <dave.hansen@linux.intel.com> The code comments are fine. I'm much less picky about selftests. I'm also open to other solutions here. We could, for instance, set xfrm=7 to allow AVX2 instructions (which are generated by "--with-arch_64=x86-64-v3") or use some compiler flags other than "-mno-avx". But "-mno-avx" does seem good to me.
On Wed, Apr 9, 2025 at 7:07 PM Dave Hansen <dave.hansen@intel.com> wrote: > > On 4/9/25 09:55, Vladis Dronov wrote: > ... > > Fix this by adding "-mno-avx" to ENCL_CFLAGS in Makefile. Add some comments > > about this to code locations where enclave's xfrm field is set. > > > > Suggested-by: Dave Hansen <dave.hansen@linux.intel.com> > > Signed-off-by: Vladis Dronov <vdronov@redhat.com> > > First of all, this looks fine to me: > > Acked-by: Dave Hansen <dave.hansen@linux.intel.com> > > The code comments are fine. I'm much less picky about selftests. > > I'm also open to other solutions here. We could, for instance, set > xfrm=7 to allow AVX2 instructions (which are generated by > "--with-arch_64=x86-64-v3") or use some compiler flags other than > "-mno-avx". > > But "-mno-avx" does seem good to me. > Thank you for the ACK, Dave. I've tested an enclave with xfrm=7 and it errors out at the AVX512 instruction (namely, vmovdqa64) in the same way. So if there is a compiler built with "--with-arch_64=x86-64-v4" in some future, we would get into the exact same situation. So I believe a solution when we disable extended instruction sets in an enclave as one covering all future cases.
diff --git a/tools/testing/selftests/sgx/Makefile b/tools/testing/selftests/sgx/Makefile index 03b5e13b872b..ab2561b4456d 100644 --- a/tools/testing/selftests/sgx/Makefile +++ b/tools/testing/selftests/sgx/Makefile @@ -15,7 +15,7 @@ INCLUDES := -I$(top_srcdir)/tools/include HOST_CFLAGS := -Wall -Werror -g $(INCLUDES) -fPIC $(CFLAGS) HOST_LDFLAGS := -z noexecstack -lcrypto ENCL_CFLAGS += -Wall -Werror -static-pie -nostdlib -ffreestanding -fPIE \ - -fno-stack-protector -mrdrnd $(INCLUDES) + -fno-stack-protector -mrdrnd -mno-avx $(INCLUDES) ENCL_LDFLAGS := -Wl,-T,test_encl.lds,--build-id=none ifeq ($(CAN_BUILD_X86_64), 1) diff --git a/tools/testing/selftests/sgx/load.c b/tools/testing/selftests/sgx/load.c index c9f658e44de6..79946ca8f1a5 100644 --- a/tools/testing/selftests/sgx/load.c +++ b/tools/testing/selftests/sgx/load.c @@ -88,10 +88,16 @@ static bool encl_ioc_create(struct encl *encl) memset(secs, 0, sizeof(*secs)); secs->ssa_frame_size = 1; secs->attributes = SGX_ATTR_MODE64BIT; - secs->xfrm = 3; secs->base = encl->encl_base; secs->size = encl->encl_size; + /* + * Setting xfrm to 3 disables extended CPU states and instruction sets + * like AVX2 inside the enclave. Thus the enclave code has to be built + * without instructions from extended instruction sets (-mno-avx). + */ + secs->xfrm = 3; + ioc.src = (unsigned long)secs; rc = ioctl(encl->fd, SGX_IOC_ENCLAVE_CREATE, &ioc); if (rc) { diff --git a/tools/testing/selftests/sgx/sigstruct.c b/tools/testing/selftests/sgx/sigstruct.c index d73b29becf5b..f548392a2fee 100644 --- a/tools/testing/selftests/sgx/sigstruct.c +++ b/tools/testing/selftests/sgx/sigstruct.c @@ -331,6 +331,12 @@ bool encl_measure(struct encl *encl) sigstruct->header.header2[1] = header2[1]; sigstruct->exponent = 3; sigstruct->body.attributes = SGX_ATTR_MODE64BIT; + + /* + * Setting xfrm to 3 disables extended CPU states and instruction sets + * like AVX2 inside the enclave. Thus the enclave code has to be built + * without instructions from extended instruction sets (-mno-avx). + */ sigstruct->body.xfrm = 3; /* sanity check */
Creating an enclave with xfrm == 3 disables extended CPU states and instruction sets, like AVX2 and AVX512 inside the enclave. Thus the enclave code has to be built with a compiler which does not produce instructions from the extended instruction sets. Nevertheless certain Linux distributions confgure a compiler so it produces extended instructions by default ("--with-arch_64=x86-64-v3" for gcc). Thus an enclave code from test_encl.c is built with extended instructions and an enclave execution hits the #UD exception (note exception_vector == 6): # ./test_sgx ... # RUN enclave.unclobbered_vdso_oversubscribed_remove ... # main.c:481:unclobbered_vdso_oversubscribed_remove:Expected self->run.exception_vector (6) == 0 (0) # main.c:485:unclobbered_vdso_oversubscribed_remove:Expected self->run.function (3) == EEXIT (4) # unclobbered_vdso_oversubscribed_remove: Test terminated by assertion # FAIL enclave.unclobbered_vdso_oversubscribed_remove not ok 3 enclave.unclobbered_vdso_oversubscribed_remove Fix this by adding "-mno-avx" to ENCL_CFLAGS in Makefile. Add some comments about this to code locations where enclave's xfrm field is set. Suggested-by: Dave Hansen <dave.hansen@linux.intel.com> Signed-off-by: Vladis Dronov <vdronov@redhat.com> --- an out-of-commit-message note: I would greatly appreciate if someone reviews and possibly fixes my wording of the commit message and the code comments. tools/testing/selftests/sgx/Makefile | 2 +- tools/testing/selftests/sgx/load.c | 8 +++++++- tools/testing/selftests/sgx/sigstruct.c | 6 ++++++ 3 files changed, 14 insertions(+), 2 deletions(-)