[sw, otbn] Align OTBNsim with the latest MAI RTL implementation

hw/ip/otbn/dv/otbnsim/sim/mai_ispr.py

@@ -28,19 +28,20 @@ def on_start(self) -> None:
         super().on_start()
         # On start, the default operation is set.
         self._operation = MaiOperation.A2B
+        self._raw_op: int = int(MaiOperation.A2B)
         self._start_bit = False
         self._value = self._get_value()
 
-    def _construct_value(self, start_bit: bool, operation: MaiOperation) -> int:
-        '''Construct a register value based on a operation and start bit combination.'''
-        raw = (((operation & self.OPERATION_MASK) << self.OPERATION_OFFSET) |
-               (start_bit << self.START_BIT_OFFSET))
+    def _construct_value(self, start_bit: bool, raw_op: int) -> int:
+        '''Construct a register value from raw op bits and a start bit.'''
+        raw = (((raw_op & self.OPERATION_MASK) << self.OPERATION_OFFSET) |
+               (int(start_bit) << self.START_BIT_OFFSET))
         assert 0 <= raw < (1 << self.width)
         return raw
 
     def _get_value(self) -> int:
-        '''Construct the register value based on the current operation and start bit.'''
-        return self._construct_value(self._start_bit, self._operation)
+        '''Construct the register value based on the current raw op bits and start bit.'''
+        return self._construct_value(self._start_bit, self._raw_op)
 
     def _extract_start_bit(self, value: int) -> bool:
         '''Extract the start bit from a register value.'''
@@ -52,6 +53,10 @@ def _extract_operation(self, value: int) -> MaiOperation:
         # If the conversion failed, the check when updating the operation did fail already.
         return MaiOperation((value >> self.OPERATION_OFFSET) & self.OPERATION_MASK)
 
+    def _extract_raw_op(self, value: int) -> int:
+        '''Extract the raw (possibly invalid) operation bits from a register value.'''
+        return (value >> self.OPERATION_OFFSET) & self.OPERATION_MASK
+
     def _extract_fields(self, value: int) -> tuple[bool, MaiOperation]:
         '''Extract the fields from a register value.'''
         return self._extract_start_bit(value), self._extract_operation(value)
@@ -72,11 +77,31 @@ def _update_fields(self, start_bit: Optional[bool] = None,
 
     def commit(self) -> None:
         if self._next_value is not None:
-            self._start_bit, self._operation = self._extract_fields(self._next_value)
+            self._start_bit = self._extract_start_bit(self._next_value)
+            self._raw_op = self._extract_raw_op(self._next_value)
+            try:
+                self._operation = MaiOperation(self._raw_op)
+            except ValueError:
+                pass  # keep _operation as the last valid MaiOperation
             self._value = self._next_value
         self._next_value = None
         self._pending_write = False
 
+    def has_reserved_bits(self, value: int) -> bool:
+        '''Return True if value has any bits set outside the defined [5:0] field.
+
+        Mirrors RTL ispr_mai_sw_err.rsvd_csr_write: any write with bits [31:6] non-zero.
+        '''
+        valid_mask = (self.OPERATION_MASK << self.OPERATION_OFFSET) | self.START_BIT_MASK
+        return bool(value & ~valid_mask & 0xFFFFFFFF)
+
+    def is_raw_op_valid(self) -> bool:
+        '''Return True if the currently registered op field is a valid MaiOperation.
+
+        Mirrors RTL ma_mask_op_q validity check used by invalid_op detection.
+        '''
+        return self.is_valid_operation(self._raw_op << self.OPERATION_OFFSET)
+
     def is_start_bit_set(self) -> bool:
         '''Get the start bit from the CSR.'''
         return self._start_bit
@@ -106,11 +131,12 @@ def is_valid_operation(self, value: int) -> bool:
         except ValueError:
             return False
 
-    def would_change_op(self, value: int) -> bool:
-        '''Return whether writing value to the CSR would change the operation field.
-        The value to be checked must specify a valid operation option otherwise we crash.
+    def would_change_raw_op(self, value: int) -> bool:
+        '''Return whether writing value would change the op field (compares raw bits).
+
+        Safe to call with any value, including those with invalid op encodings.
         '''
-        return self._extract_operation(value) != self.current_operation()
+        return self._extract_raw_op(value) != self._raw_op
 
 
 class MaiStatusCSR(DumbISPR):

hw/ip/otbn/dv/otbnsim/sim/sim.py

@@ -7,7 +7,7 @@
 from .constants import ErrBits, LcTx, Status, read_lc_tx_t
 from .decode import EmptyInsn
 from .isa import OTBNInsn
-from .state import OTBNState, FsmState
+from .state import OTBNState, FsmState, WIPE_CYCLES
 from .stats import ExecutionStats
 from .trace import Trace
 
@@ -433,6 +433,10 @@ def _step_wiping(self, verbose: bool) -> StepRes:
         was_wiping = self.state.wipe_cycles > 0
         if was_wiping:
             self.state.wipe_cycles -= 1
+            # Step the URND Bivium every wipe cycle, matching the RTL where prim_trivium
+            # runs continuously. This keeps the Bivium in sync with the RTL so that the
+            # MAI's cnt bits captured in on_sec_wipe_zero_step() match in_cnt_load_val_q.
+            self.state.wsrs.URND.step()
 
         is_good = not self.state.lock_after_wipe
         locking = self.state.rma_req == LcTx.ON or not is_good
@@ -469,6 +473,29 @@ def _step_wiping(self, verbose: bool) -> StepRes:
         else:
             self._delayed_insn_cnt_zero(1)
 
+        if self.state.wipe_cycles == WIPE_CYCLES - 97:
+            # Corresponds to the RTL cycle where sec_wipe_mai_i (= sec_wipe_zero_o)
+            # fires, which is when the FSM enters OtbnStartStopSecureWipeAllZero after
+            # completing the three register-overwriting phases:
+            #
+            #   OtbnStartStopSecureWipeWdrUrnd:        33 cycles (32 WDRs + 1 ACC timing)
+            #   OtbnStartStopSecureWipeAccModBaseUrnd: 32 cycles (ACC, MOD, base regs)
+            #   OtbnStartStopSecureWipeExtIsprsUrnd:   32 cycles (MAI ISPRs + reserved)
+            #                                        = 97 URND-advancing cycles total
+            #
+            # The URND value seen by the MAI at that moment is not the raw prim_trivium
+            # output but the registered copy one cycle behind it (otbn_rnd.sv:
+            # urnd_data_q <= key_o, registered each posedge). So in_cnt_load_val_q
+            # captures the Bivium output from step 97, not step 98.
+            #
+            # In the ISS, _WIPE_CYCLES = 100 and wipe_cycles decrements once per step, so
+            # after 97 steps wipe_cycles = 3. At that point _next_value holds the step-97
+            # Bivium output, matching what the RTL latches into in_cnt_load_val_q.
+            final_wipe_round_cnt = (self.state.wipe_rounds_done ==
+                                    (self.state.wipe_rounds_to_do - 1))
+            if final_wipe_round_cnt:
+                self.state.mai.on_sec_wipe_zero_step()
+
         if self.state.wipe_cycles == 1:
             # This is the penultimate clock cycle of a wipe round. We want to
             # model the behaviour that we expect "at the end of a wipe round".

hw/ip/otbn/dv/otbnsim/sim/state.py

@@ -23,7 +23,7 @@
 # The number of cycles spent per round of a secure wipe. This takes constant
 # time in the RTL, mirrored here. The constant here needs to be incremented
 # by one compared to the constant found in RTL (`otbn_core_model.sv`)
-_WIPE_CYCLES = 99 + 1
+WIPE_CYCLES = 99 + 1
 
 
 class FsmState(IntEnum):
@@ -484,7 +484,7 @@ def set_fsm_state(self, new_state: FsmState) -> None:
         # the wiping operation itself will take.
         wiping_next = new_state == FsmState.WIPING
         if wiping_next:
-            self.wipe_cycles = _WIPE_CYCLES
+            self.wipe_cycles = WIPE_CYCLES
         self._next_fsm_state = new_state
 
     def set_flags(self, fg: int, flags: FlagReg) -> None:

hw/ip/otbn/dv/otbnsim/sim/wsr.py

@@ -148,6 +148,10 @@ def set_seed(self, value: int) -> None:
         # generate a keystream.
         self.running = True
 
+    def pending_value(self) -> int:
+        '''Return the Bivium output scheduled by step(), before commit() latches it.'''
+        return self._next_value
+
     def step(self) -> None:
         # Schedule an state update and readout the keystream.
         self._trivium.update()

hw/ip/otbn/dv/smoke/smoke_expected.txt

@@ -35,7 +35,7 @@ x26 | 0x00000016
 x27 | 0x0000001a
 x28 | 0x00400000
 x29 | 0x00018000
-x30 | 0x53bcb7d3
+x30 | 0x9ccf1600
 x31 | 0x00000804
 
 Final Bignum Register Values:
@@ -71,5 +71,5 @@ w26 | 0x78fccc06_2228e9d6_89c9b54f_887cf1ee_efbafabd_f9bfd9ee_baeebbbb_dbff9bfa
 w27 | 0x28a88802_00088990_8888a00a_88189108_828aa820_09981808_8822aa2a_11109898
 w28 | 0xd25666ac_bbb1704f_23631fe5_11e568d7_6d30528f_f027c1f7_32cc1191_caef0343
 w29 | 0x4f0d4b81_9f24f0c1_64341d3c_26628bdb_5763bcdf_63388709_e0654fef_eb0953c2
-w30 | 0x2167f87d_e9ee7ac7_ffa3d88b_ab123192_aee49292_4efa2ec9_b55098e0_68ba2fa1
-w31 | 0x37adadae_f9dbff5e_73880075_5466a52c_67a8c221_6978ad1b_25769434_0f09b7c8
+w30 | 0x0fa66f64_739babfd_463a6559_44ad0257_98ef0ef8_84628bf7_edbbd1d6_8b78539b
+w31 | 0x00000000_00000000_00000000_00000000_00000000_00000000_00000000_00000000

hw/ip/otbn/dv/smoke/smoke_test.s

@@ -372,17 +372,24 @@ bn.wsrr w13, MAI_IN0_S1
 bn.wsrr w14, MAI_IN1_S0
 bn.wsrr w15, MAI_IN1_S1
 
-# TODO: uncomment the code below once the OTBNsim model of MAI is aligned with the RTL implementation.
 # Execute SecAdd on MAI
-# li x30, 0x2f
-# csrrw x0, MAI_CTRL, x30
+li x30, 0x2f
+csrrw x0, MAI_CTRL, x30
 
 # Poll for completion of MAI execution
-# jal x1, mai_poll_busy
+jal x1, mai_poll_busy
 
 # Read back results from MAI
-# bn.wsrr w30, MAI_RES_S0
-# bn.wsrr w31, MAI_RES_S1
+bn.wsrr w30, MAI_RES_S0
+bn.wsrr w31, MAI_RES_S1
+# SecAdd result: in0[k] = w12[k] ^ w13[k], in1[k] = w14[k] ^ w15[k],
+# w30[k] = in0[k] + in1[k] mod 2^32 for each 32-bit coefficient k.
+# in0 = 0x8d09995b_664e0f38_de9ee010_e8d217a0_98ef0d70_8f12b880_edbbce4e_b598fc3d
+# in1 = 0x829cd609_0d4d9cc5_679b8549_5bdaeab7_00000188_f54fd377_00000388_d5df575e
+# w30 = in0 + in1 mod 2^32 per lane = 0x0fa66f64_739babfd_463a6559_44ad0257_98ef0ef8_84628bf7_edbbd1d6_8b78539b
+bn.xor w30, w30, w31
+# w31 = 0x00000000_00000000_00000000_00000000_00000000_00000000_00000000_00000000
+bn.xor w31, w31, w31
 
 # Read from URND in the OTBN Verilator smoke test and 0x0 when running on the FPGA or chip-level test
 .ifnotdef deterministic

sw/device/tests/otbn_isa_test.c

@@ -31,7 +31,7 @@ static const otbn_addr_t kWdrState = OTBN_ADDR_T_INIT(smoke_test, wdr_state);
 enum {
   kNumExpectedGprs = 30,
   kNumExpectedWdrs = 32,
-  kExpectedInstrCount = 299,
+  kExpectedInstrCount = 322,
 };
 
 // The expected values of the GPRs and WDRs are taken from
@@ -104,10 +104,10 @@ static const uint32_t kExpectedWdrs[kNumExpectedWdrs][8] = {
             0x23631fe5, 0xbbb1704f, 0xd25666ac},
     [29] = {0xeb0953c2, 0xe0654fef, 0x63388709, 0x5763bcdf, 0x26628bdb,
             0x64341d3c, 0x9f24f0c1, 0x4f0d4b81},
-    [30] = {0x68ba2fa1, 0xb55098e0, 0x4efa2ec9, 0xaee49292, 0xab123192,
-            0xffa3d88b, 0xe9ee7ac7, 0x2167f87d},
-    [31] = {0x0f09b7c8, 0x25769434, 0x6978ad1b, 0x67a8c221, 0x5466a52c,
-            0x73880075, 0xf9dbff5e, 0x37adadae},
+    [30] = {0x8b78539b, 0xedbbd1d6, 0x84628bf7, 0x98ef0ef8, 0x44ad0257,
+            0x463a6559, 0x739babfd, 0x0fa66f64},
+    [31] = {0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000,
+            0x00000000, 0x00000000, 0x00000000},
 };
 
 bool test_main(void) {