Re: [Qemu-devel] [PATCH 3/9] arm: add missing v7 cp15 registers

[Peter Maydell]

On 20 December 2011 19:10, Mark Langsdorf <address@hidden> wrote:
> From: Rob Herring <address@hidden>
>
> Signed-off-by: Rob Herring <address@hidden>
> Signed-off-by: Mark Langsdorf <address@hidden>
>
> Conflicts:
>
>        target-arm/cpu.h
>        target-arm/helper.c

Commit messages shouldn't have Conflicts notes in them.
You could also use more detail here (for a start, you've
only added one register).

> ---
>  target-arm/cpu.h    |    1 +
>  target-arm/helper.c |    7 +++++++
>  2 files changed, 8 insertions(+), 0 deletions(-)
>
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index 129edbb..b89c085 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -152,6 +152,7 @@ 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_scubase; /* SCU base address.  */
>     } cp15;
>
>     struct {
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index 816c4c4..37110bc 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -2197,6 +2197,13 @@ 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:
> +                return env->cp15.c15_scubase;
> +            }
> +            goto bad_reg;
> +        }

This is underdecoded: the A9 has two registers in c15,c0:
CRn     Op1     CRm     Op2     Name
15      0       c0      0       Power Control Register
15      4       c0      0       Configuration Base Address

I'm guessing you're after the Configuration Base Address register.
(please call the struct name something vaguely relating to the
official register name, incidentally.)

>         return 0;
>     }
>  bad_reg:

This commit leaves the register with a reset value of 0, which
isn't right (we only implement A9MP, not A9UP, so the reset value
should be settable by the board at init time somehow depending
where the a9mpcore_priv device is mapped. Not sure what the
cleanest way to do that is.)

-- PMM