| Message ID | 1447813217-10532-2-git-send-email-huaitong.han@intel.com |
|---|---|
| State | New |
| Headers | show |
On Wed, Nov 18, 2015 at 10:20:15AM +0800, Huaitong Han wrote: [...] > @@ -408,6 +420,13 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = { > .cpuid_reg = R_EBX, > .tcg_features = TCG_7_0_EBX_FEATURES, > }, > + [FEAT_7_0_ECX] = { > + .feat_names = cpuid_7_0_ecx_feature_name, > + .cpuid_eax = 7, > + .cpuid_needs_ecx = true, .cpuid_ecx = 0, > + .cpuid_reg = R_ECX, > + .tcg_features = TCG_7_0_ECX_FEATURES, > + }, The patch looks good, but when we add the feature names to cpuid_7_0_ecx_feature_name, QEMU will consider them as migratable, but they are truly migratable only after we add the ext_save_areas entry. We can fix this by moving cpuid_7_0_ecx_feature_name to patch 2/3 (or to a separate patch, to be applied after 2/3).
On Wed, 2015-11-18 at 13:58 -0200, Eduardo Habkost wrote: > On Wed, Nov 18, 2015 at 10:20:15AM +0800, Huaitong Han wrote: > [...] > > @@ -408,6 +420,13 @@ static FeatureWordInfo > > feature_word_info[FEATURE_WORDS] = { > > .cpuid_reg = R_EBX, > > .tcg_features = TCG_7_0_EBX_FEATURES, > > }, > > + [FEAT_7_0_ECX] = { > > + .feat_names = cpuid_7_0_ecx_feature_name, > > + .cpuid_eax = 7, > > + .cpuid_needs_ecx = true, .cpuid_ecx = 0, > > + .cpuid_reg = R_ECX, > > + .tcg_features = TCG_7_0_ECX_FEATURES, > > + }, > > The patch looks good, but when we add the feature names to > cpuid_7_0_ecx_feature_name, QEMU will consider them as > migratable, but they are truly migratable only after we add the > ext_save_areas entry. > We can fix this by moving cpuid_7_0_ecx_feature_name to patch 2/3 > (or to a separate patch, to be applied after 2/3). I understand it has always been that QEMU considers the feature of cpuid_7_0_ecx_feature_name as migratable. If the feature is unmigratable, it will been added to unmigratable_flags. A series of patches do complete a full function, moving cpuid_7_0_ecx_feature_name to 2/3 patch may make 2/3 patch look better, but make 1/3 patch look somewhat incomplete. Maybe it is a solution that adding the feature to unmigratable_flags in 1/3 patch, and deleting unmigratable_flags in 2/3 patch, but I think it is pointless.
On 19/11/2015 07:36, Han, Huaitong wrote: > I understand it has always been that QEMU considers the feature of > cpuid_7_0_ecx_feature_name as migratable. If the feature is > unmigratable, it will been added to unmigratable_flags. > > A series of patches do complete a full function, moving > cpuid_7_0_ecx_feature_name to 2/3 patch may make 2/3 patch look > better, but make 1/3 patch look somewhat incomplete. > > Maybe it is a solution that adding the feature to unmigratable_flags in > 1/3 patch, and deleting unmigratable_flags in 2/3 patch, but I think it > is pointless. Or just squash everything together. After all we're talking of 4 files changed, 55 insertions(+), 1 deletion(-) It's not a large patch. Paolo
On Thu, Nov 19, 2015 at 12:10:49PM +0100, Paolo Bonzini wrote: > > > On 19/11/2015 07:36, Han, Huaitong wrote: > > I understand it has always been that QEMU considers the feature of > > cpuid_7_0_ecx_feature_name as migratable. If the feature is > > unmigratable, it will been added to unmigratable_flags. > > > > A series of patches do complete a full function, moving > > cpuid_7_0_ecx_feature_name to 2/3 patch may make 2/3 patch look > > better, but make 1/3 patch look somewhat incomplete. > > > > Maybe it is a solution that adding the feature to unmigratable_flags in > > 1/3 patch, and deleting unmigratable_flags in 2/3 patch, but I think it > > is pointless. > > Or just squash everything together. After all we're talking of > > 4 files changed, 55 insertions(+), 1 deletion(-) > > It's not a large patch. It makes sense. Adding the state to X86CPU (2/3) is useful only if we migrate it (3/3), and adding the feature names (1/3) is useful only if we can handle the new state. I will squash everything together when applying, in case there's no new version.
diff --git a/target-i386/cpu.c b/target-i386/cpu.c index 4d1b085..3c11e02 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -264,6 +264,17 @@ static const char *cpuid_7_0_ebx_feature_name[] = { NULL, NULL, "avx512pf", "avx512er", "avx512cd", NULL, NULL, NULL, }; +static const char *cpuid_7_0_ecx_feature_name[] = { + NULL, NULL, NULL, "pku", + "ospke", NULL, NULL, NULL, + NULL, NULL, NULL, NULL, + NULL, NULL, NULL, NULL, + NULL, NULL, NULL, NULL, + NULL, NULL, NULL, NULL, + NULL, NULL, NULL, NULL, + NULL, NULL, NULL, NULL, +}; + static const char *cpuid_apm_edx_feature_name[] = { NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, @@ -351,6 +362,7 @@ static const char *cpuid_6_feature_name[] = { CPUID_7_0_EBX_FSGSBASE, CPUID_7_0_EBX_HLE, CPUID_7_0_EBX_AVX2, CPUID_7_0_EBX_ERMS, CPUID_7_0_EBX_INVPCID, CPUID_7_0_EBX_RTM, CPUID_7_0_EBX_RDSEED */ +#define TCG_7_0_ECX_FEATURES 0 #define TCG_APM_FEATURES 0 #define TCG_6_EAX_FEATURES CPUID_6_EAX_ARAT @@ -408,6 +420,13 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = { .cpuid_reg = R_EBX, .tcg_features = TCG_7_0_EBX_FEATURES, }, + [FEAT_7_0_ECX] = { + .feat_names = cpuid_7_0_ecx_feature_name, + .cpuid_eax = 7, + .cpuid_needs_ecx = true, .cpuid_ecx = 0, + .cpuid_reg = R_ECX, + .tcg_features = TCG_7_0_ECX_FEATURES, + }, [FEAT_8000_0007_EDX] = { .feat_names = cpuid_apm_edx_feature_name, .cpuid_eax = 0x80000007, @@ -2401,7 +2420,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count, if (count == 0) { *eax = 0; /* Maximum ECX value for sub-leaves */ *ebx = env->features[FEAT_7_0_EBX]; /* Feature flags */ - *ecx = 0; /* Reserved */ + *ecx = env->features[FEAT_7_0_ECX]; /* Feature flags */ *edx = 0; /* Reserved */ } else { *eax = 0; diff --git a/target-i386/cpu.h b/target-i386/cpu.h index ead2832..c2e7501 100644 --- a/target-i386/cpu.h +++ b/target-i386/cpu.h @@ -408,6 +408,7 @@ typedef enum FeatureWord { FEAT_1_EDX, /* CPUID[1].EDX */ FEAT_1_ECX, /* CPUID[1].ECX */ FEAT_7_0_EBX, /* CPUID[EAX=7,ECX=0].EBX */ + FEAT_7_0_ECX, /* CPUID[EAX=7,ECX=0].ECX */ FEAT_8000_0001_EDX, /* CPUID[8000_0001].EDX */ FEAT_8000_0001_ECX, /* CPUID[8000_0001].ECX */ FEAT_8000_0007_EDX, /* CPUID[8000_0007].EDX */ @@ -576,6 +577,9 @@ typedef uint32_t FeatureWordArray[FEATURE_WORDS]; #define CPUID_7_0_EBX_AVX512ER (1U << 27) /* AVX-512 Exponential and Reciprocal */ #define CPUID_7_0_EBX_AVX512CD (1U << 28) /* AVX-512 Conflict Detection */ +#define CPUID_7_0_ECX_PKU (1U << 3) +#define CPUID_7_0_ECX_OSPKE (1U << 4) + #define CPUID_XSAVE_XSAVEOPT (1U << 0) #define CPUID_XSAVE_XSAVEC (1U << 1) #define CPUID_XSAVE_XGETBV1 (1U << 2)
This patch adds pkeys support for cpuid handling. Signed-off-by: Huaitong Han <huaitong.han@intel.com>