From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:44700) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RrgtG-0004E9-N2 for qemu-devel@nongnu.org; Sun, 29 Jan 2012 21:22:43 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RrgtF-0002Bd-53 for qemu-devel@nongnu.org; Sun, 29 Jan 2012 21:22:42 -0500 Received: from mail-tul01m020-f173.google.com ([209.85.214.173]:40705) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RrgtE-0002BZ-VL for qemu-devel@nongnu.org; Sun, 29 Jan 2012 21:22:41 -0500 Received: by obbup16 with SMTP id up16so4123141obb.4 for ; Sun, 29 Jan 2012 18:22:40 -0800 (PST) Message-ID: <4F25FEEE.5040504@codemonkey.ws> Date: Sun, 29 Jan 2012 20:22:38 -0600 From: Anthony Liguori MIME-Version: 1.0 References: <1327843531-32403-1-git-send-email-afaerber@suse.de> <1327843531-32403-8-git-send-email-afaerber@suse.de> In-Reply-To: <1327843531-32403-8-git-send-email-afaerber@suse.de> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH RFC 7/7] target-arm: Embed CPUARMState in QOM ARMCPU List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?UTF-8?B?QW5kcmVhcyBGw6RyYmVy?= Cc: Peter Maydell , Anthony Liguori , Paul Brook , qemu-devel@nongnu.org, Richard Henderson On 01/29/2012 07:25 AM, Andreas Färber wrote: > We g_malloc0()'ed CPUARMState ourself, and exec.c's cpu_copy() runs > through cpu_init() as well, so we are at liberty to supply the CPUState > any way we see fit. Having CPUARMState as field in the QOM CPU allows > both to access env from an ARMCPU object and to access the QOM Object > and its ObjectClass from an env pointer, in ARM code for now. > > The goal is to convert all CPUs to QOM and to use CPU objects in central > places, especially once we have property support for Object. > This will then allow to have TCG AREG0 point to target-specific fields > where small immediate offsets are desired (as pointed out by rth) while > allowing for common fields at known offsets from the base class. > > Having the CPUID in ARMCPUClass, we can set it from the realize function. > Same for cpu_model_str, which is now the QOM class name. > > Signed-off-by: Andreas Färber > Cc: Anthony Liguori > Cc: Paul Brook > Cc: Peter Maydell > Cc: Richard Henderson > --- > target-arm/cpu-core.c | 13 +++++++++++++ > target-arm/cpu-core.h | 7 +++++++ > target-arm/helper.c | 13 ++++++------- > 3 files changed, 26 insertions(+), 7 deletions(-) > > diff --git a/target-arm/cpu-core.c b/target-arm/cpu-core.c > index 9761d8e..b1ac22c 100644 > --- a/target-arm/cpu-core.c > +++ b/target-arm/cpu-core.c > @@ -234,12 +234,25 @@ static const struct ARMCPUDef arm_cpu_models[] = { > { } > }; > > +static void arm_cpu_realize(Object *obj) > +{ > + ARMCPU *cpu = ARM_CPU(obj); > + ARMCPUClass *cpu_class = ARM_CPU_GET_CLASS(obj); > + > + memset(&cpu->env, 0, sizeof(CPUARMState)); > + cpu_exec_init(&cpu->env); > + > + cpu->env.cpu_model_str = object_get_typename(obj); > + cpu->env.cp15.c0_cpuid = cpu_class->id; > +} > + > static void cpu_register(const struct ARMCPUDef *def) > { > TypeInfo type = { > .name = def->name, > .parent = TYPE_ARM_CPU, > .instance_size = sizeof(ARMCPU), > + .instance_init = arm_cpu_realize, The convention I'm using is: instance_init => type_name_initfn. DeviceState::init => type_name_realize, Eventually, realized will become a property and there will be a realize and unrealize method. > .class_size = sizeof(ARMCPUClass), > .class_init = def->class_init, > }; > diff --git a/target-arm/cpu-core.h b/target-arm/cpu-core.h > index be4bbc3..08b6b2b 100644 > --- a/target-arm/cpu-core.h > +++ b/target-arm/cpu-core.h > @@ -10,6 +10,7 @@ > #define QEMU_ARM_CPU_CORE_H > > #include "qemu/cpu.h" > +#include "cpu.h" > > #define TYPE_ARM_CPU "arm-cpu-core" > #define ARM_CPU_CLASS(klass) \ > @@ -27,7 +28,13 @@ typedef struct ARMCPUClass { > > typedef struct ARMCPU { > CPU parent_obj; > + > + /* TODO Inline this and split off common state */ > + CPUARMState env; This is an interesting (and good) idea. I think this could make it fairly easy to qomify things. > } ARMCPU; > > +#define ENV_GET_OBJECT(e) \ > + (Object *)((void *)(e) - offsetof(ARMCPU, env)) sizeof(CPU) should be sizeof(void *). Presumably it's okay to add 8 bytes to the beginning of CPUState? If so, that would make things much nicer from a QOM perspective. Regards, Anthony Liguori > > #endif > diff --git a/target-arm/helper.c b/target-arm/helper.c > index ece9635..1ffd7ba 100644 > --- a/target-arm/helper.c > +++ b/target-arm/helper.c > @@ -400,7 +400,7 @@ static int vfp_gdb_set_reg(CPUState *env, uint8_t *buf, int reg) > CPUARMState *cpu_arm_init(const char *cpu_model) > { > ObjectClass *klass; > - ARMCPUClass *cpu_class; > + ARMCPU *cpu; > CPUARMState *env; > static int inited = 0; > > @@ -408,16 +408,14 @@ CPUARMState *cpu_arm_init(const char *cpu_model) > if (klass == NULL) { > return NULL; > } > - cpu_class = ARM_CPU_CLASS(klass); > - env = g_malloc0(sizeof(CPUARMState)); > - cpu_exec_init(env); > + cpu = ARM_CPU(object_new_with_type(klass->type)); > + env =&cpu->env; > + > if (tcg_enabled()&& !inited) { > inited = 1; > arm_translate_init(); > } > > - env->cpu_model_str = cpu_model; > - env->cp15.c0_cpuid = cpu_class->id; > cpu_reset(env); > if (arm_feature(env, ARM_FEATURE_NEON)) { > gdb_register_coprocessor(env, vfp_gdb_get_reg, vfp_gdb_set_reg, > @@ -459,7 +457,8 @@ void arm_cpu_list(FILE *f, fprintf_function cpu_fprintf) > > void cpu_arm_close(CPUARMState *env) > { > - g_free(env); > + Object *obj = ENV_GET_OBJECT(env); > + object_delete(obj); > } > > static int bad_mode_switch(CPUState *env, int mode)