* [PATCH 1/2] i386/cpu: Add the Intel PT capabilities checking before extend the CPUID level @ 2020-10-14 8:04 Luwei Kang 2020-10-14 8:04 ` [PATCH 2/2] i386/cpu: Make the Intel PT LIP feature configurable Luwei Kang 2020-12-01 21:12 ` [PATCH 1/2] i386/cpu: Add the Intel PT capabilities checking before extend the CPUID level Eduardo Habkost 0 siblings, 2 replies; 7+ messages in thread From: Luwei Kang @ 2020-10-14 8:04 UTC (permalink / raw) To: pbonzini, rth, ehabkost; +Cc: Luwei Kang, qemu-devel The current implementation will extend the CPUID level to 0x14 if Intel PT is enabled in the guest(in x86_cpu_expand_features()) and the Intel PT will be disabled if it can't pass the capabilities checking later(in x86_cpu_filter_features()). In this case, the level of CPUID will be still 0x14 and the CPUID values from leaf 0xe to 0x14 are all zero. This patch moves the capabilities checking before setting the level of the CPUID. Signed-off-by: Luwei Kang <luwei.kang@intel.com> --- target/i386/cpu.c | 63 ++++++++++++++++++++++++----------------------- 1 file changed, 32 insertions(+), 31 deletions(-) diff --git a/target/i386/cpu.c b/target/i386/cpu.c index 9eafbe3690..24644abfd4 100644 --- a/target/i386/cpu.c +++ b/target/i386/cpu.c @@ -6401,12 +6401,40 @@ static void x86_cpu_expand_features(X86CPU *cpu, Error **errp) /* Intel Processor Trace requires CPUID[0x14] */ if ((env->features[FEAT_7_0_EBX] & CPUID_7_0_EBX_INTEL_PT)) { - if (cpu->intel_pt_auto_level) { - x86_cpu_adjust_level(cpu, &cpu->env.cpuid_min_level, 0x14); - } else if (cpu->env.cpuid_min_level < 0x14) { + uint32_t eax_0, ebx_0, ecx_0, eax_1, ebx_1; + + eax_0 = kvm_arch_get_supported_cpuid(kvm_state, 0x14, 0, R_EAX); + ebx_0 = kvm_arch_get_supported_cpuid(kvm_state, 0x14, 0, R_EBX); + ecx_0 = kvm_arch_get_supported_cpuid(kvm_state, 0x14, 0, R_ECX); + eax_1 = kvm_arch_get_supported_cpuid(kvm_state, 0x14, 1, R_EAX); + ebx_1 = kvm_arch_get_supported_cpuid(kvm_state, 0x14, 1, R_EBX); + + if (eax_0 && + ((ebx_0 & INTEL_PT_MINIMAL_EBX) == INTEL_PT_MINIMAL_EBX) && + ((ecx_0 & INTEL_PT_MINIMAL_ECX) == INTEL_PT_MINIMAL_ECX) && + ((eax_1 & INTEL_PT_MTC_BITMAP) == INTEL_PT_MTC_BITMAP) && + ((eax_1 & INTEL_PT_ADDR_RANGES_NUM_MASK) >= + INTEL_PT_ADDR_RANGES_NUM) && + ((ebx_1 & (INTEL_PT_PSB_BITMAP | INTEL_PT_CYCLE_BITMAP)) == + (INTEL_PT_PSB_BITMAP | INTEL_PT_CYCLE_BITMAP)) && + !(ecx_0 & INTEL_PT_IP_LIP)) { + if (cpu->intel_pt_auto_level) { + x86_cpu_adjust_level(cpu, &cpu->env.cpuid_min_level, 0x14); + } else if (cpu->env.cpuid_min_level < 0x14) { + mark_unavailable_features(cpu, FEAT_7_0_EBX, + CPUID_7_0_EBX_INTEL_PT, + "Intel PT need CPUID leaf 0x14, please set by \"-cpu ...,+intel-pt,min-level=0x14\""); + } + } else { + /* + * 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. + */ mark_unavailable_features(cpu, FEAT_7_0_EBX, CPUID_7_0_EBX_INTEL_PT, - "Intel PT need CPUID leaf 0x14, please set by \"-cpu ...,+intel-pt,min-level=0x14\""); + "host Intel PT features doesn't satisfy the guest request."); } } @@ -6466,33 +6494,6 @@ static void x86_cpu_filter_features(X86CPU *cpu, bool verbose) uint64_t unavailable_features = requested_features & ~host_feat; 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_MINIMAL_EBX) != INTEL_PT_MINIMAL_EBX) || - ((ecx_0 & INTEL_PT_MINIMAL_ECX) != INTEL_PT_MINIMAL_ECX) || - ((eax_1 & INTEL_PT_MTC_BITMAP) != INTEL_PT_MTC_BITMAP) || - ((eax_1 & INTEL_PT_ADDR_RANGES_NUM_MASK) < - INTEL_PT_ADDR_RANGES_NUM) || - ((ebx_1 & (INTEL_PT_PSB_BITMAP | INTEL_PT_CYCLE_BITMAP)) != - (INTEL_PT_PSB_BITMAP | INTEL_PT_CYCLE_BITMAP)) || - (ecx_0 & INTEL_PT_IP_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. - */ - mark_unavailable_features(cpu, FEAT_7_0_EBX, CPUID_7_0_EBX_INTEL_PT, prefix); - } - } } static void x86_cpu_realizefn(DeviceState *dev, Error **errp) -- 2.18.4 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/2] i386/cpu: Make the Intel PT LIP feature configurable 2020-10-14 8:04 [PATCH 1/2] i386/cpu: Add the Intel PT capabilities checking before extend the CPUID level Luwei Kang @ 2020-10-14 8:04 ` Luwei Kang 2020-11-11 1:47 ` Kang, Luwei 2020-12-01 21:19 ` Eduardo Habkost 2020-12-01 21:12 ` [PATCH 1/2] i386/cpu: Add the Intel PT capabilities checking before extend the CPUID level Eduardo Habkost 1 sibling, 2 replies; 7+ messages in thread From: Luwei Kang @ 2020-10-14 8:04 UTC (permalink / raw) To: pbonzini, rth, ehabkost; +Cc: Luwei Kang, qemu-devel The current implementation will disable the guest Intel PT feature if the Intel PT LIP feature is supported on the host, but the LIP feature is comming soon(e.g. SnowRidge and later). This patch will make the guest LIP feature configurable and Intel PT feature can be enabled in guest when the guest LIP status same with the host. Signed-off-by: Luwei Kang <luwei.kang@intel.com> --- target/i386/cpu.c | 29 +++++++++++++++++++++++++++-- target/i386/cpu.h | 4 ++++ 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/target/i386/cpu.c b/target/i386/cpu.c index 24644abfd4..aeabdd5bd4 100644 --- a/target/i386/cpu.c +++ b/target/i386/cpu.c @@ -672,6 +672,7 @@ static void x86_cpu_vendor_words2str(char *dst, uint32_t vendor1, #define TCG_XSAVE_FEATURES (CPUID_XSAVE_XSAVEOPT | CPUID_XSAVE_XGETBV1) /* missing: CPUID_XSAVE_XSAVEC, CPUID_XSAVE_XSAVES */ +#define TCG_14_0_ECX_FEATURES 0 typedef enum FeatureWordType { CPUID_FEATURE_WORD, @@ -1301,6 +1302,26 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = { } }, + [FEAT_14_0_ECX] = { + .type = CPUID_FEATURE_WORD, + .feat_names = { + 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, NULL, + NULL, NULL, NULL, "intel-pt-lip", + }, + .cpuid = { + .eax = 0x14, + .needs_ecx = true, .ecx = 0, + .reg = R_ECX, + }, + .tcg_features = TCG_14_0_ECX_FEATURES, + }, + }; typedef struct FeatureMask { @@ -5743,6 +5764,9 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count, *eax = INTEL_PT_MAX_SUBLEAF; *ebx = INTEL_PT_MINIMAL_EBX; *ecx = INTEL_PT_MINIMAL_ECX; + if (env->features[FEAT_14_0_ECX] & CPUID_14_0_ECX_LIP) { + *ecx |= CPUID_14_0_ECX_LIP; + } } else if (count == 1) { *eax = INTEL_PT_MTC_BITMAP | INTEL_PT_ADDR_RANGES_NUM; *ebx = INTEL_PT_PSB_BITMAP | INTEL_PT_CYCLE_BITMAP; @@ -6416,8 +6440,9 @@ static void x86_cpu_expand_features(X86CPU *cpu, Error **errp) ((eax_1 & INTEL_PT_ADDR_RANGES_NUM_MASK) >= INTEL_PT_ADDR_RANGES_NUM) && ((ebx_1 & (INTEL_PT_PSB_BITMAP | INTEL_PT_CYCLE_BITMAP)) == - (INTEL_PT_PSB_BITMAP | INTEL_PT_CYCLE_BITMAP)) && - !(ecx_0 & INTEL_PT_IP_LIP)) { + (INTEL_PT_PSB_BITMAP | INTEL_PT_CYCLE_BITMAP)) && + ((ecx_0 & CPUID_14_0_ECX_LIP) == + (env->features[FEAT_14_0_ECX] & CPUID_14_0_ECX_LIP))) { if (cpu->intel_pt_auto_level) { x86_cpu_adjust_level(cpu, &cpu->env.cpuid_min_level, 0x14); } else if (cpu->env.cpuid_min_level < 0x14) { diff --git a/target/i386/cpu.h b/target/i386/cpu.h index 51c1d5f60a..1fcd93e39a 100644 --- a/target/i386/cpu.h +++ b/target/i386/cpu.h @@ -541,6 +541,7 @@ typedef enum FeatureWord { FEAT_VMX_EPT_VPID_CAPS, FEAT_VMX_BASIC, FEAT_VMX_VMFUNC, + FEAT_14_0_ECX, FEATURE_WORDS, } FeatureWord; @@ -797,6 +798,9 @@ typedef uint64_t FeatureWordArray[FEATURE_WORDS]; /* AVX512 BFloat16 Instruction */ #define CPUID_7_1_EAX_AVX512_BF16 (1U << 5) +/* Packets which contain IP payload have LIP values */ +#define CPUID_14_0_ECX_LIP (1U << 31) + /* CLZERO instruction */ #define CPUID_8000_0008_EBX_CLZERO (1U << 0) /* Always save/restore FP error pointers */ -- 2.18.4 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* RE: [PATCH 2/2] i386/cpu: Make the Intel PT LIP feature configurable 2020-10-14 8:04 ` [PATCH 2/2] i386/cpu: Make the Intel PT LIP feature configurable Luwei Kang @ 2020-11-11 1:47 ` Kang, Luwei 2020-12-01 21:19 ` Eduardo Habkost 1 sibling, 0 replies; 7+ messages in thread From: Kang, Luwei @ 2020-11-11 1:47 UTC (permalink / raw) To: pbonzini, rth, ehabkost; +Cc: qemu-devel > -----Original Message----- > From: Kang, Luwei <luwei.kang@intel.com> > Sent: Wednesday, October 14, 2020 4:05 PM > To: pbonzini@redhat.com; rth@twiddle.net; ehabkost@redhat.com > Cc: qemu-devel@nongnu.org; Kang, Luwei <luwei.kang@intel.com> > Subject: [PATCH 2/2] i386/cpu: Make the Intel PT LIP feature configurable > > The current implementation will disable the guest Intel PT feature if the Intel > PT LIP feature is supported on the host, but the LIP feature is comming > soon(e.g. SnowRidge and later). > > This patch will make the guest LIP feature configurable and Intel PT feature can > be enabled in guest when the guest LIP status same with the host. Ping. 😊 Thanks, Luwei Kang > > Signed-off-by: Luwei Kang <luwei.kang@intel.com> > --- > target/i386/cpu.c | 29 +++++++++++++++++++++++++++-- target/i386/cpu.h > | 4 ++++ > 2 files changed, 31 insertions(+), 2 deletions(-) > > diff --git a/target/i386/cpu.c b/target/i386/cpu.c index > 24644abfd4..aeabdd5bd4 100644 > --- a/target/i386/cpu.c > +++ b/target/i386/cpu.c > @@ -672,6 +672,7 @@ static void x86_cpu_vendor_words2str(char *dst, > uint32_t vendor1, #define TCG_XSAVE_FEATURES (CPUID_XSAVE_XSAVEOPT > | CPUID_XSAVE_XGETBV1) > /* missing: > CPUID_XSAVE_XSAVEC, CPUID_XSAVE_XSAVES */ > +#define TCG_14_0_ECX_FEATURES 0 > > typedef enum FeatureWordType { > CPUID_FEATURE_WORD, > @@ -1301,6 +1302,26 @@ static FeatureWordInfo > feature_word_info[FEATURE_WORDS] = { > } > }, > > + [FEAT_14_0_ECX] = { > + .type = CPUID_FEATURE_WORD, > + .feat_names = { > + 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, NULL, > + NULL, NULL, NULL, "intel-pt-lip", > + }, > + .cpuid = { > + .eax = 0x14, > + .needs_ecx = true, .ecx = 0, > + .reg = R_ECX, > + }, > + .tcg_features = TCG_14_0_ECX_FEATURES, > + }, > + > }; > > typedef struct FeatureMask { > @@ -5743,6 +5764,9 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t > index, uint32_t count, > *eax = INTEL_PT_MAX_SUBLEAF; > *ebx = INTEL_PT_MINIMAL_EBX; > *ecx = INTEL_PT_MINIMAL_ECX; > + if (env->features[FEAT_14_0_ECX] & CPUID_14_0_ECX_LIP) { > + *ecx |= CPUID_14_0_ECX_LIP; > + } > } else if (count == 1) { > *eax = INTEL_PT_MTC_BITMAP | INTEL_PT_ADDR_RANGES_NUM; > *ebx = INTEL_PT_PSB_BITMAP | INTEL_PT_CYCLE_BITMAP; @@ -6416,8 > +6440,9 @@ static void x86_cpu_expand_features(X86CPU *cpu, Error **errp) > ((eax_1 & INTEL_PT_ADDR_RANGES_NUM_MASK) >= > INTEL_PT_ADDR_RANGES_NUM) && > ((ebx_1 & (INTEL_PT_PSB_BITMAP | INTEL_PT_CYCLE_BITMAP)) == > - (INTEL_PT_PSB_BITMAP | INTEL_PT_CYCLE_BITMAP)) && > - !(ecx_0 & INTEL_PT_IP_LIP)) { > + (INTEL_PT_PSB_BITMAP | INTEL_PT_CYCLE_BITMAP)) && > + ((ecx_0 & CPUID_14_0_ECX_LIP) == > + (env->features[FEAT_14_0_ECX] & > + CPUID_14_0_ECX_LIP))) { > if (cpu->intel_pt_auto_level) { > x86_cpu_adjust_level(cpu, &cpu->env.cpuid_min_level, 0x14); > } else if (cpu->env.cpuid_min_level < 0x14) { diff --git > a/target/i386/cpu.h b/target/i386/cpu.h index 51c1d5f60a..1fcd93e39a 100644 > --- a/target/i386/cpu.h > +++ b/target/i386/cpu.h > @@ -541,6 +541,7 @@ typedef enum FeatureWord { > FEAT_VMX_EPT_VPID_CAPS, > FEAT_VMX_BASIC, > FEAT_VMX_VMFUNC, > + FEAT_14_0_ECX, > FEATURE_WORDS, > } FeatureWord; > > @@ -797,6 +798,9 @@ typedef uint64_t FeatureWordArray[FEATURE_WORDS]; > /* AVX512 BFloat16 Instruction */ > #define CPUID_7_1_EAX_AVX512_BF16 (1U << 5) > > +/* Packets which contain IP payload have LIP values */ > +#define CPUID_14_0_ECX_LIP (1U << 31) > + > /* CLZERO instruction */ > #define CPUID_8000_0008_EBX_CLZERO (1U << 0) > /* Always save/restore FP error pointers */ > -- > 2.18.4 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] i386/cpu: Make the Intel PT LIP feature configurable 2020-10-14 8:04 ` [PATCH 2/2] i386/cpu: Make the Intel PT LIP feature configurable Luwei Kang 2020-11-11 1:47 ` Kang, Luwei @ 2020-12-01 21:19 ` Eduardo Habkost 2020-12-02 10:18 ` Kang, Luwei 1 sibling, 1 reply; 7+ messages in thread From: Eduardo Habkost @ 2020-12-01 21:19 UTC (permalink / raw) To: Luwei Kang; +Cc: pbonzini, qemu-devel, rth On Wed, Oct 14, 2020 at 04:04:43PM +0800, Luwei Kang wrote: > The current implementation will disable the guest Intel PT > feature if the Intel PT LIP feature is supported on the host, > but the LIP feature is comming soon(e.g. SnowRidge and later). > > This patch will make the guest LIP feature configurable and > Intel PT feature can be enabled in guest when the guest LIP > status same with the host. > > Signed-off-by: Luwei Kang <luwei.kang@intel.com> The patch looks good, but I would like to confirm if the feature works as expected if patch 1/2 is not applied. I have manually fixed it up to make patch 1/1 not required, but I would like to confirm it really works on both Intel hosts with LIP=1 and hosts with LIP=0. Could you please test the following version of the patch? ------- From 8a2c9fdbe50f7ffd9900c67e72f53c8dc85ff3fd Mon Sep 17 00:00:00 2001 From: Luwei Kang <luwei.kang@intel.com> Date: Wed, 14 Oct 2020 16:04:43 +0800 Subject: [PATCH] i386/cpu: Make the Intel PT LIP feature configurable The current implementation will disable the guest Intel PT feature if the Intel PT LIP feature is supported on the host, but the LIP feature is comming soon(e.g. SnowRidge and later). This patch will make the guest LIP feature configurable and Intel PT feature can be enabled in guest when the guest LIP status same with the host. Signed-off-by: Luwei Kang <luwei.kang@intel.com> Message-Id: <20201014080443.23751-2-luwei.kang@intel.com> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- target/i386/cpu.h | 4 ++++ target/i386/cpu.c | 27 ++++++++++++++++++++++++++- 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/target/i386/cpu.h b/target/i386/cpu.h index 88e8586f8f..c4a49c06a8 100644 --- a/target/i386/cpu.h +++ b/target/i386/cpu.h @@ -541,6 +541,7 @@ typedef enum FeatureWord { FEAT_VMX_EPT_VPID_CAPS, FEAT_VMX_BASIC, FEAT_VMX_VMFUNC, + FEAT_14_0_ECX, FEATURE_WORDS, } FeatureWord; @@ -797,6 +798,9 @@ typedef uint64_t FeatureWordArray[FEATURE_WORDS]; /* AVX512 BFloat16 Instruction */ #define CPUID_7_1_EAX_AVX512_BF16 (1U << 5) +/* Packets which contain IP payload have LIP values */ +#define CPUID_14_0_ECX_LIP (1U << 31) + /* CLZERO instruction */ #define CPUID_8000_0008_EBX_CLZERO (1U << 0) /* Always save/restore FP error pointers */ diff --git a/target/i386/cpu.c b/target/i386/cpu.c index 5a8c96072e..db9e48033d 100644 --- a/target/i386/cpu.c +++ b/target/i386/cpu.c @@ -672,6 +672,7 @@ static void x86_cpu_vendor_words2str(char *dst, uint32_t vendor1, #define TCG_XSAVE_FEATURES (CPUID_XSAVE_XSAVEOPT | CPUID_XSAVE_XGETBV1) /* missing: CPUID_XSAVE_XSAVEC, CPUID_XSAVE_XSAVES */ +#define TCG_14_0_ECX_FEATURES 0 typedef enum FeatureWordType { CPUID_FEATURE_WORD, @@ -1301,6 +1302,26 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = { } }, + [FEAT_14_0_ECX] = { + .type = CPUID_FEATURE_WORD, + .feat_names = { + 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, NULL, + NULL, NULL, NULL, "intel-pt-lip", + }, + .cpuid = { + .eax = 0x14, + .needs_ecx = true, .ecx = 0, + .reg = R_ECX, + }, + .tcg_features = TCG_14_0_ECX_FEATURES, + }, + }; typedef struct FeatureMask { @@ -5752,6 +5773,9 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count, *eax = INTEL_PT_MAX_SUBLEAF; *ebx = INTEL_PT_MINIMAL_EBX; *ecx = INTEL_PT_MINIMAL_ECX; + if (env->features[FEAT_14_0_ECX] & CPUID_14_0_ECX_LIP) { + *ecx |= CPUID_14_0_ECX_LIP; + } } else if (count == 1) { *eax = INTEL_PT_MTC_BITMAP | INTEL_PT_ADDR_RANGES_NUM; *ebx = INTEL_PT_PSB_BITMAP | INTEL_PT_CYCLE_BITMAP; @@ -6498,7 +6522,8 @@ static void x86_cpu_filter_features(X86CPU *cpu, bool verbose) INTEL_PT_ADDR_RANGES_NUM) || ((ebx_1 & (INTEL_PT_PSB_BITMAP | INTEL_PT_CYCLE_BITMAP)) != (INTEL_PT_PSB_BITMAP | INTEL_PT_CYCLE_BITMAP)) || - (ecx_0 & INTEL_PT_IP_LIP)) { + ((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 -- 2.28.0 -- Eduardo ^ permalink raw reply related [flat|nested] 7+ messages in thread
* RE: [PATCH 2/2] i386/cpu: Make the Intel PT LIP feature configurable 2020-12-01 21:19 ` Eduardo Habkost @ 2020-12-02 10:18 ` Kang, Luwei 0 siblings, 0 replies; 7+ messages in thread From: Kang, Luwei @ 2020-12-02 10:18 UTC (permalink / raw) To: Eduardo Habkost; +Cc: pbonzini, qemu-devel, rth > > The current implementation will disable the guest Intel PT feature if > > the Intel PT LIP feature is supported on the host, but the LIP feature > > is comming soon(e.g. SnowRidge and later). > > > > This patch will make the guest LIP feature configurable and Intel PT > > feature can be enabled in guest when the guest LIP status same with > > the host. > > > > Signed-off-by: Luwei Kang <luwei.kang@intel.com> > > The patch looks good, but I would like to confirm if the feature works as > expected if patch 1/2 is not applied. > > I have manually fixed it up to make patch 1/1 not required, but I would like to > confirm it really works on both Intel hosts with > LIP=1 and hosts with LIP=0. Could you please test the following version of the > patch? Hi Eduardo, Thanks for your reply. I sent out a new version(V2) which adds a dependency flag based on your modification. And all the test cases and results include HW w/ and w/o LIP list in version 2. Thanks, Luwei Kang > > ------- > From 8a2c9fdbe50f7ffd9900c67e72f53c8dc85ff3fd Mon Sep 17 00:00:00 2001 > From: Luwei Kang <luwei.kang@intel.com> > Date: Wed, 14 Oct 2020 16:04:43 +0800 > Subject: [PATCH] i386/cpu: Make the Intel PT LIP feature configurable > > The current implementation will disable the guest Intel PT feature if the Intel > PT LIP feature is supported on the host, but the LIP feature is comming > soon(e.g. SnowRidge and later). > > This patch will make the guest LIP feature configurable and Intel PT feature can > be enabled in guest when the guest LIP status same with the host. > > Signed-off-by: Luwei Kang <luwei.kang@intel.com> > Message-Id: <20201014080443.23751-2-luwei.kang@intel.com> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> > --- > target/i386/cpu.h | 4 ++++ > target/i386/cpu.c | 27 ++++++++++++++++++++++++++- > 2 files changed, 30 insertions(+), 1 deletion(-) > > diff --git a/target/i386/cpu.h b/target/i386/cpu.h index 88e8586f8f..c4a49c06a8 > 100644 > --- a/target/i386/cpu.h > +++ b/target/i386/cpu.h > @@ -541,6 +541,7 @@ typedef enum FeatureWord { > FEAT_VMX_EPT_VPID_CAPS, > FEAT_VMX_BASIC, > FEAT_VMX_VMFUNC, > + FEAT_14_0_ECX, > FEATURE_WORDS, > } FeatureWord; > > @@ -797,6 +798,9 @@ typedef uint64_t FeatureWordArray[FEATURE_WORDS]; > /* AVX512 BFloat16 Instruction */ > #define CPUID_7_1_EAX_AVX512_BF16 (1U << 5) > > +/* Packets which contain IP payload have LIP values */ > +#define CPUID_14_0_ECX_LIP (1U << 31) > + > /* CLZERO instruction */ > #define CPUID_8000_0008_EBX_CLZERO (1U << 0) > /* Always save/restore FP error pointers */ diff --git a/target/i386/cpu.c > b/target/i386/cpu.c index 5a8c96072e..db9e48033d 100644 > --- a/target/i386/cpu.c > +++ b/target/i386/cpu.c > @@ -672,6 +672,7 @@ static void x86_cpu_vendor_words2str(char *dst, > uint32_t vendor1, #define TCG_XSAVE_FEATURES (CPUID_XSAVE_XSAVEOPT > | CPUID_XSAVE_XGETBV1) > /* missing: > CPUID_XSAVE_XSAVEC, CPUID_XSAVE_XSAVES */ > +#define TCG_14_0_ECX_FEATURES 0 > > typedef enum FeatureWordType { > CPUID_FEATURE_WORD, > @@ -1301,6 +1302,26 @@ static FeatureWordInfo > feature_word_info[FEATURE_WORDS] = { > } > }, > > + [FEAT_14_0_ECX] = { > + .type = CPUID_FEATURE_WORD, > + .feat_names = { > + 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, NULL, > + NULL, NULL, NULL, "intel-pt-lip", > + }, > + .cpuid = { > + .eax = 0x14, > + .needs_ecx = true, .ecx = 0, > + .reg = R_ECX, > + }, > + .tcg_features = TCG_14_0_ECX_FEATURES, > + }, > + > }; > > typedef struct FeatureMask { > @@ -5752,6 +5773,9 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t > index, uint32_t count, > *eax = INTEL_PT_MAX_SUBLEAF; > *ebx = INTEL_PT_MINIMAL_EBX; > *ecx = INTEL_PT_MINIMAL_ECX; > + if (env->features[FEAT_14_0_ECX] & CPUID_14_0_ECX_LIP) { > + *ecx |= CPUID_14_0_ECX_LIP; > + } > } else if (count == 1) { > *eax = INTEL_PT_MTC_BITMAP | INTEL_PT_ADDR_RANGES_NUM; > *ebx = INTEL_PT_PSB_BITMAP | INTEL_PT_CYCLE_BITMAP; @@ -6498,7 > +6522,8 @@ static void x86_cpu_filter_features(X86CPU *cpu, bool verbose) > INTEL_PT_ADDR_RANGES_NUM) || > ((ebx_1 & (INTEL_PT_PSB_BITMAP | INTEL_PT_CYCLE_BITMAP)) != > (INTEL_PT_PSB_BITMAP | INTEL_PT_CYCLE_BITMAP)) || > - (ecx_0 & INTEL_PT_IP_LIP)) { > + ((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 > -- > 2.28.0 > > -- > Eduardo ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] i386/cpu: Add the Intel PT capabilities checking before extend the CPUID level 2020-10-14 8:04 [PATCH 1/2] i386/cpu: Add the Intel PT capabilities checking before extend the CPUID level Luwei Kang 2020-10-14 8:04 ` [PATCH 2/2] i386/cpu: Make the Intel PT LIP feature configurable Luwei Kang @ 2020-12-01 21:12 ` Eduardo Habkost 2020-12-02 8:40 ` Kang, Luwei 1 sibling, 1 reply; 7+ messages in thread From: Eduardo Habkost @ 2020-12-01 21:12 UTC (permalink / raw) To: Luwei Kang; +Cc: pbonzini, qemu-devel, rth Hi, Sorry for the long delay in reviewing this. Now that 5.2 is about to be released, we can try to merge this. Comments below: On Wed, Oct 14, 2020 at 04:04:42PM +0800, Luwei Kang wrote: > The current implementation will extend the CPUID level to 0x14 if > Intel PT is enabled in the guest(in x86_cpu_expand_features()) and > the Intel PT will be disabled if it can't pass the capabilities > checking later(in x86_cpu_filter_features()). In this case, the > level of CPUID will be still 0x14 and the CPUID values from leaf > 0xe to 0x14 are all zero. > > This patch moves the capabilities checking before setting the > level of the CPUID. Why is this patch necessary and what problem does it fix? Is it a nice to have feature, or a bug fix? If you still want to change how the x86_cpu_adjust_level() code behaves, it should apply to all features filtered by x86_cpu_filter_features(), not just intel-pt, shouldn't it? > > Signed-off-by: Luwei Kang <luwei.kang@intel.com> > --- > target/i386/cpu.c | 63 ++++++++++++++++++++++++----------------------- > 1 file changed, 32 insertions(+), 31 deletions(-) > > diff --git a/target/i386/cpu.c b/target/i386/cpu.c > index 9eafbe3690..24644abfd4 100644 > --- a/target/i386/cpu.c > +++ b/target/i386/cpu.c > @@ -6401,12 +6401,40 @@ static void x86_cpu_expand_features(X86CPU *cpu, Error **errp) > > /* Intel Processor Trace requires CPUID[0x14] */ > if ((env->features[FEAT_7_0_EBX] & CPUID_7_0_EBX_INTEL_PT)) { > - if (cpu->intel_pt_auto_level) { > - x86_cpu_adjust_level(cpu, &cpu->env.cpuid_min_level, 0x14); > - } else if (cpu->env.cpuid_min_level < 0x14) { > + uint32_t eax_0, ebx_0, ecx_0, eax_1, ebx_1; > + > + eax_0 = kvm_arch_get_supported_cpuid(kvm_state, 0x14, 0, R_EAX); > + ebx_0 = kvm_arch_get_supported_cpuid(kvm_state, 0x14, 0, R_EBX); > + ecx_0 = kvm_arch_get_supported_cpuid(kvm_state, 0x14, 0, R_ECX); > + eax_1 = kvm_arch_get_supported_cpuid(kvm_state, 0x14, 1, R_EAX); > + ebx_1 = kvm_arch_get_supported_cpuid(kvm_state, 0x14, 1, R_EBX); > + > + if (eax_0 && > + ((ebx_0 & INTEL_PT_MINIMAL_EBX) == INTEL_PT_MINIMAL_EBX) && > + ((ecx_0 & INTEL_PT_MINIMAL_ECX) == INTEL_PT_MINIMAL_ECX) && > + ((eax_1 & INTEL_PT_MTC_BITMAP) == INTEL_PT_MTC_BITMAP) && > + ((eax_1 & INTEL_PT_ADDR_RANGES_NUM_MASK) >= > + INTEL_PT_ADDR_RANGES_NUM) && > + ((ebx_1 & (INTEL_PT_PSB_BITMAP | INTEL_PT_CYCLE_BITMAP)) == > + (INTEL_PT_PSB_BITMAP | INTEL_PT_CYCLE_BITMAP)) && > + !(ecx_0 & INTEL_PT_IP_LIP)) { > + if (cpu->intel_pt_auto_level) { > + x86_cpu_adjust_level(cpu, &cpu->env.cpuid_min_level, 0x14); > + } else if (cpu->env.cpuid_min_level < 0x14) { > + mark_unavailable_features(cpu, FEAT_7_0_EBX, > + CPUID_7_0_EBX_INTEL_PT, > + "Intel PT need CPUID leaf 0x14, please set by \"-cpu ...,+intel-pt,min-level=0x14\""); > + } > + } else { > + /* > + * 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. > + */ > mark_unavailable_features(cpu, FEAT_7_0_EBX, > CPUID_7_0_EBX_INTEL_PT, > - "Intel PT need CPUID leaf 0x14, please set by \"-cpu ...,+intel-pt,min-level=0x14\""); > + "host Intel PT features doesn't satisfy the guest request."); > } > } > > @@ -6466,33 +6494,6 @@ static void x86_cpu_filter_features(X86CPU *cpu, bool verbose) > uint64_t unavailable_features = requested_features & ~host_feat; > 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_MINIMAL_EBX) != INTEL_PT_MINIMAL_EBX) || > - ((ecx_0 & INTEL_PT_MINIMAL_ECX) != INTEL_PT_MINIMAL_ECX) || > - ((eax_1 & INTEL_PT_MTC_BITMAP) != INTEL_PT_MTC_BITMAP) || > - ((eax_1 & INTEL_PT_ADDR_RANGES_NUM_MASK) < > - INTEL_PT_ADDR_RANGES_NUM) || > - ((ebx_1 & (INTEL_PT_PSB_BITMAP | INTEL_PT_CYCLE_BITMAP)) != > - (INTEL_PT_PSB_BITMAP | INTEL_PT_CYCLE_BITMAP)) || > - (ecx_0 & INTEL_PT_IP_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. > - */ > - mark_unavailable_features(cpu, FEAT_7_0_EBX, CPUID_7_0_EBX_INTEL_PT, prefix); > - } > - } > } > > static void x86_cpu_realizefn(DeviceState *dev, Error **errp) > -- > 2.18.4 > -- Eduardo ^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [PATCH 1/2] i386/cpu: Add the Intel PT capabilities checking before extend the CPUID level 2020-12-01 21:12 ` [PATCH 1/2] i386/cpu: Add the Intel PT capabilities checking before extend the CPUID level Eduardo Habkost @ 2020-12-02 8:40 ` Kang, Luwei 0 siblings, 0 replies; 7+ messages in thread From: Kang, Luwei @ 2020-12-02 8:40 UTC (permalink / raw) To: Eduardo Habkost; +Cc: pbonzini, qemu-devel, rth > -----Original Message----- > From: Eduardo Habkost <ehabkost@redhat.com> > Sent: Wednesday, December 2, 2020 5:12 AM > To: Kang, Luwei <luwei.kang@intel.com> > Cc: pbonzini@redhat.com; rth@twiddle.net; qemu-devel@nongnu.org > Subject: Re: [PATCH 1/2] i386/cpu: Add the Intel PT capabilities checking before > extend the CPUID level > > Hi, > > Sorry for the long delay in reviewing this. Now that 5.2 is about to be released, > we can try to merge this. > > Comments below: > > On Wed, Oct 14, 2020 at 04:04:42PM +0800, Luwei Kang wrote: > > The current implementation will extend the CPUID level to 0x14 if > > Intel PT is enabled in the guest(in x86_cpu_expand_features()) and the > > Intel PT will be disabled if it can't pass the capabilities checking > > later(in x86_cpu_filter_features()). In this case, the level of CPUID > > will be still 0x14 and the CPUID values from leaf 0xe to 0x14 are all > > zero. > > > > This patch moves the capabilities checking before setting the level of > > the CPUID. > > Why is this patch necessary and what problem does it fix? Is it a nice to have > feature, or a bug fix? > > If you still want to change how the x86_cpu_adjust_level() code behaves, it > should apply to all features filtered by x86_cpu_filter_features(), not just intel- > pt, shouldn't it? Hi Eduardo, Let me clarify the issue. The cpuid level is 0xd if create a VM(cpu model qemu64) w/o intel pt feature on Snowridge HW. CMD: qemu-system-x86_64 -cpu qemu64 ... (Intel PT is disabled by default) The cpuid level will be extended to 0x14 if create a VM(cpu model qemu64) w/ intel pt feature on Snowridage. CMD: qemu-system-x86_64 -cpu qemu64,+intel-pt ... But the current software implementation will mask off intel pt if the host has LIP, and Snowridge support it. So cpuid level has been extended to 0x14(x86_cpu_expand_features) and Intel PT is disabled later(x86_cpu_filter_features), and the guest CPUID will include some zero items like this. 0x0000000e 0x00: eax=0x00000000 ebx=0x00000000 ecx=0x00000000 edx=0x00000000 0x0000000f 0x00: eax=0x00000000 ebx=0x00000000 ecx=0x00000000 edx=0x00000000 0x00000010 0x00: eax=0x00000000 ebx=0x00000000 ecx=0x00000000 edx=0x00000000 0x00000011 0x00: eax=0x00000000 ebx=0x00000000 ecx=0x00000000 edx=0x00000000 0x00000012 0x00: eax=0x00000000 ebx=0x00000000 ecx=0x00000000 edx=0x00000000 0x00000013 0x00: eax=0x00000000 ebx=0x00000000 ecx=0x00000000 edx=0x00000000 0x00000014 0x00: eax=0x00000000 ebx=0x00000000 ecx=0x00000000 edx=0x00000000 (Intel PT feature) x86_cpu_realizefn() |-> x86_cpu_expand_features() | IF has Intel PT feature | Then extended the cpuid level to 0x14; |-> x86_cpu_filter_features() IF has Intel PT feature & has LIP THEN mask off the guest Intel PT feature, *but* the cpuid level still 0x14. Expect result and how to: Don't impact the cpuid level if Intel PT can't be supported in the guest. So this patch moves the intel PT capabilities check before extending the cpuid level. Thanks, Luwei Kang > > > > > Signed-off-by: Luwei Kang <luwei.kang@intel.com> > > --- > > target/i386/cpu.c | 63 > > ++++++++++++++++++++++++----------------------- > > 1 file changed, 32 insertions(+), 31 deletions(-) > > > > diff --git a/target/i386/cpu.c b/target/i386/cpu.c index > > 9eafbe3690..24644abfd4 100644 > > --- a/target/i386/cpu.c > > +++ b/target/i386/cpu.c > > @@ -6401,12 +6401,40 @@ static void x86_cpu_expand_features(X86CPU > > *cpu, Error **errp) > > > > /* Intel Processor Trace requires CPUID[0x14] */ > > if ((env->features[FEAT_7_0_EBX] & CPUID_7_0_EBX_INTEL_PT)) { > > - if (cpu->intel_pt_auto_level) { > > - x86_cpu_adjust_level(cpu, &cpu->env.cpuid_min_level, 0x14); > > - } else if (cpu->env.cpuid_min_level < 0x14) { > > + uint32_t eax_0, ebx_0, ecx_0, eax_1, ebx_1; > > + > > + eax_0 = kvm_arch_get_supported_cpuid(kvm_state, 0x14, 0, R_EAX); > > + ebx_0 = kvm_arch_get_supported_cpuid(kvm_state, 0x14, 0, R_EBX); > > + ecx_0 = kvm_arch_get_supported_cpuid(kvm_state, 0x14, 0, R_ECX); > > + eax_1 = kvm_arch_get_supported_cpuid(kvm_state, 0x14, 1, R_EAX); > > + ebx_1 = kvm_arch_get_supported_cpuid(kvm_state, 0x14, 1, > > + R_EBX); > > + > > + if (eax_0 && > > + ((ebx_0 & INTEL_PT_MINIMAL_EBX) == INTEL_PT_MINIMAL_EBX) > && > > + ((ecx_0 & INTEL_PT_MINIMAL_ECX) == INTEL_PT_MINIMAL_ECX) > && > > + ((eax_1 & INTEL_PT_MTC_BITMAP) == INTEL_PT_MTC_BITMAP) && > > + ((eax_1 & INTEL_PT_ADDR_RANGES_NUM_MASK) >= > > + INTEL_PT_ADDR_RANGES_NUM) && > > + ((ebx_1 & (INTEL_PT_PSB_BITMAP | INTEL_PT_CYCLE_BITMAP)) == > > + (INTEL_PT_PSB_BITMAP | INTEL_PT_CYCLE_BITMAP)) && > > + !(ecx_0 & INTEL_PT_IP_LIP)) { > > + if (cpu->intel_pt_auto_level) { > > + x86_cpu_adjust_level(cpu, &cpu->env.cpuid_min_level, 0x14); > > + } else if (cpu->env.cpuid_min_level < 0x14) { > > + mark_unavailable_features(cpu, FEAT_7_0_EBX, > > + CPUID_7_0_EBX_INTEL_PT, > > + "Intel PT need CPUID leaf 0x14, please set by \"-cpu ...,+intel- > pt,min-level=0x14\""); > > + } > > + } else { > > + /* > > + * 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. > > + */ > > mark_unavailable_features(cpu, FEAT_7_0_EBX, > > CPUID_7_0_EBX_INTEL_PT, > > - "Intel PT need CPUID leaf 0x14, please set by \"-cpu ...,+intel- > pt,min-level=0x14\""); > > + "host Intel PT features doesn't satisfy the guest > > + request."); > > } > > } > > > > @@ -6466,33 +6494,6 @@ static void x86_cpu_filter_features(X86CPU *cpu, > bool verbose) > > uint64_t unavailable_features = requested_features & ~host_feat; > > 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_MINIMAL_EBX) != INTEL_PT_MINIMAL_EBX) || > > - ((ecx_0 & INTEL_PT_MINIMAL_ECX) != INTEL_PT_MINIMAL_ECX) || > > - ((eax_1 & INTEL_PT_MTC_BITMAP) != INTEL_PT_MTC_BITMAP) || > > - ((eax_1 & INTEL_PT_ADDR_RANGES_NUM_MASK) < > > - INTEL_PT_ADDR_RANGES_NUM) || > > - ((ebx_1 & (INTEL_PT_PSB_BITMAP | INTEL_PT_CYCLE_BITMAP)) != > > - (INTEL_PT_PSB_BITMAP | INTEL_PT_CYCLE_BITMAP)) || > > - (ecx_0 & INTEL_PT_IP_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. > > - */ > > - mark_unavailable_features(cpu, FEAT_7_0_EBX, > CPUID_7_0_EBX_INTEL_PT, prefix); > > - } > > - } > > } > > > > static void x86_cpu_realizefn(DeviceState *dev, Error **errp) > > -- > > 2.18.4 > > > > -- > Eduardo ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-12-02 10:55 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-10-14 8:04 [PATCH 1/2] i386/cpu: Add the Intel PT capabilities checking before extend the CPUID level Luwei Kang 2020-10-14 8:04 ` [PATCH 2/2] i386/cpu: Make the Intel PT LIP feature configurable Luwei Kang 2020-11-11 1:47 ` Kang, Luwei 2020-12-01 21:19 ` Eduardo Habkost 2020-12-02 10:18 ` Kang, Luwei 2020-12-01 21:12 ` [PATCH 1/2] i386/cpu: Add the Intel PT capabilities checking before extend the CPUID level Eduardo Habkost 2020-12-02 8:40 ` Kang, Luwei
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).