| Message ID | 84sj2s75yv.wl%peter@chubb.wattle.id.au |
|---|---|
| State | New |
| Headers | show |
On 15 April 2013 05:50, Peter Chubb <peter.chubb@nicta.com.au> wrote: > > > Since patch > 81465888c5306cd94abb9847e560796fd13d3c2f > target-arm: factor out handling of SRS instruction > the SRS instruction has not worked in QEMU. > > The problem is a return directive that was removed in the > refactoring, so after decoding the instruction, qemu would fall > through to do stuff that it should not have done. Nice catch for the ARM decoder, but not needed for thumb2 I think? thanks -- PMM
>>>>> "Peter" == Peter Maydell <peter.maydell@linaro.org> writes: Peter> On 15 April 2013 05:50, Peter Chubb <peter.chubb@nicta.com.au> Peter> wrote: >> >> >> Since patch 81465888c5306cd94abb9847e560796fd13d3c2f target-arm: >> factor out handling of SRS instruction the SRS instruction has not >> worked in QEMU. >> >> The problem is a return directive that was removed in the >> refactoring, so after decoding the instruction, qemu would fall >> through to do stuff that it should not have done. Peter> Nice catch for the ARM decoder, but not needed for thumb2 I Peter> think? It was there in the code that was removed. I didn't analyse too deeply, as nothing we do uses the thumb version. Peter C -- Dr Peter Chubb peter.chubb AT nicta.com.au http://www.ssrg.nicta.com.au Software Systems Research Group/NICTA
On 15 April 2013 08:29, Peter Chubb <peter.chubb@nicta.com.au> wrote: >>>>>> "Peter" == Peter Maydell <peter.maydell@linaro.org> writes: > Peter> On 15 April 2013 05:50, Peter Chubb <peter.chubb@nicta.com.au> > Peter> wrote: >>> The problem is a return directive that was removed in the >>> refactoring, so after decoding the instruction, qemu would fall >>> through to do stuff that it should not have done. > > Peter> Nice catch for the ARM decoder, but not needed for thumb2 I > Peter> think? > > It was there in the code that was removed. No it wasn't, I just looked at the broken commit which is why I pointed it out. (Also you can see that the gen_rfe() above isn't followed by a return, so I checked code flow to be sure it was OK.) thanks -- PMM
diff --git a/target-arm/translate.c b/target-arm/translate.c index 35a21be..c870246 100644 --- a/target-arm/translate.c +++ b/target-arm/translate.c @@ -6762,6 +6762,7 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s) } ARCH(6); gen_srs(s, (insn & 0x1f), (insn >> 23) & 3, insn & (1 << 21)); + return; } else if ((insn & 0x0e50ffe0) == 0x08100a00) { /* rfe */ int32_t offset; @@ -8209,6 +8210,7 @@ static int disas_thumb2_insn(CPUARMState *env, DisasContext *s, uint16_t insn_hw /* srs */ gen_srs(s, (insn & 0x1f), (insn & (1 << 24)) ? 1 : 2, insn & (1 << 21)); + return; } } else { int i, loaded_base = 0;
Since patch 81465888c5306cd94abb9847e560796fd13d3c2f target-arm: factor out handling of SRS instruction the SRS instruction has not worked in QEMU. The problem is a return directive that was removed in the refactoring, so after decoding the instruction, qemu would fall through to do stuff that it should not have done. Signed-off-by: Peter Chubb <peter.chubb@nicta.com.au>