On Sun, Jan 20, 2013 at 08:22:26AM +0100, Andreas Färber wrote:
> Turn arm_cpu_realize() into a QOM realize function, no longer called
> via cpu.h prototype. To maintain the semantics of cpu_init(), set
> realized = true explicitly in cpu_arm_init().
>
> Move GDB coprocessor registration, CPU reset and vCPU initialization
> into the realizefn.
>
> Signed-off-by: Andreas Färber <afaer...@suse.de>
> ---
> target-arm/cpu-qom.h | 3 ++-
> target-arm/cpu.c | 21 ++++++++++++++-------
> target-arm/cpu.h | 1 +
> target-arm/helper.c | 14 ++++++++++----
> 4 Dateien geändert, 27 Zeilen hinzugefügt(+), 12 Zeilen entfernt(-)
>
> diff --git a/target-arm/cpu-qom.h b/target-arm/cpu-qom.h
> index 0f455c4..aff7bf3 100644
> --- a/target-arm/cpu-qom.h
> +++ b/target-arm/cpu-qom.h
> @@ -33,6 +33,7 @@
>
> /**
> * ARMCPUClass:
> + * @parent_realize: The parent class' realize handler.
> * @parent_reset: The parent class' reset handler.
> *
> * An ARM CPU model.
> @@ -42,6 +43,7 @@ typedef struct ARMCPUClass {
> CPUClass parent_class;
> /*< public >*/
>
> + DeviceRealize parent_realize;
> void (*parent_reset)(CPUState *cpu);
> } ARMCPUClass;
>
> @@ -107,7 +109,6 @@ static inline ARMCPU *arm_env_get_cpu(CPUARMState *env)
>
> #define ENV_GET_CPU(e) CPU(arm_env_get_cpu(e))
>
> -void arm_cpu_realize(ARMCPU *cpu);
> void register_cp_regs_for_features(ARMCPU *cpu);
>
> #endif
> diff --git a/target-arm/cpu.c b/target-arm/cpu.c
> index 07588a1..19d5ae4 100644
> --- a/target-arm/cpu.c
> +++ b/target-arm/cpu.c
> @@ -147,15 +147,12 @@ static void arm_cpu_finalizefn(Object *obj)
> g_hash_table_destroy(cpu->cp_regs);
> }
>
> -void arm_cpu_realize(ARMCPU *cpu)
> +static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
> {
> - /* This function is called by cpu_arm_init() because it
> - * needs to do common actions based on feature bits, etc
> - * that have been set by the subclass init functions.
> - * When we have QOM realize support it should become
> - * a true realize function instead.
> - */
> + ARMCPU *cpu = ARM_CPU(dev);
> + ARMCPUClass *acc = ARM_CPU_GET_CLASS(dev);
> CPUARMState *env = &cpu->env;
> +
> /* Some features automatically imply others: */
> if (arm_feature(env, ARM_FEATURE_V7)) {
> set_feature(env, ARM_FEATURE_VAPA);
> @@ -197,6 +194,12 @@ void arm_cpu_realize(ARMCPU *cpu)
> }
>
> register_cp_regs_for_features(cpu);
> + arm_cpu_register_gdb_regs_for_features(cpu);
> +
> + cpu_reset(CPU(cpu));
> + qemu_init_vcpu(env);
> +
> + acc->parent_realize(dev, errp);
> }
>
> /* CPU models */
> @@ -763,6 +766,10 @@ static void arm_cpu_class_init(ObjectClass *oc, void
> *data)
> {
> ARMCPUClass *acc = ARM_CPU_CLASS(oc);
> CPUClass *cc = CPU_CLASS(acc);
> + DeviceClass *dc = DEVICE_CLASS(oc);
> +
> + acc->parent_realize = dc->realize;
> + dc->realize = arm_cpu_realizefn;
>
> acc->parent_reset = cc->reset;
> cc->reset = arm_cpu_reset;
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index ffddfcb..2902ba5 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -234,6 +234,7 @@ typedef struct CPUARMState {
>
> ARMCPU *cpu_arm_init(const char *cpu_model);
> void arm_translate_init(void);
> +void arm_cpu_register_gdb_regs_for_features(ARMCPU *cpu);
> int cpu_arm_exec(CPUARMState *s);
> void do_interrupt(CPUARMState *);
> void switch_mode(CPUARMState *, int);
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index 37c34a1..f412143 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -1270,14 +1270,22 @@ ARMCPU *cpu_arm_init(const char *cpu_model)
> cpu = ARM_CPU(object_new(cpu_model));
> env = &cpu->env;
> env->cpu_model_str = cpu_model;
> - arm_cpu_realize(cpu);
> +
> + /* TODO this should be set centrally, once possible */
> + object_property_set_bool(OBJECT(cpu), true, "realized", NULL);
>
> if (tcg_enabled() && !inited) {
> inited = 1;
> arm_translate_init();
> }
My first question I had when I saw this was: are you really sure it is
safe to call cpu_reset() and qemu_init_vcpu() before
arm_translate_init()?
But I see that you change this on commit 092028dbf1. So now I have the
opposite question: are you really sure it is safe to call
arm_translate_init() before arm_cpu_realizefn()?
>
> - cpu_reset(CPU(cpu));
> + return cpu;
> +}
> +
> +void arm_cpu_register_gdb_regs_for_features(ARMCPU *cpu)
> +{
> + CPUARMState *env = &cpu->env;
> +
> if (arm_feature(env, ARM_FEATURE_NEON)) {
> gdb_register_coprocessor(env, vfp_gdb_get_reg, vfp_gdb_set_reg,
> 51, "arm-neon.xml", 0);
> @@ -1288,8 +1296,6 @@ ARMCPU *cpu_arm_init(const char *cpu_model)
> gdb_register_coprocessor(env, vfp_gdb_get_reg, vfp_gdb_set_reg,
> 19, "arm-vfp.xml", 0);
> }
> - qemu_init_vcpu(env);
> - return cpu;
> }
>
> /* Sort alphabetically by type name, except for "any". */
> --
> 1.7.10.4
>
>
--
Eduardo
