Re: [Qemu-devel] [PATCH v3 15/32] target-arm: add NSACR register

[Greg Bellows <greg.bellows@linaro.org>]

[-- Attachment #1: Type: text/plain, Size: 5720 bytes --]

On 10 June 2014 18:54, Fabian Aggeler <aggelerf@ethz.ch> wrote:

> Implements NSACR register with corresponding read/write functions
> for ARMv7 and ARMv8.
>
> Signed-off-by: Sergey Fedorov <s.fedorov@samsung.com>
> Signed-off-by: Fabian Aggeler <aggelerf@ethz.ch>
> ---
>  target-arm/cpu.h    |  6 +++++
>  target-arm/helper.c | 68
> ++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 73 insertions(+), 1 deletion(-)
>
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index 52e679f..bc9edaa 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -182,6 +182,7 @@ typedef struct CPUARMState {
>          uint64_t c1_coproc; /* Coprocessor access register.  */
>          uint32_t c1_xscaleauxcr; /* XScale auxiliary control register.  */
>          uint32_t c1_scr; /* secure config register.  */
> +        uint32_t c1_nsacr; /* Non-secure access control register. */
>          uint64_t ttbr0_el1; /* MMU translation table base 0. */
>          uint64_t ttbr1_el1; /* MMU translation table base 1. */
>          uint64_t c2_control; /* MMU translation table base control.  */
> @@ -593,6 +594,11 @@ static inline void xpsr_write(CPUARMState *env,
> uint32_t val, uint32_t mask)
>  #define SCR_RES1_MASK (3U << 4)
>  #define SCR_MASK      (0x3fff & ~SCR_RES1_MASK)
>
> +#define NSACR_NSTRCDIS (1U << 20)
> +#define NSACR_RFR      (1U << 19)
> +#define NSACR_NSASEDIS (1U << 15)
> +#define NSACR_NSD32DIS (1U << 14)
> +
>  /* Return the current FPSCR value.  */
>  uint32_t vfp_get_fpscr(CPUARMState *env);
>  void vfp_set_fpscr(CPUARMState *env, uint32_t val);
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index f6ff4aa..9671f9f 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -489,7 +489,19 @@ static void cpacr_write(CPUARMState *env, const
> ARMCPRegInfo *ri,
>              /* VFP coprocessor: cp10 & cp11 [23:20] */
>              mask |= (1 << 31) | (1 << 30) | (0xf << 20);
>
> -            if (!arm_feature(env, ARM_FEATURE_NEON)) {
> +            if (arm_feature(env, ARM_FEATURE_NEON)) {
> +                /* NSACR can disable non-secure writes to
> +                 * ASEDIS [31] or D32DIS [30]
> +                 */
> +                if (arm_feature(env, ARM_FEATURE_EL3) &&
> !arm_is_secure(env)) {
> +                    if ((env->cp15.c1_nsacr & NSACR_NSASEDIS)) {
> +                        mask &= ~(1 << 31);
> +                    }
> +                    if ((env->cp15.c1_nsacr & NSACR_NSD32DIS)) {
> +                        mask &= ~(1 << 30);
> +                    }
> +                }
> +            } else {
>                  /* ASEDIS [31] bit is RAO/WI */
>                  value |= (1 << 31);
>              }
> @@ -501,6 +513,7 @@ static void cpacr_write(CPUARMState *env, const
> ARMCPRegInfo *ri,
>                      !arm_feature(env, ARM_FEATURE_VFP3)) {
>                  /* D32DIS [30] is RAO/WI if D16-31 are not implemented. */
>                  value |= (1 << 30);
> +                mask |= (1 << 30);
>              }
>          }
>          value &= mask;
> @@ -2184,6 +2197,55 @@ static void scr_write(CPUARMState *env, const
> ARMCPRegInfo *ri, uint64_t value)
>      raw_write(env, ri, value);
>  }
>
> +static void nsacr_write(CPUARMState *env, const ARMCPRegInfo *ri,
> +                      uint64_t value)
> +{
> +    uint32_t mask = 0;
> +
> +    /* Pre ARMv8 some bits are RAO or UNK/SBZP */
> +    if (!arm_feature(env, ARM_FEATURE_V8)) {
> +
> +        if (arm_feature(env, ARM_FEATURE_VFP)) {
> +            mask |= NSACR_NSASEDIS | NSACR_NSD32DIS;
> +
> +            if (!arm_feature(env, ARM_FEATURE_NEON)) {
> +                /* NSASEDIS are RAO/WI */
> +                value |= NSACR_NSASEDIS;
> +            }
> +
> +            /* VFPv3 and upwards with NEON implement 32 double precision
> +             * registers (D0-D31).
> +             */
> +            if (!arm_feature(env, ARM_FEATURE_NEON) ||
> +                    !arm_feature(env, ARM_FEATURE_VFP3)) {
> +                /* NSD32DIS is RAO/WI if D16-31 are not implemented. */
> +                value |= NSACR_NSD32DIS;
> +            }
> +        }
> +
> +        /* cpn bits [13:0] */
> +        mask = 0x3fff;
> +
> +        value &= mask;
> +    }
> +
> +    raw_write(env, ri, value);
> +}
> +
> +static uint64_t nsacr_read(CPUARMState *env, const ARMCPRegInfo *ri)
> +{
> +    uint64_t ret = raw_read(env, ri);
> +
> +    if (arm_feature(env, ARM_FEATURE_V8)) {
> +        if (!arm_feature(env, ARM_FEATURE_EL3) || (
> +                arm_el_is_aa64(env, 3) && !is_a64(env) &&
> +                arm_current_pl(env) != 3)) {
> +            ret = 0x0000C00;
> +        }
>

It appears we are missing a case where 0xc00 is returned because we check
for the non-existence of EL3.

+    }
> +    return ret;
> +}
> +
>

The ARMv8 spec suggests that if EL3 is aarch64 that a read or write of this
register occurs from secure EL1 when aarch32 it is trapped as an exception
to EL3.  Is this omitted?


>  static const ARMCPRegInfo v8_el3_cp_reginfo[] = {
>      { .name = "ELR_EL3", .state = ARM_CP_STATE_AA64,
>        .type = ARM_CP_NO_MIGRATE,
> @@ -2217,6 +2279,10 @@ static const ARMCPRegInfo security_cp_reginfo[] = {
>      { .name = "SCR", .cp = 15, .crn = 1, .crm = 1, .opc1 = 0, .opc2 = 0,
>        .access = PL3_RW, .fieldoffset = offsetof(CPUARMState,
> cp15.scr_el3),
>        .resetvalue = 0, },
> +    { .name = "NSACR", .cp = 15, .crn = 1, .crm = 1, .opc1 = 0, .opc2 = 2,
> +      .access = PL3_RW | PL1_R, .resetvalue = 0,
> +      .writefn = nsacr_write, .readfn = nsacr_read,
> +      .fieldoffset = offsetof(CPUARMState, cp15.c1_nsacr) },
>      REGINFO_SENTINEL
>  };
>
> --
> 1.8.3.2
>
>

[-- Attachment #2: Type: text/html, Size: 7747 bytes --]