| Message ID | 20201014080443.23751-2-luwei.kang@intel.com |
|---|---|
| State | New |
| Headers | show |
| Series | [1/2] i386/cpu: Add the Intel PT capabilities checking before extend the CPUID level | expand |
> -----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
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
> > 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
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 */
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(-)