| Message ID | 1358314380-9400-3-git-send-email-afaerber@suse.de |
|---|---|
| State | New |
| Headers | show |
On Wed, Jan 16, 2013 at 06:32:47AM +0100, Andreas Färber wrote: [...] > diff --git a/target-arm/helper.c b/target-arm/helper.c > index 37c34a1..f4553de 100644 > --- a/target-arm/helper.c > +++ b/target-arm/helper.c > @@ -1270,14 +1270,12 @@ 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); > > if (tcg_enabled() && !inited) { > inited = 1; > arm_translate_init(); > } > > - cpu_reset(CPU(cpu)); > if (arm_feature(env, ARM_FEATURE_NEON)) { > gdb_register_coprocessor(env, vfp_gdb_get_reg, vfp_gdb_set_reg, > 51, "arm-neon.xml", 0); Some arm_feature() checks here (e.g. ARM_FEATURE_VFP3) depend on set_feature() calls done by arm_cpu_realize[fn](). On the other hand, I won't be surprised if gdb_register_coprocessor() needs to be called before qemu_init_vcpu(). I also don't know if it is safe to call gdb_register_coprocessor() before cpu_reset(). Why not move all the code between the "arm_cpu_realize(cpu)" and "return cpu" lines to the realize function as-is, instead of moving only part of the code? If arm requires these steps to be run after creating a CPU, I consider all of them part of the CPU realization process. > @@ -1288,7 +1286,10 @@ 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); > + > + /* TODO this should be set centrally, once possible */ > + object_property_set_bool(OBJECT(cpu), true, "realized", NULL); > + > return cpu; > } > > -- > 1.7.10.4 > >
Am 16.01.2013 16:52, schrieb Eduardo Habkost: > On Wed, Jan 16, 2013 at 06:32:47AM +0100, Andreas Färber wrote: > [...] >> diff --git a/target-arm/helper.c b/target-arm/helper.c >> index 37c34a1..f4553de 100644 >> --- a/target-arm/helper.c >> +++ b/target-arm/helper.c >> @@ -1270,14 +1270,12 @@ 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); >> >> if (tcg_enabled() && !inited) { >> inited = 1; >> arm_translate_init(); >> } >> >> - cpu_reset(CPU(cpu)); >> if (arm_feature(env, ARM_FEATURE_NEON)) { >> gdb_register_coprocessor(env, vfp_gdb_get_reg, vfp_gdb_set_reg, >> 51, "arm-neon.xml", 0); > > Some arm_feature() checks here (e.g. ARM_FEATURE_VFP3) depend on > set_feature() calls done by arm_cpu_realize[fn](). Ouch! > On the other hand, I won't be surprised if gdb_register_coprocessor() > needs to be called before qemu_init_vcpu(). I also don't know if it is > safe to call gdb_register_coprocessor() before cpu_reset(). > > Why not move all the code between the "arm_cpu_realize(cpu)" and "return > cpu" lines to the realize function as-is, instead of moving only part of > the code? If arm requires these steps to be run after creating a CPU, I > consider all of them part of the CPU realization process. That was not directly possible because the helper functions registered are here in helper.c. What I'll do is to put these into a separate function that I can call from realizefn. Thanks, Andreas
diff --git a/target-arm/cpu-qom.h b/target-arm/cpu-qom.h index 0f455c4..91c6eb1 100644 --- a/target-arm/cpu-qom.h +++ b/target-arm/cpu-qom.h @@ -107,7 +107,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..480acbe 100644 --- a/target-arm/cpu.c +++ b/target-arm/cpu.c @@ -147,14 +147,9 @@ 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); CPUARMState *env = &cpu->env; /* Some features automatically imply others: */ if (arm_feature(env, ARM_FEATURE_V7)) { @@ -197,6 +192,9 @@ void arm_cpu_realize(ARMCPU *cpu) } register_cp_regs_for_features(cpu); + + cpu_reset(CPU(cpu)); + qemu_init_vcpu(env); } /* CPU models */ @@ -763,6 +761,9 @@ 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); + + dc->realize = arm_cpu_realizefn; acc->parent_reset = cc->reset; cc->reset = arm_cpu_reset; diff --git a/target-arm/helper.c b/target-arm/helper.c index 37c34a1..f4553de 100644 --- a/target-arm/helper.c +++ b/target-arm/helper.c @@ -1270,14 +1270,12 @@ 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); if (tcg_enabled() && !inited) { inited = 1; arm_translate_init(); } - cpu_reset(CPU(cpu)); 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,7 +1286,10 @@ 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); + + /* TODO this should be set centrally, once possible */ + object_property_set_bool(OBJECT(cpu), true, "realized", NULL); + return cpu; }
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 CPU reset and vCPU initialization into the realizefn. Signed-off-by: Andreas Färber <afaerber@suse.de> --- target-arm/cpu-qom.h | 1 - target-arm/cpu.c | 15 ++++++++------- target-arm/helper.c | 7 ++++--- 3 Dateien geändert, 12 Zeilen hinzugefügt(+), 11 Zeilen entfernt(-)