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 <[email protected]> Signed-off-by: Zhao Liu <[email protected]> --- 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
