Il 23/07/2013 16:13, Eduardo Habkost ha scritto:
> 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 think "passthrough PMU" should be considered a bug except of course
with "-cpu host".
If "-cpu Nehalem,pmu=on" goes from passthrough to Nehalem-compatible in
a future QEMU release, that'll be a bugfix.
Paolo
> 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;
>>>
>>
>
