[Qemu-devel] [qom-cpu PATCH 0/2] i386: disable PMU CPUID leaf by default

[Qemu-devel] [qom-cpu PATCH 0/2] i386: disable PMU CPUID leaf by default

[Eduardo Habkost]

Changes v1 -> v2:
 * compat property is now named "pmu" instead of "pmu-passthrough"

Eduardo Habkost (2):
  i386: pass X86CPU object to cpu_x86_find_by_name()
  i386: disable PMU CPUID leaves by default

 include/hw/i386/pc.h  |  4 ++++
 target-i386/cpu-qom.h |  7 +++++++
 target-i386/cpu.c     | 16 +++++++++++++---
 3 files changed, 24 insertions(+), 3 deletions(-)

-- 
1.8.3.1

[Qemu-devel] [qom-cpu PATCH v2 1/2] i386: pass X86CPU object to cpu_x86_find_by_name()

[Eduardo Habkost]

This will help us change the initialization code to not require carrying
some intermediate values in a x86_def_t struct (and eventually kill the
x86_def_t struct entirely).

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 target-i386/cpu.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index cd350cb..601febc 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1475,7 +1475,8 @@ static void x86_cpu_get_feature_words(Object *obj, Visitor *v, void *opaque,
     error_propagate(errp, err);
 }
 
-static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *name)
+static int cpu_x86_find_by_name(X86CPU *cpu, x86_def_t *x86_cpu_def,
+                                const char *name)
 {
     x86_def_t *def;
     int i;
@@ -1742,7 +1743,7 @@ static void cpu_x86_register(X86CPU *cpu, const char *name, Error **errp)
 
     memset(def, 0, sizeof(*def));
 
-    if (cpu_x86_find_by_name(def, name) < 0) {
+    if (cpu_x86_find_by_name(cpu, def, name) < 0) {
         error_setg(errp, "Unable to find CPU definition: %s", name);
         return;
     }
-- 
1.8.3.1

[Qemu-devel] [qom-cpu PATCH v2 2/2] i386: disable PMU CPUID leaf by default

[Eduardo Habkost]

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" property to X86CPU, and set it to true only on
"-cpu host", or on pc-*-1.5 and older machine-types.

By now, setting pmu=on will enable the current passthrough mode that
doesn't have any ABI stability guarantees, but in the future we may
implement a mode where the PMU CPUID bits are stable and configurable.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Cc: Paolo Bonzini <pbonzini@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..09c2dd4 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",\
+            .value = "on",\
         }
 
 #define PC_COMPAT_1_4 \
diff --git a/target-i386/cpu-qom.h b/target-i386/cpu-qom.h
index d928562..6b865e7 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];
+
+    /* Enable PMU CPUID bits. This can't be enabled by default yet because
+     * it doesn't have ABI stability guarantees, as it passes all PMU CPUID
+     * bits returned by GET_SUPPORTED_CPUID (that depend on host CPU and kernel
+     * capabilities) directly to the guest.
+     */
+    bool enable_pmu;
 } X86CPU;
 
 static inline X86CPU *x86_env_get_cpu(CPUX86State *env)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 601febc..73c7a09 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", X86CPU, enable_pmu, 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", &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->enable_pmu) {
             KVMState *s = cs->kvm_state;
 
             *eax = kvm_arch_get_supported_cpuid(s, 0xA, count, R_EAX);
@@ -2530,6 +2538,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;
-- 
1.8.3.1

Re: [Qemu-devel] [qom-cpu PATCH 0/2] i386: disable PMU CPUID leaf by default

[Andreas Färber]

Am 26.07.2013 22:09, schrieb Eduardo Habkost:
> Changes v1 -> v2:
>  * compat property is now named "pmu" instead of "pmu-passthrough"
> 
> Eduardo Habkost (2):
>   i386: pass X86CPU object to cpu_x86_find_by_name()
>   i386: disable PMU CPUID leaves by default
> 
>  include/hw/i386/pc.h  |  4 ++++
>  target-i386/cpu-qom.h |  7 +++++++
>  target-i386/cpu.c     | 16 +++++++++++++---
>  3 files changed, 24 insertions(+), 3 deletions(-)

Technically the patch looks okay for 1.6. Waiting for functional acks.

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [qom-cpu PATCH 0/2] i386: disable PMU CPUID leaf by default

[Andreas Färber]

Am 26.07.2013 22:15, schrieb Andreas Färber:
> Am 26.07.2013 22:09, schrieb Eduardo Habkost:
>> Changes v1 -> v2:
>>  * compat property is now named "pmu" instead of "pmu-passthrough"
>>
>> Eduardo Habkost (2):
>>   i386: pass X86CPU object to cpu_x86_find_by_name()
>>   i386: disable PMU CPUID leaves by default
>>
>>  include/hw/i386/pc.h  |  4 ++++
>>  target-i386/cpu-qom.h |  7 +++++++
>>  target-i386/cpu.c     | 16 +++++++++++++---
>>  3 files changed, 24 insertions(+), 3 deletions(-)
> 
> Technically the patch looks okay for 1.6. Waiting for functional acks.

Since time is running out and the name requested by Paolo was adopted, I
have queued this on qom-cpu with minor adjustments, most notably moving
and renaming the property array:
https://github.com/afaerber/qemu-cpu/commits/qom-cpu

Paolo, if there's still corrections necessary, let me know whether to
unqueue or whether we can follow-up during Hard Freeze.

Thanks,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg