2018-12-26 08:22:23

by Yang Weijiang

[permalink] [raw]
Subject: [Qemu-devel][PATCH 0/4] This patch-set is to enable Guest CET support.

Control-flow Enforcement Technology (CET) provides protection against
return/jump-oriented programming (ROP) attacks. To make kvm Guest OS
own the capability, this patch-set is required. It enables CET related
CPUID report and xsaves/xrstors support etc in qemu.

Yang Weijiang (4):
Add CET xsaves/xrstors related macros and structures.
Add CET SHSTK and IBT CPUID feature-word definitions.
Add hepler functions for CPUID xsave area size calculation.
Report CPUID xsave area support for CET.

target/i386/cpu.c | 67 ++++++++++++++++++++++++++++++++++++++++++++---
target/i386/cpu.h | 36 ++++++++++++++++++++++++-
2 files changed, 99 insertions(+), 4 deletions(-)

--
2.17.1



2018-12-26 08:23:11

by Yang Weijiang

[permalink] [raw]
Subject: [Qemu-devel][PATCH 4/4] Report CPUID xsave area support for CET.

CPUID bit definition as below:
CPUID.(EAX=d, ECX=1):ECX.CET_U(bit 11): user mode state
CPUID.(EAX=d, ECX=1):ECX.CET_S(bit 12): kernel mode state

Signed-off-by: Zhang Yi <[email protected]>
Signed-off-by: Yang Weijiang <[email protected]>
---
target/i386/cpu.c | 10 ++++++++++
1 file changed, 10 insertions(+)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index cf4f2798dc..78994bfa1d 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -4396,12 +4396,22 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
*ebx = xsave_area_size(env->xcr0);
} else if (count == 1) {
*eax = env->features[FEAT_XSAVE];
+ *ecx = env->features[FEAT_XSAVE_SV_LO];
+ *edx = env->features[FEAT_XSAVE_SV_HI];
+ *ebx = xsave_area_size_compat(x86_cpu_xsave_components(cpu) |
+ x86_cpu_xsave_sv_components(cpu));
} else if (count < ARRAY_SIZE(x86_ext_save_areas)) {
if ((x86_cpu_xsave_components(cpu) >> count) & 1) {
const ExtSaveArea *esa = &x86_ext_save_areas[count];
*eax = esa->size;
*ebx = esa->offset;
}
+ if ((x86_cpu_xsave_sv_components(cpu) >> count) & 1) {
+ const ExtSaveArea *esa_sv = &x86_ext_save_areas[count];
+ *eax = esa_sv->size;
+ *ebx = esa_sv->offset;
+ *ecx = 1;
+ }
}
break;
}
--
2.17.1


2018-12-26 08:23:11

by Yang Weijiang

[permalink] [raw]
Subject: [Qemu-devel][PATCH 3/4] Add hepler functions for CPUID xsave area size calculation.

These functions are called when return CPUID xsave area
size information.

Signed-off-by: Zhang Yi <[email protected]>
Signed-off-by: Yang Weijiang <[email protected]>
---
target/i386/cpu.c | 26 +++++++++++++++++++++++++-
1 file changed, 25 insertions(+), 1 deletion(-)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 3630c688d6..cf4f2798dc 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -1281,12 +1281,34 @@ static inline bool accel_uses_host_cpuid(void)
return kvm_enabled() || hvf_enabled();
}

+static uint32_t xsave_area_size_compat(uint64_t mask)
+{
+ int i;
+ uint64_t ret = 0;
+ uint32_t offset;
+
+ for (i = 0; i < ARRAY_SIZE(x86_ext_save_areas); i++) {
+ const ExtSaveArea *esa = &x86_ext_save_areas[i];
+ offset = i > 1 ? ret : esa->offset;
+ if ((mask >> i) & 1) {
+ ret = MAX(ret, offset + esa->size);
+ }
+ }
+ return ret;
+}
+
static inline uint64_t x86_cpu_xsave_components(X86CPU *cpu)
{
return ((uint64_t)cpu->env.features[FEAT_XSAVE_COMP_HI]) << 32 |
cpu->env.features[FEAT_XSAVE_COMP_LO];
}

+static inline uint64_t x86_cpu_xsave_sv_components(X86CPU *cpu)
+{
+ return ((uint64_t)cpu->env.features[FEAT_XSAVE_SV_HI]) << 32 |
+ cpu->env.features[FEAT_XSAVE_SV_LO];
+}
+
const char *get_register_name_32(unsigned int reg)
{
if (reg >= CPU_NB_REGS32) {
@@ -4913,8 +4935,10 @@ static void x86_cpu_enable_xsave_components(X86CPU *cpu)
}
}

- env->features[FEAT_XSAVE_COMP_LO] = mask;
+ env->features[FEAT_XSAVE_COMP_LO] = mask & CPUID_XSTATE_USER_MASK;
env->features[FEAT_XSAVE_COMP_HI] = mask >> 32;
+ env->features[FEAT_XSAVE_SV_LO] = mask & CPUID_XSTATE_KERNEL_MASK;
+ env->features[FEAT_XSAVE_SV_HI] = mask >> 32;
}

/***** Steps involved on loading and filtering CPUID data
--
2.17.1


2018-12-26 08:38:12

by Yang Weijiang

[permalink] [raw]
Subject: [Qemu-devel][PATCH 2/4] Add CET SHSTK and IBT CPUID feature-word definitions.

XSS[bit 11] and XSS[bit 12] correspond to CET
user mode area and supervisor mode area respectively.

Signed-off-by: Zhang Yi <[email protected]>
Signed-off-by: Yang Weijiang <[email protected]>
---
target/i386/cpu.c | 31 +++++++++++++++++++++++++++++--
1 file changed, 29 insertions(+), 2 deletions(-)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index f81d35e1f9..3630c688d6 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -1018,7 +1018,7 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
.type = CPUID_FEATURE_WORD,
.feat_names = {
NULL, "avx512vbmi", "umip", "pku",
- NULL /* ospke */, NULL, "avx512vbmi2", NULL,
+ NULL /* ospke */, NULL, "avx512vbmi2", "shstk",
"gfni", "vaes", "vpclmulqdq", "avx512vnni",
"avx512bitalg", NULL, "avx512-vpopcntdq", NULL,
"la57", NULL, NULL, NULL,
@@ -1041,7 +1041,7 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
NULL, NULL, NULL, NULL,
NULL, NULL, NULL, NULL,
NULL, NULL, "pconfig", NULL,
- NULL, NULL, NULL, NULL,
+ "ibt", NULL, NULL, NULL,
NULL, NULL, "spec-ctrl", NULL,
NULL, "arch-capabilities", NULL, "ssbd",
},
@@ -1162,6 +1162,25 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
}
},
},
+ /* Below are CET supervisor xsave features */
+ [FEAT_XSAVE_SV_LO] = {
+ .type = CPUID_FEATURE_WORD,
+ .cpuid = {
+ .eax = 0xD,
+ .needs_ecx = true,
+ .ecx = 1,
+ .reg = R_ECX,
+ },
+ },
+ [FEAT_XSAVE_SV_HI] = {
+ .type = CPUID_FEATURE_WORD,
+ .cpuid = {
+ .eax = 0xD,
+ .needs_ecx = true,
+ .ecx = 1,
+ .reg = R_EDX
+ },
+ }
};

typedef struct X86RegisterInfo32 {
@@ -1233,6 +1252,14 @@ static const ExtSaveArea x86_ext_save_areas[] = {
{ .feature = FEAT_7_0_ECX, .bits = CPUID_7_0_ECX_PKU,
.offset = offsetof(X86XSaveArea, pkru_state),
.size = sizeof(XSavePKRU) },
+ [XSTATE_CET_U_BIT] = {
+ .feature = FEAT_7_0_ECX, .bits = CPUID_7_0_ECX_CET_SHSTK,
+ .offset = offsetof(X86XSaveArea, cet_u),
+ .size = sizeof(XSaveCETU) },
+ [XSTATE_CET_S_BIT] = {
+ .feature = FEAT_7_0_ECX, .bits = CPUID_7_0_ECX_CET_SHSTK,
+ .offset = offsetof(X86XSaveArea, cet_s),
+ .size = sizeof(XSaveCETS) },
};

static uint32_t xsave_area_size(uint64_t mask)
--
2.17.1


2018-12-26 08:38:29

by Yang Weijiang

[permalink] [raw]
Subject: [Qemu-devel][PATCH 1/4] Add CET xsaves/xrstors related macros and structures.

CET protection in user mode and kernel mode relies on
specific MSRs, these MSRs' contents are automatically
saved/restored by xsaves/xrstors instructions.

Signed-off-by: Zhang Yi <[email protected]>
Signed-off-by: Yang Weijiang <[email protected]>
---
target/i386/cpu.h | 36 +++++++++++++++++++++++++++++++++++-
1 file changed, 35 insertions(+), 1 deletion(-)

diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 9c52d0cbeb..f3f724d8e6 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -469,6 +469,9 @@ typedef enum X86Seg {
#define XSTATE_ZMM_Hi256_BIT 6
#define XSTATE_Hi16_ZMM_BIT 7
#define XSTATE_PKRU_BIT 9
+#define XSTATE_RESERVED_BIT 10
+#define XSTATE_CET_U_BIT 11
+#define XSTATE_CET_S_BIT 12

#define XSTATE_FP_MASK (1ULL << XSTATE_FP_BIT)
#define XSTATE_SSE_MASK (1ULL << XSTATE_SSE_BIT)
@@ -479,6 +482,19 @@ typedef enum X86Seg {
#define XSTATE_ZMM_Hi256_MASK (1ULL << XSTATE_ZMM_Hi256_BIT)
#define XSTATE_Hi16_ZMM_MASK (1ULL << XSTATE_Hi16_ZMM_BIT)
#define XSTATE_PKRU_MASK (1ULL << XSTATE_PKRU_BIT)
+#define XSTATE_RESERVED_MASK (1ULL << XSTATE_RESERVED_BIT)
+#define XSTATE_CET_U_MASK (1ULL << XSTATE_CET_U_BIT)
+#define XSTATE_CET_S_MASK (1ULL << XSTATE_CET_S_BIT)
+
+/* CPUID feature bits available in XCR0 */
+#define CPUID_XSTATE_USER_MASK (XSTATE_FP_MASK | XSTATE_SSE_MASK \
+ | XSTATE_YMM_MASK | XSTATE_BNDREGS_MASK \
+ | XSTATE_BNDCSR_MASK | XSTATE_OPMASK_MASK \
+ | XSTATE_ZMM_Hi256_MASK \
+ | XSTATE_Hi16_ZMM_MASK | XSTATE_PKRU_MASK)
+
+/* CPUID feature bits available in XSS */
+#define CPUID_XSTATE_KERNEL_MASK (XSTATE_CET_U_MASK | XSTATE_CET_S_MASK)

/* CPUID feature words */
typedef enum FeatureWord {
@@ -503,6 +519,8 @@ typedef enum FeatureWord {
FEAT_XSAVE_COMP_LO, /* CPUID[EAX=0xd,ECX=0].EAX */
FEAT_XSAVE_COMP_HI, /* CPUID[EAX=0xd,ECX=0].EDX */
FEAT_ARCH_CAPABILITIES,
+ FEAT_XSAVE_SV_LO, /* CPUID[EAX=0xd,ECX=1].ECX */
+ FEAT_XSAVE_SV_HI, /* CPUID[EAX=0xd,ECX=1].EDX */
FEATURE_WORDS,
} FeatureWord;

@@ -687,7 +705,7 @@ typedef uint32_t FeatureWordArray[FEATURE_WORDS];
#define CPUID_7_0_ECX_LA57 (1U << 16)
#define CPUID_7_0_ECX_RDPID (1U << 22)
#define CPUID_7_0_ECX_CLDEMOTE (1U << 25) /* CLDEMOTE Instruction */
-
+#define CPUID_7_0_ECX_CET_SHSTK (1U << 7) /* CET SHSTK feature bit */
#define CPUID_7_0_EDX_AVX512_4VNNIW (1U << 2) /* AVX512 Neural Network Instructions */
#define CPUID_7_0_EDX_AVX512_4FMAPS (1U << 3) /* AVX512 Multiply Accumulation Single Precision */
#define CPUID_7_0_EDX_PCONFIG (1U << 18) /* Platform Configuration */
@@ -1021,6 +1039,19 @@ typedef struct XSavePKRU {
uint32_t padding;
} XSavePKRU;

+/* Ext. save area 11: User mode CET state */
+typedef struct XSaveCETU {
+ uint64_t u_cet;
+ uint64_t user_ssp;
+} XSaveCETU;
+
+/* Ext. save area 12: Supervisor mode CET state */
+typedef struct XSaveCETS {
+ uint64_t kernel_ssp;
+ uint64_t pl1_ssp;
+ uint64_t pl2_ssp;
+} XSaveCETS;
+
typedef struct X86XSaveArea {
X86LegacyXSaveArea legacy;
X86XSaveHeader header;
@@ -1039,6 +1070,9 @@ typedef struct X86XSaveArea {
XSaveHi16_ZMM hi16_zmm_state;
/* PKRU State: */
XSavePKRU pkru_state;
+ /* CET State: */
+ XSaveCETU cet_u;
+ XSaveCETS cet_s;
} X86XSaveArea;

QEMU_BUILD_BUG_ON(offsetof(X86XSaveArea, avx_state) != 0x240);
--
2.17.1


2018-12-29 01:32:11

by Paolo Bonzini

[permalink] [raw]
Subject: Re: [Qemu-devel][PATCH 2/4] Add CET SHSTK and IBT CPUID feature-word definitions.

On 26/12/18 09:25, Yang Weijiang wrote:
> @@ -1233,6 +1252,14 @@ static const ExtSaveArea x86_ext_save_areas[] = {
> { .feature = FEAT_7_0_ECX, .bits = CPUID_7_0_ECX_PKU,
> .offset = offsetof(X86XSaveArea, pkru_state),
> .size = sizeof(XSavePKRU) },
> + [XSTATE_CET_U_BIT] = {
> + .feature = FEAT_7_0_ECX, .bits = CPUID_7_0_ECX_CET_SHSTK,
> + .offset = offsetof(X86XSaveArea, cet_u),

These offsets are incorrect, since supervisor states are only stored in
the compacted format. In fact, in patch 4, supervisor states should
return 0 in CPUID(EAX=0Dh,ECX=n).EBX.

You can use offset == 0 to distinguish supervisor and user states, so
that supervisor states are skipped in xsave_area_size and x86_cpu_reset.

Thanks,

Paolo

> + .size = sizeof(XSaveCETU) },
> + [XSTATE_CET_S_BIT] = {
> + .feature = FEAT_7_0_ECX, .bits = CPUID_7_0_ECX_CET_SHSTK,
> + .offset = offsetof(X86XSaveArea, cet_s),
> + .size = sizeof(XSaveCETS) },
> };
>
> static uint32_t xsave_area_size(uint64_t mask)


2018-12-29 22:04:21

by Yang Weijiang

[permalink] [raw]
Subject: Re: [Qemu-devel][PATCH 2/4] Add CET SHSTK and IBT CPUID feature-word definitions.

On Fri, Dec 28, 2018 at 03:25:10PM +0100, Paolo Bonzini wrote:
Thanks a lot Paolo for the comments!

I'll fix the issue in next version.

> On 26/12/18 09:25, Yang Weijiang wrote:
> > @@ -1233,6 +1252,14 @@ static const ExtSaveArea x86_ext_save_areas[] = {
> > { .feature = FEAT_7_0_ECX, .bits = CPUID_7_0_ECX_PKU,
> > .offset = offsetof(X86XSaveArea, pkru_state),
> > .size = sizeof(XSavePKRU) },
> > + [XSTATE_CET_U_BIT] = {
> > + .feature = FEAT_7_0_ECX, .bits = CPUID_7_0_ECX_CET_SHSTK,
> > + .offset = offsetof(X86XSaveArea, cet_u),
>
> These offsets are incorrect, since supervisor states are only stored in
> the compacted format. In fact, in patch 4, supervisor states should
> return 0 in CPUID(EAX=0Dh,ECX=n).EBX.
>
> You can use offset == 0 to distinguish supervisor and user states, so
> that supervisor states are skipped in xsave_area_size and x86_cpu_reset.
>
> Thanks,
>
> Paolo
>
> > + .size = sizeof(XSaveCETU) },
> > + [XSTATE_CET_S_BIT] = {
> > + .feature = FEAT_7_0_ECX, .bits = CPUID_7_0_ECX_CET_SHSTK,
> > + .offset = offsetof(X86XSaveArea, cet_s),
> > + .size = sizeof(XSaveCETS) },
> > };
> >
> > static uint32_t xsave_area_size(uint64_t mask)

Subject: Re: [Qemu-devel][PATCH 3/4] Add hepler functions for CPUID xsave area size calculation.

Typo in subject line (helper)

> On 26 Dec 2018, at 09:25, Yang Weijiang <[email protected]> wrote:
>
> These functions are called when return CPUID xsave area
> size information.
>
> Signed-off-by: Zhang Yi <[email protected]>
> Signed-off-by: Yang Weijiang <[email protected]>
> ---
> target/i386/cpu.c | 26 +++++++++++++++++++++++++-
> 1 file changed, 25 insertions(+), 1 deletion(-)
>
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 3630c688d6..cf4f2798dc 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -1281,12 +1281,34 @@ static inline bool accel_uses_host_cpuid(void)
> return kvm_enabled() || hvf_enabled();
> }
>
> +static uint32_t xsave_area_size_compat(uint64_t mask)

Just curious, why “compat”?

> +{
> + int i;
> + uint64_t ret = 0;
> + uint32_t offset;
> +
> + for (i = 0; i < ARRAY_SIZE(x86_ext_save_areas); i++) {
> + const ExtSaveArea *esa = &x86_ext_save_areas[i];
> + offset = i > 1 ? ret : esa->offset;

What about a named constant instead of ‘1’?

(note that a loop around line 4657 starts at 2, so I guess it’s OK to use 1 here, or at least >= 2 ;-)


> + if ((mask >> i) & 1) {
> + ret = MAX(ret, offset + esa->size);
> + }
> + }
> + return ret;
> +}
> +
> static inline uint64_t x86_cpu_xsave_components(X86CPU *cpu)
> {
> return ((uint64_t)cpu->env.features[FEAT_XSAVE_COMP_HI]) << 32 |
> cpu->env.features[FEAT_XSAVE_COMP_LO];
> }
>
> +static inline uint64_t x86_cpu_xsave_sv_components(X86CPU *cpu)
> +{
> + return ((uint64_t)cpu->env.features[FEAT_XSAVE_SV_HI]) << 32 |
> + cpu->env.features[FEAT_XSAVE_SV_LO];
> +}
> +
> const char *get_register_name_32(unsigned int reg)
> {
> if (reg >= CPU_NB_REGS32) {
> @@ -4913,8 +4935,10 @@ static void x86_cpu_enable_xsave_components(X86CPU *cpu)
> }
> }
>
> - env->features[FEAT_XSAVE_COMP_LO] = mask;
> + env->features[FEAT_XSAVE_COMP_LO] = mask & CPUID_XSTATE_USER_MASK;
> env->features[FEAT_XSAVE_COMP_HI] = mask >> 32;
> + env->features[FEAT_XSAVE_SV_LO] = mask & CPUID_XSTATE_KERNEL_MASK;
> + env->features[FEAT_XSAVE_SV_HI] = mask >> 32;
> }
>
> /***** Steps involved on loading and filtering CPUID data
> --
> 2.17.1
>