riscvarchive / riscv-qemu

QEMU with RISC-V (RV64G, RV32G) Emulation Support
384 stars 154 forks source link

si does not stop to next instruction if current instruction is csrr/csrw in remote debug #170

Open lbmeng opened 5 years ago

lbmeng commented 5 years ago

$ qemu-system-riscv64 -nographic -kernel bbl -M virt -s -S

Using 'si' to step one instruction, and when it executes to 0x1008, type 'si' the QEMU target should stop at 0x100c, however it jumps directoy to 0x1010.

0x0000000000001000 ? auipc t0,0x0 0x0000000000001004 ? addi a1,t0,32 0x0000000000001008 ? csrr a0,mhartid 0x000000000000100c ? ld t0,24(t0) 0x0000000000001010 ? jr t0

It looks the issue only occurs if current instruction is csrr or csrw, and 'si' will stop to the 2nd instruction after current location. For example, with the following sequence.

256 csrw mscratch, x0 257 258 # write mtvec and make sure it sticks 259 la t0, trap_vector 260 csrw mtvec, t0 261 csrr t1, mtvec 262 1:bne t0, t1, 1b 263 264 la sp, stacks + RISCV_PGSIZE - MENTRY_FRAME_SIZE

'si' at line 256 will jump to line 260, then 'si' will jump to line 264 (because 261 is csrr, so it skips one additional one instruction and stop at line 264).

jim-wilson commented 5 years ago

I can make this work with the attached patch, but I haven't convinced myself that it is correct yet.

csr-single-step.txt

The problem seems related to the use of DISAS_NORETURN for csr insns. It appears that almost everything else using DISAS_NORETURN is either generating an exception, or calling gen_goto_tb which checks the singlestep flag via use_goto_tb. The only exceptions are the csr insns, fence.i, and jalr. But single stepping does work for jalr, and it isn't clear why. The code looks similar, other than the fact that cpu_pc doesn't equal ctx->pc_succ_insn after jalr, so maybe that is being checked somewhere. Anyways, I can make single step work for csr insns with an explicit singlestep check, which doesn't look quite right, but does appear to work fine.

lbmeng commented 5 years ago

Thanks for the quick response. I tried the patch and it works.