Merge pull request chipsalliance#143 from antmicro/pmp-style

Improve PMP microarchitectural tests

verification/block/dccm/test_readwrite.py

@@ -28,12 +28,8 @@ async def body(self):
         dwidth = ConfigDB().get(None, "", "DCCM_FDATA_WIDTH")
 
         for i in range(count):
-
             # Randomize unique addresses (aligned)
-            addrs = set([
-                random.randrange(0, 1 << awidth) & 0xFFFFFFFC
-                for i in range(burst)
-            ])
+            addrs = set([random.randrange(0, 1 << awidth) & 0xFFFFFFFC for i in range(burst)])
 
             # Issue writes, randomize data
             for addr in addrs:

verification/block/dccm/testbench.py

@@ -5,11 +5,7 @@
 
 import pyuvm
 from cocotb.clock import Clock
-from cocotb.triggers import (
-    ClockCycles,
-    FallingEdge,
-    RisingEdge,
-)
+from cocotb.triggers import ClockCycles, FallingEdge, RisingEdge
 from pyuvm import *
 
 # ==============================================================================

verification/block/dec_tl/test_dec_tl.py

@@ -5,7 +5,7 @@
 import pyuvm
 from cocotb.triggers import ClockCycles
 from pyuvm import *
-from testbench import TlSequence, BaseTest
+from testbench import BaseTest, TlSequence
 
 # =============================================================================
 

verification/block/dec_tl/testbench.py

@@ -1,15 +1,14 @@
 # Copyright (c) 2023 Antmicro
 # SPDX-License-Identifier: Apache-2.0
 
+import copy
 import math
 import os
 import random
 import struct
-import copy
 
 import pyuvm
 from cocotb.binary import BinaryValue
-from cocotb.types import Array, Range
 from cocotb.clock import Clock
 from cocotb.triggers import (
     ClockCycles,
@@ -20,6 +19,7 @@
     RisingEdge,
     Timer,
 )
+from cocotb.types import Array, Range
 from pyuvm import *
 
 # ==============================================================================

verification/block/pmp/common.py

@@ -31,7 +31,7 @@ async def randomAccessInAddrRange(self, start_addr, end_addr):
     # Access memory at a given address and at adjacent addresses
     async def checkRangeBoundary(self, addr):
         # Ensure access address is always aligned and doesn't extend 32 bits,
-        # address is assumed to be inclusive so increment it by 1 intially.
+        # address is assumed to be inclusive so increment it by 1 initially.
         addr = min(self.MAX_ADDR, (addr + 1) & 0xFFFFFFFC)
 
         if addr >= 4:

verification/block/pmp/test_address_matching.py

@@ -4,7 +4,13 @@
 
 from common import BaseSequence
 from pyuvm import ConfigDB, test
-from testbench import BaseEnv, BaseTest, PMPWriteCSRItem, getDecodedEntryCfg
+from testbench import (
+    BaseEnv,
+    BaseTest,
+    PMPWriteAddrCSRItem,
+    PMPWriteCfgCSRItem,
+    getDecodedEntryCfg,
+)
 
 pmp_configurations = [
     {
@@ -83,16 +89,20 @@ def __init__(self, name):
         super().__init__(name)
 
     async def body(self):
+        test_iterations = ConfigDB().get(None, "", "TEST_ITERATIONS")
         pmp_entries = ConfigDB().get(None, "", "PMP_ENTRIES")
 
+        # Ensure to not use more configurations than PMP entries
+        assert len(pmp_configurations) <= pmp_entries
+
         # Configure PMP entries
-        possible_configs = min(pmp_entries, len(pmp_configurations))
         for i, cfg in enumerate(pmp_configurations):
-            # Ensure to not use more configurations than PMP entries
-            if i == possible_configs:
-                break
+            item = PMPWriteAddrCSRItem(index=i, pmpaddr=cfg["pmpaddr"])
+            await self.pmp_seqr.start_item(item)
+            await self.pmp_seqr.finish_item(item)
 
-            item = PMPWriteCSRItem(index=i, pmpcfg=cfg["pmpcfg"], pmpaddr=cfg["pmpaddr"])
+        for i, cfg in enumerate(pmp_configurations):
+            item = PMPWriteCfgCSRItem(index=i, pmpcfg=cfg["pmpcfg"])
             await self.pmp_seqr.start_item(item)
             await self.pmp_seqr.finish_item(item)
 
@@ -110,8 +120,8 @@ async def body(self):
 
             await self.checkRangeBoundary(end_addr)
 
-        # In the end check 1000 accesses at random memory locations
-        for _ in range(1000):
+        # In the end check accesses at random memory locations
+        for _ in range(test_iterations):
             await self.randomAccessInAddrRange(0x00000000, 0xFFFFFFFF)
 
 

verification/block/pmp/test_multiple_configs.py

@@ -3,7 +3,7 @@
 
 from common import BaseSequence
 from pyuvm import ConfigDB, test
-from testbench import BaseEnv, BaseTest, PMPWriteCSRItem
+from testbench import BaseEnv, BaseTest, PMPWriteAddrCSRItem, PMPWriteCfgCSRItem
 
 LOWER_BOUNDARY = 0x00000
 UPPER_BOUNDARY = 0x20000
@@ -65,21 +65,25 @@ def __init__(self, name):
         super().__init__(name)
 
     async def body(self):
+        test_iterations = ConfigDB().get(None, "", "TEST_ITERATIONS")
         pmp_entries = ConfigDB().get(None, "", "PMP_ENTRIES")
 
+        # Ensure to not use more configurations than PMP entries
+        assert len(pmp_configurations) <= pmp_entries
+
         # Configure PMP entries
-        possible_configs = min(pmp_entries, len(pmp_configurations))
         for i, cfg in enumerate(pmp_configurations):
-            # Ensure to not use more configurations than PMP entries
-            if i == possible_configs:
-                break
+            item = PMPWriteAddrCSRItem(index=i, pmpaddr=cfg["pmpaddr"])
+            await self.pmp_seqr.start_item(item)
+            await self.pmp_seqr.finish_item(item)
 
-            item = PMPWriteCSRItem(index=i, pmpcfg=cfg["pmpcfg"], pmpaddr=cfg["pmpaddr"])
+        for i, cfg in enumerate(pmp_configurations):
+            item = PMPWriteCfgCSRItem(index=i, pmpcfg=cfg["pmpcfg"])
             await self.pmp_seqr.start_item(item)
             await self.pmp_seqr.finish_item(item)
 
         self.checkRangeBoundary(LOWER_BOUNDARY)
-        for _ in range(1000):
+        for _ in range(test_iterations):
             await self.randomAccessInAddrRange(LOWER_BOUNDARY, UPPER_BOUNDARY)
         self.checkRangeBoundary(UPPER_BOUNDARY)
 

verification/block/pmp/test_xwr_access.py

@@ -2,7 +2,13 @@
 # SPDX-License-Identifier: Apache-2.0
 
 from pyuvm import ConfigDB, test, uvm_sequence
-from testbench import BaseEnv, BaseTest, PMPCheckItem, PMPWriteCSRItem
+from testbench import (
+    BaseEnv,
+    BaseTest,
+    PMPCheckItem,
+    PMPWriteAddrCSRItem,
+    PMPWriteCfgCSRItem,
+)
 
 # =============================================================================
 
@@ -17,7 +23,7 @@ async def body(self):
         pmp_entries = ConfigDB().get(None, "", "PMP_ENTRIES")
         pmp_channels = ConfigDB().get(None, "", "PMP_CHANNELS")
 
-        # Configure first 8 entries to all possible XWR configurations
+        # Configure entries to all possible XWR configurations
         # Use TOR address matching for simplicity
         # 0b10001000
         # - bit 7 - Locked status (1 is locked)
@@ -26,22 +32,28 @@ async def body(self):
         # - bit 2 - Execute permission
         # - bit 1 - Write permission
         # - bit 0 - Read permission
-        for i in range(8):
-            cfg = 0b10001000 + i
+        MAX_XWR_CONFIGS = 8
+        for i in range(MAX_XWR_CONFIGS):
             addr = ((i + 1) * 0x1000) >> 2
-            item = PMPWriteCSRItem(index=i, pmpcfg=cfg, pmpaddr=addr)
+            item = PMPWriteAddrCSRItem(index=i, pmpaddr=addr)
+            await self.pmp_seqr.start_item(item)
+            await self.pmp_seqr.finish_item(item)
+
+        for i in range(MAX_XWR_CONFIGS):
+            cfg = 0b10001000 + i
+            item = PMPWriteCfgCSRItem(index=i, pmpcfg=cfg)
             await self.pmp_seqr.start_item(item)
             await self.pmp_seqr.finish_item(item)
 
         # Check all possible access variants on configured entries
         for i in range(pmp_channels):
+            channel = i
             # Set type to each of 3 available (R or W or X)
             for j in range(3):
                 type = 1 << j
                 for k in range(pmp_entries):
                     # Set address somewhere in the 0x1000 wide entry
                     addr = (0x200 + (k * 0x1000)) >> 2
-                    channel = i
 
                     item = PMPCheckItem(channel, addr, type)
                     await self.pmp_seqr.start_item(item)

verification/block/pmp/testbench.py

@@ -40,7 +40,7 @@ def getDecodedEntryCfg(regs, index, range_only=False):
     address_matching = pmpcfg[4:3].integer
     locked = pmpcfg[7].integer
 
-    if index != 0:
+    if index:
         start_address = regs.reg["pmpaddr{}".format(index - 1)].integer << 2
     else:
         start_address = 0
@@ -75,7 +75,7 @@ def getDecodedEntryCfg(regs, index, range_only=False):
             end_address = start_address + 2**napot
 
     # PMP upper address bundary is non-inclusive
-    end_address = end_address - 1
+    end_address -= 1
 
     if range_only:
         return start_address, end_address
@@ -86,15 +86,18 @@ def getDecodedEntryCfg(regs, index, range_only=False):
 # ==============================================================================
 
 
-class PMPWriteCSRItem(uvm_sequence_item):
-    def __init__(self, index, pmpcfg=None, pmpaddr=None):
-        super().__init__("PMPWriteCSRItem")
+class PMPWriteCfgCSRItem(uvm_sequence_item):
+    def __init__(self, index, pmpcfg):
+        super().__init__("PMPWriteCfgCSRItem")
         self.index = index
+        self.pmpcfg = pmpcfg
 
-        if pmpcfg is not None:
-            self.pmpcfg = pmpcfg
-        if pmpaddr is not None:
-            self.pmpaddr = pmpaddr
+
+class PMPWriteAddrCSRItem(uvm_sequence_item):
+    def __init__(self, index, pmpaddr):
+        super().__init__("PMPWriteAddrCSRItem")
+        self.index = index
+        self.pmpaddr = pmpaddr
 
 
 class PMPCheckItem(uvm_sequence_item):
@@ -152,11 +155,12 @@ async def run_phase(self):
         while True:
             it = await self.seq_item_port.get_next_item()
 
-            if isinstance(it, PMPWriteCSRItem):
-                self.pmp_pmpcfg[it.index].value = it.pmpcfg
+            if isinstance(it, PMPWriteAddrCSRItem):
                 self.pmp_pmpaddr[it.index].value = it.pmpaddr
-                self.regs.reg["pmpcfg{}".format(it.index)].integer = it.pmpcfg
                 self.regs.reg["pmpaddr{}".format(it.index)].integer = it.pmpaddr
+            elif isinstance(it, PMPWriteCfgCSRItem):
+                self.pmp_pmpcfg[it.index].value = it.pmpcfg
+                self.regs.reg["pmpcfg{}".format(it.index)].integer = it.pmpcfg
             elif isinstance(it, PMPCheckItem):
                 self.pmp_chan_addr[it.channel].value = it.addr
                 self.pmp_chan_type[it.channel].value = it.type
@@ -189,50 +193,21 @@ def __init__(self, *args, **kwargs):
 
         self.pmp_channels = ConfigDB().get(None, "", "PMP_CHANNELS")
         self.pmp_entries = ConfigDB().get(None, "", "PMP_ENTRIES")
-        self.prev_addr = [None for _ in range(self.pmp_channels)]
-        self.prev_type = [None for _ in range(self.pmp_channels)]
-        self.prev_err = [None for _ in range(self.pmp_channels)]
-        self.prev_pmpcfg = [None for _ in range(self.pmp_entries)]
-        self.prev_pmpaddr = [None for _ in range(self.pmp_entries)]
 
     def build_phase(self):
         self.ap = uvm_analysis_port("ap", self)
 
     async def run_phase(self):
         while True:
-            # Even though the signals are not sequential sample them on
-            # rising clock edge
             await RisingEdge(self.clk)
 
-            # Sample signals
+            # Check all PMP channels
             for i in range(self.pmp_channels):
-                curr_addr = int(self.pmp_chan_addr[i].value)
-                curr_type = int(self.pmp_chan_type[i].value)
-                curr_err = int(self.pmp_chan_err.value[i])
-
-                # Send an item in case of a change
-                if (
-                    self.prev_err[i] != curr_err
-                    or self.prev_addr[i] != curr_addr
-                    or self.prev_type[i] != curr_type
-                ):
-                    self.ap.write(PMPCheckItem(i, curr_addr, curr_type, curr_err))
-
-                self.prev_err[i] = curr_err
-                self.prev_addr[i] = curr_addr
-                self.prev_type[i] = curr_type
-
-            # If any PMP entry has changed, check all PMP channels
-            for i in range(self.pmp_entries):
-                curr_pmpcfg = int(self.pmp_pmpcfg[i].value)
-                curr_pmpaddr = int(self.pmp_pmpaddr[i].value)
-
-                if (curr_pmpcfg != self.prev_pmpcfg[i]) or (curr_pmpaddr != self.prev_pmpaddr[i]):
-                    for j in range(self.pmp_channels):
-                        self.ap.write(PMPCheckItem(j, curr_addr, curr_type, curr_err))
+                access_addr = int(self.pmp_chan_addr[i].value)
+                access_type = int(self.pmp_chan_type[i].value)
+                access_err = int(self.pmp_chan_err.value[i])
 
-                self.prev_pmpcfg[i] = curr_pmpcfg
-                self.prev_pmpaddr[i] = curr_pmpaddr
+                self.ap.write(PMPCheckItem(i, access_addr, access_type, access_err))
 
 
 # ==============================================================================
@@ -321,7 +296,7 @@ def build_phase(self):
         ConfigDB().set(None, "*", "PMP_GRANULARITY", 0)
 
         ConfigDB().set(None, "*", "TEST_CLK_PERIOD", 1)
-        ConfigDB().set(None, "*", "TEST_ITERATIONS", 50)
+        ConfigDB().set(None, "*", "TEST_ITERATIONS", 100)
 
         # PMP Registers
         self.regs = RegisterMap(pmp_entries)
@@ -356,9 +331,9 @@ def __init__(self, name, parent, env_class=BaseEnv):
         super().__init__(name, parent)
         self.env_class = env_class
 
-        # Syncrhonize pyuvm logging level with cocotb logging level. Unclear
+        # Synchronize pyuvm logging level with cocotb logging level. Unclear
         # why it does not happen automatically.
-        level = logging.getLevelName(os.environ.get("COCOTB_LOG_LEVEL", "DEBUG"))
+        level = logging.getLevelName(os.environ.get("COCOTB_LOG_LEVEL", "INFO"))
         uvm_report_object.set_default_logging_level(level)
 
     def build_phase(self):