Re: [Qemu-devel] [PULL 17/29] target/riscv: Convert quadrant 1 of RVXC insns to decodetree

[Bastian Koppelmann]

On 3/15/19 1:07 PM, Palmer Dabbelt wrote:
> On Fri, Mar 15, 2019 at 4:19 AM Palmer Dabbelt <address@hidden> wrote:
>
> > On Fri, 15 Mar 2019 02:06:07 PDT (-0700), Bastian Koppelmann wrote:
> > > Hi Alistair
> > >
> > > On 3/14/19 9:28 PM, Alistair Francis wrote:
> > > > On Wed, Mar 13, 2019 at 7:53 AM Palmer Dabbelt <address@hidden>
> > wrote:
> > > > > From: Bastian Koppelmann <address@hidden>
> > > > >
> > > > > Reviewed-by: Richard Henderson <address@hidden>
> > > > > Signed-off-by: Bastian Koppelmann <address@hidden>
> > > > > Signed-off-by: Peer Adelt <address@hidden>
> > > > This commit is the first bad commit in breaking 32-bit boot.
> > > >
> > > > It looks like the jal doesn't jump to the correct address:
> > > >
> > > > ----------------
> > > > IN:
> > > > 0x80000022:  00050433          add             s0,a0,zero
> > > > 0x80000026:  000584b3          add             s1,a1,zero
> > > > 0x8000002a:  2c79              jal             ra,670          #
> > 0x800002c8
> > > > ----------------
> > > > IN:
> > > > 0x800002c8:  00000533          add             a0,zero,zero
> > > > 0x800002cc:  8082              ret
> > >
> > > Oops! Can you point me to the binary to reproduce this?
> > I think I've traced it down to something simple: in my hello world binary
> > I see
> >
> >      20401a8c:       2a45                    jal     20401c3c <atexit>
> >
> > in the objdump, and I see
> >
> >      IN: _start
> >      0x20401a8c:  2a45              jal             ra,432          #
> > 0x20401c3c
> >
> > but then QEMU jumps to 0x20401a9d.  I have a feeling it's something wrong
> > with
> > gen_jal() that disappeared during the cleanups that we dropped in order to
> > fix
> > the build issues.
> >
> > I'm running
> >
> >      ./riscv32-softmmu/qemu-system-riscv32 -machine sifive_e -kernel
> > ~/work/sifive/freedom-e-sdk/software/hello/hello -nographic -d
> > in_asm,out_asm,exec,cpu -singlestep |& tee out.log
> >
> > on the "hifive1" branch of github.com/palmer-dabbelt/qemu, which just has
> > a
> > PRCI fixup that I forgot about and haven't sent upstream yet (I'll do that
> > after this issue).  The binary should be at
> >
> >      http://www.dabbelt.com/~palmer/hello.elf
> >
> > and the debug log at
> >
> >      http://www.dabbelt.com/~palmer/out.log
> >
> > You can build the binary from github.com/sifive/freedom-e-sdk via
> >
> >     make software PROGRAM=hello TARGET=sifive-hifive1
> >
> > using the riscv64-unknown-elf-gcc-20181127-x86_64-linux-ubuntu14 toolchain
> > binaries from our website (newer ones should work, but probably won't
> > produce
> > exactly the same output).
> >
> > I'll poke around after grabbing some dinner...
> OK, I'm pretty sure this is it:
>
> c_jal_addiw       001 .  .....  ..... 01 @ci #Note: parse rd and/or imm
> manually

Thanks for the digging. Yes this bug was fixed in the patches we dropped 
:/. Heres the fix:

diff --git a/target/riscv/insn_trans/trans_rvc.inc.c 
b/target/riscv/insn_trans/trans_rvc.inc.c
index bcdf64d3b7..5241a8414e 100644
--- a/target/riscv/insn_trans/trans_rvc.inc.c
+++ b/target/riscv/insn_trans/trans_rvc.inc.c
@@ -88,7 +88,9 @@ static bool trans_c_jal_addiw(DisasContext *ctx, 
arg_c_jal_addiw *a)
 {
 #ifdef TARGET_RISCV32
     /* C.JAL */
-    arg_jal arg = { .rd = 1, .imm = a->imm };
+    arg_c_j tmp;
+    extract_cj(&tmp, ctx->opcode);
+    arg_jal arg = { .rd = 1, .imm = tmp.imm };
     return trans_jal(ctx, &arg);
 #else
     /* C.ADDIW */

I'll send a patch as soon as I have checked the other cases that require 
manual parsing.

Cheers,

Bastian