Am 15.04.2012 15:45, schrieb Peter Maydell: > Initial infrastructure for data-driven registration of > coprocessor register implementations. > > We still fall back to the old-style switch statements pending > complete conversion of all existing registers. > > Signed-off-by: Peter Maydell <peter.mayd...@linaro.org> > --- > 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(-) > > 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 > > +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 = value; > + CPUARMState *env = 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) = ri->resetvalue; > + } else { > + CPREG_FIELD32(env, ri) = ri->resetvalue; > + } > +} > + > /* CPUClass::reset() */ > static void arm_cpu_reset(CPUState *s) > { [...] > @@ -130,6 +162,8 @@ static void arm_cpu_initfn(Object *obj) > ARMCPU *cpu = ARM_CPU(obj); > > cpu_exec_init(&cpu->env); > + cpu->env.cp_regs = g_hash_table_new_full(g_int_hash, g_int_equal, > + g_free, g_free); > } > > 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; > > + /* 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 -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg