On 29 November 2016 at 15:43, Cédric Le Goater <c...@kaod.org> wrote:
> ARM1176 CPUs support the Vector Base Address Register but currently,
> qemu only supports VBAR on ARMv7 CPUs. Fix this by adding a new
> feature ARM_FEATURE_VBAR which can used for ARMv7 and ARM1176 CPUs.
>
> The VBAR feature is always set for ARMv7 because some legacy boards
> require it even if this is not architecturally correct. However, to
> support arm1176 CPUs without TrustZone, which doesn't exist in real
> hardware but which is used in old qemu boards, we need to disable the
> feature when 'has_el3' is not set.
>
> Signed-off-by: Cédric Le Goater <c...@kaod.org>
> ---
> target-arm/cpu.c | 6 ++++++
> target-arm/cpu.h | 1 +
> target-arm/helper.c | 24 ++++++++++++++++++------
> 3 files changed, 25 insertions(+), 6 deletions(-)
>
> diff --git a/target-arm/cpu.c b/target-arm/cpu.c
> index 99f0dbebb9f6..1007e504248a 100644
> --- a/target-arm/cpu.c
> +++ b/target-arm/cpu.c
> @@ -685,6 +685,11 @@ static void arm_cpu_realizefn(DeviceState *dev, Error
> **errp)
> */
> cpu->id_pfr1 &= ~0xf0;
> cpu->id_aa64pfr0 &= ~0xf000;
> +
> + /* Also disable VBAR support for boards using a arm1176 CPU
> + * without EL3.
> + */
> + unset_feature(env, ARM_FEATURE_VBAR);
> }
>
> if (!cpu->has_pmu || !kvm_enabled()) {
> @@ -911,6 +916,7 @@ static void arm1176_initfn(Object *obj)
>
> cpu->dtb_compatible = "arm,arm1176";
> set_feature(&cpu->env, ARM_FEATURE_V6K);
> + set_feature(&cpu->env, ARM_FEATURE_VBAR);
> set_feature(&cpu->env, ARM_FEATURE_VFP);
> set_feature(&cpu->env, ARM_FEATURE_VAPA);
> set_feature(&cpu->env, ARM_FEATURE_DUMMY_C15_REGS);
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index ca5c849ed65e..ab119e62ab0f 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -1125,6 +1125,7 @@ enum arm_features {
> ARM_FEATURE_V8_PMULL, /* implements PMULL part of v8 Crypto Extensions */
> ARM_FEATURE_THUMB_DSP, /* DSP insns supported in the Thumb encodings */
> ARM_FEATURE_PMU, /* has PMU support */
> + ARM_FEATURE_VBAR, /* has cp15 VBAR */
> };
>
> static inline int arm_feature(CPUARMState *env, int feature)
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index b5b65caadf8a..d417c8ba802f 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -1252,12 +1252,6 @@ static const ARMCPRegInfo v7_cp_reginfo[] = {
> .access = PL1_RW, .accessfn = access_tpm, .type = ARM_CP_ALIAS,
> .fieldoffset = offsetof(CPUARMState, cp15.c9_pminten),
> .writefn = pmintenclr_write },
> - { .name = "VBAR", .state = ARM_CP_STATE_BOTH,
> - .opc0 = 3, .crn = 12, .crm = 0, .opc1 = 0, .opc2 = 0,
> - .access = PL1_RW, .writefn = vbar_write,
> - .bank_fieldoffsets = { offsetof(CPUARMState, cp15.vbar_s),
> - offsetof(CPUARMState, cp15.vbar_ns) },
> - .resetvalue = 0 },
> { .name = "CCSIDR", .state = ARM_CP_STATE_BOTH,
> .opc0 = 3, .crn = 0, .crm = 0, .opc1 = 1, .opc2 = 0,
> .access = PL1_R, .readfn = ccsidr_read, .type = ARM_CP_NO_RAW },
> @@ -1412,6 +1406,16 @@ static const ARMCPRegInfo v6k_cp_reginfo[] = {
> REGINFO_SENTINEL
> };
>
> +static const ARMCPRegInfo vbar_cp_reginfo[] = {
> + { .name = "VBAR", .state = ARM_CP_STATE_BOTH,
> + .opc0 = 3, .crn = 12, .crm = 0, .opc1 = 0, .opc2 = 0,
> + .access = PL1_RW, .writefn = vbar_write,
> + .bank_fieldoffsets = { offsetof(CPUARMState, cp15.vbar_s),
> + offsetof(CPUARMState, cp15.vbar_ns) },
> + .resetvalue = 0 },
> + REGINFO_SENTINEL
> +};
> +
> #ifndef CONFIG_USER_ONLY
>
> static CPAccessResult gt_cntfrq_access(CPUARMState *env, const ARMCPRegInfo
> *ri,
> @@ -4506,6 +4510,9 @@ void register_cp_regs_for_features(ARMCPU *cpu)
> if (arm_feature(env, ARM_FEATURE_V6K)) {
> define_arm_cp_regs(cpu, v6k_cp_reginfo);
> }
> + if (arm_feature(env, ARM_FEATURE_VBAR)) {
> + define_arm_cp_regs(cpu, vbar_cp_reginfo);
> + }
> if (arm_feature(env, ARM_FEATURE_V7MP) &&
> !arm_feature(env, ARM_FEATURE_MPU)) {
> define_arm_cp_regs(cpu, v7mp_cp_reginfo);
> @@ -4543,6 +4550,11 @@ void register_cp_regs_for_features(ARMCPU *cpu)
> };
> define_one_arm_cp_reg(cpu, &clidr);
> define_arm_cp_regs(cpu, v7_cp_reginfo);
> +
> + /* Always define VBAR even if it doesn't exist in non-EL3
> + * configs. This is needed by some legacy boards.
> + */
> + define_arm_cp_regs(cpu, vbar_cp_reginfo);
> define_debug_regs(cpu);
> } else {
> define_arm_cp_regs(cpu, not_v7_cp_reginfo);
> --
> 2.7.4
This all seems overcomplicated to me. The aim is to define VBAR for:
* CPUs with TrustZone/EL3
* ARMv7 CPUs (even if no EL3, for legacy reasons)
right?
We do that by changing arm_cpu_realizefn():
* add set_feature(arm, ARM_FEATURE_VBAR) to the things we enable
when the ARM_FEATURE_V7 bit is set (with a comment about why)
* add
if (arm_feature(env, ARM_FEATURE_EL3)) {
set_feature(env, ARM_FEATURE_VBAR));
}
in the appropriate place
and then register_cp_regs_for_features() just does
if (arm_feature(env, ARM_FEATURE_VBAR)) {
define_arm_cp_regs(cpu, vbar_cp_reginfo);
}
You don't need to explicitly set the VBAR feature in the 1176
initfn, or unset set feature bits, or register vbar_cp_reginfo
in two separate places.
thanks
-- PMM
