[PATCH] selftests/sgx: Fix an enclave built with extended instructions

[PATCH] selftests/sgx: Fix an enclave built with extended instructions

[Vladis Dronov]

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(-)

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 */
-- 
2.49.0

Re: [PATCH] selftests/sgx: Fix an enclave built with extended instructions

[Dave Hansen]

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.

Re: [PATCH] selftests/sgx: Fix an enclave built with extended instructions

[Vladis Dronov]

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.