The indentation style in `x86_ext_save_areas[]` is extremely inconsistent. Clean it up to ensure a uniform style. Tested-by: Farrah Chen Reviewed-by: Xiaoyao Li Signed-off-by: Zhao Liu --- target/i386/cpu.c | 58 +++++++++++++++++++++++++++-------------------- 1 file changed, 33 insertions(+), 25 deletions(-) diff --git a/target/i386/cpu.c b/target/i386/cpu.c index 641777578637..c598f09f3d50 100644 --- a/target/i386/cpu.c +++ b/target/i386/cpu.c @@ -2028,38 +2028,46 @@ ExtSaveArea x86_ext_save_areas[XSAVE_STATE_AREA_COUNT] = { .feature = FEAT_1_ECX, .bits = CPUID_EXT_XSAVE, .size = sizeof(X86LegacyXSaveArea) + sizeof(X86XSaveHeader), }, - [XSTATE_YMM_BIT] = - { .feature = FEAT_1_ECX, .bits = CPUID_EXT_AVX, - .size = sizeof(XSaveAVX) }, - [XSTATE_BNDREGS_BIT] = - { .feature = FEAT_7_0_EBX, .bits = CPUID_7_0_EBX_MPX, - .size = sizeof(XSaveBNDREG) }, - [XSTATE_BNDCSR_BIT] = - { .feature = FEAT_7_0_EBX, .bits = CPUID_7_0_EBX_MPX, - .size = sizeof(XSaveBNDCSR) }, - [XSTATE_OPMASK_BIT] = - { .feature = FEAT_7_0_EBX, .bits = CPUID_7_0_EBX_AVX512F, - .size = sizeof(XSaveOpmask) }, - [XSTATE_ZMM_Hi256_BIT] = - { .feature = FEAT_7_0_EBX, .bits = CPUID_7_0_EBX_AVX512F, - .size = sizeof(XSaveZMM_Hi256) }, - [XSTATE_Hi16_ZMM_BIT] = - { .feature = FEAT_7_0_EBX, .bits = CPUID_7_0_EBX_AVX512F, - .size = sizeof(XSaveHi16_ZMM) }, - [XSTATE_PKRU_BIT] = - { .feature = FEAT_7_0_ECX, .bits = CPUID_7_0_ECX_PKU, - .size = sizeof(XSavePKRU) }, + [XSTATE_YMM_BIT] = { + .feature = FEAT_1_ECX, .bits = CPUID_EXT_AVX, + .size = sizeof(XSaveAVX), + }, + [XSTATE_BNDREGS_BIT] = { + .feature = FEAT_7_0_EBX, .bits = CPUID_7_0_EBX_MPX, + .size = sizeof(XSaveBNDREG), + }, + [XSTATE_BNDCSR_BIT] = { + .feature = FEAT_7_0_EBX, .bits = CPUID_7_0_EBX_MPX, + .size = sizeof(XSaveBNDCSR), + }, + [XSTATE_OPMASK_BIT] = { + .feature = FEAT_7_0_EBX, .bits = CPUID_7_0_EBX_AVX512F, + .size = sizeof(XSaveOpmask), + }, + [XSTATE_ZMM_Hi256_BIT] = { + .feature = FEAT_7_0_EBX, .bits = CPUID_7_0_EBX_AVX512F, + .size = sizeof(XSaveZMM_Hi256), + }, + [XSTATE_Hi16_ZMM_BIT] = { + .feature = FEAT_7_0_EBX, .bits = CPUID_7_0_EBX_AVX512F, + .size = sizeof(XSaveHi16_ZMM), + }, + [XSTATE_PKRU_BIT] = { + .feature = FEAT_7_0_ECX, .bits = CPUID_7_0_ECX_PKU, + .size = sizeof(XSavePKRU), + }, [XSTATE_ARCH_LBR_BIT] = { - .feature = FEAT_7_0_EDX, .bits = CPUID_7_0_EDX_ARCH_LBR, - .offset = 0 /*supervisor mode component, offset = 0 */, - .size = sizeof(XSavesArchLBR) }, + .feature = FEAT_7_0_EDX, .bits = CPUID_7_0_EDX_ARCH_LBR, + .offset = 0 /*supervisor mode component, offset = 0 */, + .size = sizeof(XSavesArchLBR), + }, [XSTATE_XTILE_CFG_BIT] = { .feature = FEAT_7_0_EDX, .bits = CPUID_7_0_EDX_AMX_TILE, .size = sizeof(XSaveXTILECFG), }, [XSTATE_XTILE_DATA_BIT] = { .feature = FEAT_7_0_EDX, .bits = CPUID_7_0_EDX_AMX_TILE, - .size = sizeof(XSaveXTILEDATA) + .size = sizeof(XSaveXTILEDATA), }, }; -- 2.34.1 Arch LBR state is area 15, not 19. Fix this comment. And considerring other areas don't mention user or supervisor state, for consistent style, remove "Supervisor mode" from its comment. Moreover, rename XSavesArchLBR to XSaveArchLBR since there's no need to emphasize XSAVES in naming; the XSAVE related structure is mainly used to represent memory layout. In addition, arch lbr specifies its offset of xsave component as 0. But this cannot help on anything. The offset of ExtSaveArea is initialized by accelerators (e.g., hvf_cpu_xsave_init(), kvm_cpu_xsave_init() and x86_tcg_cpu_xsave_init()), so explicitly setting the offset doesn't work and CPUID 0xD encoding has already ensure supervisor states won't have non-zero offsets. Drop the offset initialization and its comment from the xsave area of arch lbr. Tested-by: Farrah Chen Reviewed-by: Zide Chen Reviewed-by: Xiaoyao Li Signed-off-by: Zhao Liu --- target/i386/cpu.c | 3 +-- target/i386/cpu.h | 8 ++++---- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/target/i386/cpu.c b/target/i386/cpu.c index c598f09f3d50..34a4c2410d03 100644 --- a/target/i386/cpu.c +++ b/target/i386/cpu.c @@ -2058,8 +2058,7 @@ ExtSaveArea x86_ext_save_areas[XSAVE_STATE_AREA_COUNT] = { }, [XSTATE_ARCH_LBR_BIT] = { .feature = FEAT_7_0_EDX, .bits = CPUID_7_0_EDX_ARCH_LBR, - .offset = 0 /*supervisor mode component, offset = 0 */, - .size = sizeof(XSavesArchLBR), + .size = sizeof(XSaveArchLBR), }, [XSTATE_XTILE_CFG_BIT] = { .feature = FEAT_7_0_EDX, .bits = CPUID_7_0_EDX_AMX_TILE, diff --git a/target/i386/cpu.h b/target/i386/cpu.h index cee1f692a1c3..c95b772719ce 100644 --- a/target/i386/cpu.h +++ b/target/i386/cpu.h @@ -1747,15 +1747,15 @@ typedef struct { #define ARCH_LBR_NR_ENTRIES 32 -/* Ext. save area 19: Supervisor mode Arch LBR state */ -typedef struct XSavesArchLBR { +/* Ext. save area 15: Arch LBR state */ +typedef struct XSaveArchLBR { uint64_t lbr_ctl; uint64_t lbr_depth; uint64_t ler_from; uint64_t ler_to; uint64_t ler_info; LBREntry lbr_records[ARCH_LBR_NR_ENTRIES]; -} XSavesArchLBR; +} XSaveArchLBR; QEMU_BUILD_BUG_ON(sizeof(XSaveAVX) != 0x100); QEMU_BUILD_BUG_ON(sizeof(XSaveBNDREG) != 0x40); @@ -1766,7 +1766,7 @@ QEMU_BUILD_BUG_ON(sizeof(XSaveHi16_ZMM) != 0x400); QEMU_BUILD_BUG_ON(sizeof(XSavePKRU) != 0x8); QEMU_BUILD_BUG_ON(sizeof(XSaveXTILECFG) != 0x40); QEMU_BUILD_BUG_ON(sizeof(XSaveXTILEDATA) != 0x2000); -QEMU_BUILD_BUG_ON(sizeof(XSavesArchLBR) != 0x328); +QEMU_BUILD_BUG_ON(sizeof(XSaveArchLBR) != 0x328); typedef struct ExtSaveArea { uint32_t feature, bits; -- 2.34.1 - Move ARCH_LBR_NR_ENTRIES macro and LBREntry definition before XSAVE areas definitions. - Reorder XSavesArchLBR (area 15) between XSavePKRU (area 9) and XSaveXTILECFG (area 17), and reorder the related QEMU_BUILD_BUG_ON check to keep the same ordering. This makes xsave structures to be organized together and makes them clearer. Tested-by: Farrah Chen Reviewed-by: Zide Chen Reviewed-by: Xiaoyao Li Signed-off-by: Zhao Liu --- target/i386/cpu.h | 38 +++++++++++++++++++------------------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/target/i386/cpu.h b/target/i386/cpu.h index c95b772719ce..a183394eca7f 100644 --- a/target/i386/cpu.h +++ b/target/i386/cpu.h @@ -1652,6 +1652,14 @@ typedef struct { #define NB_OPMASK_REGS 8 +typedef struct { + uint64_t from; + uint64_t to; + uint64_t info; +} LBREntry; + +#define ARCH_LBR_NR_ENTRIES 32 + /* CPU can't have 0xFFFFFFFF APIC ID, use that value to distinguish * that APIC ID hasn't been set yet */ @@ -1729,24 +1737,6 @@ typedef struct XSavePKRU { uint32_t padding; } XSavePKRU; -/* Ext. save area 17: AMX XTILECFG state */ -typedef struct XSaveXTILECFG { - uint8_t xtilecfg[64]; -} XSaveXTILECFG; - -/* Ext. save area 18: AMX XTILEDATA state */ -typedef struct XSaveXTILEDATA { - uint8_t xtiledata[8][1024]; -} XSaveXTILEDATA; - -typedef struct { - uint64_t from; - uint64_t to; - uint64_t info; -} LBREntry; - -#define ARCH_LBR_NR_ENTRIES 32 - /* Ext. save area 15: Arch LBR state */ typedef struct XSaveArchLBR { uint64_t lbr_ctl; @@ -1757,6 +1747,16 @@ typedef struct XSaveArchLBR { LBREntry lbr_records[ARCH_LBR_NR_ENTRIES]; } XSaveArchLBR; +/* Ext. save area 17: AMX XTILECFG state */ +typedef struct XSaveXTILECFG { + uint8_t xtilecfg[64]; +} XSaveXTILECFG; + +/* Ext. save area 18: AMX XTILEDATA state */ +typedef struct XSaveXTILEDATA { + uint8_t xtiledata[8][1024]; +} XSaveXTILEDATA; + QEMU_BUILD_BUG_ON(sizeof(XSaveAVX) != 0x100); QEMU_BUILD_BUG_ON(sizeof(XSaveBNDREG) != 0x40); QEMU_BUILD_BUG_ON(sizeof(XSaveBNDCSR) != 0x40); @@ -1764,9 +1764,9 @@ QEMU_BUILD_BUG_ON(sizeof(XSaveOpmask) != 0x40); QEMU_BUILD_BUG_ON(sizeof(XSaveZMM_Hi256) != 0x200); QEMU_BUILD_BUG_ON(sizeof(XSaveHi16_ZMM) != 0x400); QEMU_BUILD_BUG_ON(sizeof(XSavePKRU) != 0x8); +QEMU_BUILD_BUG_ON(sizeof(XSaveArchLBR) != 0x328); QEMU_BUILD_BUG_ON(sizeof(XSaveXTILECFG) != 0x40); QEMU_BUILD_BUG_ON(sizeof(XSaveXTILEDATA) != 0x2000); -QEMU_BUILD_BUG_ON(sizeof(XSaveArchLBR) != 0x328); typedef struct ExtSaveArea { uint32_t feature, bits; -- 2.34.1 Some XSAVE components depend on multiple features. For example, Opmask/ ZMM_Hi256/Hi16_ZMM depend on avx512f OR avx10, and for CET (which will be supported later), cet_u/cet_s will depend on shstk OR ibt. Although previously there's the special check for the dependencies of AVX512F OR AVX10 on their respective XSAVE components (in cpuid_has_xsave_feature()), to make the code more general and avoid adding more special cases, make ExtSaveArea store a features array instead of a single feature, so that it can describe multiple dependencies. Tested-by: Farrah Chen Signed-off-by: Zhao Liu --- Changes Since v3: - Add a FIXME in x86_cpu_feature_name() as the note to improve the case with multiple dependencies. --- target/i386/cpu.c | 78 ++++++++++++++++++++++++++++++++++------------- target/i386/cpu.h | 9 +++++- 2 files changed, 65 insertions(+), 22 deletions(-) diff --git a/target/i386/cpu.c b/target/i386/cpu.c index 34a4c2410d03..e495e6d9b21c 100644 --- a/target/i386/cpu.c +++ b/target/i386/cpu.c @@ -2020,53 +2020,77 @@ static const X86RegisterInfo32 x86_reg_info_32[CPU_NB_REGS32] = { ExtSaveArea x86_ext_save_areas[XSAVE_STATE_AREA_COUNT] = { [XSTATE_FP_BIT] = { /* x87 FP state component is always enabled if XSAVE is supported */ - .feature = FEAT_1_ECX, .bits = CPUID_EXT_XSAVE, .size = sizeof(X86LegacyXSaveArea) + sizeof(X86XSaveHeader), + .features = { + { FEAT_1_ECX, CPUID_EXT_XSAVE }, + }, }, [XSTATE_SSE_BIT] = { /* SSE state component is always enabled if XSAVE is supported */ - .feature = FEAT_1_ECX, .bits = CPUID_EXT_XSAVE, .size = sizeof(X86LegacyXSaveArea) + sizeof(X86XSaveHeader), + .features = { + { FEAT_1_ECX, CPUID_EXT_XSAVE }, + }, }, [XSTATE_YMM_BIT] = { - .feature = FEAT_1_ECX, .bits = CPUID_EXT_AVX, .size = sizeof(XSaveAVX), + .features = { + { FEAT_1_ECX, CPUID_EXT_AVX }, + }, }, [XSTATE_BNDREGS_BIT] = { - .feature = FEAT_7_0_EBX, .bits = CPUID_7_0_EBX_MPX, .size = sizeof(XSaveBNDREG), + .features = { + { FEAT_7_0_EBX, CPUID_7_0_EBX_MPX }, + }, }, [XSTATE_BNDCSR_BIT] = { - .feature = FEAT_7_0_EBX, .bits = CPUID_7_0_EBX_MPX, .size = sizeof(XSaveBNDCSR), + .features = { + { FEAT_7_0_EBX, CPUID_7_0_EBX_MPX }, + }, }, [XSTATE_OPMASK_BIT] = { - .feature = FEAT_7_0_EBX, .bits = CPUID_7_0_EBX_AVX512F, .size = sizeof(XSaveOpmask), + .features = { + { FEAT_7_0_EBX, CPUID_7_0_EBX_AVX512F }, + }, }, [XSTATE_ZMM_Hi256_BIT] = { - .feature = FEAT_7_0_EBX, .bits = CPUID_7_0_EBX_AVX512F, .size = sizeof(XSaveZMM_Hi256), + .features = { + { FEAT_7_0_EBX, CPUID_7_0_EBX_AVX512F }, + }, }, [XSTATE_Hi16_ZMM_BIT] = { - .feature = FEAT_7_0_EBX, .bits = CPUID_7_0_EBX_AVX512F, .size = sizeof(XSaveHi16_ZMM), + .features = { + { FEAT_7_0_EBX, CPUID_7_0_EBX_AVX512F }, + }, }, [XSTATE_PKRU_BIT] = { - .feature = FEAT_7_0_ECX, .bits = CPUID_7_0_ECX_PKU, .size = sizeof(XSavePKRU), + .features = { + { FEAT_7_0_ECX, CPUID_7_0_ECX_PKU }, + }, }, [XSTATE_ARCH_LBR_BIT] = { - .feature = FEAT_7_0_EDX, .bits = CPUID_7_0_EDX_ARCH_LBR, .size = sizeof(XSaveArchLBR), + .features = { + { FEAT_7_0_EDX, CPUID_7_0_EDX_ARCH_LBR }, + }, }, [XSTATE_XTILE_CFG_BIT] = { - .feature = FEAT_7_0_EDX, .bits = CPUID_7_0_EDX_AMX_TILE, .size = sizeof(XSaveXTILECFG), + .features = { + { FEAT_7_0_EDX, CPUID_7_0_EDX_AMX_TILE }, + }, }, [XSTATE_XTILE_DATA_BIT] = { - .feature = FEAT_7_0_EDX, .bits = CPUID_7_0_EDX_AMX_TILE, .size = sizeof(XSaveXTILEDATA), + .features = { + { FEAT_7_0_EDX, CPUID_7_0_EDX_AMX_TILE }, + }, }, }; @@ -7131,16 +7155,24 @@ static inline void feat2prop(char *s) static const char *x86_cpu_feature_name(FeatureWord w, int bitnr) { const char *name; - /* XSAVE components are automatically enabled by other features, + /* + * XSAVE components are automatically enabled by other features, * so return the original feature name instead */ if (w == FEAT_XSAVE_XCR0_LO || w == FEAT_XSAVE_XCR0_HI) { int comp = (w == FEAT_XSAVE_XCR0_HI) ? bitnr + 32 : bitnr; - if (comp < ARRAY_SIZE(x86_ext_save_areas) && - x86_ext_save_areas[comp].bits) { - w = x86_ext_save_areas[comp].feature; - bitnr = ctz32(x86_ext_save_areas[comp].bits); + if (comp < ARRAY_SIZE(x86_ext_save_areas)) { + /* + * Present the first feature as the default. + * FIXME: select and present the one which is actually enabled + * among multiple dependencies. + */ + const FeatureMask *fm = &x86_ext_save_areas[comp].features[0]; + if (fm->mask) { + w = fm->index; + bitnr = ctz32(fm->mask); + } } } @@ -8610,11 +8642,15 @@ static bool cpuid_has_xsave_feature(CPUX86State *env, const ExtSaveArea *esa) return false; } - if (env->features[esa->feature] & esa->bits) { - return true; + for (int i = 0; i < ARRAY_SIZE(esa->features); i++) { + if (env->features[esa->features[i].index] & esa->features[i].mask) { + return true; + } } - if (esa->feature == FEAT_7_0_EBX && esa->bits == CPUID_7_0_EBX_AVX512F - && (env->features[FEAT_7_1_EDX] & CPUID_7_1_EDX_AVX10)) { + + if (esa->features[0].index == FEAT_7_0_EBX && + esa->features[0].mask == CPUID_7_0_EBX_AVX512F && + (env->features[FEAT_7_1_EDX] & CPUID_7_1_EDX_AVX10)) { return true; } diff --git a/target/i386/cpu.h b/target/i386/cpu.h index a183394eca7f..3d74afc5a8e7 100644 --- a/target/i386/cpu.h +++ b/target/i386/cpu.h @@ -1769,9 +1769,16 @@ QEMU_BUILD_BUG_ON(sizeof(XSaveXTILECFG) != 0x40); QEMU_BUILD_BUG_ON(sizeof(XSaveXTILEDATA) != 0x2000); typedef struct ExtSaveArea { - uint32_t feature, bits; uint32_t offset, size; uint32_t ecx; + /* + * The dependencies in the array work as OR relationships, which + * means having just one of those features is enough. + * + * At most two features are sharing the same xsave area. + * Number of features can be adjusted if necessary. + */ + const FeatureMask features[2]; } ExtSaveArea; #define XSAVE_STATE_AREA_COUNT (XSTATE_XTILE_DATA_BIT + 1) -- 2.34.1 With feature array in ExtSaveArea, add avx10 as the second dependency for Opmask/ZMM_Hi256/Hi16_ZMM xsave components, and drop the special check in cpuid_has_xsave_feature(). Tested-by: Farrah Chen Reviewed-by: Xiaoyao Li Signed-off-by: Zhao Liu --- target/i386/cpu.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/target/i386/cpu.c b/target/i386/cpu.c index e495e6d9b21c..70a282668f72 100644 --- a/target/i386/cpu.c +++ b/target/i386/cpu.c @@ -2054,18 +2054,21 @@ ExtSaveArea x86_ext_save_areas[XSAVE_STATE_AREA_COUNT] = { .size = sizeof(XSaveOpmask), .features = { { FEAT_7_0_EBX, CPUID_7_0_EBX_AVX512F }, + { FEAT_7_1_EDX, CPUID_7_1_EDX_AVX10 }, }, }, [XSTATE_ZMM_Hi256_BIT] = { .size = sizeof(XSaveZMM_Hi256), .features = { { FEAT_7_0_EBX, CPUID_7_0_EBX_AVX512F }, + { FEAT_7_1_EDX, CPUID_7_1_EDX_AVX10 }, }, }, [XSTATE_Hi16_ZMM_BIT] = { .size = sizeof(XSaveHi16_ZMM), .features = { { FEAT_7_0_EBX, CPUID_7_0_EBX_AVX512F }, + { FEAT_7_1_EDX, CPUID_7_1_EDX_AVX10 }, }, }, [XSTATE_PKRU_BIT] = { @@ -8648,12 +8651,6 @@ static bool cpuid_has_xsave_feature(CPUX86State *env, const ExtSaveArea *esa) } } - if (esa->features[0].index == FEAT_7_0_EBX && - esa->features[0].mask == CPUID_7_0_EBX_AVX512F && - (env->features[FEAT_7_1_EDX] & CPUID_7_1_EDX_AVX10)) { - return true; - } - return false; } -- 2.34.1 At present, in KVM, x86_ext_save_areas[] is initialized based on Host CPUIDs. Except AMX xstates, both user & supervisor xstates have their information exposed in KVM_GET_SUPPORTED_CPUID. Therefore, their entries in x86_ext_save_areas[] should be filled based on KVM support. For AMX xstates (XFEATURE_MASK_XTILE_DATA and XFEATURE_MASK_XTILE_CFG), KVM doesn't report their details before they (mainly XSTATE_XTILE_DATA_MASK) get permission on host. But this happens within the function kvm_request_xsave_components(), after the current initialization. So still fill AMX entries with Host CPUIDs. In addition, drop a check: "if (eax != 0)" when assert the assert the size of xstate. In fact, this check is incorrect, since any valid xstate should have non-zero size of xstate area. Tested-by: Farrah Chen Signed-off-by: Zhao Liu --- Changes Since v3: - New commit. --- target/i386/cpu.h | 3 +++ target/i386/kvm/kvm-cpu.c | 23 +++++++++++++++++------ 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/target/i386/cpu.h b/target/i386/cpu.h index 3d74afc5a8e7..f065527757c4 100644 --- a/target/i386/cpu.h +++ b/target/i386/cpu.h @@ -609,6 +609,9 @@ typedef enum X86Seg { #define XSTATE_DYNAMIC_MASK (XSTATE_XTILE_DATA_MASK) +#define XSTATE_XTILE_MASK (XSTATE_XTILE_DATA_MASK | \ + XSTATE_XTILE_CFG_MASK) + #define ESA_FEATURE_ALIGN64_BIT 1 #define ESA_FEATURE_XFD_BIT 2 diff --git a/target/i386/kvm/kvm-cpu.c b/target/i386/kvm/kvm-cpu.c index 9c25b5583955..2e2d47d2948a 100644 --- a/target/i386/kvm/kvm-cpu.c +++ b/target/i386/kvm/kvm-cpu.c @@ -136,7 +136,7 @@ static void kvm_cpu_max_instance_init(X86CPU *cpu) static void kvm_cpu_xsave_init(void) { static bool first = true; - uint32_t eax, ebx, ecx, edx; + uint32_t eax, ebx, ecx, unused; int i; if (!first) { @@ -154,12 +154,23 @@ static void kvm_cpu_xsave_init(void) if (!esa->size) { continue; } - host_cpuid(0xd, i, &eax, &ebx, &ecx, &edx); - if (eax != 0) { - assert(esa->size == eax); - esa->offset = ebx; - esa->ecx = ecx; + + /* + * AMX xstates are supported in KVM_GET_SUPPORTED_CPUID only when + * XSTATE_XTILE_DATA_MASK gets guest permission in + * kvm_request_xsave_components(). + */ + if (!((1 << i) & XSTATE_XTILE_MASK)) { + eax = kvm_arch_get_supported_cpuid(kvm_state, 0xd, i, R_EAX); + ebx = kvm_arch_get_supported_cpuid(kvm_state, 0xd, i, R_EBX); + ecx = kvm_arch_get_supported_cpuid(kvm_state, 0xd, i, R_ECX); + } else { + host_cpuid(0xd, i, &eax, &ebx, &ecx, &unused); } + + assert(esa->size == eax); + esa->offset = ebx; + esa->ecx = ecx; } } -- 2.34.1 The x86_ext_save_areas[] is expected to be well initialized by accelerators and its xstate detail information cannot be changed by user. So use x86_ext_save_areas[] to encode CPUID.0XD subleaves directly without other hardcoding & masking. And for arch LBR case, since its entry in x86_ext_save_areas[] has been initialized based KVM support in kvm_cpu_xsave_init(), so use x86_ext_save_areas[] for encoding. Tested-by: Farrah Chen Signed-off-by: Zhao Liu --- Changes Since v3: - New commit. --- target/i386/cpu.c | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/target/i386/cpu.c b/target/i386/cpu.c index 70a282668f72..f3bf7f892214 100644 --- a/target/i386/cpu.c +++ b/target/i386/cpu.c @@ -8188,20 +8188,17 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count, } } else if (count == 0xf && cpu->enable_pmu && (env->features[FEAT_7_0_EDX] & CPUID_7_0_EDX_ARCH_LBR)) { - x86_cpu_get_supported_cpuid(0xD, count, eax, ebx, ecx, edx); + const ExtSaveArea *esa = &x86_ext_save_areas[count]; + + *eax = esa->size; + *ebx = esa->offset; + *ecx = esa->ecx; } else if (count < ARRAY_SIZE(x86_ext_save_areas)) { const ExtSaveArea *esa = &x86_ext_save_areas[count]; - if (x86_cpu_xsave_xcr0_components(cpu) & (1ULL << count)) { - *eax = esa->size; - *ebx = esa->offset; - *ecx = esa->ecx & - (ESA_FEATURE_ALIGN64_MASK | ESA_FEATURE_XFD_MASK); - } else if (x86_cpu_xsave_xss_components(cpu) & (1ULL << count)) { - *eax = esa->size; - *ebx = 0; - *ecx = 1; - } + *eax = esa->size; + *ebx = esa->offset; + *ecx = esa->ecx; } break; } -- 2.34.1 The arch lbr state has 2 dependencies: * Arch lbr feature bit (CPUID 0x7.0x0:EDX[bit 19]): This bit also depends on pmu property. Mask it off if pmu is disabled in x86_cpu_expand_features(), so that it is not needed to repeatedly check whether this bit is set as well as pmu is enabled. Note this doesn't need compat option, since even KVM hasn't support arch lbr yet. The supported xstate is constructed based such dependency in cpuid_has_xsave_feature(), so if pmu is disabled and arch lbr bit is masked off, then arch lbr state won't be included in supported xstates. Thus it's safe to drop the check on arch lbr bit in CPUID 0xD encoding. * XSAVES feature bit (CPUID 0xD.0x1.EAX[bit 3]): Arch lbr state is a supervisor state, which requires the XSAVES feature support. Enumerate supported supervisor state based on XSAVES feature bit in x86_cpu_enable_xsave_components(). Then it's safe to drop the check on XSAVES feature support during CPUID 0XD encoding. Suggested-by: Zide Chen Tested-by: Farrah Chen Reviewed-by: Zide Chen Signed-off-by: Zhao Liu --- target/i386/cpu.c | 26 ++++++++++---------------- 1 file changed, 10 insertions(+), 16 deletions(-) diff --git a/target/i386/cpu.c b/target/i386/cpu.c index f3bf7f892214..56b4c57969c1 100644 --- a/target/i386/cpu.c +++ b/target/i386/cpu.c @@ -8179,20 +8179,6 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count, *ebx = xsave_area_size(xstate, true); *ecx = env->features[FEAT_XSAVE_XSS_LO]; *edx = env->features[FEAT_XSAVE_XSS_HI]; - if (kvm_enabled() && cpu->enable_pmu && - (env->features[FEAT_7_0_EDX] & CPUID_7_0_EDX_ARCH_LBR) && - (*eax & CPUID_XSAVE_XSAVES)) { - *ecx |= XSTATE_ARCH_LBR_MASK; - } else { - *ecx &= ~XSTATE_ARCH_LBR_MASK; - } - } else if (count == 0xf && cpu->enable_pmu - && (env->features[FEAT_7_0_EDX] & CPUID_7_0_EDX_ARCH_LBR)) { - const ExtSaveArea *esa = &x86_ext_save_areas[count]; - - *eax = esa->size; - *ebx = esa->offset; - *ecx = esa->ecx; } else if (count < ARRAY_SIZE(x86_ext_save_areas)) { const ExtSaveArea *esa = &x86_ext_save_areas[count]; @@ -8910,6 +8896,12 @@ static void x86_cpu_enable_xsave_components(X86CPU *cpu) mask = 0; for (i = 0; i < ARRAY_SIZE(x86_ext_save_areas); i++) { + /* Skip supervisor states if XSAVES is not supported. */ + if (CPUID_XSTATE_XSS_MASK & (1 << i) && + !(env->features[FEAT_XSAVE] & CPUID_XSAVE_XSAVES)) { + continue; + } + const ExtSaveArea *esa = &x86_ext_save_areas[i]; if (cpuid_has_xsave_feature(env, esa)) { mask |= (1ULL << i); @@ -9027,11 +9019,13 @@ void x86_cpu_expand_features(X86CPU *cpu, Error **errp) } } - if (!cpu->pdcm_on_even_without_pmu) { + if (!cpu->enable_pmu) { /* PDCM is fixed1 bit for TDX */ - if (!cpu->enable_pmu && !is_tdx_vm()) { + if (!cpu->pdcm_on_even_without_pmu && !is_tdx_vm()) { env->features[FEAT_1_ECX] &= ~CPUID_EXT_PDCM; } + + env->features[FEAT_7_0_EDX] &= ~CPUID_7_0_EDX_ARCH_LBR; } for (i = 0; i < ARRAY_SIZE(feature_dependencies); i++) { -- 2.34.1 Since CPUID_7_0_EDX_ARCH_LBR will be masked off if pmu is disabled, there's no need to check CPUID_7_0_EDX_ARCH_LBR feature with pmu. Tested-by: Farrah Chen Reviewed-by: Zide Chen Reviewed-by: Xiaoyao Li Signed-off-by: Zhao Liu --- target/i386/cpu.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/target/i386/cpu.c b/target/i386/cpu.c index 56b4c57969c1..62769db3ebb7 100644 --- a/target/i386/cpu.c +++ b/target/i386/cpu.c @@ -8273,11 +8273,16 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count, } break; } - case 0x1C: - if (cpu->enable_pmu && (env->features[FEAT_7_0_EDX] & CPUID_7_0_EDX_ARCH_LBR)) { - x86_cpu_get_supported_cpuid(0x1C, 0, eax, ebx, ecx, edx); - *edx = 0; + case 0x1C: /* Last Branch Records Information Leaf */ + *eax = 0; + *ebx = 0; + *ecx = 0; + *edx = 0; + if (!(env->features[FEAT_7_0_EDX] & CPUID_7_0_EDX_ARCH_LBR)) { + break; } + x86_cpu_get_supported_cpuid(0x1C, 0, eax, ebx, ecx, edx); + *edx = 0; /* EDX is reserved. */ break; case 0x1D: { /* AMX TILE, for now hardcoded for Sapphire Rapids*/ -- 2.34.1 From: Chao Gao Arch lbr is a supervisor xstate, but its area is not covered in x86_cpu_init_xsave(). Fix it by checking supported xss bitmap. In addition, drop the (uint64_t) type casts for supported_xcr0 since x86_cpu_get_supported_feature_word() returns uint64_t so that the cast is not needed. Then ensure line length is within 90 characters. Tested-by: Farrah Chen Reviewed-by: Xiaoyao Li Signed-off-by: Chao Gao Co-developed-by: Zhao Liu Signed-off-by: Zhao Liu --- Changes Since v3: - Fix shift for CXRO high 32 bits. --- target/i386/cpu.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/target/i386/cpu.c b/target/i386/cpu.c index 62769db3ebb7..859cb889a37c 100644 --- a/target/i386/cpu.c +++ b/target/i386/cpu.c @@ -9711,20 +9711,23 @@ static void x86_cpu_post_initfn(Object *obj) static void x86_cpu_init_xsave(void) { static bool first = true; - uint64_t supported_xcr0; + uint64_t supported_xcr0, supported_xss; int i; if (first) { first = false; supported_xcr0 = - ((uint64_t) x86_cpu_get_supported_feature_word(NULL, FEAT_XSAVE_XCR0_HI) << 32) | + x86_cpu_get_supported_feature_word(NULL, FEAT_XSAVE_XCR0_HI) << 32 | x86_cpu_get_supported_feature_word(NULL, FEAT_XSAVE_XCR0_LO); + supported_xss = + x86_cpu_get_supported_feature_word(NULL, FEAT_XSAVE_XSS_HI) << 32 | + x86_cpu_get_supported_feature_word(NULL, FEAT_XSAVE_XSS_LO); for (i = XSTATE_SSE_BIT + 1; i < XSAVE_STATE_AREA_COUNT; i++) { ExtSaveArea *esa = &x86_ext_save_areas[i]; - if (!(supported_xcr0 & (1 << i))) { + if (!((supported_xcr0 | supported_xss) & (1 << i))) { esa->size = 0; } } -- 2.34.1 Xtile-cfg & xtile-data are both user xstates. Their xstates are cached in X86CPUState, and there's a related vmsd "vmstate_amx_xtile", so that it's safe to mark them as migratable. Arch lbr xstate is a supervisor xstate, and it is save & load by saving & loading related arch lbr MSRs, which are cached in X86CPUState, and there's a related vmsd "vmstate_arch_lbr". So it should be migratable. PT is still unmigratable since KVM disabled it and there's no vmsd and no other emulation/simulation support. Note, though the migratable_flags get fixed, x86_cpu_enable_xsave_components() still overrides supported xstates bitmaps regardless the masking of migratable_flags. This is another issue, and would be fixed in follow-up refactoring. Tested-by: Farrah Chen Signed-off-by: Zhao Liu --- Changes Since v3: - Mark XSTATE_ARCH_LBR_MASK as migratable in FEAT_XSAVE_XSS_LO. - Add TODO comment. --- target/i386/cpu.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/target/i386/cpu.c b/target/i386/cpu.c index 859cb889a37c..d2a89c03caec 100644 --- a/target/i386/cpu.c +++ b/target/i386/cpu.c @@ -1484,6 +1484,7 @@ FeatureWordInfo feature_word_info[FEATURE_WORDS] = { .ecx = 1, .reg = R_ECX, }, + .migratable_flags = XSTATE_ARCH_LBR_MASK, }, [FEAT_XSAVE_XSS_HI] = { .type = CPUID_FEATURE_WORD, @@ -1522,7 +1523,7 @@ FeatureWordInfo feature_word_info[FEATURE_WORDS] = { .migratable_flags = 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, + XSTATE_PKRU_MASK | XSTATE_XTILE_CFG_MASK | XSTATE_XTILE_DATA_MASK, }, [FEAT_XSAVE_XCR0_HI] = { .type = CPUID_FEATURE_WORD, @@ -2154,8 +2155,13 @@ static uint64_t x86_cpu_get_migratable_flags(X86CPU *cpu, FeatureWord w) for (i = 0; i < 64; i++) { uint64_t f = 1ULL << i; - /* If the feature name is known, it is implicitly considered migratable, - * unless it is explicitly set in unmigratable_flags */ + /* + * If the feature name is known, it is implicitly considered migratable, + * unless it is explicitly set in unmigratable_flags. + * + * TODO: Make the behavior of x86_cpu_enable_xsave_components() align + * with migratable_flags masking. + */ if ((wi->migratable_flags & f) || (wi->feat_names[i] && !(wi->unmigratable_flags & f))) { r |= f; -- 2.34.1 From: Yang Weijiang Add CET_U/S bits in xstate area and report support in xstate feature mask. MSR_XSS[bit 11] corresponds to CET user mode states. MSR_XSS[bit 12] corresponds to CET supervisor mode states. CET Shadow Stack(SHSTK) and Indirect Branch Tracking(IBT) features are enumerated via CPUID.(EAX=07H,ECX=0H):ECX[7] and EDX[20] respectively, two features share the same state bits in XSS, so if either of the features is enabled, set CET_U and CET_S bits together. Tested-by: Farrah Chen Reviewed-by: Xiaoyao Li Signed-off-by: Yang Weijiang Co-developed-by: Chao Gao Signed-off-by: Chao Gao Co-developed-by: Zhao Liu Signed-off-by: Zhao Liu --- Changes Since v2: - Rename XSavesCETU/XSavesCETS to XSaveCETU/XSaveCETS. - Refine the comments. - Drop ".offset = 0" and its comment. - Re-describe xstate dependencies via features array. - Drop "cet-u" & "cet-s" enumeration from FEAT_XSAVE_XSS_LO's feat_name array sicne currently xsave doesn't use named features. --- target/i386/cpu.c | 14 ++++++++++++++ target/i386/cpu.h | 26 +++++++++++++++++++++++++- 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/target/i386/cpu.c b/target/i386/cpu.c index d2a89c03caec..4d29e784061c 100644 --- a/target/i386/cpu.c +++ b/target/i386/cpu.c @@ -2078,6 +2078,20 @@ ExtSaveArea x86_ext_save_areas[XSAVE_STATE_AREA_COUNT] = { { FEAT_7_0_ECX, CPUID_7_0_ECX_PKU }, }, }, + [XSTATE_CET_U_BIT] = { + .size = sizeof(XSaveCETU), + .features = { + { FEAT_7_0_ECX, CPUID_7_0_ECX_CET_SHSTK }, + { FEAT_7_0_EDX, CPUID_7_0_EDX_CET_IBT }, + }, + }, + [XSTATE_CET_S_BIT] = { + .size = sizeof(XSaveCETS), + .features = { + { FEAT_7_0_ECX, CPUID_7_0_ECX_CET_SHSTK }, + { FEAT_7_0_EDX, CPUID_7_0_EDX_CET_IBT }, + }, + }, [XSTATE_ARCH_LBR_BIT] = { .size = sizeof(XSaveArchLBR), .features = { diff --git a/target/i386/cpu.h b/target/i386/cpu.h index f065527757c4..bfc38830e29e 100644 --- a/target/i386/cpu.h +++ b/target/i386/cpu.h @@ -589,6 +589,8 @@ typedef enum X86Seg { #define XSTATE_Hi16_ZMM_BIT 7 #define XSTATE_PT_BIT 8 #define XSTATE_PKRU_BIT 9 +#define XSTATE_CET_U_BIT 11 +#define XSTATE_CET_S_BIT 12 #define XSTATE_ARCH_LBR_BIT 15 #define XSTATE_XTILE_CFG_BIT 17 #define XSTATE_XTILE_DATA_BIT 18 @@ -603,6 +605,8 @@ typedef enum X86Seg { #define XSTATE_Hi16_ZMM_MASK (1ULL << XSTATE_Hi16_ZMM_BIT) #define XSTATE_PT_MASK (1ULL << XSTATE_PT_BIT) #define XSTATE_PKRU_MASK (1ULL << XSTATE_PKRU_BIT) +#define XSTATE_CET_U_MASK (1ULL << XSTATE_CET_U_BIT) +#define XSTATE_CET_S_MASK (1ULL << XSTATE_CET_S_BIT) #define XSTATE_ARCH_LBR_MASK (1ULL << XSTATE_ARCH_LBR_BIT) #define XSTATE_XTILE_CFG_MASK (1ULL << XSTATE_XTILE_CFG_BIT) #define XSTATE_XTILE_DATA_MASK (1ULL << XSTATE_XTILE_DATA_BIT) @@ -628,7 +632,8 @@ typedef enum X86Seg { XSTATE_XTILE_CFG_MASK | XSTATE_XTILE_DATA_MASK) /* CPUID feature bits available in XSS */ -#define CPUID_XSTATE_XSS_MASK (XSTATE_ARCH_LBR_MASK) +#define CPUID_XSTATE_XSS_MASK (XSTATE_ARCH_LBR_MASK | XSTATE_CET_U_MASK | \ + XSTATE_CET_S_MASK) #define CPUID_XSTATE_MASK (CPUID_XSTATE_XCR0_MASK | CPUID_XSTATE_XSS_MASK) @@ -907,6 +912,8 @@ uint64_t x86_cpu_get_supported_feature_word(X86CPU *cpu, FeatureWord w); #define CPUID_7_0_ECX_WAITPKG (1U << 5) /* Additional AVX-512 Vector Byte Manipulation Instruction */ #define CPUID_7_0_ECX_AVX512_VBMI2 (1U << 6) +/* Control-flow enforcement technology: shadow stack */ +#define CPUID_7_0_ECX_CET_SHSTK (1U << 7) /* Galois Field New Instructions */ #define CPUID_7_0_ECX_GFNI (1U << 8) /* Vector AES Instructions */ @@ -954,6 +961,8 @@ uint64_t x86_cpu_get_supported_feature_word(X86CPU *cpu, FeatureWord w); #define CPUID_7_0_EDX_TSX_LDTRK (1U << 16) /* Architectural LBRs */ #define CPUID_7_0_EDX_ARCH_LBR (1U << 19) +/* Control-flow enforcement technology: indirect branch tracking */ +#define CPUID_7_0_EDX_CET_IBT (1U << 20) /* AMX_BF16 instruction */ #define CPUID_7_0_EDX_AMX_BF16 (1U << 22) /* AVX512_FP16 instruction */ @@ -1740,6 +1749,19 @@ typedef struct XSavePKRU { uint32_t padding; } XSavePKRU; +/* Ext. save area 11: CET_U state */ +typedef struct XSaveCETU { + uint64_t u_cet; + uint64_t pl3_ssp; +} XSaveCETU; + +/* Ext. save area 12: CET_S state */ +typedef struct XSaveCETS { + uint64_t pl0_ssp; + uint64_t pl1_ssp; + uint64_t pl2_ssp; +} XSaveCETS; + /* Ext. save area 15: Arch LBR state */ typedef struct XSaveArchLBR { uint64_t lbr_ctl; @@ -1767,6 +1789,8 @@ QEMU_BUILD_BUG_ON(sizeof(XSaveOpmask) != 0x40); QEMU_BUILD_BUG_ON(sizeof(XSaveZMM_Hi256) != 0x200); QEMU_BUILD_BUG_ON(sizeof(XSaveHi16_ZMM) != 0x400); QEMU_BUILD_BUG_ON(sizeof(XSavePKRU) != 0x8); +QEMU_BUILD_BUG_ON(sizeof(XSaveCETU) != 0x10); +QEMU_BUILD_BUG_ON(sizeof(XSaveCETS) != 0x18); QEMU_BUILD_BUG_ON(sizeof(XSaveArchLBR) != 0x328); QEMU_BUILD_BUG_ON(sizeof(XSaveXTILECFG) != 0x40); QEMU_BUILD_BUG_ON(sizeof(XSaveXTILEDATA) != 0x2000); -- 2.34.1 CR4.CET bit (bit 23) is as master enable for CET. Check and adjust CR4.CET bit based on CET CPUIDs. Tested-by: Farrah Chen Signed-off-by: Zhao Liu --- Changes Since v3: - Reorder CR4_RESERVED_MASK. --- target/i386/cpu.h | 9 +++++++-- target/i386/helper.c | 12 ++++++++++++ 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/target/i386/cpu.h b/target/i386/cpu.h index bfc38830e29e..f4cb1dc49b71 100644 --- a/target/i386/cpu.h +++ b/target/i386/cpu.h @@ -257,6 +257,7 @@ typedef enum X86Seg { #define CR4_SMEP_MASK (1U << 20) #define CR4_SMAP_MASK (1U << 21) #define CR4_PKE_MASK (1U << 22) +#define CR4_CET_MASK (1U << 23) #define CR4_PKS_MASK (1U << 24) #define CR4_LAM_SUP_MASK (1U << 28) @@ -273,8 +274,8 @@ typedef enum X86Seg { | CR4_OSFXSR_MASK | CR4_OSXMMEXCPT_MASK | CR4_UMIP_MASK \ | CR4_LA57_MASK \ | CR4_FSGSBASE_MASK | CR4_PCIDE_MASK | CR4_OSXSAVE_MASK \ - | CR4_SMEP_MASK | CR4_SMAP_MASK | CR4_PKE_MASK | CR4_PKS_MASK \ - | CR4_LAM_SUP_MASK | CR4_FRED_MASK)) + | CR4_SMEP_MASK | CR4_SMAP_MASK | CR4_PKE_MASK | CR4_CET_MASK \ + | CR4_PKS_MASK | CR4_LAM_SUP_MASK | CR4_FRED_MASK)) #define DR6_BD (1 << 13) #define DR6_BS (1 << 14) @@ -2951,6 +2952,10 @@ static inline uint64_t cr4_reserved_bits(CPUX86State *env) if (!(env->features[FEAT_7_1_EAX] & CPUID_7_1_EAX_FRED)) { reserved_bits |= CR4_FRED_MASK; } + if (!(env->features[FEAT_7_0_ECX] & CPUID_7_0_ECX_CET_SHSTK) && + !(env->features[FEAT_7_0_EDX] & CPUID_7_0_EDX_CET_IBT)) { + reserved_bits |= CR4_CET_MASK; + } return reserved_bits; } diff --git a/target/i386/helper.c b/target/i386/helper.c index 72b2e195a31e..3f179c6c11f8 100644 --- a/target/i386/helper.c +++ b/target/i386/helper.c @@ -232,6 +232,18 @@ void cpu_x86_update_cr4(CPUX86State *env, uint32_t new_cr4) new_cr4 &= ~CR4_LAM_SUP_MASK; } + /* + * In fact, "CR4.CET can be set only if CR0.WP is set, and it must be + * clear before CR0.WP can be cleared". However, here we only check + * CR4.CET based on the supported CPUID CET bit, without checking the + * dependency on CR4.WP - the latter need to be determined by the + * underlying accelerators. + */ + if (!(env->features[FEAT_7_0_ECX] & CPUID_7_0_ECX_CET_SHSTK) && + !(env->features[FEAT_7_0_EDX] & CPUID_7_0_EDX_CET_IBT)) { + new_cr4 &= ~CR4_CET_MASK; + } + env->cr[4] = new_cr4; env->hflags = hflags; -- 2.34.1 From: "Xin Li (Intel)" Both FRED and CET shadow stack define the MSR MSR_IA32_PL0_SSP (aka MSR_IA32_FRED_SSP0 in FRED spec). MSR_IA32_PL0_SSP is a FRED SSP MSR, so that if a processor doesn't support CET shadow stack, FRED transitions won't use MSR_IA32_PL0_SSP, but this MSR would still be accessible using MSR-access instructions (e.g., RDMSR, WRMSR). Therefore, save/restore SSP0 MSR for FRED. Tested-by: Farrah Chen Signed-off-by: Xin Li (Intel) Signed-off-by: Zhao Liu --- Changes Since v3: - New commit. --- target/i386/cpu.h | 6 ++++++ target/i386/kvm/kvm.c | 13 +++++++++++++ 2 files changed, 19 insertions(+) diff --git a/target/i386/cpu.h b/target/i386/cpu.h index f4cb1dc49b71..0432af1769d2 100644 --- a/target/i386/cpu.h +++ b/target/i386/cpu.h @@ -554,6 +554,9 @@ typedef enum X86Seg { #define MSR_IA32_FRED_SSP3 0x000001d3 /* Stack level 3 shadow stack pointer in ring 0 */ #define MSR_IA32_FRED_CONFIG 0x000001d4 /* FRED Entrypoint and interrupt stack level */ +/* FRED and CET MSR */ +#define MSR_IA32_PL0_SSP 0x000006a4 /* ring-0 shadow stack pointer (aka MSR_IA32_FRED_SSP0 for FRED) */ + #define MSR_IA32_BNDCFGS 0x00000d90 #define MSR_IA32_XSS 0x00000da0 #define MSR_IA32_UMWAIT_CONTROL 0xe1 @@ -1973,6 +1976,9 @@ typedef struct CPUArchState { uint64_t fred_config; #endif + /* MSR used for both FRED and CET (SHSTK) */ + uint64_t pl0_ssp; + uint64_t tsc_adjust; uint64_t tsc_deadline; uint64_t tsc_aux; diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c index 60c798113823..00fead0827ed 100644 --- a/target/i386/kvm/kvm.c +++ b/target/i386/kvm/kvm.c @@ -4008,6 +4008,11 @@ static int kvm_put_msrs(X86CPU *cpu, KvmPutState level) kvm_msr_entry_add(cpu, MSR_IA32_FRED_SSP2, env->fred_ssp2); kvm_msr_entry_add(cpu, MSR_IA32_FRED_SSP3, env->fred_ssp3); kvm_msr_entry_add(cpu, MSR_IA32_FRED_CONFIG, env->fred_config); + /* + * Aka MSR_IA32_FRED_SSP0. This MSR is accessible even if + * CET shadow stack is not supported. + */ + kvm_msr_entry_add(cpu, MSR_IA32_PL0_SSP, env->pl0_ssp); } } #endif @@ -4495,6 +4500,11 @@ static int kvm_get_msrs(X86CPU *cpu) kvm_msr_entry_add(cpu, MSR_IA32_FRED_SSP2, 0); kvm_msr_entry_add(cpu, MSR_IA32_FRED_SSP3, 0); kvm_msr_entry_add(cpu, MSR_IA32_FRED_CONFIG, 0); + /* + * Aka MSR_IA32_FRED_SSP0. This MSR is accessible even if + * CET shadow stack is not supported. + */ + kvm_msr_entry_add(cpu, MSR_IA32_PL0_SSP, 0); } } #endif @@ -4746,6 +4756,9 @@ static int kvm_get_msrs(X86CPU *cpu) case MSR_IA32_FRED_CONFIG: env->fred_config = msrs[i].data; break; + case MSR_IA32_PL0_SSP: /* aka MSR_IA32_FRED_SSP0 */ + env->pl0_ssp = msrs[i].data; + break; #endif case MSR_IA32_TSC: env->tsc = msrs[i].data; -- 2.34.1 From: Yang Weijiang CET (architectural) MSRs include: MSR_IA32_U_CET - user mode CET control bits. MSR_IA32_S_CET - supervisor mode CET control bits. MSR_IA32_PL{0,1,2,3}_SSP - linear addresses of SSPs for user/kernel modes. MSR_IA32_INT_SSP_TAB - linear address of interrupt SSP table Since FRED also needs to save/restore MSR_IA32_PL0_SSP, to avoid duplicate operations, make FRED only save/restore MSR_IA32_PL0_SSP when CET-SHSTK is not enumerated. And considerring MSR_IA32_SSP_TBL_ADDR is only presented on 64 bit processor, wrap it with TARGET_X86_64 macro. For other MSRs, add save/restore support directly. Tested-by: Farrah Chen Suggested-by: Xin Li (Intel) Signed-off-by: Yang Weijiang Co-developed-by: Chao Gao Signed-off-by: Chao Gao Co-developed-by: Zhao Liu Signed-off-by: Zhao Liu --- Changes Since v3: - Make FRED only save/restore MSR_IA32_PL0_SSP when CET-SHSTK isn't enumerated. - Wrap MSR_IA32_INT_SSP_TAB with TARGET_X86_64 since it's a x86_64-specific MSR. Changes Since v2: - Rename MSR_IA32_SSP_TBL_ADDR to MSR_IA32_INT_SSP_TAB. - Rename X86CPUState.ssp_table_addr to X86CPUState.int_ssp_table. - Drop X86CPUStete.guest_ssp since it is not used in current commit. - Do not check CET-S & CET-U xtates when get/set MSTs since CET is XSAVE-managed feature but is not XSAVE-enabled. --- target/i386/cpu.h | 26 ++++++++++--- target/i386/kvm/kvm.c | 91 ++++++++++++++++++++++++++++++++++++------- 2 files changed, 98 insertions(+), 19 deletions(-) diff --git a/target/i386/cpu.h b/target/i386/cpu.h index 0432af1769d2..661b798da4d0 100644 --- a/target/i386/cpu.h +++ b/target/i386/cpu.h @@ -543,7 +543,7 @@ typedef enum X86Seg { #define MSR_IA32_XFD 0x000001c4 #define MSR_IA32_XFD_ERR 0x000001c5 -/* FRED MSRs */ +/* FRED MSRs (MSR_IA32_FRED_SSP0 is defined as MSR_IA32_PL0_SSP in CET MSRs) */ #define MSR_IA32_FRED_RSP0 0x000001cc /* Stack level 0 regular stack pointer */ #define MSR_IA32_FRED_RSP1 0x000001cd /* Stack level 1 regular stack pointer */ #define MSR_IA32_FRED_RSP2 0x000001ce /* Stack level 2 regular stack pointer */ @@ -554,9 +554,6 @@ typedef enum X86Seg { #define MSR_IA32_FRED_SSP3 0x000001d3 /* Stack level 3 shadow stack pointer in ring 0 */ #define MSR_IA32_FRED_CONFIG 0x000001d4 /* FRED Entrypoint and interrupt stack level */ -/* FRED and CET MSR */ -#define MSR_IA32_PL0_SSP 0x000006a4 /* ring-0 shadow stack pointer (aka MSR_IA32_FRED_SSP0 for FRED) */ - #define MSR_IA32_BNDCFGS 0x00000d90 #define MSR_IA32_XSS 0x00000da0 #define MSR_IA32_UMWAIT_CONTROL 0xe1 @@ -583,6 +580,15 @@ typedef enum X86Seg { #define MSR_APIC_START 0x00000800 #define MSR_APIC_END 0x000008ff +/* CET MSRs */ +#define MSR_IA32_U_CET 0x000006a0 /* user mode cet */ +#define MSR_IA32_S_CET 0x000006a2 /* kernel mode cet */ +#define MSR_IA32_PL0_SSP 0x000006a4 /* ring-0 shadow stack pointer */ +#define MSR_IA32_PL1_SSP 0x000006a5 /* ring-1 shadow stack pointer */ +#define MSR_IA32_PL2_SSP 0x000006a6 /* ring-2 shadow stack pointer */ +#define MSR_IA32_PL3_SSP 0x000006a7 /* ring-3 shadow stack pointer */ +#define MSR_IA32_INT_SSP_TAB 0x000006a8 /* exception shadow stack table */ + #define XSTATE_FP_BIT 0 #define XSTATE_SSE_BIT 1 #define XSTATE_YMM_BIT 2 @@ -1976,8 +1982,16 @@ typedef struct CPUArchState { uint64_t fred_config; #endif - /* MSR used for both FRED and CET (SHSTK) */ - uint64_t pl0_ssp; + /* CET MSRs */ + uint64_t u_cet; + uint64_t s_cet; + uint64_t pl0_ssp; /* also used for FRED */ + uint64_t pl1_ssp; + uint64_t pl2_ssp; + uint64_t pl3_ssp; +#ifdef TARGET_X86_64 + uint64_t int_ssp_table; +#endif uint64_t tsc_adjust; uint64_t tsc_deadline; diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c index 00fead0827ed..4eb58ca19d79 100644 --- a/target/i386/kvm/kvm.c +++ b/target/i386/kvm/kvm.c @@ -4008,11 +4008,14 @@ static int kvm_put_msrs(X86CPU *cpu, KvmPutState level) kvm_msr_entry_add(cpu, MSR_IA32_FRED_SSP2, env->fred_ssp2); kvm_msr_entry_add(cpu, MSR_IA32_FRED_SSP3, env->fred_ssp3); kvm_msr_entry_add(cpu, MSR_IA32_FRED_CONFIG, env->fred_config); - /* - * Aka MSR_IA32_FRED_SSP0. This MSR is accessible even if - * CET shadow stack is not supported. - */ - kvm_msr_entry_add(cpu, MSR_IA32_PL0_SSP, env->pl0_ssp); + + if (!(env->features[FEAT_7_0_ECX] & CPUID_7_0_ECX_CET_SHSTK)) { + /* + * Aka MSR_IA32_FRED_SSP0. This MSR is accessible even if + * CET shadow stack is not supported. + */ + kvm_msr_entry_add(cpu, MSR_IA32_PL0_SSP, env->pl0_ssp); + } } } #endif @@ -4266,6 +4269,26 @@ static int kvm_put_msrs(X86CPU *cpu, KvmPutState level) } } + if (env->features[FEAT_7_0_ECX] & CPUID_7_0_ECX_CET_SHSTK || + env->features[FEAT_7_0_EDX] & CPUID_7_0_EDX_CET_IBT) { + kvm_msr_entry_add(cpu, MSR_IA32_U_CET, env->u_cet); + kvm_msr_entry_add(cpu, MSR_IA32_S_CET, env->s_cet); + + if (env->features[FEAT_7_0_ECX] & CPUID_7_0_ECX_CET_SHSTK) { + kvm_msr_entry_add(cpu, MSR_IA32_PL0_SSP, env->pl0_ssp); + kvm_msr_entry_add(cpu, MSR_IA32_PL1_SSP, env->pl1_ssp); + kvm_msr_entry_add(cpu, MSR_IA32_PL2_SSP, env->pl2_ssp); + kvm_msr_entry_add(cpu, MSR_IA32_PL3_SSP, env->pl3_ssp); + +#ifdef TARGET_X86_64 + if (lm_capable_kernel) { + kvm_msr_entry_add(cpu, MSR_IA32_INT_SSP_TAB, + env->int_ssp_table); + } +#endif + } + } + return kvm_buf_set_msrs(cpu); } @@ -4500,11 +4523,14 @@ static int kvm_get_msrs(X86CPU *cpu) kvm_msr_entry_add(cpu, MSR_IA32_FRED_SSP2, 0); kvm_msr_entry_add(cpu, MSR_IA32_FRED_SSP3, 0); kvm_msr_entry_add(cpu, MSR_IA32_FRED_CONFIG, 0); - /* - * Aka MSR_IA32_FRED_SSP0. This MSR is accessible even if - * CET shadow stack is not supported. - */ - kvm_msr_entry_add(cpu, MSR_IA32_PL0_SSP, 0); + + if (!(env->features[FEAT_7_0_ECX] & CPUID_7_0_ECX_CET_SHSTK)) { + /* + * Aka MSR_IA32_FRED_SSP0. This MSR is accessible even if + * CET shadow stack is not supported. + */ + kvm_msr_entry_add(cpu, MSR_IA32_PL0_SSP, 0); + } } } #endif @@ -4662,6 +4688,25 @@ static int kvm_get_msrs(X86CPU *cpu) } } + if (env->features[FEAT_7_0_ECX] & CPUID_7_0_ECX_CET_SHSTK || + env->features[FEAT_7_0_EDX] & CPUID_7_0_EDX_CET_IBT) { + kvm_msr_entry_add(cpu, MSR_IA32_U_CET, 0); + kvm_msr_entry_add(cpu, MSR_IA32_S_CET, 0); + + if (env->features[FEAT_7_0_EDX] & CPUID_7_0_ECX_CET_SHSTK) { + kvm_msr_entry_add(cpu, MSR_IA32_PL0_SSP, 0); + kvm_msr_entry_add(cpu, MSR_IA32_PL1_SSP, 0); + kvm_msr_entry_add(cpu, MSR_IA32_PL2_SSP, 0); + kvm_msr_entry_add(cpu, MSR_IA32_PL3_SSP, 0); + +#ifdef TARGET_X86_64 + if (lm_capable_kernel) { + kvm_msr_entry_add(cpu, MSR_IA32_INT_SSP_TAB, 0); + } +#endif + } + } + ret = kvm_vcpu_ioctl(CPU(cpu), KVM_GET_MSRS, cpu->kvm_msr_buf); if (ret < 0) { return ret; @@ -4756,9 +4801,6 @@ static int kvm_get_msrs(X86CPU *cpu) case MSR_IA32_FRED_CONFIG: env->fred_config = msrs[i].data; break; - case MSR_IA32_PL0_SSP: /* aka MSR_IA32_FRED_SSP0 */ - env->pl0_ssp = msrs[i].data; - break; #endif case MSR_IA32_TSC: env->tsc = msrs[i].data; @@ -5012,6 +5054,29 @@ static int kvm_get_msrs(X86CPU *cpu) case MSR_ARCH_LBR_INFO_0 ... MSR_ARCH_LBR_INFO_0 + 31: env->lbr_records[index - MSR_ARCH_LBR_INFO_0].info = msrs[i].data; break; + case MSR_IA32_U_CET: + env->u_cet = msrs[i].data; + break; + case MSR_IA32_S_CET: + env->s_cet = msrs[i].data; + break; + case MSR_IA32_PL0_SSP: /* aka MSR_IA32_FRED_SSP0 */ + env->pl0_ssp = msrs[i].data; + break; + case MSR_IA32_PL1_SSP: + env->pl1_ssp = msrs[i].data; + break; + case MSR_IA32_PL2_SSP: + env->pl2_ssp = msrs[i].data; + break; + case MSR_IA32_PL3_SSP: + env->pl3_ssp = msrs[i].data; + break; +#ifdef TARGET_X86_64 + case MSR_IA32_INT_SSP_TAB: + env->int_ssp_table = msrs[i].data; + break; +#endif case MSR_K7_HWCR: env->msr_hwcr = msrs[i].data; break; -- 2.34.1 From: Yang Weijiang CET provides a new architectural register, shadow stack pointer (SSP), which cannot be directly encoded as a source, destination or memory operand in instructions. But Intel VMCS & VMCB provide fields to save/load guest & host's ssp. It's necessary to save & restore Guest's ssp before & after migration. To support this, KVM implements Guest's SSP as a special KVM internal register - KVM_REG_GUEST_SSP, and allows QEMU to save & load it via KVM_GET_ONE_REG/KVM_SET_ONE_REG. Cache KVM_REG_GUEST_SSP in X86CPUState. Tested-by: Farrah Chen Reviewed-by: Xiaoyao Li Signed-off-by: Yang Weijiang Co-developed-by: Chao Gao Signed-off-by: Chao Gao Co-developed-by: Zhao Liu Signed-off-by: Zhao Liu --- target/i386/cpu.h | 3 ++- target/i386/kvm/kvm.c | 39 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 1 deletion(-) diff --git a/target/i386/cpu.h b/target/i386/cpu.h index 661b798da4d0..c4412012c780 100644 --- a/target/i386/cpu.h +++ b/target/i386/cpu.h @@ -1982,7 +1982,7 @@ typedef struct CPUArchState { uint64_t fred_config; #endif - /* CET MSRs */ + /* CET MSRs and register */ uint64_t u_cet; uint64_t s_cet; uint64_t pl0_ssp; /* also used for FRED */ @@ -1992,6 +1992,7 @@ typedef struct CPUArchState { #ifdef TARGET_X86_64 uint64_t int_ssp_table; #endif + uint64_t guest_ssp; uint64_t tsc_adjust; uint64_t tsc_deadline; diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c index 4eb58ca19d79..1ebe20f7fb57 100644 --- a/target/i386/kvm/kvm.c +++ b/target/i386/kvm/kvm.c @@ -4292,6 +4292,35 @@ static int kvm_put_msrs(X86CPU *cpu, KvmPutState level) return kvm_buf_set_msrs(cpu); } +static int kvm_put_kvm_regs(X86CPU *cpu) +{ + CPUX86State *env = &cpu->env; + int ret; + + if ((env->features[FEAT_7_0_ECX] & CPUID_7_0_ECX_CET_SHSTK)) { + ret = kvm_set_one_reg(CPU(cpu), KVM_X86_REG_KVM(KVM_REG_GUEST_SSP), + &env->guest_ssp); + if (ret) { + return ret; + } + } + return 0; +} + +static int kvm_get_kvm_regs(X86CPU *cpu) +{ + CPUX86State *env = &cpu->env; + int ret; + + if ((env->features[FEAT_7_0_ECX] & CPUID_7_0_ECX_CET_SHSTK)) { + ret = kvm_get_one_reg(CPU(cpu), KVM_X86_REG_KVM(KVM_REG_GUEST_SSP), + &env->guest_ssp); + if (ret) { + return ret; + } + } + return 0; +} static int kvm_get_xsave(X86CPU *cpu) { @@ -5445,6 +5474,11 @@ int kvm_arch_put_registers(CPUState *cpu, KvmPutState level, Error **errp) error_setg_errno(errp, -ret, "Failed to set MSRs"); return ret; } + ret = kvm_put_kvm_regs(x86_cpu); + if (ret < 0) { + error_setg_errno(errp, -ret, "Failed to set KVM type registers"); + return ret; + } ret = kvm_put_vcpu_events(x86_cpu, level); if (ret < 0) { error_setg_errno(errp, -ret, "Failed to set vCPU events"); @@ -5517,6 +5551,11 @@ int kvm_arch_get_registers(CPUState *cs, Error **errp) error_setg_errno(errp, -ret, "Failed to get MSRs"); goto out; } + ret = kvm_get_kvm_regs(cpu); + if (ret < 0) { + error_setg_errno(errp, -ret, "Failed to get KVM type registers"); + goto out; + } ret = kvm_get_apic(cpu); if (ret < 0) { error_setg_errno(errp, -ret, "Failed to get APIC"); -- 2.34.1 From: "Xin Li (Intel)" Both FRED and CET-SHSTK need MSR_IA32_PL0_SSP, so add the vmstate for this MSR. When CET-SHSTK is not supported, MSR_IA32_PL0_SSP keeps accessible, but its value doesn't take effect. Therefore, treat this vmstate as a subsection rather than a fix for the previous FRED vmstate. Tested-by: Farrah Chen Signed-off-by: Xin Li (Intel) Co-developed-by: Zhao Liu Signed-off-by: Zhao Liu --- Changes Since v3: - New commit. --- target/i386/machine.c | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/target/i386/machine.c b/target/i386/machine.c index 45b7cea80aa7..0a756573b6cd 100644 --- a/target/i386/machine.c +++ b/target/i386/machine.c @@ -1668,6 +1668,31 @@ static const VMStateDescription vmstate_triple_fault = { } }; +static bool pl0_ssp_needed(void *opaque) +{ + X86CPU *cpu = opaque; + CPUX86State *env = &cpu->env; + +#ifdef TARGET_X86_64 + if (env->features[FEAT_7_1_EAX] & CPUID_7_1_EAX_FRED) { + return true; + } +#endif + + return !!(env->features[FEAT_7_0_ECX] & CPUID_7_0_ECX_CET_SHSTK); +} + +static const VMStateDescription vmstate_pl0_ssp = { + .name = "cpu/msr_pl0_ssp", + .version_id = 1, + .minimum_version_id = 1, + .needed = pl0_ssp_needed, + .fields = (const VMStateField[]) { + VMSTATE_UINT64(env.pl0_ssp, X86CPU), + VMSTATE_END_OF_LIST() + } +}; + const VMStateDescription vmstate_x86_cpu = { .name = "cpu", .version_id = 12, @@ -1817,6 +1842,7 @@ const VMStateDescription vmstate_x86_cpu = { #endif &vmstate_arch_lbr, &vmstate_triple_fault, + &vmstate_pl0_ssp, NULL } }; -- 2.34.1 From: Yang Weijiang Add vmstates for cet-shstk and cet-ibt Tested-by: Farrah Chen Signed-off-by: Yang Weijiang Co-developed-by: Chao Gao Signed-off-by: Chao Gao Co-developed-by: Zhao Liu Signed-off-by: Zhao Liu --- Changes Since v3: - Rename vmstate_ss to vmstate_shstk. - Split pl0_ssp into a seperate vmstate in another patch. Changes Since v2: - Split a subsection "vmstate_ss" since shstk is user-configurable. --- target/i386/machine.c | 52 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/target/i386/machine.c b/target/i386/machine.c index 0a756573b6cd..265388f1fd36 100644 --- a/target/i386/machine.c +++ b/target/i386/machine.c @@ -1693,6 +1693,57 @@ static const VMStateDescription vmstate_pl0_ssp = { } }; +static bool shstk_needed(void *opaque) +{ + X86CPU *cpu = opaque; + CPUX86State *env = &cpu->env; + + return !!(env->features[FEAT_7_0_ECX] & CPUID_7_0_ECX_CET_SHSTK); +} + +static const VMStateDescription vmstate_shstk = { + .name = "cpu/cet_shstk", + .version_id = 1, + .minimum_version_id = 1, + .needed = shstk_needed, + .fields = (VMStateField[]) { + /* pl0_ssp has been covered by vmstate_pl0_ssp. */ + VMSTATE_UINT64(env.pl1_ssp, X86CPU), + VMSTATE_UINT64(env.pl2_ssp, X86CPU), + VMSTATE_UINT64(env.pl3_ssp, X86CPU), +#ifdef TARGET_X86_64 + VMSTATE_UINT64(env.int_ssp_table, X86CPU), +#endif + VMSTATE_UINT64(env.guest_ssp, X86CPU), + VMSTATE_END_OF_LIST() + } +}; + +static bool cet_needed(void *opaque) +{ + X86CPU *cpu = opaque; + CPUX86State *env = &cpu->env; + + return !!((env->features[FEAT_7_0_ECX] & CPUID_7_0_ECX_CET_SHSTK) || + (env->features[FEAT_7_0_EDX] & CPUID_7_0_EDX_CET_IBT)); +} + +static const VMStateDescription vmstate_cet = { + .name = "cpu/cet", + .version_id = 1, + .minimum_version_id = 1, + .needed = cet_needed, + .fields = (VMStateField[]) { + VMSTATE_UINT64(env.u_cet, X86CPU), + VMSTATE_UINT64(env.s_cet, X86CPU), + VMSTATE_END_OF_LIST() + }, + .subsections = (const VMStateDescription * const []) { + &vmstate_shstk, + NULL, + }, +}; + const VMStateDescription vmstate_x86_cpu = { .name = "cpu", .version_id = 12, @@ -1843,6 +1894,7 @@ const VMStateDescription vmstate_x86_cpu = { &vmstate_arch_lbr, &vmstate_triple_fault, &vmstate_pl0_ssp, + &vmstate_cet, NULL } }; -- 2.34.1 Cet-u and cet-s are supervisor xstates. Their states are saved/loaded by saving/loading related CET MSRs. And there're the "vmstate_cet" and "vmstate_pl0_ssp" to migrate these MSRs. Thus, it's safe to mark them as migratable. Tested-by: Farrah Chen Signed-off-by: Zhao Liu --- Changes Since v3: - Add the flags in FEAT_XSAVE_XSS_LO. --- target/i386/cpu.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/target/i386/cpu.c b/target/i386/cpu.c index 4d29e784061c..848e3ccbb8e3 100644 --- a/target/i386/cpu.c +++ b/target/i386/cpu.c @@ -1484,7 +1484,8 @@ FeatureWordInfo feature_word_info[FEATURE_WORDS] = { .ecx = 1, .reg = R_ECX, }, - .migratable_flags = XSTATE_ARCH_LBR_MASK, + .migratable_flags = XSTATE_CET_U_MASK | XSTATE_CET_S_MASK | + XSTATE_ARCH_LBR_MASK, }, [FEAT_XSAVE_XSS_HI] = { .type = CPUID_FEATURE_WORD, -- 2.34.1 From: Yang Weijiang Add SHSTK and IBT flags in feature words with entry/exit control flags. CET SHSTK and IBT feature are enumerated via CPUID(EAX=7,ECX=0) ECX[bit 7] and EDX[bit 20]. CET states load/restore at vmentry/ vmexit are controlled by VMX_ENTRY_CTLS[bit 20] and VMX_EXIT_CTLS[bit 28]. Enable these flags so that KVM can enumerate the features properly. Tested-by: Farrah Chen Reviewed-by: Xiaoyao Li Signed-off-by: Yang Weijiang Co-developed-by: Chao Gao Signed-off-by: Chao Gao Co-developed-by: Zhao Liu Signed-off-by: Zhao Liu --- Changes Since v2: - Rename "shstk"/"ibt" to "cet-ss"/"cet-ibt" to match feature names in SDM & APM. - Rename "vmx-exit-save-cet-ctl"/"vmx-entry-load-cet-ctl" to "vmx-exit-save-cet"/"vmx-entry-load-cet". - Define the feature mask macro for easier double check. --- target/i386/cpu.c | 8 ++++---- target/i386/cpu.h | 2 ++ 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/target/i386/cpu.c b/target/i386/cpu.c index 848e3ccbb8e3..a65fd4111c31 100644 --- a/target/i386/cpu.c +++ b/target/i386/cpu.c @@ -1221,7 +1221,7 @@ FeatureWordInfo feature_word_info[FEATURE_WORDS] = { .type = CPUID_FEATURE_WORD, .feat_names = { NULL, "avx512vbmi", "umip", "pku", - NULL /* ospke */, "waitpkg", "avx512vbmi2", NULL, + NULL /* ospke */, "waitpkg", "avx512vbmi2", "cet-ss", "gfni", "vaes", "vpclmulqdq", "avx512vnni", "avx512bitalg", NULL, "avx512-vpopcntdq", NULL, "la57", NULL, NULL, NULL, @@ -1244,7 +1244,7 @@ FeatureWordInfo feature_word_info[FEATURE_WORDS] = { "avx512-vp2intersect", NULL, "md-clear", NULL, NULL, NULL, "serialize", NULL, "tsx-ldtrk", NULL, NULL /* pconfig */, "arch-lbr", - NULL, NULL, "amx-bf16", "avx512-fp16", + "cet-ibt", NULL, "amx-bf16", "avx512-fp16", "amx-tile", "amx-int8", "spec-ctrl", "stibp", "flush-l1d", "arch-capabilities", "core-capability", "ssbd", }, @@ -1666,7 +1666,7 @@ FeatureWordInfo feature_word_info[FEATURE_WORDS] = { "vmx-exit-save-efer", "vmx-exit-load-efer", "vmx-exit-save-preemption-timer", "vmx-exit-clear-bndcfgs", NULL, "vmx-exit-clear-rtit-ctl", NULL, NULL, - NULL, "vmx-exit-load-pkrs", NULL, "vmx-exit-secondary-ctls", + "vmx-exit-save-cet", "vmx-exit-load-pkrs", NULL, "vmx-exit-secondary-ctls", }, .msr = { .index = MSR_IA32_VMX_TRUE_EXIT_CTLS, @@ -1681,7 +1681,7 @@ FeatureWordInfo feature_word_info[FEATURE_WORDS] = { NULL, "vmx-entry-ia32e-mode", NULL, NULL, NULL, "vmx-entry-load-perf-global-ctrl", "vmx-entry-load-pat", "vmx-entry-load-efer", "vmx-entry-load-bndcfgs", NULL, "vmx-entry-load-rtit-ctl", NULL, - NULL, NULL, "vmx-entry-load-pkrs", "vmx-entry-load-fred", + "vmx-entry-load-cet", NULL, "vmx-entry-load-pkrs", "vmx-entry-load-fred", NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, }, diff --git a/target/i386/cpu.h b/target/i386/cpu.h index c4412012c780..d8bdf342f98d 100644 --- a/target/i386/cpu.h +++ b/target/i386/cpu.h @@ -1373,6 +1373,7 @@ uint64_t x86_cpu_get_supported_feature_word(X86CPU *cpu, FeatureWord w); #define VMX_VM_EXIT_CLEAR_BNDCFGS 0x00800000 #define VMX_VM_EXIT_PT_CONCEAL_PIP 0x01000000 #define VMX_VM_EXIT_CLEAR_IA32_RTIT_CTL 0x02000000 +#define VMX_VM_EXIT_SAVE_CET 0x10000000 #define VMX_VM_EXIT_LOAD_IA32_PKRS 0x20000000 #define VMX_VM_EXIT_ACTIVATE_SECONDARY_CONTROLS 0x80000000 @@ -1386,6 +1387,7 @@ uint64_t x86_cpu_get_supported_feature_word(X86CPU *cpu, FeatureWord w); #define VMX_VM_ENTRY_LOAD_BNDCFGS 0x00010000 #define VMX_VM_ENTRY_PT_CONCEAL_PIP 0x00020000 #define VMX_VM_ENTRY_LOAD_IA32_RTIT_CTL 0x00040000 +#define VMX_VM_ENTRY_LOAD_CET 0x00100000 #define VMX_VM_ENTRY_LOAD_IA32_PKRS 0x00400000 /* Supported Hyper-V Enlightenments */ -- 2.34.1 Add new versioned CPU models for Sapphire Rapids, Sierra Forest, Granite Rapids and Clearwater Forest, to enable shadow stack and indirect branch tracking. Tested-by: Farrah Chen Reviewed-by: Xiaoyao Li Signed-off-by: Zhao Liu --- target/i386/cpu.c | 44 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/target/i386/cpu.c b/target/i386/cpu.c index a65fd4111c31..84adfaf99dc8 100644 --- a/target/i386/cpu.c +++ b/target/i386/cpu.c @@ -5166,6 +5166,17 @@ static const X86CPUDefinition builtin_x86_defs[] = { { /* end of list */ }, } }, + { + .version = 5, + .note = "with cet-ss and cet-ibt", + .props = (PropValue[]) { + { "cet-ss", "on" }, + { "cet-ibt", "on" }, + { "vmx-exit-save-cet", "on" }, + { "vmx-entry-load-cet", "on" }, + { /* end of list */ }, + } + }, { /* end of list */ } } }, @@ -5328,6 +5339,17 @@ static const X86CPUDefinition builtin_x86_defs[] = { { /* end of list */ }, } }, + { + .version = 4, + .note = "with cet-ss and cet-ibt", + .props = (PropValue[]) { + { "cet-ss", "on" }, + { "cet-ibt", "on" }, + { "vmx-exit-save-cet", "on" }, + { "vmx-entry-load-cet", "on" }, + { /* end of list */ }, + } + }, { /* end of list */ }, }, }, @@ -5482,6 +5504,17 @@ static const X86CPUDefinition builtin_x86_defs[] = { { /* end of list */ }, } }, + { + .version = 4, + .note = "with cet-ss and cet-ibt", + .props = (PropValue[]) { + { "cet-ss", "on" }, + { "cet-ibt", "on" }, + { "vmx-exit-save-cet", "on" }, + { "vmx-entry-load-cet", "on" }, + { /* end of list */ }, + } + }, { /* end of list */ }, }, }, @@ -5617,6 +5650,17 @@ static const X86CPUDefinition builtin_x86_defs[] = { .model_id = "Intel Xeon Processor (ClearwaterForest)", .versions = (X86CPUVersionDefinition[]) { { .version = 1 }, + { + .version = 2, + .note = "with cet-ss and cet-ibt", + .props = (PropValue[]) { + { "cet-ss", "on" }, + { "cet-ibt", "on" }, + { "vmx-exit-save-cet", "on" }, + { "vmx-entry-load-cet", "on" }, + { /* end of list */ }, + } + }, { /* end of list */ }, }, }, -- 2.34.1 The checkpatch.pl always complains: "ERROR: space required after that close brace '}'". Fix this issue. Tested-by: Farrah Chen Reviewed-by: Xiaoyao Li Signed-off-by: Zhao Liu --- target/i386/kvm/tdx.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/target/i386/kvm/tdx.c b/target/i386/kvm/tdx.c index dbf0fa2c9180..a3444623657f 100644 --- a/target/i386/kvm/tdx.c +++ b/target/i386/kvm/tdx.c @@ -520,15 +520,15 @@ typedef struct TdxXFAMDep { * supported. */ TdxXFAMDep tdx_xfam_deps[] = { - { XSTATE_YMM_BIT, { FEAT_1_ECX, CPUID_EXT_FMA }}, - { XSTATE_YMM_BIT, { FEAT_7_0_EBX, CPUID_7_0_EBX_AVX2 }}, - { XSTATE_OPMASK_BIT, { FEAT_7_0_ECX, CPUID_7_0_ECX_AVX512_VBMI}}, - { XSTATE_OPMASK_BIT, { FEAT_7_0_EDX, CPUID_7_0_EDX_AVX512_FP16}}, - { XSTATE_PT_BIT, { FEAT_7_0_EBX, CPUID_7_0_EBX_INTEL_PT}}, - { XSTATE_PKRU_BIT, { FEAT_7_0_ECX, CPUID_7_0_ECX_PKU}}, - { XSTATE_XTILE_CFG_BIT, { FEAT_7_0_EDX, CPUID_7_0_EDX_AMX_BF16 }}, - { XSTATE_XTILE_CFG_BIT, { FEAT_7_0_EDX, CPUID_7_0_EDX_AMX_TILE }}, - { XSTATE_XTILE_CFG_BIT, { FEAT_7_0_EDX, CPUID_7_0_EDX_AMX_INT8 }}, + { XSTATE_YMM_BIT, { FEAT_1_ECX, CPUID_EXT_FMA } }, + { XSTATE_YMM_BIT, { FEAT_7_0_EBX, CPUID_7_0_EBX_AVX2 } }, + { XSTATE_OPMASK_BIT, { FEAT_7_0_ECX, CPUID_7_0_ECX_AVX512_VBMI } }, + { XSTATE_OPMASK_BIT, { FEAT_7_0_EDX, CPUID_7_0_EDX_AVX512_FP16 } }, + { XSTATE_PT_BIT, { FEAT_7_0_EBX, CPUID_7_0_EBX_INTEL_PT } }, + { XSTATE_PKRU_BIT, { FEAT_7_0_ECX, CPUID_7_0_ECX_PKU } }, + { XSTATE_XTILE_CFG_BIT, { FEAT_7_0_EDX, CPUID_7_0_EDX_AMX_BF16 } }, + { XSTATE_XTILE_CFG_BIT, { FEAT_7_0_EDX, CPUID_7_0_EDX_AMX_TILE } }, + { XSTATE_XTILE_CFG_BIT, { FEAT_7_0_EDX, CPUID_7_0_EDX_AMX_INT8 } }, }; static struct kvm_cpuid_entry2 *find_in_supported_entry(uint32_t function, -- 2.34.1 From: Chenyi Qiang So that it can be configured in TD guest. And considerring CET_U and CET_S bits are always same in supported XFAM reported by TDX module, i.e., either 00 or 11. So, only need to choose one of them. Tested-by: Farrah Chen Reviewed-by: Xiaoyao Li Signed-off-by: Chenyi Qiang Signed-off-by: Zhao Liu --- Changes Since v3: - Refine the commit message. --- target/i386/kvm/tdx.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/target/i386/kvm/tdx.c b/target/i386/kvm/tdx.c index a3444623657f..01619857685b 100644 --- a/target/i386/kvm/tdx.c +++ b/target/i386/kvm/tdx.c @@ -526,6 +526,8 @@ TdxXFAMDep tdx_xfam_deps[] = { { XSTATE_OPMASK_BIT, { FEAT_7_0_EDX, CPUID_7_0_EDX_AVX512_FP16 } }, { XSTATE_PT_BIT, { FEAT_7_0_EBX, CPUID_7_0_EBX_INTEL_PT } }, { XSTATE_PKRU_BIT, { FEAT_7_0_ECX, CPUID_7_0_ECX_PKU } }, + { XSTATE_CET_U_BIT, { FEAT_7_0_ECX, CPUID_7_0_ECX_CET_SHSTK } }, + { XSTATE_CET_U_BIT, { FEAT_7_0_EDX, CPUID_7_0_EDX_CET_IBT } }, { XSTATE_XTILE_CFG_BIT, { FEAT_7_0_EDX, CPUID_7_0_EDX_AMX_BF16 } }, { XSTATE_XTILE_CFG_BIT, { FEAT_7_0_EDX, CPUID_7_0_EDX_AMX_TILE } }, { XSTATE_XTILE_CFG_BIT, { FEAT_7_0_EDX, CPUID_7_0_EDX_AMX_INT8 } }, -- 2.34.1