Re: [Qemu-devel] [PATCH arm-devs v3 3/9] target-arm: Define and use ARM_FEATURE_CBAR

[Peter Maydell]

On 3 December 2013 07:00, Peter Crosthwaite
<address@hidden> wrote:
> Some processors (notably A9 within Highbank) define and use the
> CP15 configuration base address (CBAR). This is vendor specific
> so its best implemented as a CPU property (otherwise we would need
> vendor specific child classes for every ARM implementation).
>
> This patch prepares support for converting CBAR reset value to
> a CPU property by moving the CP registration out of the CPU
> init fn, as registration will need to happen at realize time
> to pick up any property updates. The easiest way to do this
> is via definition of a new ARM_FEATURE to flag the existence
> of the register.
>
> Signed-off-by: Peter Crosthwaite <address@hidden>
> ---
> changed since v2:
> msg typo: existence
> Enable CBAR for a15 as well
>
>  target-arm/cpu.c    | 12 +++---------
>  target-arm/cpu.h    |  1 +
>  target-arm/helper.c |  9 +++++++++
>  3 files changed, 13 insertions(+), 9 deletions(-)
>
> diff --git a/target-arm/cpu.c b/target-arm/cpu.c
> index d40f2a7..90413ee 100644
> --- a/target-arm/cpu.c
> +++ b/target-arm/cpu.c
> @@ -590,6 +590,7 @@ static void cortex_a9_initfn(Object *obj)
>       * and valid configurations; we don't model A9UP).
>       */
>      set_feature(&cpu->env, ARM_FEATURE_V7MP);
> +    set_feature(&cpu->env, ARM_FEATURE_CBAR);
>      cpu->midr = 0x410fc090;
>      cpu->reset_fpsid = 0x41033090;
>      cpu->mvfr0 = 0x11110222;
> @@ -612,15 +613,7 @@ static void cortex_a9_initfn(Object *obj)
>      cpu->clidr = (1 << 27) | (1 << 24) | 3;
>      cpu->ccsidr[0] = 0xe00fe015; /* 16k L1 dcache. */
>      cpu->ccsidr[1] = 0x200fe015; /* 16k L1 icache. */
> -    {
> -        ARMCPRegInfo cbar = {
> -            .name = "CBAR", .cp = 15, .crn = 15,  .crm = 0, .opc1 = 4,
> -            .opc2 = 0, .access = PL1_R|PL3_W, .resetvalue = cpu->reset_cbar,
> -            .fieldoffset = offsetof(CPUARMState, 
> cp15.c15_config_base_address)
> -        };
> -        define_one_arm_cp_reg(cpu, &cbar);
> -        define_arm_cp_regs(cpu, cortexa9_cp_reginfo);
> -    }
> +    define_arm_cp_regs(cpu, cortexa9_cp_reginfo);
>  }
>
>  #ifndef CONFIG_USER_ONLY
> @@ -657,6 +650,7 @@ static void cortex_a15_initfn(Object *obj)
>      set_feature(&cpu->env, ARM_FEATURE_ARM_DIV);
>      set_feature(&cpu->env, ARM_FEATURE_GENERIC_TIMER);
>      set_feature(&cpu->env, ARM_FEATURE_DUMMY_C15_REGS);
> +    set_feature(&cpu->env, ARM_FEATURE_CBAR);
>      set_feature(&cpu->env, ARM_FEATURE_LPAE);
>      cpu->midr = 0x412fc0f1;
>      cpu->reset_fpsid = 0x410430f0;
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index 9f110f1..859750a 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -461,6 +461,7 @@ enum arm_features {
>      ARM_FEATURE_CACHE_DIRTY_REG, /* 1136/1176 cache dirty status register */
>      ARM_FEATURE_CACHE_BLOCK_OPS, /* v6 optional cache block operations */
>      ARM_FEATURE_MPIDR, /* has cp15 MPIDR */
> +    ARM_FEATURE_CBAR, /* has cp15 CBAR */
>      ARM_FEATURE_PXN, /* has Privileged Execute Never bit */
>      ARM_FEATURE_LPAE, /* has Large Physical Address Extension */
>      ARM_FEATURE_V8,

env->features gets migrated, so reordering feature bits breaks
cross-version migration; better just to add at the end of the list.

Otherwise
Reviewed-by: Peter Maydell <address@hidden>

thanks
-- PMM