[otbn] Fix BN.LID when the destination WDR is read from x1

The problem comes up with an instruction like:

      bn.lid x1, 0(x0)

The previous code read both input GPRs to BN.LID (x1 and x0) on the
first cycle. This is good for the base address, but is the wrong thing
for the destination address, which is needed on the following cycle.

Note that the behaviour was technically wrong for something like

      bn.lid x2, 0(x0)

as well, but didn't cause a bug because the base register file doesn't
factor the read-enable in unless we're reading from the call stack.

To see the bug, run the following example code:

      addi x1, x0, 1
      bn.lid x1, 0(x0)
      ecall

    .section .data
    .word 0x00000000
    .word 0x00000001
    .word 0x00000002
    .word 0x00000003
    .word 0x00000004
    .word 0x00000005
    .word 0x00000006
    .word 0x00000007

If that is saved as bug.s, assemble and link it with

    hw/ip/otbn/util/otbn-as -o bug.o bug.s
    hw/ip/otbn/util/otbn-ld -o bug bug.o

Next, build a Verilator-based simulation with:

    fusesoc --cores-root=. run --target=sim --setup --build lowrisc:ip:otbn_top_sim

and then run it with:

    build/lowrisc_ip_otbn_top_sim_0.1/sim-verilator/Votbn_top_sim -t --load-elf=bug

Signed-off-by: Rupert Swarbrick <rswarbrick@lowrisc.org>

hw/ip/otbn/rtl/otbn_controller.sv

diff --git a/hw/ip/otbn/rtl/otbn_controller.sv b/hw/ip/otbn/rtl/otbn_controller.sv
index 7b5d1d4..b0d268b 100644
--- a/hw/ip/otbn/rtl/otbn_controller.sv
+++ b/hw/ip/otbn/rtl/otbn_controller.sv
@@ -444,11 +444,17 @@
     rf_base_rd_en_b_o   = 1'b0;
 
     if (insn_valid_i) begin
-      if (insn_dec_shared_i.ld_insn || insn_dec_shared_i.st_insn) begin
-        // For loads and stores the read happens in the same cycle as the request because the read
-        // is used to form the request (the address and data if executing a store).
+      if (insn_dec_shared_i.st_insn) begin
+        // For stores, both base reads happen in the same cycle as the request because they give the
+        // address and data, which make up the request.
         rf_base_rd_en_a_o   = insn_dec_base_i.rf_ren_a & (lsu_load_req_raw | lsu_store_req_raw);
         rf_base_rd_en_b_o   = insn_dec_base_i.rf_ren_b & (lsu_load_req_raw | lsu_store_req_raw);
+      end else if (insn_dec_shared_i.ld_insn) begin
+        // For loads, the A read happens in the same cycle as the request, giving the address from
+        // which to load. The B read is only used for BN.LID and should take place when the
+        // instruction is unstalled, giving the index of the register to write the result to.
+        rf_base_rd_en_a_o   = insn_dec_base_i.rf_ren_a & (lsu_load_req_raw | lsu_store_req_raw);
+        rf_base_rd_en_b_o   = insn_dec_base_i.rf_ren_b & ~stall;
       end else begin
         // For all other instructions the read happens when the instruction is unstalled.
         rf_base_rd_en_a_o   = insn_dec_base_i.rf_ren_a & ~stall;