* [PATCH RESEND v1 1/2] i386: Add Intel Processor Trace feature support
@ 2018-01-08 20:36 ` Luwei Kang
0 siblings, 0 replies; 44+ messages in thread
From: Luwei Kang @ 2018-01-08 20:36 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.
Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>
Signed-off-by: Luwei Kang <luwei.kang@intel.com>
---
target/i386/cpu.c | 19 ++++++++++++++++++-
target/i386/cpu.h | 1 +
target/i386/kvm.c | 23 +++++++++++++++++++++++
3 files changed, 42 insertions(+), 1 deletion(-)
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 3818d72..57f8370 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -427,7 +427,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,
@@ -3006,6 +3006,23 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
}
break;
}
+ case 0x14: {
+ if ((env->features[FEAT_7_0_EBX] & CPUID_7_0_EBX_INTEL_PT) &&
+ kvm_enabled()) {
+ KVMState *s = cs->kvm_state;
+
+ *eax = kvm_arch_get_supported_cpuid(s, 0x14, count, R_EAX);
+ *ebx = kvm_arch_get_supported_cpuid(s, 0x14, count, R_EBX);
+ *ecx = kvm_arch_get_supported_cpuid(s, 0x14, count, R_ECX);
+ *edx = kvm_arch_get_supported_cpuid(s, 0x14, count, R_EDX);
+ } else {
+ *eax = 0;
+ *ebx = 0;
+ *ecx = 0;
+ *edx = 0;
+ }
+ 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 62c4742..58a4b6c 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -642,6 +642,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 */
diff --git a/target/i386/kvm.c b/target/i386/kvm.c
index 6f69e2f..e13ab58 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -863,6 +863,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] 44+ messages in thread
* [Qemu-devel] [PATCH RESEND v1 1/2] i386: Add Intel Processor Trace feature support
@ 2018-01-08 20:36 ` Luwei Kang
0 siblings, 0 replies; 44+ messages in thread
From: Luwei Kang @ 2018-01-08 20:36 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.
Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>
Signed-off-by: Luwei Kang <luwei.kang@intel.com>
---
target/i386/cpu.c | 19 ++++++++++++++++++-
target/i386/cpu.h | 1 +
target/i386/kvm.c | 23 +++++++++++++++++++++++
3 files changed, 42 insertions(+), 1 deletion(-)
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 3818d72..57f8370 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -427,7 +427,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,
@@ -3006,6 +3006,23 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
}
break;
}
+ case 0x14: {
+ if ((env->features[FEAT_7_0_EBX] & CPUID_7_0_EBX_INTEL_PT) &&
+ kvm_enabled()) {
+ KVMState *s = cs->kvm_state;
+
+ *eax = kvm_arch_get_supported_cpuid(s, 0x14, count, R_EAX);
+ *ebx = kvm_arch_get_supported_cpuid(s, 0x14, count, R_EBX);
+ *ecx = kvm_arch_get_supported_cpuid(s, 0x14, count, R_ECX);
+ *edx = kvm_arch_get_supported_cpuid(s, 0x14, count, R_EDX);
+ } else {
+ *eax = 0;
+ *ebx = 0;
+ *ecx = 0;
+ *edx = 0;
+ }
+ 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 62c4742..58a4b6c 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -642,6 +642,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 */
diff --git a/target/i386/kvm.c b/target/i386/kvm.c
index 6f69e2f..e13ab58 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -863,6 +863,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] 44+ messages in thread
* [PATCH RESEND v1 2/2] i386: Add support to get/set/migrate Intel Processor Trace feature
2018-01-08 20:36 ` [Qemu-devel] " Luwei Kang
@ 2018-01-08 20:36 ` Luwei Kang
-1 siblings, 0 replies; 44+ messages in thread
From: Luwei Kang @ 2018-01-08 20:36 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: Chao Peng <chao.p.peng@linux.intel.com>
Signed-off-by: Luwei Kang <luwei.kang@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 58a4b6c..331bf94 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -418,6 +418,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)
@@ -1151,6 +1166,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 e13ab58..ba893bc 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -1789,6 +1789,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. */
@@ -2135,6 +2154,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;
@@ -2372,6 +2405,24 @@ static int kvm_get_msrs(X86CPU *cpu)
env->mtrr_var[MSR_MTRRphysIndex(index)].base = 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 df5ec35..ace01b2 100644
--- a/target/i386/machine.c
+++ b/target/i386/machine.c
@@ -818,6 +818,43 @@ static const VMStateDescription vmstate_mcg_ext_ctl = {
}
};
+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,
@@ -937,6 +974,7 @@ VMStateDescription vmstate_x86_cpu = {
&vmstate_pkru,
#endif
&vmstate_mcg_ext_ctl,
+ &vmstate_msr_intel_pt,
NULL
}
};
--
1.8.3.1
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [Qemu-devel] [PATCH RESEND v1 2/2] i386: Add support to get/set/migrate Intel Processor Trace feature
@ 2018-01-08 20:36 ` Luwei Kang
0 siblings, 0 replies; 44+ messages in thread
From: Luwei Kang @ 2018-01-08 20:36 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: Chao Peng <chao.p.peng@linux.intel.com>
Signed-off-by: Luwei Kang <luwei.kang@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 58a4b6c..331bf94 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -418,6 +418,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)
@@ -1151,6 +1166,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 e13ab58..ba893bc 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -1789,6 +1789,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. */
@@ -2135,6 +2154,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;
@@ -2372,6 +2405,24 @@ static int kvm_get_msrs(X86CPU *cpu)
env->mtrr_var[MSR_MTRRphysIndex(index)].base = 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 df5ec35..ace01b2 100644
--- a/target/i386/machine.c
+++ b/target/i386/machine.c
@@ -818,6 +818,43 @@ static const VMStateDescription vmstate_mcg_ext_ctl = {
}
};
+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,
@@ -937,6 +974,7 @@ VMStateDescription vmstate_x86_cpu = {
&vmstate_pkru,
#endif
&vmstate_mcg_ext_ctl,
+ &vmstate_msr_intel_pt,
NULL
}
};
--
1.8.3.1
^ permalink raw reply related [flat|nested] 44+ messages in thread
* Re: [PATCH RESEND v1 1/2] i386: Add Intel Processor Trace feature support
2018-01-08 20:36 ` [Qemu-devel] " Luwei Kang
@ 2018-01-12 14:22 ` Eduardo Habkost
-1 siblings, 0 replies; 44+ messages in thread
From: Eduardo Habkost @ 2018-01-12 14:22 UTC (permalink / raw)
To: Luwei Kang; +Cc: qemu-devel, kvm, pbonzini, rth, mtosatti, Chao Peng
On Tue, Jan 09, 2018 at 04:36:36AM +0800, Luwei Kang wrote:
> From: Chao Peng <chao.p.peng@linux.intel.com>
>
> Expose Intel Processor Trace feature to guest.
>
> Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>
> Signed-off-by: Luwei Kang <luwei.kang@intel.com>
> ---
> target/i386/cpu.c | 19 ++++++++++++++++++-
> target/i386/cpu.h | 1 +
> target/i386/kvm.c | 23 +++++++++++++++++++++++
> 3 files changed, 42 insertions(+), 1 deletion(-)
>
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 3818d72..57f8370 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -427,7 +427,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,
> @@ -3006,6 +3006,23 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
> }
> break;
> }
> + case 0x14: {
> + if ((env->features[FEAT_7_0_EBX] & CPUID_7_0_EBX_INTEL_PT) &&
> + kvm_enabled()) {
> + KVMState *s = cs->kvm_state;
> +
> + *eax = kvm_arch_get_supported_cpuid(s, 0x14, count, R_EAX);
> + *ebx = kvm_arch_get_supported_cpuid(s, 0x14, count, R_EBX);
> + *ecx = kvm_arch_get_supported_cpuid(s, 0x14, count, R_ECX);
> + *edx = kvm_arch_get_supported_cpuid(s, 0x14, count, R_EDX);
If you are forwarding host info directly to the guest, the
feature is not migration-safe. The new feature needs to be added
to feature_word_info[FEAT_7_0_EBX].unmigratable_flags.
> + } else {
> + *eax = 0;
> + *ebx = 0;
> + *ecx = 0;
> + *edx = 0;
> + }
> + 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 62c4742..58a4b6c 100644
> --- a/target/i386/cpu.h
> +++ b/target/i386/cpu.h
> @@ -642,6 +642,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 */
> diff --git a/target/i386/kvm.c b/target/i386/kvm.c
> index 6f69e2f..e13ab58 100644
> --- a/target/i386/kvm.c
> +++ b/target/i386/kvm.c
> @@ -863,6 +863,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] 44+ messages in thread
* Re: [Qemu-devel] [PATCH RESEND v1 1/2] i386: Add Intel Processor Trace feature support
@ 2018-01-12 14:22 ` Eduardo Habkost
0 siblings, 0 replies; 44+ messages in thread
From: Eduardo Habkost @ 2018-01-12 14:22 UTC (permalink / raw)
To: Luwei Kang; +Cc: qemu-devel, kvm, pbonzini, rth, mtosatti, Chao Peng
On Tue, Jan 09, 2018 at 04:36:36AM +0800, Luwei Kang wrote:
> From: Chao Peng <chao.p.peng@linux.intel.com>
>
> Expose Intel Processor Trace feature to guest.
>
> Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>
> Signed-off-by: Luwei Kang <luwei.kang@intel.com>
> ---
> target/i386/cpu.c | 19 ++++++++++++++++++-
> target/i386/cpu.h | 1 +
> target/i386/kvm.c | 23 +++++++++++++++++++++++
> 3 files changed, 42 insertions(+), 1 deletion(-)
>
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 3818d72..57f8370 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -427,7 +427,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,
> @@ -3006,6 +3006,23 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
> }
> break;
> }
> + case 0x14: {
> + if ((env->features[FEAT_7_0_EBX] & CPUID_7_0_EBX_INTEL_PT) &&
> + kvm_enabled()) {
> + KVMState *s = cs->kvm_state;
> +
> + *eax = kvm_arch_get_supported_cpuid(s, 0x14, count, R_EAX);
> + *ebx = kvm_arch_get_supported_cpuid(s, 0x14, count, R_EBX);
> + *ecx = kvm_arch_get_supported_cpuid(s, 0x14, count, R_ECX);
> + *edx = kvm_arch_get_supported_cpuid(s, 0x14, count, R_EDX);
If you are forwarding host info directly to the guest, the
feature is not migration-safe. The new feature needs to be added
to feature_word_info[FEAT_7_0_EBX].unmigratable_flags.
> + } else {
> + *eax = 0;
> + *ebx = 0;
> + *ecx = 0;
> + *edx = 0;
> + }
> + 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 62c4742..58a4b6c 100644
> --- a/target/i386/cpu.h
> +++ b/target/i386/cpu.h
> @@ -642,6 +642,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 */
> diff --git a/target/i386/kvm.c b/target/i386/kvm.c
> index 6f69e2f..e13ab58 100644
> --- a/target/i386/kvm.c
> +++ b/target/i386/kvm.c
> @@ -863,6 +863,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] 44+ messages in thread
* RE: [PATCH RESEND v1 1/2] i386: Add Intel Processor Trace feature support
2018-01-12 14:22 ` [Qemu-devel] " Eduardo Habkost
@ 2018-01-15 7:19 ` Kang, Luwei
-1 siblings, 0 replies; 44+ messages in thread
From: Kang, Luwei @ 2018-01-15 7:19 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.
> >
> > Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>
> > Signed-off-by: Luwei Kang <luwei.kang@intel.com>
> > ---
> > target/i386/cpu.c | 19 ++++++++++++++++++- target/i386/cpu.h | 1 +
> > target/i386/kvm.c | 23 +++++++++++++++++++++++
> > 3 files changed, 42 insertions(+), 1 deletion(-)
> >
> > diff --git a/target/i386/cpu.c b/target/i386/cpu.c index
> > 3818d72..57f8370 100644
> > --- a/target/i386/cpu.c
> > +++ b/target/i386/cpu.c
> > @@ -427,7 +427,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,
> > @@ -3006,6 +3006,23 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
> > }
> > break;
> > }
> > + case 0x14: {
> > + if ((env->features[FEAT_7_0_EBX] & CPUID_7_0_EBX_INTEL_PT) &&
> > + kvm_enabled()) {
> > + KVMState *s = cs->kvm_state;
> > +
> > + *eax = kvm_arch_get_supported_cpuid(s, 0x14, count, R_EAX);
> > + *ebx = kvm_arch_get_supported_cpuid(s, 0x14, count, R_EBX);
> > + *ecx = kvm_arch_get_supported_cpuid(s, 0x14, count, R_ECX);
> > + *edx = kvm_arch_get_supported_cpuid(s, 0x14, count,
> > + R_EDX);
>
> If you are forwarding host info directly to the guest, the feature is not migration-safe. The new feature needs to be added to
> feature_word_info[FEAT_7_0_EBX].unmigratable_flags.
>
Hi,
Thanks for you review. I want to support Intel PT live migration and patch [2/2] to do this. I don't understand why need to add this feature in feature_word_info[FEAT_7_0_EBX].unmigratable_flags first to disable live migration.
Thanks,
Luwei Kang
>
> > + } else {
> > + *eax = 0;
> > + *ebx = 0;
> > + *ecx = 0;
> > + *edx = 0;
> > + }
> > + 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 62c4742..58a4b6c
> > 100644
> > --- a/target/i386/cpu.h
> > +++ b/target/i386/cpu.h
> > @@ -642,6 +642,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 */ diff --git a/target/i386/kvm.c
> > b/target/i386/kvm.c index 6f69e2f..e13ab58 100644
> > --- a/target/i386/kvm.c
> > +++ b/target/i386/kvm.c
> > @@ -863,6 +863,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] 44+ messages in thread
* Re: [Qemu-devel] [PATCH RESEND v1 1/2] i386: Add Intel Processor Trace feature support
@ 2018-01-15 7:19 ` Kang, Luwei
0 siblings, 0 replies; 44+ messages in thread
From: Kang, Luwei @ 2018-01-15 7:19 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.
> >
> > Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>
> > Signed-off-by: Luwei Kang <luwei.kang@intel.com>
> > ---
> > target/i386/cpu.c | 19 ++++++++++++++++++- target/i386/cpu.h | 1 +
> > target/i386/kvm.c | 23 +++++++++++++++++++++++
> > 3 files changed, 42 insertions(+), 1 deletion(-)
> >
> > diff --git a/target/i386/cpu.c b/target/i386/cpu.c index
> > 3818d72..57f8370 100644
> > --- a/target/i386/cpu.c
> > +++ b/target/i386/cpu.c
> > @@ -427,7 +427,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,
> > @@ -3006,6 +3006,23 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
> > }
> > break;
> > }
> > + case 0x14: {
> > + if ((env->features[FEAT_7_0_EBX] & CPUID_7_0_EBX_INTEL_PT) &&
> > + kvm_enabled()) {
> > + KVMState *s = cs->kvm_state;
> > +
> > + *eax = kvm_arch_get_supported_cpuid(s, 0x14, count, R_EAX);
> > + *ebx = kvm_arch_get_supported_cpuid(s, 0x14, count, R_EBX);
> > + *ecx = kvm_arch_get_supported_cpuid(s, 0x14, count, R_ECX);
> > + *edx = kvm_arch_get_supported_cpuid(s, 0x14, count,
> > + R_EDX);
>
> If you are forwarding host info directly to the guest, the feature is not migration-safe. The new feature needs to be added to
> feature_word_info[FEAT_7_0_EBX].unmigratable_flags.
>
Hi,
Thanks for you review. I want to support Intel PT live migration and patch [2/2] to do this. I don't understand why need to add this feature in feature_word_info[FEAT_7_0_EBX].unmigratable_flags first to disable live migration.
Thanks,
Luwei Kang
>
> > + } else {
> > + *eax = 0;
> > + *ebx = 0;
> > + *ecx = 0;
> > + *edx = 0;
> > + }
> > + 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 62c4742..58a4b6c
> > 100644
> > --- a/target/i386/cpu.h
> > +++ b/target/i386/cpu.h
> > @@ -642,6 +642,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 */ diff --git a/target/i386/kvm.c
> > b/target/i386/kvm.c index 6f69e2f..e13ab58 100644
> > --- a/target/i386/kvm.c
> > +++ b/target/i386/kvm.c
> > @@ -863,6 +863,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] 44+ messages in thread
* Re: [PATCH RESEND v1 1/2] i386: Add Intel Processor Trace feature support
2018-01-15 7:19 ` [Qemu-devel] " Kang, Luwei
@ 2018-01-15 9:33 ` Paolo Bonzini
-1 siblings, 0 replies; 44+ messages in thread
From: Paolo Bonzini @ 2018-01-15 9:33 UTC (permalink / raw)
To: Kang, Luwei, Eduardo Habkost; +Cc: qemu-devel, kvm, rth, mtosatti, Chao Peng
On 15/01/2018 08:19, Kang, Luwei wrote:
>> If you are forwarding host info directly to the guest, the feature
>> is not migration-safe. The new feature needs to be added to
>> feature_word_info[FEAT_7_0_EBX].unmigratable_flags.
>>
> Hi, Thanks for you review. I want to support Intel PT live migration
> and patch [2/2] to do this. I don't understand why need to add this
> feature in feature_word_info[FEAT_7_0_EBX].unmigratable_flags first
> to disable live migration.
Hi Luwei,
the issue is that different hosts can have different CPUID flags. You
don't have a way to set the CPUID flags from the "-cpu" command line,
and it's not clear what values will be there in the various Ice Lake SKUs.
I am not sure if there is a mechanism to allow live migration only for
"-cpu host", but it cannot be supported for any other "-cpu" model.
Paolo
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [Qemu-devel] [PATCH RESEND v1 1/2] i386: Add Intel Processor Trace feature support
@ 2018-01-15 9:33 ` Paolo Bonzini
0 siblings, 0 replies; 44+ messages in thread
From: Paolo Bonzini @ 2018-01-15 9:33 UTC (permalink / raw)
To: Kang, Luwei, Eduardo Habkost; +Cc: qemu-devel, kvm, rth, mtosatti, Chao Peng
On 15/01/2018 08:19, Kang, Luwei wrote:
>> If you are forwarding host info directly to the guest, the feature
>> is not migration-safe. The new feature needs to be added to
>> feature_word_info[FEAT_7_0_EBX].unmigratable_flags.
>>
> Hi, Thanks for you review. I want to support Intel PT live migration
> and patch [2/2] to do this. I don't understand why need to add this
> feature in feature_word_info[FEAT_7_0_EBX].unmigratable_flags first
> to disable live migration.
Hi Luwei,
the issue is that different hosts can have different CPUID flags. You
don't have a way to set the CPUID flags from the "-cpu" command line,
and it's not clear what values will be there in the various Ice Lake SKUs.
I am not sure if there is a mechanism to allow live migration only for
"-cpu host", but it cannot be supported for any other "-cpu" model.
Paolo
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH RESEND v1 1/2] i386: Add Intel Processor Trace feature support
2018-01-15 9:33 ` [Qemu-devel] " Paolo Bonzini
@ 2018-01-15 14:04 ` Eduardo Habkost
-1 siblings, 0 replies; 44+ messages in thread
From: Eduardo Habkost @ 2018-01-15 14:04 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Kang, Luwei, qemu-devel, kvm, rth, mtosatti, Chao Peng,
Jiri Denemark, libvir-list
CCing libvirt developers.
On Mon, Jan 15, 2018 at 10:33:35AM +0100, Paolo Bonzini wrote:
> On 15/01/2018 08:19, Kang, Luwei wrote:
> >> If you are forwarding host info directly to the guest, the feature
> >> is not migration-safe. The new feature needs to be added to
> >> feature_word_info[FEAT_7_0_EBX].unmigratable_flags.
> >>
> > Hi, Thanks for you review. I want to support Intel PT live migration
> > and patch [2/2] to do this. I don't understand why need to add this
> > feature in feature_word_info[FEAT_7_0_EBX].unmigratable_flags first
> > to disable live migration.
>
> Hi Luwei,
>
> the issue is that different hosts can have different CPUID flags. You
> don't have a way to set the CPUID flags from the "-cpu" command line,
> and it's not clear what values will be there in the various Ice Lake SKUs.
>
> I am not sure if there is a mechanism to allow live migration only for
> "-cpu host", but it cannot be supported for any other "-cpu" model.
IIRC, we don't have any mechanism to actually prevent migration
if an unmigratable flag is enabled. We just avoid enabling them
by accident on "-cpu host".
This case is slightly more problematic, however: the new feature
is actually migratable (under very controlled circumstances)
because of patch 2/2, but it is not migration-safe[1]. This
means libvirt shouldn't include it in "host-model" expansion
(which uses the query-cpu-model-expansion QMP command) until we
make the feature migration-safe.
For QEMU, this means the feature shouldn't be returned by
"query-cpu-model-expansion type=static model=max" (but it can be
returned by "query-cpu-model-expansion type=full model=max").
Jiri, it looks like libvirt uses type=full on
query-cpu-model-expansion on x86. It needs to use
type=static[2], or it will have no way to find out if a feature
is migration-safe or not.
This case is similar to "pmu", which is not migration-safe but
enabled by -cpu host by default. But "pmu" is less problematic
because it is already skipped by query-cpu-model-expansion
type=static.
---
[1] "migration-safe" is defined in the documentation for CpuDefinitionInfo on
the QAPI schema:
# @migration-safe: whether a CPU definition can be safely used for
# migration in combination with a QEMU compatibility machine
# when migrating between different QMU versions and between
# hosts with different sets of (hardware or software)
# capabilities. If not provided, information is not available
# and callers should not assume the CPU definition to be
# migration-safe. (since 2.8)
[2] It looks like libvirt uses type=full because it wants to get
all QOM property aliases returned. In this case, one
solution for libvirt is to use:
static_expansion = query_cpu_model_expansion(type=static, model)
all_props = query_cpu_model_expansion(type=full, static_expansion)
--
Eduardo
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [Qemu-devel] [PATCH RESEND v1 1/2] i386: Add Intel Processor Trace feature support
@ 2018-01-15 14:04 ` Eduardo Habkost
0 siblings, 0 replies; 44+ messages in thread
From: Eduardo Habkost @ 2018-01-15 14:04 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Kang, Luwei, qemu-devel, kvm, rth, mtosatti, Chao Peng,
Jiri Denemark, libvir-list
CCing libvirt developers.
On Mon, Jan 15, 2018 at 10:33:35AM +0100, Paolo Bonzini wrote:
> On 15/01/2018 08:19, Kang, Luwei wrote:
> >> If you are forwarding host info directly to the guest, the feature
> >> is not migration-safe. The new feature needs to be added to
> >> feature_word_info[FEAT_7_0_EBX].unmigratable_flags.
> >>
> > Hi, Thanks for you review. I want to support Intel PT live migration
> > and patch [2/2] to do this. I don't understand why need to add this
> > feature in feature_word_info[FEAT_7_0_EBX].unmigratable_flags first
> > to disable live migration.
>
> Hi Luwei,
>
> the issue is that different hosts can have different CPUID flags. You
> don't have a way to set the CPUID flags from the "-cpu" command line,
> and it's not clear what values will be there in the various Ice Lake SKUs.
>
> I am not sure if there is a mechanism to allow live migration only for
> "-cpu host", but it cannot be supported for any other "-cpu" model.
IIRC, we don't have any mechanism to actually prevent migration
if an unmigratable flag is enabled. We just avoid enabling them
by accident on "-cpu host".
This case is slightly more problematic, however: the new feature
is actually migratable (under very controlled circumstances)
because of patch 2/2, but it is not migration-safe[1]. This
means libvirt shouldn't include it in "host-model" expansion
(which uses the query-cpu-model-expansion QMP command) until we
make the feature migration-safe.
For QEMU, this means the feature shouldn't be returned by
"query-cpu-model-expansion type=static model=max" (but it can be
returned by "query-cpu-model-expansion type=full model=max").
Jiri, it looks like libvirt uses type=full on
query-cpu-model-expansion on x86. It needs to use
type=static[2], or it will have no way to find out if a feature
is migration-safe or not.
This case is similar to "pmu", which is not migration-safe but
enabled by -cpu host by default. But "pmu" is less problematic
because it is already skipped by query-cpu-model-expansion
type=static.
---
[1] "migration-safe" is defined in the documentation for CpuDefinitionInfo on
the QAPI schema:
# @migration-safe: whether a CPU definition can be safely used for
# migration in combination with a QEMU compatibility machine
# when migrating between different QMU versions and between
# hosts with different sets of (hardware or software)
# capabilities. If not provided, information is not available
# and callers should not assume the CPU definition to be
# migration-safe. (since 2.8)
[2] It looks like libvirt uses type=full because it wants to get
all QOM property aliases returned. In this case, one
solution for libvirt is to use:
static_expansion = query_cpu_model_expansion(type=static, model)
all_props = query_cpu_model_expansion(type=full, static_expansion)
--
Eduardo
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH RESEND v1 1/2] i386: Add Intel Processor Trace feature support
2018-01-15 14:04 ` [Qemu-devel] " Eduardo Habkost
@ 2018-01-15 14:25 ` Jiri Denemark
-1 siblings, 0 replies; 44+ messages in thread
From: Jiri Denemark @ 2018-01-15 14:25 UTC (permalink / raw)
To: Eduardo Habkost
Cc: Paolo Bonzini, Kang, Luwei, qemu-devel, kvm, rth, mtosatti,
Chao Peng, libvir-list
On Mon, Jan 15, 2018 at 12:04:55 -0200, Eduardo Habkost wrote:
> CCing libvirt developers.
...
> This case is slightly more problematic, however: the new feature
> is actually migratable (under very controlled circumstances)
> because of patch 2/2, but it is not migration-safe[1]. This
> means libvirt shouldn't include it in "host-model" expansion
> (which uses the query-cpu-model-expansion QMP command) until we
> make the feature migration-safe.
>
> For QEMU, this means the feature shouldn't be returned by
> "query-cpu-model-expansion type=static model=max" (but it can be
> returned by "query-cpu-model-expansion type=full model=max").
>
> Jiri, it looks like libvirt uses type=full on
> query-cpu-model-expansion on x86. It needs to use
> type=static[2], or it will have no way to find out if a feature
> is migration-safe or not.
...
> [2] It looks like libvirt uses type=full because it wants to get
> all QOM property aliases returned. In this case, one
> solution for libvirt is to use:
>
> static_expansion = query_cpu_model_expansion(type=static, model)
> all_props = query_cpu_model_expansion(type=full, static_expansion)
This is exactly what libvirt is doing (with model = "host") ever since
query-cpu-model-expansion support was implemented for x86.
Jirka
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [Qemu-devel] [PATCH RESEND v1 1/2] i386: Add Intel Processor Trace feature support
@ 2018-01-15 14:25 ` Jiri Denemark
0 siblings, 0 replies; 44+ messages in thread
From: Jiri Denemark @ 2018-01-15 14:25 UTC (permalink / raw)
To: Eduardo Habkost
Cc: Paolo Bonzini, Kang, Luwei, qemu-devel, kvm, rth, mtosatti,
Chao Peng, libvir-list
On Mon, Jan 15, 2018 at 12:04:55 -0200, Eduardo Habkost wrote:
> CCing libvirt developers.
...
> This case is slightly more problematic, however: the new feature
> is actually migratable (under very controlled circumstances)
> because of patch 2/2, but it is not migration-safe[1]. This
> means libvirt shouldn't include it in "host-model" expansion
> (which uses the query-cpu-model-expansion QMP command) until we
> make the feature migration-safe.
>
> For QEMU, this means the feature shouldn't be returned by
> "query-cpu-model-expansion type=static model=max" (but it can be
> returned by "query-cpu-model-expansion type=full model=max").
>
> Jiri, it looks like libvirt uses type=full on
> query-cpu-model-expansion on x86. It needs to use
> type=static[2], or it will have no way to find out if a feature
> is migration-safe or not.
...
> [2] It looks like libvirt uses type=full because it wants to get
> all QOM property aliases returned. In this case, one
> solution for libvirt is to use:
>
> static_expansion = query_cpu_model_expansion(type=static, model)
> all_props = query_cpu_model_expansion(type=full, static_expansion)
This is exactly what libvirt is doing (with model = "host") ever since
query-cpu-model-expansion support was implemented for x86.
Jirka
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH RESEND v1 1/2] i386: Add Intel Processor Trace feature support
2018-01-15 14:25 ` [Qemu-devel] " Jiri Denemark
@ 2018-01-15 14:31 ` Eduardo Habkost
-1 siblings, 0 replies; 44+ messages in thread
From: Eduardo Habkost @ 2018-01-15 14:31 UTC (permalink / raw)
To: Paolo Bonzini, Kang, Luwei, qemu-devel, kvm, rth, mtosatti,
Chao Peng, libvir-list
On Mon, Jan 15, 2018 at 03:25:18PM +0100, Jiri Denemark wrote:
> On Mon, Jan 15, 2018 at 12:04:55 -0200, Eduardo Habkost wrote:
> > CCing libvirt developers.
> ...
> > This case is slightly more problematic, however: the new feature
> > is actually migratable (under very controlled circumstances)
> > because of patch 2/2, but it is not migration-safe[1]. This
> > means libvirt shouldn't include it in "host-model" expansion
> > (which uses the query-cpu-model-expansion QMP command) until we
> > make the feature migration-safe.
> >
> > For QEMU, this means the feature shouldn't be returned by
> > "query-cpu-model-expansion type=static model=max" (but it can be
> > returned by "query-cpu-model-expansion type=full model=max").
> >
> > Jiri, it looks like libvirt uses type=full on
> > query-cpu-model-expansion on x86. It needs to use
> > type=static[2], or it will have no way to find out if a feature
> > is migration-safe or not.
> ...
> > [2] It looks like libvirt uses type=full because it wants to get
> > all QOM property aliases returned. In this case, one
> > solution for libvirt is to use:
> >
> > static_expansion = query_cpu_model_expansion(type=static, model)
> > all_props = query_cpu_model_expansion(type=full, static_expansion)
>
> This is exactly what libvirt is doing (with model = "host") ever since
> query-cpu-model-expansion support was implemented for x86.
Oh, now I see that the x86 code uses
QEMU_MONITOR_CPU_MODEL_EXPANSION_STATIC_FULL and not just
QEMU_MONITOR_CPU_MODEL_EXPANSION_FULL. Nice!
--
Eduardo
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [Qemu-devel] [PATCH RESEND v1 1/2] i386: Add Intel Processor Trace feature support
@ 2018-01-15 14:31 ` Eduardo Habkost
0 siblings, 0 replies; 44+ messages in thread
From: Eduardo Habkost @ 2018-01-15 14:31 UTC (permalink / raw)
To: Paolo Bonzini, Kang, Luwei, qemu-devel, kvm, rth, mtosatti,
Chao Peng, libvir-list
On Mon, Jan 15, 2018 at 03:25:18PM +0100, Jiri Denemark wrote:
> On Mon, Jan 15, 2018 at 12:04:55 -0200, Eduardo Habkost wrote:
> > CCing libvirt developers.
> ...
> > This case is slightly more problematic, however: the new feature
> > is actually migratable (under very controlled circumstances)
> > because of patch 2/2, but it is not migration-safe[1]. This
> > means libvirt shouldn't include it in "host-model" expansion
> > (which uses the query-cpu-model-expansion QMP command) until we
> > make the feature migration-safe.
> >
> > For QEMU, this means the feature shouldn't be returned by
> > "query-cpu-model-expansion type=static model=max" (but it can be
> > returned by "query-cpu-model-expansion type=full model=max").
> >
> > Jiri, it looks like libvirt uses type=full on
> > query-cpu-model-expansion on x86. It needs to use
> > type=static[2], or it will have no way to find out if a feature
> > is migration-safe or not.
> ...
> > [2] It looks like libvirt uses type=full because it wants to get
> > all QOM property aliases returned. In this case, one
> > solution for libvirt is to use:
> >
> > static_expansion = query_cpu_model_expansion(type=static, model)
> > all_props = query_cpu_model_expansion(type=full, static_expansion)
>
> This is exactly what libvirt is doing (with model = "host") ever since
> query-cpu-model-expansion support was implemented for x86.
Oh, now I see that the x86 code uses
QEMU_MONITOR_CPU_MODEL_EXPANSION_STATIC_FULL and not just
QEMU_MONITOR_CPU_MODEL_EXPANSION_FULL. Nice!
--
Eduardo
^ permalink raw reply [flat|nested] 44+ messages in thread
* RE: [PATCH RESEND v1 1/2] i386: Add Intel Processor Trace feature support
2018-01-15 14:31 ` [Qemu-devel] " Eduardo Habkost
@ 2018-01-16 6:10 ` Kang, Luwei
-1 siblings, 0 replies; 44+ messages in thread
From: Kang, Luwei @ 2018-01-16 6:10 UTC (permalink / raw)
To: Eduardo Habkost, Paolo Bonzini, qemu-devel, kvm, rth, mtosatti,
Chao Peng, libvir-list
> > On Mon, Jan 15, 2018 at 12:04:55 -0200, Eduardo Habkost wrote:
> > > CCing libvirt developers.
> > ...
> > > This case is slightly more problematic, however: the new feature is
> > > actually migratable (under very controlled circumstances) because of
> > > patch 2/2, but it is not migration-safe[1]. This means libvirt
> > > shouldn't include it in "host-model" expansion (which uses the
> > > query-cpu-model-expansion QMP command) until we make the feature
> > > migration-safe.
> > >
> > > For QEMU, this means the feature shouldn't be returned by
> > > "query-cpu-model-expansion type=static model=max" (but it can be
> > > returned by "query-cpu-model-expansion type=full model=max").
> > >
> > > Jiri, it looks like libvirt uses type=full on
> > > query-cpu-model-expansion on x86. It needs to use type=static[2],
> > > or it will have no way to find out if a feature is migration-safe or
> > > not.
> > ...
> > > [2] It looks like libvirt uses type=full because it wants to get
> > > all QOM property aliases returned. In this case, one
> > > solution for libvirt is to use:
> > >
> > > static_expansion = query_cpu_model_expansion(type=static, model)
> > > all_props = query_cpu_model_expansion(type=full,
> > > static_expansion)
> >
> > This is exactly what libvirt is doing (with model = "host") ever since
> > query-cpu-model-expansion support was implemented for x86.
>
> Oh, now I see that the x86 code uses
> QEMU_MONITOR_CPU_MODEL_EXPANSION_STATIC_FULL and not just QEMU_MONITOR_CPU_MODEL_EXPANSION_FULL. Nice!
>
So, I need to add Intel PT feature in "X86CPUDefinition builtin_x86_defs[]" so that we can get this CPUID in specific CPU model not only "-cpu host". Is that right?
Intel PT is first supported in Intel Core M and 5th generation Intel Core processors that are based on the Intel micro-architecture code name Broadwell but Intel PT use EPT is first supported in Ice Lake. Intel PT virtualization depend on PT use EPT. I will add Intel PT to "Broadwell" CPU model and later to make sure a "Broadwell" guest can use Intel PT if the host is Ice Lake.
Thanks,
Luwei Kang
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [Qemu-devel] [PATCH RESEND v1 1/2] i386: Add Intel Processor Trace feature support
@ 2018-01-16 6:10 ` Kang, Luwei
0 siblings, 0 replies; 44+ messages in thread
From: Kang, Luwei @ 2018-01-16 6:10 UTC (permalink / raw)
To: Eduardo Habkost, Paolo Bonzini, qemu-devel, kvm, rth, mtosatti,
Chao Peng, libvir-list
> > On Mon, Jan 15, 2018 at 12:04:55 -0200, Eduardo Habkost wrote:
> > > CCing libvirt developers.
> > ...
> > > This case is slightly more problematic, however: the new feature is
> > > actually migratable (under very controlled circumstances) because of
> > > patch 2/2, but it is not migration-safe[1]. This means libvirt
> > > shouldn't include it in "host-model" expansion (which uses the
> > > query-cpu-model-expansion QMP command) until we make the feature
> > > migration-safe.
> > >
> > > For QEMU, this means the feature shouldn't be returned by
> > > "query-cpu-model-expansion type=static model=max" (but it can be
> > > returned by "query-cpu-model-expansion type=full model=max").
> > >
> > > Jiri, it looks like libvirt uses type=full on
> > > query-cpu-model-expansion on x86. It needs to use type=static[2],
> > > or it will have no way to find out if a feature is migration-safe or
> > > not.
> > ...
> > > [2] It looks like libvirt uses type=full because it wants to get
> > > all QOM property aliases returned. In this case, one
> > > solution for libvirt is to use:
> > >
> > > static_expansion = query_cpu_model_expansion(type=static, model)
> > > all_props = query_cpu_model_expansion(type=full,
> > > static_expansion)
> >
> > This is exactly what libvirt is doing (with model = "host") ever since
> > query-cpu-model-expansion support was implemented for x86.
>
> Oh, now I see that the x86 code uses
> QEMU_MONITOR_CPU_MODEL_EXPANSION_STATIC_FULL and not just QEMU_MONITOR_CPU_MODEL_EXPANSION_FULL. Nice!
>
So, I need to add Intel PT feature in "X86CPUDefinition builtin_x86_defs[]" so that we can get this CPUID in specific CPU model not only "-cpu host". Is that right?
Intel PT is first supported in Intel Core M and 5th generation Intel Core processors that are based on the Intel micro-architecture code name Broadwell but Intel PT use EPT is first supported in Ice Lake. Intel PT virtualization depend on PT use EPT. I will add Intel PT to "Broadwell" CPU model and later to make sure a "Broadwell" guest can use Intel PT if the host is Ice Lake.
Thanks,
Luwei Kang
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH RESEND v1 1/2] i386: Add Intel Processor Trace feature support
2018-01-16 6:10 ` [Qemu-devel] " Kang, Luwei
@ 2018-01-16 11:51 ` Eduardo Habkost
-1 siblings, 0 replies; 44+ messages in thread
From: Eduardo Habkost @ 2018-01-16 11:51 UTC (permalink / raw)
To: Kang, Luwei
Cc: Paolo Bonzini, qemu-devel, kvm, rth, mtosatti, Chao Peng, libvir-list
On Tue, Jan 16, 2018 at 06:10:17AM +0000, Kang, Luwei wrote:
> > > On Mon, Jan 15, 2018 at 12:04:55 -0200, Eduardo Habkost wrote:
> > > > CCing libvirt developers.
> > > ...
> > > > This case is slightly more problematic, however: the new feature is
> > > > actually migratable (under very controlled circumstances) because of
> > > > patch 2/2, but it is not migration-safe[1]. This means libvirt
> > > > shouldn't include it in "host-model" expansion (which uses the
> > > > query-cpu-model-expansion QMP command) until we make the feature
> > > > migration-safe.
> > > >
> > > > For QEMU, this means the feature shouldn't be returned by
> > > > "query-cpu-model-expansion type=static model=max" (but it can be
> > > > returned by "query-cpu-model-expansion type=full model=max").
> > > >
> > > > Jiri, it looks like libvirt uses type=full on
> > > > query-cpu-model-expansion on x86. It needs to use type=static[2],
> > > > or it will have no way to find out if a feature is migration-safe or
> > > > not.
> > > ...
> > > > [2] It looks like libvirt uses type=full because it wants to get
> > > > all QOM property aliases returned. In this case, one
> > > > solution for libvirt is to use:
> > > >
> > > > static_expansion = query_cpu_model_expansion(type=static, model)
> > > > all_props = query_cpu_model_expansion(type=full,
> > > > static_expansion)
> > >
> > > This is exactly what libvirt is doing (with model = "host") ever since
> > > query-cpu-model-expansion support was implemented for x86.
> >
> > Oh, now I see that the x86 code uses
> > QEMU_MONITOR_CPU_MODEL_EXPANSION_STATIC_FULL and not just QEMU_MONITOR_CPU_MODEL_EXPANSION_FULL. Nice!
> >
>
> So, I need to add Intel PT feature in "X86CPUDefinition
> builtin_x86_defs[]" so that we can get this CPUID in specific
> CPU model not only "-cpu host". Is that right?
The problem is that you won't be able to add intel-pt to any CPU
model unless the feature is made migration-safe (by not calling
kvm_arch_get_supported_cpuid() from cpu_x86_cpuid()).
What's missing here is to either: (a) make cpu_x86_cpuid() return
host-independent data (it can be constant, or it can be
configurable on the command-line); or (b) add a mechanism to skip
intel-pt from "query-cpu-model-expansion type=static". Probably
(a) is easier to implement, and it also makes the feature more
useful (by making it migration-safe).
>
> Intel PT is first supported in Intel Core M and 5th generation
> Intel Core processors that are based on the Intel
> micro-architecture code name Broadwell but Intel PT use EPT is
> first supported in Ice Lake. Intel PT virtualization depend on
> PT use EPT. I will add Intel PT to "Broadwell" CPU model and
> later to make sure a "Broadwell" guest can use Intel PT if the
> host is Ice Lake.
The "if the host is Ice Lake" part is problematic. On
migration-safe CPU models (all of them except "max" and "host"),
the data seen on CPUID can't depend on the host at all. It
should depend only on the machine-type + command-line options.
--
Eduardo
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [Qemu-devel] [PATCH RESEND v1 1/2] i386: Add Intel Processor Trace feature support
@ 2018-01-16 11:51 ` Eduardo Habkost
0 siblings, 0 replies; 44+ messages in thread
From: Eduardo Habkost @ 2018-01-16 11:51 UTC (permalink / raw)
To: Kang, Luwei
Cc: Paolo Bonzini, qemu-devel, kvm, rth, mtosatti, Chao Peng, libvir-list
On Tue, Jan 16, 2018 at 06:10:17AM +0000, Kang, Luwei wrote:
> > > On Mon, Jan 15, 2018 at 12:04:55 -0200, Eduardo Habkost wrote:
> > > > CCing libvirt developers.
> > > ...
> > > > This case is slightly more problematic, however: the new feature is
> > > > actually migratable (under very controlled circumstances) because of
> > > > patch 2/2, but it is not migration-safe[1]. This means libvirt
> > > > shouldn't include it in "host-model" expansion (which uses the
> > > > query-cpu-model-expansion QMP command) until we make the feature
> > > > migration-safe.
> > > >
> > > > For QEMU, this means the feature shouldn't be returned by
> > > > "query-cpu-model-expansion type=static model=max" (but it can be
> > > > returned by "query-cpu-model-expansion type=full model=max").
> > > >
> > > > Jiri, it looks like libvirt uses type=full on
> > > > query-cpu-model-expansion on x86. It needs to use type=static[2],
> > > > or it will have no way to find out if a feature is migration-safe or
> > > > not.
> > > ...
> > > > [2] It looks like libvirt uses type=full because it wants to get
> > > > all QOM property aliases returned. In this case, one
> > > > solution for libvirt is to use:
> > > >
> > > > static_expansion = query_cpu_model_expansion(type=static, model)
> > > > all_props = query_cpu_model_expansion(type=full,
> > > > static_expansion)
> > >
> > > This is exactly what libvirt is doing (with model = "host") ever since
> > > query-cpu-model-expansion support was implemented for x86.
> >
> > Oh, now I see that the x86 code uses
> > QEMU_MONITOR_CPU_MODEL_EXPANSION_STATIC_FULL and not just QEMU_MONITOR_CPU_MODEL_EXPANSION_FULL. Nice!
> >
>
> So, I need to add Intel PT feature in "X86CPUDefinition
> builtin_x86_defs[]" so that we can get this CPUID in specific
> CPU model not only "-cpu host". Is that right?
The problem is that you won't be able to add intel-pt to any CPU
model unless the feature is made migration-safe (by not calling
kvm_arch_get_supported_cpuid() from cpu_x86_cpuid()).
What's missing here is to either: (a) make cpu_x86_cpuid() return
host-independent data (it can be constant, or it can be
configurable on the command-line); or (b) add a mechanism to skip
intel-pt from "query-cpu-model-expansion type=static". Probably
(a) is easier to implement, and it also makes the feature more
useful (by making it migration-safe).
>
> Intel PT is first supported in Intel Core M and 5th generation
> Intel Core processors that are based on the Intel
> micro-architecture code name Broadwell but Intel PT use EPT is
> first supported in Ice Lake. Intel PT virtualization depend on
> PT use EPT. I will add Intel PT to "Broadwell" CPU model and
> later to make sure a "Broadwell" guest can use Intel PT if the
> host is Ice Lake.
The "if the host is Ice Lake" part is problematic. On
migration-safe CPU models (all of them except "max" and "host"),
the data seen on CPUID can't depend on the host at all. It
should depend only on the machine-type + command-line options.
--
Eduardo
^ permalink raw reply [flat|nested] 44+ messages in thread
* RE: [PATCH RESEND v1 1/2] i386: Add Intel Processor Trace feature support
2018-01-16 11:51 ` [Qemu-devel] " Eduardo Habkost
@ 2018-01-17 10:32 ` Kang, Luwei
-1 siblings, 0 replies; 44+ messages in thread
From: Kang, Luwei @ 2018-01-17 10:32 UTC (permalink / raw)
To: Eduardo Habkost
Cc: Paolo Bonzini, qemu-devel, kvm, rth, mtosatti, Chao Peng, libvir-list
> > > > On Mon, Jan 15, 2018 at 12:04:55 -0200, Eduardo Habkost wrote:
> > > > > CCing libvirt developers.
> > > > ...
> > > > > This case is slightly more problematic, however: the new feature
> > > > > is actually migratable (under very controlled circumstances)
> > > > > because of patch 2/2, but it is not migration-safe[1]. This
> > > > > means libvirt shouldn't include it in "host-model" expansion
> > > > > (which uses the query-cpu-model-expansion QMP command) until we
> > > > > make the feature migration-safe.
> > > > >
> > > > > For QEMU, this means the feature shouldn't be returned by
> > > > > "query-cpu-model-expansion type=static model=max" (but it can be
> > > > > returned by "query-cpu-model-expansion type=full model=max").
> > > > >
> > > > > Jiri, it looks like libvirt uses type=full on
> > > > > query-cpu-model-expansion on x86. It needs to use
> > > > > type=static[2], or it will have no way to find out if a feature
> > > > > is migration-safe or not.
> > > > ...
> > > > > [2] It looks like libvirt uses type=full because it wants to get
> > > > > all QOM property aliases returned. In this case, one
> > > > > solution for libvirt is to use:
> > > > >
> > > > > static_expansion = query_cpu_model_expansion(type=static, model)
> > > > > all_props = query_cpu_model_expansion(type=full,
> > > > > static_expansion)
> > > >
> > > > This is exactly what libvirt is doing (with model = "host") ever
> > > > since query-cpu-model-expansion support was implemented for x86.
> > >
> > > Oh, now I see that the x86 code uses
> > > QEMU_MONITOR_CPU_MODEL_EXPANSION_STATIC_FULL and not just QEMU_MONITOR_CPU_MODEL_EXPANSION_FULL.
> Nice!
> > >
> >
> > So, I need to add Intel PT feature in "X86CPUDefinition
> > builtin_x86_defs[]" so that we can get this CPUID in specific CPU
> > model not only "-cpu host". Is that right?
>
> The problem is that you won't be able to add intel-pt to any CPU model unless the feature is made migration-safe (by not calling
> kvm_arch_get_supported_cpuid() from cpu_x86_cpuid()).
Hi Eduardo,
Have some question need you help clear. What is "migration-safe" feature mean? I find all the feature which included in CPU model (builtin_x86_defs[]) will make "xcc->migration_safe = true;" in x86_cpu_cpudef_class_init(). If create a Skylake guest on Skylake HW and live migrate this guest to another machine with old HW(e.g Haswell). Can we think the new feature or cpu model(Skylake guest) which only supported in Skylake HW is safe?
>
> What's missing here is to either: (a) make cpu_x86_cpuid() return host-independent data (it can be constant, or it can be
> configurable on the command-line); or (b) add a mechanism to skip intel-pt from "query-cpu-model-expansion type=static".
==> it can be constant:
I think it shouldn't be constant because Intel PT virtualization can only supported in Ice Lake hardware now. Intel PT cpuid info would be mask off on old platform.
==> or it can be configurable on the command-line:
Because of I didn't add this feature in any CPU model. We only can enable this feature by "-cpu Skylake-Server,+intel-pt" at present.
What about add a new cpu model name "Icelake" and add PT in this. Is that would be migration safe?
Thanks,
Luwei Kang
> Probably
> (a) is easier to implement, and it also makes the feature more useful (by making it migration-safe).
>
> >
> > Intel PT is first supported in Intel Core M and 5th generation Intel
> > Core processors that are based on the Intel micro-architecture code
> > name Broadwell but Intel PT use EPT is first supported in Ice Lake.
> > Intel PT virtualization depend on PT use EPT. I will add Intel PT to
> > "Broadwell" CPU model and later to make sure a "Broadwell" guest can
> > use Intel PT if the host is Ice Lake.
>
> The "if the host is Ice Lake" part is problematic. On migration-safe CPU models (all of them except "max" and "host"), the data seen
> on CPUID can't depend on the host at all. It should depend only on the machine-type + command-line options.
>
> --
> Eduardo
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [Qemu-devel] [PATCH RESEND v1 1/2] i386: Add Intel Processor Trace feature support
@ 2018-01-17 10:32 ` Kang, Luwei
0 siblings, 0 replies; 44+ messages in thread
From: Kang, Luwei @ 2018-01-17 10:32 UTC (permalink / raw)
To: Eduardo Habkost
Cc: Paolo Bonzini, qemu-devel, kvm, rth, mtosatti, Chao Peng, libvir-list
> > > > On Mon, Jan 15, 2018 at 12:04:55 -0200, Eduardo Habkost wrote:
> > > > > CCing libvirt developers.
> > > > ...
> > > > > This case is slightly more problematic, however: the new feature
> > > > > is actually migratable (under very controlled circumstances)
> > > > > because of patch 2/2, but it is not migration-safe[1]. This
> > > > > means libvirt shouldn't include it in "host-model" expansion
> > > > > (which uses the query-cpu-model-expansion QMP command) until we
> > > > > make the feature migration-safe.
> > > > >
> > > > > For QEMU, this means the feature shouldn't be returned by
> > > > > "query-cpu-model-expansion type=static model=max" (but it can be
> > > > > returned by "query-cpu-model-expansion type=full model=max").
> > > > >
> > > > > Jiri, it looks like libvirt uses type=full on
> > > > > query-cpu-model-expansion on x86. It needs to use
> > > > > type=static[2], or it will have no way to find out if a feature
> > > > > is migration-safe or not.
> > > > ...
> > > > > [2] It looks like libvirt uses type=full because it wants to get
> > > > > all QOM property aliases returned. In this case, one
> > > > > solution for libvirt is to use:
> > > > >
> > > > > static_expansion = query_cpu_model_expansion(type=static, model)
> > > > > all_props = query_cpu_model_expansion(type=full,
> > > > > static_expansion)
> > > >
> > > > This is exactly what libvirt is doing (with model = "host") ever
> > > > since query-cpu-model-expansion support was implemented for x86.
> > >
> > > Oh, now I see that the x86 code uses
> > > QEMU_MONITOR_CPU_MODEL_EXPANSION_STATIC_FULL and not just QEMU_MONITOR_CPU_MODEL_EXPANSION_FULL.
> Nice!
> > >
> >
> > So, I need to add Intel PT feature in "X86CPUDefinition
> > builtin_x86_defs[]" so that we can get this CPUID in specific CPU
> > model not only "-cpu host". Is that right?
>
> The problem is that you won't be able to add intel-pt to any CPU model unless the feature is made migration-safe (by not calling
> kvm_arch_get_supported_cpuid() from cpu_x86_cpuid()).
Hi Eduardo,
Have some question need you help clear. What is "migration-safe" feature mean? I find all the feature which included in CPU model (builtin_x86_defs[]) will make "xcc->migration_safe = true;" in x86_cpu_cpudef_class_init(). If create a Skylake guest on Skylake HW and live migrate this guest to another machine with old HW(e.g Haswell). Can we think the new feature or cpu model(Skylake guest) which only supported in Skylake HW is safe?
>
> What's missing here is to either: (a) make cpu_x86_cpuid() return host-independent data (it can be constant, or it can be
> configurable on the command-line); or (b) add a mechanism to skip intel-pt from "query-cpu-model-expansion type=static".
==> it can be constant:
I think it shouldn't be constant because Intel PT virtualization can only supported in Ice Lake hardware now. Intel PT cpuid info would be mask off on old platform.
==> or it can be configurable on the command-line:
Because of I didn't add this feature in any CPU model. We only can enable this feature by "-cpu Skylake-Server,+intel-pt" at present.
What about add a new cpu model name "Icelake" and add PT in this. Is that would be migration safe?
Thanks,
Luwei Kang
> Probably
> (a) is easier to implement, and it also makes the feature more useful (by making it migration-safe).
>
> >
> > Intel PT is first supported in Intel Core M and 5th generation Intel
> > Core processors that are based on the Intel micro-architecture code
> > name Broadwell but Intel PT use EPT is first supported in Ice Lake.
> > Intel PT virtualization depend on PT use EPT. I will add Intel PT to
> > "Broadwell" CPU model and later to make sure a "Broadwell" guest can
> > use Intel PT if the host is Ice Lake.
>
> The "if the host is Ice Lake" part is problematic. On migration-safe CPU models (all of them except "max" and "host"), the data seen
> on CPUID can't depend on the host at all. It should depend only on the machine-type + command-line options.
>
> --
> Eduardo
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH RESEND v1 1/2] i386: Add Intel Processor Trace feature support
2018-01-17 10:32 ` [Qemu-devel] " Kang, Luwei
@ 2018-01-18 2:42 ` Eduardo Habkost
-1 siblings, 0 replies; 44+ messages in thread
From: Eduardo Habkost @ 2018-01-18 2:42 UTC (permalink / raw)
To: Kang, Luwei
Cc: Paolo Bonzini, qemu-devel, kvm, rth, mtosatti, Chao Peng, libvir-list
On Wed, Jan 17, 2018 at 10:32:56AM +0000, Kang, Luwei wrote:
> > > > > On Mon, Jan 15, 2018 at 12:04:55 -0200, Eduardo Habkost wrote:
> > > > > > CCing libvirt developers.
> > > > > ...
> > > > > > This case is slightly more problematic, however: the new feature
> > > > > > is actually migratable (under very controlled circumstances)
> > > > > > because of patch 2/2, but it is not migration-safe[1]. This
> > > > > > means libvirt shouldn't include it in "host-model" expansion
> > > > > > (which uses the query-cpu-model-expansion QMP command) until we
> > > > > > make the feature migration-safe.
> > > > > >
> > > > > > For QEMU, this means the feature shouldn't be returned by
> > > > > > "query-cpu-model-expansion type=static model=max" (but it can be
> > > > > > returned by "query-cpu-model-expansion type=full model=max").
> > > > > >
> > > > > > Jiri, it looks like libvirt uses type=full on
> > > > > > query-cpu-model-expansion on x86. It needs to use
> > > > > > type=static[2], or it will have no way to find out if a feature
> > > > > > is migration-safe or not.
> > > > > ...
> > > > > > [2] It looks like libvirt uses type=full because it wants to get
> > > > > > all QOM property aliases returned. In this case, one
> > > > > > solution for libvirt is to use:
> > > > > >
> > > > > > static_expansion = query_cpu_model_expansion(type=static, model)
> > > > > > all_props = query_cpu_model_expansion(type=full,
> > > > > > static_expansion)
> > > > >
> > > > > This is exactly what libvirt is doing (with model = "host") ever
> > > > > since query-cpu-model-expansion support was implemented for x86.
> > > >
> > > > Oh, now I see that the x86 code uses
> > > > QEMU_MONITOR_CPU_MODEL_EXPANSION_STATIC_FULL and not just QEMU_MONITOR_CPU_MODEL_EXPANSION_FULL.
> > Nice!
> > > >
> > >
> > > So, I need to add Intel PT feature in "X86CPUDefinition
> > > builtin_x86_defs[]" so that we can get this CPUID in specific CPU
> > > model not only "-cpu host". Is that right?
> >
> > The problem is that you won't be able to add intel-pt to any CPU model unless the feature is made migration-safe (by not calling
> > kvm_arch_get_supported_cpuid() from cpu_x86_cpuid()).
>
> Hi Eduardo,
> Have some question need you help clear. What is
> "migration-safe" feature mean? I find all the feature which
> included in CPU model (builtin_x86_defs[]) will make
> "xcc->migration_safe = true;" in
> x86_cpu_cpudef_class_init(). If create a Skylake guest on
> Skylake HW and live migrate this guest to another machine
> with old HW(e.g Haswell). Can we think the new feature or
> cpu model(Skylake guest) which only supported in Skylake HW
> is safe?
I mean matching the requirements so we can say the feature is migration-safe,
that means exposing the same CPUID data to the guest on any host, if the
machine-type + command-line is the same. The data on CPUID[7] is OK (it
changes according to the command-line options only), but the data exposed on
CPUID[14h] on this patch is not migration-safe (it changes depending on the
host it is running).
>
> >
> > What's missing here is to either: (a) make cpu_x86_cpuid() return host-independent data (it can be constant, or it can be
> > configurable on the command-line); or (b) add a mechanism to skip intel-pt from "query-cpu-model-expansion type=static".
>
> ==> it can be constant:
> I think it shouldn't be constant because Intel PT virtualization can only supported in Ice Lake hardware now. Intel PT cpuid info would be mask off on old platform.
> ==> or it can be configurable on the command-line:
> Because of I didn't add this feature in any CPU model. We only can enable this feature by "-cpu Skylake-Server,+intel-pt" at present.
Note that I'm talking about CPUID[14h], not CPUID[7]. The CPUID[7] bits are
safe because they are set according to the CPU model + command-line options
only. The bits on CPUID[14h] change depending on the host and are not
migration-safe. CPUID[7].EBX[bit 25] is just the (already configurable) bit
that enables the migration-unsafe code for CPUID[14h].
>
> What about add a new cpu model name "Icelake" and add PT in this. Is that would be migration safe?
It won't, because of the CPUID[14h] code that makes it unsafe to migrate
between hosts with different Intel-PT capabilities (i.e. different data on
CPUID[14h]).
>
> Thanks,
> Luwei Kang
>
> > Probably
> > (a) is easier to implement, and it also makes the feature more useful (by making it migration-safe).
> >
> > >
> > > Intel PT is first supported in Intel Core M and 5th generation Intel
> > > Core processors that are based on the Intel micro-architecture code
> > > name Broadwell but Intel PT use EPT is first supported in Ice Lake.
> > > Intel PT virtualization depend on PT use EPT. I will add Intel PT to
> > > "Broadwell" CPU model and later to make sure a "Broadwell" guest can
> > > use Intel PT if the host is Ice Lake.
> >
> > The "if the host is Ice Lake" part is problematic. On migration-safe CPU models (all of them except "max" and "host"), the data seen
> > on CPUID can't depend on the host at all. It should depend only on the machine-type + command-line options.
> >
> > --
> > Eduardo
--
Eduardo
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [Qemu-devel] [PATCH RESEND v1 1/2] i386: Add Intel Processor Trace feature support
@ 2018-01-18 2:42 ` Eduardo Habkost
0 siblings, 0 replies; 44+ messages in thread
From: Eduardo Habkost @ 2018-01-18 2:42 UTC (permalink / raw)
To: Kang, Luwei
Cc: Paolo Bonzini, qemu-devel, kvm, rth, mtosatti, Chao Peng, libvir-list
On Wed, Jan 17, 2018 at 10:32:56AM +0000, Kang, Luwei wrote:
> > > > > On Mon, Jan 15, 2018 at 12:04:55 -0200, Eduardo Habkost wrote:
> > > > > > CCing libvirt developers.
> > > > > ...
> > > > > > This case is slightly more problematic, however: the new feature
> > > > > > is actually migratable (under very controlled circumstances)
> > > > > > because of patch 2/2, but it is not migration-safe[1]. This
> > > > > > means libvirt shouldn't include it in "host-model" expansion
> > > > > > (which uses the query-cpu-model-expansion QMP command) until we
> > > > > > make the feature migration-safe.
> > > > > >
> > > > > > For QEMU, this means the feature shouldn't be returned by
> > > > > > "query-cpu-model-expansion type=static model=max" (but it can be
> > > > > > returned by "query-cpu-model-expansion type=full model=max").
> > > > > >
> > > > > > Jiri, it looks like libvirt uses type=full on
> > > > > > query-cpu-model-expansion on x86. It needs to use
> > > > > > type=static[2], or it will have no way to find out if a feature
> > > > > > is migration-safe or not.
> > > > > ...
> > > > > > [2] It looks like libvirt uses type=full because it wants to get
> > > > > > all QOM property aliases returned. In this case, one
> > > > > > solution for libvirt is to use:
> > > > > >
> > > > > > static_expansion = query_cpu_model_expansion(type=static, model)
> > > > > > all_props = query_cpu_model_expansion(type=full,
> > > > > > static_expansion)
> > > > >
> > > > > This is exactly what libvirt is doing (with model = "host") ever
> > > > > since query-cpu-model-expansion support was implemented for x86.
> > > >
> > > > Oh, now I see that the x86 code uses
> > > > QEMU_MONITOR_CPU_MODEL_EXPANSION_STATIC_FULL and not just QEMU_MONITOR_CPU_MODEL_EXPANSION_FULL.
> > Nice!
> > > >
> > >
> > > So, I need to add Intel PT feature in "X86CPUDefinition
> > > builtin_x86_defs[]" so that we can get this CPUID in specific CPU
> > > model not only "-cpu host". Is that right?
> >
> > The problem is that you won't be able to add intel-pt to any CPU model unless the feature is made migration-safe (by not calling
> > kvm_arch_get_supported_cpuid() from cpu_x86_cpuid()).
>
> Hi Eduardo,
> Have some question need you help clear. What is
> "migration-safe" feature mean? I find all the feature which
> included in CPU model (builtin_x86_defs[]) will make
> "xcc->migration_safe = true;" in
> x86_cpu_cpudef_class_init(). If create a Skylake guest on
> Skylake HW and live migrate this guest to another machine
> with old HW(e.g Haswell). Can we think the new feature or
> cpu model(Skylake guest) which only supported in Skylake HW
> is safe?
I mean matching the requirements so we can say the feature is migration-safe,
that means exposing the same CPUID data to the guest on any host, if the
machine-type + command-line is the same. The data on CPUID[7] is OK (it
changes according to the command-line options only), but the data exposed on
CPUID[14h] on this patch is not migration-safe (it changes depending on the
host it is running).
>
> >
> > What's missing here is to either: (a) make cpu_x86_cpuid() return host-independent data (it can be constant, or it can be
> > configurable on the command-line); or (b) add a mechanism to skip intel-pt from "query-cpu-model-expansion type=static".
>
> ==> it can be constant:
> I think it shouldn't be constant because Intel PT virtualization can only supported in Ice Lake hardware now. Intel PT cpuid info would be mask off on old platform.
> ==> or it can be configurable on the command-line:
> Because of I didn't add this feature in any CPU model. We only can enable this feature by "-cpu Skylake-Server,+intel-pt" at present.
Note that I'm talking about CPUID[14h], not CPUID[7]. The CPUID[7] bits are
safe because they are set according to the CPU model + command-line options
only. The bits on CPUID[14h] change depending on the host and are not
migration-safe. CPUID[7].EBX[bit 25] is just the (already configurable) bit
that enables the migration-unsafe code for CPUID[14h].
>
> What about add a new cpu model name "Icelake" and add PT in this. Is that would be migration safe?
It won't, because of the CPUID[14h] code that makes it unsafe to migrate
between hosts with different Intel-PT capabilities (i.e. different data on
CPUID[14h]).
>
> Thanks,
> Luwei Kang
>
> > Probably
> > (a) is easier to implement, and it also makes the feature more useful (by making it migration-safe).
> >
> > >
> > > Intel PT is first supported in Intel Core M and 5th generation Intel
> > > Core processors that are based on the Intel micro-architecture code
> > > name Broadwell but Intel PT use EPT is first supported in Ice Lake.
> > > Intel PT virtualization depend on PT use EPT. I will add Intel PT to
> > > "Broadwell" CPU model and later to make sure a "Broadwell" guest can
> > > use Intel PT if the host is Ice Lake.
> >
> > The "if the host is Ice Lake" part is problematic. On migration-safe CPU models (all of them except "max" and "host"), the data seen
> > on CPUID can't depend on the host at all. It should depend only on the machine-type + command-line options.
> >
> > --
> > Eduardo
--
Eduardo
^ permalink raw reply [flat|nested] 44+ messages in thread
* RE: [PATCH RESEND v1 1/2] i386: Add Intel Processor Trace feature support
2018-01-18 2:42 ` [Qemu-devel] " Eduardo Habkost
@ 2018-01-18 5:33 ` Kang, Luwei
-1 siblings, 0 replies; 44+ messages in thread
From: Kang, Luwei @ 2018-01-18 5:33 UTC (permalink / raw)
To: Eduardo Habkost
Cc: Paolo Bonzini, qemu-devel, kvm, rth, mtosatti, Chao Peng, libvir-list
> > > > > > On Mon, Jan 15, 2018 at 12:04:55 -0200, Eduardo Habkost wrote:
> > > > > > > CCing libvirt developers.
> > > > > > ...
> > > > > > > This case is slightly more problematic, however: the new
> > > > > > > feature is actually migratable (under very controlled
> > > > > > > circumstances) because of patch 2/2, but it is not
> > > > > > > migration-safe[1]. This means libvirt shouldn't include it
> > > > > > > in "host-model" expansion (which uses the
> > > > > > > query-cpu-model-expansion QMP command) until we make the feature migration-safe.
> > > > > > >
> > > > > > > For QEMU, this means the feature shouldn't be returned by
> > > > > > > "query-cpu-model-expansion type=static model=max" (but it
> > > > > > > can be returned by "query-cpu-model-expansion type=full model=max").
> > > > > > >
> > > > > > > Jiri, it looks like libvirt uses type=full on
> > > > > > > query-cpu-model-expansion on x86. It needs to use
> > > > > > > type=static[2], or it will have no way to find out if a
> > > > > > > feature is migration-safe or not.
> > > > > > ...
> > > > > > > [2] It looks like libvirt uses type=full because it wants to get
> > > > > > > all QOM property aliases returned. In this case, one
> > > > > > > solution for libvirt is to use:
> > > > > > >
> > > > > > > static_expansion = query_cpu_model_expansion(type=static, model)
> > > > > > > all_props = query_cpu_model_expansion(type=full,
> > > > > > > static_expansion)
> > > > > >
> > > > > > This is exactly what libvirt is doing (with model = "host")
> > > > > > ever since query-cpu-model-expansion support was implemented for x86.
> > > > >
> > > > > Oh, now I see that the x86 code uses
> > > > > QEMU_MONITOR_CPU_MODEL_EXPANSION_STATIC_FULL and not just QEMU_MONITOR_CPU_MODEL_EXPANSION_FULL.
> > > Nice!
> > > > >
> > > >
> > > > So, I need to add Intel PT feature in "X86CPUDefinition
> > > > builtin_x86_defs[]" so that we can get this CPUID in specific CPU
> > > > model not only "-cpu host". Is that right?
> > >
> > > The problem is that you won't be able to add intel-pt to any CPU
> > > model unless the feature is made migration-safe (by not calling
> > > kvm_arch_get_supported_cpuid() from cpu_x86_cpuid()).
> >
> > Hi Eduardo,
> > Have some question need you help clear. What is
> > "migration-safe" feature mean? I find all the feature which
> > included in CPU model (builtin_x86_defs[]) will make
> > "xcc->migration_safe = true;" in
> > x86_cpu_cpudef_class_init(). If create a Skylake guest on
> > Skylake HW and live migrate this guest to another machine
> > with old HW(e.g Haswell). Can we think the new feature or
> > cpu model(Skylake guest) which only supported in Skylake HW
> > is safe?
>
> I mean matching the requirements so we can say the feature is migration-safe, that means exposing the same CPUID data to the
> guest on any host, if the machine-type + command-line is the same. The data on CPUID[7] is OK (it changes according to the
> command-line options only), but the data exposed on CPUID[14h] on this patch is not migration-safe (it changes depending on the
> host it is running).
>
Many thanks for your clarification. I think I have understood. But CPUID[14h] is depend on CPUID[7].EBX[bit25] and it would be zero if CPUID[7].EBX[bit25] is not set. Or what you concerned is make live migration on two different HW which all support Intel PT virtualization but have different CPUID[14h] result? This may need to think about.
Thanks,
Luwei Kang
>
> >
> > >
> > > What's missing here is to either: (a) make cpu_x86_cpuid() return
> > > host-independent data (it can be constant, or it can be configurable on the command-line); or (b) add a mechanism to skip intel-
> pt from "query-cpu-model-expansion type=static".
> >
> > ==> it can be constant:
> > I think it shouldn't be constant because Intel PT virtualization can only supported in Ice Lake hardware now. Intel PT cpuid info
> would be mask off on old platform.
> > ==> or it can be configurable on the command-line:
> > Because of I didn't add this feature in any CPU model. We only can enable this feature by "-cpu Skylake-Server,+intel-pt" at
> present.
>
> Note that I'm talking about CPUID[14h], not CPUID[7]. The CPUID[7] bits are safe because they are set according to the CPU model
> + command-line options only. The bits on CPUID[14h] change depending on the host and are not migration-safe. CPUID[7].EBX[bit
> 25] is just the (already configurable) bit that enables the migration-unsafe code for CPUID[14h].
>
> >
> > What about add a new cpu model name "Icelake" and add PT in this. Is that would be migration safe?
>
> It won't, because of the CPUID[14h] code that makes it unsafe to migrate between hosts with different Intel-PT capabilities (i.e.
> different data on CPUID[14h]).
>
>
> >
> > Thanks,
> > Luwei Kang
> >
> > > Probably
> > > (a) is easier to implement, and it also makes the feature more useful (by making it migration-safe).
> > >
> > > >
> > > > Intel PT is first supported in Intel Core M and 5th generation
> > > > Intel Core processors that are based on the Intel
> > > > micro-architecture code name Broadwell but Intel PT use EPT is first supported in Ice Lake.
> > > > Intel PT virtualization depend on PT use EPT. I will add Intel PT
> > > > to "Broadwell" CPU model and later to make sure a "Broadwell"
> > > > guest can use Intel PT if the host is Ice Lake.
> > >
> > > The "if the host is Ice Lake" part is problematic. On
> > > migration-safe CPU models (all of them except "max" and "host"), the data seen on CPUID can't depend on the host at all. It
> should depend only on the machine-type + command-line options.
> > >
> > > --
> > > Eduardo
>
> --
> Eduardo
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [Qemu-devel] [PATCH RESEND v1 1/2] i386: Add Intel Processor Trace feature support
@ 2018-01-18 5:33 ` Kang, Luwei
0 siblings, 0 replies; 44+ messages in thread
From: Kang, Luwei @ 2018-01-18 5:33 UTC (permalink / raw)
To: Eduardo Habkost
Cc: Paolo Bonzini, qemu-devel, kvm, rth, mtosatti, Chao Peng, libvir-list
> > > > > > On Mon, Jan 15, 2018 at 12:04:55 -0200, Eduardo Habkost wrote:
> > > > > > > CCing libvirt developers.
> > > > > > ...
> > > > > > > This case is slightly more problematic, however: the new
> > > > > > > feature is actually migratable (under very controlled
> > > > > > > circumstances) because of patch 2/2, but it is not
> > > > > > > migration-safe[1]. This means libvirt shouldn't include it
> > > > > > > in "host-model" expansion (which uses the
> > > > > > > query-cpu-model-expansion QMP command) until we make the feature migration-safe.
> > > > > > >
> > > > > > > For QEMU, this means the feature shouldn't be returned by
> > > > > > > "query-cpu-model-expansion type=static model=max" (but it
> > > > > > > can be returned by "query-cpu-model-expansion type=full model=max").
> > > > > > >
> > > > > > > Jiri, it looks like libvirt uses type=full on
> > > > > > > query-cpu-model-expansion on x86. It needs to use
> > > > > > > type=static[2], or it will have no way to find out if a
> > > > > > > feature is migration-safe or not.
> > > > > > ...
> > > > > > > [2] It looks like libvirt uses type=full because it wants to get
> > > > > > > all QOM property aliases returned. In this case, one
> > > > > > > solution for libvirt is to use:
> > > > > > >
> > > > > > > static_expansion = query_cpu_model_expansion(type=static, model)
> > > > > > > all_props = query_cpu_model_expansion(type=full,
> > > > > > > static_expansion)
> > > > > >
> > > > > > This is exactly what libvirt is doing (with model = "host")
> > > > > > ever since query-cpu-model-expansion support was implemented for x86.
> > > > >
> > > > > Oh, now I see that the x86 code uses
> > > > > QEMU_MONITOR_CPU_MODEL_EXPANSION_STATIC_FULL and not just QEMU_MONITOR_CPU_MODEL_EXPANSION_FULL.
> > > Nice!
> > > > >
> > > >
> > > > So, I need to add Intel PT feature in "X86CPUDefinition
> > > > builtin_x86_defs[]" so that we can get this CPUID in specific CPU
> > > > model not only "-cpu host". Is that right?
> > >
> > > The problem is that you won't be able to add intel-pt to any CPU
> > > model unless the feature is made migration-safe (by not calling
> > > kvm_arch_get_supported_cpuid() from cpu_x86_cpuid()).
> >
> > Hi Eduardo,
> > Have some question need you help clear. What is
> > "migration-safe" feature mean? I find all the feature which
> > included in CPU model (builtin_x86_defs[]) will make
> > "xcc->migration_safe = true;" in
> > x86_cpu_cpudef_class_init(). If create a Skylake guest on
> > Skylake HW and live migrate this guest to another machine
> > with old HW(e.g Haswell). Can we think the new feature or
> > cpu model(Skylake guest) which only supported in Skylake HW
> > is safe?
>
> I mean matching the requirements so we can say the feature is migration-safe, that means exposing the same CPUID data to the
> guest on any host, if the machine-type + command-line is the same. The data on CPUID[7] is OK (it changes according to the
> command-line options only), but the data exposed on CPUID[14h] on this patch is not migration-safe (it changes depending on the
> host it is running).
>
Many thanks for your clarification. I think I have understood. But CPUID[14h] is depend on CPUID[7].EBX[bit25] and it would be zero if CPUID[7].EBX[bit25] is not set. Or what you concerned is make live migration on two different HW which all support Intel PT virtualization but have different CPUID[14h] result? This may need to think about.
Thanks,
Luwei Kang
>
> >
> > >
> > > What's missing here is to either: (a) make cpu_x86_cpuid() return
> > > host-independent data (it can be constant, or it can be configurable on the command-line); or (b) add a mechanism to skip intel-
> pt from "query-cpu-model-expansion type=static".
> >
> > ==> it can be constant:
> > I think it shouldn't be constant because Intel PT virtualization can only supported in Ice Lake hardware now. Intel PT cpuid info
> would be mask off on old platform.
> > ==> or it can be configurable on the command-line:
> > Because of I didn't add this feature in any CPU model. We only can enable this feature by "-cpu Skylake-Server,+intel-pt" at
> present.
>
> Note that I'm talking about CPUID[14h], not CPUID[7]. The CPUID[7] bits are safe because they are set according to the CPU model
> + command-line options only. The bits on CPUID[14h] change depending on the host and are not migration-safe. CPUID[7].EBX[bit
> 25] is just the (already configurable) bit that enables the migration-unsafe code for CPUID[14h].
>
> >
> > What about add a new cpu model name "Icelake" and add PT in this. Is that would be migration safe?
>
> It won't, because of the CPUID[14h] code that makes it unsafe to migrate between hosts with different Intel-PT capabilities (i.e.
> different data on CPUID[14h]).
>
>
> >
> > Thanks,
> > Luwei Kang
> >
> > > Probably
> > > (a) is easier to implement, and it also makes the feature more useful (by making it migration-safe).
> > >
> > > >
> > > > Intel PT is first supported in Intel Core M and 5th generation
> > > > Intel Core processors that are based on the Intel
> > > > micro-architecture code name Broadwell but Intel PT use EPT is first supported in Ice Lake.
> > > > Intel PT virtualization depend on PT use EPT. I will add Intel PT
> > > > to "Broadwell" CPU model and later to make sure a "Broadwell"
> > > > guest can use Intel PT if the host is Ice Lake.
> > >
> > > The "if the host is Ice Lake" part is problematic. On
> > > migration-safe CPU models (all of them except "max" and "host"), the data seen on CPUID can't depend on the host at all. It
> should depend only on the machine-type + command-line options.
> > >
> > > --
> > > Eduardo
>
> --
> Eduardo
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH RESEND v1 1/2] i386: Add Intel Processor Trace feature support
2018-01-18 5:33 ` [Qemu-devel] " Kang, Luwei
@ 2018-01-18 13:24 ` Eduardo Habkost
-1 siblings, 0 replies; 44+ messages in thread
From: Eduardo Habkost @ 2018-01-18 13:24 UTC (permalink / raw)
To: Kang, Luwei
Cc: Paolo Bonzini, qemu-devel, kvm, rth, mtosatti, Chao Peng, libvir-list
On Thu, Jan 18, 2018 at 05:33:53AM +0000, Kang, Luwei wrote:
> > > > > > > On Mon, Jan 15, 2018 at 12:04:55 -0200, Eduardo Habkost wrote:
> > > > > > > > CCing libvirt developers.
> > > > > > > ...
> > > > > > > > This case is slightly more problematic, however: the new
> > > > > > > > feature is actually migratable (under very controlled
> > > > > > > > circumstances) because of patch 2/2, but it is not
> > > > > > > > migration-safe[1]. This means libvirt shouldn't include it
> > > > > > > > in "host-model" expansion (which uses the
> > > > > > > > query-cpu-model-expansion QMP command) until we make the feature migration-safe.
> > > > > > > >
> > > > > > > > For QEMU, this means the feature shouldn't be returned by
> > > > > > > > "query-cpu-model-expansion type=static model=max" (but it
> > > > > > > > can be returned by "query-cpu-model-expansion type=full model=max").
> > > > > > > >
> > > > > > > > Jiri, it looks like libvirt uses type=full on
> > > > > > > > query-cpu-model-expansion on x86. It needs to use
> > > > > > > > type=static[2], or it will have no way to find out if a
> > > > > > > > feature is migration-safe or not.
> > > > > > > ...
> > > > > > > > [2] It looks like libvirt uses type=full because it wants to get
> > > > > > > > all QOM property aliases returned. In this case, one
> > > > > > > > solution for libvirt is to use:
> > > > > > > >
> > > > > > > > static_expansion = query_cpu_model_expansion(type=static, model)
> > > > > > > > all_props = query_cpu_model_expansion(type=full,
> > > > > > > > static_expansion)
> > > > > > >
> > > > > > > This is exactly what libvirt is doing (with model = "host")
> > > > > > > ever since query-cpu-model-expansion support was implemented for x86.
> > > > > >
> > > > > > Oh, now I see that the x86 code uses
> > > > > > QEMU_MONITOR_CPU_MODEL_EXPANSION_STATIC_FULL and not just QEMU_MONITOR_CPU_MODEL_EXPANSION_FULL.
> > > > Nice!
> > > > > >
> > > > >
> > > > > So, I need to add Intel PT feature in "X86CPUDefinition
> > > > > builtin_x86_defs[]" so that we can get this CPUID in specific CPU
> > > > > model not only "-cpu host". Is that right?
> > > >
> > > > The problem is that you won't be able to add intel-pt to any CPU
> > > > model unless the feature is made migration-safe (by not calling
> > > > kvm_arch_get_supported_cpuid() from cpu_x86_cpuid()).
> > >
> > > Hi Eduardo,
> > > Have some question need you help clear. What is
> > > "migration-safe" feature mean? I find all the feature which
> > > included in CPU model (builtin_x86_defs[]) will make
> > > "xcc->migration_safe = true;" in
> > > x86_cpu_cpudef_class_init(). If create a Skylake guest on
> > > Skylake HW and live migrate this guest to another machine
> > > with old HW(e.g Haswell). Can we think the new feature or
> > > cpu model(Skylake guest) which only supported in Skylake HW
> > > is safe?
> >
> > I mean matching the requirements so we can say the feature is migration-safe, that means exposing the same CPUID data to the
> > guest on any host, if the machine-type + command-line is the same. The data on CPUID[7] is OK (it changes according to the
> > command-line options only), but the data exposed on CPUID[14h] on this patch is not migration-safe (it changes depending on the
> > host it is running).
> >
>
> Many thanks for your clarification. I think I have understood.
> But CPUID[14h] is depend on CPUID[7].EBX[bit25] and it would be
> zero if CPUID[7].EBX[bit25] is not set. Or what you concerned
> is make live migration on two different HW which all support
> Intel PT virtualization but have different CPUID[14h] result?
> This may need to think about.
Yes. If intel-pt is off, the results (CPUID[14h] all zeroes) are
migration-safe. Setting intel-pt=on makes the command-line not
migration-safe.
This is OK, in principle (e.g. the "pmu" option is not
migration-safe and behaves the same way). The only problem is
that this patch would make query-cpu-model-expansion return
intel-pt=on on type=static expansion.
Probably the easiest way to fix that is to specifically exclude
intel-pt on x86_cpu_static_props().
However, if there's a simple way to make it possible to migrate
between hosts with different CPUID[14h] data, it would be even
better. With the current KVM intel-pt implementation, what
happens if the CPUID[14h] data seen by the guest doesn't match
exactly the CPUID[14h] leaves from the host?
>
> Thanks,
> Luwei Kang
>
> >
> > >
> > > >
> > > > What's missing here is to either: (a) make cpu_x86_cpuid() return
> > > > host-independent data (it can be constant, or it can be configurable on the command-line); or (b) add a mechanism to skip intel-
> > pt from "query-cpu-model-expansion type=static".
> > >
> > > ==> it can be constant:
> > > I think it shouldn't be constant because Intel PT virtualization can only supported in Ice Lake hardware now. Intel PT cpuid info
> > would be mask off on old platform.
> > > ==> or it can be configurable on the command-line:
> > > Because of I didn't add this feature in any CPU model. We only can enable this feature by "-cpu Skylake-Server,+intel-pt" at
> > present.
> >
> > Note that I'm talking about CPUID[14h], not CPUID[7]. The CPUID[7] bits are safe because they are set according to the CPU model
> > + command-line options only. The bits on CPUID[14h] change depending on the host and are not migration-safe. CPUID[7].EBX[bit
> > 25] is just the (already configurable) bit that enables the migration-unsafe code for CPUID[14h].
> >
> > >
> > > What about add a new cpu model name "Icelake" and add PT in this. Is that would be migration safe?
> >
> > It won't, because of the CPUID[14h] code that makes it unsafe to migrate between hosts with different Intel-PT capabilities (i.e.
> > different data on CPUID[14h]).
> >
> >
> > >
> > > Thanks,
> > > Luwei Kang
> > >
> > > > Probably
> > > > (a) is easier to implement, and it also makes the feature more useful (by making it migration-safe).
> > > >
> > > > >
> > > > > Intel PT is first supported in Intel Core M and 5th generation
> > > > > Intel Core processors that are based on the Intel
> > > > > micro-architecture code name Broadwell but Intel PT use EPT is first supported in Ice Lake.
> > > > > Intel PT virtualization depend on PT use EPT. I will add Intel PT
> > > > > to "Broadwell" CPU model and later to make sure a "Broadwell"
> > > > > guest can use Intel PT if the host is Ice Lake.
> > > >
> > > > The "if the host is Ice Lake" part is problematic. On
> > > > migration-safe CPU models (all of them except "max" and "host"), the data seen on CPUID can't depend on the host at all. It
> > should depend only on the machine-type + command-line options.
> > > >
> > > > --
> > > > Eduardo
> >
> > --
> > Eduardo
>
--
Eduardo
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [Qemu-devel] [PATCH RESEND v1 1/2] i386: Add Intel Processor Trace feature support
@ 2018-01-18 13:24 ` Eduardo Habkost
0 siblings, 0 replies; 44+ messages in thread
From: Eduardo Habkost @ 2018-01-18 13:24 UTC (permalink / raw)
To: Kang, Luwei
Cc: Paolo Bonzini, qemu-devel, kvm, rth, mtosatti, Chao Peng, libvir-list
On Thu, Jan 18, 2018 at 05:33:53AM +0000, Kang, Luwei wrote:
> > > > > > > On Mon, Jan 15, 2018 at 12:04:55 -0200, Eduardo Habkost wrote:
> > > > > > > > CCing libvirt developers.
> > > > > > > ...
> > > > > > > > This case is slightly more problematic, however: the new
> > > > > > > > feature is actually migratable (under very controlled
> > > > > > > > circumstances) because of patch 2/2, but it is not
> > > > > > > > migration-safe[1]. This means libvirt shouldn't include it
> > > > > > > > in "host-model" expansion (which uses the
> > > > > > > > query-cpu-model-expansion QMP command) until we make the feature migration-safe.
> > > > > > > >
> > > > > > > > For QEMU, this means the feature shouldn't be returned by
> > > > > > > > "query-cpu-model-expansion type=static model=max" (but it
> > > > > > > > can be returned by "query-cpu-model-expansion type=full model=max").
> > > > > > > >
> > > > > > > > Jiri, it looks like libvirt uses type=full on
> > > > > > > > query-cpu-model-expansion on x86. It needs to use
> > > > > > > > type=static[2], or it will have no way to find out if a
> > > > > > > > feature is migration-safe or not.
> > > > > > > ...
> > > > > > > > [2] It looks like libvirt uses type=full because it wants to get
> > > > > > > > all QOM property aliases returned. In this case, one
> > > > > > > > solution for libvirt is to use:
> > > > > > > >
> > > > > > > > static_expansion = query_cpu_model_expansion(type=static, model)
> > > > > > > > all_props = query_cpu_model_expansion(type=full,
> > > > > > > > static_expansion)
> > > > > > >
> > > > > > > This is exactly what libvirt is doing (with model = "host")
> > > > > > > ever since query-cpu-model-expansion support was implemented for x86.
> > > > > >
> > > > > > Oh, now I see that the x86 code uses
> > > > > > QEMU_MONITOR_CPU_MODEL_EXPANSION_STATIC_FULL and not just QEMU_MONITOR_CPU_MODEL_EXPANSION_FULL.
> > > > Nice!
> > > > > >
> > > > >
> > > > > So, I need to add Intel PT feature in "X86CPUDefinition
> > > > > builtin_x86_defs[]" so that we can get this CPUID in specific CPU
> > > > > model not only "-cpu host". Is that right?
> > > >
> > > > The problem is that you won't be able to add intel-pt to any CPU
> > > > model unless the feature is made migration-safe (by not calling
> > > > kvm_arch_get_supported_cpuid() from cpu_x86_cpuid()).
> > >
> > > Hi Eduardo,
> > > Have some question need you help clear. What is
> > > "migration-safe" feature mean? I find all the feature which
> > > included in CPU model (builtin_x86_defs[]) will make
> > > "xcc->migration_safe = true;" in
> > > x86_cpu_cpudef_class_init(). If create a Skylake guest on
> > > Skylake HW and live migrate this guest to another machine
> > > with old HW(e.g Haswell). Can we think the new feature or
> > > cpu model(Skylake guest) which only supported in Skylake HW
> > > is safe?
> >
> > I mean matching the requirements so we can say the feature is migration-safe, that means exposing the same CPUID data to the
> > guest on any host, if the machine-type + command-line is the same. The data on CPUID[7] is OK (it changes according to the
> > command-line options only), but the data exposed on CPUID[14h] on this patch is not migration-safe (it changes depending on the
> > host it is running).
> >
>
> Many thanks for your clarification. I think I have understood.
> But CPUID[14h] is depend on CPUID[7].EBX[bit25] and it would be
> zero if CPUID[7].EBX[bit25] is not set. Or what you concerned
> is make live migration on two different HW which all support
> Intel PT virtualization but have different CPUID[14h] result?
> This may need to think about.
Yes. If intel-pt is off, the results (CPUID[14h] all zeroes) are
migration-safe. Setting intel-pt=on makes the command-line not
migration-safe.
This is OK, in principle (e.g. the "pmu" option is not
migration-safe and behaves the same way). The only problem is
that this patch would make query-cpu-model-expansion return
intel-pt=on on type=static expansion.
Probably the easiest way to fix that is to specifically exclude
intel-pt on x86_cpu_static_props().
However, if there's a simple way to make it possible to migrate
between hosts with different CPUID[14h] data, it would be even
better. With the current KVM intel-pt implementation, what
happens if the CPUID[14h] data seen by the guest doesn't match
exactly the CPUID[14h] leaves from the host?
>
> Thanks,
> Luwei Kang
>
> >
> > >
> > > >
> > > > What's missing here is to either: (a) make cpu_x86_cpuid() return
> > > > host-independent data (it can be constant, or it can be configurable on the command-line); or (b) add a mechanism to skip intel-
> > pt from "query-cpu-model-expansion type=static".
> > >
> > > ==> it can be constant:
> > > I think it shouldn't be constant because Intel PT virtualization can only supported in Ice Lake hardware now. Intel PT cpuid info
> > would be mask off on old platform.
> > > ==> or it can be configurable on the command-line:
> > > Because of I didn't add this feature in any CPU model. We only can enable this feature by "-cpu Skylake-Server,+intel-pt" at
> > present.
> >
> > Note that I'm talking about CPUID[14h], not CPUID[7]. The CPUID[7] bits are safe because they are set according to the CPU model
> > + command-line options only. The bits on CPUID[14h] change depending on the host and are not migration-safe. CPUID[7].EBX[bit
> > 25] is just the (already configurable) bit that enables the migration-unsafe code for CPUID[14h].
> >
> > >
> > > What about add a new cpu model name "Icelake" and add PT in this. Is that would be migration safe?
> >
> > It won't, because of the CPUID[14h] code that makes it unsafe to migrate between hosts with different Intel-PT capabilities (i.e.
> > different data on CPUID[14h]).
> >
> >
> > >
> > > Thanks,
> > > Luwei Kang
> > >
> > > > Probably
> > > > (a) is easier to implement, and it also makes the feature more useful (by making it migration-safe).
> > > >
> > > > >
> > > > > Intel PT is first supported in Intel Core M and 5th generation
> > > > > Intel Core processors that are based on the Intel
> > > > > micro-architecture code name Broadwell but Intel PT use EPT is first supported in Ice Lake.
> > > > > Intel PT virtualization depend on PT use EPT. I will add Intel PT
> > > > > to "Broadwell" CPU model and later to make sure a "Broadwell"
> > > > > guest can use Intel PT if the host is Ice Lake.
> > > >
> > > > The "if the host is Ice Lake" part is problematic. On
> > > > migration-safe CPU models (all of them except "max" and "host"), the data seen on CPUID can't depend on the host at all. It
> > should depend only on the machine-type + command-line options.
> > > >
> > > > --
> > > > Eduardo
> >
> > --
> > Eduardo
>
--
Eduardo
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH RESEND v1 1/2] i386: Add Intel Processor Trace feature support
2018-01-18 13:24 ` [Qemu-devel] " Eduardo Habkost
@ 2018-01-18 13:39 ` Paolo Bonzini
-1 siblings, 0 replies; 44+ messages in thread
From: Paolo Bonzini @ 2018-01-18 13:39 UTC (permalink / raw)
To: Eduardo Habkost, Kang, Luwei
Cc: qemu-devel, kvm, rth, mtosatti, Chao Peng, libvir-list
On 18/01/2018 14:24, Eduardo Habkost wrote:
> However, if there's a simple way to make it possible to migrate
> between hosts with different CPUID[14h] data, it would be even
> better. With the current KVM intel-pt implementation, what
> happens if the CPUID[14h] data seen by the guest doesn't match
> exactly the CPUID[14h] leaves from the host?
Some bits in there can be treated as CPU features (e.g. EBX bit 0 "CR3
filtering support"). Probably we should handle these in KVM right now.
KVM needs to compute a mask of valid 1 bits for IA32_RTIT_CTL based on
CPUID, and apply it when the MSR is written. It also needs to whitelist
bits like we do for other feature words. These include:
- CPUID[EAX=14h,ECX=0].EBX
- CPUID[EAX=14h,ECX=0].ECX except bit 31
- CPUID[EAX=14h,ECX=1].EAX bits 16:31 (if CPUID[EAX=14h,ECX=0].EBX[3]=1)
- CPUID[EAX=14h,ECX=1].EBX (if CPUID[EAX=14h,ECX=0].EBX[1]=1)
Others, currently only CPUID[EAX=14h,ECX=0].ECX[31] must match, there is
no way to emulate the "wrong" value.
Others, currently only CPUID[EAX=14h,ECX=1].EAX[2:0] are numeric values,
and it's possible to emulate a lower value than the one in the processor.
CPUID[EAX=14h,ECX=0].EAX is the maximum subleaf. It should be (barring
guest bugs) okay to always present leaf 1.
Paolo
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [Qemu-devel] [PATCH RESEND v1 1/2] i386: Add Intel Processor Trace feature support
@ 2018-01-18 13:39 ` Paolo Bonzini
0 siblings, 0 replies; 44+ messages in thread
From: Paolo Bonzini @ 2018-01-18 13:39 UTC (permalink / raw)
To: Eduardo Habkost, Kang, Luwei
Cc: qemu-devel, kvm, rth, mtosatti, Chao Peng, libvir-list
On 18/01/2018 14:24, Eduardo Habkost wrote:
> However, if there's a simple way to make it possible to migrate
> between hosts with different CPUID[14h] data, it would be even
> better. With the current KVM intel-pt implementation, what
> happens if the CPUID[14h] data seen by the guest doesn't match
> exactly the CPUID[14h] leaves from the host?
Some bits in there can be treated as CPU features (e.g. EBX bit 0 "CR3
filtering support"). Probably we should handle these in KVM right now.
KVM needs to compute a mask of valid 1 bits for IA32_RTIT_CTL based on
CPUID, and apply it when the MSR is written. It also needs to whitelist
bits like we do for other feature words. These include:
- CPUID[EAX=14h,ECX=0].EBX
- CPUID[EAX=14h,ECX=0].ECX except bit 31
- CPUID[EAX=14h,ECX=1].EAX bits 16:31 (if CPUID[EAX=14h,ECX=0].EBX[3]=1)
- CPUID[EAX=14h,ECX=1].EBX (if CPUID[EAX=14h,ECX=0].EBX[1]=1)
Others, currently only CPUID[EAX=14h,ECX=0].ECX[31] must match, there is
no way to emulate the "wrong" value.
Others, currently only CPUID[EAX=14h,ECX=1].EAX[2:0] are numeric values,
and it's possible to emulate a lower value than the one in the processor.
CPUID[EAX=14h,ECX=0].EAX is the maximum subleaf. It should be (barring
guest bugs) okay to always present leaf 1.
Paolo
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH RESEND v1 1/2] i386: Add Intel Processor Trace feature support
2018-01-18 13:39 ` [Qemu-devel] " Paolo Bonzini
@ 2018-01-18 14:37 ` Eduardo Habkost
-1 siblings, 0 replies; 44+ messages in thread
From: Eduardo Habkost @ 2018-01-18 14:37 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Kang, Luwei, qemu-devel, kvm, rth, mtosatti, Chao Peng, libvir-list
On Thu, Jan 18, 2018 at 02:39:57PM +0100, Paolo Bonzini wrote:
> On 18/01/2018 14:24, Eduardo Habkost wrote:
> > However, if there's a simple way to make it possible to migrate
> > between hosts with different CPUID[14h] data, it would be even
> > better. With the current KVM intel-pt implementation, what
> > happens if the CPUID[14h] data seen by the guest doesn't match
> > exactly the CPUID[14h] leaves from the host?
>
> Some bits in there can be treated as CPU features (e.g. EBX bit 0 "CR3
> filtering support"). Probably we should handle these in KVM right now.
> KVM needs to compute a mask of valid 1 bits for IA32_RTIT_CTL based on
> CPUID, and apply it when the MSR is written.
Does this mean QEMU can't set CPUID values that won't match the
host with the existing implementation, or this won't matter for
well-behaved guests that don't try to set reserved bits on the
MSRs?
> It also needs to whitelist
> bits like we do for other feature words. These include:
>
> - CPUID[EAX=14h,ECX=0].EBX
>
> - CPUID[EAX=14h,ECX=0].ECX except bit 31
>
> - CPUID[EAX=14h,ECX=1].EAX bits 16:31 (if CPUID[EAX=14h,ECX=0].EBX[3]=1)
>
> - CPUID[EAX=14h,ECX=1].EBX (if CPUID[EAX=14h,ECX=0].EBX[1]=1)
What do you mean by whitelist?
>
> Others, currently only CPUID[EAX=14h,ECX=0].ECX[31] must match, there is
> no way to emulate the "wrong" value.
In this case we could make it configurable but require the host
and guest value to always match.
This might be an obstacle to enabling intel-pt by default
(because it could make VMs not migratable to newer hosts), but
may allow the feature to be configured in a predictable
way.
>
> Others, currently only CPUID[EAX=14h,ECX=1].EAX[2:0] are numeric values,
> and it's possible to emulate a lower value than the one in the processor.
This could be handled by QEMU. There's no requirement that all
GET_SUPPORTED_CPUID values should be validated by simple bit
masking.
>
> CPUID[EAX=14h,ECX=0].EAX is the maximum subleaf. It should be (barring
> guest bugs) okay to always present leaf 1.
>
> Paolo
--
Eduardo
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [Qemu-devel] [PATCH RESEND v1 1/2] i386: Add Intel Processor Trace feature support
@ 2018-01-18 14:37 ` Eduardo Habkost
0 siblings, 0 replies; 44+ messages in thread
From: Eduardo Habkost @ 2018-01-18 14:37 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Kang, Luwei, qemu-devel, kvm, rth, mtosatti, Chao Peng, libvir-list
On Thu, Jan 18, 2018 at 02:39:57PM +0100, Paolo Bonzini wrote:
> On 18/01/2018 14:24, Eduardo Habkost wrote:
> > However, if there's a simple way to make it possible to migrate
> > between hosts with different CPUID[14h] data, it would be even
> > better. With the current KVM intel-pt implementation, what
> > happens if the CPUID[14h] data seen by the guest doesn't match
> > exactly the CPUID[14h] leaves from the host?
>
> Some bits in there can be treated as CPU features (e.g. EBX bit 0 "CR3
> filtering support"). Probably we should handle these in KVM right now.
> KVM needs to compute a mask of valid 1 bits for IA32_RTIT_CTL based on
> CPUID, and apply it when the MSR is written.
Does this mean QEMU can't set CPUID values that won't match the
host with the existing implementation, or this won't matter for
well-behaved guests that don't try to set reserved bits on the
MSRs?
> It also needs to whitelist
> bits like we do for other feature words. These include:
>
> - CPUID[EAX=14h,ECX=0].EBX
>
> - CPUID[EAX=14h,ECX=0].ECX except bit 31
>
> - CPUID[EAX=14h,ECX=1].EAX bits 16:31 (if CPUID[EAX=14h,ECX=0].EBX[3]=1)
>
> - CPUID[EAX=14h,ECX=1].EBX (if CPUID[EAX=14h,ECX=0].EBX[1]=1)
What do you mean by whitelist?
>
> Others, currently only CPUID[EAX=14h,ECX=0].ECX[31] must match, there is
> no way to emulate the "wrong" value.
In this case we could make it configurable but require the host
and guest value to always match.
This might be an obstacle to enabling intel-pt by default
(because it could make VMs not migratable to newer hosts), but
may allow the feature to be configured in a predictable
way.
>
> Others, currently only CPUID[EAX=14h,ECX=1].EAX[2:0] are numeric values,
> and it's possible to emulate a lower value than the one in the processor.
This could be handled by QEMU. There's no requirement that all
GET_SUPPORTED_CPUID values should be validated by simple bit
masking.
>
> CPUID[EAX=14h,ECX=0].EAX is the maximum subleaf. It should be (barring
> guest bugs) okay to always present leaf 1.
>
> Paolo
--
Eduardo
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH RESEND v1 1/2] i386: Add Intel Processor Trace feature support
2018-01-18 14:37 ` [Qemu-devel] " Eduardo Habkost
@ 2018-01-18 14:44 ` Paolo Bonzini
-1 siblings, 0 replies; 44+ messages in thread
From: Paolo Bonzini @ 2018-01-18 14:44 UTC (permalink / raw)
To: Eduardo Habkost
Cc: Kang, Luwei, qemu-devel, kvm, rth, mtosatti, Chao Peng, libvir-list
On 18/01/2018 15:37, Eduardo Habkost wrote:
> On Thu, Jan 18, 2018 at 02:39:57PM +0100, Paolo Bonzini wrote:
>> On 18/01/2018 14:24, Eduardo Habkost wrote:
>>> However, if there's a simple way to make it possible to migrate
>>> between hosts with different CPUID[14h] data, it would be even
>>> better. With the current KVM intel-pt implementation, what
>>> happens if the CPUID[14h] data seen by the guest doesn't match
>>> exactly the CPUID[14h] leaves from the host?
>>
>> Some bits in there can be treated as CPU features (e.g. EBX bit 0 "CR3
>> filtering support"). Probably we should handle these in KVM right now.
>> KVM needs to compute a mask of valid 1 bits for IA32_RTIT_CTL based on
>> CPUID, and apply it when the MSR is written.
>
> Does this mean QEMU can't set CPUID values that won't match the
> host with the existing implementation, or this won't matter for
> well-behaved guests that don't try to set reserved bits on the
> MSRs?
All the features could be handled exactly like regular feature bits. If
QEMU sets them incorrectly and "enforce" is not used, bad things happen
but it's the user's fault.
>
>> It also needs to whitelist
>> bits like we do for other feature words. These include:
>>
>> - CPUID[EAX=14h,ECX=0].EBX
>>
>> - CPUID[EAX=14h,ECX=0].ECX except bit 31
>>
>> - CPUID[EAX=14h,ECX=1].EAX bits 16:31 (if CPUID[EAX=14h,ECX=0].EBX[3]=1)
>>
>> - CPUID[EAX=14h,ECX=1].EBX (if CPUID[EAX=14h,ECX=0].EBX[1]=1)
>
> What do you mean by whitelist?
KVM needs to tell QEMU the bits it knows about.
>> Others, currently only CPUID[EAX=14h,ECX=0].ECX[31] must match, there is
>> no way to emulate the "wrong" value.
>
> In this case we could make it configurable but require the host
> and guest value to always match.
>
> This might be an obstacle to enabling intel-pt by default
> (because it could make VMs not migratable to newer hosts), but
> may allow the feature to be configured in a predictable
> way.
Yeah, but consider that virtualized PT anyway would only be enabled on
Ice Lake processors. It's a few years away anyway!
>> Others, currently only CPUID[EAX=14h,ECX=1].EAX[2:0] are numeric values,
>> and it's possible to emulate a lower value than the one in the processor.
>
> This could be handled by QEMU. There's no requirement that all
> GET_SUPPORTED_CPUID values should be validated by simple bit
> masking.
Good!
Paolo
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [Qemu-devel] [PATCH RESEND v1 1/2] i386: Add Intel Processor Trace feature support
@ 2018-01-18 14:44 ` Paolo Bonzini
0 siblings, 0 replies; 44+ messages in thread
From: Paolo Bonzini @ 2018-01-18 14:44 UTC (permalink / raw)
To: Eduardo Habkost
Cc: Kang, Luwei, qemu-devel, kvm, rth, mtosatti, Chao Peng, libvir-list
On 18/01/2018 15:37, Eduardo Habkost wrote:
> On Thu, Jan 18, 2018 at 02:39:57PM +0100, Paolo Bonzini wrote:
>> On 18/01/2018 14:24, Eduardo Habkost wrote:
>>> However, if there's a simple way to make it possible to migrate
>>> between hosts with different CPUID[14h] data, it would be even
>>> better. With the current KVM intel-pt implementation, what
>>> happens if the CPUID[14h] data seen by the guest doesn't match
>>> exactly the CPUID[14h] leaves from the host?
>>
>> Some bits in there can be treated as CPU features (e.g. EBX bit 0 "CR3
>> filtering support"). Probably we should handle these in KVM right now.
>> KVM needs to compute a mask of valid 1 bits for IA32_RTIT_CTL based on
>> CPUID, and apply it when the MSR is written.
>
> Does this mean QEMU can't set CPUID values that won't match the
> host with the existing implementation, or this won't matter for
> well-behaved guests that don't try to set reserved bits on the
> MSRs?
All the features could be handled exactly like regular feature bits. If
QEMU sets them incorrectly and "enforce" is not used, bad things happen
but it's the user's fault.
>
>> It also needs to whitelist
>> bits like we do for other feature words. These include:
>>
>> - CPUID[EAX=14h,ECX=0].EBX
>>
>> - CPUID[EAX=14h,ECX=0].ECX except bit 31
>>
>> - CPUID[EAX=14h,ECX=1].EAX bits 16:31 (if CPUID[EAX=14h,ECX=0].EBX[3]=1)
>>
>> - CPUID[EAX=14h,ECX=1].EBX (if CPUID[EAX=14h,ECX=0].EBX[1]=1)
>
> What do you mean by whitelist?
KVM needs to tell QEMU the bits it knows about.
>> Others, currently only CPUID[EAX=14h,ECX=0].ECX[31] must match, there is
>> no way to emulate the "wrong" value.
>
> In this case we could make it configurable but require the host
> and guest value to always match.
>
> This might be an obstacle to enabling intel-pt by default
> (because it could make VMs not migratable to newer hosts), but
> may allow the feature to be configured in a predictable
> way.
Yeah, but consider that virtualized PT anyway would only be enabled on
Ice Lake processors. It's a few years away anyway!
>> Others, currently only CPUID[EAX=14h,ECX=1].EAX[2:0] are numeric values,
>> and it's possible to emulate a lower value than the one in the processor.
>
> This could be handled by QEMU. There's no requirement that all
> GET_SUPPORTED_CPUID values should be validated by simple bit
> masking.
Good!
Paolo
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH RESEND v1 1/2] i386: Add Intel Processor Trace feature support
2018-01-18 14:44 ` [Qemu-devel] " Paolo Bonzini
@ 2018-01-18 16:52 ` Eduardo Habkost
-1 siblings, 0 replies; 44+ messages in thread
From: Eduardo Habkost @ 2018-01-18 16:52 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Kang, Luwei, qemu-devel, kvm, rth, mtosatti, Chao Peng, libvir-list
On Thu, Jan 18, 2018 at 03:44:49PM +0100, Paolo Bonzini wrote:
> On 18/01/2018 15:37, Eduardo Habkost wrote:
> > On Thu, Jan 18, 2018 at 02:39:57PM +0100, Paolo Bonzini wrote:
> >> On 18/01/2018 14:24, Eduardo Habkost wrote:
> >>> However, if there's a simple way to make it possible to migrate
> >>> between hosts with different CPUID[14h] data, it would be even
> >>> better. With the current KVM intel-pt implementation, what
> >>> happens if the CPUID[14h] data seen by the guest doesn't match
> >>> exactly the CPUID[14h] leaves from the host?
> >>
> >> Some bits in there can be treated as CPU features (e.g. EBX bit 0 "CR3
> >> filtering support"). Probably we should handle these in KVM right now.
> >> KVM needs to compute a mask of valid 1 bits for IA32_RTIT_CTL based on
> >> CPUID, and apply it when the MSR is written.
> >
> > Does this mean QEMU can't set CPUID values that won't match the
> > host with the existing implementation, or this won't matter for
> > well-behaved guests that don't try to set reserved bits on the
> > MSRs?
>
> All the features could be handled exactly like regular feature bits. If
> QEMU sets them incorrectly and "enforce" is not used, bad things happen
> but it's the user's fault.
Oh, I mean setting the bit to 0 when it's 1 on the host (if it's
0 on the host, QEMU would never set it anyway). Is it safe to do
it with the current KVM intel-pt implementation?
>
> >
> >> It also needs to whitelist
> >> bits like we do for other feature words. These include:
> >>
> >> - CPUID[EAX=14h,ECX=0].EBX
> >>
> >> - CPUID[EAX=14h,ECX=0].ECX except bit 31
> >>
> >> - CPUID[EAX=14h,ECX=1].EAX bits 16:31 (if CPUID[EAX=14h,ECX=0].EBX[3]=1)
> >>
> >> - CPUID[EAX=14h,ECX=1].EBX (if CPUID[EAX=14h,ECX=0].EBX[1]=1)
> >
> > What do you mean by whitelist?
>
> KVM needs to tell QEMU the bits it knows about.
So KVM isn't currently doing it on GET_SUPPORTED_CPUID? Oops.
>
> >> Others, currently only CPUID[EAX=14h,ECX=0].ECX[31] must match, there is
> >> no way to emulate the "wrong" value.
> >
> > In this case we could make it configurable but require the host
> > and guest value to always match.
> >
> > This might be an obstacle to enabling intel-pt by default
> > (because it could make VMs not migratable to newer hosts), but
> > may allow the feature to be configured in a predictable
> > way.
>
> Yeah, but consider that virtualized PT anyway would only be enabled on
> Ice Lake processors. It's a few years away anyway!
>
> >> Others, currently only CPUID[EAX=14h,ECX=1].EAX[2:0] are numeric values,
> >> and it's possible to emulate a lower value than the one in the processor.
> >
> > This could be handled by QEMU. There's no requirement that all
> > GET_SUPPORTED_CPUID values should be validated by simple bit
> > masking.
>
> Good!
>
> Paolo
--
Eduardo
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [Qemu-devel] [PATCH RESEND v1 1/2] i386: Add Intel Processor Trace feature support
@ 2018-01-18 16:52 ` Eduardo Habkost
0 siblings, 0 replies; 44+ messages in thread
From: Eduardo Habkost @ 2018-01-18 16:52 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Kang, Luwei, qemu-devel, kvm, rth, mtosatti, Chao Peng, libvir-list
On Thu, Jan 18, 2018 at 03:44:49PM +0100, Paolo Bonzini wrote:
> On 18/01/2018 15:37, Eduardo Habkost wrote:
> > On Thu, Jan 18, 2018 at 02:39:57PM +0100, Paolo Bonzini wrote:
> >> On 18/01/2018 14:24, Eduardo Habkost wrote:
> >>> However, if there's a simple way to make it possible to migrate
> >>> between hosts with different CPUID[14h] data, it would be even
> >>> better. With the current KVM intel-pt implementation, what
> >>> happens if the CPUID[14h] data seen by the guest doesn't match
> >>> exactly the CPUID[14h] leaves from the host?
> >>
> >> Some bits in there can be treated as CPU features (e.g. EBX bit 0 "CR3
> >> filtering support"). Probably we should handle these in KVM right now.
> >> KVM needs to compute a mask of valid 1 bits for IA32_RTIT_CTL based on
> >> CPUID, and apply it when the MSR is written.
> >
> > Does this mean QEMU can't set CPUID values that won't match the
> > host with the existing implementation, or this won't matter for
> > well-behaved guests that don't try to set reserved bits on the
> > MSRs?
>
> All the features could be handled exactly like regular feature bits. If
> QEMU sets them incorrectly and "enforce" is not used, bad things happen
> but it's the user's fault.
Oh, I mean setting the bit to 0 when it's 1 on the host (if it's
0 on the host, QEMU would never set it anyway). Is it safe to do
it with the current KVM intel-pt implementation?
>
> >
> >> It also needs to whitelist
> >> bits like we do for other feature words. These include:
> >>
> >> - CPUID[EAX=14h,ECX=0].EBX
> >>
> >> - CPUID[EAX=14h,ECX=0].ECX except bit 31
> >>
> >> - CPUID[EAX=14h,ECX=1].EAX bits 16:31 (if CPUID[EAX=14h,ECX=0].EBX[3]=1)
> >>
> >> - CPUID[EAX=14h,ECX=1].EBX (if CPUID[EAX=14h,ECX=0].EBX[1]=1)
> >
> > What do you mean by whitelist?
>
> KVM needs to tell QEMU the bits it knows about.
So KVM isn't currently doing it on GET_SUPPORTED_CPUID? Oops.
>
> >> Others, currently only CPUID[EAX=14h,ECX=0].ECX[31] must match, there is
> >> no way to emulate the "wrong" value.
> >
> > In this case we could make it configurable but require the host
> > and guest value to always match.
> >
> > This might be an obstacle to enabling intel-pt by default
> > (because it could make VMs not migratable to newer hosts), but
> > may allow the feature to be configured in a predictable
> > way.
>
> Yeah, but consider that virtualized PT anyway would only be enabled on
> Ice Lake processors. It's a few years away anyway!
>
> >> Others, currently only CPUID[EAX=14h,ECX=1].EAX[2:0] are numeric values,
> >> and it's possible to emulate a lower value than the one in the processor.
> >
> > This could be handled by QEMU. There's no requirement that all
> > GET_SUPPORTED_CPUID values should be validated by simple bit
> > masking.
>
> Good!
>
> Paolo
--
Eduardo
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH RESEND v1 1/2] i386: Add Intel Processor Trace feature support
2018-01-18 16:52 ` [Qemu-devel] " Eduardo Habkost
@ 2018-01-18 16:53 ` Paolo Bonzini
-1 siblings, 0 replies; 44+ messages in thread
From: Paolo Bonzini @ 2018-01-18 16:53 UTC (permalink / raw)
To: Eduardo Habkost
Cc: Kang, Luwei, qemu-devel, kvm, rth, mtosatti, Chao Peng, libvir-list
On 18/01/2018 17:52, Eduardo Habkost wrote:
> On Thu, Jan 18, 2018 at 03:44:49PM +0100, Paolo Bonzini wrote:
>> On 18/01/2018 15:37, Eduardo Habkost wrote:
>>> On Thu, Jan 18, 2018 at 02:39:57PM +0100, Paolo Bonzini wrote:
>>>> On 18/01/2018 14:24, Eduardo Habkost wrote:
>>>>> However, if there's a simple way to make it possible to migrate
>>>>> between hosts with different CPUID[14h] data, it would be even
>>>>> better. With the current KVM intel-pt implementation, what
>>>>> happens if the CPUID[14h] data seen by the guest doesn't match
>>>>> exactly the CPUID[14h] leaves from the host?
>>>>
>>>> Some bits in there can be treated as CPU features (e.g. EBX bit 0 "CR3
>>>> filtering support"). Probably we should handle these in KVM right now.
>>>> KVM needs to compute a mask of valid 1 bits for IA32_RTIT_CTL based on
>>>> CPUID, and apply it when the MSR is written.
>>>
>>> Does this mean QEMU can't set CPUID values that won't match the
>>> host with the existing implementation, or this won't matter for
>>> well-behaved guests that don't try to set reserved bits on the
>>> MSRs?
>>
>> All the features could be handled exactly like regular feature bits. If
>> QEMU sets them incorrectly and "enforce" is not used, bad things happen
>> but it's the user's fault.
>
> Oh, I mean setting the bit to 0 when it's 1 on the host (if it's
> 0 on the host, QEMU would never set it anyway). Is it safe to do
> it with the current KVM intel-pt implementation?
It's not, but it's (very) easy to fix.
Paolo
>
>>
>>>
>>>> It also needs to whitelist
>>>> bits like we do for other feature words. These include:
>>>>
>>>> - CPUID[EAX=14h,ECX=0].EBX
>>>>
>>>> - CPUID[EAX=14h,ECX=0].ECX except bit 31
>>>>
>>>> - CPUID[EAX=14h,ECX=1].EAX bits 16:31 (if CPUID[EAX=14h,ECX=0].EBX[3]=1)
>>>>
>>>> - CPUID[EAX=14h,ECX=1].EBX (if CPUID[EAX=14h,ECX=0].EBX[1]=1)
>>>
>>> What do you mean by whitelist?
>>
>> KVM needs to tell QEMU the bits it knows about.
>
> So KVM isn't currently doing it on GET_SUPPORTED_CPUID? Oops.
>
>
>>
>>>> Others, currently only CPUID[EAX=14h,ECX=0].ECX[31] must match, there is
>>>> no way to emulate the "wrong" value.
>>>
>>> In this case we could make it configurable but require the host
>>> and guest value to always match.
>>>
>>> This might be an obstacle to enabling intel-pt by default
>>> (because it could make VMs not migratable to newer hosts), but
>>> may allow the feature to be configured in a predictable
>>> way.
>>
>> Yeah, but consider that virtualized PT anyway would only be enabled on
>> Ice Lake processors. It's a few years away anyway!
>>
>>>> Others, currently only CPUID[EAX=14h,ECX=1].EAX[2:0] are numeric values,
>>>> and it's possible to emulate a lower value than the one in the processor.
>>>
>>> This could be handled by QEMU. There's no requirement that all
>>> GET_SUPPORTED_CPUID values should be validated by simple bit
>>> masking.
>>
>> Good!
>>
>> Paolo
>
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [Qemu-devel] [PATCH RESEND v1 1/2] i386: Add Intel Processor Trace feature support
@ 2018-01-18 16:53 ` Paolo Bonzini
0 siblings, 0 replies; 44+ messages in thread
From: Paolo Bonzini @ 2018-01-18 16:53 UTC (permalink / raw)
To: Eduardo Habkost
Cc: Kang, Luwei, qemu-devel, kvm, rth, mtosatti, Chao Peng, libvir-list
On 18/01/2018 17:52, Eduardo Habkost wrote:
> On Thu, Jan 18, 2018 at 03:44:49PM +0100, Paolo Bonzini wrote:
>> On 18/01/2018 15:37, Eduardo Habkost wrote:
>>> On Thu, Jan 18, 2018 at 02:39:57PM +0100, Paolo Bonzini wrote:
>>>> On 18/01/2018 14:24, Eduardo Habkost wrote:
>>>>> However, if there's a simple way to make it possible to migrate
>>>>> between hosts with different CPUID[14h] data, it would be even
>>>>> better. With the current KVM intel-pt implementation, what
>>>>> happens if the CPUID[14h] data seen by the guest doesn't match
>>>>> exactly the CPUID[14h] leaves from the host?
>>>>
>>>> Some bits in there can be treated as CPU features (e.g. EBX bit 0 "CR3
>>>> filtering support"). Probably we should handle these in KVM right now.
>>>> KVM needs to compute a mask of valid 1 bits for IA32_RTIT_CTL based on
>>>> CPUID, and apply it when the MSR is written.
>>>
>>> Does this mean QEMU can't set CPUID values that won't match the
>>> host with the existing implementation, or this won't matter for
>>> well-behaved guests that don't try to set reserved bits on the
>>> MSRs?
>>
>> All the features could be handled exactly like regular feature bits. If
>> QEMU sets them incorrectly and "enforce" is not used, bad things happen
>> but it's the user's fault.
>
> Oh, I mean setting the bit to 0 when it's 1 on the host (if it's
> 0 on the host, QEMU would never set it anyway). Is it safe to do
> it with the current KVM intel-pt implementation?
It's not, but it's (very) easy to fix.
Paolo
>
>>
>>>
>>>> It also needs to whitelist
>>>> bits like we do for other feature words. These include:
>>>>
>>>> - CPUID[EAX=14h,ECX=0].EBX
>>>>
>>>> - CPUID[EAX=14h,ECX=0].ECX except bit 31
>>>>
>>>> - CPUID[EAX=14h,ECX=1].EAX bits 16:31 (if CPUID[EAX=14h,ECX=0].EBX[3]=1)
>>>>
>>>> - CPUID[EAX=14h,ECX=1].EBX (if CPUID[EAX=14h,ECX=0].EBX[1]=1)
>>>
>>> What do you mean by whitelist?
>>
>> KVM needs to tell QEMU the bits it knows about.
>
> So KVM isn't currently doing it on GET_SUPPORTED_CPUID? Oops.
>
>
>>
>>>> Others, currently only CPUID[EAX=14h,ECX=0].ECX[31] must match, there is
>>>> no way to emulate the "wrong" value.
>>>
>>> In this case we could make it configurable but require the host
>>> and guest value to always match.
>>>
>>> This might be an obstacle to enabling intel-pt by default
>>> (because it could make VMs not migratable to newer hosts), but
>>> may allow the feature to be configured in a predictable
>>> way.
>>
>> Yeah, but consider that virtualized PT anyway would only be enabled on
>> Ice Lake processors. It's a few years away anyway!
>>
>>>> Others, currently only CPUID[EAX=14h,ECX=1].EAX[2:0] are numeric values,
>>>> and it's possible to emulate a lower value than the one in the processor.
>>>
>>> This could be handled by QEMU. There's no requirement that all
>>> GET_SUPPORTED_CPUID values should be validated by simple bit
>>> masking.
>>
>> Good!
>>
>> Paolo
>
^ permalink raw reply [flat|nested] 44+ messages in thread
* RE: [PATCH RESEND v1 1/2] i386: Add Intel Processor Trace feature support
2018-01-18 16:53 ` [Qemu-devel] " Paolo Bonzini
@ 2018-01-22 10:36 ` Kang, Luwei
-1 siblings, 0 replies; 44+ messages in thread
From: Kang, Luwei @ 2018-01-22 10:36 UTC (permalink / raw)
To: Paolo Bonzini, Eduardo Habkost
Cc: qemu-devel, kvm, rth, mtosatti, Chao Peng, libvir-list
> > On Thu, Jan 18, 2018 at 03:44:49PM +0100, Paolo Bonzini wrote:
> >> On 18/01/2018 15:37, Eduardo Habkost wrote:
> >>> On Thu, Jan 18, 2018 at 02:39:57PM +0100, Paolo Bonzini wrote:
> >>>> On 18/01/2018 14:24, Eduardo Habkost wrote:
> >>>>> However, if there's a simple way to make it possible to migrate
> >>>>> between hosts with different CPUID[14h] data, it would be even
> >>>>> better. With the current KVM intel-pt implementation, what
> >>>>> happens if the CPUID[14h] data seen by the guest doesn't match
> >>>>> exactly the CPUID[14h] leaves from the host?
> >>>>
> >>>> Some bits in there can be treated as CPU features (e.g. EBX bit 0
> >>>> "CR3 filtering support"). Probably we should handle these in KVM right now.
> >>>> KVM needs to compute a mask of valid 1 bits for IA32_RTIT_CTL based
> >>>> on CPUID, and apply it when the MSR is written.
> >>>
> >>> Does this mean QEMU can't set CPUID values that won't match the host
> >>> with the existing implementation, or this won't matter for
> >>> well-behaved guests that don't try to set reserved bits on the MSRs?
> >>
> >> All the features could be handled exactly like regular feature bits.
> >> If QEMU sets them incorrectly and "enforce" is not used, bad things
> >> happen but it's the user's fault.
> >
> > Oh, I mean setting the bit to 0 when it's 1 on the host (if it's
> > 0 on the host, QEMU would never set it anyway). Is it safe to do it
> > with the current KVM intel-pt implementation?
>
> It's not, but it's (very) easy to fix.
Hi Paolo,
Do you mean there need to add some check before setting IA32_RTIT_CTL MSR in KVM because some bits of this MSR is depend on the result of CPUID[14]. Any attempts to change these reserved bit should cause a #GP.
> >
> >>
> >>>
> >>>> It also needs to
> >>>> whitelist bits like we do for other feature words. These include:
> >>>>
> >>>> - CPUID[EAX=14h,ECX=0].EBX
> >>>>
> >>>> - CPUID[EAX=14h,ECX=0].ECX except bit 31
> >>>>
> >>>> - CPUID[EAX=14h,ECX=1].EAX bits 16:31 (if
> >>>> CPUID[EAX=14h,ECX=0].EBX[3]=1)
> >>>>
> >>>> - CPUID[EAX=14h,ECX=1].EBX (if CPUID[EAX=14h,ECX=0].EBX[1]=1)
> >>>
> >>> What do you mean by whitelist?
> >>
> >> KVM needs to tell QEMU the bits it knows about.
I think kvm_arch_get_supported_cpuid() function can get the result of CPUID[14] from KVM. Is this the whitelist what you mentioned?
Thanks,
Luwei Kang
> >
> > So KVM isn't currently doing it on GET_SUPPORTED_CPUID? Oops.
> >
> >
> >>
> >>>> Others, currently only CPUID[EAX=14h,ECX=0].ECX[31] must match,
> >>>> there is no way to emulate the "wrong" value.
> >>>
> >>> In this case we could make it configurable but require the host and
> >>> guest value to always match.
> >>>
> >>> This might be an obstacle to enabling intel-pt by default (because
> >>> it could make VMs not migratable to newer hosts), but may allow the
> >>> feature to be configured in a predictable way.
> >>
> >> Yeah, but consider that virtualized PT anyway would only be enabled
> >> on Ice Lake processors. It's a few years away anyway!
> >>
> >>>> Others, currently only CPUID[EAX=14h,ECX=1].EAX[2:0] are numeric
> >>>> values, and it's possible to emulate a lower value than the one in the processor.
> >>>
> >>> This could be handled by QEMU. There's no requirement that all
> >>> GET_SUPPORTED_CPUID values should be validated by simple bit
> >>> masking.
> >>
> >> Good!
> >>
> >> Paolo
> >
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [Qemu-devel] [PATCH RESEND v1 1/2] i386: Add Intel Processor Trace feature support
@ 2018-01-22 10:36 ` Kang, Luwei
0 siblings, 0 replies; 44+ messages in thread
From: Kang, Luwei @ 2018-01-22 10:36 UTC (permalink / raw)
To: Paolo Bonzini, Eduardo Habkost
Cc: qemu-devel, kvm, rth, mtosatti, Chao Peng, libvir-list
> > On Thu, Jan 18, 2018 at 03:44:49PM +0100, Paolo Bonzini wrote:
> >> On 18/01/2018 15:37, Eduardo Habkost wrote:
> >>> On Thu, Jan 18, 2018 at 02:39:57PM +0100, Paolo Bonzini wrote:
> >>>> On 18/01/2018 14:24, Eduardo Habkost wrote:
> >>>>> However, if there's a simple way to make it possible to migrate
> >>>>> between hosts with different CPUID[14h] data, it would be even
> >>>>> better. With the current KVM intel-pt implementation, what
> >>>>> happens if the CPUID[14h] data seen by the guest doesn't match
> >>>>> exactly the CPUID[14h] leaves from the host?
> >>>>
> >>>> Some bits in there can be treated as CPU features (e.g. EBX bit 0
> >>>> "CR3 filtering support"). Probably we should handle these in KVM right now.
> >>>> KVM needs to compute a mask of valid 1 bits for IA32_RTIT_CTL based
> >>>> on CPUID, and apply it when the MSR is written.
> >>>
> >>> Does this mean QEMU can't set CPUID values that won't match the host
> >>> with the existing implementation, or this won't matter for
> >>> well-behaved guests that don't try to set reserved bits on the MSRs?
> >>
> >> All the features could be handled exactly like regular feature bits.
> >> If QEMU sets them incorrectly and "enforce" is not used, bad things
> >> happen but it's the user's fault.
> >
> > Oh, I mean setting the bit to 0 when it's 1 on the host (if it's
> > 0 on the host, QEMU would never set it anyway). Is it safe to do it
> > with the current KVM intel-pt implementation?
>
> It's not, but it's (very) easy to fix.
Hi Paolo,
Do you mean there need to add some check before setting IA32_RTIT_CTL MSR in KVM because some bits of this MSR is depend on the result of CPUID[14]. Any attempts to change these reserved bit should cause a #GP.
> >
> >>
> >>>
> >>>> It also needs to
> >>>> whitelist bits like we do for other feature words. These include:
> >>>>
> >>>> - CPUID[EAX=14h,ECX=0].EBX
> >>>>
> >>>> - CPUID[EAX=14h,ECX=0].ECX except bit 31
> >>>>
> >>>> - CPUID[EAX=14h,ECX=1].EAX bits 16:31 (if
> >>>> CPUID[EAX=14h,ECX=0].EBX[3]=1)
> >>>>
> >>>> - CPUID[EAX=14h,ECX=1].EBX (if CPUID[EAX=14h,ECX=0].EBX[1]=1)
> >>>
> >>> What do you mean by whitelist?
> >>
> >> KVM needs to tell QEMU the bits it knows about.
I think kvm_arch_get_supported_cpuid() function can get the result of CPUID[14] from KVM. Is this the whitelist what you mentioned?
Thanks,
Luwei Kang
> >
> > So KVM isn't currently doing it on GET_SUPPORTED_CPUID? Oops.
> >
> >
> >>
> >>>> Others, currently only CPUID[EAX=14h,ECX=0].ECX[31] must match,
> >>>> there is no way to emulate the "wrong" value.
> >>>
> >>> In this case we could make it configurable but require the host and
> >>> guest value to always match.
> >>>
> >>> This might be an obstacle to enabling intel-pt by default (because
> >>> it could make VMs not migratable to newer hosts), but may allow the
> >>> feature to be configured in a predictable way.
> >>
> >> Yeah, but consider that virtualized PT anyway would only be enabled
> >> on Ice Lake processors. It's a few years away anyway!
> >>
> >>>> Others, currently only CPUID[EAX=14h,ECX=1].EAX[2:0] are numeric
> >>>> values, and it's possible to emulate a lower value than the one in the processor.
> >>>
> >>> This could be handled by QEMU. There's no requirement that all
> >>> GET_SUPPORTED_CPUID values should be validated by simple bit
> >>> masking.
> >>
> >> Good!
> >>
> >> Paolo
> >
^ permalink raw reply [flat|nested] 44+ messages in thread
* RE: [PATCH RESEND v1 1/2] i386: Add Intel Processor Trace feature support
2018-01-18 14:37 ` [Qemu-devel] " Eduardo Habkost
@ 2018-01-22 10:45 ` Kang, Luwei
-1 siblings, 0 replies; 44+ messages in thread
From: Kang, Luwei @ 2018-01-22 10:45 UTC (permalink / raw)
To: Eduardo Habkost, Paolo Bonzini
Cc: qemu-devel, kvm, rth, mtosatti, Chao Peng, libvir-list
> > On 18/01/2018 14:24, Eduardo Habkost wrote:
> > > However, if there's a simple way to make it possible to migrate
> > > between hosts with different CPUID[14h] data, it would be even
> > > better. With the current KVM intel-pt implementation, what happens
> > > if the CPUID[14h] data seen by the guest doesn't match exactly the
> > > CPUID[14h] leaves from the host?
> >
> > Some bits in there can be treated as CPU features (e.g. EBX bit 0 "CR3
> > filtering support"). Probably we should handle these in KVM right now.
> > KVM needs to compute a mask of valid 1 bits for IA32_RTIT_CTL based on
> > CPUID, and apply it when the MSR is written.
>
> Does this mean QEMU can't set CPUID values that won't match the host with the existing implementation, or this won't matter for
> well-behaved guests that don't try to set reserved bits on the MSRs?
>
>
> > It also needs to
> > whitelist bits like we do for other feature words. These include:
> >
> > - CPUID[EAX=14h,ECX=0].EBX
> >
> > - CPUID[EAX=14h,ECX=0].ECX except bit 31
> >
> > - CPUID[EAX=14h,ECX=1].EAX bits 16:31 (if
> > CPUID[EAX=14h,ECX=0].EBX[3]=1)
> >
> > - CPUID[EAX=14h,ECX=1].EBX (if CPUID[EAX=14h,ECX=0].EBX[1]=1)
>
> What do you mean by whitelist?
>
>
> >
> > Others, currently only CPUID[EAX=14h,ECX=0].ECX[31] must match, there
> > is no way to emulate the "wrong" value.
>
> In this case we could make it configurable but require the host and guest value to always match.
>
> This might be an obstacle to enabling intel-pt by default (because it could make VMs not migratable to newer hosts), but may allow
> the feature to be configured in a predictable way.
>
>
> >
> > Others, currently only CPUID[EAX=14h,ECX=1].EAX[2:0] are numeric
> > values, and it's possible to emulate a lower value than the one in the processor.
>
> This could be handled by QEMU. There's no requirement that all GET_SUPPORTED_CPUID values should be validated by simple bit
> masking.
So, we can get a lower value on the numeric of CPUID[EAX=14h,ECX=1].EAX[2:0] between different host. How about other bits of CPUID[14] ? Can we do like this as well?
Thanks,
Luwei Kang
>
>
> >
> > CPUID[EAX=14h,ECX=0].EAX is the maximum subleaf. It should be
> > (barring guest bugs) okay to always present leaf 1.
> >
> > Paolo
>
> --
> Eduardo
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [Qemu-devel] [PATCH RESEND v1 1/2] i386: Add Intel Processor Trace feature support
@ 2018-01-22 10:45 ` Kang, Luwei
0 siblings, 0 replies; 44+ messages in thread
From: Kang, Luwei @ 2018-01-22 10:45 UTC (permalink / raw)
To: Eduardo Habkost, Paolo Bonzini
Cc: qemu-devel, kvm, rth, mtosatti, Chao Peng, libvir-list
> > On 18/01/2018 14:24, Eduardo Habkost wrote:
> > > However, if there's a simple way to make it possible to migrate
> > > between hosts with different CPUID[14h] data, it would be even
> > > better. With the current KVM intel-pt implementation, what happens
> > > if the CPUID[14h] data seen by the guest doesn't match exactly the
> > > CPUID[14h] leaves from the host?
> >
> > Some bits in there can be treated as CPU features (e.g. EBX bit 0 "CR3
> > filtering support"). Probably we should handle these in KVM right now.
> > KVM needs to compute a mask of valid 1 bits for IA32_RTIT_CTL based on
> > CPUID, and apply it when the MSR is written.
>
> Does this mean QEMU can't set CPUID values that won't match the host with the existing implementation, or this won't matter for
> well-behaved guests that don't try to set reserved bits on the MSRs?
>
>
> > It also needs to
> > whitelist bits like we do for other feature words. These include:
> >
> > - CPUID[EAX=14h,ECX=0].EBX
> >
> > - CPUID[EAX=14h,ECX=0].ECX except bit 31
> >
> > - CPUID[EAX=14h,ECX=1].EAX bits 16:31 (if
> > CPUID[EAX=14h,ECX=0].EBX[3]=1)
> >
> > - CPUID[EAX=14h,ECX=1].EBX (if CPUID[EAX=14h,ECX=0].EBX[1]=1)
>
> What do you mean by whitelist?
>
>
> >
> > Others, currently only CPUID[EAX=14h,ECX=0].ECX[31] must match, there
> > is no way to emulate the "wrong" value.
>
> In this case we could make it configurable but require the host and guest value to always match.
>
> This might be an obstacle to enabling intel-pt by default (because it could make VMs not migratable to newer hosts), but may allow
> the feature to be configured in a predictable way.
>
>
> >
> > Others, currently only CPUID[EAX=14h,ECX=1].EAX[2:0] are numeric
> > values, and it's possible to emulate a lower value than the one in the processor.
>
> This could be handled by QEMU. There's no requirement that all GET_SUPPORTED_CPUID values should be validated by simple bit
> masking.
So, we can get a lower value on the numeric of CPUID[EAX=14h,ECX=1].EAX[2:0] between different host. How about other bits of CPUID[14] ? Can we do like this as well?
Thanks,
Luwei Kang
>
>
> >
> > CPUID[EAX=14h,ECX=0].EAX is the maximum subleaf. It should be
> > (barring guest bugs) okay to always present leaf 1.
> >
> > Paolo
>
> --
> Eduardo
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH RESEND v1 1/2] i386: Add Intel Processor Trace feature support
2018-01-22 10:36 ` [Qemu-devel] " Kang, Luwei
@ 2018-01-26 9:19 ` Paolo Bonzini
-1 siblings, 0 replies; 44+ messages in thread
From: Paolo Bonzini @ 2018-01-26 9:19 UTC (permalink / raw)
To: Kang, Luwei, Eduardo Habkost
Cc: qemu-devel, kvm, rth, mtosatti, Chao Peng, libvir-list
On 22/01/2018 11:36, Kang, Luwei wrote:
>>> On Thu, Jan 18, 2018 at 03:44:49PM +0100, Paolo Bonzini wrote:
>>>> On 18/01/2018 15:37, Eduardo Habkost wrote:
>>>>> On Thu, Jan 18, 2018 at 02:39:57PM +0100, Paolo Bonzini wrote:
>>>>>> On 18/01/2018 14:24, Eduardo Habkost wrote:
>>>>>>> However, if there's a simple way to make it possible to migrate
>>>>>>> between hosts with different CPUID[14h] data, it would be even
>>>>>>> better. With the current KVM intel-pt implementation, what
>>>>>>> happens if the CPUID[14h] data seen by the guest doesn't match
>>>>>>> exactly the CPUID[14h] leaves from the host?
>>>>>>
>>>>>> Some bits in there can be treated as CPU features (e.g. EBX bit 0
>>>>>> "CR3 filtering support"). Probably we should handle these in KVM right now.
>>>>>> KVM needs to compute a mask of valid 1 bits for IA32_RTIT_CTL based
>>>>>> on CPUID, and apply it when the MSR is written.
>>>>>
>>>>> Does this mean QEMU can't set CPUID values that won't match the host
>>>>> with the existing implementation, or this won't matter for
>>>>> well-behaved guests that don't try to set reserved bits on the MSRs?
>>>>
>>>> All the features could be handled exactly like regular feature bits.
>>>> If QEMU sets them incorrectly and "enforce" is not used, bad things
>>>> happen but it's the user's fault.
>>>
>>> Oh, I mean setting the bit to 0 when it's 1 on the host (if it's
>>> 0 on the host, QEMU would never set it anyway). Is it safe to do it
>>> with the current KVM intel-pt implementation?
>>
>> It's not, but it's (very) easy to fix.
>
> Hi Paolo,
> Do you mean there need to add some check before setting IA32_RTIT_CTL
> MSR in KVM because some bits of this MSR is depend on the result of
> CPUID[14]. Any attempts to change these reserved bit should cause a #GP.
Yes, but the guest's CPUID[14] need not match the host.
Likewise, the number of address range MSRs in the guest, from
CPUID[EAX=14h,ECX=1].EAX[2:0], might be lower than in the host.
>>>>>> It also needs to
>>>>>> whitelist bits like we do for other feature words. These include:
>>>>>>
>>>>>> - CPUID[EAX=14h,ECX=0].EBX
>>>>>>
>>>>>> - CPUID[EAX=14h,ECX=0].ECX except bit 31
>>>>>>
>>>>>> - CPUID[EAX=14h,ECX=1].EAX bits 16:31 (if
>>>>>> CPUID[EAX=14h,ECX=0].EBX[3]=1)
>>>>>>
>>>>>> - CPUID[EAX=14h,ECX=1].EBX (if CPUID[EAX=14h,ECX=0].EBX[1]=1)
>>>>>
>>>>> What do you mean by whitelist?
>>>>
>>>> KVM needs to tell QEMU the bits it knows about.
>
> I think kvm_arch_get_supported_cpuid() function can get the result of CPUID[14] from KVM. Is this the whitelist what you mentioned?
Whitelist means that KVM must not return all the bits from CPUID[14];
only those it knows about.
Paolo
> Thanks,
> Luwei Kang
>
>>>
>>> So KVM isn't currently doing it on GET_SUPPORTED_CPUID? Oops.
>>>
>>>
>>>>
>>>>>> Others, currently only CPUID[EAX=14h,ECX=0].ECX[31] must match,
>>>>>> there is no way to emulate the "wrong" value.
>>>>>
>>>>> In this case we could make it configurable but require the host and
>>>>> guest value to always match.
>>>>>
>>>>> This might be an obstacle to enabling intel-pt by default (because
>>>>> it could make VMs not migratable to newer hosts), but may allow the
>>>>> feature to be configured in a predictable way.
>>>>
>>>> Yeah, but consider that virtualized PT anyway would only be enabled
>>>> on Ice Lake processors. It's a few years away anyway!
>>>>
>>>>>> Others, currently only CPUID[EAX=14h,ECX=1].EAX[2:0] are numeric
>>>>>> values, and it's possible to emulate a lower value than the one in the processor.
>>>>>
>>>>> This could be handled by QEMU. There's no requirement that all
>>>>> GET_SUPPORTED_CPUID values should be validated by simple bit
>>>>> masking.
>>>>
>>>> Good!
>>>>
>>>> Paolo
>>>
>
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [Qemu-devel] [PATCH RESEND v1 1/2] i386: Add Intel Processor Trace feature support
@ 2018-01-26 9:19 ` Paolo Bonzini
0 siblings, 0 replies; 44+ messages in thread
From: Paolo Bonzini @ 2018-01-26 9:19 UTC (permalink / raw)
To: Kang, Luwei, Eduardo Habkost
Cc: qemu-devel, kvm, rth, mtosatti, Chao Peng, libvir-list
On 22/01/2018 11:36, Kang, Luwei wrote:
>>> On Thu, Jan 18, 2018 at 03:44:49PM +0100, Paolo Bonzini wrote:
>>>> On 18/01/2018 15:37, Eduardo Habkost wrote:
>>>>> On Thu, Jan 18, 2018 at 02:39:57PM +0100, Paolo Bonzini wrote:
>>>>>> On 18/01/2018 14:24, Eduardo Habkost wrote:
>>>>>>> However, if there's a simple way to make it possible to migrate
>>>>>>> between hosts with different CPUID[14h] data, it would be even
>>>>>>> better. With the current KVM intel-pt implementation, what
>>>>>>> happens if the CPUID[14h] data seen by the guest doesn't match
>>>>>>> exactly the CPUID[14h] leaves from the host?
>>>>>>
>>>>>> Some bits in there can be treated as CPU features (e.g. EBX bit 0
>>>>>> "CR3 filtering support"). Probably we should handle these in KVM right now.
>>>>>> KVM needs to compute a mask of valid 1 bits for IA32_RTIT_CTL based
>>>>>> on CPUID, and apply it when the MSR is written.
>>>>>
>>>>> Does this mean QEMU can't set CPUID values that won't match the host
>>>>> with the existing implementation, or this won't matter for
>>>>> well-behaved guests that don't try to set reserved bits on the MSRs?
>>>>
>>>> All the features could be handled exactly like regular feature bits.
>>>> If QEMU sets them incorrectly and "enforce" is not used, bad things
>>>> happen but it's the user's fault.
>>>
>>> Oh, I mean setting the bit to 0 when it's 1 on the host (if it's
>>> 0 on the host, QEMU would never set it anyway). Is it safe to do it
>>> with the current KVM intel-pt implementation?
>>
>> It's not, but it's (very) easy to fix.
>
> Hi Paolo,
> Do you mean there need to add some check before setting IA32_RTIT_CTL
> MSR in KVM because some bits of this MSR is depend on the result of
> CPUID[14]. Any attempts to change these reserved bit should cause a #GP.
Yes, but the guest's CPUID[14] need not match the host.
Likewise, the number of address range MSRs in the guest, from
CPUID[EAX=14h,ECX=1].EAX[2:0], might be lower than in the host.
>>>>>> It also needs to
>>>>>> whitelist bits like we do for other feature words. These include:
>>>>>>
>>>>>> - CPUID[EAX=14h,ECX=0].EBX
>>>>>>
>>>>>> - CPUID[EAX=14h,ECX=0].ECX except bit 31
>>>>>>
>>>>>> - CPUID[EAX=14h,ECX=1].EAX bits 16:31 (if
>>>>>> CPUID[EAX=14h,ECX=0].EBX[3]=1)
>>>>>>
>>>>>> - CPUID[EAX=14h,ECX=1].EBX (if CPUID[EAX=14h,ECX=0].EBX[1]=1)
>>>>>
>>>>> What do you mean by whitelist?
>>>>
>>>> KVM needs to tell QEMU the bits it knows about.
>
> I think kvm_arch_get_supported_cpuid() function can get the result of CPUID[14] from KVM. Is this the whitelist what you mentioned?
Whitelist means that KVM must not return all the bits from CPUID[14];
only those it knows about.
Paolo
> Thanks,
> Luwei Kang
>
>>>
>>> So KVM isn't currently doing it on GET_SUPPORTED_CPUID? Oops.
>>>
>>>
>>>>
>>>>>> Others, currently only CPUID[EAX=14h,ECX=0].ECX[31] must match,
>>>>>> there is no way to emulate the "wrong" value.
>>>>>
>>>>> In this case we could make it configurable but require the host and
>>>>> guest value to always match.
>>>>>
>>>>> This might be an obstacle to enabling intel-pt by default (because
>>>>> it could make VMs not migratable to newer hosts), but may allow the
>>>>> feature to be configured in a predictable way.
>>>>
>>>> Yeah, but consider that virtualized PT anyway would only be enabled
>>>> on Ice Lake processors. It's a few years away anyway!
>>>>
>>>>>> Others, currently only CPUID[EAX=14h,ECX=1].EAX[2:0] are numeric
>>>>>> values, and it's possible to emulate a lower value than the one in the processor.
>>>>>
>>>>> This could be handled by QEMU. There's no requirement that all
>>>>> GET_SUPPORTED_CPUID values should be validated by simple bit
>>>>> masking.
>>>>
>>>> Good!
>>>>
>>>> Paolo
>>>
>
^ permalink raw reply [flat|nested] 44+ messages in thread
end of thread, other threads:[~2018-01-26 16:27 UTC | newest]
Thread overview: 44+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-08 20:36 [PATCH RESEND v1 1/2] i386: Add Intel Processor Trace feature support Luwei Kang
2018-01-08 20:36 ` [Qemu-devel] " Luwei Kang
2018-01-08 20:36 ` [PATCH RESEND v1 2/2] i386: Add support to get/set/migrate Intel Processor Trace feature Luwei Kang
2018-01-08 20:36 ` [Qemu-devel] " Luwei Kang
2018-01-12 14:22 ` [PATCH RESEND v1 1/2] i386: Add Intel Processor Trace feature support Eduardo Habkost
2018-01-12 14:22 ` [Qemu-devel] " Eduardo Habkost
2018-01-15 7:19 ` Kang, Luwei
2018-01-15 7:19 ` [Qemu-devel] " Kang, Luwei
2018-01-15 9:33 ` Paolo Bonzini
2018-01-15 9:33 ` [Qemu-devel] " Paolo Bonzini
2018-01-15 14:04 ` Eduardo Habkost
2018-01-15 14:04 ` [Qemu-devel] " Eduardo Habkost
2018-01-15 14:25 ` Jiri Denemark
2018-01-15 14:25 ` [Qemu-devel] " Jiri Denemark
2018-01-15 14:31 ` Eduardo Habkost
2018-01-15 14:31 ` [Qemu-devel] " Eduardo Habkost
2018-01-16 6:10 ` Kang, Luwei
2018-01-16 6:10 ` [Qemu-devel] " Kang, Luwei
2018-01-16 11:51 ` Eduardo Habkost
2018-01-16 11:51 ` [Qemu-devel] " Eduardo Habkost
2018-01-17 10:32 ` Kang, Luwei
2018-01-17 10:32 ` [Qemu-devel] " Kang, Luwei
2018-01-18 2:42 ` Eduardo Habkost
2018-01-18 2:42 ` [Qemu-devel] " Eduardo Habkost
2018-01-18 5:33 ` Kang, Luwei
2018-01-18 5:33 ` [Qemu-devel] " Kang, Luwei
2018-01-18 13:24 ` Eduardo Habkost
2018-01-18 13:24 ` [Qemu-devel] " Eduardo Habkost
2018-01-18 13:39 ` Paolo Bonzini
2018-01-18 13:39 ` [Qemu-devel] " Paolo Bonzini
2018-01-18 14:37 ` Eduardo Habkost
2018-01-18 14:37 ` [Qemu-devel] " Eduardo Habkost
2018-01-18 14:44 ` Paolo Bonzini
2018-01-18 14:44 ` [Qemu-devel] " Paolo Bonzini
2018-01-18 16:52 ` Eduardo Habkost
2018-01-18 16:52 ` [Qemu-devel] " Eduardo Habkost
2018-01-18 16:53 ` Paolo Bonzini
2018-01-18 16:53 ` [Qemu-devel] " Paolo Bonzini
2018-01-22 10:36 ` Kang, Luwei
2018-01-22 10:36 ` [Qemu-devel] " Kang, Luwei
2018-01-26 9:19 ` Paolo Bonzini
2018-01-26 9:19 ` [Qemu-devel] " Paolo Bonzini
2018-01-22 10:45 ` Kang, Luwei
2018-01-22 10:45 ` [Qemu-devel] " Kang, Luwei
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.