On 15.02.2016 20:22, Peter Maydell wrote:
> Raw CPSR writes should skip the architectural checks for whether
> we're allowed to set the A or F bits and should also not do
> the switching of register banks if the mode changes. Handle
> this inside cpsr_write(), which allows us to drop the "manually
> set the mode bits to avoid the bank switch" code from all the
> callsites which are using CPSRWriteRaw.
>
> This fixes a bug in 32-bit KVM handling where we had forgotten
> the "manually set the mode bits" part and could thus potentially
> trash the register state if the mode from the last exit to userspace
> differed from the mode on this exit.
>
> Signed-off-by: Peter Maydell <peter.mayd...@linaro.org>
Reviewed-by: Sergey Fedorov <serge.f...@gmail.com>
> ---
> target-arm/helper.c | 5 +++--
> target-arm/kvm64.c | 1 -
> target-arm/machine.c | 2 --
> target-arm/op_helper.c | 5 ++++-
> 4 files changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index 828822b..d1919bb 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -5234,7 +5234,7 @@ void cpsr_write(CPUARMState *env, uint32_t val,
> uint32_t mask,
> * 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) &&
> + if (write_type != CPSRWriteRaw && !arm_feature(env, ARM_FEATURE_V8) &&
> arm_feature(env, ARM_FEATURE_EL3) &&
> !arm_feature(env, ARM_FEATURE_EL2) &&
> !arm_is_secure(env)) {
> @@ -5281,7 +5281,8 @@ void cpsr_write(CPUARMState *env, uint32_t val,
> uint32_t mask,
> env->daif &= ~(CPSR_AIF & mask);
> env->daif |= val & CPSR_AIF & mask;
>
> - if ((env->uncached_cpsr ^ val) & mask & CPSR_M) {
> + if (write_type != CPSRWriteRaw &&
> + ((env->uncached_cpsr ^ val) & mask & CPSR_M)) {
> if (bad_mode_switch(env, val & CPSR_M)) {
> /* Attempt to switch to an invalid mode: this is UNPREDICTABLE.
> * We choose to ignore the attempt and leave the CPSR M field
> diff --git a/target-arm/kvm64.c b/target-arm/kvm64.c
> index 08c2c81..e8527bf 100644
> --- a/target-arm/kvm64.c
> +++ b/target-arm/kvm64.c
> @@ -722,7 +722,6 @@ int kvm_arch_get_registers(CPUState *cs)
> if (is_a64(env)) {
> pstate_write(env, val);
> } else {
> - env->uncached_cpsr = val & CPSR_M;
> cpsr_write(env, val, 0xffffffff, CPSRWriteRaw);
> }
>
> diff --git a/target-arm/machine.c b/target-arm/machine.c
> index 0fc7df0..03a73d9 100644
> --- a/target-arm/machine.c
> +++ b/target-arm/machine.c
> @@ -173,8 +173,6 @@ static int get_cpsr(QEMUFile *f, void *opaque, size_t
> size)
> return 0;
> }
>
> - /* Avoid mode switch when restoring CPSR */
> - env->uncached_cpsr = val & CPSR_M;
> cpsr_write(env, val, 0xffffffff, CPSRWriteRaw);
> return 0;
> }
> diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
> index 543d33a..4881e34 100644
> --- a/target-arm/op_helper.c
> +++ b/target-arm/op_helper.c
> @@ -779,7 +779,10 @@ void HELPER(exception_return)(CPUARMState *env)
>
> if (!return_to_aa64) {
> env->aarch64 = 0;
> - env->uncached_cpsr = spsr & CPSR_M;
> + /* We do a raw CPSR write because aarch64_sync_64_to_32()
> + * will sort the register banks out for us, and we've already
> + * caught all the bad-mode cases in el_from_spsr().
> + */
> cpsr_write(env, spsr, ~0, CPSRWriteRaw);
> if (!arm_singlestep_active(env)) {
> env->uncached_cpsr &= ~PSTATE_SS;
