Misaligned JAL(R) RD register writeback expectations

[ultraembedded]

Hi,

If I've read this thread correctly;

riscv/riscv-isa-manual#269
@kasanovic says 'Misaligned address trap should be taken precisely, implying rd would not be updated.'

This appears to be not what the RVFI checkers are expecting when rvfi_trap is raised on an unaligned branch, as they barf on mismatching rd_addr/rd_wdata (if the DUT has indeed squashed the writeback to RD of the faulting JAL/JALR instructions).

Any thoughts?

[ultraembedded]

I guess this is effectively a duplicate of issue #26 so I’ll guess it will get same response!

[cliffordwolf]

This appears to be not what the RVFI checkers are expecting when rvfi_trap is raised on an unaligned branch, as they barf on mismatching rd_addr/rd_wdata (if the DUT has indeed squashed the writeback to RD of the faulting JAL/JALR instructions).

I don't think this is a dup of #26 because in #26 the core said it wrote a register when it should not, but you are saying the core reports that it does not write the destination register for a misaligned branch (as is the correct behavior) and the check is still complaining? That is weird.

Are you sure the riscv-formal spec agrees that the instruction should trap, i.e. is spec_trap high? Because from the little information you provided I would guess you have configured riscv-formal for a core with support for compressed instructions (RISCV_FORMAL_COMPRESSED), and then the core should never encounter an instruction address misaligned trap.

[ultraembedded]

The checker was in agreement that the trap should happen (spec_trap was high), however it also expects that the core continues to writeback to RD.
This is not the case with my core or the spike reference model, as the faulting instruction (a branch to a misaligned destination) should be stopped from causing architectural side effects and should fault instead (effectively the instruction would be left un-executed).
This would make it a precise fault, e.g. like a recoverable fault like a page fault.

I am not using compressed instructions.

I can’t see that the checker has enough information to disambiguate what to do on the various fault causes.

[cliffordwolf]

The checker was in agreement that the trap should happen (spec_trap was high), however it also expects that the core continues to writeback to RD.

I don't think this is true. Currently the insn checker only checks rd writeback when spec_trap is low:

riscv-formal/checks/rvfi_insn_check.sv

Lines 149 to 158 in 726255f

	if (!spec_trap) begin
	if (spec_rs1_addr != 0)
	assert(spec_rs1_addr == rs1_addr);
	if (spec_rs2_addr != 0)
	assert(spec_rs2_addr == rs2_addr);
	assert(spec_rd_addr == rd_addr);
	assert(spec_rd_wdata == rd_wdata);
	assert(`rvformal_addr_eq(spec_pc_wdata, pc_wdata));

Arguably it should check that rd_addr = 0 and rd_wdata = 0 when spec_trap is high, but right now it doesn't check rd writeback at all in case of a trap.