From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:41856) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ViQM5-0005wi-M4 for qemu-devel@nongnu.org; Mon, 18 Nov 2013 10:03:18 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ViQM0-0000O7-Gq for qemu-devel@nongnu.org; Mon, 18 Nov 2013 10:03:13 -0500 Received: from cantor2.suse.de ([195.135.220.15]:60977 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ViQLz-0000Mq-SU for qemu-devel@nongnu.org; Mon, 18 Nov 2013 10:03:08 -0500 Message-ID: <528A2C27.6070200@suse.de> Date: Mon, 18 Nov 2013 16:03:03 +0100 From: =?UTF-8?B?QW5kcmVhcyBGw6RyYmVy?= MIME-Version: 1.0 References: <525C26EA.8010909@suse.de> <1381790819-2740-1-git-send-email-michael@walle.cc> In-Reply-To: <1381790819-2740-1-git-send-email-michael@walle.cc> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v2] target-lm32: move model features to LM32CPU List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Michael Walle , qemu-devel@nongnu.org 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. >=20 > Cc: Andreas F=C3=A4rber > Signed-off-by: Michael Walle > --- >=20 > 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. >=20 >=20 > 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(-) >=20 > 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 >*/ > =20 > + 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; > =20 > 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 valu= e) > cpu->env.pc =3D value; > } > =20 > +/* Sort alphabetically by type name. */ > +static gint lm32_cpu_list_compare(gconstpointer a, gconstpointer b) > +{ > + ObjectClass *class_a =3D (ObjectClass *)a; > + ObjectClass *class_b =3D (ObjectClass *)b; > + const char *name_a, *name_b; > + > + name_a =3D object_class_get_name(class_a); > + name_b =3D 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 =3D data; > + CPUListState *s =3D user_data; > + const char *typename =3D object_class_get_name(oc); > + char *name; > + > + name =3D g_strndup(typename, strlen(typename) - strlen("-" TYPE_LM= 32_CPU)); > + (*s->cpu_fprintf)(s->file, " %s\n", name); > + g_free(name); > +} > + > + > +void lm32_cpu_list(FILE *f, fprintf_function cpu_fprintf) > +{ > + CPUListState s =3D { > + .file =3D f, > + .cpu_fprintf =3D cpu_fprintf, > + }; > + GSList *list; > + > + list =3D object_class_get_list(TYPE_LM32_CPU, false); > + list =3D 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 =3D &cpu->env; > + uint32_t cfg =3D 0; > + > + if (cpu->features & LM32_FEATURE_MULTIPLY) { > + cfg |=3D CFG_M; > + } > + > + if (cpu->features & LM32_FEATURE_DIVIDE) { > + cfg |=3D CFG_D; > + } > + > + if (cpu->features & LM32_FEATURE_SHIFT) { > + cfg |=3D CFG_S; > + } > + > + if (cpu->features & LM32_FEATURE_SIGN_EXTEND) { > + cfg |=3D CFG_X; > + } > + > + if (cpu->features & LM32_FEATURE_I_CACHE) { > + cfg |=3D CFG_IC; > + } > + > + if (cpu->features & LM32_FEATURE_D_CACHE) { > + cfg |=3D CFG_DC; > + } > + > + if (cpu->features & LM32_FEATURE_CYCLE_COUNT) { > + cfg |=3D CFG_CC; > + } > + > + cfg |=3D (cpu->num_interrupts << CFG_INT_SHIFT); > + cfg |=3D (cpu->num_breakpoints << CFG_BP_SHIFT); > + cfg |=3D (cpu->num_watchpoints << CFG_WP_SHIFT); > + cfg |=3D (cpu->revision << CFG_REV_SHIFT); > + > + env->cfg =3D 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)); > =20 > + init_cfg_reg(cpu); > tlb_flush(env, 1); > } > =20 > @@ -74,6 +156,91 @@ static void lm32_cpu_initfn(Object *obj) > } > } > =20 > +static void lm32_basic_cpu_initfn(Object *obj) > +{ > + LM32CPU *cpu =3D LM32_CPU(obj); > + > + cpu->revision =3D 3; > + cpu->num_interrupts =3D 32; > + cpu->num_breakpoints =3D 4; > + cpu->num_watchpoints =3D 4; > + cpu->features =3D 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 =3D. 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 =3D LM32_CPU(obj); > + > + cpu->revision =3D 3; > + cpu->num_interrupts =3D 32; > + cpu->num_breakpoints =3D 4; > + cpu->num_watchpoints =3D 4; > + cpu->features =3D 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 =3D LM32_CPU(obj); > + > + cpu->revision =3D 3; > + cpu->num_interrupts =3D 32; > + cpu->num_breakpoints =3D 4; > + cpu->num_watchpoints =3D 4; > + cpu->features =3D LM32_FEATURE_MULTIPLY > + | LM32_FEATURE_DIVIDE > + | LM32_FEATURE_SHIFT > + | LM32_FEATURE_SIGN_EXTEND > + | LM32_FEATURE_I_CACHE > + | LM32_FEATURE_D_CACHE > + | LM32_FEATURE_CYCLE_COUNT; > +} > + > +typedef struct LM32CPUInfo { > + const char *name; > + void (*initfn)(Object *obj); > +} LM32CPUInfo; > + > +static const LM32CPUInfo lm32_cpus[] =3D { > + { > + .name =3D "lm32-basic", > + .initfn =3D lm32_basic_cpu_initfn, > + }, > + { > + .name =3D "lm32-standard", > + .initfn =3D lm32_standard_cpu_initfn, > + }, > + { > + .name =3D "lm32-full", > + .initfn =3D lm32_full_cpu_initfn, > + }, > +}; > + > +static ObjectClass *lm32_cpu_class_by_name(const char *cpu_model) > +{ > + ObjectClass *oc; > + char *typename; > + > + if (cpu_model =3D=3D NULL) { > + return NULL; > + } > + > + typename =3D g_strdup_printf("%s-" TYPE_LM32_CPU, cpu_model); > + oc =3D object_class_by_name(typename); > + g_free(typename); > + if (oc !=3D NULL && (!object_class_dynamic_cast(oc, TYPE_LM32_CPU)= || > + object_class_is_abstract(oc))) { > + oc =3D NULL; > + } > + return oc; > +} > + > static void lm32_cpu_class_init(ObjectClass *oc, void *data) > { > LM32CPUClass *lcc =3D LM32_CPU_CLASS(oc); > @@ -86,6 +253,7 @@ static void lm32_cpu_class_init(ObjectClass *oc, voi= d *data) > lcc->parent_reset =3D cc->reset; > cc->reset =3D lm32_cpu_reset; > =20 > + cc->class_by_name =3D lm32_cpu_class_by_name; > cc->do_interrupt =3D lm32_cpu_do_interrupt; > cc->dump_state =3D lm32_cpu_dump_state; > cc->set_pc =3D lm32_cpu_set_pc; > @@ -98,19 +266,36 @@ static void lm32_cpu_class_init(ObjectClass *oc, v= oid *data) > cc->gdb_num_core_regs =3D 32 + 7; > } > =20 > +static void lm32_register_cpu_type(const LM32CPUInfo *info) > +{ > + TypeInfo type_info =3D { > + .parent =3D TYPE_LM32_CPU, > + .instance_init =3D info->initfn, > + }; > + > + type_info.name =3D g_strdup_printf("%s-" TYPE_LM32_CPU, info->name= ); > + type_register(&type_info); > + g_free((void *)type_info.name); > +} > + > static const TypeInfo lm32_cpu_type_info =3D { > .name =3D TYPE_LM32_CPU, > .parent =3D TYPE_CPU, > .instance_size =3D sizeof(LM32CPU), > .instance_init =3D lm32_cpu_initfn, > - .abstract =3D false, > + .abstract =3D true, > .class_size =3D sizeof(LM32CPUClass), > .class_init =3D lm32_cpu_class_init, > }; > =20 > static void lm32_cpu_register_types(void) > { > + int i; > + > type_register_static(&lm32_cpu_type_info); > + for (i =3D 0; i < ARRAY_SIZE(lm32_cpus); i++) { > + lm32_register_cpu_type(&lm32_cpus[i]); > + } > } > =20 > type_init(lm32_cpu_register_types) > diff --git a/target-lm32/cpu.h b/target-lm32/cpu.h > index dbfe043..101df80 100644 > --- a/target-lm32/cpu.h > +++ b/target-lm32/cpu.h > @@ -177,23 +177,20 @@ struct CPULM32State { > DeviceState *juart_state; > =20 > /* processor core features */ > - uint32_t features; > uint32_t flags; > - uint8_t num_bps; > - uint8_t num_wps; > =20 > }; > =20 > #include "cpu-qom.h" > =20 > LM32CPU *cpu_lm32_init(const char *cpu_model); > -void cpu_lm32_list(FILE *f, fprintf_function cpu_fprintf); > int cpu_lm32_exec(CPULM32State *s); > /* you can call this signal handler from your SIGBUS and SIGSEGV > signal handlers to inform the virtual CPU of exceptions. non zero > is returned if the signal was handled by the virtual CPU. */ > int cpu_lm32_signal_handler(int host_signum, void *pinfo, > void *puc); > +void lm32_cpu_list(FILE *f, fprintf_function cpu_fprintf); > void lm32_translate_init(void); > void cpu_lm32_set_phys_msb_ignore(CPULM32State *env, int value); > =20 > @@ -206,7 +203,7 @@ static inline CPULM32State *cpu_init(const char *cp= u_model) > return &cpu->env; > } > =20 > -#define cpu_list cpu_lm32_list > +#define cpu_list lm32_cpu_list > #define cpu_exec cpu_lm32_exec > #define cpu_gen_code cpu_lm32_gen_code > #define cpu_signal_handler cpu_lm32_signal_handler > diff --git a/target-lm32/helper.c b/target-lm32/helper.c > index 15bc615..f85ff2e 100644 > --- a/target-lm32/helper.c > +++ b/target-lm32/helper.c > @@ -90,136 +90,16 @@ void lm32_cpu_do_interrupt(CPUState *cs) > } > } > =20 > -typedef struct { > - const char *name; > - uint32_t revision; > - uint8_t num_interrupts; > - uint8_t num_breakpoints; > - uint8_t num_watchpoints; > - uint32_t features; > -} LM32Def; > - > -static const LM32Def lm32_defs[] =3D { > - { > - .name =3D "lm32-basic", > - .revision =3D 3, > - .num_interrupts =3D 32, > - .num_breakpoints =3D 4, > - .num_watchpoints =3D 4, > - .features =3D (LM32_FEATURE_SHIFT > - | LM32_FEATURE_SIGN_EXTEND > - | LM32_FEATURE_CYCLE_COUNT), > - }, > - { > - .name =3D "lm32-standard", > - .revision =3D 3, > - .num_interrupts =3D 32, > - .num_breakpoints =3D 4, > - .num_watchpoints =3D 4, > - .features =3D (LM32_FEATURE_MULTIPLY > - | LM32_FEATURE_DIVIDE > - | LM32_FEATURE_SHIFT > - | LM32_FEATURE_SIGN_EXTEND > - | LM32_FEATURE_I_CACHE > - | LM32_FEATURE_CYCLE_COUNT), > - }, > - { > - .name =3D "lm32-full", > - .revision =3D 3, > - .num_interrupts =3D 32, > - .num_breakpoints =3D 4, > - .num_watchpoints =3D 4, > - .features =3D (LM32_FEATURE_MULTIPLY > - | LM32_FEATURE_DIVIDE > - | LM32_FEATURE_SHIFT > - | LM32_FEATURE_SIGN_EXTEND > - | LM32_FEATURE_I_CACHE > - | LM32_FEATURE_D_CACHE > - | LM32_FEATURE_CYCLE_COUNT), > - } > -}; > - > -void cpu_lm32_list(FILE *f, fprintf_function cpu_fprintf) > -{ > - int i; > - > - cpu_fprintf(f, "Available CPUs:\n"); > - for (i =3D 0; i < ARRAY_SIZE(lm32_defs); i++) { > - cpu_fprintf(f, " %s\n", lm32_defs[i].name); > - } > -} > - > -static const LM32Def *cpu_lm32_find_by_name(const char *name) > -{ > - int i; > - > - for (i =3D 0; i < ARRAY_SIZE(lm32_defs); i++) { > - if (strcasecmp(name, lm32_defs[i].name) =3D=3D 0) { > - return &lm32_defs[i]; > - } > - } > - > - return NULL; > -} > - > -static uint32_t cfg_by_def(const LM32Def *def) > -{ > - uint32_t cfg =3D 0; > - > - if (def->features & LM32_FEATURE_MULTIPLY) { > - cfg |=3D CFG_M; > - } > - > - if (def->features & LM32_FEATURE_DIVIDE) { > - cfg |=3D CFG_D; > - } > - > - if (def->features & LM32_FEATURE_SHIFT) { > - cfg |=3D CFG_S; > - } > - > - if (def->features & LM32_FEATURE_SIGN_EXTEND) { > - cfg |=3D CFG_X; > - } > - > - if (def->features & LM32_FEATURE_I_CACHE) { > - cfg |=3D CFG_IC; > - } > - > - if (def->features & LM32_FEATURE_D_CACHE) { > - cfg |=3D CFG_DC; > - } > - > - if (def->features & LM32_FEATURE_CYCLE_COUNT) { > - cfg |=3D CFG_CC; > - } > - > - cfg |=3D (def->num_interrupts << CFG_INT_SHIFT); > - cfg |=3D (def->num_breakpoints << CFG_BP_SHIFT); > - cfg |=3D (def->num_watchpoints << CFG_WP_SHIFT); > - cfg |=3D (def->revision << CFG_REV_SHIFT); > - > - return cfg; > -} > - > LM32CPU *cpu_lm32_init(const char *cpu_model) > { > LM32CPU *cpu; > - CPULM32State *env; > - const LM32Def *def; > + ObjectClass *oc; > =20 > - def =3D cpu_lm32_find_by_name(cpu_model); > - if (!def) { > + oc =3D cpu_class_by_name(TYPE_LM32_CPU, cpu_model); > + if (oc =3D=3D NULL) { > return NULL; > } > - > - cpu =3D LM32_CPU(object_new(TYPE_LM32_CPU)); > - env =3D &cpu->env; > - > - env->features =3D def->features; > - env->num_bps =3D def->num_breakpoints; > - env->num_wps =3D def->num_watchpoints; > - env->cfg =3D cfg_by_def(def); > + cpu =3D LM32_CPU(object_new(object_class_get_name(oc))); > =20 > object_property_set_bool(OBJECT(cpu), true, "realized", NULL); > =20 > diff --git a/target-lm32/translate.c b/target-lm32/translate.c > index e292e1c..93075e4 100644 > --- a/target-lm32/translate.c > +++ b/target-lm32/translate.c > @@ -64,7 +64,6 @@ enum { > =20 > /* This is the state at translation time. */ > typedef struct DisasContext { > - CPULM32State *env; > target_ulong pc; > =20 > /* Decoder. */ > @@ -82,6 +81,10 @@ typedef struct DisasContext { > =20 > struct TranslationBlock *tb; > int singlestep_enabled; > + > + uint32_t features; > + uint8_t num_breakpoints; > + uint8_t num_watchpoints; > } DisasContext; > =20 > static const char *regnames[] =3D { > @@ -420,7 +423,7 @@ static void dec_divu(DisasContext *dc) > =20 > LOG_DIS("divu r%d, r%d, r%d\n", dc->r2, dc->r0, dc->r1); > =20 > - if (!(dc->env->features & LM32_FEATURE_DIVIDE)) { > + if (!(dc->features & LM32_FEATURE_DIVIDE)) { > qemu_log_mask(LOG_GUEST_ERROR, "hardware divider is not availa= ble\n"); > return; > } > @@ -499,7 +502,7 @@ static void dec_modu(DisasContext *dc) > =20 > LOG_DIS("modu r%d, r%d, %d\n", dc->r2, dc->r0, dc->r1); > =20 > - if (!(dc->env->features & LM32_FEATURE_DIVIDE)) { > + if (!(dc->features & LM32_FEATURE_DIVIDE)) { > qemu_log_mask(LOG_GUEST_ERROR, "hardware divider is not availa= ble\n"); > return; > } > @@ -521,7 +524,7 @@ static void dec_mul(DisasContext *dc) > LOG_DIS("mul r%d, r%d, r%d\n", dc->r2, dc->r0, dc->r1); > } > =20 > - if (!(dc->env->features & LM32_FEATURE_MULTIPLY)) { > + if (!(dc->features & LM32_FEATURE_MULTIPLY)) { > qemu_log_mask(LOG_GUEST_ERROR, > "hardware multiplier is not available\n"); > return; > @@ -675,7 +678,7 @@ static void dec_sextb(DisasContext *dc) > { > LOG_DIS("sextb r%d, r%d\n", dc->r2, dc->r0); > =20 > - if (!(dc->env->features & LM32_FEATURE_SIGN_EXTEND)) { > + if (!(dc->features & LM32_FEATURE_SIGN_EXTEND)) { > qemu_log_mask(LOG_GUEST_ERROR, > "hardware sign extender is not available\n"); > return; > @@ -688,7 +691,7 @@ static void dec_sexth(DisasContext *dc) > { > LOG_DIS("sexth r%d, r%d\n", dc->r2, dc->r0); > =20 > - if (!(dc->env->features & LM32_FEATURE_SIGN_EXTEND)) { > + if (!(dc->features & LM32_FEATURE_SIGN_EXTEND)) { > qemu_log_mask(LOG_GUEST_ERROR, > "hardware sign extender is not available\n"); > return; > @@ -717,7 +720,7 @@ static void dec_sl(DisasContext *dc) > LOG_DIS("sl r%d, r%d, r%d\n", dc->r2, dc->r0, dc->r1); > } > =20 > - if (!(dc->env->features & LM32_FEATURE_SHIFT)) { > + if (!(dc->features & LM32_FEATURE_SHIFT)) { > qemu_log_mask(LOG_GUEST_ERROR, "hardware shifter is not availa= ble\n"); > return; > } > @@ -740,7 +743,7 @@ static void dec_sr(DisasContext *dc) > LOG_DIS("sr r%d, r%d, r%d\n", dc->r2, dc->r0, dc->r1); > } > =20 > - if (!(dc->env->features & LM32_FEATURE_SHIFT)) { > + if (!(dc->features & LM32_FEATURE_SHIFT)) { > if (dc->format =3D=3D OP_FMT_RI) { > /* TODO: check r1 =3D=3D 1 during runtime */ > } else { > @@ -770,7 +773,7 @@ static void dec_sru(DisasContext *dc) > LOG_DIS("sru r%d, r%d, r%d\n", dc->r2, dc->r0, dc->r1); > } > =20 > - if (!(dc->env->features & LM32_FEATURE_SHIFT)) { > + if (!(dc->features & LM32_FEATURE_SHIFT)) { > if (dc->format =3D=3D OP_FMT_RI) { > /* TODO: check r1 =3D=3D 1 during runtime */ > } else { > @@ -880,7 +883,7 @@ static void dec_wcsr(DisasContext *dc) > case CSR_BP2: > case CSR_BP3: > no =3D dc->csr - CSR_BP0; > - if (dc->env->num_bps <=3D no) { > + if (dc->num_breakpoints <=3D no) { > qemu_log_mask(LOG_GUEST_ERROR, > "breakpoint #%i is not available\n", no); > break; > @@ -892,7 +895,7 @@ static void dec_wcsr(DisasContext *dc) > case CSR_WP2: > case CSR_WP3: > no =3D dc->csr - CSR_WP0; > - if (dc->env->num_wps <=3D no) { > + if (dc->num_watchpoints <=3D no) { > qemu_log_mask(LOG_GUEST_ERROR, > "watchpoint #%i is not available\n", no); > break; > @@ -1033,7 +1036,9 @@ void gen_intermediate_code_internal(LM32CPU *cpu, > int max_insns; > =20 > pc_start =3D tb->pc; > - dc->env =3D env; > + dc->features =3D cpu->features; > + dc->num_breakpoints =3D cpu->num_breakpoints; > + dc->num_watchpoints =3D cpu->num_watchpoints; > dc->tb =3D tb; > =20 > gen_opc_end =3D tcg_ctx.gen_opc_buf + OPC_MAX_SIZE; --=20 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=C3=BCrnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imend=C3=B6rffer; HRB 16746 AG N=C3=BC= rnberg