Re: [UNIKRAFT PATCH v2 3/4] plat/kvm: enable XSAVE for PKRU register

[Gabriel Mocanu <gabi.mocanu98@xxxxxxxxx>]

Hi, Hugo.

Looks good to me.

Mocanu Gabriel

Reviewed-by: Mocanu Gabriel <gabi.mocanu98@xxxxxxxxx>

În vin., 11 dec. 2020 la 18:36, Hugo Lefeuvre <hle@xxxxxxxxxx> a scris:

According to Intel 64 and IA-32 Architectures Software Developer’s
Manual Vol. 1, Section 13.5.7 'PKRU State':

        software can use the XSAVE feature set to manage PKRU state
        only if XCR0[9] = 1

This patch sets XCR0[9] if PKU is available (and enabled via
CONFIG_HAVE_X86PKU).

Note: we do not support PKU without XSAVE. If CONFIG_HAVE_X86PKU is
selected but XSAVE is not available, we simply don't initialize PKU
and let the boot process gracefully abort later.

Signed-off-by: Hugo Lefeuvre <hugo.lefeuvre@xxxxxxxxxxxxxxxx>
---
 plat/common/include/x86/cpu_defs.h |  1 +
 plat/kvm/x86/entry64.S             | 19 +++++++++++++++----
 2 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/plat/common/include/x86/cpu_defs.h b/plat/common/include/x86/cpu_defs.h
index 74cff459..1cf5367d 100644
--- a/plat/common/include/x86/cpu_defs.h
+++ b/plat/common/include/x86/cpu_defs.h
@@ -100,6 +100,7 @@
 #define X86_XCR0_X87            (1 << 0)
 #define X86_XCR0_SSE            (1 << 1)
 #define X86_XCR0_AVX            (1 << 2)
+#define X86_XCR0_PKRU          (1 << 9)

 /*
  * Model-specific register addresses
diff --git a/plat/kvm/x86/entry64.S b/plat/kvm/x86/entry64.S
index ef5e0e77..287f9029 100644
--- a/plat/kvm/x86/entry64.S
+++ b/plat/kvm/x86/entry64.S
@@ -196,18 +196,19 @@ ENTRY(_libkvmplat_start64)
        movq %rdi, %cr4
        ldmxcsr (mxcsr_ptr)
 #endif /* __SSE__ */
+#if (__AVX__ || CONFIG_HAVE_X86PKU)
        /* Check capabilities subject to availability as indicated by cpuid.
         * First, start off with "standard features" */
        movl $0x1, %eax
        cpuid
-#if __AVX__
        /* ecx and edx now contain capability information, so we can now
         * enable capabilities based on the indicated features */
-       /* OSXSAVE needs to be enabled before AVX */
+       /* note: OSXSAVE needs to be enabled before AVX and PKU */
        testl $(X86_CPUID1_ECX_XSAVE), %ecx
        jz noxsave
        orl $(X86_CR4_OSXSAVE), %edi
        movq %rdi, %cr4
+#if __AVX__
        /* now enable AVX. This needs to be last checking cpuid features from
         * the eax=1 cpuid call, because it clobbers ecx */
        testl $(X86_CPUID1_ECX_AVX), %ecx
@@ -217,8 +218,10 @@ ENTRY(_libkvmplat_start64)
        orl $(X86_XCR0_SSE | X86_XCR0_AVX), %eax
        xsetbv
 noavx:
-noxsave:
 #endif /* __AVX__ */
+/* Do not enable AVX without XSAVE, otherwise we'll get #UD */
+noxsave:
+#endif /* __AVX__ || CONFIG_HAVE_X86PKU */
        /* Now, check for extended features. */
        movl $0x7, %eax
        movl $0x0, %ecx
@@ -234,9 +237,17 @@ nofsgsbase:
        /* check for Memory Protection Keys (PKU) */
        testl $(X86_CPUID7_ECX_PKU), %ecx
        jz nopku
-       /* if PKU is supported, enable it via CR4 */
+       /* only enable PKU if we support XSAVE */
+       testl $(X86_CR4_OSXSAVE), %edi
+       jz nopku
+       /* PKU is supported, enable it via CR4 */
        orl $(X86_CR4_PKE), %edi
        movq %rdi, %cr4
+       /* also enable XSAVE for the PKRU */
+       xorl %ecx, %ecx
+       xgetbv
+       orl $(X86_XCR0_PKRU), %eax
+       xsetbv
 nopku:
 #endif /* CONFIG_HAVE_X86PKU */
        /* done setting up CPU capabilities */
--
2.20.1