From c0ff85b97953c6fd92b285dd6efd6fdc864ca3cf Mon Sep 17 00:00:00 2001
From: Luke Kenneth Casson Leighton <lkcl@lkcl.net>
Date: Tue, 2 Mar 2021 13:55:17 +0000
Subject: [PATCH] operating correctly, not directing MMU SPRs to SPR Pipeline,
 failure with PC likely due to ISACaller not supporting SPR 720

---
 src/soc/decoder/power_decoder2.py | 54 +++++++++++++++++++++----------
 src/soc/fu/mmu/fsm.py             |  9 +++++-
 2 files changed, 45 insertions(+), 18 deletions(-)

diff --git a/src/soc/decoder/power_decoder2.py b/src/soc/decoder/power_decoder2.py
index 6b353081..6077aa73 100644
--- a/src/soc/decoder/power_decoder2.py
+++ b/src/soc/decoder/power_decoder2.py
@@ -713,7 +713,23 @@ class PowerDecodeSubset(Elaboratable):
         return subset
 
     def rowsubsetfn(self, opcode, row):
-        return row['unit'] == self.fn_name
+        """select per-Function-Unit subset of opcodes to be processed
+
+        normally this just looks at the "unit" column.  MMU is different
+        in that it processes specific SPR set/get operations that the SPR
+        pipeline should not.
+        """
+        return (row['unit'] == self.fn_name or
+                # sigh a dreadful hack: MTSPR and MFSPR need to be processed
+                # by the MMU pipeline so we direct those opcodes to MMU **AND**
+                # SPR pipelines, then selectively weed out the SPRs that should
+                # or should not not go to each pipeline, further down.
+                # really this should be done by modifying the CSV syntax
+                # to support multiple tasks (unit column multiple entries)
+                # see https://bugs.libre-soc.org/show_bug.cgi?id=310
+               (self.fn_name == 'MMU' and row['unit'] == 'SPR' and
+                row['internal op'] in ['OP_MTSPR', 'OP_MFSPR'])
+                )
 
     def ports(self):
         return self.dec.ports() + self.e.ports() + self.sv_rm.ports()
@@ -771,16 +787,8 @@ class PowerDecodeSubset(Elaboratable):
 
         # set up instruction type
         # no op: defaults to OP_ILLEGAL
-        if self.fn_name=="MMU":
-            # mmu is special case: needs SPR opcode as well
-            mmu0 = self.mmu0_spr_dec
-            with m.If(((mmu0.dec.op.internal_op == MicrOp.OP_MTSPR) |
-                       (mmu0.dec.op.internal_op == MicrOp.OP_MFSPR))):
-                comb += self.do_copy("insn_type", mmu0.op_get("internal_op"))
-            with m.Else():
-                comb += self.do_copy("insn_type", self.op_get("internal_op"))
-        else:
-            comb += self.do_copy("insn_type", self.op_get("internal_op"))
+        internal_op = self.op_get("internal_op")
+        comb += self.do_copy("insn_type", internal_op)
 
         # function unit for decoded instruction: requires minor redirect
         # for SPR set/get
@@ -790,13 +798,25 @@ class PowerDecodeSubset(Elaboratable):
 
         # Microwatt doesn't implement the partition table
         # instead has PRTBL(SVSRR0) register (SPR) to point to process table
-        with m.If(((self.dec.op.internal_op == MicrOp.OP_MTSPR) |
-                   (self.dec.op.internal_op == MicrOp.OP_MFSPR)) &
-                   ((spr == SPR.DSISR.value) | (spr == SPR.DAR.value) |
-                     (spr==SPR.SVSRR0.value) | (spr==SPR.PIDR.value))):
-            comb += self.do_copy("fn_unit", Function.MMU)
+        is_spr_mv = Signal()
+        is_mmu_spr = Signal()
+        comb += is_spr_mv.eq((internal_op == MicrOp.OP_MTSPR) |
+                             (internal_op == MicrOp.OP_MFSPR))
+        comb += is_mmu_spr.eq((spr == SPR.DSISR.value) |
+                              (spr == SPR.DAR.value) |
+                              (spr == SPR.SVSRR0.value) |
+                              (spr == SPR.PIDR.value))
+        # MMU must receive MMU SPRs
+        with m.If(is_spr_mv & (fn == Function.SPR) & is_mmu_spr):
+            comb += self.do_copy("fn_unit", Function.NONE)
+            comb += self.do_copy("insn_type", MicrOp.OP_ILLEGAL)
+        # SPR pipe must *not* receive MMU SPRs
+        with m.Elif(is_spr_mv & (fn == Function.MMU) & ~is_mmu_spr):
+            comb += self.do_copy("fn_unit", Function.NONE)
+            comb += self.do_copy("insn_type", MicrOp.OP_ILLEGAL)
+        # all others ok
         with m.Else():
-            comb += self.do_copy("fn_unit",fn)
+            comb += self.do_copy("fn_unit", fn)
 
         # immediates
         if self.needs_field("zero_a", "in1_sel"):
diff --git a/src/soc/fu/mmu/fsm.py b/src/soc/fu/mmu/fsm.py
index 0b3b728e..bd0c36ee 100644
--- a/src/soc/fu/mmu/fsm.py
+++ b/src/soc/fu/mmu/fsm.py
@@ -192,7 +192,7 @@ class FSMMMUStage(ControlBase):
         comb += self.ldst.d_out.eq(self.dcache.d_out)
 
         data_i, data_o = self.p.data_i, self.n.data_o
-        a_i, b_i, o = data_i.ra, data_i.rb, data_o.o
+        a_i, b_i, o, spr1_o = data_i.ra, data_i.rb, data_o.o, data_o.spr1
         op = data_i.ctx.op
 
         # TODO: link these SPRs somewhere
@@ -230,6 +230,13 @@ class FSMMMUStage(ControlBase):
 
             with m.Switch(op.insn_type):
                 with m.Case(MicrOp.OP_MTSPR):
+                    # despite redirection this FU **MUST** behave exactly
+                    # like the SPR FU.  this **INCLUDES** updating the SPR
+                    # regfile because the CSV file entry for OP_MTSPR
+                    # categorically defines and requires the expectation
+                    # that the CompUnit **WILL** write to the regfile.
+                    comb += spr1_o.data.eq(spr)
+                    comb += spr1_o.ok.eq(1)
                     # subset SPR: first check a few bits
                     with m.If(~spr[9] & ~spr[5]):
                         comb += self.debug0.eq(3)
-- 
2.30.2