* [PATCH v2 1/2] i386: Add Intel Processor Trace feature support
@ 2018-01-25 18:25 ` Luwei Kang
0 siblings, 0 replies; 10+ messages in thread
From: Luwei Kang @ 2018-01-25 18:25 UTC (permalink / raw)
To: qemu-devel, kvm; +Cc: pbonzini, rth, ehabkost, mtosatti, Chao Peng, Luwei Kang
From: Chao Peng <chao.p.peng@linux.intel.com>
Expose Intel Processor Trace feature to guest.
In order to make this feature migration-safe, new feature word
information "FEAT_INTEL_PT_EBX" and "FEAT_INTEL_PT_ECX" be added.
Some constant value initialized in CPUID[0x14].0x01 to guarantee
get same result in diffrent hardware when this feature is enabled.
Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>
Signed-off-by: Luwei Kang <luwei.kang@intel.com>
---
v1->v2:
- In order to make this feature migration-safe, new feature word
information "FEAT_INTEL_PT_EBX" and "FEAT_INTEL_PT_ECX" be added.
Some constant value initialized in CPUID[0x14].0x01 to guarantee
get same result in diffrent hardware when this feature is enabled.
---
target/i386/cpu.c | 68 +++++++++++++++++++++++++++++++++++++++++++++++++++++--
target/i386/cpu.h | 6 +++++
target/i386/kvm.c | 23 +++++++++++++++++++
3 files changed, 95 insertions(+), 2 deletions(-)
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index a49d222..ee8c6e6 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -172,7 +172,11 @@
#define L2_ITLB_4K_ASSOC 4
#define L2_ITLB_4K_ENTRIES 512
-
+/* CPUID Leaf 0x14 constants: */
+#define INTLE_PT_ADDR_RANGES_NUM 2
+#define INTEL_PT_MTC_BITMAP (0x0249 << 16) /* Support ART(0,3,6,9) */
+#define INTEL_PT_CYCLE_BITMAP 0x3fff /* Support 0,2^(0~12) */
+#define INTEL_PT_PSB_BITMAP (0x003f << 16) /* Support 2K,4K,8K,16K,32K,64K */
static void x86_cpu_vendor_words2str(char *dst, uint32_t vendor1,
uint32_t vendor2, uint32_t vendor3)
@@ -427,7 +431,7 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
NULL, NULL, "mpx", NULL,
"avx512f", "avx512dq", "rdseed", "adx",
"smap", "avx512ifma", "pcommit", "clflushopt",
- "clwb", NULL, "avx512pf", "avx512er",
+ "clwb", "intel-pt", "avx512pf", "avx512er",
"avx512cd", "sha-ni", "avx512bw", "avx512vl",
},
.cpuid_eax = 7,
@@ -545,6 +549,38 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
.cpuid_reg = R_EDX,
.tcg_features = ~0U,
},
+ [FEAT_INTEL_PT_EBX] = {
+ .feat_names = {
+ "cr3-filter", "psb-cycle", "filtering", "mtc-cofi",
+ "ptwrite", "power-evt", 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,
+ },
+ .cpuid_eax = 0x14,
+ .cpuid_needs_ecx = true, .cpuid_ecx = 0,
+ .cpuid_reg = R_EBX,
+ .tcg_features = ~0U,
+ },
+ [FEAT_INTEL_PT_ECX] = {
+ .feat_names = {
+ "topa-output", "any-entries", "single-range", "transport",
+ 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,
+ },
+ .cpuid_eax = 0x14,
+ .cpuid_needs_ecx = true, .cpuid_ecx = 0,
+ .cpuid_reg = R_ECX,
+ .tcg_features = ~0U,
+ },
};
typedef struct X86RegisterInfo32 {
@@ -3452,6 +3488,34 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
}
break;
}
+ case 0x14: {
+ /* Intel Processor Trace Enumeration */
+ *eax = 0;
+ *ebx = 0;
+ *ecx = 0;
+ *edx = 0;
+ if (!(env->features[FEAT_7_0_EBX] & CPUID_7_0_EBX_INTEL_PT) ||
+ !kvm_enabled()) {
+ break;
+ }
+
+ if (count == 0) {
+ *eax = 1;
+ *ebx = env->features[FEAT_INTEL_PT_EBX];
+ *ecx = env->features[FEAT_INTEL_PT_ECX];
+ } else if (count == 1) {
+ *eax = INTLE_PT_ADDR_RANGES_NUM;
+ if (env->features[FEAT_INTEL_PT_EBX] &
+ CPUID_INTEL_PT_EBX_MTC_COFI) {
+ *eax |= INTEL_PT_MTC_BITMAP;
+ }
+ if (env->features[FEAT_INTEL_PT_EBX] &
+ CPUID_INTEL_PT_EBX_PSB_CYCLE) {
+ *ebx = INTEL_PT_PSB_BITMAP | INTEL_PT_CYCLE_BITMAP;
+ }
+ }
+ break;
+ }
case 0x40000000:
/*
* CPUID code in kvm_arch_init_vcpu() ignores stuff
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 30cc562..53908ff 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -483,6 +483,8 @@ typedef enum FeatureWord {
FEAT_6_EAX, /* CPUID[6].EAX */
FEAT_XSAVE_COMP_LO, /* CPUID[EAX=0xd,ECX=0].EAX */
FEAT_XSAVE_COMP_HI, /* CPUID[EAX=0xd,ECX=0].EDX */
+ FEAT_INTEL_PT_EBX, /* CPUID[EAX=0x14,ECX=0].EBX */
+ FEAT_INTEL_PT_ECX, /* CPUID[EAX=0x14,ECX=0].ECX */
FEATURE_WORDS,
} FeatureWord;
@@ -644,6 +646,7 @@ typedef uint32_t FeatureWordArray[FEATURE_WORDS];
#define CPUID_7_0_EBX_PCOMMIT (1U << 22) /* Persistent Commit */
#define CPUID_7_0_EBX_CLFLUSHOPT (1U << 23) /* Flush a Cache Line Optimized */
#define CPUID_7_0_EBX_CLWB (1U << 24) /* Cache Line Write Back */
+#define CPUID_7_0_EBX_INTEL_PT (1U << 25) /* Intel Processor Trace */
#define CPUID_7_0_EBX_AVX512PF (1U << 26) /* AVX-512 Prefetch */
#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 */
@@ -677,6 +680,9 @@ typedef uint32_t FeatureWordArray[FEATURE_WORDS];
#define CPUID_XSAVE_XGETBV1 (1U << 2)
#define CPUID_XSAVE_XSAVES (1U << 3)
+#define CPUID_INTEL_PT_EBX_PSB_CYCLE (1U << 1) /* Support configurable PSB and Cycle-Accurate Mode */
+#define CPUID_INTEL_PT_EBX_MTC_COFI (1U << 3) /* Support MTC timing and suppression of COFI-based packets */
+
#define CPUID_6_EAX_ARAT (1U << 2)
/* CPUID[0x80000007].EDX flags: */
diff --git a/target/i386/kvm.c b/target/i386/kvm.c
index ad4b159..f9f4cd1 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -865,6 +865,29 @@ int kvm_arch_init_vcpu(CPUState *cs)
c = &cpuid_data.entries[cpuid_i++];
}
break;
+ case 0x14: {
+ uint32_t times;
+
+ c->function = i;
+ c->index = 0;
+ c->flags = KVM_CPUID_FLAG_SIGNIFCANT_INDEX;
+ cpu_x86_cpuid(env, i, 0, &c->eax, &c->ebx, &c->ecx, &c->edx);
+ times = c->eax;
+
+ for (j = 1; j <= times; ++j) {
+ if (cpuid_i == KVM_MAX_CPUID_ENTRIES) {
+ fprintf(stderr, "cpuid_data is full, no space for "
+ "cpuid(eax:0x14,ecx:0x%x)\n", j);
+ abort();
+ }
+ c = &cpuid_data.entries[cpuid_i++];
+ c->function = i;
+ c->index = j;
+ c->flags = KVM_CPUID_FLAG_SIGNIFCANT_INDEX;
+ cpu_x86_cpuid(env, i, j, &c->eax, &c->ebx, &c->ecx, &c->edx);
+ }
+ break;
+ }
default:
c->function = i;
c->flags = 0;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [Qemu-devel] [PATCH v2 1/2] i386: Add Intel Processor Trace feature support
@ 2018-01-25 18:25 ` Luwei Kang
0 siblings, 0 replies; 10+ messages in thread
From: Luwei Kang @ 2018-01-25 18:25 UTC (permalink / raw)
To: qemu-devel, kvm; +Cc: pbonzini, rth, ehabkost, mtosatti, Chao Peng, Luwei Kang
From: Chao Peng <chao.p.peng@linux.intel.com>
Expose Intel Processor Trace feature to guest.
In order to make this feature migration-safe, new feature word
information "FEAT_INTEL_PT_EBX" and "FEAT_INTEL_PT_ECX" be added.
Some constant value initialized in CPUID[0x14].0x01 to guarantee
get same result in diffrent hardware when this feature is enabled.
Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>
Signed-off-by: Luwei Kang <luwei.kang@intel.com>
---
v1->v2:
- In order to make this feature migration-safe, new feature word
information "FEAT_INTEL_PT_EBX" and "FEAT_INTEL_PT_ECX" be added.
Some constant value initialized in CPUID[0x14].0x01 to guarantee
get same result in diffrent hardware when this feature is enabled.
---
target/i386/cpu.c | 68 +++++++++++++++++++++++++++++++++++++++++++++++++++++--
target/i386/cpu.h | 6 +++++
target/i386/kvm.c | 23 +++++++++++++++++++
3 files changed, 95 insertions(+), 2 deletions(-)
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index a49d222..ee8c6e6 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -172,7 +172,11 @@
#define L2_ITLB_4K_ASSOC 4
#define L2_ITLB_4K_ENTRIES 512
-
+/* CPUID Leaf 0x14 constants: */
+#define INTLE_PT_ADDR_RANGES_NUM 2
+#define INTEL_PT_MTC_BITMAP (0x0249 << 16) /* Support ART(0,3,6,9) */
+#define INTEL_PT_CYCLE_BITMAP 0x3fff /* Support 0,2^(0~12) */
+#define INTEL_PT_PSB_BITMAP (0x003f << 16) /* Support 2K,4K,8K,16K,32K,64K */
static void x86_cpu_vendor_words2str(char *dst, uint32_t vendor1,
uint32_t vendor2, uint32_t vendor3)
@@ -427,7 +431,7 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
NULL, NULL, "mpx", NULL,
"avx512f", "avx512dq", "rdseed", "adx",
"smap", "avx512ifma", "pcommit", "clflushopt",
- "clwb", NULL, "avx512pf", "avx512er",
+ "clwb", "intel-pt", "avx512pf", "avx512er",
"avx512cd", "sha-ni", "avx512bw", "avx512vl",
},
.cpuid_eax = 7,
@@ -545,6 +549,38 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
.cpuid_reg = R_EDX,
.tcg_features = ~0U,
},
+ [FEAT_INTEL_PT_EBX] = {
+ .feat_names = {
+ "cr3-filter", "psb-cycle", "filtering", "mtc-cofi",
+ "ptwrite", "power-evt", 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,
+ },
+ .cpuid_eax = 0x14,
+ .cpuid_needs_ecx = true, .cpuid_ecx = 0,
+ .cpuid_reg = R_EBX,
+ .tcg_features = ~0U,
+ },
+ [FEAT_INTEL_PT_ECX] = {
+ .feat_names = {
+ "topa-output", "any-entries", "single-range", "transport",
+ 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,
+ },
+ .cpuid_eax = 0x14,
+ .cpuid_needs_ecx = true, .cpuid_ecx = 0,
+ .cpuid_reg = R_ECX,
+ .tcg_features = ~0U,
+ },
};
typedef struct X86RegisterInfo32 {
@@ -3452,6 +3488,34 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
}
break;
}
+ case 0x14: {
+ /* Intel Processor Trace Enumeration */
+ *eax = 0;
+ *ebx = 0;
+ *ecx = 0;
+ *edx = 0;
+ if (!(env->features[FEAT_7_0_EBX] & CPUID_7_0_EBX_INTEL_PT) ||
+ !kvm_enabled()) {
+ break;
+ }
+
+ if (count == 0) {
+ *eax = 1;
+ *ebx = env->features[FEAT_INTEL_PT_EBX];
+ *ecx = env->features[FEAT_INTEL_PT_ECX];
+ } else if (count == 1) {
+ *eax = INTLE_PT_ADDR_RANGES_NUM;
+ if (env->features[FEAT_INTEL_PT_EBX] &
+ CPUID_INTEL_PT_EBX_MTC_COFI) {
+ *eax |= INTEL_PT_MTC_BITMAP;
+ }
+ if (env->features[FEAT_INTEL_PT_EBX] &
+ CPUID_INTEL_PT_EBX_PSB_CYCLE) {
+ *ebx = INTEL_PT_PSB_BITMAP | INTEL_PT_CYCLE_BITMAP;
+ }
+ }
+ break;
+ }
case 0x40000000:
/*
* CPUID code in kvm_arch_init_vcpu() ignores stuff
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 30cc562..53908ff 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -483,6 +483,8 @@ typedef enum FeatureWord {
FEAT_6_EAX, /* CPUID[6].EAX */
FEAT_XSAVE_COMP_LO, /* CPUID[EAX=0xd,ECX=0].EAX */
FEAT_XSAVE_COMP_HI, /* CPUID[EAX=0xd,ECX=0].EDX */
+ FEAT_INTEL_PT_EBX, /* CPUID[EAX=0x14,ECX=0].EBX */
+ FEAT_INTEL_PT_ECX, /* CPUID[EAX=0x14,ECX=0].ECX */
FEATURE_WORDS,
} FeatureWord;
@@ -644,6 +646,7 @@ typedef uint32_t FeatureWordArray[FEATURE_WORDS];
#define CPUID_7_0_EBX_PCOMMIT (1U << 22) /* Persistent Commit */
#define CPUID_7_0_EBX_CLFLUSHOPT (1U << 23) /* Flush a Cache Line Optimized */
#define CPUID_7_0_EBX_CLWB (1U << 24) /* Cache Line Write Back */
+#define CPUID_7_0_EBX_INTEL_PT (1U << 25) /* Intel Processor Trace */
#define CPUID_7_0_EBX_AVX512PF (1U << 26) /* AVX-512 Prefetch */
#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 */
@@ -677,6 +680,9 @@ typedef uint32_t FeatureWordArray[FEATURE_WORDS];
#define CPUID_XSAVE_XGETBV1 (1U << 2)
#define CPUID_XSAVE_XSAVES (1U << 3)
+#define CPUID_INTEL_PT_EBX_PSB_CYCLE (1U << 1) /* Support configurable PSB and Cycle-Accurate Mode */
+#define CPUID_INTEL_PT_EBX_MTC_COFI (1U << 3) /* Support MTC timing and suppression of COFI-based packets */
+
#define CPUID_6_EAX_ARAT (1U << 2)
/* CPUID[0x80000007].EDX flags: */
diff --git a/target/i386/kvm.c b/target/i386/kvm.c
index ad4b159..f9f4cd1 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -865,6 +865,29 @@ int kvm_arch_init_vcpu(CPUState *cs)
c = &cpuid_data.entries[cpuid_i++];
}
break;
+ case 0x14: {
+ uint32_t times;
+
+ c->function = i;
+ c->index = 0;
+ c->flags = KVM_CPUID_FLAG_SIGNIFCANT_INDEX;
+ cpu_x86_cpuid(env, i, 0, &c->eax, &c->ebx, &c->ecx, &c->edx);
+ times = c->eax;
+
+ for (j = 1; j <= times; ++j) {
+ if (cpuid_i == KVM_MAX_CPUID_ENTRIES) {
+ fprintf(stderr, "cpuid_data is full, no space for "
+ "cpuid(eax:0x14,ecx:0x%x)\n", j);
+ abort();
+ }
+ c = &cpuid_data.entries[cpuid_i++];
+ c->function = i;
+ c->index = j;
+ c->flags = KVM_CPUID_FLAG_SIGNIFCANT_INDEX;
+ cpu_x86_cpuid(env, i, j, &c->eax, &c->ebx, &c->ecx, &c->edx);
+ }
+ break;
+ }
default:
c->function = i;
c->flags = 0;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 2/2] i386: Add support to get/set/migrate Intel Processor Trace feature
2018-01-25 18:25 ` [Qemu-devel] " Luwei Kang
@ 2018-01-25 18:25 ` Luwei Kang
-1 siblings, 0 replies; 10+ messages in thread
From: Luwei Kang @ 2018-01-25 18:25 UTC (permalink / raw)
To: qemu-devel, kvm; +Cc: pbonzini, rth, ehabkost, mtosatti, Chao Peng, Luwei Kang
From: Chao Peng <chao.p.peng@linux.intel.com>
Add Intel Processor Trace related definition. It also add
corresponding part to kvm_get/set_msr and vmstate.
Signed-off-by: Luwei Kang <luwei.kang@intel.com>
Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>
---
target/i386/cpu.h | 22 ++++++++++++++++++++++
target/i386/kvm.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++
target/i386/machine.c | 38 ++++++++++++++++++++++++++++++++++++++
3 files changed, 111 insertions(+)
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 53908ff..85f0a04 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -419,6 +419,21 @@ typedef enum X86Seg {
#define MSR_MC0_ADDR 0x402
#define MSR_MC0_MISC 0x403
+#define MSR_IA32_RTIT_OUTPUT_BASE 0x560
+#define MSR_IA32_RTIT_OUTPUT_MASK 0x561
+#define MSR_IA32_RTIT_CTL 0x570
+#define MSR_IA32_RTIT_STATUS 0x571
+#define MSR_IA32_RTIT_CR3_MATCH 0x572
+#define MSR_IA32_RTIT_ADDR0_A 0x580
+#define MSR_IA32_RTIT_ADDR0_B 0x581
+#define MSR_IA32_RTIT_ADDR1_A 0x582
+#define MSR_IA32_RTIT_ADDR1_B 0x583
+#define MSR_IA32_RTIT_ADDR2_A 0x584
+#define MSR_IA32_RTIT_ADDR2_B 0x585
+#define MSR_IA32_RTIT_ADDR3_A 0x586
+#define MSR_IA32_RTIT_ADDR3_B 0x587
+#define MAX_RTIT_ADDRS 8
+
#define MSR_EFER 0xc0000080
#define MSR_EFER_SCE (1 << 0)
@@ -1163,6 +1178,13 @@ typedef struct CPUX86State {
uint64_t msr_hv_stimer_config[HV_STIMER_COUNT];
uint64_t msr_hv_stimer_count[HV_STIMER_COUNT];
+ uint64_t msr_rtit_ctrl;
+ uint64_t msr_rtit_status;
+ uint64_t msr_rtit_output_base;
+ uint64_t msr_rtit_output_mask;
+ uint64_t msr_rtit_cr3_match;
+ uint64_t msr_rtit_addrs[MAX_RTIT_ADDRS];
+
/* exception/interrupt handling */
int error_code;
int exception_is_int;
diff --git a/target/i386/kvm.c b/target/i386/kvm.c
index f9f4cd1..097c953 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -1811,6 +1811,25 @@ static int kvm_put_msrs(X86CPU *cpu, int level)
kvm_msr_entry_add(cpu, MSR_MTRRphysMask(i), mask);
}
}
+ if (env->features[FEAT_7_0_EBX] & CPUID_7_0_EBX_INTEL_PT) {
+ int addr_num = kvm_arch_get_supported_cpuid(kvm_state,
+ 0x14, 1, R_EAX) & 0x7;
+
+ kvm_msr_entry_add(cpu, MSR_IA32_RTIT_CTL,
+ env->msr_rtit_ctrl);
+ kvm_msr_entry_add(cpu, MSR_IA32_RTIT_STATUS,
+ env->msr_rtit_status);
+ kvm_msr_entry_add(cpu, MSR_IA32_RTIT_OUTPUT_BASE,
+ env->msr_rtit_output_base);
+ kvm_msr_entry_add(cpu, MSR_IA32_RTIT_OUTPUT_MASK,
+ env->msr_rtit_output_mask);
+ kvm_msr_entry_add(cpu, MSR_IA32_RTIT_CR3_MATCH,
+ env->msr_rtit_cr3_match);
+ for (i = 0; i < addr_num; i++) {
+ kvm_msr_entry_add(cpu, MSR_IA32_RTIT_ADDR0_A + i,
+ env->msr_rtit_addrs[i]);
+ }
+ }
/* Note: MSR_IA32_FEATURE_CONTROL is written separately, see
* kvm_put_msr_feature_control. */
@@ -2124,6 +2143,20 @@ static int kvm_get_msrs(X86CPU *cpu)
}
}
+ if (env->features[FEAT_7_0_EBX] & CPUID_7_0_EBX_INTEL_PT) {
+ int addr_num =
+ kvm_arch_get_supported_cpuid(kvm_state, 0x14, 1, R_EAX) & 0x7;
+
+ kvm_msr_entry_add(cpu, MSR_IA32_RTIT_CTL, 0);
+ kvm_msr_entry_add(cpu, MSR_IA32_RTIT_STATUS, 0);
+ kvm_msr_entry_add(cpu, MSR_IA32_RTIT_OUTPUT_BASE, 0);
+ kvm_msr_entry_add(cpu, MSR_IA32_RTIT_OUTPUT_MASK, 0);
+ kvm_msr_entry_add(cpu, MSR_IA32_RTIT_CR3_MATCH, 0);
+ for (i = 0; i < addr_num; i++) {
+ kvm_msr_entry_add(cpu, MSR_IA32_RTIT_ADDR0_A + i, 0);
+ }
+ }
+
ret = kvm_vcpu_ioctl(CPU(cpu), KVM_GET_MSRS, cpu->kvm_msr_buf);
if (ret < 0) {
return ret;
@@ -2364,6 +2397,24 @@ static int kvm_get_msrs(X86CPU *cpu)
case MSR_IA32_SPEC_CTRL:
env->spec_ctrl = msrs[i].data;
break;
+ case MSR_IA32_RTIT_CTL:
+ env->msr_rtit_ctrl = msrs[i].data;
+ break;
+ case MSR_IA32_RTIT_STATUS:
+ env->msr_rtit_status = msrs[i].data;
+ break;
+ case MSR_IA32_RTIT_OUTPUT_BASE:
+ env->msr_rtit_output_base = msrs[i].data;
+ break;
+ case MSR_IA32_RTIT_OUTPUT_MASK:
+ env->msr_rtit_output_mask = msrs[i].data;
+ break;
+ case MSR_IA32_RTIT_CR3_MATCH:
+ env->msr_rtit_cr3_match = msrs[i].data;
+ break;
+ case MSR_IA32_RTIT_ADDR0_A ... MSR_IA32_RTIT_ADDR3_B:
+ env->msr_rtit_addrs[index - MSR_IA32_RTIT_ADDR0_A] = msrs[i].data;
+ break;
}
}
diff --git a/target/i386/machine.c b/target/i386/machine.c
index 361c05a..c05fe6f 100644
--- a/target/i386/machine.c
+++ b/target/i386/machine.c
@@ -837,6 +837,43 @@ static const VMStateDescription vmstate_spec_ctrl = {
}
};
+static bool intel_pt_enable_needed(void *opaque)
+{
+ X86CPU *cpu = opaque;
+ CPUX86State *env = &cpu->env;
+ int i;
+
+ if (env->msr_rtit_ctrl || env->msr_rtit_status ||
+ env->msr_rtit_output_base || env->msr_rtit_output_mask ||
+ env->msr_rtit_cr3_match) {
+ return true;
+ }
+
+ for (i = 0; i < MAX_RTIT_ADDRS; i++) {
+ if (env->msr_rtit_addrs[i]) {
+ return true;
+ }
+ }
+
+ return false;
+}
+
+static const VMStateDescription vmstate_msr_intel_pt = {
+ .name = "cpu/intel_pt",
+ .version_id = 1,
+ .minimum_version_id = 1,
+ .needed = intel_pt_enable_needed,
+ .fields = (VMStateField[]) {
+ VMSTATE_UINT64(env.msr_rtit_ctrl, X86CPU),
+ VMSTATE_UINT64(env.msr_rtit_status, X86CPU),
+ VMSTATE_UINT64(env.msr_rtit_output_base, X86CPU),
+ VMSTATE_UINT64(env.msr_rtit_output_mask, X86CPU),
+ VMSTATE_UINT64(env.msr_rtit_cr3_match, X86CPU),
+ VMSTATE_UINT64_ARRAY(env.msr_rtit_addrs, X86CPU, MAX_RTIT_ADDRS),
+ VMSTATE_END_OF_LIST()
+ }
+};
+
VMStateDescription vmstate_x86_cpu = {
.name = "cpu",
.version_id = 12,
@@ -957,6 +994,7 @@ VMStateDescription vmstate_x86_cpu = {
#endif
&vmstate_spec_ctrl,
&vmstate_mcg_ext_ctl,
+ &vmstate_msr_intel_pt,
NULL
}
};
--
1.8.3.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [Qemu-devel] [PATCH v2 2/2] i386: Add support to get/set/migrate Intel Processor Trace feature
@ 2018-01-25 18:25 ` Luwei Kang
0 siblings, 0 replies; 10+ messages in thread
From: Luwei Kang @ 2018-01-25 18:25 UTC (permalink / raw)
To: qemu-devel, kvm; +Cc: pbonzini, rth, ehabkost, mtosatti, Chao Peng, Luwei Kang
From: Chao Peng <chao.p.peng@linux.intel.com>
Add Intel Processor Trace related definition. It also add
corresponding part to kvm_get/set_msr and vmstate.
Signed-off-by: Luwei Kang <luwei.kang@intel.com>
Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>
---
target/i386/cpu.h | 22 ++++++++++++++++++++++
target/i386/kvm.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++
target/i386/machine.c | 38 ++++++++++++++++++++++++++++++++++++++
3 files changed, 111 insertions(+)
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 53908ff..85f0a04 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -419,6 +419,21 @@ typedef enum X86Seg {
#define MSR_MC0_ADDR 0x402
#define MSR_MC0_MISC 0x403
+#define MSR_IA32_RTIT_OUTPUT_BASE 0x560
+#define MSR_IA32_RTIT_OUTPUT_MASK 0x561
+#define MSR_IA32_RTIT_CTL 0x570
+#define MSR_IA32_RTIT_STATUS 0x571
+#define MSR_IA32_RTIT_CR3_MATCH 0x572
+#define MSR_IA32_RTIT_ADDR0_A 0x580
+#define MSR_IA32_RTIT_ADDR0_B 0x581
+#define MSR_IA32_RTIT_ADDR1_A 0x582
+#define MSR_IA32_RTIT_ADDR1_B 0x583
+#define MSR_IA32_RTIT_ADDR2_A 0x584
+#define MSR_IA32_RTIT_ADDR2_B 0x585
+#define MSR_IA32_RTIT_ADDR3_A 0x586
+#define MSR_IA32_RTIT_ADDR3_B 0x587
+#define MAX_RTIT_ADDRS 8
+
#define MSR_EFER 0xc0000080
#define MSR_EFER_SCE (1 << 0)
@@ -1163,6 +1178,13 @@ typedef struct CPUX86State {
uint64_t msr_hv_stimer_config[HV_STIMER_COUNT];
uint64_t msr_hv_stimer_count[HV_STIMER_COUNT];
+ uint64_t msr_rtit_ctrl;
+ uint64_t msr_rtit_status;
+ uint64_t msr_rtit_output_base;
+ uint64_t msr_rtit_output_mask;
+ uint64_t msr_rtit_cr3_match;
+ uint64_t msr_rtit_addrs[MAX_RTIT_ADDRS];
+
/* exception/interrupt handling */
int error_code;
int exception_is_int;
diff --git a/target/i386/kvm.c b/target/i386/kvm.c
index f9f4cd1..097c953 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -1811,6 +1811,25 @@ static int kvm_put_msrs(X86CPU *cpu, int level)
kvm_msr_entry_add(cpu, MSR_MTRRphysMask(i), mask);
}
}
+ if (env->features[FEAT_7_0_EBX] & CPUID_7_0_EBX_INTEL_PT) {
+ int addr_num = kvm_arch_get_supported_cpuid(kvm_state,
+ 0x14, 1, R_EAX) & 0x7;
+
+ kvm_msr_entry_add(cpu, MSR_IA32_RTIT_CTL,
+ env->msr_rtit_ctrl);
+ kvm_msr_entry_add(cpu, MSR_IA32_RTIT_STATUS,
+ env->msr_rtit_status);
+ kvm_msr_entry_add(cpu, MSR_IA32_RTIT_OUTPUT_BASE,
+ env->msr_rtit_output_base);
+ kvm_msr_entry_add(cpu, MSR_IA32_RTIT_OUTPUT_MASK,
+ env->msr_rtit_output_mask);
+ kvm_msr_entry_add(cpu, MSR_IA32_RTIT_CR3_MATCH,
+ env->msr_rtit_cr3_match);
+ for (i = 0; i < addr_num; i++) {
+ kvm_msr_entry_add(cpu, MSR_IA32_RTIT_ADDR0_A + i,
+ env->msr_rtit_addrs[i]);
+ }
+ }
/* Note: MSR_IA32_FEATURE_CONTROL is written separately, see
* kvm_put_msr_feature_control. */
@@ -2124,6 +2143,20 @@ static int kvm_get_msrs(X86CPU *cpu)
}
}
+ if (env->features[FEAT_7_0_EBX] & CPUID_7_0_EBX_INTEL_PT) {
+ int addr_num =
+ kvm_arch_get_supported_cpuid(kvm_state, 0x14, 1, R_EAX) & 0x7;
+
+ kvm_msr_entry_add(cpu, MSR_IA32_RTIT_CTL, 0);
+ kvm_msr_entry_add(cpu, MSR_IA32_RTIT_STATUS, 0);
+ kvm_msr_entry_add(cpu, MSR_IA32_RTIT_OUTPUT_BASE, 0);
+ kvm_msr_entry_add(cpu, MSR_IA32_RTIT_OUTPUT_MASK, 0);
+ kvm_msr_entry_add(cpu, MSR_IA32_RTIT_CR3_MATCH, 0);
+ for (i = 0; i < addr_num; i++) {
+ kvm_msr_entry_add(cpu, MSR_IA32_RTIT_ADDR0_A + i, 0);
+ }
+ }
+
ret = kvm_vcpu_ioctl(CPU(cpu), KVM_GET_MSRS, cpu->kvm_msr_buf);
if (ret < 0) {
return ret;
@@ -2364,6 +2397,24 @@ static int kvm_get_msrs(X86CPU *cpu)
case MSR_IA32_SPEC_CTRL:
env->spec_ctrl = msrs[i].data;
break;
+ case MSR_IA32_RTIT_CTL:
+ env->msr_rtit_ctrl = msrs[i].data;
+ break;
+ case MSR_IA32_RTIT_STATUS:
+ env->msr_rtit_status = msrs[i].data;
+ break;
+ case MSR_IA32_RTIT_OUTPUT_BASE:
+ env->msr_rtit_output_base = msrs[i].data;
+ break;
+ case MSR_IA32_RTIT_OUTPUT_MASK:
+ env->msr_rtit_output_mask = msrs[i].data;
+ break;
+ case MSR_IA32_RTIT_CR3_MATCH:
+ env->msr_rtit_cr3_match = msrs[i].data;
+ break;
+ case MSR_IA32_RTIT_ADDR0_A ... MSR_IA32_RTIT_ADDR3_B:
+ env->msr_rtit_addrs[index - MSR_IA32_RTIT_ADDR0_A] = msrs[i].data;
+ break;
}
}
diff --git a/target/i386/machine.c b/target/i386/machine.c
index 361c05a..c05fe6f 100644
--- a/target/i386/machine.c
+++ b/target/i386/machine.c
@@ -837,6 +837,43 @@ static const VMStateDescription vmstate_spec_ctrl = {
}
};
+static bool intel_pt_enable_needed(void *opaque)
+{
+ X86CPU *cpu = opaque;
+ CPUX86State *env = &cpu->env;
+ int i;
+
+ if (env->msr_rtit_ctrl || env->msr_rtit_status ||
+ env->msr_rtit_output_base || env->msr_rtit_output_mask ||
+ env->msr_rtit_cr3_match) {
+ return true;
+ }
+
+ for (i = 0; i < MAX_RTIT_ADDRS; i++) {
+ if (env->msr_rtit_addrs[i]) {
+ return true;
+ }
+ }
+
+ return false;
+}
+
+static const VMStateDescription vmstate_msr_intel_pt = {
+ .name = "cpu/intel_pt",
+ .version_id = 1,
+ .minimum_version_id = 1,
+ .needed = intel_pt_enable_needed,
+ .fields = (VMStateField[]) {
+ VMSTATE_UINT64(env.msr_rtit_ctrl, X86CPU),
+ VMSTATE_UINT64(env.msr_rtit_status, X86CPU),
+ VMSTATE_UINT64(env.msr_rtit_output_base, X86CPU),
+ VMSTATE_UINT64(env.msr_rtit_output_mask, X86CPU),
+ VMSTATE_UINT64(env.msr_rtit_cr3_match, X86CPU),
+ VMSTATE_UINT64_ARRAY(env.msr_rtit_addrs, X86CPU, MAX_RTIT_ADDRS),
+ VMSTATE_END_OF_LIST()
+ }
+};
+
VMStateDescription vmstate_x86_cpu = {
.name = "cpu",
.version_id = 12,
@@ -957,6 +994,7 @@ VMStateDescription vmstate_x86_cpu = {
#endif
&vmstate_spec_ctrl,
&vmstate_mcg_ext_ctl,
+ &vmstate_msr_intel_pt,
NULL
}
};
--
1.8.3.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2 1/2] i386: Add Intel Processor Trace feature support
2018-01-25 18:25 ` [Qemu-devel] " Luwei Kang
@ 2018-01-26 19:14 ` Eduardo Habkost
-1 siblings, 0 replies; 10+ messages in thread
From: Eduardo Habkost @ 2018-01-26 19:14 UTC (permalink / raw)
To: Luwei Kang; +Cc: qemu-devel, kvm, pbonzini, rth, mtosatti, Chao Peng
On Fri, Jan 26, 2018 at 02:25:50AM +0800, Luwei Kang wrote:
> From: Chao Peng <chao.p.peng@linux.intel.com>
>
> Expose Intel Processor Trace feature to guest.
>
> In order to make this feature migration-safe, new feature word
> information "FEAT_INTEL_PT_EBX" and "FEAT_INTEL_PT_ECX" be added.
> Some constant value initialized in CPUID[0x14].0x01 to guarantee
> get same result in diffrent hardware when this feature is enabled.
>
> Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>
> Signed-off-by: Luwei Kang <luwei.kang@intel.com>
> ---
> v1->v2:
> - In order to make this feature migration-safe, new feature word
> information "FEAT_INTEL_PT_EBX" and "FEAT_INTEL_PT_ECX" be added.
> Some constant value initialized in CPUID[0x14].0x01 to guarantee
> get same result in diffrent hardware when this feature is enabled.
>
> ---
> target/i386/cpu.c | 68 +++++++++++++++++++++++++++++++++++++++++++++++++++++--
> target/i386/cpu.h | 6 +++++
> target/i386/kvm.c | 23 +++++++++++++++++++
> 3 files changed, 95 insertions(+), 2 deletions(-)
>
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index a49d222..ee8c6e6 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -172,7 +172,11 @@
> #define L2_ITLB_4K_ASSOC 4
> #define L2_ITLB_4K_ENTRIES 512
>
> -
> +/* CPUID Leaf 0x14 constants: */
> +#define INTLE_PT_ADDR_RANGES_NUM 2
> +#define INTEL_PT_MTC_BITMAP (0x0249 << 16) /* Support ART(0,3,6,9) */
> +#define INTEL_PT_CYCLE_BITMAP 0x3fff /* Support 0,2^(0~12) */
> +#define INTEL_PT_PSB_BITMAP (0x003f << 16) /* Support 2K,4K,8K,16K,32K,64K */
>
> static void x86_cpu_vendor_words2str(char *dst, uint32_t vendor1,
> uint32_t vendor2, uint32_t vendor3)
> @@ -427,7 +431,7 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
> NULL, NULL, "mpx", NULL,
> "avx512f", "avx512dq", "rdseed", "adx",
> "smap", "avx512ifma", "pcommit", "clflushopt",
> - "clwb", NULL, "avx512pf", "avx512er",
> + "clwb", "intel-pt", "avx512pf", "avx512er",
> "avx512cd", "sha-ni", "avx512bw", "avx512vl",
> },
> .cpuid_eax = 7,
> @@ -545,6 +549,38 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
> .cpuid_reg = R_EDX,
> .tcg_features = ~0U,
> },
> + [FEAT_INTEL_PT_EBX] = {
> + .feat_names = {
> + "cr3-filter", "psb-cycle", "filtering", "mtc-cofi",
> + "ptwrite", "power-evt", NULL, NULL,
Note that this will make all those bits configurable on the
command-line. You don't really need to do this in the first
version, if you don't really want to.
Also, if we want to make this configurable, we will need to
choose carefully the property names, because changing them later
would be more difficult.
Note that the feature names live in a single QOM property
namespace, so "filtering", "any-entries" and "transport" are
probably too generic. Maybe we could prefix all names here with
"intel-pt-" or "pt-"?
If you don't want to make all bits configurable on the first
version, you can fill the CPUID[14h] bits with constant default
values just like you did with the INTEL_PT_* constants below.
> + NULL, NULL, NULL, NULL,
> + NULL, NULL, NULL, NULL,
> + NULL, NULL, NULL, NULL,
> + NULL, NULL, NULL, NULL,
> + NULL, NULL, NULL, NULL,
> + NULL, NULL, NULL, NULL,
> + },
> + .cpuid_eax = 0x14,
> + .cpuid_needs_ecx = true, .cpuid_ecx = 0,
> + .cpuid_reg = R_EBX,
> + .tcg_features = ~0U,
TCG doesn't implement this feature, so ~0U doesn't look right.
> + },
> + [FEAT_INTEL_PT_ECX] = {
> + .feat_names = {
> + "topa-output", "any-entries", "single-range", "transport",
> + 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,
> + },
> + .cpuid_eax = 0x14,
> + .cpuid_needs_ecx = true, .cpuid_ecx = 0,
> + .cpuid_reg = R_ECX,
> + .tcg_features = ~0U,
Same as above.
> + },
> };
>
> typedef struct X86RegisterInfo32 {
> @@ -3452,6 +3488,34 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
> }
> break;
> }
> + case 0x14: {
> + /* Intel Processor Trace Enumeration */
> + *eax = 0;
> + *ebx = 0;
> + *ecx = 0;
> + *edx = 0;
> + if (!(env->features[FEAT_7_0_EBX] & CPUID_7_0_EBX_INTEL_PT) ||
> + !kvm_enabled()) {
> + break;
> + }
> +
> + if (count == 0) {
> + *eax = 1;
> + *ebx = env->features[FEAT_INTEL_PT_EBX];
> + *ecx = env->features[FEAT_INTEL_PT_ECX];
> + } else if (count == 1) {
> + *eax = INTLE_PT_ADDR_RANGES_NUM;
> + if (env->features[FEAT_INTEL_PT_EBX] &
> + CPUID_INTEL_PT_EBX_MTC_COFI) {
> + *eax |= INTEL_PT_MTC_BITMAP;
> + }
> + if (env->features[FEAT_INTEL_PT_EBX] &
> + CPUID_INTEL_PT_EBX_PSB_CYCLE) {
> + *ebx = INTEL_PT_PSB_BITMAP | INTEL_PT_CYCLE_BITMAP;
> + }
We still need to validate the bitmaps and number of ranges
against the host capabilities (reported on GET_SUPPORTED_CPUID),
don't we?
If you are going to set CPUID bits that are not already present
on env->features[], you will want x86_cpu_filter_features() to
manually validate the constants against
x86_cpu_get_supported_feature_word(), to ensure we won't try to
enable unsupported bits.
(If doing that, we need to make sure CPUID_7_0_EBX_INTEL_PT will
be set on xc->filtered_features[FEAT_7_0_EBX] if something is
unsupported, to tell the calling code that intel-pt can't be
enabled on the current host)
In the future we could extend FeatureWordInfo to make it easier
to handle counter/bitmap fields like those, then we won't need
special cases inside x86_cpu_filter_features().
> + }
> + break;
> + }
> case 0x40000000:
> /*
> * CPUID code in kvm_arch_init_vcpu() ignores stuff
> diff --git a/target/i386/cpu.h b/target/i386/cpu.h
> index 30cc562..53908ff 100644
> --- a/target/i386/cpu.h
> +++ b/target/i386/cpu.h
> @@ -483,6 +483,8 @@ typedef enum FeatureWord {
> FEAT_6_EAX, /* CPUID[6].EAX */
> FEAT_XSAVE_COMP_LO, /* CPUID[EAX=0xd,ECX=0].EAX */
> FEAT_XSAVE_COMP_HI, /* CPUID[EAX=0xd,ECX=0].EDX */
> + FEAT_INTEL_PT_EBX, /* CPUID[EAX=0x14,ECX=0].EBX */
> + FEAT_INTEL_PT_ECX, /* CPUID[EAX=0x14,ECX=0].ECX */
> FEATURE_WORDS,
> } FeatureWord;
>
> @@ -644,6 +646,7 @@ typedef uint32_t FeatureWordArray[FEATURE_WORDS];
> #define CPUID_7_0_EBX_PCOMMIT (1U << 22) /* Persistent Commit */
> #define CPUID_7_0_EBX_CLFLUSHOPT (1U << 23) /* Flush a Cache Line Optimized */
> #define CPUID_7_0_EBX_CLWB (1U << 24) /* Cache Line Write Back */
> +#define CPUID_7_0_EBX_INTEL_PT (1U << 25) /* Intel Processor Trace */
> #define CPUID_7_0_EBX_AVX512PF (1U << 26) /* AVX-512 Prefetch */
> #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 */
> @@ -677,6 +680,9 @@ typedef uint32_t FeatureWordArray[FEATURE_WORDS];
> #define CPUID_XSAVE_XGETBV1 (1U << 2)
> #define CPUID_XSAVE_XSAVES (1U << 3)
>
> +#define CPUID_INTEL_PT_EBX_PSB_CYCLE (1U << 1) /* Support configurable PSB and Cycle-Accurate Mode */
> +#define CPUID_INTEL_PT_EBX_MTC_COFI (1U << 3) /* Support MTC timing and suppression of COFI-based packets */
> +
> #define CPUID_6_EAX_ARAT (1U << 2)
>
> /* CPUID[0x80000007].EDX flags: */
> diff --git a/target/i386/kvm.c b/target/i386/kvm.c
> index ad4b159..f9f4cd1 100644
> --- a/target/i386/kvm.c
> +++ b/target/i386/kvm.c
> @@ -865,6 +865,29 @@ int kvm_arch_init_vcpu(CPUState *cs)
> c = &cpuid_data.entries[cpuid_i++];
> }
> break;
> + case 0x14: {
> + uint32_t times;
> +
> + c->function = i;
> + c->index = 0;
> + c->flags = KVM_CPUID_FLAG_SIGNIFCANT_INDEX;
> + cpu_x86_cpuid(env, i, 0, &c->eax, &c->ebx, &c->ecx, &c->edx);
> + times = c->eax;
> +
> + for (j = 1; j <= times; ++j) {
> + if (cpuid_i == KVM_MAX_CPUID_ENTRIES) {
> + fprintf(stderr, "cpuid_data is full, no space for "
> + "cpuid(eax:0x14,ecx:0x%x)\n", j);
> + abort();
> + }
> + c = &cpuid_data.entries[cpuid_i++];
> + c->function = i;
> + c->index = j;
> + c->flags = KVM_CPUID_FLAG_SIGNIFCANT_INDEX;
> + cpu_x86_cpuid(env, i, j, &c->eax, &c->ebx, &c->ecx, &c->edx);
> + }
> + break;
> + }
> default:
> c->function = i;
> c->flags = 0;
> --
> 1.8.3.1
>
--
Eduardo
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/2] i386: Add Intel Processor Trace feature support
@ 2018-01-26 19:14 ` Eduardo Habkost
0 siblings, 0 replies; 10+ messages in thread
From: Eduardo Habkost @ 2018-01-26 19:14 UTC (permalink / raw)
To: Luwei Kang; +Cc: qemu-devel, kvm, pbonzini, rth, mtosatti, Chao Peng
On Fri, Jan 26, 2018 at 02:25:50AM +0800, Luwei Kang wrote:
> From: Chao Peng <chao.p.peng@linux.intel.com>
>
> Expose Intel Processor Trace feature to guest.
>
> In order to make this feature migration-safe, new feature word
> information "FEAT_INTEL_PT_EBX" and "FEAT_INTEL_PT_ECX" be added.
> Some constant value initialized in CPUID[0x14].0x01 to guarantee
> get same result in diffrent hardware when this feature is enabled.
>
> Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>
> Signed-off-by: Luwei Kang <luwei.kang@intel.com>
> ---
> v1->v2:
> - In order to make this feature migration-safe, new feature word
> information "FEAT_INTEL_PT_EBX" and "FEAT_INTEL_PT_ECX" be added.
> Some constant value initialized in CPUID[0x14].0x01 to guarantee
> get same result in diffrent hardware when this feature is enabled.
>
> ---
> target/i386/cpu.c | 68 +++++++++++++++++++++++++++++++++++++++++++++++++++++--
> target/i386/cpu.h | 6 +++++
> target/i386/kvm.c | 23 +++++++++++++++++++
> 3 files changed, 95 insertions(+), 2 deletions(-)
>
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index a49d222..ee8c6e6 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -172,7 +172,11 @@
> #define L2_ITLB_4K_ASSOC 4
> #define L2_ITLB_4K_ENTRIES 512
>
> -
> +/* CPUID Leaf 0x14 constants: */
> +#define INTLE_PT_ADDR_RANGES_NUM 2
> +#define INTEL_PT_MTC_BITMAP (0x0249 << 16) /* Support ART(0,3,6,9) */
> +#define INTEL_PT_CYCLE_BITMAP 0x3fff /* Support 0,2^(0~12) */
> +#define INTEL_PT_PSB_BITMAP (0x003f << 16) /* Support 2K,4K,8K,16K,32K,64K */
>
> static void x86_cpu_vendor_words2str(char *dst, uint32_t vendor1,
> uint32_t vendor2, uint32_t vendor3)
> @@ -427,7 +431,7 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
> NULL, NULL, "mpx", NULL,
> "avx512f", "avx512dq", "rdseed", "adx",
> "smap", "avx512ifma", "pcommit", "clflushopt",
> - "clwb", NULL, "avx512pf", "avx512er",
> + "clwb", "intel-pt", "avx512pf", "avx512er",
> "avx512cd", "sha-ni", "avx512bw", "avx512vl",
> },
> .cpuid_eax = 7,
> @@ -545,6 +549,38 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
> .cpuid_reg = R_EDX,
> .tcg_features = ~0U,
> },
> + [FEAT_INTEL_PT_EBX] = {
> + .feat_names = {
> + "cr3-filter", "psb-cycle", "filtering", "mtc-cofi",
> + "ptwrite", "power-evt", NULL, NULL,
Note that this will make all those bits configurable on the
command-line. You don't really need to do this in the first
version, if you don't really want to.
Also, if we want to make this configurable, we will need to
choose carefully the property names, because changing them later
would be more difficult.
Note that the feature names live in a single QOM property
namespace, so "filtering", "any-entries" and "transport" are
probably too generic. Maybe we could prefix all names here with
"intel-pt-" or "pt-"?
If you don't want to make all bits configurable on the first
version, you can fill the CPUID[14h] bits with constant default
values just like you did with the INTEL_PT_* constants below.
> + NULL, NULL, NULL, NULL,
> + NULL, NULL, NULL, NULL,
> + NULL, NULL, NULL, NULL,
> + NULL, NULL, NULL, NULL,
> + NULL, NULL, NULL, NULL,
> + NULL, NULL, NULL, NULL,
> + },
> + .cpuid_eax = 0x14,
> + .cpuid_needs_ecx = true, .cpuid_ecx = 0,
> + .cpuid_reg = R_EBX,
> + .tcg_features = ~0U,
TCG doesn't implement this feature, so ~0U doesn't look right.
> + },
> + [FEAT_INTEL_PT_ECX] = {
> + .feat_names = {
> + "topa-output", "any-entries", "single-range", "transport",
> + 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,
> + },
> + .cpuid_eax = 0x14,
> + .cpuid_needs_ecx = true, .cpuid_ecx = 0,
> + .cpuid_reg = R_ECX,
> + .tcg_features = ~0U,
Same as above.
> + },
> };
>
> typedef struct X86RegisterInfo32 {
> @@ -3452,6 +3488,34 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
> }
> break;
> }
> + case 0x14: {
> + /* Intel Processor Trace Enumeration */
> + *eax = 0;
> + *ebx = 0;
> + *ecx = 0;
> + *edx = 0;
> + if (!(env->features[FEAT_7_0_EBX] & CPUID_7_0_EBX_INTEL_PT) ||
> + !kvm_enabled()) {
> + break;
> + }
> +
> + if (count == 0) {
> + *eax = 1;
> + *ebx = env->features[FEAT_INTEL_PT_EBX];
> + *ecx = env->features[FEAT_INTEL_PT_ECX];
> + } else if (count == 1) {
> + *eax = INTLE_PT_ADDR_RANGES_NUM;
> + if (env->features[FEAT_INTEL_PT_EBX] &
> + CPUID_INTEL_PT_EBX_MTC_COFI) {
> + *eax |= INTEL_PT_MTC_BITMAP;
> + }
> + if (env->features[FEAT_INTEL_PT_EBX] &
> + CPUID_INTEL_PT_EBX_PSB_CYCLE) {
> + *ebx = INTEL_PT_PSB_BITMAP | INTEL_PT_CYCLE_BITMAP;
> + }
We still need to validate the bitmaps and number of ranges
against the host capabilities (reported on GET_SUPPORTED_CPUID),
don't we?
If you are going to set CPUID bits that are not already present
on env->features[], you will want x86_cpu_filter_features() to
manually validate the constants against
x86_cpu_get_supported_feature_word(), to ensure we won't try to
enable unsupported bits.
(If doing that, we need to make sure CPUID_7_0_EBX_INTEL_PT will
be set on xc->filtered_features[FEAT_7_0_EBX] if something is
unsupported, to tell the calling code that intel-pt can't be
enabled on the current host)
In the future we could extend FeatureWordInfo to make it easier
to handle counter/bitmap fields like those, then we won't need
special cases inside x86_cpu_filter_features().
> + }
> + break;
> + }
> case 0x40000000:
> /*
> * CPUID code in kvm_arch_init_vcpu() ignores stuff
> diff --git a/target/i386/cpu.h b/target/i386/cpu.h
> index 30cc562..53908ff 100644
> --- a/target/i386/cpu.h
> +++ b/target/i386/cpu.h
> @@ -483,6 +483,8 @@ typedef enum FeatureWord {
> FEAT_6_EAX, /* CPUID[6].EAX */
> FEAT_XSAVE_COMP_LO, /* CPUID[EAX=0xd,ECX=0].EAX */
> FEAT_XSAVE_COMP_HI, /* CPUID[EAX=0xd,ECX=0].EDX */
> + FEAT_INTEL_PT_EBX, /* CPUID[EAX=0x14,ECX=0].EBX */
> + FEAT_INTEL_PT_ECX, /* CPUID[EAX=0x14,ECX=0].ECX */
> FEATURE_WORDS,
> } FeatureWord;
>
> @@ -644,6 +646,7 @@ typedef uint32_t FeatureWordArray[FEATURE_WORDS];
> #define CPUID_7_0_EBX_PCOMMIT (1U << 22) /* Persistent Commit */
> #define CPUID_7_0_EBX_CLFLUSHOPT (1U << 23) /* Flush a Cache Line Optimized */
> #define CPUID_7_0_EBX_CLWB (1U << 24) /* Cache Line Write Back */
> +#define CPUID_7_0_EBX_INTEL_PT (1U << 25) /* Intel Processor Trace */
> #define CPUID_7_0_EBX_AVX512PF (1U << 26) /* AVX-512 Prefetch */
> #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 */
> @@ -677,6 +680,9 @@ typedef uint32_t FeatureWordArray[FEATURE_WORDS];
> #define CPUID_XSAVE_XGETBV1 (1U << 2)
> #define CPUID_XSAVE_XSAVES (1U << 3)
>
> +#define CPUID_INTEL_PT_EBX_PSB_CYCLE (1U << 1) /* Support configurable PSB and Cycle-Accurate Mode */
> +#define CPUID_INTEL_PT_EBX_MTC_COFI (1U << 3) /* Support MTC timing and suppression of COFI-based packets */
> +
> #define CPUID_6_EAX_ARAT (1U << 2)
>
> /* CPUID[0x80000007].EDX flags: */
> diff --git a/target/i386/kvm.c b/target/i386/kvm.c
> index ad4b159..f9f4cd1 100644
> --- a/target/i386/kvm.c
> +++ b/target/i386/kvm.c
> @@ -865,6 +865,29 @@ int kvm_arch_init_vcpu(CPUState *cs)
> c = &cpuid_data.entries[cpuid_i++];
> }
> break;
> + case 0x14: {
> + uint32_t times;
> +
> + c->function = i;
> + c->index = 0;
> + c->flags = KVM_CPUID_FLAG_SIGNIFCANT_INDEX;
> + cpu_x86_cpuid(env, i, 0, &c->eax, &c->ebx, &c->ecx, &c->edx);
> + times = c->eax;
> +
> + for (j = 1; j <= times; ++j) {
> + if (cpuid_i == KVM_MAX_CPUID_ENTRIES) {
> + fprintf(stderr, "cpuid_data is full, no space for "
> + "cpuid(eax:0x14,ecx:0x%x)\n", j);
> + abort();
> + }
> + c = &cpuid_data.entries[cpuid_i++];
> + c->function = i;
> + c->index = j;
> + c->flags = KVM_CPUID_FLAG_SIGNIFCANT_INDEX;
> + cpu_x86_cpuid(env, i, j, &c->eax, &c->ebx, &c->ecx, &c->edx);
> + }
> + break;
> + }
> default:
> c->function = i;
> c->flags = 0;
> --
> 1.8.3.1
>
--
Eduardo
^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH v2 1/2] i386: Add Intel Processor Trace feature support
2018-01-26 19:14 ` [Qemu-devel] " Eduardo Habkost
@ 2018-01-29 7:03 ` Kang, Luwei
-1 siblings, 0 replies; 10+ messages in thread
From: Kang, Luwei @ 2018-01-29 7:03 UTC (permalink / raw)
To: Eduardo Habkost; +Cc: qemu-devel, kvm, pbonzini, rth, mtosatti, Chao Peng
> > From: Chao Peng <chao.p.peng@linux.intel.com>
> >
> > Expose Intel Processor Trace feature to guest.
> >
> > In order to make this feature migration-safe, new feature word
> > information "FEAT_INTEL_PT_EBX" and "FEAT_INTEL_PT_ECX" be added.
> > Some constant value initialized in CPUID[0x14].0x01 to guarantee get
> > same result in diffrent hardware when this feature is enabled.
> >
> > Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>
> > Signed-off-by: Luwei Kang <luwei.kang@intel.com>
> > ---
> > v1->v2:
> > - In order to make this feature migration-safe, new feature word
> > information "FEAT_INTEL_PT_EBX" and "FEAT_INTEL_PT_ECX" be added.
> > Some constant value initialized in CPUID[0x14].0x01 to guarantee
> > get same result in diffrent hardware when this feature is enabled.
> >
> > ---
> > target/i386/cpu.c | 68
> > +++++++++++++++++++++++++++++++++++++++++++++++++++++--
> > target/i386/cpu.h | 6 +++++
> > target/i386/kvm.c | 23 +++++++++++++++++++
> > 3 files changed, 95 insertions(+), 2 deletions(-)
> >
> > diff --git a/target/i386/cpu.c b/target/i386/cpu.c index
> > a49d222..ee8c6e6 100644
> > --- a/target/i386/cpu.c
> > +++ b/target/i386/cpu.c
> > @@ -172,7 +172,11 @@
> > #define L2_ITLB_4K_ASSOC 4
> > #define L2_ITLB_4K_ENTRIES 512
> >
> > -
> > +/* CPUID Leaf 0x14 constants: */
> > +#define INTLE_PT_ADDR_RANGES_NUM 2
> > +#define INTEL_PT_MTC_BITMAP (0x0249 << 16) /* Support ART(0,3,6,9) */
> > +#define INTEL_PT_CYCLE_BITMAP 0x3fff /* Support 0,2^(0~12) */
> > +#define INTEL_PT_PSB_BITMAP (0x003f << 16) /* Support 2K,4K,8K,16K,32K,64K */
> >
> > static void x86_cpu_vendor_words2str(char *dst, uint32_t vendor1,
> > uint32_t vendor2, uint32_t
> > vendor3) @@ -427,7 +431,7 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
> > NULL, NULL, "mpx", NULL,
> > "avx512f", "avx512dq", "rdseed", "adx",
> > "smap", "avx512ifma", "pcommit", "clflushopt",
> > - "clwb", NULL, "avx512pf", "avx512er",
> > + "clwb", "intel-pt", "avx512pf", "avx512er",
> > "avx512cd", "sha-ni", "avx512bw", "avx512vl",
> > },
> > .cpuid_eax = 7,
> > @@ -545,6 +549,38 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
> > .cpuid_reg = R_EDX,
> > .tcg_features = ~0U,
> > },
> > + [FEAT_INTEL_PT_EBX] = {
> > + .feat_names = {
> > + "cr3-filter", "psb-cycle", "filtering", "mtc-cofi",
> > + "ptwrite", "power-evt", NULL, NULL,
>
> Note that this will make all those bits configurable on the command-line. You don't really need to do this in the first version, if you
> don't really want to.
>
> Also, if we want to make this configurable, we will need to choose carefully the property names, because changing them later would
> be more difficult.
>
> Note that the feature names live in a single QOM property namespace, so "filtering", "any-entries" and "transport" are probably too
> generic. Maybe we could prefix all names here with "intel-pt-" or "pt-"?
>
> If you don't want to make all bits configurable on the first version, you can fill the CPUID[14h] bits with constant default values just
> like you did with the INTEL_PT_* constants below.
>
Agree, I think it is too complex to make all Intel PT sub-feature configurable in command line. We also need make some short and clear enough property names for each sub-feature.
>
> > + NULL, NULL, NULL, NULL,
> > + NULL, NULL, NULL, NULL,
> > + NULL, NULL, NULL, NULL,
> > + NULL, NULL, NULL, NULL,
> > + NULL, NULL, NULL, NULL,
> > + NULL, NULL, NULL, NULL,
> > + },
> > + .cpuid_eax = 0x14,
> > + .cpuid_needs_ecx = true, .cpuid_ecx = 0,
> > + .cpuid_reg = R_EBX,
> > + .tcg_features = ~0U,
>
> TCG doesn't implement this feature, so ~0U doesn't look right.
>
>
> > + },
> > + [FEAT_INTEL_PT_ECX] = {
> > + .feat_names = {
> > + "topa-output", "any-entries", "single-range", "transport",
> > + 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,
> > + },
> > + .cpuid_eax = 0x14,
> > + .cpuid_needs_ecx = true, .cpuid_ecx = 0,
> > + .cpuid_reg = R_ECX,
> > + .tcg_features = ~0U,
>
> Same as above.
>
> > + },
> > };
> >
> > typedef struct X86RegisterInfo32 {
> > @@ -3452,6 +3488,34 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
> > }
> > break;
> > }
> > + case 0x14: {
> > + /* Intel Processor Trace Enumeration */
> > + *eax = 0;
> > + *ebx = 0;
> > + *ecx = 0;
> > + *edx = 0;
> > + if (!(env->features[FEAT_7_0_EBX] & CPUID_7_0_EBX_INTEL_PT) ||
> > + !kvm_enabled()) {
> > + break;
> > + }
> > +
> > + if (count == 0) {
> > + *eax = 1;
> > + *ebx = env->features[FEAT_INTEL_PT_EBX];
> > + *ecx = env->features[FEAT_INTEL_PT_ECX];
> > + } else if (count == 1) {
> > + *eax = INTLE_PT_ADDR_RANGES_NUM;
> > + if (env->features[FEAT_INTEL_PT_EBX] &
> > + CPUID_INTEL_PT_EBX_MTC_COFI) {
> > + *eax |= INTEL_PT_MTC_BITMAP;
> > + }
> > + if (env->features[FEAT_INTEL_PT_EBX] &
> > + CPUID_INTEL_PT_EBX_PSB_CYCLE) {
> > + *ebx = INTEL_PT_PSB_BITMAP | INTEL_PT_CYCLE_BITMAP;
> > + }
>
> We still need to validate the bitmaps and number of ranges against the host capabilities (reported on GET_SUPPORTED_CPUID),
> don't we?
Yes, we need to validate the bitmaps and number of ranges against the host capabilities. For example, MSR_IA32_RTIT_CTL.MTCFreq only support the value defined in bitmap or will cause #GP fault.
>
> If you are going to set CPUID bits that are not already present on env->features[], you will want x86_cpu_filter_features() to
> manually validate the constants against x86_cpu_get_supported_feature_word(), to ensure we won't try to enable unsupported
> bits.
>
> (If doing that, we need to make sure CPUID_7_0_EBX_INTEL_PT will be set on xc->filtered_features[FEAT_7_0_EBX] if something is
> unsupported, to tell the calling code that intel-pt can't be enabled on the current host)
>
So, Can I make all the value in CPUID[14] as constant and make the CPUID information get from IceLake hardware as default(minimal) value.
CPUID[14] available only when Intel PT is supported and enabled.
We also need to check the host value by kvm_arch_get_supported_cpuid(). If something is unsupported in minimal value Intel PT can't be enabled. I didn't use x86_cpu_get_supported_feature_word() because the value of CPUID[14] will all be constant hence sub-leaf FEAT_INTEL_PT_EBX/ FEAT_INTEL_PT_ECX are unnecessary(will remove in next version).
Please help correct me if anything wrong.
Thanks,
Luwei Kang
> In the future we could extend FeatureWordInfo to make it easier to handle counter/bitmap fields like those, then we won't need
> special cases inside x86_cpu_filter_features().
>
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/2] i386: Add Intel Processor Trace feature support
@ 2018-01-29 7:03 ` Kang, Luwei
0 siblings, 0 replies; 10+ messages in thread
From: Kang, Luwei @ 2018-01-29 7:03 UTC (permalink / raw)
To: Eduardo Habkost; +Cc: qemu-devel, kvm, pbonzini, rth, mtosatti, Chao Peng
> > From: Chao Peng <chao.p.peng@linux.intel.com>
> >
> > Expose Intel Processor Trace feature to guest.
> >
> > In order to make this feature migration-safe, new feature word
> > information "FEAT_INTEL_PT_EBX" and "FEAT_INTEL_PT_ECX" be added.
> > Some constant value initialized in CPUID[0x14].0x01 to guarantee get
> > same result in diffrent hardware when this feature is enabled.
> >
> > Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>
> > Signed-off-by: Luwei Kang <luwei.kang@intel.com>
> > ---
> > v1->v2:
> > - In order to make this feature migration-safe, new feature word
> > information "FEAT_INTEL_PT_EBX" and "FEAT_INTEL_PT_ECX" be added.
> > Some constant value initialized in CPUID[0x14].0x01 to guarantee
> > get same result in diffrent hardware when this feature is enabled.
> >
> > ---
> > target/i386/cpu.c | 68
> > +++++++++++++++++++++++++++++++++++++++++++++++++++++--
> > target/i386/cpu.h | 6 +++++
> > target/i386/kvm.c | 23 +++++++++++++++++++
> > 3 files changed, 95 insertions(+), 2 deletions(-)
> >
> > diff --git a/target/i386/cpu.c b/target/i386/cpu.c index
> > a49d222..ee8c6e6 100644
> > --- a/target/i386/cpu.c
> > +++ b/target/i386/cpu.c
> > @@ -172,7 +172,11 @@
> > #define L2_ITLB_4K_ASSOC 4
> > #define L2_ITLB_4K_ENTRIES 512
> >
> > -
> > +/* CPUID Leaf 0x14 constants: */
> > +#define INTLE_PT_ADDR_RANGES_NUM 2
> > +#define INTEL_PT_MTC_BITMAP (0x0249 << 16) /* Support ART(0,3,6,9) */
> > +#define INTEL_PT_CYCLE_BITMAP 0x3fff /* Support 0,2^(0~12) */
> > +#define INTEL_PT_PSB_BITMAP (0x003f << 16) /* Support 2K,4K,8K,16K,32K,64K */
> >
> > static void x86_cpu_vendor_words2str(char *dst, uint32_t vendor1,
> > uint32_t vendor2, uint32_t
> > vendor3) @@ -427,7 +431,7 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
> > NULL, NULL, "mpx", NULL,
> > "avx512f", "avx512dq", "rdseed", "adx",
> > "smap", "avx512ifma", "pcommit", "clflushopt",
> > - "clwb", NULL, "avx512pf", "avx512er",
> > + "clwb", "intel-pt", "avx512pf", "avx512er",
> > "avx512cd", "sha-ni", "avx512bw", "avx512vl",
> > },
> > .cpuid_eax = 7,
> > @@ -545,6 +549,38 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
> > .cpuid_reg = R_EDX,
> > .tcg_features = ~0U,
> > },
> > + [FEAT_INTEL_PT_EBX] = {
> > + .feat_names = {
> > + "cr3-filter", "psb-cycle", "filtering", "mtc-cofi",
> > + "ptwrite", "power-evt", NULL, NULL,
>
> Note that this will make all those bits configurable on the command-line. You don't really need to do this in the first version, if you
> don't really want to.
>
> Also, if we want to make this configurable, we will need to choose carefully the property names, because changing them later would
> be more difficult.
>
> Note that the feature names live in a single QOM property namespace, so "filtering", "any-entries" and "transport" are probably too
> generic. Maybe we could prefix all names here with "intel-pt-" or "pt-"?
>
> If you don't want to make all bits configurable on the first version, you can fill the CPUID[14h] bits with constant default values just
> like you did with the INTEL_PT_* constants below.
>
Agree, I think it is too complex to make all Intel PT sub-feature configurable in command line. We also need make some short and clear enough property names for each sub-feature.
>
> > + NULL, NULL, NULL, NULL,
> > + NULL, NULL, NULL, NULL,
> > + NULL, NULL, NULL, NULL,
> > + NULL, NULL, NULL, NULL,
> > + NULL, NULL, NULL, NULL,
> > + NULL, NULL, NULL, NULL,
> > + },
> > + .cpuid_eax = 0x14,
> > + .cpuid_needs_ecx = true, .cpuid_ecx = 0,
> > + .cpuid_reg = R_EBX,
> > + .tcg_features = ~0U,
>
> TCG doesn't implement this feature, so ~0U doesn't look right.
>
>
> > + },
> > + [FEAT_INTEL_PT_ECX] = {
> > + .feat_names = {
> > + "topa-output", "any-entries", "single-range", "transport",
> > + 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,
> > + },
> > + .cpuid_eax = 0x14,
> > + .cpuid_needs_ecx = true, .cpuid_ecx = 0,
> > + .cpuid_reg = R_ECX,
> > + .tcg_features = ~0U,
>
> Same as above.
>
> > + },
> > };
> >
> > typedef struct X86RegisterInfo32 {
> > @@ -3452,6 +3488,34 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
> > }
> > break;
> > }
> > + case 0x14: {
> > + /* Intel Processor Trace Enumeration */
> > + *eax = 0;
> > + *ebx = 0;
> > + *ecx = 0;
> > + *edx = 0;
> > + if (!(env->features[FEAT_7_0_EBX] & CPUID_7_0_EBX_INTEL_PT) ||
> > + !kvm_enabled()) {
> > + break;
> > + }
> > +
> > + if (count == 0) {
> > + *eax = 1;
> > + *ebx = env->features[FEAT_INTEL_PT_EBX];
> > + *ecx = env->features[FEAT_INTEL_PT_ECX];
> > + } else if (count == 1) {
> > + *eax = INTLE_PT_ADDR_RANGES_NUM;
> > + if (env->features[FEAT_INTEL_PT_EBX] &
> > + CPUID_INTEL_PT_EBX_MTC_COFI) {
> > + *eax |= INTEL_PT_MTC_BITMAP;
> > + }
> > + if (env->features[FEAT_INTEL_PT_EBX] &
> > + CPUID_INTEL_PT_EBX_PSB_CYCLE) {
> > + *ebx = INTEL_PT_PSB_BITMAP | INTEL_PT_CYCLE_BITMAP;
> > + }
>
> We still need to validate the bitmaps and number of ranges against the host capabilities (reported on GET_SUPPORTED_CPUID),
> don't we?
Yes, we need to validate the bitmaps and number of ranges against the host capabilities. For example, MSR_IA32_RTIT_CTL.MTCFreq only support the value defined in bitmap or will cause #GP fault.
>
> If you are going to set CPUID bits that are not already present on env->features[], you will want x86_cpu_filter_features() to
> manually validate the constants against x86_cpu_get_supported_feature_word(), to ensure we won't try to enable unsupported
> bits.
>
> (If doing that, we need to make sure CPUID_7_0_EBX_INTEL_PT will be set on xc->filtered_features[FEAT_7_0_EBX] if something is
> unsupported, to tell the calling code that intel-pt can't be enabled on the current host)
>
So, Can I make all the value in CPUID[14] as constant and make the CPUID information get from IceLake hardware as default(minimal) value.
CPUID[14] available only when Intel PT is supported and enabled.
We also need to check the host value by kvm_arch_get_supported_cpuid(). If something is unsupported in minimal value Intel PT can't be enabled. I didn't use x86_cpu_get_supported_feature_word() because the value of CPUID[14] will all be constant hence sub-leaf FEAT_INTEL_PT_EBX/ FEAT_INTEL_PT_ECX are unnecessary(will remove in next version).
Please help correct me if anything wrong.
Thanks,
Luwei Kang
> In the future we could extend FeatureWordInfo to make it easier to handle counter/bitmap fields like those, then we won't need
> special cases inside x86_cpu_filter_features().
>
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 1/2] i386: Add Intel Processor Trace feature support
2018-01-29 7:03 ` [Qemu-devel] " Kang, Luwei
@ 2018-01-29 16:27 ` Eduardo Habkost
-1 siblings, 0 replies; 10+ messages in thread
From: Eduardo Habkost @ 2018-01-29 16:27 UTC (permalink / raw)
To: Kang, Luwei; +Cc: qemu-devel, kvm, pbonzini, rth, mtosatti, Chao Peng
On Mon, Jan 29, 2018 at 07:03:13AM +0000, Kang, Luwei wrote:
> > > From: Chao Peng <chao.p.peng@linux.intel.com>
> > >
> > > Expose Intel Processor Trace feature to guest.
> > >
> > > In order to make this feature migration-safe, new feature word
> > > information "FEAT_INTEL_PT_EBX" and "FEAT_INTEL_PT_ECX" be added.
> > > Some constant value initialized in CPUID[0x14].0x01 to guarantee get
> > > same result in diffrent hardware when this feature is enabled.
> > >
> > > Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>
> > > Signed-off-by: Luwei Kang <luwei.kang@intel.com>
> > > ---
> > > v1->v2:
> > > - In order to make this feature migration-safe, new feature word
> > > information "FEAT_INTEL_PT_EBX" and "FEAT_INTEL_PT_ECX" be added.
> > > Some constant value initialized in CPUID[0x14].0x01 to guarantee
> > > get same result in diffrent hardware when this feature is enabled.
> > >
[...]
> > > @@ -3452,6 +3488,34 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
> > > }
> > > break;
> > > }
> > > + case 0x14: {
> > > + /* Intel Processor Trace Enumeration */
> > > + *eax = 0;
> > > + *ebx = 0;
> > > + *ecx = 0;
> > > + *edx = 0;
> > > + if (!(env->features[FEAT_7_0_EBX] & CPUID_7_0_EBX_INTEL_PT) ||
> > > + !kvm_enabled()) {
> > > + break;
> > > + }
> > > +
> > > + if (count == 0) {
> > > + *eax = 1;
> > > + *ebx = env->features[FEAT_INTEL_PT_EBX];
> > > + *ecx = env->features[FEAT_INTEL_PT_ECX];
> > > + } else if (count == 1) {
> > > + *eax = INTLE_PT_ADDR_RANGES_NUM;
> > > + if (env->features[FEAT_INTEL_PT_EBX] &
> > > + CPUID_INTEL_PT_EBX_MTC_COFI) {
> > > + *eax |= INTEL_PT_MTC_BITMAP;
> > > + }
> > > + if (env->features[FEAT_INTEL_PT_EBX] &
> > > + CPUID_INTEL_PT_EBX_PSB_CYCLE) {
> > > + *ebx = INTEL_PT_PSB_BITMAP | INTEL_PT_CYCLE_BITMAP;
> > > + }
> >
> > We still need to validate the bitmaps and number of ranges against the host capabilities (reported on GET_SUPPORTED_CPUID),
> > don't we?
>
> Yes, we need to validate the bitmaps and number of ranges against the host capabilities. For example, MSR_IA32_RTIT_CTL.MTCFreq only support the value defined in bitmap or will cause #GP fault.
>
> >
> > If you are going to set CPUID bits that are not already present on env->features[], you will want x86_cpu_filter_features() to
> > manually validate the constants against x86_cpu_get_supported_feature_word(), to ensure we won't try to enable unsupported
> > bits.
> >
> > (If doing that, we need to make sure CPUID_7_0_EBX_INTEL_PT will be set on xc->filtered_features[FEAT_7_0_EBX] if something is
> > unsupported, to tell the calling code that intel-pt can't be enabled on the current host)
> >
>
> So, Can I make all the value in CPUID[14] as constant and make the CPUID information get from IceLake hardware as default(minimal) value.
> CPUID[14] available only when Intel PT is supported and enabled.
> We also need to check the host value by kvm_arch_get_supported_cpuid(). If something is unsupported in minimal value Intel PT can't be enabled.
Exactly.
> I didn't use x86_cpu_get_supported_feature_word() because the value of CPUID[14] will all be constant hence sub-leaf FEAT_INTEL_PT_EBX/ FEAT_INTEL_PT_ECX are unnecessary(will remove in next version).
Yes, if you make CPUID[14h] constant in the first version, you
won't need FEAT_INTEL_PT_* yet.
However, if you introduce FeatureWord values for
CPUID[EAX=14h,ECX=0].EBX, CPUID[EAX=14h,ECX=0].ECX,
CPUID[EAX=14h,ECX=1].EAX, and CPUID[EAX=14h,ECX=1].EBX,
you will be able to write more generic code using
x86_cpu_get_supported_feature_word(), and make it easier to make
the PT features configurable by CPU models in the future.
But I would be OK with an initial version that simply uses
constants, and not introducing new FeatureWord values.
> Please help correct me if anything wrong.
>
> Thanks,
> Luwei Kang
>
> > In the future we could extend FeatureWordInfo to make it easier to handle counter/bitmap fields like those, then we won't need
> > special cases inside x86_cpu_filter_features().
> >
> >
>
--
Eduardo
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/2] i386: Add Intel Processor Trace feature support
@ 2018-01-29 16:27 ` Eduardo Habkost
0 siblings, 0 replies; 10+ messages in thread
From: Eduardo Habkost @ 2018-01-29 16:27 UTC (permalink / raw)
To: Kang, Luwei; +Cc: qemu-devel, kvm, pbonzini, rth, mtosatti, Chao Peng
On Mon, Jan 29, 2018 at 07:03:13AM +0000, Kang, Luwei wrote:
> > > From: Chao Peng <chao.p.peng@linux.intel.com>
> > >
> > > Expose Intel Processor Trace feature to guest.
> > >
> > > In order to make this feature migration-safe, new feature word
> > > information "FEAT_INTEL_PT_EBX" and "FEAT_INTEL_PT_ECX" be added.
> > > Some constant value initialized in CPUID[0x14].0x01 to guarantee get
> > > same result in diffrent hardware when this feature is enabled.
> > >
> > > Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>
> > > Signed-off-by: Luwei Kang <luwei.kang@intel.com>
> > > ---
> > > v1->v2:
> > > - In order to make this feature migration-safe, new feature word
> > > information "FEAT_INTEL_PT_EBX" and "FEAT_INTEL_PT_ECX" be added.
> > > Some constant value initialized in CPUID[0x14].0x01 to guarantee
> > > get same result in diffrent hardware when this feature is enabled.
> > >
[...]
> > > @@ -3452,6 +3488,34 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
> > > }
> > > break;
> > > }
> > > + case 0x14: {
> > > + /* Intel Processor Trace Enumeration */
> > > + *eax = 0;
> > > + *ebx = 0;
> > > + *ecx = 0;
> > > + *edx = 0;
> > > + if (!(env->features[FEAT_7_0_EBX] & CPUID_7_0_EBX_INTEL_PT) ||
> > > + !kvm_enabled()) {
> > > + break;
> > > + }
> > > +
> > > + if (count == 0) {
> > > + *eax = 1;
> > > + *ebx = env->features[FEAT_INTEL_PT_EBX];
> > > + *ecx = env->features[FEAT_INTEL_PT_ECX];
> > > + } else if (count == 1) {
> > > + *eax = INTLE_PT_ADDR_RANGES_NUM;
> > > + if (env->features[FEAT_INTEL_PT_EBX] &
> > > + CPUID_INTEL_PT_EBX_MTC_COFI) {
> > > + *eax |= INTEL_PT_MTC_BITMAP;
> > > + }
> > > + if (env->features[FEAT_INTEL_PT_EBX] &
> > > + CPUID_INTEL_PT_EBX_PSB_CYCLE) {
> > > + *ebx = INTEL_PT_PSB_BITMAP | INTEL_PT_CYCLE_BITMAP;
> > > + }
> >
> > We still need to validate the bitmaps and number of ranges against the host capabilities (reported on GET_SUPPORTED_CPUID),
> > don't we?
>
> Yes, we need to validate the bitmaps and number of ranges against the host capabilities. For example, MSR_IA32_RTIT_CTL.MTCFreq only support the value defined in bitmap or will cause #GP fault.
>
> >
> > If you are going to set CPUID bits that are not already present on env->features[], you will want x86_cpu_filter_features() to
> > manually validate the constants against x86_cpu_get_supported_feature_word(), to ensure we won't try to enable unsupported
> > bits.
> >
> > (If doing that, we need to make sure CPUID_7_0_EBX_INTEL_PT will be set on xc->filtered_features[FEAT_7_0_EBX] if something is
> > unsupported, to tell the calling code that intel-pt can't be enabled on the current host)
> >
>
> So, Can I make all the value in CPUID[14] as constant and make the CPUID information get from IceLake hardware as default(minimal) value.
> CPUID[14] available only when Intel PT is supported and enabled.
> We also need to check the host value by kvm_arch_get_supported_cpuid(). If something is unsupported in minimal value Intel PT can't be enabled.
Exactly.
> I didn't use x86_cpu_get_supported_feature_word() because the value of CPUID[14] will all be constant hence sub-leaf FEAT_INTEL_PT_EBX/ FEAT_INTEL_PT_ECX are unnecessary(will remove in next version).
Yes, if you make CPUID[14h] constant in the first version, you
won't need FEAT_INTEL_PT_* yet.
However, if you introduce FeatureWord values for
CPUID[EAX=14h,ECX=0].EBX, CPUID[EAX=14h,ECX=0].ECX,
CPUID[EAX=14h,ECX=1].EAX, and CPUID[EAX=14h,ECX=1].EBX,
you will be able to write more generic code using
x86_cpu_get_supported_feature_word(), and make it easier to make
the PT features configurable by CPU models in the future.
But I would be OK with an initial version that simply uses
constants, and not introducing new FeatureWord values.
> Please help correct me if anything wrong.
>
> Thanks,
> Luwei Kang
>
> > In the future we could extend FeatureWordInfo to make it easier to handle counter/bitmap fields like those, then we won't need
> > special cases inside x86_cpu_filter_features().
> >
> >
>
--
Eduardo
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2018-01-29 16:27 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-25 18:25 [PATCH v2 1/2] i386: Add Intel Processor Trace feature support Luwei Kang
2018-01-25 18:25 ` [Qemu-devel] " Luwei Kang
2018-01-25 18:25 ` [PATCH v2 2/2] i386: Add support to get/set/migrate Intel Processor Trace feature Luwei Kang
2018-01-25 18:25 ` [Qemu-devel] " Luwei Kang
2018-01-26 19:14 ` [PATCH v2 1/2] i386: Add Intel Processor Trace feature support Eduardo Habkost
2018-01-26 19:14 ` [Qemu-devel] " Eduardo Habkost
2018-01-29 7:03 ` Kang, Luwei
2018-01-29 7:03 ` [Qemu-devel] " Kang, Luwei
2018-01-29 16:27 ` Eduardo Habkost
2018-01-29 16:27 ` [Qemu-devel] " Eduardo Habkost
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.