Re: [Qemu-devel] [PATCH v4 15/33] target-arm: add NSACR register

[Greg Bellows]

That sounds like a valid check to perform.   Digging deeper, it seems there is some downstream FIXME code in translate.c that should account for cpacr_fpen indicating FP being disabled.

I'll plan to fix this code.

On 7 July 2014 04:40, Aggeler Fabian <address@hidden> wrote:

On 01 Jul 2014, at 01:09, address@hidden wrote:

> From: Fabian Aggeler <address@hidden>
>
> Implements NSACR register with corresponding read/write functions
> for ARMv7 and ARMv8.
>

Actually, in this patch we could add a check in cpu_get_tb_cpu_state() (cpu.h) to not set
the ARM_TBFLAG_CPACR_FPEN_MASK if NSACR disables it.

What do you think?

> Signed-off-by: Sergey Fedorov <address@hidden>
> Signed-off-by: Fabian Aggeler <address@hidden>
> Signed-off-by: Greg Bellows <address@hidden>
> ---
> 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 1e8d5ee..4625088 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.  */
> @@ -609,6 +610,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 e43545a..6342dbf 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;
> @@ -2195,6 +2208,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;
> +        }
> +    }
> +    return ret;
> +}
> +
> static const ARMCPRegInfo v8_el3_cp_reginfo[] = {
>     { .name = "ELR_EL3", .state = ARM_CP_STATE_AA64,
>       .type = ARM_CP_NO_MIGRATE,
> @@ -2228,6 +2290,10 @@ static const ARMCPRegInfo v7_el3_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
>
>
>