[Qemu-devel] [PATCH] target-arm: modifying pc in tcg code for load/store multiple

[Qemu-devel] [PATCH] target-arm: modifying pc in tcg code for load/store multiple

[ild]

From: Ildar Isaev <ild@inbox.ru>

pc wasn't modified in tcg code for load/store multiple,
causing translation block to be executed in infinite loop forever

Signed-off-by: Ildar Isaev <ild@inbox.ru>
---
 target-arm/translate.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target-arm/translate.c b/target-arm/translate.c
index 36868ed..622aa03 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -8973,7 +8973,7 @@ static void disas_arm_insn(DisasContext *s, unsigned int insn)
                     tmp = load_cpu_field(spsr);
                     gen_set_cpsr(tmp, CPSR_ERET_MASK);
                     tcg_temp_free_i32(tmp);
-                    s->is_jmp = DISAS_UPDATE;
+                    gen_lookup_tb(s);
                 }
             }
             break;
-- 
1.9.3

Re: [Qemu-devel] [PATCH] target-arm: modifying pc in tcg code for load/store multiple

[Peter Maydell]

On 19 February 2015 at 21:26,  <ild@inbox.ru> wrote:
> From: Ildar Isaev <ild@inbox.ru>
>
> pc wasn't modified in tcg code for load/store multiple,
> causing translation block to be executed in infinite loop forever
>
> Signed-off-by: Ildar Isaev <ild@inbox.ru>

It would be helpful if you gave an example of guest
code which we mishandle. Do you have a test case?

> ---
>  target-arm/translate.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/target-arm/translate.c b/target-arm/translate.c
> index 36868ed..622aa03 100644
> --- a/target-arm/translate.c
> +++ b/target-arm/translate.c
> @@ -8973,7 +8973,7 @@ static void disas_arm_insn(DisasContext *s, unsigned int insn)
>                      tmp = load_cpu_field(spsr);
>                      gen_set_cpsr(tmp, CPSR_ERET_MASK);
>                      tcg_temp_free_i32(tmp);
> -                    s->is_jmp = DISAS_UPDATE;
> +                    gen_lookup_tb(s);
>                  }
>              }
>              break;

This doesn't look right. What if the load-multiple loaded PC?
Calling gen_lookup_tb() will overwrite that.

-- PMM

Re: [Qemu-devel] [PATCH] target-arm: modifying pc in tcg code for load/store multiple

[Ildar Isaev]

[-- Attachment #1: Type: text/plain, Size: 1813 bytes --]




> On 19 February 2015 at 21:26,  <ild@inbox.ru> wrote:
> > From: Ildar Isaev <ild@inbox.ru>
> >
> > pc wasn't modified in tcg code for load/store multiple,
> > causing translation block to be executed in infinite loop forever
> >
> > Signed-off-by: Ildar Isaev <ild@inbox.ru>
> 
> It would be helpful if you gave an example of guest
> code which we mishandle. Do you have a test case?
> 

A bit clumsy, but something like that. Qemu never gets to the code past stmda.

-bash-4.1$ cat add.s


        .text
        mov   r0, #5         
        mov   r1, #4
        add   r2, r1, r0
        stmda sp, {r1, r2, r5, sp, lr, pc}^
        mov   r0, #26
        mov   r1, #30


-bash-4.1$ arm-linux-gnueabihf-as -o add.o add.s

-bash-4.1$ arm-linux-gnueabihf-ld -Ttext=0x80000000 -o add.elf add.o

-bash-4.1$ arm-linux-gnueabihf-objcopy -O binary add.elf add.bin

-bash-4.1$ dd if=/dev/zero of=test.bin bs=4096 count=4096

-bash-4.1$ dd if=add.bin of=test.bin bs=4096 conv=notrunc

-bash-4.1$ qemu-system-arm -M connex -pflash test.bin -nographic -serial /dev/null

QEMU 2.2.0 monitor - type 'help' for more information
(qemu) info registers
R00=00000005 R01=00000004 R02=00000009 R03=00000000
R04=00000000 R05=00000000 R06=00000000 R07=00000000
R08=00000000 R09=00000000 R10=00000000 R11=00000000
R12=00000000 R13=00000000 R14=00000000 R15=00000000
PSR=00000013 ---- A svc32
FPSCR: 00000000
(qemu) cont
(qemu) cont
(qemu) cont
(qemu) cont
(qemu) cont
(qemu) cont
(qemu) cont
(qemu) cont
(qemu) cont
(qemu) cont
(qemu) cont
(qemu) cont
(qemu) cont
(qemu) cont
(qemu) info registers
R00=00000005 R01=00000004 R02=00000009 R03=00000000
R04=00000000 R05=00000000 R06=00000000 R07=00000000
R08=00000000 R09=00000000 R10=00000000 R11=00000000
R12=00000000 R13=00000000 R14=00000000 R15=00000000
PSR=00000013 ---- A svc32

[-- Attachment #2: add.s --]
[-- Type: application/octet-stream, Size: 182 bytes --]



        .text
        mov   r0, #5         
        mov   r1, #4
        add   r2, r1, r0
        stmda sp, {r1, r2, r5, sp, lr, pc}^
        mov   r0, #26
        mov   r1, #30

Re: [Qemu-devel] [PATCH] target-arm: modifying pc in tcg code for load/store multiple

[Peter Maydell]

On 19 February 2015 at 16:04, Ildar Isaev <ild@inbox.ru> wrote:
> A bit clumsy, but something like that. Qemu never gets to the code past stmda.
>
> -bash-4.1$ cat add.s
>
>
>         .text
>         mov   r0, #5
>         mov   r1, #4
>         add   r2, r1, r0
>         stmda sp, {r1, r2, r5, sp, lr, pc}^
>         mov   r0, #26
>         mov   r1, #30

Oh, I see. You're doing a store-multiple user-registers
form with the PC in the register list. That's pretty weird,
because in that case the PC stored is the current PC
whereas the rest of the register are user-mode ones. QEMU
mishandles this because it misidentifies it as being some
kind of exception-return instruction (when in fact the
exception-return insns are only the LDM-user-regs-with-PC
encodings). This is a real bug, but your fix is wrong.
I'll send out a patch.

-- PMM