| Message ID | 20221208062513.2589476-7-xiaoyao.li@intel.com |
|---|---|
| State | New |
| Headers | show |
| Series | Make Intel PT configurable | expand |
On 12/8/2022 2:25 PM, Xiaoyao Li wrote: > commit e37a5c7fa459 ("i386: Add Intel Processor Trace feature support") > added the support of Intel PT by making CPUID[14] of PT as fixed feature > set (from ICX) for any CPU model on any host. This truly breaks the PT > exposure on Intel SPR platform because SPR has less supported bitmap of > CPUID(0x14,1):EBX[15:0] than ICX. > > To fix the problem, enable pass through of host's PT capabilities for > the cases "-cpu host/max" that it won't use default fixed PT feature set > of ICX but expand automatically based on get_supported_cpuid reported by > host. Meanwhile, it needs to ensure named CPU model still has the fixed > PT feature set to not break the live migration case of > "-cpu named_cpu_model,+intel-pt" > > Introduces env->use_default_intel_pt flag. > - True means it's old CPU model that uses fixed PT feature set of ICX. > - False means the named CPU model has its own PT feature set. > > Besides, to keep the same behavior for old CPU models that validate PT > feature set against default fixed PT feature set of ICX in addition to > validate from host's capabilities (via get_supported_cpuid) in > x86_cpu_filter_features(). > > In the future, new named CPU model, e.g., Sapphire Rapids, can define > its own PT feature set by setting @has_specific_intel_pt_feature_set to It seems @has_specific_intel_pt_feature_set is not introduced in this series. Then don't need to mention the specific flag name here. > true and defines it's own FEAT_14_0_EBX, FEAT_14_0_ECX, FEAT_14_1_EAX > and FEAT_14_1_EBX. > > Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com> > ---
On 12/9/2022 2:55 PM, Chenyi Qiang wrote: > > > On 12/8/2022 2:25 PM, Xiaoyao Li wrote: >> commit e37a5c7fa459 ("i386: Add Intel Processor Trace feature support") >> added the support of Intel PT by making CPUID[14] of PT as fixed feature >> set (from ICX) for any CPU model on any host. This truly breaks the PT >> exposure on Intel SPR platform because SPR has less supported bitmap of >> CPUID(0x14,1):EBX[15:0] than ICX. >> >> To fix the problem, enable pass through of host's PT capabilities for >> the cases "-cpu host/max" that it won't use default fixed PT feature set >> of ICX but expand automatically based on get_supported_cpuid reported by >> host. Meanwhile, it needs to ensure named CPU model still has the fixed >> PT feature set to not break the live migration case of >> "-cpu named_cpu_model,+intel-pt" >> >> Introduces env->use_default_intel_pt flag. >> - True means it's old CPU model that uses fixed PT feature set of ICX. >> - False means the named CPU model has its own PT feature set. >> >> Besides, to keep the same behavior for old CPU models that validate PT >> feature set against default fixed PT feature set of ICX in addition to >> validate from host's capabilities (via get_supported_cpuid) in >> x86_cpu_filter_features(). >> >> In the future, new named CPU model, e.g., Sapphire Rapids, can define >> its own PT feature set by setting @has_specific_intel_pt_feature_set to > > > It seems @has_specific_intel_pt_feature_set is not introduced in this > series. Then don't need to mention the specific flag name here. Thanks for catching it. It's leftover of previous version. I'll update the commit log for next version. >> true and defines it's own FEAT_14_0_EBX, FEAT_14_0_ECX, FEAT_14_1_EAX >> and FEAT_14_1_EBX. >> >> Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com> >> ---
On 12/8/2022 2:25 PM, Xiaoyao Li wrote: > commit e37a5c7fa459 ("i386: Add Intel Processor Trace feature support") > added the support of Intel PT by making CPUID[14] of PT as fixed feature > set (from ICX) for any CPU model on any host. This truly breaks the PT > exposure on Intel SPR platform because SPR has less supported bitmap of > CPUID(0x14,1):EBX[15:0] than ICX. > > To fix the problem, enable pass through of host's PT capabilities for > the cases "-cpu host/max" that it won't use default fixed PT feature set > of ICX but expand automatically based on get_supported_cpuid reported by > host. Meanwhile, it needs to ensure named CPU model still has the fixed > PT feature set to not break the live migration case of > "-cpu named_cpu_model,+intel-pt" > > Introduces env->use_default_intel_pt flag. > - True means it's old CPU model that uses fixed PT feature set of ICX. > - False means the named CPU model has its own PT feature set. > > Besides, to keep the same behavior for old CPU models that validate PT > feature set against default fixed PT feature set of ICX in addition to > validate from host's capabilities (via get_supported_cpuid) in > x86_cpu_filter_features(). > > In the future, new named CPU model, e.g., Sapphire Rapids, can define > its own PT feature set by setting @has_specific_intel_pt_feature_set to > true and defines it's own FEAT_14_0_EBX, FEAT_14_0_ECX, FEAT_14_1_EAX > and FEAT_14_1_EBX. > > Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com> > --- > target/i386/cpu.c | 71 ++++++++++++++++++++++++++--------------------- > target/i386/cpu.h | 1 + > 2 files changed, 40 insertions(+), 32 deletions(-) > > diff --git a/target/i386/cpu.c b/target/i386/cpu.c > index e302cbbebfc5..24f3c7b06698 100644 > --- a/target/i386/cpu.c > +++ b/target/i386/cpu.c > @@ -5194,6 +5194,21 @@ static void x86_cpu_load_model(X86CPU *cpu, X86CPUModel *model) > env->features[w] = def->features[w]; > } > > + /* > + * All (old) named CPU models have the same default values for INTEL_PT_* > + * > + * Assign the default value here since we don't want to manually copy/paste > + * it to all entries in builtin_x86_defs. > + */ > + if (!env->features[FEAT_14_0_EBX] && !env->features[FEAT_14_0_ECX] && > + !env->features[FEAT_14_1_EAX] && !env->features[FEAT_14_1_EBX]) { > + env->use_default_intel_pt = true; > + env->features[FEAT_14_0_EBX] = INTEL_PT_DEFAULT_0_EBX; > + env->features[FEAT_14_0_ECX] = INTEL_PT_DEFAULT_0_ECX; > + env->features[FEAT_14_1_EAX] = INTEL_PT_DEFAULT_1_EAX; > + env->features[FEAT_14_1_EBX] = INTEL_PT_DEFAULT_1_EBX; > + } > + > /* legacy-cache defaults to 'off' if CPU model provides cache info */ > cpu->legacy_cache = !def->cache_info; > > @@ -5716,14 +5731,11 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count, > > if (count == 0) { > *eax = INTEL_PT_MAX_SUBLEAF; > - *ebx = INTEL_PT_DEFAULT_0_EBX; > - *ecx = INTEL_PT_DEFAULT_0_ECX; > - if (env->features[FEAT_14_0_ECX] & CPUID_14_0_ECX_LIP) { > - *ecx |= CPUID_14_0_ECX_LIP; > - } > + *ebx = env->features[FEAT_14_0_EBX]; > + *ecx = env->features[FEAT_14_0_ECX]; > } else if (count == 1) { > - *eax = INTEL_PT_DEFAULT_1_EAX; > - *ebx = INTEL_PT_DEFAULT_1_EBX; > + *eax = env->features[FEAT_14_1_EAX]; > + *ebx = env->features[FEAT_14_1_EBX]; > } > break; > } > @@ -6425,6 +6437,7 @@ static void x86_cpu_filter_features(X86CPU *cpu, bool verbose) > CPUX86State *env = &cpu->env; > FeatureWord w; > const char *prefix = NULL; > + uint64_t host_feat; > > if (verbose) { > prefix = accel_uses_host_cpuid() > @@ -6433,8 +6446,7 @@ static void x86_cpu_filter_features(X86CPU *cpu, bool verbose) > } > > for (w = 0; w < FEATURE_WORDS; w++) { > - uint64_t host_feat = > - x86_cpu_get_supported_feature_word(w, false); > + host_feat = x86_cpu_get_supported_feature_word(w, false); > uint64_t requested_features = env->features[w]; > uint64_t unavailable_features; > > @@ -6458,31 +6470,26 @@ static void x86_cpu_filter_features(X86CPU *cpu, bool verbose) > mark_unavailable_features(cpu, w, unavailable_features, prefix); > } > > - if ((env->features[FEAT_7_0_EBX] & CPUID_7_0_EBX_INTEL_PT) && > - kvm_enabled()) { > - KVMState *s = CPU(cpu)->kvm_state; > - uint32_t eax_0 = kvm_arch_get_supported_cpuid(s, 0x14, 0, R_EAX); > - uint32_t ebx_0 = kvm_arch_get_supported_cpuid(s, 0x14, 0, R_EBX); > - uint32_t ecx_0 = kvm_arch_get_supported_cpuid(s, 0x14, 0, R_ECX); > - uint32_t eax_1 = kvm_arch_get_supported_cpuid(s, 0x14, 1, R_EAX); > - uint32_t ebx_1 = kvm_arch_get_supported_cpuid(s, 0x14, 1, R_EBX); > - > - if (!eax_0 || > - ((ebx_0 & INTEL_PT_DEFAULT_0_EBX) != INTEL_PT_DEFAULT_0_EBX) || > - ((ecx_0 & INTEL_PT_DEFAULT_0_ECX) != INTEL_PT_DEFAULT_0_ECX) || > - ((eax_1 & INTEL_PT_DEFAULT_MTC_BITMAP) != INTEL_PT_DEFAULT_MTC_BITMAP) || > - ((eax_1 & INTEL_PT_ADDR_RANGES_NUM_MASK) < > - INTEL_PT_DEFAULT_ADDR_RANGES_NUM) || > - ((ebx_1 & INTEL_PT_DEFAULT_1_EBX) != INTEL_PT_DEFAULT_1_EBX) || > - ((ecx_0 & CPUID_14_0_ECX_LIP) != > - (env->features[FEAT_14_0_ECX] & CPUID_14_0_ECX_LIP))) { > - /* > - * Processor Trace capabilities aren't configurable, so if the > - * host can't emulate the capabilities we report on > - * cpu_x86_cpuid(), intel-pt can't be enabled on the current host. > - */ > + if (env->features[FEAT_7_0_EBX] & CPUID_7_0_EBX_INTEL_PT) { > + /* > + * env->use_default_intel_pt is true means the CPU model doesn't have > + * INTEL_PT_* specified. In this case, we need to check it has the > + * value of default INTEL_PT to not break live migration > + */ > + if (env->use_default_intel_pt && > + ((env->features[FEAT_14_0_EBX] != INTEL_PT_DEFAULT_0_EBX) || When will the env->use_default_intel_pt be true and env->features[FEAT_14_0_EBX] != INTEL_PT_DEFAULT_0_EBX? It seems they will always be equal if env->use_default_intel_pt is true according to your code above. > + ((env->features[FEAT_14_0_ECX] & ~CPUID_14_0_ECX_LIP) != > + INTEL_PT_DEFAULT_0_ECX) || > + (env->features[FEAT_14_1_EAX] != INTEL_PT_DEFAULT_1_EAX) || > + (env->features[FEAT_14_1_EBX] != INTEL_PT_DEFAULT_1_EBX))) { > mark_unavailable_features(cpu, FEAT_7_0_EBX, CPUID_7_0_EBX_INTEL_PT, prefix); > } > + > + host_feat = x86_cpu_get_supported_feature_word(FEAT_14_0_ECX, false); > + if ((env->features[FEAT_14_0_ECX] ^ host_feat) & CPUID_14_0_ECX_LIP) { > + warn_report("Cannot configure different Intel PT IP payload format than hardware"); > + mark_unavailable_features(cpu, FEAT_7_0_EBX, CPUID_7_0_EBX_INTEL_PT, NULL); > + } > } > } > > diff --git a/target/i386/cpu.h b/target/i386/cpu.h > index 93fb5a87b40e..91a3971c1c29 100644 > --- a/target/i386/cpu.h > +++ b/target/i386/cpu.h > @@ -1784,6 +1784,7 @@ typedef struct CPUArchState { > uint32_t cpuid_vendor2; > uint32_t cpuid_vendor3; > uint32_t cpuid_version; > + bool use_default_intel_pt; > FeatureWordArray features; > /* Features that were explicitly enabled/disabled */ > FeatureWordArray user_features;
On 2/21/2023 1:14 PM, Wang, Lei wrote: > > On 12/8/2022 2:25 PM, Xiaoyao Li wrote: >> commit e37a5c7fa459 ("i386: Add Intel Processor Trace feature support") >> added the support of Intel PT by making CPUID[14] of PT as fixed feature >> set (from ICX) for any CPU model on any host. This truly breaks the PT >> exposure on Intel SPR platform because SPR has less supported bitmap of >> CPUID(0x14,1):EBX[15:0] than ICX. >> >> To fix the problem, enable pass through of host's PT capabilities for >> the cases "-cpu host/max" that it won't use default fixed PT feature set >> of ICX but expand automatically based on get_supported_cpuid reported by >> host. Meanwhile, it needs to ensure named CPU model still has the fixed >> PT feature set to not break the live migration case of >> "-cpu named_cpu_model,+intel-pt" >> >> Introduces env->use_default_intel_pt flag. >> - True means it's old CPU model that uses fixed PT feature set of ICX. >> - False means the named CPU model has its own PT feature set. >> >> Besides, to keep the same behavior for old CPU models that validate PT >> feature set against default fixed PT feature set of ICX in addition to >> validate from host's capabilities (via get_supported_cpuid) in >> x86_cpu_filter_features(). >> >> In the future, new named CPU model, e.g., Sapphire Rapids, can define >> its own PT feature set by setting @has_specific_intel_pt_feature_set to >> true and defines it's own FEAT_14_0_EBX, FEAT_14_0_ECX, FEAT_14_1_EAX >> and FEAT_14_1_EBX. >> >> Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com> >> --- >> target/i386/cpu.c | 71 ++++++++++++++++++++++++++--------------------- >> target/i386/cpu.h | 1 + >> 2 files changed, 40 insertions(+), 32 deletions(-) >> >> diff --git a/target/i386/cpu.c b/target/i386/cpu.c >> index e302cbbebfc5..24f3c7b06698 100644 >> --- a/target/i386/cpu.c >> +++ b/target/i386/cpu.c >> @@ -5194,6 +5194,21 @@ static void x86_cpu_load_model(X86CPU *cpu, X86CPUModel *model) >> env->features[w] = def->features[w]; >> } >> >> + /* >> + * All (old) named CPU models have the same default values for INTEL_PT_* >> + * >> + * Assign the default value here since we don't want to manually copy/paste >> + * it to all entries in builtin_x86_defs. >> + */ >> + if (!env->features[FEAT_14_0_EBX] && !env->features[FEAT_14_0_ECX] && >> + !env->features[FEAT_14_1_EAX] && !env->features[FEAT_14_1_EBX]) { >> + env->use_default_intel_pt = true; >> + env->features[FEAT_14_0_EBX] = INTEL_PT_DEFAULT_0_EBX; >> + env->features[FEAT_14_0_ECX] = INTEL_PT_DEFAULT_0_ECX; >> + env->features[FEAT_14_1_EAX] = INTEL_PT_DEFAULT_1_EAX; >> + env->features[FEAT_14_1_EBX] = INTEL_PT_DEFAULT_1_EBX; >> + } >> + >> /* legacy-cache defaults to 'off' if CPU model provides cache info */ >> cpu->legacy_cache = !def->cache_info; >> >> @@ -5716,14 +5731,11 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count, >> >> if (count == 0) { >> *eax = INTEL_PT_MAX_SUBLEAF; >> - *ebx = INTEL_PT_DEFAULT_0_EBX; >> - *ecx = INTEL_PT_DEFAULT_0_ECX; >> - if (env->features[FEAT_14_0_ECX] & CPUID_14_0_ECX_LIP) { >> - *ecx |= CPUID_14_0_ECX_LIP; >> - } >> + *ebx = env->features[FEAT_14_0_EBX]; >> + *ecx = env->features[FEAT_14_0_ECX]; >> } else if (count == 1) { >> - *eax = INTEL_PT_DEFAULT_1_EAX; >> - *ebx = INTEL_PT_DEFAULT_1_EBX; >> + *eax = env->features[FEAT_14_1_EAX]; >> + *ebx = env->features[FEAT_14_1_EBX]; >> } >> break; >> } >> @@ -6425,6 +6437,7 @@ static void x86_cpu_filter_features(X86CPU *cpu, bool verbose) >> CPUX86State *env = &cpu->env; >> FeatureWord w; >> const char *prefix = NULL; >> + uint64_t host_feat; >> >> if (verbose) { >> prefix = accel_uses_host_cpuid() >> @@ -6433,8 +6446,7 @@ static void x86_cpu_filter_features(X86CPU *cpu, bool verbose) >> } >> >> for (w = 0; w < FEATURE_WORDS; w++) { >> - uint64_t host_feat = >> - x86_cpu_get_supported_feature_word(w, false); >> + host_feat = x86_cpu_get_supported_feature_word(w, false); >> uint64_t requested_features = env->features[w]; >> uint64_t unavailable_features; >> >> @@ -6458,31 +6470,26 @@ static void x86_cpu_filter_features(X86CPU *cpu, bool verbose) >> mark_unavailable_features(cpu, w, unavailable_features, prefix); >> } >> >> - if ((env->features[FEAT_7_0_EBX] & CPUID_7_0_EBX_INTEL_PT) && >> - kvm_enabled()) { >> - KVMState *s = CPU(cpu)->kvm_state; >> - uint32_t eax_0 = kvm_arch_get_supported_cpuid(s, 0x14, 0, R_EAX); >> - uint32_t ebx_0 = kvm_arch_get_supported_cpuid(s, 0x14, 0, R_EBX); >> - uint32_t ecx_0 = kvm_arch_get_supported_cpuid(s, 0x14, 0, R_ECX); >> - uint32_t eax_1 = kvm_arch_get_supported_cpuid(s, 0x14, 1, R_EAX); >> - uint32_t ebx_1 = kvm_arch_get_supported_cpuid(s, 0x14, 1, R_EBX); >> - >> - if (!eax_0 || >> - ((ebx_0 & INTEL_PT_DEFAULT_0_EBX) != INTEL_PT_DEFAULT_0_EBX) || >> - ((ecx_0 & INTEL_PT_DEFAULT_0_ECX) != INTEL_PT_DEFAULT_0_ECX) || >> - ((eax_1 & INTEL_PT_DEFAULT_MTC_BITMAP) != INTEL_PT_DEFAULT_MTC_BITMAP) || >> - ((eax_1 & INTEL_PT_ADDR_RANGES_NUM_MASK) < >> - INTEL_PT_DEFAULT_ADDR_RANGES_NUM) || >> - ((ebx_1 & INTEL_PT_DEFAULT_1_EBX) != INTEL_PT_DEFAULT_1_EBX) || >> - ((ecx_0 & CPUID_14_0_ECX_LIP) != >> - (env->features[FEAT_14_0_ECX] & CPUID_14_0_ECX_LIP))) { >> - /* >> - * Processor Trace capabilities aren't configurable, so if the >> - * host can't emulate the capabilities we report on >> - * cpu_x86_cpuid(), intel-pt can't be enabled on the current host. >> - */ >> + if (env->features[FEAT_7_0_EBX] & CPUID_7_0_EBX_INTEL_PT) { >> + /* >> + * env->use_default_intel_pt is true means the CPU model doesn't have >> + * INTEL_PT_* specified. In this case, we need to check it has the >> + * value of default INTEL_PT to not break live migration >> + */ >> + if (env->use_default_intel_pt && >> + ((env->features[FEAT_14_0_EBX] != INTEL_PT_DEFAULT_0_EBX) || > > When will the env->use_default_intel_pt be true and env->features[FEAT_14_0_EBX] > != INTEL_PT_DEFAULT_0_EBX? It seems they will always be equal if > env->use_default_intel_pt is true according to your code above. When +/-feature are used to configure them. However, after thinking I realize this can be dropped. The original purpose of this handling is to validate what KVM reports satisfying what QEMU configures. Now the validation is performed in x86_cpu_filter_features() The purpose for not breaking live migration, targets specifically for the case where migrating from the old QEMU (without this patch) to new QEMU. However, old qemu has no ability to +/- feature bit of leaf 0x14. Thus no need to keep this code. I will remove them in next version. >> + ((env->features[FEAT_14_0_ECX] & ~CPUID_14_0_ECX_LIP) != >> + INTEL_PT_DEFAULT_0_ECX) || >> + (env->features[FEAT_14_1_EAX] != INTEL_PT_DEFAULT_1_EAX) || >> + (env->features[FEAT_14_1_EBX] != INTEL_PT_DEFAULT_1_EBX))) { >> mark_unavailable_features(cpu, FEAT_7_0_EBX, CPUID_7_0_EBX_INTEL_PT, prefix); >> } >> + >> + host_feat = x86_cpu_get_supported_feature_word(FEAT_14_0_ECX, false); >> + if ((env->features[FEAT_14_0_ECX] ^ host_feat) & CPUID_14_0_ECX_LIP) { >> + warn_report("Cannot configure different Intel PT IP payload format than hardware"); >> + mark_unavailable_features(cpu, FEAT_7_0_EBX, CPUID_7_0_EBX_INTEL_PT, NULL); >> + } >> } >> } >> >> diff --git a/target/i386/cpu.h b/target/i386/cpu.h >> index 93fb5a87b40e..91a3971c1c29 100644 >> --- a/target/i386/cpu.h >> +++ b/target/i386/cpu.h >> @@ -1784,6 +1784,7 @@ typedef struct CPUArchState { >> uint32_t cpuid_vendor2; >> uint32_t cpuid_vendor3; >> uint32_t cpuid_version; >> + bool use_default_intel_pt; >> FeatureWordArray features; >> /* Features that were explicitly enabled/disabled */ >> FeatureWordArray user_features;
diff --git a/target/i386/cpu.c b/target/i386/cpu.c index e302cbbebfc5..24f3c7b06698 100644 --- a/target/i386/cpu.c +++ b/target/i386/cpu.c @@ -5194,6 +5194,21 @@ static void x86_cpu_load_model(X86CPU *cpu, X86CPUModel *model) env->features[w] = def->features[w]; } + /* + * All (old) named CPU models have the same default values for INTEL_PT_* + * + * Assign the default value here since we don't want to manually copy/paste + * it to all entries in builtin_x86_defs. + */ + if (!env->features[FEAT_14_0_EBX] && !env->features[FEAT_14_0_ECX] && + !env->features[FEAT_14_1_EAX] && !env->features[FEAT_14_1_EBX]) { + env->use_default_intel_pt = true; + env->features[FEAT_14_0_EBX] = INTEL_PT_DEFAULT_0_EBX; + env->features[FEAT_14_0_ECX] = INTEL_PT_DEFAULT_0_ECX; + env->features[FEAT_14_1_EAX] = INTEL_PT_DEFAULT_1_EAX; + env->features[FEAT_14_1_EBX] = INTEL_PT_DEFAULT_1_EBX; + } + /* legacy-cache defaults to 'off' if CPU model provides cache info */ cpu->legacy_cache = !def->cache_info; @@ -5716,14 +5731,11 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count, if (count == 0) { *eax = INTEL_PT_MAX_SUBLEAF; - *ebx = INTEL_PT_DEFAULT_0_EBX; - *ecx = INTEL_PT_DEFAULT_0_ECX; - if (env->features[FEAT_14_0_ECX] & CPUID_14_0_ECX_LIP) { - *ecx |= CPUID_14_0_ECX_LIP; - } + *ebx = env->features[FEAT_14_0_EBX]; + *ecx = env->features[FEAT_14_0_ECX]; } else if (count == 1) { - *eax = INTEL_PT_DEFAULT_1_EAX; - *ebx = INTEL_PT_DEFAULT_1_EBX; + *eax = env->features[FEAT_14_1_EAX]; + *ebx = env->features[FEAT_14_1_EBX]; } break; } @@ -6425,6 +6437,7 @@ static void x86_cpu_filter_features(X86CPU *cpu, bool verbose) CPUX86State *env = &cpu->env; FeatureWord w; const char *prefix = NULL; + uint64_t host_feat; if (verbose) { prefix = accel_uses_host_cpuid() @@ -6433,8 +6446,7 @@ static void x86_cpu_filter_features(X86CPU *cpu, bool verbose) } for (w = 0; w < FEATURE_WORDS; w++) { - uint64_t host_feat = - x86_cpu_get_supported_feature_word(w, false); + host_feat = x86_cpu_get_supported_feature_word(w, false); uint64_t requested_features = env->features[w]; uint64_t unavailable_features; @@ -6458,31 +6470,26 @@ static void x86_cpu_filter_features(X86CPU *cpu, bool verbose) mark_unavailable_features(cpu, w, unavailable_features, prefix); } - if ((env->features[FEAT_7_0_EBX] & CPUID_7_0_EBX_INTEL_PT) && - kvm_enabled()) { - KVMState *s = CPU(cpu)->kvm_state; - uint32_t eax_0 = kvm_arch_get_supported_cpuid(s, 0x14, 0, R_EAX); - uint32_t ebx_0 = kvm_arch_get_supported_cpuid(s, 0x14, 0, R_EBX); - uint32_t ecx_0 = kvm_arch_get_supported_cpuid(s, 0x14, 0, R_ECX); - uint32_t eax_1 = kvm_arch_get_supported_cpuid(s, 0x14, 1, R_EAX); - uint32_t ebx_1 = kvm_arch_get_supported_cpuid(s, 0x14, 1, R_EBX); - - if (!eax_0 || - ((ebx_0 & INTEL_PT_DEFAULT_0_EBX) != INTEL_PT_DEFAULT_0_EBX) || - ((ecx_0 & INTEL_PT_DEFAULT_0_ECX) != INTEL_PT_DEFAULT_0_ECX) || - ((eax_1 & INTEL_PT_DEFAULT_MTC_BITMAP) != INTEL_PT_DEFAULT_MTC_BITMAP) || - ((eax_1 & INTEL_PT_ADDR_RANGES_NUM_MASK) < - INTEL_PT_DEFAULT_ADDR_RANGES_NUM) || - ((ebx_1 & INTEL_PT_DEFAULT_1_EBX) != INTEL_PT_DEFAULT_1_EBX) || - ((ecx_0 & CPUID_14_0_ECX_LIP) != - (env->features[FEAT_14_0_ECX] & CPUID_14_0_ECX_LIP))) { - /* - * Processor Trace capabilities aren't configurable, so if the - * host can't emulate the capabilities we report on - * cpu_x86_cpuid(), intel-pt can't be enabled on the current host. - */ + if (env->features[FEAT_7_0_EBX] & CPUID_7_0_EBX_INTEL_PT) { + /* + * env->use_default_intel_pt is true means the CPU model doesn't have + * INTEL_PT_* specified. In this case, we need to check it has the + * value of default INTEL_PT to not break live migration + */ + if (env->use_default_intel_pt && + ((env->features[FEAT_14_0_EBX] != INTEL_PT_DEFAULT_0_EBX) || + ((env->features[FEAT_14_0_ECX] & ~CPUID_14_0_ECX_LIP) != + INTEL_PT_DEFAULT_0_ECX) || + (env->features[FEAT_14_1_EAX] != INTEL_PT_DEFAULT_1_EAX) || + (env->features[FEAT_14_1_EBX] != INTEL_PT_DEFAULT_1_EBX))) { mark_unavailable_features(cpu, FEAT_7_0_EBX, CPUID_7_0_EBX_INTEL_PT, prefix); } + + host_feat = x86_cpu_get_supported_feature_word(FEAT_14_0_ECX, false); + if ((env->features[FEAT_14_0_ECX] ^ host_feat) & CPUID_14_0_ECX_LIP) { + warn_report("Cannot configure different Intel PT IP payload format than hardware"); + mark_unavailable_features(cpu, FEAT_7_0_EBX, CPUID_7_0_EBX_INTEL_PT, NULL); + } } } diff --git a/target/i386/cpu.h b/target/i386/cpu.h index 93fb5a87b40e..91a3971c1c29 100644 --- a/target/i386/cpu.h +++ b/target/i386/cpu.h @@ -1784,6 +1784,7 @@ typedef struct CPUArchState { uint32_t cpuid_vendor2; uint32_t cpuid_vendor3; uint32_t cpuid_version; + bool use_default_intel_pt; FeatureWordArray features; /* Features that were explicitly enabled/disabled */ FeatureWordArray user_features;
commit e37a5c7fa459 ("i386: Add Intel Processor Trace feature support") added the support of Intel PT by making CPUID[14] of PT as fixed feature set (from ICX) for any CPU model on any host. This truly breaks the PT exposure on Intel SPR platform because SPR has less supported bitmap of CPUID(0x14,1):EBX[15:0] than ICX. To fix the problem, enable pass through of host's PT capabilities for the cases "-cpu host/max" that it won't use default fixed PT feature set of ICX but expand automatically based on get_supported_cpuid reported by host. Meanwhile, it needs to ensure named CPU model still has the fixed PT feature set to not break the live migration case of "-cpu named_cpu_model,+intel-pt" Introduces env->use_default_intel_pt flag. - True means it's old CPU model that uses fixed PT feature set of ICX. - False means the named CPU model has its own PT feature set. Besides, to keep the same behavior for old CPU models that validate PT feature set against default fixed PT feature set of ICX in addition to validate from host's capabilities (via get_supported_cpuid) in x86_cpu_filter_features(). In the future, new named CPU model, e.g., Sapphire Rapids, can define its own PT feature set by setting @has_specific_intel_pt_feature_set to true and defines it's own FEAT_14_0_EBX, FEAT_14_0_ECX, FEAT_14_1_EAX and FEAT_14_1_EBX. Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com> --- target/i386/cpu.c | 71 ++++++++++++++++++++++++++--------------------- target/i386/cpu.h | 1 + 2 files changed, 40 insertions(+), 32 deletions(-)