[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2] target-lm32: move model features to LM32CPU
|
From: |
Andreas Färber |
|
Subject: |
Re: [Qemu-devel] [PATCH v2] target-lm32: move model features to LM32CPU |
|
Date: |
Mon, 18 Nov 2013 16:03:03 +0100 |
|
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.1.0 |
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 <address@hidden>
> Signed-off-by: Michael Walle <address@hidden>
> ---
>
> 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;
> + 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
> + | LM32_FEATURE_CYCLE_COUNT;
> +}
> +
> +typedef struct LM32CPUInfo {
> + const char *name;
> + void (*initfn)(Object *obj);
> +} LM32CPUInfo;
> +
> +static const LM32CPUInfo lm32_cpus[] = {
> + {
> + .name = "lm32-basic",
> + .initfn = lm32_basic_cpu_initfn,
> + },
> + {
> + .name = "lm32-standard",
> + .initfn = lm32_standard_cpu_initfn,
> + },
> + {
> + .name = "lm32-full",
> + .initfn = lm32_full_cpu_initfn,
> + },
> +};
> +
> +static ObjectClass *lm32_cpu_class_by_name(const char *cpu_model)
> +{
> + ObjectClass *oc;
> + char *typename;
> +
> + if (cpu_model == NULL) {
> + return NULL;
> + }
> +
> + typename = g_strdup_printf("%s-" TYPE_LM32_CPU, cpu_model);
> + oc = object_class_by_name(typename);
> + g_free(typename);
> + if (oc != NULL && (!object_class_dynamic_cast(oc, TYPE_LM32_CPU) ||
> + object_class_is_abstract(oc))) {
> + oc = NULL;
> + }
> + return oc;
> +}
> +
> static void lm32_cpu_class_init(ObjectClass *oc, void *data)
> {
> LM32CPUClass *lcc = LM32_CPU_CLASS(oc);
> @@ -86,6 +253,7 @@ static void lm32_cpu_class_init(ObjectClass *oc, void
> *data)
> lcc->parent_reset = cc->reset;
> cc->reset = lm32_cpu_reset;
>
> + cc->class_by_name = lm32_cpu_class_by_name;
> cc->do_interrupt = lm32_cpu_do_interrupt;
> cc->dump_state = lm32_cpu_dump_state;
> cc->set_pc = lm32_cpu_set_pc;
> @@ -98,19 +266,36 @@ static void lm32_cpu_class_init(ObjectClass *oc, void
> *data)
> cc->gdb_num_core_regs = 32 + 7;
> }
>
> +static void lm32_register_cpu_type(const LM32CPUInfo *info)
> +{
> + TypeInfo type_info = {
> + .parent = TYPE_LM32_CPU,
> + .instance_init = info->initfn,
> + };
> +
> + type_info.name = 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 = {
> .name = TYPE_LM32_CPU,
> .parent = TYPE_CPU,
> .instance_size = sizeof(LM32CPU),
> .instance_init = lm32_cpu_initfn,
> - .abstract = false,
> + .abstract = true,
> .class_size = sizeof(LM32CPUClass),
> .class_init = lm32_cpu_class_init,
> };
>
> static void lm32_cpu_register_types(void)
> {
> + int i;
> +
> type_register_static(&lm32_cpu_type_info);
> + for (i = 0; i < ARRAY_SIZE(lm32_cpus); i++) {
> + lm32_register_cpu_type(&lm32_cpus[i]);
> + }
> }
>
> 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;
>
> /* processor core features */
> - uint32_t features;
> uint32_t flags;
> - uint8_t num_bps;
> - uint8_t num_wps;
>
> };
>
> #include "cpu-qom.h"
>
> 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);
>
> @@ -206,7 +203,7 @@ static inline CPULM32State *cpu_init(const char
> *cpu_model)
> return &cpu->env;
> }
>
> -#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)
> }
> }
>
> -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[] = {
> - {
> - .name = "lm32-basic",
> - .revision = 3,
> - .num_interrupts = 32,
> - .num_breakpoints = 4,
> - .num_watchpoints = 4,
> - .features = (LM32_FEATURE_SHIFT
> - | LM32_FEATURE_SIGN_EXTEND
> - | LM32_FEATURE_CYCLE_COUNT),
> - },
> - {
> - .name = "lm32-standard",
> - .revision = 3,
> - .num_interrupts = 32,
> - .num_breakpoints = 4,
> - .num_watchpoints = 4,
> - .features = (LM32_FEATURE_MULTIPLY
> - | LM32_FEATURE_DIVIDE
> - | LM32_FEATURE_SHIFT
> - | LM32_FEATURE_SIGN_EXTEND
> - | LM32_FEATURE_I_CACHE
> - | LM32_FEATURE_CYCLE_COUNT),
> - },
> - {
> - .name = "lm32-full",
> - .revision = 3,
> - .num_interrupts = 32,
> - .num_breakpoints = 4,
> - .num_watchpoints = 4,
> - .features = (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 = 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 = 0; i < ARRAY_SIZE(lm32_defs); i++) {
> - if (strcasecmp(name, lm32_defs[i].name) == 0) {
> - return &lm32_defs[i];
> - }
> - }
> -
> - return NULL;
> -}
> -
> -static uint32_t cfg_by_def(const LM32Def *def)
> -{
> - uint32_t cfg = 0;
> -
> - if (def->features & LM32_FEATURE_MULTIPLY) {
> - cfg |= CFG_M;
> - }
> -
> - if (def->features & LM32_FEATURE_DIVIDE) {
> - cfg |= CFG_D;
> - }
> -
> - if (def->features & LM32_FEATURE_SHIFT) {
> - cfg |= CFG_S;
> - }
> -
> - if (def->features & LM32_FEATURE_SIGN_EXTEND) {
> - cfg |= CFG_X;
> - }
> -
> - if (def->features & LM32_FEATURE_I_CACHE) {
> - cfg |= CFG_IC;
> - }
> -
> - if (def->features & LM32_FEATURE_D_CACHE) {
> - cfg |= CFG_DC;
> - }
> -
> - if (def->features & LM32_FEATURE_CYCLE_COUNT) {
> - cfg |= CFG_CC;
> - }
> -
> - cfg |= (def->num_interrupts << CFG_INT_SHIFT);
> - cfg |= (def->num_breakpoints << CFG_BP_SHIFT);
> - cfg |= (def->num_watchpoints << CFG_WP_SHIFT);
> - cfg |= (def->revision << CFG_REV_SHIFT);
> -
> - return cfg;
> -}
> -
> LM32CPU *cpu_lm32_init(const char *cpu_model)
> {
> LM32CPU *cpu;
> - CPULM32State *env;
> - const LM32Def *def;
> + ObjectClass *oc;
>
> - def = cpu_lm32_find_by_name(cpu_model);
> - if (!def) {
> + oc = cpu_class_by_name(TYPE_LM32_CPU, cpu_model);
> + if (oc == NULL) {
> return NULL;
> }
> -
> - cpu = LM32_CPU(object_new(TYPE_LM32_CPU));
> - env = &cpu->env;
> -
> - env->features = def->features;
> - env->num_bps = def->num_breakpoints;
> - env->num_wps = def->num_watchpoints;
> - env->cfg = cfg_by_def(def);
> + cpu = LM32_CPU(object_new(object_class_get_name(oc)));
>
> object_property_set_bool(OBJECT(cpu), true, "realized", NULL);
>
> 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 {
>
> /* This is the state at translation time. */
> typedef struct DisasContext {
> - CPULM32State *env;
> target_ulong pc;
>
> /* Decoder. */
> @@ -82,6 +81,10 @@ typedef struct DisasContext {
>
> struct TranslationBlock *tb;
> int singlestep_enabled;
> +
> + uint32_t features;
> + uint8_t num_breakpoints;
> + uint8_t num_watchpoints;
> } DisasContext;
>
> static const char *regnames[] = {
> @@ -420,7 +423,7 @@ static void dec_divu(DisasContext *dc)
>
> LOG_DIS("divu r%d, r%d, r%d\n", dc->r2, dc->r0, dc->r1);
>
> - if (!(dc->env->features & LM32_FEATURE_DIVIDE)) {
> + if (!(dc->features & LM32_FEATURE_DIVIDE)) {
> qemu_log_mask(LOG_GUEST_ERROR, "hardware divider is not
> available\n");
> return;
> }
> @@ -499,7 +502,7 @@ static void dec_modu(DisasContext *dc)
>
> LOG_DIS("modu r%d, r%d, %d\n", dc->r2, dc->r0, dc->r1);
>
> - if (!(dc->env->features & LM32_FEATURE_DIVIDE)) {
> + if (!(dc->features & LM32_FEATURE_DIVIDE)) {
> qemu_log_mask(LOG_GUEST_ERROR, "hardware divider is not
> available\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);
> }
>
> - 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);
>
> - 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);
>
> - 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);
> }
>
> - if (!(dc->env->features & LM32_FEATURE_SHIFT)) {
> + if (!(dc->features & LM32_FEATURE_SHIFT)) {
> qemu_log_mask(LOG_GUEST_ERROR, "hardware shifter is not
> available\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);
> }
>
> - if (!(dc->env->features & LM32_FEATURE_SHIFT)) {
> + if (!(dc->features & LM32_FEATURE_SHIFT)) {
> if (dc->format == OP_FMT_RI) {
> /* TODO: check r1 == 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);
> }
>
> - if (!(dc->env->features & LM32_FEATURE_SHIFT)) {
> + if (!(dc->features & LM32_FEATURE_SHIFT)) {
> if (dc->format == OP_FMT_RI) {
> /* TODO: check r1 == 1 during runtime */
> } else {
> @@ -880,7 +883,7 @@ static void dec_wcsr(DisasContext *dc)
> case CSR_BP2:
> case CSR_BP3:
> no = dc->csr - CSR_BP0;
> - if (dc->env->num_bps <= no) {
> + if (dc->num_breakpoints <= 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 = dc->csr - CSR_WP0;
> - if (dc->env->num_wps <= no) {
> + if (dc->num_watchpoints <= 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;
>
> pc_start = tb->pc;
> - dc->env = env;
> + dc->features = cpu->features;
> + dc->num_breakpoints = cpu->num_breakpoints;
> + dc->num_watchpoints = cpu->num_watchpoints;
> dc->tb = tb;
>
> gen_opc_end = tcg_ctx.gen_opc_buf + OPC_MAX_SIZE;
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg