msi: add IRQ arbitration (#36)

Makefile

@@ -109,7 +109,7 @@ defconfig:
 .config:
 	cp defconfig .config
 
-test: test-fc_xcvr test-fejkon_led test-fejkon_pcie_avalon test-fejkon_pcie_data test-freq_gauge test-si570_ctrl test-pcie-hip
+test: test-fc_xcvr test-fejkon_led test-fejkon_pcie_avalon test-fejkon_pcie_data test-freq_gauge test-si570_ctrl test-pcie-hip test-pcie_msi_intr
 
 test-%: ip/%
 	make QPATH=${QPATH} -C "$<" test

ip/fejkon_pcie_data/test.py

@@ -1,7 +1,10 @@
 #!/usr/bin/env python3
-"""TODO(bluecmd): String here
+"""Transaction testbench for Fejkon PCIe.
 
-Documentation
+This test bench tests simplified behaviour of the PCIe TLP generation.
+It is not as correct and detailed as a full PCIe endpoint simulation
+(we have that as well in fejkon/test/pcie-hip/) but it is fast and easier
+to debug. It also uses only open-source tools which is cool.
 """
 import logging as log
 import os
@@ -177,8 +180,8 @@ def test_dma_c2h(self):
             self.data_tx_channel.next = 0
             self.data_tx_data.next = 0xdeadbeefcafef00d1234567890abcdefaaaaaaaaaaaaaaaa5555555555555500 + i
             if not self.data_tx_ready:
-                val = yield self.data_tx_ready.posedge, myhdl.delay(1000)
-                if not val:
+                yield self.data_tx_ready.posedge, myhdl.delay(1000)
+                if not self.data_tx_ready:
                     raise Exception("Timeout waiting for data_tx_ready")
             yield self.clk.posedge
         for channel in range(4):

ip/pcie_msi_intr/.gitignore

@@ -0,0 +1,2 @@
+test.vvp
+test_*/

ip/pcie_msi_intr/Makefile

@@ -0,0 +1,21 @@
+.PHONY: lint test wave clean
+
+lint:
+	verilator --lint-only pcie_msi_intr.sv
+	iverilog -g2012 -t null tb.sv pcie_msi_intr.sv
+
+test.vvp: pcie_msi_intr.sv tb.sv
+	iverilog -g2012 -o $@ $^
+
+test: test.vvp
+	python3 test.py -v
+
+wave: $(TEST)/wave.fst test.gtkw
+	gtkwave --save test.gtkw --dump=$<
+	sed -i '/^\[dumpfile/d' test.gtkw
+	sed -i '/^\[savefile/d' test.gtkw
+	sed -i '/^\[\*]/d' test.gtkw
+
+clean:
+	\rm -f test.vvp
+	\rm -fr test_*/

ip/pcie_msi_intr/pcie_msi_intr.sv

@@ -8,7 +8,7 @@ module pcie_msi_intr (
     input  wire        app_msi_ack,    //        .app_msi_ack
     input  wire        clk,            //     clk.clk
     input  wire        reset,          //   reset.reset
-    input  wire [23:0] irq             //     irq.irq
+    input  wire [7:0]  irq             //     irq.irq
   );
 
   // Always use traffic class zero for MSI.
@@ -20,11 +20,73 @@ module pcie_msi_intr (
   // so this is always low.
   assign app_int_sts = 1'b0;
 
-  // TODO: Good thing: no TLP magic needed here, it's all generated in the
-  // hard IP core. Just assert app_msi_req with the correct
-  // app_msi_{num,tc} and you're good to go.
 
-  assign app_msi_req = 1'b0;
-  assign app_msi_num = 5'b00000;
+  // Fit all MSI numbers even though we are not using all of them.
+  // This is to make it easier for synth to know we are not overflowing our
+  // array access.
+  logic [31:0] irq_pending = 0;
+  logic [31:0] irq_clear = 0;
+
+  logic [7:0] irq_r = 0;
+
+  // This is not well-documented but the alternative is spamming the host with
+  // MSI MWrs. Doing a one-shot works fine, and we should not be worried about
+  // losing them (they are real transactions after all, losing TLPs would be
+  // catastrophic anyway).
+  //
+  // We just have to be sure that when the driver is loaded, all IRQs are let
+  // down before MSI is re-enabled so the host doesn't get swamped by old
+  // IRQs it cannot yet handle (or that it knows to ignore them).
+  always @(posedge clk) begin: irq_one_shot
+    if (reset) begin
+      irq_r <= 0;
+    end else begin
+      irq_r <= irq;
+    end
+  end
+
+  always @(posedge clk) begin: irq_buffer
+    if (reset) begin
+      irq_pending <= 0;
+    end else begin
+      irq_pending <= (irq_pending | {24'b0, irq & ~irq_r}) & ~irq_clear;
+    end
+  end
+
+  logic       msi_req = 0;
+  logic [4:0] msi_num = 0;
+
+  always @(posedge clk) begin: msi_tx
+    if (reset) begin
+      msi_req = 0;
+      msi_num = 0;
+      irq_clear <= 0;
+    end else begin
+      if (app_msi_ack) begin
+        msi_req = 0;
+        msi_num = 0;
+      end
+      msi_req = 1'b1;
+      casez (irq_pending[7:0])
+        8'b???????1: msi_num = 0;
+        8'b??????10: msi_num = 1;
+        8'b?????100: msi_num = 2;
+        8'b????1000: msi_num = 3;
+        8'b???10000: msi_num = 4;
+        8'b??100000: msi_num = 5;
+        8'b?1000000: msi_num = 6;
+        8'b10000000: msi_num = 7;
+        default: msi_req = 0;
+      endcase
+      if (msi_req) begin
+        irq_clear <= 1'b1 << msi_num;
+      end else begin
+        irq_clear <= 0;
+      end
+    end
+  end
+
+  assign app_msi_req = msi_req;
+  assign app_msi_num = msi_num;
 
 endmodule

ip/pcie_msi_intr/pcie_msi_intr_hw.tcl

@@ -121,5 +121,5 @@ set_interface_property irq PORT_NAME_MAP ""
 set_interface_property irq CMSIS_SVD_VARIABLES ""
 set_interface_property irq SVD_ADDRESS_GROUP ""
 
-add_interface_port irq irq irq Input 24
+add_interface_port irq irq irq Input 8
 

ip/pcie_msi_intr/tb.sv

@@ -0,0 +1,30 @@
+module tb;
+
+  logic [4:0]  app_msi_num;
+  logic        app_msi_req;
+  logic        app_msi_ack;
+  logic        clk;
+  logic        reset;
+  logic [7:0]  irq;
+
+  initial begin
+    $from_myhdl(clk, reset, irq, app_msi_ack);
+    $to_myhdl(app_msi_num, app_msi_req);
+  end
+
+  pcie_msi_intr dut(
+    .app_int_sts(),
+    .app_msi_num(app_msi_num),
+    .app_msi_req(app_msi_req),
+    .app_msi_tc(),
+    .app_int_ack(1'b0),
+    .app_msi_ack(app_msi_ack),
+    .clk(clk),
+    .reset(reset),
+    .irq(irq));
+
+  initial begin
+    $dumpfile("wave.fst");
+    $dumpvars(0, tb);
+  end
+endmodule

ip/pcie_msi_intr/test.gtkw

@@ -0,0 +1,34 @@
+[timestart] 0
+[size] 2201 1199
+[pos] -1 -1
+*-13.840914 -1 -1 -1 -1 -1 -1 -1 -1 -1 -1 -1 -1 -1 -1 -1 -1 -1 -1 -1 -1 -1 -1 -1 -1 -1 -1 -1
+[treeopen] tb.
+[sst_width] 213
+[signals_width] 219
+[sst_expanded] 1
+[sst_vpaned_height] 362
+@28
+tb.dut.clk
+tb.dut.reset
+@22
+tb.dut.irq[7:0]
+@200
+-
+@22
+tb.dut.irq_pending[31:0]
+tb.dut.irq_clear[31:0]
+@200
+-
+@28
+tb.dut.app_msi_req
+tb.dut.app_msi_ack
+@23
+tb.dut.app_msi_num[4:0]
+@200
+-
+@28
+tb.dut.app_msi_tc[2:0]
+tb.dut.app_int_ack
+tb.dut.app_int_sts
+[pattern_trace] 1
+[pattern_trace] 0

ip/pcie_msi_intr/test.py

@@ -0,0 +1,156 @@
+#/usr/bin/env/python3
+"""Testbench for PCIe MSI generation."""
+
+import logging as log
+import os
+import unittest
+
+import myhdl
+
+def testcase(*blocks):
+    """Runs a decorated function as a block in MyHDL.
+
+    Arguments:
+      *blocks: Any argument given is a function to create other blocks
+    """
+    def inner(func):
+        def block(self):
+            def run_and_stop():
+                yield from func(self)
+                raise myhdl.StopSimulation
+            insts = [x(self, func) for x in blocks] + [run_and_stop()]
+            sim = myhdl.Simulation(insts)
+            sim.run(quiet=1)
+        return block
+    return inner
+
+class Test(unittest.TestCase):
+    """Collection of PCIe MSI test cases"""
+
+    # pylint: disable=too-many-instance-attributes
+    def setUp(self):
+        # To HDL
+        self.clk = myhdl.Signal(bool(0))
+        self.rst = myhdl.Signal(bool(0))
+        self.irq = myhdl.Signal(myhdl.intbv()[8:])
+        self.msi_ack = myhdl.Signal(bool(0))
+        # From HDL
+        self.msi_num = myhdl.Signal(myhdl.intbv()[5:])
+        self.msi_req = myhdl.Signal(bool(0))
+
+    # === MyHDL instances ===
+    # MyHDL instances that can be enable to co-execute alongside the test case
+    # These are processes that may or may not be included depending on if that
+    # signal is required for a particular test case.
+    # dutgen and clkgen is probably always required.
+
+    def clkgen(self, unused_test):
+        @myhdl.always(myhdl.delay(2))
+        def block():
+            # pylint: disable=multiple-statements
+            self.clk.next = not self.clk
+        return block
+
+    def dutgen(self, test):
+        """Cosimulation of actual Verilog code"""
+        # This is because icarus does not allow changing the dumpfile in
+        # a nice way
+        oldpath = os.getcwd()
+        newpath = os.path.join(oldpath, test.__name__)
+        os.makedirs(newpath, exist_ok=True)
+        os.chdir(newpath)
+        ret = myhdl.Cosimulation(
+            "vvp -m myhdl ../test.vvp -fst",
+            # To HDL
+            clk=self.clk,
+            reset=self.rst,
+            irq=self.irq,
+            app_msi_ack=self.msi_ack,
+            # From HDL
+            app_msi_num=self.msi_num,
+            app_msi_req=self.msi_req)
+        os.chdir(oldpath)
+        return ret
+
+    # === End of MyHDL instances ===
+    def reset(self):
+        yield myhdl.delay(10)
+        self.rst.next = 1
+        yield myhdl.delay(10)
+        self.rst.next = 0
+        yield myhdl.delay(10)
+
+    @testcase(clkgen, dutgen)
+    def test_single_irq(self):
+        """Test sending an interrupt."""
+        yield from self.reset()
+        yield myhdl.delay(10)
+        self.irq.next |= 1 << 5
+        yield self.msi_req.posedge, myhdl.delay(1000)
+        if not self.msi_req:
+            raise Exception("Timeout waiting for app_msi_req")
+        self.irq.next = 0
+        yield self.clk.posedge
+        self.assertEqual(self.msi_num, 0x5)
+        self.msi_ack.next = 1
+        yield self.clk.posedge
+        self.msi_ack.next = 0
+        yield self.clk.negedge
+        if self.msi_req:
+            raise Exception("Expected cleared app_msi_req 1 cycle after ack")
+        # Expect no more events
+        yield self.msi_req.posedge, myhdl.delay(100)
+        if self.msi_req:
+            raise Exception("Got unexpected secondary MSI")
+        yield myhdl.delay(10)
+
+    @testcase(clkgen, dutgen)
+    def test_continuous_irq(self):
+        """Test sending a continuous interrupt."""
+        yield from self.reset()
+        yield myhdl.delay(10)
+        self.irq.next |= 1 << 5
+        yield self.msi_req.posedge, myhdl.delay(1000)
+        if not self.msi_req:
+            raise Exception("Timeout waiting for app_msi_req")
+        yield self.clk.posedge
+        self.assertEqual(self.msi_num, 0x5)
+        self.msi_ack.next = 1
+        yield self.clk.posedge
+        self.msi_ack.next = 0
+        yield self.clk.negedge
+        if self.msi_req:
+            raise Exception("Expected cleared app_msi_req 1 cycle after ack")
+        # Expect no more events
+        yield self.msi_req.posedge, myhdl.delay(100)
+        if self.msi_req:
+            raise Exception("Got unexpected secondary MSI")
+        yield myhdl.delay(10)
+
+    @testcase(clkgen, dutgen)
+    def test_multiple_irq(self):
+        """Test sending multiple interrupts."""
+        yield from self.reset()
+        yield myhdl.delay(10)
+        self.irq.next |= 1 << 0 | 1 << 2 | 1 << 3
+        yield self.msi_req.posedge, myhdl.delay(1000)
+        if not self.msi_req:
+            raise Exception("Timeout waiting for app_msi_req")
+        self.irq.next = 0
+        for ei in [0, 2, 3]:
+            yield self.clk.posedge
+            self.assertEqual(self.msi_num, ei)
+            self.msi_ack.next = 1
+            yield self.clk.posedge
+            self.msi_ack.next = 0
+            yield self.clk.negedge
+        if self.msi_req:
+            raise Exception("Expected cleared app_msi_req 1 cycle after ack")
+        yield myhdl.delay(10)
+
+if __name__ == '__main__':
+    os.chdir(os.path.dirname(os.path.abspath(__file__)))
+    log.basicConfig(
+            level=log.DEBUG,
+            format='[%(asctime)-15s] %(funcName)-15s %(levelname)-8s %(message)s')
+    unittest.main()