On 24 October 2017 at 16:53, Peter Maydell <peter.mayd...@linaro.org> wrote: > On 21 October 2017 at 19:09, Stefano Stabellini <sstabell...@kernel.org> > wrote: >> diff --git a/target/arm/translate.c b/target/arm/translate.c >> index 4da1a4c..a89518f 100644 >> --- a/target/arm/translate.c >> +++ b/target/arm/translate.c >> @@ -12325,12 +12325,15 @@ static void arm_tr_tb_stop(DisasContextBase >> *dcbase, CPUState *cpu) >> /* nothing more to generate */ >> break; >> case DISAS_WFI: >> - gen_helper_wfi(cpu_env); >> + { >> + TCGv_i32 tmp = tcg_const_i32((dc->insn & (1U << 31)) ? 4 : 2); > > This won't work, because dc->insn is only populated by the translate-a64.c > A64 translator, not by the A32/T32 code. I guess 'principle of least > surprise' suggests we should populate it for Thumb and ARM too.
...and also, this code path is used for both A32 and Thumb instruction sets, so just looking at the top bit of dc->insn isn't sufficient, because you might have the A32 encoding with a cond field with the top bit clear. You can either look also at dc->thumb, or alternatively have a new field is_16bit in DisasContext which you populate in arm_tr_translate_insn() and thumb_tr_translate_insn(). thanks -- PMM