[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 01/32] target-arm: initial coprocessor register
From: |
Andreas Färber |
Subject: |
Re: [Qemu-devel] [PATCH 01/32] target-arm: initial coprocessor register framework |
Date: |
Mon, 14 May 2012 00:57:01 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:12.0) Gecko/20120421 Thunderbird/12.0 |
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 <address@hidden>
> ---
> 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
- Re: [Qemu-devel] [PATCH 01/32] target-arm: initial coprocessor register framework,
Andreas Färber <=