Re: [Qemu-devel] [uq/master PATCH 7/7 v8] target-i386: CPU model subclasses

2014-02-10 Thread Eduardo Habkost 
On Mon, Feb 10, 2014 at 01:23:37AM +0100, Andreas Färber wrote:
[...]
   /**
* X86CPUClass:
* @parent_realize: The parent class' realize handler.
  @@ -49,6 +52,16 @@ typedef struct X86CPUClass {
   CPUClass parent_class;
   /* public */
   
  +/* CPU model definition
  + * Should be eventually replaced by subclass-specific property defaults
  + */
  +X86CPUDefinition *cpu_def;
  +/* CPU model requires KVM to be enabled */
  +bool kvm_required;
  +/* Optional description of CPU model.
  + * If unavailable, cpu_def-model_id is used */
  +const char *model_description;
 
 Here I wondered why you needed this? For PowerPCCPU subclasses we have
 reused DeviceClass::desc.

I was not aware of DeviceClass::desc. We can use it instead.

Do you prefer a respin, or an additional patch?

-- 
Eduardo
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [Qemu-devel] [uq/master PATCH 7/7 v8] target-i386: CPU model subclasses

2014-02-10 Thread Eduardo Habkost 
On Mon, Feb 10, 2014 at 06:19:58AM -0200, Eduardo Habkost wrote:
 On Mon, Feb 10, 2014 at 01:23:37AM +0100, Andreas Färber wrote:
 [...]
/**
 * X86CPUClass:
 * @parent_realize: The parent class' realize handler.
   @@ -49,6 +52,16 @@ typedef struct X86CPUClass {
CPUClass parent_class;
/* public */

   +/* CPU model definition
   + * Should be eventually replaced by subclass-specific property 
   defaults
   + */
   +X86CPUDefinition *cpu_def;
   +/* CPU model requires KVM to be enabled */
   +bool kvm_required;
   +/* Optional description of CPU model.
   + * If unavailable, cpu_def-model_id is used */
   +const char *model_description;
  
  Here I wondered why you needed this? For PowerPCCPU subclasses we have
  reused DeviceClass::desc.
 
 I was not aware of DeviceClass::desc. We can use it instead.
 
 Do you prefer a respin, or an additional patch?
 

Actually we don't even need model_description or DeviceClass::desc yet,
because the code to list CPU models using
object_class_get_list(TYPE_X86_CPU) was moved to a separate patch I will
send later.

I will send a new version of this patch without model_description, then
change the new CPU model listing code to use DeviceClass::desc.

-- 
Eduardo
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [Qemu-devel] [uq/master PATCH 7/7 v8] target-i386: CPU model subclasses

2014-02-10 Thread Igor Mammedov 
On Mon, 10 Feb 2014 01:23:37 +0100
Andreas Färber afaer...@suse.de wrote:

 Am 31.01.2014 19:13, schrieb Eduardo Habkost:
  Register separate QOM classes for each x86 CPU model.
  
  This will allow management code to more easily probe what each CPU model
  provides, by simply creating objects using the appropriate class name,
  without having to restart QEMU.
  
  This also allows us to eliminate the qdev_prop_set_globals_for_type()
  hack to set CPU-model-specific global properties.
  
  Instead of creating separate class_init functions for each class, I just
  used class_data to store a pointer to the X86CPUDefinition struct for
  each CPU model. This should make the patch shorter and easier to review.
  Later we can gradually convert each X86CPUDefinition field to lists of
  per-class property defaults.
  
  Written based on the ideas from the patch [RFC v5] target-i386: Slim
  conversion to X86CPU subclasses + KVM subclasses written by Andreas
  Färber afaer...@suse.de, Igor Mammedov imamm...@redhat.com.
  
  The host CPU model is special, as the feature flags depend on KVM
  being initialized. So it has its own class_init and instance_init
  function, and feature flags are set on instance_init instead of
  class_init.
  
  Signed-off-by: Andreas Färber afaer...@suse.de
  Signed-off-by: Igor Mammedov imamm...@redhat.com
  Signed-off-by: Eduardo Habkost ehabk...@redhat.com
  ---
  This patch is similar to the one sent by Andrea and then later
  resubmitted by Igor as [RFC v5] target-i386: Slim conversion to X86CPU
  subclasses + KVM subclasses, as it doesn't create one new class_init
  function for each subclass.
  
  Main differences v5 - v6 are:
   * Code was written from scratch (instead of using the previous patches
 as base)
 * I didn't mean to rewrite it entirely, but when doing additional
   simplification of the CPU init logic on other patches, I ended up
   rewriting it.
 * I chose to keep the Signed-off-by lines because I built upon
   Andreas's and Igor's ideas. Is that OK?
 
 Yes, your From and our Sobs in order is the expected way in this case.
 If Igor agrees I would propose to drop the textual repetition of this.
I'm ok with it, but it doesn't matter since  this part is under ---, so
it's dropped at commit time anyway.
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [Qemu-devel] [uq/master PATCH 7/7 v8] target-i386: CPU model subclasses

2014-02-09 Thread Andreas Färber 
Am 31.01.2014 19:13, schrieb Eduardo Habkost:
 Register separate QOM classes for each x86 CPU model.
 
 This will allow management code to more easily probe what each CPU model
 provides, by simply creating objects using the appropriate class name,
 without having to restart QEMU.
 
 This also allows us to eliminate the qdev_prop_set_globals_for_type()
 hack to set CPU-model-specific global properties.
 
 Instead of creating separate class_init functions for each class, I just
 used class_data to store a pointer to the X86CPUDefinition struct for
 each CPU model. This should make the patch shorter and easier to review.
 Later we can gradually convert each X86CPUDefinition field to lists of
 per-class property defaults.
 
 Written based on the ideas from the patch [RFC v5] target-i386: Slim
 conversion to X86CPU subclasses + KVM subclasses written by Andreas
 Färber afaer...@suse.de, Igor Mammedov imamm...@redhat.com.
 
 The host CPU model is special, as the feature flags depend on KVM
 being initialized. So it has its own class_init and instance_init
 function, and feature flags are set on instance_init instead of
 class_init.
 
 Signed-off-by: Andreas Färber afaer...@suse.de
 Signed-off-by: Igor Mammedov imamm...@redhat.com
 Signed-off-by: Eduardo Habkost ehabk...@redhat.com
 ---
 This patch is similar to the one sent by Andrea and then later
 resubmitted by Igor as [RFC v5] target-i386: Slim conversion to X86CPU
 subclasses + KVM subclasses, as it doesn't create one new class_init
 function for each subclass.
 
 Main differences v5 - v6 are:
  * Code was written from scratch (instead of using the previous patches
as base)
* I didn't mean to rewrite it entirely, but when doing additional
  simplification of the CPU init logic on other patches, I ended up
  rewriting it.
* I chose to keep the Signed-off-by lines because I built upon
  Andreas's and Igor's ideas. Is that OK?

Yes, your From and our Sobs in order is the expected way in this case.
If Igor agrees I would propose to drop the textual repetition of this.

I am ~1/3 through reviewing this and it looks pretty promising so far!
Thanks a lot for your efforts. Meanwhile one cleanup idea inline...

  * No KVM-specific subclasses, to keep things simpler.
  * No embedding of X86CPUDefinition (x86_def_t) inside the class struct,
instead keeping a pointer to the existing X86CPUDefinition struct.
  * The host class is registered on cpu.c, but the CPUID data
is filled on instance_init instead of class_init (because KVM has to
be initialized already).
* kvm_required field introduced to make sure the host class can't
  be used without KVM.
 
 Changes v6 - v7:
  * Rebase
 
 Changes v7 - v8:
  * Removed CPU listing code (will be sent as a separate patch)
  * Kept x86_cpudef_setup() (will be addressed in a separate patch)
 ---
  target-i386/cpu-qom.h |  13 
  target-i386/cpu.c | 197 
 --
  2 files changed, 138 insertions(+), 72 deletions(-)
 
 diff --git a/target-i386/cpu-qom.h b/target-i386/cpu-qom.h
 index 722f11a..60c5c32 100644
 --- a/target-i386/cpu-qom.h
 +++ b/target-i386/cpu-qom.h
 @@ -37,6 +37,9 @@
  #define X86_CPU_GET_CLASS(obj) \
  OBJECT_GET_CLASS(X86CPUClass, (obj), TYPE_X86_CPU)
  
 +
 +typedef struct X86CPUDefinition X86CPUDefinition;
 +
  /**
   * X86CPUClass:
   * @parent_realize: The parent class' realize handler.
 @@ -49,6 +52,16 @@ typedef struct X86CPUClass {
  CPUClass parent_class;
  /* public */
  
 +/* CPU model definition
 + * Should be eventually replaced by subclass-specific property defaults
 + */
 +X86CPUDefinition *cpu_def;
 +/* CPU model requires KVM to be enabled */
 +bool kvm_required;
 +/* Optional description of CPU model.
 + * If unavailable, cpu_def-model_id is used */
 +const char *model_description;

Here I wondered why you needed this? For PowerPCCPU subclasses we have
reused DeviceClass::desc.

Regards,
Andreas

 +
  DeviceRealize parent_realize;
  void (*parent_reset)(CPUState *cpu);
  } X86CPUClass;
[snip]

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[uq/master PATCH 7/7 v8] target-i386: CPU model subclasses

2014-01-31 Thread Eduardo Habkost 
Register separate QOM classes for each x86 CPU model.

This will allow management code to more easily probe what each CPU model
provides, by simply creating objects using the appropriate class name,
without having to restart QEMU.

This also allows us to eliminate the qdev_prop_set_globals_for_type()
hack to set CPU-model-specific global properties.

Instead of creating separate class_init functions for each class, I just
used class_data to store a pointer to the X86CPUDefinition struct for
each CPU model. This should make the patch shorter and easier to review.
Later we can gradually convert each X86CPUDefinition field to lists of
per-class property defaults.

Written based on the ideas from the patch [RFC v5] target-i386: Slim
conversion to X86CPU subclasses + KVM subclasses written by Andreas
Färber afaer...@suse.de, Igor Mammedov imamm...@redhat.com.

The host CPU model is special, as the feature flags depend on KVM
being initialized. So it has its own class_init and instance_init
function, and feature flags are set on instance_init instead of
class_init.

Signed-off-by: Andreas Färber afaer...@suse.de
Signed-off-by: Igor Mammedov imamm...@redhat.com
Signed-off-by: Eduardo Habkost ehabk...@redhat.com
---
This patch is similar to the one sent by Andrea and then later
resubmitted by Igor as [RFC v5] target-i386: Slim conversion to X86CPU
subclasses + KVM subclasses, as it doesn't create one new class_init
function for each subclass.

Main differences v5 - v6 are:
 * Code was written from scratch (instead of using the previous patches
   as base)
   * I didn't mean to rewrite it entirely, but when doing additional
 simplification of the CPU init logic on other patches, I ended up
 rewriting it.
   * I chose to keep the Signed-off-by lines because I built upon
 Andreas's and Igor's ideas. Is that OK?
 * No KVM-specific subclasses, to keep things simpler.
 * No embedding of X86CPUDefinition (x86_def_t) inside the class struct,
   instead keeping a pointer to the existing X86CPUDefinition struct.
 * The host class is registered on cpu.c, but the CPUID data
   is filled on instance_init instead of class_init (because KVM has to
   be initialized already).
   * kvm_required field introduced to make sure the host class can't
 be used without KVM.

Changes v6 - v7:
 * Rebase

Changes v7 - v8:
 * Removed CPU listing code (will be sent as a separate patch)
 * Kept x86_cpudef_setup() (will be addressed in a separate patch)
---
 target-i386/cpu-qom.h |  13 
 target-i386/cpu.c | 197 --
 2 files changed, 138 insertions(+), 72 deletions(-)

diff --git a/target-i386/cpu-qom.h b/target-i386/cpu-qom.h
index 722f11a..60c5c32 100644
--- a/target-i386/cpu-qom.h
+++ b/target-i386/cpu-qom.h
@@ -37,6 +37,9 @@
 #define X86_CPU_GET_CLASS(obj) \
 OBJECT_GET_CLASS(X86CPUClass, (obj), TYPE_X86_CPU)
 
+
+typedef struct X86CPUDefinition X86CPUDefinition;
+
 /**
  * X86CPUClass:
  * @parent_realize: The parent class' realize handler.
@@ -49,6 +52,16 @@ typedef struct X86CPUClass {
 CPUClass parent_class;
 /* public */
 
+/* CPU model definition
+ * Should be eventually replaced by subclass-specific property defaults
+ */
+X86CPUDefinition *cpu_def;
+/* CPU model requires KVM to be enabled */
+bool kvm_required;
+/* Optional description of CPU model.
+ * If unavailable, cpu_def-model_id is used */
+const char *model_description;
+
 DeviceRealize parent_realize;
 void (*parent_reset)(CPUState *cpu);
 } X86CPUClass;
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 6659527..bb72e5b 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -484,7 +484,10 @@ static void add_flagname_to_bitmaps(const char *flagname,
 }
 }
 
-typedef struct X86CPUDefinition {
+/* CPU model definition data that was not converted to QOM per-subclass
+ * property defaults yet.
+ */
+struct X86CPUDefinition {
 const char *name;
 uint32_t level;
 uint32_t xlevel;
@@ -497,7 +500,7 @@ typedef struct X86CPUDefinition {
 FeatureWordArray features;
 char model_id[48];
 bool cache_info_passthrough;
-} X86CPUDefinition;
+};
 
 #define I486_FEATURES (CPUID_FP87 | CPUID_VME | CPUID_PSE)
 #define PENTIUM_FEATURES (I486_FEATURES | CPUID_DE | CPUID_TSC | \
@@ -547,8 +550,29 @@ typedef struct X86CPUDefinition {
   CPUID_7_0_EBX_ERMS, CPUID_7_0_EBX_INVPCID, CPUID_7_0_EBX_RTM,
   CPUID_7_0_EBX_RDSEED */
 
-/* built-in CPU model definitions
+/* CPU class name definitions: */
+
+#define X86_CPU_CLASS_SUFFIX - TYPE_X86_CPU
+#define CPU_CLASS_NAME(name) (name X86_CPU_CLASS_SUFFIX)
+
+/* Return class name for a given CPU model name
+ * Caller is responsible for freeing the returned string.
  */
+static char *x86_cpu_class_name(const char *model_name)
+{
+return g_strdup_printf(CPU_CLASS_NAME(%s), model_name);
+}
+
+/* Return X86CPUClass for a CPU model name */
+static X86CPUClass