search

SpinalHDL / NaxRiscv

MIT License
249 stars 39 forks source link

The check of the register Rd/Rs is not equal to zero is missing for some compressed instructions #114

Closed Bill94l closed 3 weeks ago

Bill94l commented 1 month ago

Hi,

I have generated random code with unaligned instructions enabled. During code execution, an unaligned jump causes an illegal compressed instruction to be executed. The opcode of the illegal instruction is 0x401a, it consists of c.lwsp with a target register rd=0, which is illegal according to the specification (RD must be different from zero)

clwsp

The execution of this instruction causes a trap_illegal_instruction on the spike side, but the DUT continues to execute the instruction because the target register is not checked for non-zero, and tries to load at address x80000000 + 132 = 0x80000084, causing a trap_load_page_fault.

Here is the execution result

[info] *** DUT code missmatch DUT=d REF=2 ***

Dump

    80000298:   00000e97            auipc   t4,0x0
    8000029c:   2bee8e93            addi    t4,t4,702 # 80000556 <sub_1>
    .....
    .....
    .....
00000000800002c2 <main_j2>:
    800002c2:   edee8367            jalr    t1,-290(t4)
    .....
    .....
    .....
0000000080000556 <sub_1>:
    80000556:   401a5dbb            sraw    s11,s4,ra

Spike log

core   0: 0x00000000000002c2 (0xedee8367) jalr    t1, t4, -290
core   0: 0 0x00000000000002c2 (0xedee8367) x 6 0x00000000000002c6
core   0: 0x0000000000000558 (0x0000401a) c.lwsp  zero, 132(sp)
core   0: exception trap_illegal_instruction, epc 0x0000000000000558
core   0:           tval 0x000000000000401a

Tracer log

rv rf w 0 0 32 00000000000002c6
rv commit 0 00000000000002c2
rv load flu 0
rv load flu 0
rv trap 0 0 13 2147483780

To fix this, we added checking the target register to see if it is different from zero

fix_issue_clwsp

wave

wavz_clwsp

Here are the output signals after the correction, we can clearly see that the instruction is considered illegal.

The signals and logs after the correction also show that the tval of the trap in the spike is 0x0000401A and in the DUT is 0x8931401A because the mask is not applied as you mentioned (TODO)

https://github.com/SpinalHDL/NaxRiscv/blob/bc18f667c11ed05ddbc248dc90e255d4c16798ad/src/main/scala/naxriscv/frontend/DecompressorPlugin.scala#L51-L52

According to the riscv-compressed-spec, there are several compressed instructions that require a check that the RS and RD registers are different from zero, and these registers are not checked in the DUT, so there are other checks to be done for other instructions.

spec_compressed_riscv

Dolu1990 commented 1 month ago

Hi,

Hoo right. Hmm would you feel ok to send a PR on github ?

Thanks ^^

Dolu1990 commented 3 weeks ago

Hi, I think it can be closed, as the PR with compressed instruction fix is merged ? Or there is something else pending ?