On Sat, Mar 29, 2014 at 2:10 AM, Peter Maydell <peter.maydell@xxxxxxxxxx> wrote:
> The Cortex-A57, like most of the other ARM cores, has a CBAR
> register which defines the base address of the per-CPU
> peripherals. However it has a 64-bit view as well as a
> 32-bit view; expand the QOM reset-cbar property from UINT32
> to UINT64 so this can be specified, and implement the
> 32-bit and 64-bit views of a 64-bit CBAR.
>
> Signed-off-by: Peter Maydell <peter.maydell@xxxxxxxxxx>
Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xxxxxxxxxx>
> ---
> target-arm/cpu-qom.h | 2 +-
> target-arm/cpu.c | 8 ++++++--
> target-arm/cpu.h | 1 +
> target-arm/cpu64.c | 1 +
> target-arm/helper.c | 39 +++++++++++++++++++++++++++++++++------
> 5 files changed, 42 insertions(+), 9 deletions(-)
>
> diff --git a/target-arm/cpu-qom.h b/target-arm/cpu-qom.h
> index 743985e..82f1bc7 100644
> --- a/target-arm/cpu-qom.h
> +++ b/target-arm/cpu-qom.h
> @@ -148,7 +148,7 @@ typedef struct ARMCPU {
> * in the order L1DCache, L1ICache, L2DCache, L2ICache, etc.
> */
> uint32_t ccsidr[16];
> - uint32_t reset_cbar;
> + uint64_t reset_cbar;
> uint32_t reset_auxcr;
> bool reset_hivecs;
> /* DCZ blocksize, in log_2(words), ie low 4 bits of DCZID_EL0 */
> diff --git a/target-arm/cpu.c b/target-arm/cpu.c
> index 783fc73..f9f6187 100644
> --- a/target-arm/cpu.c
> +++ b/target-arm/cpu.c
> @@ -262,7 +262,7 @@ static void arm_cpu_initfn(Object *obj)
> }
>
> static Property arm_cpu_reset_cbar_property =
> - DEFINE_PROP_UINT32("reset-cbar", ARMCPU, reset_cbar, 0);
> + DEFINE_PROP_UINT64("reset-cbar", ARMCPU, reset_cbar, 0);
>
> static Property arm_cpu_reset_hivecs_property =
> DEFINE_PROP_BOOL("reset-hivecs", ARMCPU, reset_hivecs, false);
> @@ -274,7 +274,8 @@ static void arm_cpu_post_init(Object *obj)
> {
> ARMCPU *cpu = ARM_CPU(obj);
>
> - if (arm_feature(&cpu->env, ARM_FEATURE_CBAR)) {
> + if (arm_feature(&cpu->env, ARM_FEATURE_CBAR) ||
> + arm_feature(&cpu->env, ARM_FEATURE_CBAR_RO)) {
> qdev_property_add_static(DEVICE(obj), &arm_cpu_reset_cbar_property,
> &error_abort);
> }
> @@ -349,6 +350,9 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
> set_feature(env, ARM_FEATURE_V7MP);
> set_feature(env, ARM_FEATURE_PXN);
> }
> + if (arm_feature(env, ARM_FEATURE_CBAR_RO)) {
> + set_feature(env, ARM_FEATURE_CBAR);
> + }
>
> if (cpu->reset_hivecs) {
> cpu->reset_sctlr |= (1 << 13);
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index bebb333..c83f249 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -630,6 +630,7 @@ enum arm_features {
> ARM_FEATURE_V8_AES, /* implements AES part of v8 Crypto Extensions */
> ARM_FEATURE_CBAR, /* has cp15 CBAR */
> ARM_FEATURE_CRC, /* ARMv8 CRC instructions */
> + ARM_FEATURE_CBAR_RO, /* has cp15 CBAR and it is read-only */
> };
>
> static inline int arm_feature(CPUARMState *env, int feature)
> diff --git a/target-arm/cpu64.c b/target-arm/cpu64.c
> index 70a83fc..9a0c431 100644
> --- a/target-arm/cpu64.c
> +++ b/target-arm/cpu64.c
> @@ -97,6 +97,7 @@ static void aarch64_a57_initfn(Object *obj)
> set_feature(&cpu->env, ARM_FEATURE_NEON);
> set_feature(&cpu->env, ARM_FEATURE_GENERIC_TIMER);
> set_feature(&cpu->env, ARM_FEATURE_AARCH64);
> + set_feature(&cpu->env, ARM_FEATURE_CBAR_RO);
> cpu->kvm_target = QEMU_KVM_ARM_TARGET_CORTEX_A57;
> cpu->midr = 0x411fd070;
> cpu->reset_fpsid = 0x41034070;
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index 4a74249..f169bea 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -2463,12 +2463,39 @@ void register_cp_regs_for_features(ARMCPU *cpu)
> }
>
> if (arm_feature(env, ARM_FEATURE_CBAR)) {
> - 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);
> + if (arm_feature(env, ARM_FEATURE_AARCH64)) {
> + /* 32 bit view is [31:18] 0...0 [43:32]. */
> + uint32_t cbar32 = cpu->reset_cbar
Should you extract64 on the lower order bits as well to avoid weird |
results on a misaligned reset_cbar (or perhaps its worth an assert?).
> + | extract64(cpu->reset_cbar, 32, 12);
> + ARMCPRegInfo cbar_reginfo[] = {
> + { .name = "CBAR",
> + .type = ARM_CP_CONST,
> + .cp = 15, .crn = 15, .crm = 0, .opc1 = 4, .opc2 = 0,
> + .access = PL1_R, .resetvalue = cpu->reset_cbar },
> + { .name = "CBAR_EL1", .state = ARM_CP_STATE_AA64,
> + .type = ARM_CP_CONST,
> + .opc0 = 3, .opc1 = 1, .crn = 15, .crm = 3, .opc2 = 0,
> + .access = PL1_R, .resetvalue = cbar32 },
> + REGINFO_SENTINEL
> + };
> + /* We don't implement a r/w 64 bit CBAR currently */
Is it even valid? Writable CBAR seems like a bug to me that's just
fixed in the V8 version bump.
> + assert(arm_feature(env, ARM_FEATURE_CBAR_RO));
> + define_arm_cp_regs(cpu, cbar_reginfo);
> + } else {
> + 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)
> + };
> + if (arm_feature(env, ARM_FEATURE_CBAR_RO)) {
> + cbar.access = PL1_R;
> + cbar.fieldoffset = 0;
> + cbar.type = ARM_CP_CONST;
> + }
> + define_one_arm_cp_reg(cpu, &cbar);
> + }
> }
>
> /* Generic registers whose values depend on the implementation */
> --
> 1.9.0
>
>
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
