Marshaling of processor tracing MSRs is not yet implemented in
QEMU, mark the feature as unmigratable.
Cc: qemu-stable@nongnu.org
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
target/i386/cpu.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 9c54c41e7a..a6a597d2bf 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -1012,6 +1012,7 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
.needs_ecx = true, .ecx = 0,
.reg = R_EBX,
},
+ .unmigratable_features = CPUID_7_0_EBX_INTEL_PT,
.tcg_features = TCG_7_0_EBX_FEATURES,
},
[FEAT_7_0_ECX] = {
--
2.20.1
> From: Qemu-devel [mailto:qemu-devel-bounces+luwei.kang=intel.com@nongnu.org] On Behalf Of Paolo Bonzini > Sent: Friday, December 21, 2018 8:44 PM > To: qemu-devel@nongnu.org > Cc: ehabkost@redhat.com; qemu-stable@nongnu.org > Subject: [Qemu-devel] [PATCH] i386: mark the 'INTEL_PT' CPUID bit as unmigratable > > Marshaling of processor tracing MSRs is not yet implemented in QEMU, mark the feature as unmigratable. Hi Paolo, I think Intel PT has supported live migration. I don't understand what you mean. commit b77146e9a129bcdb60edc23639211679ae846a92 Author: Chao Peng <chao.p.peng@linux.intel.com> Date: Mon Mar 5 00:48:36 2018 +0800 i386: Add support to get/set/migrate Intel Processor Trace feature Thanks, Luwei Kang > > Cc: qemu-stable@nongnu.org > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- > target/i386/cpu.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/target/i386/cpu.c b/target/i386/cpu.c index 9c54c41e7a..a6a597d2bf 100644 > --- a/target/i386/cpu.c > +++ b/target/i386/cpu.c > @@ -1012,6 +1012,7 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = { > .needs_ecx = true, .ecx = 0, > .reg = R_EBX, > }, > + .unmigratable_features = CPUID_7_0_EBX_INTEL_PT, > .tcg_features = TCG_7_0_EBX_FEATURES, > }, > [FEAT_7_0_ECX] = { > -- > 2.20.1 >
On 25/12/18 09:23, Kang, Luwei wrote: >> From: Qemu-devel [mailto:qemu-devel-bounces+luwei.kang=intel.com@nongnu.org] On Behalf Of Paolo Bonzini >> Sent: Friday, December 21, 2018 8:44 PM >> To: qemu-devel@nongnu.org >> Cc: ehabkost@redhat.com; qemu-stable@nongnu.org >> Subject: [Qemu-devel] [PATCH] i386: mark the 'INTEL_PT' CPUID bit as unmigratable >> >> Marshaling of processor tracing MSRs is not yet implemented in QEMU, mark the feature as unmigratable. > > Hi Paolo, > I think Intel PT has supported live migration. I don't understand what you mean. > > commit b77146e9a129bcdb60edc23639211679ae846a92 > Author: Chao Peng <chao.p.peng@linux.intel.com> > Date: Mon Mar 5 00:48:36 2018 +0800 > i386: Add support to get/set/migrate Intel Processor Trace feature Indeed. I had forgotten this patch because it was committed so long before the kernel parts (which really should not happen, but Eduardo and I miscommunicated on it). Can you check that it still works? Paolo
On Fri, Dec 28, 2018 at 02:53:22PM +0100, Paolo Bonzini wrote: > On 25/12/18 09:23, Kang, Luwei wrote: > >> From: Qemu-devel [mailto:qemu-devel-bounces+luwei.kang=intel.com@nongnu.org] On Behalf Of Paolo Bonzini > >> Sent: Friday, December 21, 2018 8:44 PM > >> To: qemu-devel@nongnu.org > >> Cc: ehabkost@redhat.com; qemu-stable@nongnu.org > >> Subject: [Qemu-devel] [PATCH] i386: mark the 'INTEL_PT' CPUID bit as unmigratable > >> > >> Marshaling of processor tracing MSRs is not yet implemented in QEMU, mark the feature as unmigratable. > > > > Hi Paolo, > > I think Intel PT has supported live migration. I don't understand what you mean. > > > > commit b77146e9a129bcdb60edc23639211679ae846a92 > > Author: Chao Peng <chao.p.peng@linux.intel.com> > > Date: Mon Mar 5 00:48:36 2018 +0800 > > i386: Add support to get/set/migrate Intel Processor Trace feature > > Indeed. I had forgotten this patch because it was committed so long > before the kernel parts (which really should not happen, but Eduardo and > I miscommunicated on it). Can you check that it still works? My mistake, sorry. I should have checked the status of the kernel code before merging the original series, or waited for your review. I'm re-reading the code now and I'm worried about two things: The code will break if GET_SUPPORTED_CPUID returns INTEL_PT, but the MSR emulation code is not present yet. Maybe it won't be an issue in practice because it happens only between the two Linux commits (commit 86f5201df0d3 "KVM: x86: Add Intel Processor Trace cpuid emulation" and commit bf8c55d8dc09 "KVM: x86: Implement Intel PT MSRs read/write emulation") and shipping a kernel with the CPUID code without the MSR commit seems pointless. The kvm_arch_get_supported_cpuid() call inside kvm_get_msrs() looks suspicious. What should happen if we try to migrate to a host that returns a smaller number on kvm_arch_get_supported_cpuid(0x14, 1, R_EAX)? -- Eduardo
> > On 25/12/18 09:23, Kang, Luwei wrote: > > >> From: Qemu-devel > > >> [mailto:qemu-devel-bounces+luwei.kang=intel.com@nongnu.org] On > > >> Behalf Of Paolo Bonzini > > >> Sent: Friday, December 21, 2018 8:44 PM > > >> To: qemu-devel@nongnu.org > > >> Cc: ehabkost@redhat.com; qemu-stable@nongnu.org > > >> Subject: [Qemu-devel] [PATCH] i386: mark the 'INTEL_PT' CPUID bit > > >> as unmigratable > > >> > > >> Marshaling of processor tracing MSRs is not yet implemented in QEMU, mark the feature as unmigratable. > > > > > > Hi Paolo, > > > I think Intel PT has supported live migration. I don't understand what you mean. > > > > > > commit b77146e9a129bcdb60edc23639211679ae846a92 > > > Author: Chao Peng <chao.p.peng@linux.intel.com> > > > Date: Mon Mar 5 00:48:36 2018 +0800 > > > i386: Add support to get/set/migrate Intel Processor Trace > > > feature > > > > Indeed. I had forgotten this patch because it was committed so long > > before the kernel parts (which really should not happen, but Eduardo > > and I miscommunicated on it). Can you check that it still works? > > My mistake, sorry. I should have checked the status of the kernel code before merging the original series, or waited for your review. > > I'm re-reading the code now and I'm worried about two things: > > The code will break if GET_SUPPORTED_CPUID returns INTEL_PT, but the MSR emulation code is not present yet. Maybe it won't be an > issue in practice because it happens only between the two Linux commits (commit 86f5201df0d3 "KVM: x86: Add Intel Processor Trace > cpuid emulation" and commit bf8c55d8dc09 "KVM: x86: Implement Intel PT MSRs read/write emulation") and shipping a kernel with the > CPUID code without the MSR commit seems pointless. > > The kvm_arch_get_supported_cpuid() call inside kvm_get_msrs() looks suspicious. What should happen if we try to migrate to a host that > returns a smaller number on kvm_arch_get_supported_cpuid(0x14, 1, R_EAX)? Hi Eduardo, I think we have some discussion on this feature about live migration safe about two years ago. In order to make live migration safe, we set all the values of PT CPUID as constant. I think what your concern is the number of address range (CPUID:14H.01.EAX[bit02:00]). Currently, it is a constant value (#define INTEL_PT_ADDR_RANGES_NUM 0x2) for guest. 1. if the hardware support < 2, Intel PT will not exposed to guest; 2. if the hardware support >= 2, we just expose 2 to guest. So the number of address range in guest is always 2 if Intel PT is supported in guest (there also have other pre-condition check). The value of guest Intel PT CPUID information. + if (count == 0) { + *eax = INTEL_PT_MAX_SUBLEAF; + *ebx = INTEL_PT_MINIMAL_EBX; + *ecx = INTEL_PT_MINIMAL_ECX; + } else if (count == 1) { + *eax = INTEL_PT_MTC_BITMAP | INTEL_PT_ADDR_RANGES_NUM; + *ebx = INTEL_PT_PSB_BITMAP | INTEL_PT_CYCLE_BITMAP; + } The condition check if we can expose Intel PT to guest. + if (!eax_0 || + ((ebx_0 & INTEL_PT_MINIMAL_EBX) != INTEL_PT_MINIMAL_EBX) || + ((ecx_0 & INTEL_PT_MINIMAL_ECX) != INTEL_PT_MINIMAL_ECX) || + ((eax_1 & INTEL_PT_MTC_BITMAP) != INTEL_PT_MTC_BITMAP) || + ((eax_1 & INTEL_PT_ADDR_RANGES_NUM_MASK) < + INTEL_PT_ADDR_RANGES_NUM) || + ((ebx_1 & (INTEL_PT_PSB_BITMAP | INTEL_PT_CYCLE_BITMAP)) != + (INTEL_PT_PSB_BITMAP | INTEL_PT_CYCLE_BITMAP))) { + /* + * Processor Trace capabilities aren't configurable, so if the + * host can't emulate the capabilities we report on + * cpu_x86_cpuid(), intel-pt can't be enabled on the current host. + */ + env->features[FEAT_7_0_EBX] &= ~CPUID_7_0_EBX_INTEL_PT; + cpu->filtered_features[FEAT_7_0_EBX] |= CPUID_7_0_EBX_INTEL_PT; + rv = 1; + } Thanks, Luwei Kang
On Wed, Jan 02, 2019 at 01:30:28AM +0000, Kang, Luwei wrote: > > > On 25/12/18 09:23, Kang, Luwei wrote: > > > >> From: Qemu-devel > > > >> [mailto:qemu-devel-bounces+luwei.kang=intel.com@nongnu.org] On > > > >> Behalf Of Paolo Bonzini > > > >> Sent: Friday, December 21, 2018 8:44 PM > > > >> To: qemu-devel@nongnu.org > > > >> Cc: ehabkost@redhat.com; qemu-stable@nongnu.org > > > >> Subject: [Qemu-devel] [PATCH] i386: mark the 'INTEL_PT' CPUID bit > > > >> as unmigratable > > > >> > > > >> Marshaling of processor tracing MSRs is not yet implemented in QEMU, mark the feature as unmigratable. > > > > > > > > Hi Paolo, > > > > I think Intel PT has supported live migration. I don't understand what you mean. > > > > > > > > commit b77146e9a129bcdb60edc23639211679ae846a92 > > > > Author: Chao Peng <chao.p.peng@linux.intel.com> > > > > Date: Mon Mar 5 00:48:36 2018 +0800 > > > > i386: Add support to get/set/migrate Intel Processor Trace > > > > feature > > > > > > Indeed. I had forgotten this patch because it was committed so long > > > before the kernel parts (which really should not happen, but Eduardo > > > and I miscommunicated on it). Can you check that it still works? > > > > My mistake, sorry. I should have checked the status of the kernel code before merging the original series, or waited for your review. > > > > I'm re-reading the code now and I'm worried about two things: > > > > The code will break if GET_SUPPORTED_CPUID returns INTEL_PT, but the MSR emulation code is not present yet. Maybe it won't be an > > issue in practice because it happens only between the two Linux commits (commit 86f5201df0d3 "KVM: x86: Add Intel Processor Trace > > cpuid emulation" and commit bf8c55d8dc09 "KVM: x86: Implement Intel PT MSRs read/write emulation") and shipping a kernel with the > > CPUID code without the MSR commit seems pointless. > > > > The kvm_arch_get_supported_cpuid() call inside kvm_get_msrs() looks suspicious. What should happen if we try to migrate to a host that > > returns a smaller number on kvm_arch_get_supported_cpuid(0x14, 1, R_EAX)? > > Hi Eduardo, > I think we have some discussion on this feature about live migration safe about two years ago. In order to make live migration safe, we set all the values of PT CPUID as constant. > I think what your concern is the number of address range (CPUID:14H.01.EAX[bit02:00]). Currently, it is a constant value (#define INTEL_PT_ADDR_RANGES_NUM 0x2) for guest. > 1. if the hardware support < 2, Intel PT will not exposed to guest; > 2. if the hardware support >= 2, we just expose 2 to guest. > So the number of address range in guest is always 2 if Intel PT is supported in guest (there also have other pre-condition check). Right, I forgot about that part of the code. So the CPU state save/load code simply saves/loads everything supported by the host, and the responsibility of keeping guest ABI is lies on target/i386/cpu.c. The code looks safe to me. The only unexpected side-effect is unnecessarily loading/saving of MSR_IA32_RTIT_ADDR[123]*, which should be harmless. Thanks for the explanation! > > The value of guest Intel PT CPUID information. > + if (count == 0) { > + *eax = INTEL_PT_MAX_SUBLEAF; > + *ebx = INTEL_PT_MINIMAL_EBX; > + *ecx = INTEL_PT_MINIMAL_ECX; > + } else if (count == 1) { > + *eax = INTEL_PT_MTC_BITMAP | INTEL_PT_ADDR_RANGES_NUM; > + *ebx = INTEL_PT_PSB_BITMAP | INTEL_PT_CYCLE_BITMAP; > + } > > The condition check if we can expose Intel PT to guest. > + if (!eax_0 || > + ((ebx_0 & INTEL_PT_MINIMAL_EBX) != INTEL_PT_MINIMAL_EBX) || > + ((ecx_0 & INTEL_PT_MINIMAL_ECX) != INTEL_PT_MINIMAL_ECX) || > + ((eax_1 & INTEL_PT_MTC_BITMAP) != INTEL_PT_MTC_BITMAP) || > + ((eax_1 & INTEL_PT_ADDR_RANGES_NUM_MASK) < > + INTEL_PT_ADDR_RANGES_NUM) || > + ((ebx_1 & (INTEL_PT_PSB_BITMAP | INTEL_PT_CYCLE_BITMAP)) != > + (INTEL_PT_PSB_BITMAP | INTEL_PT_CYCLE_BITMAP))) { > + /* > + * Processor Trace capabilities aren't configurable, so if the > + * host can't emulate the capabilities we report on > + * cpu_x86_cpuid(), intel-pt can't be enabled on the current host. > + */ > + env->features[FEAT_7_0_EBX] &= ~CPUID_7_0_EBX_INTEL_PT; > + cpu->filtered_features[FEAT_7_0_EBX] |= CPUID_7_0_EBX_INTEL_PT; > + rv = 1; > + } > > > Thanks, > Luwei Kang -- Eduardo
> On 25/12/18 09:23, Kang, Luwei wrote: > >> From: Qemu-devel > >> [mailto:qemu-devel-bounces+luwei.kang=intel.com@nongnu.org] On Behalf > >> Of Paolo Bonzini > >> Sent: Friday, December 21, 2018 8:44 PM > >> To: qemu-devel@nongnu.org > >> Cc: ehabkost@redhat.com; qemu-stable@nongnu.org > >> Subject: [Qemu-devel] [PATCH] i386: mark the 'INTEL_PT' CPUID bit as > >> unmigratable > >> > >> Marshaling of processor tracing MSRs is not yet implemented in QEMU, mark the feature as unmigratable. > > > > Hi Paolo, > > I think Intel PT has supported live migration. I don't understand what you mean. > > > > commit b77146e9a129bcdb60edc23639211679ae846a92 > > Author: Chao Peng <chao.p.peng@linux.intel.com> > > Date: Mon Mar 5 00:48:36 2018 +0800 > > i386: Add support to get/set/migrate Intel Processor Trace feature > > Indeed. I had forgotten this patch because it was committed so long before the kernel parts (which really should not happen, but Eduardo > and I miscommunicated on it). Can you check that it still works? Yes, it is indeed so long. I will check if it still works. Thanks, Luwei Kang
Patchew URL: https://patchew.org/QEMU/20181221124410.11920-1-pbonzini@redhat.com/ Hi, This series failed the docker-quick@centos7 build test. Please find the testing commands and their output below. If you have Docker installed, you can probably reproduce it locally. === TEST SCRIPT BEGIN === #!/bin/bash time make docker-test-quick@centos7 SHOW_ENV=1 J=8 === TEST SCRIPT END === libpmem support no libudev no WARNING: Use of SDL 1.2 is deprecated and will be removed in WARNING: future releases. Please switch to using SDL 2.0 NOTE: cross-compilers enabled: 'cc' GEN x86_64-softmmu/config-devices.mak.tmp --- CC aarch64-softmmu/target/arm/translate.o CC aarch64-softmmu/target/arm/op_helper.o CC aarch64-softmmu/target/arm/helper.o /tmp/qemu-test/src/target/i386/cpu.c:1015:9: error: unknown field 'unmigratable_features' specified in initializer .unmigratable_features = CPUID_7_0_EBX_INTEL_PT, ^ make[1]: *** [target/i386/cpu.o] Error 1 The full log is available at http://patchew.org/logs/20181221124410.11920-1-pbonzini@redhat.com/testing.docker-quick@centos7/?type=message. --- Email generated automatically by Patchew [http://patchew.org/]. Please send your feedback to patchew-devel@redhat.com
Patchew URL: https://patchew.org/QEMU/20181221124410.11920-1-pbonzini@redhat.com/ Hi, This series failed the docker-mingw@fedora build test. Please find the testing commands and their output below. If you have Docker installed, you can probably reproduce it locally. === TEST SCRIPT BEGIN === #!/bin/bash time make docker-test-mingw@fedora SHOW_ENV=1 J=8 === TEST SCRIPT END === CC aarch64-softmmu/hw/misc/cbus.o CC x86_64-softmmu/trace/generated-helpers.o CC aarch64-softmmu/hw/misc/exynos4210_pmu.o /tmp/qemu-test/src/target/i386/cpu.c:1015:10: error: 'FeatureWordInfo {aka struct FeatureWordInfo}' has no member named 'unmigratable_features'; did you mean 'unmigratable_flags'? .unmigratable_features = CPUID_7_0_EBX_INTEL_PT, ^~~~~~~~~~~~~~~~~~~~~ unmigratable_flags The full log is available at http://patchew.org/logs/20181221124410.11920-1-pbonzini@redhat.com/testing.docker-mingw@fedora/?type=message. --- Email generated automatically by Patchew [http://patchew.org/]. Please send your feedback to patchew-devel@redhat.com
© 2016 - 2024 Red Hat, Inc.