Re: [Qemu-devel] [PATCH v7] arm: add dummy v7 cp15 registers

[Peter Maydell]

On 4 January 2012 19:07, Mark Langsdorf <address@hidden> wrote:
> Add dummy register support for the cp15, CRn=c15 registers.
> config_base_register and power_control_register currently
> default to 0, but may have improved support after the QOM
> CPU patches are finished.
>
> Signed-off-by: Mark Langsdorf <address@hidden>
> ---
> Changes from v6
>        Added the diagnostic registers
>        Added save/load support
> Changes from v5
>        Added handling for all c15 registers
> Changes from v3, v4
>        None
> Changes from v2
>        Added test against op2
> Changes from v1
>        renamed the config_base_register
>        added comments about how it will change when QOM CPUs are added
>
>  target-arm/cpu.h     |    4 ++++
>  target-arm/helper.c  |   48 ++++++++++++++++++++++++++++++++++++++++++++++++
>  target-arm/machine.c |    7 +++++++
>  3 files changed, 59 insertions(+), 0 deletions(-)
>
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index c4d742f..3bc90e6 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -149,6 +149,10 @@ typedef struct CPUARMState {
>         uint32_t c15_i_max; /* Maximum D-cache dirty line index.  */
>         uint32_t c15_i_min; /* Minimum D-cache dirty line index.  */
>         uint32_t c15_threadid; /* TI debugger thread-ID.  */
> +        uint32_t c15_config_base_address; /* SCU base address.  */
> +        uint32_t c15_diagnostic; /* diagnostic register */
> +        uint32_t c15_power_diagnostic;
> +        uint32_t c15_power_control; /* power control */
>     } cp15;
>
>     struct {
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index 65f4fbf..62028e5 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -1768,6 +1768,20 @@ void HELPER(set_cp15)(CPUState *env, uint32_t insn, 
> uint32_t val)
>                 goto bad_reg;
>             }
>         }
> +        if (ARM_CPUID(env) == ARM_CPUID_CORTEXA9) {
> +            switch (crm) {
> +            case 0:
> +                if ((op1 == 0) && (op2 == 0)) {
> +                    env->cp15.c15_power_control = val;
> +                } else if ((op1 == 0) && (op2 == 1)) {
> +                    env->cp15.c15_diagnostic = val;
> +                } else if ((op1 == 0) && (op2 == 2)) {
> +                    env->cp15.c15_power_diagnostic = val;
> +                }
> +            default:
> +                break;
> +            }
> +        }
>         break;
>     }
>     return;
> @@ -2111,6 +2125,40 @@ uint32_t HELPER(get_cp15)(CPUState *env, uint32_t insn)
>              * 0x200 << ($rn & 0xfff), when MMU is off.  */
>             goto bad_reg;
>         }
> +        if (ARM_CPUID(env) == ARM_CPUID_CORTEXA9) {
> +            switch (crm) {
> +            case 0:
> +                if ((op1 == 4) && (op2 == 0)) {
> +                /* The config_base_address should hold the value of

The indentation's wrong here and below (checkpatch complains).

> +                 * the peripheral base. ARM should get this from a CPU
> +                 * object property, but that support isn't available in
> +                 * December 2011. Default to 0 for now and board models
> +                 * that care can set it by a private hook */
> +                    return env->cp15.c15_config_base_address;
> +                } else if ((op1 == 0) && (op2 == 0)) {
> +                /* power_control should be set to maximum latency. Again,
> +                   default to 0 and set by private hook */
> +                    return env->cp15.c15_power_control;
> +                } else if ((op1 == 0) && (op2 == 1)) {
> +                    return env->cp15.c15_diagnostic;
> +                } else if ((op1 == 0) && (op2 == 2)) {
> +                    return env->cp15.c15_power_diagnostic;
> +                }
> +                break;
> +            case 1: /* NEON Busy */
> +                return 0;
> +            case 5: /* tlb lockdown */
> +            case 6:
> +            case 7:
> +                if ((op1 == 5) && (op2 == 2)) {
> +                    return 0;
> +                }
> +                break;
> +            default:
> +                break;
> +            }
> +            goto bad_reg;
> +        }
>         return 0;
>     }
>  bad_reg:
> diff --git a/target-arm/machine.c b/target-arm/machine.c
> index aaee9b9..7f25b0f 100644
> --- a/target-arm/machine.c
> +++ b/target-arm/machine.c
> @@ -56,6 +56,9 @@ void cpu_save(QEMUFile *f, void *opaque)
>     qemu_put_be32(f, env->cp15.c13_tls2);
>     qemu_put_be32(f, env->cp15.c13_tls3);
>     qemu_put_be32(f, env->cp15.c15_cpar);
> +    qemu_put_be32(f, env->cp15.c15_power_control);
> +    qemu_put_be32(f, env->cp15.c15_diagnostic);
> +    qemu_put_be32(f, env->cp15.c15_power_diagnostic);
>
>     qemu_put_be32(f, env->features);
>
> @@ -170,6 +173,10 @@ int cpu_load(QEMUFile *f, void *opaque, int version_id)
>     env->cp15.c13_tls2 = qemu_get_be32(f);
>     env->cp15.c13_tls3 = qemu_get_be32(f);
>     env->cp15.c15_cpar = qemu_get_be32(f);
> +    env->cp15.c15_power_control = qemu_get_be32(f);
> +    env->cp15.c15_diagnostic = qemu_get_be32(f);
> +    env->cp15.c15_power_diagnostic = qemu_get_be32(f);
> +

Stray blank line.

Adding fields to the cpu_load/cpu_save functions means you have
to bump CPU_SAVE_VERSION in target-arm/cpu.h.

>     env->features = qemu_get_be32(f);

Thanks for your efforts revising this patch; I think we're nearly there...

-- PMM