[[PATCH] 4/7] target-arm: Add AArch64 CPTR registers

Adds CPTR_EL2/3 system registers definitions and access function.

Signed-off-by: Greg Bellows <greg.bellows@linaro.org>
---
 target-arm/cpu.h    | 18 +++++++++++++++++-
 target-arm/helper.c | 43 ++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 59 insertions(+), 2 deletions(-)

On 27 March 2015 at 19:10, Greg Bellows <greg.bellows@linaro.org> wrote:
> Adds CPTR_EL2/3 system registers definitions and access function.
>
> Signed-off-by: Greg Bellows <greg.bellows@linaro.org>
> ---
>  target-arm/cpu.h    | 18 +++++++++++++++++-
>  target-arm/helper.c | 43 ++++++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 59 insertions(+), 2 deletions(-)
>
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index 2178a1f..a811369 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -202,6 +202,7 @@ typedef struct CPUARMState {
>              uint64_t sctlr_el[4];
>          };
>          uint64_t c1_coproc; /* Coprocessor access register.  */
> +        uint64_t cptr_el[4];  /* ARMv8 feature trap registers */
>          uint32_t c1_xscaleauxcr; /* XScale auxiliary control register.  */
>          uint64_t sder; /* Secure debug enable register. */
>          uint32_t nsacr; /* Non-secure access control register. */
> @@ -575,6 +576,10 @@ void pmccntr_sync(CPUARMState *env);
>  #define SCTLR_AFE     (1U << 29)
>  #define SCTLR_TE      (1U << 30)
>
> +#define CPTR_TCPAC    (1U << 31)
> +#define CPTR_TTA      (1U << 20)
> +#define CPTR_TFP      (1U << 10)
> +
>  #define CPSR_M (0x1fU)
>  #define CPSR_T (1U << 5)
>  #define CPSR_F (1U << 6)
> @@ -1813,9 +1818,20 @@ static inline void cpu_get_tb_cpu_state(CPUARMState *env, target_ulong *pc,
>                                          target_ulong *cs_base, int *flags)
>  {
>      int fpen;
> +    int cur_el = arm_current_el(env);
>
>      if (arm_feature(env, ARM_FEATURE_V6)) {
> -        fpen = extract32(env->cp15.c1_coproc, 20, 2);
> +        /* In AArch64, FP can be enabled differently depending on our EL.
> +         * If our EL is 2 or 3, we use the EL specific CPTR to determine if FP
> +         * is enabled.  Otherwise, we fall back to using CPACR.
> +         * CPTR.TFP is clear if FP is enabled whereas CPACR.FPEN is set to some
> +         * degree.
> +         */
> +        if (is_a64(env) && cur_el >= 2) {
> +            fpen = !extract32(env->cp15.cptr_el[cur_el], 10, 1);
> +        } else {
> +            fpen = extract32(env->cp15.c1_coproc, 20, 2);
> +        }

See my comments on patch 1 about what else we need to do to
be able to merge fp enable bits like this.

Also, the logic here is wrong: if we're at NS-EL0 or NS-EL1
then fpen is not just the c1_coproc bit: if cptr_el[2].TFP
is set then FP is disabled and traps to EL2 (same as if we
were at NS-EL2).

>      } else {
>          /* CPACR doesn't exist before v6, so VFP is always accessible */
>          fpen = 3;
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index 95383d5..00b457a 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -592,6 +592,39 @@ static void cpacr_write(CPUARMState *env, const ARMCPRegInfo *ri,
>      env->cp15.c1_coproc = value;
>  }
>
> +static CPAccessResult cptr_access(CPUARMState *env, const ARMCPRegInfo *ri)
> +{
> +    int cur_el = arm_current_el(env);
> +    bool secure = arm_is_secure(env);
> +
> +    switch (ri->opc1) {
> +    case 0:     /* CPACR and CPACR_EL1 */

We're not actually sharing any interesting code between these
two cases of ri->opc1, so you should just have two different
access functions.

> +        if (arm_feature(env, ARM_FEATURE_V8) && cur_el == 1) {
> +            /* Make sure we have EL2 before routine there */

"routing", but you don't need to -- if EL2 doesn't exist then
it's impossible for the trap bit to get set. Ditto EL3.

> +            if (arm_feature(env, ARM_FEATURE_EL2) && !secure &&
> +                (env->cp15.cptr_el[2] & CPTR_TCPAC)) {
> +                env->exception.target_el = 2;
> +                return CP_ACCESS_TRAP;
> +            /* Make sure we have EL3 before routine there */
> +            } else if (arm_feature(env, ARM_FEATURE_EL3) &&
> +                       env->cp15.cptr_el[3] & CPTR_TCPAC) {
> +                env->exception.target_el = 3;
> +                return CP_ACCESS_TRAP;
> +            }
> +        }
> +        break;
> +    case 4:     /* CPTR_EL2 */
> +        /* It is safe to assume we have EL2 and ARMv8 if we get here */

This register exists (as HCPTR) in ARMv7, so you can't assume
ARMv8, but on the other hand you don't have to, because again
the CPTR_EL3 bit can't get set unless we're ARMv8.

> +        if (cur_el == 2 && (env->cp15.cptr_el[3] & CPTR_TCPAC)) {
> +            env->exception.target_el = 3;
> +            return CP_ACCESS_TRAP;
> +        }
> +        break;
> +    }
> +
> +    return CP_ACCESS_OK;
> +}
> +
>  static const ARMCPRegInfo v6_cp_reginfo[] = {
>      /* prefetch by MVA in v6, NOP in v7 */
>      { .name = "MVA_prefetch",
> @@ -614,7 +647,7 @@ static const ARMCPRegInfo v6_cp_reginfo[] = {
>      { .name = "WFAR", .cp = 15, .crn = 6, .crm = 0, .opc1 = 0, .opc2 = 1,
>        .access = PL1_RW, .type = ARM_CP_CONST, .resetvalue = 0, },
>      { .name = "CPACR", .state = ARM_CP_STATE_BOTH, .opc0 = 3,
> -      .crn = 1, .crm = 0, .opc1 = 0, .opc2 = 2,
> +      .crn = 1, .crm = 0, .opc1 = 0, .opc2 = 2, .accessfn = cptr_access,
>        .access = PL1_RW, .fieldoffset = offsetof(CPUARMState, cp15.c1_coproc),
>        .resetvalue = 0, .writefn = cpacr_write },
>      REGINFO_SENTINEL
> @@ -2537,6 +2570,10 @@ static const ARMCPRegInfo v8_el2_cp_reginfo[] = {
>        .opc0 = 3, .opc1 = 6, .crn = 4, .crm = 1, .opc2 = 0,
>        .access = PL3_RW, .type = ARM_CP_ALIAS,
>        .fieldoffset = offsetof(CPUARMState, sp_el[2]) },
> +    { .name = "CPTR_EL2", .state = ARM_CP_STATE_AA64,
> +      .opc0 = 3, .opc1 = 4, .crn = 1, .crm = 1, .opc2 = 2,
> +      .access = PL2_RW, .accessfn = cptr_access, .resetvalue = 0,
> +      .fieldoffset = offsetof(CPUARMState, cp15.cptr_el[2]) },

This has an AArch32 view, as HCPTR.

Also, if EL2 isn't implemented then CPTR_EL2/HCPTR need to be RES0
if accessed from EL3, so you need an entry in v8_el3_no_el2_cp_reginfo.

>      REGINFO_SENTINEL
>  };
>
> @@ -2598,6 +2635,10 @@ static const ARMCPRegInfo el3_cp_reginfo[] = {
>        .access = PL3_RW, .writefn = vbar_write,
>        .fieldoffset = offsetof(CPUARMState, cp15.vbar_el[3]),
>        .resetvalue = 0 },
> +    { .name = "CPTR_EL3", .state = ARM_CP_STATE_AA64,
> +      .opc0 = 3, .opc1 = 6, .crn = 1, .crm = 1, .opc2 = 2,
> +      .access = PL3_RW, .accessfn = cptr_access, .resetvalue = 0,
> +      .fieldoffset = offsetof(CPUARMState, cp15.cptr_el[3]) },
>      REGINFO_SENTINEL
>  };
>
> --
> 1.8.3.2

thanks
-- PMM

On Thu, Apr 16, 2015 at 1:00 PM, Peter Maydell <peter.maydell@linaro.org>
wrote:

> On 27 March 2015 at 19:10, Greg Bellows <greg.bellows@linaro.org> wrote:
> > Adds CPTR_EL2/3 system registers definitions and access function.
> >
> > Signed-off-by: Greg Bellows <greg.bellows@linaro.org>
> > ---
> >  target-arm/cpu.h    | 18 +++++++++++++++++-
> >  target-arm/helper.c | 43 ++++++++++++++++++++++++++++++++++++++++++-
> >  2 files changed, 59 insertions(+), 2 deletions(-)
> >
> > diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> > index 2178a1f..a811369 100644
> > --- a/target-arm/cpu.h
> > +++ b/target-arm/cpu.h
> > @@ -202,6 +202,7 @@ typedef struct CPUARMState {
> >              uint64_t sctlr_el[4];
> >          };
> >          uint64_t c1_coproc; /* Coprocessor access register.  */
> > +        uint64_t cptr_el[4];  /* ARMv8 feature trap registers */
> >          uint32_t c1_xscaleauxcr; /* XScale auxiliary control register.
> */
> >          uint64_t sder; /* Secure debug enable register. */
> >          uint32_t nsacr; /* Non-secure access control register. */
> > @@ -575,6 +576,10 @@ void pmccntr_sync(CPUARMState *env);
> >  #define SCTLR_AFE     (1U << 29)
> >  #define SCTLR_TE      (1U << 30)
> >
> > +#define CPTR_TCPAC    (1U << 31)
> > +#define CPTR_TTA      (1U << 20)
> > +#define CPTR_TFP      (1U << 10)
> > +
> >  #define CPSR_M (0x1fU)
> >  #define CPSR_T (1U << 5)
> >  #define CPSR_F (1U << 6)
> > @@ -1813,9 +1818,20 @@ static inline void
> cpu_get_tb_cpu_state(CPUARMState *env, target_ulong *pc,
> >                                          target_ulong *cs_base, int
> *flags)
> >  {
> >      int fpen;
> > +    int cur_el = arm_current_el(env);
> >
> >      if (arm_feature(env, ARM_FEATURE_V6)) {
> > -        fpen = extract32(env->cp15.c1_coproc, 20, 2);
> > +        /* In AArch64, FP can be enabled differently depending on our
> EL.
> > +         * If our EL is 2 or 3, we use the EL specific CPTR to
> determine if FP
> > +         * is enabled.  Otherwise, we fall back to using CPACR.
> > +         * CPTR.TFP is clear if FP is enabled whereas CPACR.FPEN is set
> to some
> > +         * degree.
> > +         */
> > +        if (is_a64(env) && cur_el >= 2) {
> > +            fpen = !extract32(env->cp15.cptr_el[cur_el], 10, 1);
> > +        } else {
> > +            fpen = extract32(env->cp15.c1_coproc, 20, 2);
> > +        }
>
> See my comments on patch 1 about what else we need to do to
> be able to merge fp enable bits like this.
>

​Broke out FP exception support into a separate patch for v2.  The fix is
as mentioned earlier: change DisasContext flag into EL where 0 means
enabled.​


>
> Also, the logic here is wrong: if we're at NS-EL0 or NS-EL1
> then fpen is not just the c1_coproc bit: if cptr_el[2].TFP
> is set then FP is disabled and traps to EL2 (same as if we
> were at NS-EL2).
>

​Yes, you are correct.  Fixed in the FP patch.​


>
> >      } else {
> >          /* CPACR doesn't exist before v6, so VFP is always accessible */
> >          fpen = 3;
> > diff --git a/target-arm/helper.c b/target-arm/helper.c
> > index 95383d5..00b457a 100644
> > --- a/target-arm/helper.c
> > +++ b/target-arm/helper.c
> > @@ -592,6 +592,39 @@ static void cpacr_write(CPUARMState *env, const
> ARMCPRegInfo *ri,
> >      env->cp15.c1_coproc = value;
> >  }
> >
> > +static CPAccessResult cptr_access(CPUARMState *env, const ARMCPRegInfo
> *ri)
> > +{
> > +    int cur_el = arm_current_el(env);
> > +    bool secure = arm_is_secure(env);
> > +
> > +    switch (ri->opc1) {
> > +    case 0:     /* CPACR and CPACR_EL1 */
>
> We're not actually sharing any interesting code between these
> two cases of ri->opc1, so you should just have two different
> access functions.
>

​Broke into to access functions in v2.​


>
> > +        if (arm_feature(env, ARM_FEATURE_V8) && cur_el == 1) {
> > +            /* Make sure we have EL2 before routine there */
>
> "routing", but you don't need to -- if EL2 doesn't exist then
> it's impossible for the trap bit to get set. Ditto EL3.
>

​Fixed in v2.
​


>
> > +            if (arm_feature(env, ARM_FEATURE_EL2) && !secure &&
> > +                (env->cp15.cptr_el[2] & CPTR_TCPAC)) {
> > +                env->exception.target_el = 2;
> > +                return CP_ACCESS_TRAP;
> > +            /* Make sure we have EL3 before routine there */
> > +            } else if (arm_feature(env, ARM_FEATURE_EL3) &&
> > +                       env->cp15.cptr_el[3] & CPTR_TCPAC) {
> > +                env->exception.target_el = 3;
> > +                return CP_ACCESS_TRAP;
> > +            }
> > +        }
> > +        break;
> > +    case 4:     /* CPTR_EL2 */
> > +        /* It is safe to assume we have EL2 and ARMv8 if we get here */
>
> This register exists (as HCPTR) in ARMv7, so you can't assume
> ARMv8, but on the other hand you don't have to, because again
> the CPTR_EL3 bit can't get set unless we're ARMv8.
>

​Fixed comment.
​


>
> > +        if (cur_el == 2 && (env->cp15.cptr_el[3] & CPTR_TCPAC)) {
> > +            env->exception.target_el = 3;
> > +            return CP_ACCESS_TRAP;
> > +        }
> > +        break;
> > +    }
> > +
> > +    return CP_ACCESS_OK;
> > +}
> > +
> >  static const ARMCPRegInfo v6_cp_reginfo[] = {
> >      /* prefetch by MVA in v6, NOP in v7 */
> >      { .name = "MVA_prefetch",
> > @@ -614,7 +647,7 @@ static const ARMCPRegInfo v6_cp_reginfo[] = {
> >      { .name = "WFAR", .cp = 15, .crn = 6, .crm = 0, .opc1 = 0, .opc2 =
> 1,
> >        .access = PL1_RW, .type = ARM_CP_CONST, .resetvalue = 0, },
> >      { .name = "CPACR", .state = ARM_CP_STATE_BOTH, .opc0 = 3,
> > -      .crn = 1, .crm = 0, .opc1 = 0, .opc2 = 2,
> > +      .crn = 1, .crm = 0, .opc1 = 0, .opc2 = 2, .accessfn = cptr_access,
> >        .access = PL1_RW, .fieldoffset = offsetof(CPUARMState,
> cp15.c1_coproc),
> >        .resetvalue = 0, .writefn = cpacr_write },
> >      REGINFO_SENTINEL
> > @@ -2537,6 +2570,10 @@ static const ARMCPRegInfo v8_el2_cp_reginfo[] = {
> >        .opc0 = 3, .opc1 = 6, .crn = 4, .crm = 1, .opc2 = 0,
> >        .access = PL3_RW, .type = ARM_CP_ALIAS,
> >        .fieldoffset = offsetof(CPUARMState, sp_el[2]) },
> > +    { .name = "CPTR_EL2", .state = ARM_CP_STATE_AA64,
> > +      .opc0 = 3, .opc1 = 4, .crn = 1, .crm = 1, .opc2 = 2,
> > +      .access = PL2_RW, .accessfn = cptr_access, .resetvalue = 0,
> > +      .fieldoffset = offsetof(CPUARMState, cp15.cptr_el[2]) },
>
> This has an AArch32 view, as HCPTR.
>
> Also, if EL2 isn't implemented then CPTR_EL2/HCPTR need to be RES0
> if accessed from EL3, so you need an entry in v8_el3_no_el2_cp_reginfo.
>

I was short-cutting the EL2 stuff, but for completeness I added all the
above in v2.
​


>
> >      REGINFO_SENTINEL
> >  };
> >
> > @@ -2598,6 +2635,10 @@ static const ARMCPRegInfo el3_cp_reginfo[] = {
> >        .access = PL3_RW, .writefn = vbar_write,
> >        .fieldoffset = offsetof(CPUARMState, cp15.vbar_el[3]),
> >        .resetvalue = 0 },
> > +    { .name = "CPTR_EL3", .state = ARM_CP_STATE_AA64,
> > +      .opc0 = 3, .opc1 = 6, .crn = 1, .crm = 1, .opc2 = 2,
> > +      .access = PL3_RW, .accessfn = cptr_access, .resetvalue = 0,
> > +      .fieldoffset = offsetof(CPUARMState, cp15.cptr_el[3]) },
> >      REGINFO_SENTINEL
> >  };
> >
> > --
> > 1.8.3.2
>
> thanks
> -- PMM
>