On 6 November 2014 15:51, Greg Bellows <[email protected]> wrote: > From: Fabian Aggeler <[email protected]> > > Add checks of SCR AW/FW bits when performing writes of CPSR. These SCR bits > are used to control whether the CPSR masking bits can be adjusted from > non-secure state. > > Signed-off-by: Fabian Aggeler <[email protected]> > Signed-off-by: Greg Bellows <[email protected]> > > --- > > v8 -> v9 > - Move cpsr_write mask filtering above mode switch. > - Replace conditional checks removed in v8. > > v7 -> v8 > - Fixed incorrect use of env->uncached_cpsr A/I/F to use env->daif instead. > - Removed incorrect statement about SPSR to CPSR copies being affected by > SCR.AW/FW. > - Fix typo in comment. > - Simpified cpsr_write logic > > v3 -> v4 > - Fixed up conditions for ignoring CPSR.A/F updates by isolating to v7 and > checking for the existence of EL3 and non-existence of EL2. > --- > target-arm/helper.c | 59 > +++++++++++++++++++++++++++++++++++++++++++++++++++-- > 1 file changed, 57 insertions(+), 2 deletions(-) > > diff --git a/target-arm/helper.c b/target-arm/helper.c > index 948192b..9186fc7 100644 > --- a/target-arm/helper.c > +++ b/target-arm/helper.c > @@ -3644,6 +3644,8 @@ uint32_t cpsr_read(CPUARMState *env) > > void cpsr_write(CPUARMState *env, uint32_t val, uint32_t mask) > { > + uint32_t changed_daif; > + > if (mask & CPSR_NZCV) { > env->ZF = (~val) & CPSR_Z; > env->NF = val; > @@ -3666,8 +3668,57 @@ void cpsr_write(CPUARMState *env, uint32_t val, > uint32_t mask) > env->GE = (val >> 16) & 0xf; > } > > - env->daif &= ~(CPSR_AIF & mask); > - env->daif |= val & CPSR_AIF & mask; > + /* In a V7 implementation that includes the security extensions but does > + * not include Virtualization Extensions the SCR.FW and SCR.AW bits > control > + * whether non-secure software is allowed to change the CPSR_F and CPSR_A > + * bits respectively. > + * > + * In a V8 implementation, it is permitted for privileged software to > + * change the CPSR A/F bits regardless of the SCR.AW/FW bits. > + */ > + if (!arm_feature(env, ARM_FEATURE_V8) && > + arm_feature(env, ARM_FEATURE_EL3) && > + !arm_feature(env, ARM_FEATURE_EL2) && > + !arm_is_secure(env)) { > + > + changed_daif = (env->daif ^ val) & mask; > + > + if (changed_daif & CPSR_A) { > + /* Check to see if we are allowed to change the masking of async > + * abort exceptions from a non-secure state. > + */ > + if (!(env->cp15.scr_el3 & SCR_AW)) { > + qemu_log_mask(LOG_GUEST_ERROR, > + "Ignoring attempt to switch CPSR_A flag from " > + "non-secure world with SCR.AW bit clear\n"); > + mask &= ~CPSR_A; > + } > + } > + > + if (changed_daif & CPSR_F) { > + /* Check to see if we are allowed to change the masking of FIQ > + * exceptions from a non-secure state. > + */ > + if (!(env->cp15.scr_el3 & SCR_FW)) { > + qemu_log_mask(LOG_GUEST_ERROR, > + "Ignoring attempt to switch CPSR_F flag from " > + "non-secure world with SCR.FW bit clear\n"); > + mask &= ~CPSR_F; > + } > + > + /* Check whether non-maskable FIQ (NMFI) support is enabled. > + * If this bit is set software is not allowed to mask > + * FIQs, but is allowed to set CPSR_F to 0. > + */ > + if ((A32_BANKED_CURRENT_REG_GET(env, sctlr) & SCTLR_NMFI) && > + (val & CPSR_F)) { > + qemu_log_mask(LOG_GUEST_ERROR, > + "Ignoring attempt to enable CPSR_F flag " > + "(non-maskable FIQ [NMFI] support enabled)\n"); > + mask &= ~CPSR_F; > + } > + } > + } > > if ((env->uncached_cpsr ^ val) & mask & CPSR_M) { > if (bad_mode_switch(env, val & CPSR_M)) { > @@ -3680,6 +3731,10 @@ void cpsr_write(CPUARMState *env, uint32_t val, > uint32_t mask) > switch_mode(env, val & CPSR_M); > } > } > + > + env->daif &= ~(CPSR_AIF & mask); > + env->daif |= val & CPSR_AIF & mask; > +
Was moving this bit below the setting of the mode flags deliberate? I don't see any reason why it has to go here. If you move it back where it was then you can add my r-b tag. > mask &= ~CACHED_CPSR_BITS; > env->uncached_cpsr = (env->uncached_cpsr & ~mask) | (val & mask); > } thanks -- PMM
