Re: [Qemu-devel] [RFC 12/12] target-xtensa: QOM'ify CPU

[jcmvbkbc <jcmvbkbc@gmail.com>]

> Let xtensa_cpu_list() enumerate CPU classes alphabetically.
>
> Signed-off-by: Andreas Färber<afaerber@suse.de>
> ---

[...]

> diff --git a/gdbstub.c b/gdbstub.c
> index f4e97f7..773e86f 100644
> --- a/gdbstub.c
> +++ b/gdbstub.c
> @@ -1570,14 +1570,17 @@ static int cpu_gdb_write_register(CPULM32State *env, uint8_t *mem_buf, int n)
>    * reset bit 0 in the 'flags' field of the registers definitions in the
>    * gdb/xtensa-config.c inside gdb source tree or inside gdb overlay.
>    */
> -#define NUM_CORE_REGS (env->config->gdb_regmap.num_regs)
> +#define NUM_CORE_REGS \
> +    (XTENSA_CPU_GET_CLASS(xtensa_env_get_cpu(env))->gdb_regmap.num_regs)
>   #define num_g_regs NUM_CORE_REGS
>
>   static int cpu_gdb_read_register(CPUXtensaState *env, uint8_t *mem_buf, int n)
>   {
> -    const XtensaGdbReg *reg = env->config->gdb_regmap.reg + n;
> +    XtensaCPU *cpu = xtensa_env_get_cpu(env);
> +    XtensaCPUClass *klass = XTENSA_CPU_GET_CLASS(cpu);

*klass*

It's a bit strange to see patches that fix typos in comments and at the same time
to deliberately introduce this kind of misspelling. I'd suggest to call it what it is: cpu_class.

[...]

> diff --git a/target-xtensa/overlay_tool.h b/target-xtensa/overlay_tool.h
> index a3a5650..b46bca9 100644
> --- a/target-xtensa/overlay_tool.h
> +++ b/target-xtensa/overlay_tool.h
> @@ -291,16 +291,28 @@
>   #endif
>
>   #if (defined(TARGET_WORDS_BIGENDIAN) != 0) == (XCHAL_HAVE_BE != 0)
> -#define REGISTER_CORE(core) \
> -    static void __attribute__((constructor)) register_core(void) \
> +#define REGISTER_CORE(typename, class) \
> +    static void core_class_init(ObjectClass *klass, void *data) \
>       { \
> -        static XtensaConfigList node = { \
> -            .config =&core, \
> -        }; \
> -        xtensa_register_core(&node); \
> -    }
> +        /* XXX This is a really ugly but easy way to init the class... */ \
> +        memcpy((void *)klass + offsetof(XtensaCPUClass, options), \
> +               (void *)&(class) + offsetof(XtensaCPUClass, options), \
> +               sizeof(XtensaCPUClass) - offsetof(XtensaCPUClass, options)); \
> +    } \

- void pointer arithmetic is non-standard;
- (void *)&(class) + offsetof(XtensaCPUClass, options) looks suspicious, I don't think
   anything other than XtensaCPUClass instances should be passed here;

I'd suggest the following replacement:

         memcpy(&((XtensaCPUClass *)klass)->options, \
                &(class).options, \
                sizeof(XtensaCPUClass) - offsetof(XtensaCPUClass, options)); \

[...]

--
Thanks.
-- Max