On 13 May 2014 11:16, Fabian Aggeler <aggel...@ethz.ch> wrote:

> Some of SCTRL bits are common for secure and non-secure state.
> Translation table base masks are updated on NS-bit switch as
> well as on TTBCR writes.
>
> Signed-off-by: Sergey Fedorov <s.fedo...@samsung.com>
> Signed-off-by: Fabian Aggeler <aggel...@ethz.ch>
> ---
>  target-arm/cpu.h    | 10 ++++++++++
>  target-arm/helper.c | 39 ++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 48 insertions(+), 1 deletion(-)
>
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index c20c44a..7893004 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -425,6 +425,14 @@ int arm_cpu_handle_mmu_fault(CPUState *cpu, vaddr
> address, int rw,
>  #define SCTLR_AFE     (1U << 29)
>  #define SCTLR_TE      (1U << 30)
>
> +/* Bitmask for banked bits (Security Extensions) */
> +#define SCTLR_BANKED    (SCTLR_TE | SCTLR_AFE | SCTLR_TRE | SCTLR_EE | \
> +        SCTLR_VE | SCTLR_HA | SCTLR_UWXN | SCTLR_WXN | SCTLR_V | \
> +        SCTLR_I | SCTLR_Z | SCTLR_SW | SCTLR_CP15BEN | SCTLR_C | \
> +        SCTLR_A | SCTLR_M)
> +/* Treat bits that are not explicitly banked as common */
> +#define SCTLR_COMMON (~SCTLR_BANKED)
> +
>  #define CPSR_M (0x1fU)
>  #define CPSR_T (1U << 5)
>  #define CPSR_F (1U << 6)
> @@ -662,6 +670,8 @@ static inline int arm_feature(CPUARMState *env, int
> feature)
>      return (env->features & (1ULL << feature)) != 0;
>  }
>
> +#define SCR_NS (1U << 0)
> +
>  /* Return true if the processor is in secure state */
>  static inline bool arm_is_secure(CPUARMState *env)
>  {
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index c76a86b..618fd31 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -2165,12 +2165,49 @@ static void sctlr_write(CPUARMState *env, const
> ARMCPRegInfo *ri,
>      }
>
>      raw_write(env, ri, value);
> +
> +    if (!arm_el_is_aa64(env, 3)) {
> +        /* Aarch32 has some bits that are common to secure (mapped to
> +         * SCTLR_EL3) and non-secure instances of the SCTLR. */
> +
> +        env->cp15.c1_sys_el1 &= SCTLR_BANKED |
> +                (arm_current_sctlr(env) & SCTLR_COMMON);
> +        env->cp15.c1_sys_el3 &= SCTLR_BANKED |
> +                (arm_current_sctlr(env) & SCTLR_COMMON);
> +    }
> +
>      /* ??? Lots of these bits are not implemented.  */
>      /* This may enable/disable the MMU, so do a TLB flush.  */
>      tlb_flush(CPU(cpu), 1);
>  }
>
>  #ifndef CONFIG_USER_ONLY
> +static void scr_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t
> value)
> +{
> +    if (!arm_el_is_aa64(env, 3) &&
> +            (value & SCR_NS) != (env->cp15.c1_scr & SCR_NS)) {
> +        /* If EL3 is using Aarch32 switching NS-bit may make the CPU use a
> +         * different instance (secure or non-secure) when accessing CP
> +         * registers.
> +         * Common bits of otherwise banked registers need to by
> synchronized
> +         * at this point.
> +         */
> +        env->cp15.c1_sys_el1 &= SCTLR_BANKED |
> +                (arm_current_sctlr(env) & SCTLR_COMMON);
> +        env->cp15.c1_sys_el3 &= SCTLR_BANKED |
> +                (arm_current_sctlr(env) & SCTLR_COMMON);
>

I must be missing something, if the common bits are sync'd across the banks
in sctlr_write(), why do we need to do it here?


> +    }
> +
> +    env->cp15.c1_scr = value;
> +
> +    /* After possible switch, calculate c2_mask and c2_base_mask again
> for the
> +     * instance which is now active (secure or non-secure).
> +     */
> +    int maskshift = extract32(A32_BANKED_REG_GET(env, c2_control), 0, 3);
> +    env->cp15.c2_mask = ~(((uint32_t)0xffffffffu) >> maskshift);
> +    env->cp15.c2_base_mask = ~((uint32_t)0x3fffu >> maskshift);
>

As mentioned elsewhere, does it make sense to bank the c2_mask fields so
they stay in sync with c2_control?  Presumably we would not need to do
these updates in this case.


> +
> +}
>
>  static void nsacr_write(CPUARMState *env, const ARMCPRegInfo *ri,
>                        uint64_t value)
> @@ -2183,7 +2220,7 @@ static const ARMCPRegInfo tz_cp_reginfo[] = {
>  #ifndef CONFIG_USER_ONLY
>      { .name = "SCR", .cp = 15, .crn = 1, .crm = 1, .opc1 = 0, .opc2 = 0,
>        .access = PL3_RW, .fieldoffset = offsetof(CPUARMState, cp15.c1_scr),
> -      .resetvalue = 0, },
> +      .resetvalue = 0, .writefn = scr_write },
>      { .name = "SCR_EL3", .state = ARM_CP_STATE_AA64,
>        .opc0 = 3, .crn = 6, .crm = 1, .opc1 = 0, .opc2 = 0,
>        .access = PL3_RW, .fieldoffset = offsetof(CPUARMState, cp15.c1_scr),
> --
> 1.8.3.2
>
>
>

Reply via email to