From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:40719) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SThix-0004gs-Mj for qemu-devel@nongnu.org; Sun, 13 May 2012 18:57:13 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SThiv-0004bD-K9 for qemu-devel@nongnu.org; Sun, 13 May 2012 18:57:11 -0400 Received: from cantor2.suse.de ([195.135.220.15]:49309 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SThiv-0004b9-Ad for qemu-devel@nongnu.org; Sun, 13 May 2012 18:57:09 -0400 Message-ID: <4FB03C3D.5030003@suse.de> Date: Mon, 14 May 2012 00:57:01 +0200 From: =?ISO-8859-15?Q?Andreas_F=E4rber?= MIME-Version: 1.0 References: <1334497585-867-1-git-send-email-peter.maydell@linaro.org> <1334497585-867-2-git-send-email-peter.maydell@linaro.org> In-Reply-To: <1334497585-867-2-git-send-email-peter.maydell@linaro.org> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 01/32] target-arm: initial coprocessor register framework List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell Cc: patches@linaro.org, qemu-devel@nongnu.org, Rusty Russel , Paul Brook Am 15.04.2012 15:45, schrieb Peter Maydell: > Initial infrastructure for data-driven registration of > coprocessor register implementations. >=20 > We still fall back to the old-style switch statements pending > complete conversion of all existing registers. >=20 > Signed-off-by: Peter Maydell > --- > target-arm/cpu.c | 34 ++++++++ > target-arm/cpu.h | 214 ++++++++++++++++++++++++++++++++++++++++= ++++++++ > target-arm/helper.c | 83 +++++++++++++++++++ > target-arm/helper.h | 5 + > target-arm/op_helper.c | 42 +++++++++- > target-arm/translate.c | 155 ++++++++++++++++++++++++++++++++++- > 6 files changed, 530 insertions(+), 3 deletions(-) >=20 > diff --git a/target-arm/cpu.c b/target-arm/cpu.c > index 8f5e309..ae55cd0 100644 > --- a/target-arm/cpu.c > +++ b/target-arm/cpu.c > @@ -24,6 +24,37 @@ > #include "hw/loader.h" > #endif > =20 > +static void cp_reg_reset(void *key, void *value, void *udata) > +{ This ugliness is thanks to GLib, it seems. In that case please stick to their signature to make that clear, i.e. 3x gpointer. http://developer.gnome.org/glib/stable/glib-Hash-Tables.html#GHFunc user_data / data / opaque would also seem nicer than udata. > + /* Reset a single ARMCPRegInfo register */ > + ARMCPRegInfo *ri =3D value; > + CPUARMState *env =3D udata; > + > + if (ri->type & ARM_CP_SPECIAL) { > + return; > + } > + > + if (ri->resetfn) { > + ri->resetfn(env, ri); > + return; > + } > + > + /* A zero offset is never possible as it would be regs[0] > + * so we use it to indicate that reset is being handled elsewhere. > + * This is basically only used for fields in non-core coprocessors > + * (like the pxa2xx ones). > + */ > + if (!ri->fieldoffset) { > + return; > + } > + > + if (ri->type & ARM_CP_64BIT) { > + CPREG_FIELD64(env, ri) =3D ri->resetvalue; > + } else { > + CPREG_FIELD32(env, ri) =3D ri->resetvalue; > + } > +} > + > /* CPUClass::reset() */ > static void arm_cpu_reset(CPUState *s) > { [...] > @@ -130,6 +162,8 @@ static void arm_cpu_initfn(Object *obj) > ARMCPU *cpu =3D ARM_CPU(obj); > =20 > cpu_exec_init(&cpu->env); > + cpu->env.cp_regs =3D g_hash_table_new_full(g_int_hash, g_int_equal= , > + g_free, g_free); > } > =20 > void arm_cpu_realize(ARMCPU *cpu) > diff --git a/target-arm/cpu.h b/target-arm/cpu.h > index 12f5854..f35d24f 100644 > --- a/target-arm/cpu.h > +++ b/target-arm/cpu.h > @@ -228,6 +228,9 @@ typedef struct CPUARMState { > /* Internal CPU feature flags. */ > uint32_t features; > =20 > + /* Coprocessor information */ > + GHashTable *cp_regs; > + > /* Coprocessor IO used by peripherals */ > struct { > ARMReadCPFunc *cp_read; [snip] I'm aware this series predates the QOM era, but I'm not really happy how this aligns with my CPUState overhaul. Independent of what needs to be fixed for cpu_copy(), I would like to see new non-TCG fields such as this hashtable added to ARMCPU after the env field, not to the old CPUARMState (using fieldoffset +/- from env is correct though). By consequence the API should be changed to take ARMCPU *cpu rather than CPUARMState *env to avoid the QOM casts that you so loathe and to avoid us refactoring this new API in a few weeks again. The ARM cp14/cp15 specific bits I do not feel qualified to review and am counting on Rusty and Paul to review. Any chance to further split up this patch? Andreas --=20 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=FCrnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imend=F6rffer; HRB 16746 AG N=FCrnbe= rg