Error response for unmapped address or forbidden read/write access (#168)

* feat: add ability to enable error output on the cpuif, when decoding errors occur (generate_cpuif_err in API).

* fix: move signal to new place (after automatic vers)

* feat: add info about new api (generate_cpuif_err)

* fix: repair readback with latency

* Adding generate_cpuif_err argument to peakrdl-regblock to generate cpuif error response, when the address is decoded incorrectly

* add sw rd or/and wr attribure error response related and add error respone for external mem

* add sw rd or/and wr error response test

* add sw rd or/and wr error response for external register test and fix generation of rtl logic for external register

* add sw rd or/and wr error response for external mem test

* add sw rd or/and wr error response for apb3 imterfaces driver

* add error response test for APB4, AXI4Lite and Avalon interfaces

* rename --generate_cpuif_err to --generate-cpuif-err

* style: minor typo fixes and test clean-up

* refactor: move expected error check inside write/read functions

* feat: add error response check to OBI testbench interface

* feat: split generate-cpuif-err option into err-if-bad-addr and err-if-bad-rw options

* feat: add err_if_bad_addr/rw to cfg_schema

* feat: extend cpuif_err_rsp test to cover all combinations of bad_addr/bad_rw

* style: lint fixes

* fix: removed redundant if node.external condition to help coverage

* Fix dangling hwif_in signals in testcase

---------

Co-authored-by: Denis Trifonov <d.trifonov@yadro.com>
Co-authored-by: Dominik Tanous <tanous@kandou.com>
Co-authored-by: Sebastien Baillou <baillou@kandou.com>
Co-authored-by: Alex Mykyta <amykyta3@users.noreply.github.com>

Author: 5 people

src/peakrdl_regblock/__peakrdl__.py

@@ -22,6 +22,8 @@ class Exporter(ExporterSubcommandPlugin):
     cfg_schema = {
         "cpuifs": {"*": schema.PythonObjectImport()},
         "default_reset": schema.Choice(["rst", "rst_n", "arst", "arst_n"]),
+        "err_if_bad_addr": schema.Boolean(),
+        "err_if_bad_rw": schema.Boolean(),
     }
 
     @functools.lru_cache()
@@ -141,6 +143,20 @@ def add_exporter_arguments(self, arg_group: 'argparse._ActionsContainer') -> Non
             is active-high and synchronous [rst]"""
         )
 
+        arg_group.add_argument(
+            "--err-if-bad-addr",
+            action="store_true",
+            default=False,
+            help="Generate CPUIF error response, when the address is decoded incorrectly"
+        )
+
+        arg_group.add_argument(
+            "--err-if-bad-rw",
+            action="store_true",
+            default=False,
+            help="""Generate CPUIF error response, when an illegal access is
+            performed to a read-only or write-only register"""
+        )
 
     def do_export(self, top_node: 'AddrmapNode', options: 'argparse.Namespace') -> None:
         cpuifs = self.get_cpuifs()
@@ -203,5 +219,7 @@ def do_export(self, top_node: 'AddrmapNode', options: 'argparse.Namespace') -> N
             generate_hwif_report=options.hwif_report,
             address_width=options.addr_width,
             default_reset_activelow=default_reset_activelow,
+            err_if_bad_addr=options.err_if_bad_addr or self.cfg['err_if_bad_addr'],
+            err_if_bad_rw=options.err_if_bad_rw or self.cfg['err_if_bad_rw'],
             default_reset_async=default_reset_async,
         )

src/peakrdl_regblock/addr_decode.py

@@ -1,6 +1,6 @@
 from typing import TYPE_CHECKING, Union, List, Optional
 
-from systemrdl.node import FieldNode, RegNode
+from systemrdl.node import FieldNode, RegNode, MemNode
 from systemrdl.walker import WalkerAction
 
 from .utils import get_indexed_path
@@ -12,7 +12,7 @@
 if TYPE_CHECKING:
     from .exporter import RegblockExporter
     from systemrdl.node import AddrmapNode, AddressableNode
-    from systemrdl.node import RegfileNode, MemNode
+    from systemrdl.node import RegfileNode
 
 class AddressDecode:
     def __init__(self, exp:'RegblockExporter'):
@@ -132,6 +132,33 @@ def __init__(self, addr_decode: AddressDecode) -> None:
         # List of address strides for each dimension
         self._array_stride_stack = [] # type: List[int]
 
+    def _add_addressablenode_decoding_flags(self, node: 'AddressableNode') -> None:
+        addr_str = self._get_address_str(node)
+        addr_decoding_str = f"cpuif_req_masked & (cpuif_addr >= {addr_str}) & (cpuif_addr <= {addr_str} + {SVInt(node.size - 1, self.addr_decode.exp.ds.addr_width)})"
+        rhs = addr_decoding_str
+        rhs_valid_addr = addr_decoding_str
+        if isinstance(node, MemNode):
+            readable = node.is_sw_readable
+            writable = node.is_sw_writable
+            if readable and writable:
+                rhs_invalid_rw = "'0"
+            elif readable and not writable:
+                rhs = f"{addr_decoding_str} & !cpuif_req_is_wr"
+                rhs_invalid_rw = f"{addr_decoding_str} & cpuif_req_is_wr"
+            elif not readable and writable:
+                rhs = f"{addr_decoding_str} & cpuif_req_is_wr"
+                rhs_invalid_rw = f"{addr_decoding_str} & !cpuif_req_is_wr"
+            else:
+                raise RuntimeError
+        # Add decoding flags
+        self.add_content(f"{self.addr_decode.get_external_block_access_strobe(node)} = {rhs};")
+        self.add_content(f"is_external |= {rhs};")
+        if self.addr_decode.exp.ds.err_if_bad_addr:
+            self.add_content(f"is_valid_addr |= {rhs_valid_addr};")
+        if isinstance(node, MemNode):
+            if self.addr_decode.exp.ds.err_if_bad_rw:
+                self.add_content(f"is_invalid_rw |= {rhs_invalid_rw};")
+
 
     def enter_AddressableComponent(self, node: 'AddressableNode') -> Optional[WalkerAction]:
         super().enter_AddressableComponent(node)
@@ -149,11 +176,7 @@ def enter_AddressableComponent(self, node: 'AddressableNode') -> Optional[Walker
 
         if node.external and not isinstance(node, RegNode):
             # Is an external block
-            addr_str = self._get_address_str(node)
-            strb = self.addr_decode.get_external_block_access_strobe(node)
-            rhs = f"cpuif_req_masked & (cpuif_addr >= {addr_str}) & (cpuif_addr <= {addr_str} + {SVInt(node.size - 1, self.addr_decode.exp.ds.addr_width)})"
-            self.add_content(f"{strb} = {rhs};")
-            self.add_content(f"is_external |= {rhs};")
+            self._add_addressablenode_decoding_flags(node)
             return WalkerAction.SkipDescendants
 
         return WalkerAction.Continue
@@ -170,44 +193,52 @@ def _get_address_str(self, node: 'AddressableNode', subword_offset: int=0) -> st
         return a
 
 
+    def _add_reg_decoding_flags(self,
+        node: RegNode,
+        subword_index: Union[int, None] = None,
+        subword_stride: Union[int, None] = None) -> None:
+        if subword_index is None or subword_stride is None:
+            addr_decoding_str = f"cpuif_req_masked & (cpuif_addr == {self._get_address_str(node)})"
+        else:
+            addr_decoding_str = f"cpuif_req_masked & (cpuif_addr == {self._get_address_str(node, subword_offset=subword_index*subword_stride)})"
+        rhs_valid_addr = addr_decoding_str
+        readable = node.has_sw_readable
+        writable = node.has_sw_writable
+        if readable and writable:
+            rhs = addr_decoding_str
+            rhs_invalid_rw = "'0"
+        elif readable and not writable:
+            rhs = f"{addr_decoding_str} & !cpuif_req_is_wr"
+            rhs_invalid_rw = f"{addr_decoding_str} & cpuif_req_is_wr"
+        elif not readable and writable:
+            rhs = f"{addr_decoding_str} & cpuif_req_is_wr"
+            rhs_invalid_rw = f"{addr_decoding_str} & !cpuif_req_is_wr"
+        else:
+            raise RuntimeError
+        # Add decoding flags
+        if subword_index is None:
+            self.add_content(f"{self.addr_decode.get_access_strobe(node)} = {rhs};")
+        else:
+            self.add_content(f"{self.addr_decode.get_access_strobe(node)}[{subword_index}] = {rhs};")
+        if node.external:
+            self.add_content(f"is_external |= {rhs};")
+        if self.addr_decode.exp.ds.err_if_bad_addr:
+            self.add_content(f"is_valid_addr |= {rhs_valid_addr};")
+        if self.addr_decode.exp.ds.err_if_bad_rw:
+            self.add_content(f"is_invalid_rw |= {rhs_invalid_rw};")
+
     def enter_Reg(self, node: RegNode) -> None:
         regwidth = node.get_property('regwidth')
         accesswidth = node.get_property('accesswidth')
 
         if regwidth == accesswidth:
-            rhs = f"cpuif_req_masked & (cpuif_addr == {self._get_address_str(node)})"
-            s = f"{self.addr_decode.get_access_strobe(node)} = {rhs};"
-            self.add_content(s)
-            if node.external:
-                readable = node.has_sw_readable
-                writable = node.has_sw_writable
-                if readable and writable:
-                    self.add_content(f"is_external |= {rhs};")
-                elif readable and not writable:
-                    self.add_content(f"is_external |= {rhs} & !cpuif_req_is_wr;")
-                elif not readable and writable:
-                    self.add_content(f"is_external |= {rhs} & cpuif_req_is_wr;")
-                else:
-                    raise RuntimeError
+            self._add_reg_decoding_flags(node)
         else:
             # Register is wide. Create a substrobe for each subword
             n_subwords = regwidth // accesswidth
             subword_stride = accesswidth // 8
             for i in range(n_subwords):
-                rhs = f"cpuif_req_masked & (cpuif_addr == {self._get_address_str(node, subword_offset=i*subword_stride)})"
-                s = f"{self.addr_decode.get_access_strobe(node)}[{i}] = {rhs};"
-                self.add_content(s)
-                if node.external:
-                    readable = node.has_sw_readable
-                    writable = node.has_sw_writable
-                    if readable and writable:
-                        self.add_content(f"is_external |= {rhs};")
-                    elif readable and not writable:
-                        self.add_content(f"is_external |= {rhs} & !cpuif_req_is_wr;")
-                    elif not readable and writable:
-                        self.add_content(f"is_external |= {rhs} & cpuif_req_is_wr;")
-                    else:
-                        raise RuntimeError
+                self._add_reg_decoding_flags(node, i, subword_stride)
 
     def exit_AddressableComponent(self, node: 'AddressableNode') -> None:
         super().exit_AddressableComponent(node)

src/peakrdl_regblock/exporter.py

@@ -120,6 +120,13 @@ def export(self, node: Union[RootNode, AddrmapNode], output_dir:str, **kwargs: A
             If overriden to True, default reset is active-low instead of active-high.
         default_reset_async: bool
             If overriden to True, default reset is asynchronous instead of synchronous.
+        err_if_bad_addr: bool
+            If overriden to True: If the address is decoded incorrectly, the CPUIF response
+            signal shows an error. For example: APB.PSLVERR = 1'b1, AXI4LITE.*RESP = 2'b10.
+        err_if_bad_rw: bool
+            If overriden to True: If an illegal access is performed to a read-only or write-only
+            register, the CPUIF response signal shows an error. For example: APB.PSLVERR = 1'b1,
+            AXI4LITE.*RESP = 2'b10.
         """
         # If it is the root node, skip to top addrmap
         if isinstance(node, RootNode):
@@ -230,6 +237,10 @@ def __init__(self, top_node: AddrmapNode, kwargs: Any) -> None:
         self.default_reset_activelow = kwargs.pop("default_reset_activelow", False) # type: bool
         self.default_reset_async = kwargs.pop("default_reset_async", False) # type: bool
 
+        # Generating a cpuif error
+        self.err_if_bad_addr = kwargs.pop("err_if_bad_addr", False) # type: bool
+        self.err_if_bad_rw = kwargs.pop("err_if_bad_rw", False) # type: bool
+
         #------------------------
         # Info about the design
         #------------------------

src/peakrdl_regblock/module_tmpl.sv

@@ -126,6 +126,7 @@ module {{ds.module_name}}
     //--------------------------------------------------------------------------
     {{address_decode.get_strobe_struct()|indent}}
     decoded_reg_strb_t decoded_reg_strb;
+    logic decoded_err;
 {%- if ds.has_external_addressable %}
     logic decoded_strb_is_external;
 {% endif %}
@@ -138,11 +139,20 @@ module {{ds.module_name}}
     logic [{{cpuif.data_width-1}}:0] decoded_wr_biten;
 
     always_comb begin
+        automatic logic is_valid_addr;
+        automatic logic is_invalid_rw;
     {%- if ds.has_external_addressable %}
         automatic logic is_external;
         is_external = '0;
     {%- endif %}
+    {%- if ds.err_if_bad_addr %}
+        is_valid_addr = '0;
+    {%- else %}
+        is_valid_addr = '1; // No error checking on valid address access
+    {%- endif %}
+        is_invalid_rw = '0;
         {{address_decode.get_implementation()|indent(8)}}
+        decoded_err = (~is_valid_addr | is_invalid_rw) & decoded_req;
     {%- if ds.has_external_addressable %}
         decoded_strb_is_external = is_external;
         external_req = is_external;
@@ -225,7 +235,11 @@ module {{ds.module_name}}
     assign cpuif_wr_ack = decoded_req & decoded_req_is_wr;
 {%- endif %}
     // Writes are always granted with no error response
+    {%- if ds.err_if_bad_addr or ds.err_if_bad_rw %}
+    assign cpuif_wr_err = decoded_err;
+    {%- else %}
     assign cpuif_wr_err = '0;
+    {%- endif %}
 
     //--------------------------------------------------------------------------
     // Readback

src/peakrdl_regblock/readback/templates/readback.sv

@@ -29,12 +29,15 @@ end
 
 logic [{{cpuif.data_width-1}}:0] readback_array_r[{{fanin_array_size}}];
 logic readback_done_r;
+logic readback_err_r;
 always_ff {{get_always_ff_event(cpuif.reset)}} begin
     if({{get_resetsignal(cpuif.reset)}}) begin
         for(int i=0; i<{{fanin_array_size}}; i++) readback_array_r[i] <= '0;
         readback_done_r <= '0;
+        readback_err_r <= '0;
     end else begin
         readback_array_r <= readback_array_c;
+        readback_err_r <= decoded_err;
         {%- if ds.has_external_addressable %}
         readback_done_r <= decoded_req & ~decoded_req_is_wr & ~decoded_strb_is_external;
         {%- else %}
@@ -47,7 +50,11 @@ end
 always_comb begin
     automatic logic [{{cpuif.data_width-1}}:0] readback_data_var;
     readback_done = readback_done_r;
+    {%- if ds.err_if_bad_addr or ds.err_if_bad_rw %}
+    readback_err = readback_err_r;
+    {%- else %}
     readback_err = '0;
+    {%- endif %}
     readback_data_var = '0;
     for(int i=0; i<{{fanin_array_size}}; i++) readback_data_var |= readback_array_r[i];
     readback_data = readback_data_var;
@@ -63,7 +70,11 @@ always_comb begin
     {%- else %}
     readback_done = decoded_req & ~decoded_req_is_wr;
     {%- endif %}
+    {%- if ds.err_if_bad_addr or ds.err_if_bad_rw %}
+    readback_err = decoded_err;
+    {%- else %}
     readback_err = '0;
+    {%- endif %}
     readback_data_var = '0;
     for(int i=0; i<{{array_size}}; i++) readback_data_var |= readback_array[i];
     readback_data = readback_data_var;
@@ -75,5 +86,9 @@ end
 {%- else %}
 assign readback_done = decoded_req & ~decoded_req_is_wr;
 assign readback_data = '0;
+{%- if ds.err_if_bad_addr or ds.err_if_bad_rw %}
+assign readback_err = decoded_err;
+{%- else %}
 assign readback_err = '0;
+{%- endif %}
 {% endif %}

tests/lib/base_testcase.py

@@ -39,6 +39,8 @@ class BaseTestCase(unittest.TestCase):
     retime_external = False
     default_reset_activelow = False
     default_reset_async = False
+    err_if_bad_addr = False
+    err_if_bad_rw = False
 
     #: this gets auto-loaded via the _load_request autouse fixture
     request = None # type: pytest.FixtureRequest
@@ -118,6 +120,8 @@ def export_regblock(self):
             retime_external_addrmap=self.retime_external,
             default_reset_activelow=self.default_reset_activelow,
             default_reset_async=self.default_reset_async,
+            err_if_bad_addr=self.err_if_bad_addr,
+            err_if_bad_rw=self.err_if_bad_rw,
         )
 
     def delete_run_dir(self) -> None:

tests/lib/cpuifs/apb3/apb3_intf_driver.sv

@@ -50,7 +50,7 @@ interface apb3_intf_driver #(
 
     semaphore txn_mutex = new(1);
 
-    task automatic write(logic [ADDR_WIDTH-1:0] addr, logic [DATA_WIDTH-1:0] data);
+    task automatic write(logic [ADDR_WIDTH-1:0] addr, logic [DATA_WIDTH-1:0] data, logic expects_err = 1'b0);
         txn_mutex.get();
         ##0;
 
@@ -68,11 +68,13 @@ interface apb3_intf_driver #(
 
         // Wait for response
         while(cb.PREADY !== 1'b1) @(cb);
+        assert(!$isunknown(cb.PSLVERR)) else $error("Write to 0x%0x returned X's on PSLVERR", addr);
+        assert(cb.PSLVERR == expects_err) else $error("Error write response to 0x%x returned 0x%x. Expected 0x%x", addr, cb.PSLVERR, expects_err);
         reset();
         txn_mutex.put();
     endtask
 
-    task automatic read(logic [ADDR_WIDTH-1:0] addr, output logic [DATA_WIDTH-1:0] data);
+    task automatic read(logic [ADDR_WIDTH-1:0] addr, output logic [DATA_WIDTH-1:0] data, input logic expects_err = 1'b0);
         txn_mutex.get();
         ##0;
 
@@ -92,14 +94,15 @@ interface apb3_intf_driver #(
         while(cb.PREADY !== 1'b1) @(cb);
         assert(!$isunknown(cb.PRDATA)) else $error("Read from 0x%0x returned X's on PRDATA", addr);
         assert(!$isunknown(cb.PSLVERR)) else $error("Read from 0x%0x returned X's on PSLVERR", addr);
+        assert(cb.PSLVERR == expects_err) else $error("Error read response from 0x%x returned 0x%x. Expected 0x%x", addr, cb.PSLVERR, expects_err);
         data = cb.PRDATA;
         reset();
         txn_mutex.put();
     endtask
 
-    task automatic assert_read(logic [ADDR_WIDTH-1:0] addr, logic [DATA_WIDTH-1:0] expected_data, logic [DATA_WIDTH-1:0] mask = '1);
+    task automatic assert_read(logic [ADDR_WIDTH-1:0] addr, logic [DATA_WIDTH-1:0] expected_data, input logic [DATA_WIDTH-1:0] mask = {DATA_WIDTH{1'b1}}, input logic expects_err = 1'b0);
         logic [DATA_WIDTH-1:0] data;
-        read(addr, data);
+        read(addr, data, expects_err);
         data &= mask;
         assert(data == expected_data) else $error("Read from 0x%x returned 0x%x. Expected 0x%x", addr, data, expected_data);
     endtask