Re: [Qemu-devel] [qom-cpu PATCH 2/2] i386: disable PMU passthrough mode by default

[Paolo Bonzini]

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.

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.

Paolo

> Signed-off-by: Eduardo Habkost <address@hidden>
> ---
>  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;
>