From: Luke Kenneth Casson Leighton <lkcl@lkcl.net>
Date: Sat, 30 Apr 2022 12:52:00 +0000 (+0100)
Subject: clear out DEC in core.cur_state.dec due to spurious interrupt.
X-Git-Url: https://git.libre-soc.org/?p=soc.git;a=commitdiff_plain;h=972f7cdd33ed6da0b57b2c39cddea6b0640c0ebc

clear out DEC in core.cur_state.dec due to spurious interrupt.

this is slightly complicated.  the STATE regfile contains pc, msr, svstate,
dec, and tb, being a reflection of CoreState.  reading from STATE regfile
is on a one-clock delay. the DEC/TB FSM needs to decrement DEC and increment
TB, by reading from the STATE regfile and then writing a new value.

of course, the SPR pipeline has to get a word in edgeways as well.

but...

the complication comes in that it is the PowerDecoder2 which receives
a *cached* copy of DEC, and this cached copy is what has (up until now)
been out-of-date with what is in the STATE regfile.

the hack-job-solution is to zero-out the cached copy when the SPR pipeline
writes a new value to DEC.  the DEC/TB FSM will then rewrite a correct
value into it.

given that PowerDecoder2 only uses the MSB of DEC (and the EE bit of MSR)
to determine whether to fire an interrupt, this should be perfectly fine.
---

diff --git a/src/soc/fu/spr/main_stage.py b/src/soc/fu/spr/main_stage.py
index 3e236a28..b3a49cb6 100644
--- a/src/soc/fu/spr/main_stage.py
+++ b/src/soc/fu/spr/main_stage.py
@@ -56,7 +56,8 @@ class SPRMainStage(PipeModBase):
             #### MTSPR ####
             with m.Case(MicrOp.OP_MTSPR):
                 with m.Switch(spr):
-                    # State SPRs first
+                    # State SPRs first, note that this triggers a regfile write
+                    # which is monitored right the way down in TestIssuerBase.
                     with m.Case(SPR.DEC, SPR.TB):
                         comb += state1_o.data.eq(a_i)
                         comb += state1_o.ok.eq(1)
diff --git a/src/soc/fu/spr/pipe_data.py b/src/soc/fu/spr/pipe_data.py
index 5cc7835e..21db9582 100644
--- a/src/soc/fu/spr/pipe_data.py
+++ b/src/soc/fu/spr/pipe_data.py
@@ -28,6 +28,10 @@ class SPRInputData(FUBaseData):
         # convenience
         self.a = self.ra
 
+# note that state1 gets a corresponding "state1" write port created
+# by core.py which is "monitored" by TestIssuerBase (hack-job, sigh).
+# when writes are spotted then the DEC/TB FSM resets and re-reads
+# DEC/TB.
 
 class SPROutputData(FUBaseData):
     regspec = [('INT', 'o', '0:63'),        # RT
diff --git a/src/soc/simple/issuer.py b/src/soc/simple/issuer.py
index 0756e4b6..60948dcc 100644
--- a/src/soc/simple/issuer.py
+++ b/src/soc/simple/issuer.py
@@ -319,7 +319,7 @@ class TestIssuerBase(Elaboratable):
         # hack method of keeping an eye on whether branch/trap set the PC
         self.state_nia = self.core.regs.rf['state'].w_ports['nia']
         self.state_nia.wen.name = 'state_nia_wen'
-        # and whether SPR pipeline sets DEC or TB
+        # and whether SPR pipeline sets DEC or TB (fu/spr/main_stage.py)
         self.state_spr = self.core.regs.rf['state'].w_ports['state1']
 
         # pulse to synchronize the simulator at instruction end
@@ -573,6 +573,7 @@ class TestIssuerBase(Elaboratable):
         state_r_dectb = state_rf.r_ports['issue']  # DEC/TB
         state_w_dectb = state_rf.w_ports['issue']  # DEC/TB
 
+
         with m.FSM() as fsm:
 
             # initiates read of current DEC
@@ -1487,6 +1488,7 @@ class TestIssuerInternal(TestIssuerBase):
         sync = m.d.sync
         dbg = self.dbg
         pdecode2 = self.pdecode2
+        cur_state = self.cur_state
 
         # temporaries
         core_busy_o = core.n.o_data.busy_o  # core is busy
@@ -1512,18 +1514,26 @@ class TestIssuerInternal(TestIssuerBase):
 
             # instruction started: must wait till it finishes
             with m.State("INSN_ACTIVE"):
-                # note changes to MSR, PC and SVSTATE, and DEC/TB
-                # these last two are done together, and passed to the
-                # DEC/TB FSM
+                # note changes to MSR, PC and SVSTATE
                 with m.If(self.state_nia.wen & (1 << StateRegs.SVSTATE)):
                     sync += self.sv_changed.eq(1)
                 with m.If(self.state_nia.wen & (1 << StateRegs.MSR)):
                     sync += self.msr_changed.eq(1)
                 with m.If(self.state_nia.wen & (1 << StateRegs.PC)):
                     sync += self.pc_changed.eq(1)
-                with m.If((self.state_spr.wen &
-                          ((1 << StateRegs.DEC) | (1 << StateRegs.TB))).bool()):
+                # and note changes to DEC/TB, to be passed to DEC/TB FSM
+                with m.If(self.state_spr.wen & (1 << StateRegs.TB)):
+                    comb += self.pause_dec_tb.eq(1)
+                # but also zero-out the cur_state DEC so that, on
+                # the next instruction, if it is "enable interrupt"
+                # the delay between the DEC/TB FSM reading and updating
+                # cur_state.dec doesn't trigger a spurious interrupt.
+                # the DEC/TB FSM will read the regfile and update to
+                # the correct value, so having cur_state.dec set to zero
+                # for a while is no big deal.
+                with m.If(self.state_spr.wen & (1 << StateRegs.DEC)):
                     comb += self.pause_dec_tb.eq(1)
+                    sync += cur_state.dec.eq(0) # only needs top bit clear
                 with m.If(~core_busy_o):  # instruction done!
                     comb += exec_pc_o_valid.eq(1)
                     with m.If(exec_pc_i_ready):