On Tue, Jul 23, 2013 at 11:18:03AM +0200, Paolo Bonzini wrote: > Il 22/07/2013 21:25, Eduardo Habkost ha scritto: > > 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. > > Can we just call the property "pmu"? It doesn't have to be passthough.
Yes, but the only options we have today are "no PMU" and "passthrough PMU". I wouldn't like to make "pmu=on" enable the passthrough behavior implicitly (I don't want things that break live-migration to be enabled without making it explicit that it is a host-dependent/passthrough mode). I considered creating a property named "pmu" and use "pmu=host" to enable the current passthrough behavior, but: > > Later we can support setting the right values for leaf 0xA. This way > migration will work even for -cpu other than "-cpu host", and the same > option will work. Yes. I just don't know what will be the best way to specify the PMU counters in the command-line/properties when we do it, so I thought it would be better to create a "pmu" property only after we decide how exactly it will look like. > > Paolo > > > Signed-off-by: Eduardo Habkost <ehabk...@redhat.com> > > --- > > 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); > > 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; > > > -- Eduardo