From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:32800) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1V1VfG-0006PZ-6K for qemu-devel@nongnu.org; Tue, 23 Jul 2013 02:01:39 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1V1VfE-0007Sr-BZ for qemu-devel@nongnu.org; Tue, 23 Jul 2013 02:01:38 -0400 Received: from mx1.redhat.com ([209.132.183.28]:27087) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1V1VfE-0007SF-4j for qemu-devel@nongnu.org; Tue, 23 Jul 2013 02:01:36 -0400 Date: Tue, 23 Jul 2013 08:01:29 +0200 From: Igor Mammedov Message-ID: <20130723080129.5eed23d1@nial.usersys.redhat.com> In-Reply-To: <1374521135-30404-3-git-send-email-ehabkost@redhat.com> References: <1374521135-30404-1-git-send-email-ehabkost@redhat.com> <1374521135-30404-3-git-send-email-ehabkost@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [qom-cpu PATCH 2/2] i386: disable PMU passthrough mode by default List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eduardo Habkost Cc: qemu-devel@nongnu.org, Gleb Natapov , Andreas =?ISO-8859-1?B?RuRyYmVy?= On Mon, 22 Jul 2013 16:25:35 -0300 Eduardo Habkost wrote: > Bug description: QEMU currently gets all bits from GET_SUPPORTED_CPUID > for CPUID leaf 0xA and passes them directly to the guest. This makes > the guest ABI depend on host kernel and host CPU capabilities, and > breaks live migration if we migrate between host with different > capabilities (e.g. different number of PMU counters). > > This patch adds a "pmu-passthrough" property to X86CPU, and set it to > true only on "-cpu host", or on pc-*-1.5 and older machine-types. > > Signed-off-by: Eduardo Habkost > --- > include/hw/i386/pc.h | 4 ++++ > target-i386/cpu-qom.h | 7 +++++++ > target-i386/cpu.c | 11 ++++++++++- > 3 files changed, 21 insertions(+), 1 deletion(-) > > diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h > index 7fb97b0..3cea83f 100644 > --- a/include/hw/i386/pc.h > +++ b/include/hw/i386/pc.h > @@ -235,6 +235,10 @@ int e820_add_entry(uint64_t, uint64_t, uint32_t); > .driver = "virtio-net-pci",\ > .property = "any_layout",\ > .value = "off",\ > + },{\ > + .driver = TYPE_X86_CPU,\ > + .property = "pmu-passthrough",\ > + .value = "on",\ > } > > #define PC_COMPAT_1_4 \ > diff --git a/target-i386/cpu-qom.h b/target-i386/cpu-qom.h > index 7e55e5f..b505a45 100644 > --- a/target-i386/cpu-qom.h > +++ b/target-i386/cpu-qom.h > @@ -68,6 +68,13 @@ typedef struct X86CPU { > > /* Features that were filtered out because of missing host capabilities */ > uint32_t filtered_features[FEATURE_WORDS]; > + > + /* Pass all PMU CPUID bits to the guest directly from GET_SUPPORTED_CPUID. > + * This can't be enabled by default because it breaks live-migration, > + * as it makes the guest ABI change depending on host CPU/kernel > + * capabilities. > + */ > + bool pmu_passthrough; > } X86CPU; > > static inline X86CPU *x86_env_get_cpu(CPUX86State *env) > diff --git a/target-i386/cpu.c b/target-i386/cpu.c > index 41c81af..e192f63 100644 > --- a/target-i386/cpu.c > +++ b/target-i386/cpu.c > @@ -1475,17 +1475,25 @@ static void x86_cpu_get_feature_words(Object *obj, Visitor *v, void *opaque, > error_propagate(errp, err); > } > > +static Property cpu_x86_properties[] = { > + DEFINE_PROP_BOOL("pmu-passthrough", X86CPU, pmu_passthrough, false), > + DEFINE_PROP_END_OF_LIST(), > +}; > + > static int cpu_x86_find_by_name(X86CPU *cpu, x86_def_t *x86_cpu_def, > const char *name) > { > x86_def_t *def; > int i; > + Error *err = NULL; > > if (name == NULL) { > return -1; > } > if (kvm_enabled() && strcmp(name, "host") == 0) { > kvm_cpu_fill_host(x86_cpu_def); > + object_property_set_bool(OBJECT(cpu), true, "pmu-passthrough", &err); > + assert_no_error(err); Could this hunk be implemented using compat props? That would spare us dealing with it later. > return 0; > } > > @@ -2017,7 +2025,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count, > break; > case 0xA: > /* Architectural Performance Monitoring Leaf */ > - if (kvm_enabled()) { > + if (kvm_enabled() && cpu->pmu_passthrough) { > KVMState *s = cs->kvm_state; > > *eax = kvm_arch_get_supported_cpuid(s, 0xA, count, R_EAX); > @@ -2516,6 +2524,7 @@ static void x86_cpu_common_class_init(ObjectClass *oc, void *data) > xcc->parent_realize = dc->realize; > dc->realize = x86_cpu_realizefn; > dc->bus_type = TYPE_ICC_BUS; > + dc->props = cpu_x86_properties; > > xcc->parent_reset = cc->reset; > cc->reset = x86_cpu_reset;