Patchwork [v2] target-arm: Reinsert SRS missing return statement.

login
register
mail settings
Submitter Peter Chubb
Date April 15, 2013, 9:53 a.m.
Message ID <84mwt086ik.wl%peter@chubb.wattle.id.au>
Download mbox | patch
Permalink /patch/236551/
State New
Headers show

Comments

Peter Chubb - April 15, 2013, 9:53 a.m.
Since patch
   81465888c5306cd94abb9847e560796fd13d3c2f
   target-arm: factor out handling of SRS instruction
the SRS instruction has not worked in QEMU.

The problem is a missing return directive that was removed in the
refactoring, so after decoding the instruction, qemu would do all the
generic stuff that it should do for most instructions -- but not SRS.

Signed-off-by: Peter Chubb <peter.chubb@nicta.com.au>
---
 target-arm/translate.c |    1 +
 1 file changed, 1 insertion(+)
Peter Maydell - April 15, 2013, 10:08 a.m.
On 15 April 2013 10:53, 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 missing return directive that was removed in the
> refactoring, so after decoding the instruction, qemu would do all the
> generic stuff that it should do for most instructions -- but not SRS.

Actually, the problem is not doing generic stuff, but that
we hit the 'goto illegal_op' at the bottom of the if..elseif
ladder.

> Signed-off-by: Peter Chubb <peter.chubb@nicta.com.au>
> ---
>  target-arm/translate.c |    1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/target-arm/translate.c b/target-arm/translate.c
> index 35a21be..a1b7b8c 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;
> --
> 1.7.10.4

Code is ok, though:
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

I'll pull it into the target-arm tree and tweak the commit message.

-- PMM

Patch

diff --git a/target-arm/translate.c b/target-arm/translate.c
index 35a21be..a1b7b8c 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;