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

2013-07-24 Thread Eduardo Habkost 
On Tue, Jul 23, 2013 at 09:43:06PM +0200, Paolo Bonzini wrote:
 Il 23/07/2013 19:41, Eduardo Habkost ha scritto:
  On Tue, Jul 23, 2013 at 06:23:08PM +0200, Paolo Bonzini wrote:
  Il 23/07/2013 17:40, Eduardo Habkost ha scritto:
  On Tue, Jul 23, 2013 at 05:09:02PM +0200, Paolo Bonzini wrote:
  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.
 
  Exactly. But then I don't understand your suggestion. We still need a
  property to enable pasthrough behavior on old machine-types (not
  perfect, but a best-effort way to try to keep compatibility),
 
  Do we?
 
  We only need pmu=on---which right now is buggy on old machine types
  because it will always passthrough.
  
  I am not sure I understand what you are arguing for.
  
  You agree that pmu=on needs to keep the buggy passthrough behavior on
  pc-1.5 and older, right?
 
 I agree it needs to remain enabled on 1.5.  But if, for example, 1.8
 makes pmu=on emulate a Nehalem-compatible PMU, I think it is fine if
 pc-1.5 moves from a host-compatible PMU to a Nehalem-compatible PMU.

That's where I disagree. Today users are (luckily) able to migrate
safely between hosts with the same number of PMU counters. But if we
make, e.g., qemu-1.6 -machine pc-1.5 -cpu Westmere present a smaller
number of PMU counters than qemu-1.5 -machine pc-1.5 -cpu Westmere on
the same host, we will break an existing setup where everything was
working before, which is something we could have easily avoided.

(Just to clarify what breaking this means in practice: changing the
number of PMU counters under the guest on live-migration means the guest
will crash when trying to use counters that suddenly went away, and it
may crash a very long time after it was migrated.)

 
 The reason is that pc-1.5 has never guaranteed any feature of the
 emulated PMU.

Right, current behavior is buggy and we never guaranteed anything, but
IMO we shouldn't break on purpose something that is working today.

-- 
Eduardo

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

2013-07-24 Thread Paolo Bonzini 
Il 24/07/2013 15:15, Eduardo Habkost ha scritto:
 On Tue, Jul 23, 2013 at 09:43:06PM +0200, Paolo Bonzini wrote:
 Il 23/07/2013 19:41, Eduardo Habkost ha scritto:
 On Tue, Jul 23, 2013 at 06:23:08PM +0200, Paolo Bonzini wrote:
 Il 23/07/2013 17:40, Eduardo Habkost ha scritto:
 On Tue, Jul 23, 2013 at 05:09:02PM +0200, Paolo Bonzini wrote:
 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.

 Exactly. But then I don't understand your suggestion. We still need a
 property to enable pasthrough behavior on old machine-types (not
 perfect, but a best-effort way to try to keep compatibility),

 Do we?

 We only need pmu=on---which right now is buggy on old machine types
 because it will always passthrough.

 I am not sure I understand what you are arguing for.

 You agree that pmu=on needs to keep the buggy passthrough behavior on
 pc-1.5 and older, right?

 I agree it needs to remain enabled on 1.5.  But if, for example, 1.8
 makes pmu=on emulate a Nehalem-compatible PMU, I think it is fine if
 pc-1.5 moves from a host-compatible PMU to a Nehalem-compatible PMU.
 
 That's where I disagree. Today users are (luckily) able to migrate
 safely between hosts with the same number of PMU counters. But if we
 make, e.g., qemu-1.6 -machine pc-1.5 -cpu Westmere present a smaller
 number of PMU counters than qemu-1.5 -machine pc-1.5 -cpu Westmere on
 the same host, we will break an existing setup where everything was
 working before, which is something we could have easily avoided.

But at the same time we will fix live migration from a Sandy Bridge host
to a Westmere.  So it's a choice we have to make anyway.

 (Just to clarify what breaking this means in practice: changing the
 number of PMU counters under the guest on live-migration means the guest
 will crash when trying to use counters that suddenly went away, and it
 may crash a very long time after it was migrated.)

And at the same time we fix live migration of a Sandy Bridge to a Westmere.

 The reason is that pc-1.5 has never guaranteed any feature of the
 emulated PMU.
 
 Right, current behavior is buggy and we never guaranteed anything, but
 IMO we shouldn't break on purpose something that is working today.

Even if it is to fix something else?

Paolo

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

2013-07-24 Thread Eduardo Habkost 
On Wed, Jul 24, 2013 at 03:21:48PM +0200, Paolo Bonzini wrote:
 Il 24/07/2013 15:15, Eduardo Habkost ha scritto:
  On Tue, Jul 23, 2013 at 09:43:06PM +0200, Paolo Bonzini wrote:
  Il 23/07/2013 19:41, Eduardo Habkost ha scritto:
  On Tue, Jul 23, 2013 at 06:23:08PM +0200, Paolo Bonzini wrote:
  Il 23/07/2013 17:40, Eduardo Habkost ha scritto:
  On Tue, Jul 23, 2013 at 05:09:02PM +0200, Paolo Bonzini wrote:
  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.
 
  Exactly. But then I don't understand your suggestion. We still need a
  property to enable pasthrough behavior on old machine-types (not
  perfect, but a best-effort way to try to keep compatibility),
 
  Do we?
 
  We only need pmu=on---which right now is buggy on old machine types
  because it will always passthrough.
 
  I am not sure I understand what you are arguing for.
 
  You agree that pmu=on needs to keep the buggy passthrough behavior on
  pc-1.5 and older, right?
 
  I agree it needs to remain enabled on 1.5.  But if, for example, 1.8
  makes pmu=on emulate a Nehalem-compatible PMU, I think it is fine if
  pc-1.5 moves from a host-compatible PMU to a Nehalem-compatible PMU.
  
  That's where I disagree. Today users are (luckily) able to migrate
  safely between hosts with the same number of PMU counters. But if we
  make, e.g., qemu-1.6 -machine pc-1.5 -cpu Westmere present a smaller
  number of PMU counters than qemu-1.5 -machine pc-1.5 -cpu Westmere on
  the same host, we will break an existing setup where everything was
  working before, which is something we could have easily avoided.
 
 But at the same time we will fix live migration from a Sandy Bridge host
 to a Westmere.  So it's a choice we have to make anyway.

True.

 
  (Just to clarify what breaking this means in practice: changing the
  number of PMU counters under the guest on live-migration means the guest
  will crash when trying to use counters that suddenly went away, and it
  may crash a very long time after it was migrated.)
 
 And at the same time we fix live migration of a Sandy Bridge to a Westmere.

Something that never worked in the first place. Breaking what is working
today, on the other hand, is a regression.

If users are interested in a fix for the new SandyBrige-Westmere
use-case, we can always say please upgrade your VM to a newer
machine-type.

 
  The reason is that pc-1.5 has never guaranteed any feature of the
  emulated PMU.
  
  Right, current behavior is buggy and we never guaranteed anything, but
  IMO we shouldn't break on purpose something that is working today.
 
 Even if it is to fix something else?

I believe so, because machine-types allow us to have both: we can fix
the new use-cases in new machine-types while keeping existing working
setups without regressions on the older machine-types.

-- 
Eduardo

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

2013-07-23 Thread Igor Mammedov 
On Mon, 22 Jul 2013 16:25:35 -0300
Eduardo Habkost ehabk...@redhat.com 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 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);
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;

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

2013-07-23 Thread 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 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;

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

2013-07-23 Thread Eduardo Habkost 
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

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

2013-07-23 Thread Eduardo Habkost 
On Tue, Jul 23, 2013 at 08:01:29AM +0200, Igor Mammedov wrote:
 On Mon, 22 Jul 2013 16:25:35 -0300
 Eduardo Habkost ehabk...@redhat.com 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 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);
 Could this hunk be implemented using compat props?
 That would spare us dealing with it later.

Oh, I forgot we already have the qdev_prop_set_globals_for_type() hack
that would allow us to use model-specific compat_props.

But I never expected to have a compat property that will get set on
_all_ machine-types (-cpu host will have pmu-passthrough=true on all
machine-types). Would it make sense to do it?

Normally on cases like this we would just set a property default on the
host-x86-cpu class. But we still don't have the per-CPU-model X86CPU
subclasses, so today the defaults are coded in the init functions.

 
   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

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

2013-07-23 Thread Paolo Bonzini 
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;

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

2013-07-23 Thread Eduardo Habkost 
On Tue, Jul 23, 2013 at 05:09:02PM +0200, Paolo Bonzini wrote:
 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.

Exactly. But then I don't understand your suggestion. We still need a
property to enable pasthrough behavior on old machine-types (not
perfect, but a best-effort way to try to keep compatibility), and I
named that option pmu-passthrough. How do you think we should name it?

 
 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)

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

2013-07-23 Thread Paolo Bonzini 
Il 23/07/2013 17:40, Eduardo Habkost ha scritto:
 On Tue, Jul 23, 2013 at 05:09:02PM +0200, Paolo Bonzini wrote:
 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.
 
 Exactly. But then I don't understand your suggestion. We still need a
 property to enable pasthrough behavior on old machine-types (not
 perfect, but a best-effort way to try to keep compatibility),

Do we?

We only need pmu=on---which right now is buggy on old machine types
because it will always passthrough.

Paolo

 and I
 named that option pmu-passthrough. How do you think we should name it?

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

2013-07-23 Thread Eduardo Habkost 
On Tue, Jul 23, 2013 at 06:23:08PM +0200, Paolo Bonzini wrote:
 Il 23/07/2013 17:40, Eduardo Habkost ha scritto:
  On Tue, Jul 23, 2013 at 05:09:02PM +0200, Paolo Bonzini wrote:
  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.
  
  Exactly. But then I don't understand your suggestion. We still need a
  property to enable pasthrough behavior on old machine-types (not
  perfect, but a best-effort way to try to keep compatibility),
 
 Do we?
 
 We only need pmu=on---which right now is buggy on old machine types
 because it will always passthrough.

I am not sure I understand what you are arguing for.

You agree that pmu=on needs to keep the buggy passthrough behavior on
pc-1.5 and older, right?

In that case, how do you suggest I let QEMU know that only pc-1.5 and
older should have the buggy behavior enabled when pmu=on? I understand
that compat_props is the appropriate place for that, and that's why I
need a please-enable-the-old-buggy-pmu-passthrough-behavior property
that I can add to PC_COMPAT_1_5.

 
 Paolo
 
  and I
  named that option pmu-passthrough. How do you think we should name it?
 

-- 
Eduardo

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

2013-07-23 Thread Paolo Bonzini 
Il 23/07/2013 19:41, Eduardo Habkost ha scritto:
 On Tue, Jul 23, 2013 at 06:23:08PM +0200, Paolo Bonzini wrote:
 Il 23/07/2013 17:40, Eduardo Habkost ha scritto:
 On Tue, Jul 23, 2013 at 05:09:02PM +0200, Paolo Bonzini wrote:
 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.

 Exactly. But then I don't understand your suggestion. We still need a
 property to enable pasthrough behavior on old machine-types (not
 perfect, but a best-effort way to try to keep compatibility),

 Do we?

 We only need pmu=on---which right now is buggy on old machine types
 because it will always passthrough.
 
 I am not sure I understand what you are arguing for.
 
 You agree that pmu=on needs to keep the buggy passthrough behavior on
 pc-1.5 and older, right?

I agree it needs to remain enabled on 1.5.  But if, for example, 1.8
makes pmu=on emulate a Nehalem-compatible PMU, I think it is fine if
pc-1.5 moves from a host-compatible PMU to a Nehalem-compatible PMU.

The reason is that pc-1.5 has never guaranteed any feature of the
emulated PMU.

Paolo