Re: [Qemu-devel] [PATCH v2] target-lm32: move model features to LM32CPU

2013-11-18 Thread Andreas Färber 
Am 17.11.2013 21:46, schrieb Michael Walle:
 Am 2013-10-14 23:46, schrieb Michael Walle:
 This allows us to completely remove CPULM32State from DisasContext.
 Instead, copy the fields we need to DisasContext.

 Cc: Andreas Färber afaer...@suse.de
 Signed-off-by: Michael Walle mich...@walle.cc
 ---

 changes since v1:
  - instead of storing a pointer to the cpu definitions, register
individual cpu types and store features in LM32CPU.
  - cpu_list() iterates over these types now.
 
 ping,
 
 andreas, could you please review this patch?

Sorry, didn't manage to before KVM Forum and forgot afterwards...

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] [PATCH v2] target-lm32: move model features to LM32CPU

2013-11-18 Thread Andreas Färber 
Am 15.10.2013 00:46, schrieb Michael Walle:
 This allows us to completely remove CPULM32State from DisasContext.
 Instead, copy the fields we need to DisasContext.
 
 Cc: Andreas Färber afaer...@suse.de
 Signed-off-by: Michael Walle mich...@walle.cc
 ---
 
 changes since v1:
  - instead of storing a pointer to the cpu definitions, register
individual cpu types and store features in LM32CPU.
  - cpu_list() iterates over these types now.
 
 
  target-lm32/cpu-qom.h   |5 ++
  target-lm32/cpu.c   |  187 
 ++-
  target-lm32/cpu.h   |7 +-
  target-lm32/helper.c|  128 +---
  target-lm32/translate.c |   29 +---
  5 files changed, 214 insertions(+), 142 deletions(-)
 
 diff --git a/target-lm32/cpu-qom.h b/target-lm32/cpu-qom.h
 index 723f604..3bf7956 100644
 --- a/target-lm32/cpu-qom.h
 +++ b/target-lm32/cpu-qom.h
 @@ -59,6 +59,11 @@ typedef struct LM32CPU {
  CPUState parent_obj;
  /* public */
  
 +uint32_t revision;
 +uint8_t num_interrupts;
 +uint8_t num_breakpoints;
 +uint8_t num_watchpoints;
 +uint32_t features;
  CPULM32State env;

For TCG performance reasons you should place the fields after env. In
that case please separate them from env with a white line.

  } LM32CPU;
  
 diff --git a/target-lm32/cpu.c b/target-lm32/cpu.c
 index 869878c..ae372b8 100644
 --- a/target-lm32/cpu.c
 +++ b/target-lm32/cpu.c
 @@ -29,6 +29,87 @@ static void lm32_cpu_set_pc(CPUState *cs, vaddr value)
  cpu-env.pc = value;
  }
  
 +/* Sort alphabetically by type name. */
 +static gint lm32_cpu_list_compare(gconstpointer a, gconstpointer b)
 +{
 +ObjectClass *class_a = (ObjectClass *)a;
 +ObjectClass *class_b = (ObjectClass *)b;
 +const char *name_a, *name_b;
 +
 +name_a = object_class_get_name(class_a);
 +name_b = object_class_get_name(class_b);
 +return strcmp(name_a, name_b);
 +}
 +
 +static void lm32_cpu_list_entry(gpointer data, gpointer user_data)
 +{
 +ObjectClass *oc = data;
 +CPUListState *s = user_data;
 +const char *typename = object_class_get_name(oc);
 +char *name;
 +
 +name = g_strndup(typename, strlen(typename) - strlen(- TYPE_LM32_CPU));
 +(*s-cpu_fprintf)(s-file,   %s\n, name);
 +g_free(name);
 +}
 +
 +
 +void lm32_cpu_list(FILE *f, fprintf_function cpu_fprintf)
 +{
 +CPUListState s = {
 +.file = f,
 +.cpu_fprintf = cpu_fprintf,
 +};
 +GSList *list;
 +
 +list = object_class_get_list(TYPE_LM32_CPU, false);
 +list = g_slist_sort(list, lm32_cpu_list_compare);
 +(*cpu_fprintf)(f, Available CPUs:\n);
 +g_slist_foreach(list, lm32_cpu_list_entry, s);
 +g_slist_free(list);
 +}
 +
 +static void init_cfg_reg(LM32CPU *cpu)

Optionally you could use a lm32_cpu_ prefix here for consistency.

 +{
 +CPULM32State *env = cpu-env;
 +uint32_t cfg = 0;
 +
 +if (cpu-features  LM32_FEATURE_MULTIPLY) {
 +cfg |= CFG_M;
 +}
 +
 +if (cpu-features  LM32_FEATURE_DIVIDE) {
 +cfg |= CFG_D;
 +}
 +
 +if (cpu-features  LM32_FEATURE_SHIFT) {
 +cfg |= CFG_S;
 +}
 +
 +if (cpu-features  LM32_FEATURE_SIGN_EXTEND) {
 +cfg |= CFG_X;
 +}
 +
 +if (cpu-features  LM32_FEATURE_I_CACHE) {
 +cfg |= CFG_IC;
 +}
 +
 +if (cpu-features  LM32_FEATURE_D_CACHE) {
 +cfg |= CFG_DC;
 +}
 +
 +if (cpu-features  LM32_FEATURE_CYCLE_COUNT) {
 +cfg |= CFG_CC;
 +}
 +
 +cfg |= (cpu-num_interrupts  CFG_INT_SHIFT);
 +cfg |= (cpu-num_breakpoints  CFG_BP_SHIFT);
 +cfg |= (cpu-num_watchpoints  CFG_WP_SHIFT);
 +cfg |= (cpu-revision  CFG_REV_SHIFT);
 +
 +env-cfg = cfg;
 +}
 +
  /* CPUClass::reset() */
  static void lm32_cpu_reset(CPUState *s)
  {
 @@ -41,6 +122,7 @@ static void lm32_cpu_reset(CPUState *s)
  /* reset cpu state */
  memset(env, 0, offsetof(CPULM32State, breakpoints));
  
 +init_cfg_reg(cpu);
  tlb_flush(env, 1);
  }
  
 @@ -74,6 +156,91 @@ static void lm32_cpu_initfn(Object *obj)
  }
  }
  
 +static void lm32_basic_cpu_initfn(Object *obj)
 +{
 +LM32CPU *cpu = LM32_CPU(obj);
 +
 +cpu-revision = 3;
 +cpu-num_interrupts = 32;
 +cpu-num_breakpoints = 4;
 +cpu-num_watchpoints = 4;
 +cpu-features = LM32_FEATURE_SHIFT
 +| LM32_FEATURE_SIGN_EXTEND
 +| LM32_FEATURE_CYCLE_COUNT;

Out of a personal style preference I would align the LM32_FEATURE_
prefix. Either by placing the | last or by aligning | with =. But just a
suggestion, it was already this way before.

Other than that looks good, thanks, so once you fix the env issue, feel
free to add my Reviewed-by. Sorry for the delay in reviewing changes I
suggested.

Regards,
Andreas

 +}
 +
 +static void lm32_standard_cpu_initfn(Object *obj)
 +{
 +LM32CPU *cpu = LM32_CPU(obj);
 +
 +cpu-revision = 3;
 +cpu-num_interrupts = 32;
 +cpu-num_breakpoints = 4;
 +

Re: [Qemu-devel] [PATCH v2] target-lm32: move model features to LM32CPU

2013-11-17 Thread Michael Walle 

Am 2013-10-14 23:46, schrieb Michael Walle:

This allows us to completely remove CPULM32State from DisasContext.
Instead, copy the fields we need to DisasContext.

Cc: Andreas Färber afaer...@suse.de
Signed-off-by: Michael Walle mich...@walle.cc
---

changes since v1:
 - instead of storing a pointer to the cpu definitions, register
   individual cpu types and store features in LM32CPU.
 - cpu_list() iterates over these types now.


ping,

andreas, could you please review this patch?

--
michael

[Qemu-devel] [PATCH v2] target-lm32: move model features to LM32CPU

2013-10-14 Thread Michael Walle 
This allows us to completely remove CPULM32State from DisasContext.
Instead, copy the fields we need to DisasContext.

Cc: Andreas Färber afaer...@suse.de
Signed-off-by: Michael Walle mich...@walle.cc
---

changes since v1:
 - instead of storing a pointer to the cpu definitions, register
   individual cpu types and store features in LM32CPU.
 - cpu_list() iterates over these types now.


 target-lm32/cpu-qom.h   |5 ++
 target-lm32/cpu.c   |  187 ++-
 target-lm32/cpu.h   |7 +-
 target-lm32/helper.c|  128 +---
 target-lm32/translate.c |   29 +---
 5 files changed, 214 insertions(+), 142 deletions(-)

diff --git a/target-lm32/cpu-qom.h b/target-lm32/cpu-qom.h
index 723f604..3bf7956 100644
--- a/target-lm32/cpu-qom.h
+++ b/target-lm32/cpu-qom.h
@@ -59,6 +59,11 @@ typedef struct LM32CPU {
 CPUState parent_obj;
 /* public */
 
+uint32_t revision;
+uint8_t num_interrupts;
+uint8_t num_breakpoints;
+uint8_t num_watchpoints;
+uint32_t features;
 CPULM32State env;
 } LM32CPU;
 
diff --git a/target-lm32/cpu.c b/target-lm32/cpu.c
index 869878c..ae372b8 100644
--- a/target-lm32/cpu.c
+++ b/target-lm32/cpu.c
@@ -29,6 +29,87 @@ static void lm32_cpu_set_pc(CPUState *cs, vaddr value)
 cpu-env.pc = value;
 }
 
+/* Sort alphabetically by type name. */
+static gint lm32_cpu_list_compare(gconstpointer a, gconstpointer b)
+{
+ObjectClass *class_a = (ObjectClass *)a;
+ObjectClass *class_b = (ObjectClass *)b;
+const char *name_a, *name_b;
+
+name_a = object_class_get_name(class_a);
+name_b = object_class_get_name(class_b);
+return strcmp(name_a, name_b);
+}
+
+static void lm32_cpu_list_entry(gpointer data, gpointer user_data)
+{
+ObjectClass *oc = data;
+CPUListState *s = user_data;
+const char *typename = object_class_get_name(oc);
+char *name;
+
+name = g_strndup(typename, strlen(typename) - strlen(- TYPE_LM32_CPU));
+(*s-cpu_fprintf)(s-file,   %s\n, name);
+g_free(name);
+}
+
+
+void lm32_cpu_list(FILE *f, fprintf_function cpu_fprintf)
+{
+CPUListState s = {
+.file = f,
+.cpu_fprintf = cpu_fprintf,
+};
+GSList *list;
+
+list = object_class_get_list(TYPE_LM32_CPU, false);
+list = g_slist_sort(list, lm32_cpu_list_compare);
+(*cpu_fprintf)(f, Available CPUs:\n);
+g_slist_foreach(list, lm32_cpu_list_entry, s);
+g_slist_free(list);
+}
+
+static void init_cfg_reg(LM32CPU *cpu)
+{
+CPULM32State *env = cpu-env;
+uint32_t cfg = 0;
+
+if (cpu-features  LM32_FEATURE_MULTIPLY) {
+cfg |= CFG_M;
+}
+
+if (cpu-features  LM32_FEATURE_DIVIDE) {
+cfg |= CFG_D;
+}
+
+if (cpu-features  LM32_FEATURE_SHIFT) {
+cfg |= CFG_S;
+}
+
+if (cpu-features  LM32_FEATURE_SIGN_EXTEND) {
+cfg |= CFG_X;
+}
+
+if (cpu-features  LM32_FEATURE_I_CACHE) {
+cfg |= CFG_IC;
+}
+
+if (cpu-features  LM32_FEATURE_D_CACHE) {
+cfg |= CFG_DC;
+}
+
+if (cpu-features  LM32_FEATURE_CYCLE_COUNT) {
+cfg |= CFG_CC;
+}
+
+cfg |= (cpu-num_interrupts  CFG_INT_SHIFT);
+cfg |= (cpu-num_breakpoints  CFG_BP_SHIFT);
+cfg |= (cpu-num_watchpoints  CFG_WP_SHIFT);
+cfg |= (cpu-revision  CFG_REV_SHIFT);
+
+env-cfg = cfg;
+}
+
 /* CPUClass::reset() */
 static void lm32_cpu_reset(CPUState *s)
 {
@@ -41,6 +122,7 @@ static void lm32_cpu_reset(CPUState *s)
 /* reset cpu state */
 memset(env, 0, offsetof(CPULM32State, breakpoints));
 
+init_cfg_reg(cpu);
 tlb_flush(env, 1);
 }
 
@@ -74,6 +156,91 @@ static void lm32_cpu_initfn(Object *obj)
 }
 }
 
+static void lm32_basic_cpu_initfn(Object *obj)
+{
+LM32CPU *cpu = LM32_CPU(obj);
+
+cpu-revision = 3;
+cpu-num_interrupts = 32;
+cpu-num_breakpoints = 4;
+cpu-num_watchpoints = 4;
+cpu-features = LM32_FEATURE_SHIFT
+| LM32_FEATURE_SIGN_EXTEND
+| LM32_FEATURE_CYCLE_COUNT;
+}
+
+static void lm32_standard_cpu_initfn(Object *obj)
+{
+LM32CPU *cpu = LM32_CPU(obj);
+
+cpu-revision = 3;
+cpu-num_interrupts = 32;
+cpu-num_breakpoints = 4;
+cpu-num_watchpoints = 4;
+cpu-features = LM32_FEATURE_MULTIPLY
+| LM32_FEATURE_DIVIDE
+| LM32_FEATURE_SHIFT
+| LM32_FEATURE_SIGN_EXTEND
+| LM32_FEATURE_I_CACHE
+| LM32_FEATURE_CYCLE_COUNT;
+}
+
+static void lm32_full_cpu_initfn(Object *obj)
+{
+LM32CPU *cpu = LM32_CPU(obj);
+
+cpu-revision = 3;
+cpu-num_interrupts = 32;
+cpu-num_breakpoints = 4;
+cpu-num_watchpoints = 4;
+cpu-features = LM32_FEATURE_MULTIPLY
+| LM32_FEATURE_DIVIDE
+| LM32_FEATURE_SHIFT
+| LM32_FEATURE_SIGN_EXTEND
+| LM32_FEATURE_I_CACHE
+| LM32_FEATURE_D_CACHE
+