On Tue, Mar 13, 2012 at 3:45 PM, Peter Maydell <peter.mayd...@linaro.org> wrote: > Decode the SETEND instruction correctly in Thumb mode, > rather than accidentally treating it like CPS. We don't > support BE8 mode, but this change brings the Thumb mode > in to line with behaviour in ARM mode: 'SETEND BE' is > not supported and will provoke an UNDEF exception, but > 'SETEND LE' is correctly handled as a no-op. > > Signed-off-by: Peter Maydell <peter.mayd...@linaro.org> > Reported-by: Daniel Forsgren <daniel.forsg...@enea.com>
The patch looks good to me; I guess this would qualify for a: Reviewed-by: Laurent Desnogues <laurent.desnog...@gmail.com> The only point I dislike isn't directly related to this patch: the use of illegal_op that behaves exactly as UNDEF looks odd. Laurent > --- > This is one of those patches where I wasn't sure whether to try > to split it into a whitespace-only part and a significant-change > part. Most of this is (a) indenting existing code another notch > for the extra switch statement and (b) adding braces to placate > checkpatch. > > target-arm/translate.c | 63 ++++++++++++++++++++++++++++++----------------- > 1 files changed, 40 insertions(+), 23 deletions(-) > > diff --git a/target-arm/translate.c b/target-arm/translate.c > index 280bfca..3196619 100644 > --- a/target-arm/translate.c > +++ b/target-arm/translate.c > @@ -9704,32 +9704,49 @@ static void disas_thumb_insn(CPUState *env, > DisasContext *s) > store_reg(s, rd, tmp); > break; > > - case 6: /* cps */ > - ARCH(6); > - if (IS_USER(s)) > + case 6: > + switch ((insn >> 5) & 7) { > + case 2: > + /* setend */ > + ARCH(6); > + if (insn & (1 << 3)) { > + /* BE8 mode not implemented. */ > + goto illegal_op; > + } > break; > - if (IS_M(env)) { > - tmp = tcg_const_i32((insn & (1 << 4)) != 0); > - /* FAULTMASK */ > - if (insn & 1) { > - addr = tcg_const_i32(19); > - gen_helper_v7m_msr(cpu_env, addr, tmp); > - tcg_temp_free_i32(addr); > + case 3: > + /* cps */ > + ARCH(6); > + if (IS_USER(s)) { > + break; > } > - /* PRIMASK */ > - if (insn & 2) { > - addr = tcg_const_i32(16); > - gen_helper_v7m_msr(cpu_env, addr, tmp); > - tcg_temp_free_i32(addr); > + if (IS_M(env)) { > + tmp = tcg_const_i32((insn & (1 << 4)) != 0); > + /* FAULTMASK */ > + if (insn & 1) { > + addr = tcg_const_i32(19); > + gen_helper_v7m_msr(cpu_env, addr, tmp); > + tcg_temp_free_i32(addr); > + } > + /* PRIMASK */ > + if (insn & 2) { > + addr = tcg_const_i32(16); > + gen_helper_v7m_msr(cpu_env, addr, tmp); > + tcg_temp_free_i32(addr); > + } > + tcg_temp_free_i32(tmp); > + gen_lookup_tb(s); > + } else { > + if (insn & (1 << 4)) { > + shift = CPSR_A | CPSR_I | CPSR_F; > + } else { > + shift = 0; > + } > + gen_set_psr_im(s, ((insn & 7) << 6), 0, shift); > } > - tcg_temp_free_i32(tmp); > - gen_lookup_tb(s); > - } else { > - if (insn & (1 << 4)) > - shift = CPSR_A | CPSR_I | CPSR_F; > - else > - shift = 0; > - gen_set_psr_im(s, ((insn & 7) << 6), 0, shift); > + break; > + default: > + goto undef; > } > break; > > -- > 1.7.1 > >