lowRISC/ibex

Store instructions do not always raise RV32E exceptions

mndstrmr opened this issue · 3 comments

Store instructions will be decoded with alu_op_b_mux_sel_o = OP_B_IMM:

ibex/rtl/ibex_decoder.sv

Lines 778 to 788 in 1449ed5

OPCODE_STORE: begin
alu_op_a_mux_sel_o = OP_A_REG_A;
alu_op_b_mux_sel_o = OP_B_REG_B;
alu_operator_o = ALU_ADD;
if (!instr_alu[14]) begin
// offset from immediate
imm_b_mux_sel_o = IMM_B_S;
alu_op_b_mux_sel_o = OP_B_IMM;
end
end

When RV32E = 1 an illegal instruction exception should be raised if any of the registers used by a given instruction are outside of the 0-15 range. In the case of the store instruction this should apply to both the storage address and the stored value.

The existing logic in ibex will not enforce this since alu_op_b_mux_sel_o = OP_B_IMM:

ibex/rtl/ibex_decoder.sv

Lines 179 to 183 in 1449ed5

if (RV32E) begin : gen_rv32e_reg_check_active
assign illegal_reg_rv32e = ((rf_raddr_a_o[4] & (alu_op_a_mux_sel_o == OP_A_REG_A)) |
(rf_raddr_b_o[4] & (alu_op_b_mux_sel_o == OP_B_REG_B)) |
(rf_waddr_o[4] & rf_we));
end else begin : gen_rv32e_reg_check_inactive

Therefore in RV32E mode a store instruction may attempt to store a value from registers 16-31, which is not well defined since the register file will not have those entries.

B-TYPE instructions, which have the same format, are likely unaffected since in their first cycle they do set alu_op_{a,b}_mux_sel_o = OP_{A,B}_REG_{A,B}.

Can you give me an example of an instruction that triggers this? I think the code that you quoted in the second snippet is trying to say "Don't read from a high register on A or B side, and don't write to a high register".

Is the problem that the RV32E spec doesn't allow immediate stores when the value is big? If so, I didn't know that was part of the spec. Can you give a reference?

The issue here is that for store instruction, both rs1 and rs2 are used. So if, say, we have sd x31, 0(x10), then it should be illegal instruction.

But since decoder sets alu_op_b_mux_sel_o to OP_B_IMM, the illegal_reg_rv32e check will pass because it doesn't check if rs2 is used, merely if the ALU uses rs2.

Oh, I see. Thanks for the explanation.