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

[Peter Crosthwaite]

On Sat, Dec 7, 2013 at 12:12 AM, Peter Maydell <address@hidden> wrote:
> 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.
>

Done:

diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 318642e..947a1e7 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -463,11 +463,11 @@ 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,
     ARM_FEATURE_AARCH64, /* supports 64 bit mode */
+    ARM_FEATURE_CBAR, /* has cp15 CBAR */
 };

Regards,
Peter

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