Patchwork [v3,03/13] target-sh4: Start QOM'ifying CPU init

login
register
mail settings
Submitter Andreas Färber
Date April 14, 2012, 10:12 p.m.
Message ID <1334441565-26433-4-git-send-email-afaerber@suse.de>
Download mbox | patch
Permalink /patch/152570/
State New
Headers show

Comments

Andreas Färber - April 14, 2012, 10:12 p.m.
Move code from cpu_sh4_init() into a QOM initfn.

Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 target-sh4/cpu.c       |   13 +++++++++++++
 target-sh4/translate.c |    3 ---
 2 files changed, 13 insertions(+), 3 deletions(-)
Peter Maydell - April 16, 2012, 2:39 p.m.
On 14 April 2012 23:12, Andreas Färber <afaerber@suse.de> wrote:
> +static void superh_cpu_initfn(Object *obj)
> +{
> +    SuperHCPU *cpu = SUPERH_CPU(obj);
> +    CPUSH4State *env = &cpu->env;
> +
> +    cpu_exec_init(env);
> +
> +    env->movcal_backup_tail = &(env->movcal_backup);
> +
> +    cpu_reset(CPU(cpu));
> +}
> +

[rats, made this comment on the v2 patch by mistake, here it
is again as a reply to the correct email...]

Do the other qom conversions do a cpu reset in the instance
init function? I don't think the ARM one does (and it would
probably be bad if it did since reset for some ARM cores
requires loading values from guest memory so it can't happen
before the whole of the model has been set up.)
We should aim for consistency across targets here I think.

-- PMM
Andreas Färber - April 16, 2012, 2:56 p.m.
Am 16.04.2012 16:39, schrieb Peter Maydell:
> On 14 April 2012 23:12, Andreas Färber <afaerber@suse.de> wrote:
>> +static void superh_cpu_initfn(Object *obj)
>> +{
>> +    SuperHCPU *cpu = SUPERH_CPU(obj);
>> +    CPUSH4State *env = &cpu->env;
>> +
>> +    cpu_exec_init(env);
>> +
>> +    env->movcal_backup_tail = &(env->movcal_backup);
>> +
>> +    cpu_reset(CPU(cpu));
>> +}
>> +
> 
> Do the other qom conversions do a cpu reset in the instance
> init function? I don't think the ARM one does (and it would
> probably be bad if it did since reset for some ARM cores
> requires loading values from guest memory so it can't happen
> before the whole of the model has been set up.)

This has been a per-target choice, depending on what the reset code
does. In lack of realize, in some cases I have moved it into initfn
since cpu_*_init() does it instantly and works. Not all targets do a
reset in cpu_*_init() at all though, in particular x86.

> We should aim for consistency across targets here I think.

My aim here was to cut down cpu_*_init() as much as possible.

I am hoping that with realize we will be able to move the
qemu_init_vcpu() into a realizefn and do cpu_reset() for all CPUs there
at that point in time. Moves from initfn to realizefn would (in most
cases) be local to cpu.c then.

So, I can move it back to cpu_sh4_init() but I'm hesitant to do more sh4
resends until it becomes clear who, if anyone, will actually apply the
series...

Andreas

Patch

diff --git a/target-sh4/cpu.c b/target-sh4/cpu.c
index 84d4672..5dab9ea 100644
--- a/target-sh4/cpu.c
+++ b/target-sh4/cpu.c
@@ -53,6 +53,18 @@  static void superh_cpu_reset(CPUState *s)
     set_default_nan_mode(1, &env->fp_status);
 }
 
+static void superh_cpu_initfn(Object *obj)
+{
+    SuperHCPU *cpu = SUPERH_CPU(obj);
+    CPUSH4State *env = &cpu->env;
+
+    cpu_exec_init(env);
+
+    env->movcal_backup_tail = &(env->movcal_backup);
+
+    cpu_reset(CPU(cpu));
+}
+
 static void superh_cpu_class_init(ObjectClass *oc, void *data)
 {
     CPUClass *cc = CPU_CLASS(oc);
@@ -66,6 +78,7 @@  static const TypeInfo superh_cpu_type_info = {
     .name = TYPE_SUPERH_CPU,
     .parent = TYPE_CPU,
     .instance_size = sizeof(SuperHCPU),
+    .instance_init = superh_cpu_initfn,
     .abstract = false,
     .class_size = sizeof(SuperHCPUClass),
     .class_init = superh_cpu_class_init,
diff --git a/target-sh4/translate.c b/target-sh4/translate.c
index d0568e2..b7426f1 100644
--- a/target-sh4/translate.c
+++ b/target-sh4/translate.c
@@ -259,11 +259,8 @@  CPUSH4State *cpu_sh4_init(const char *cpu_model)
     cpu = SUPERH_CPU(object_new(TYPE_SUPERH_CPU));
     env = &cpu->env;
     env->features = def->features;
-    cpu_exec_init(env);
-    env->movcal_backup_tail = &(env->movcal_backup);
     sh4_translate_init();
     env->cpu_model_str = cpu_model;
-    cpu_reset(CPU(cpu));
     cpu_register(env, def);
     qemu_init_vcpu(env);
     return env;