diff --git a/src/peakrdl_regblock/addr_decode.py b/src/peakrdl_regblock/addr_decode.py index 162ae31..7617a44 100644 --- a/src/peakrdl_regblock/addr_decode.py +++ b/src/peakrdl_regblock/addr_decode.py @@ -141,23 +141,26 @@ class DecodeLogicGenerator(RDLForLoopGenerator): readable = node.is_sw_readable writable = node.is_sw_writable if readable and writable: - rhs_invalid_rw = "'0" + pass 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: + # Also assign is_valid_adddr when err_if_bad_rw is set so that it can be used to catch + # invalid RW accesses on existing registers only. + if self.addr_decode.exp.ds.err_if_bad_addr or self.addr_decode.exp.ds.err_if_bad_rw: 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};") + self.add_content(f"is_valid_rw |= {rhs};") + else: + # For external register blocks, all accesses are valid RW + self.add_content(f"is_valid_rw |= {rhs_valid_addr};") def enter_AddressableComponent(self, node: 'AddressableNode') -> Optional[WalkerAction]: @@ -206,13 +209,10 @@ class DecodeLogicGenerator(RDLForLoopGenerator): 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 @@ -222,10 +222,12 @@ class DecodeLogicGenerator(RDLForLoopGenerator): 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: + # Also assign is_valid_adddr when err_if_bad_rw is set so that it can be used to catch + # invalid RW accesses on existing registers only. + if self.addr_decode.exp.ds.err_if_bad_addr or self.addr_decode.exp.ds.err_if_bad_rw: 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};") + self.add_content(f"is_valid_rw |= {rhs};") def enter_Reg(self, node: RegNode) -> None: regwidth = node.get_property('regwidth') diff --git a/src/peakrdl_regblock/module_tmpl.sv b/src/peakrdl_regblock/module_tmpl.sv index 65be9fa..1c15395 100644 --- a/src/peakrdl_regblock/module_tmpl.sv +++ b/src/peakrdl_regblock/module_tmpl.sv @@ -121,19 +121,31 @@ module {{ds.module_name}} always_comb begin automatic logic is_valid_addr; - automatic logic is_invalid_rw; + automatic logic is_valid_rw; {%- if ds.has_external_addressable %} automatic logic is_external; is_external = '0; {%- endif %} - {%- if ds.err_if_bad_addr %} + {%- if ds.err_if_bad_addr or ds.err_if_bad_rw %} is_valid_addr = '0; {%- else %} - is_valid_addr = '1; // No error checking on valid address access + is_valid_addr = '1; // No valid address check + {%- endif %} + {%- if ds.err_if_bad_rw %} + is_valid_rw = '0; + {%- else %} + is_valid_rw = '1; // No valid RW check {%- endif %} - is_invalid_rw = '0; {{address_decode.get_implementation()|indent(8)}} - decoded_err = (~is_valid_addr | is_invalid_rw) & decoded_req; + {%- if ds.err_if_bad_addr and ds.err_if_bad_rw %} + decoded_err = (~is_valid_addr | (is_valid_addr & ~is_valid_rw)) & decoded_req; + {%- elif ds.err_if_bad_addr %} + decoded_err = ~is_valid_addr & decoded_req; + {%- elif ds.err_if_bad_rw %} + decoded_err = (is_valid_addr & ~is_valid_rw) & decoded_req; + {%- else %} + decoded_err = '0; + {%- endif %} {%- if ds.has_external_addressable %} decoded_strb_is_external = is_external; external_req = is_external; diff --git a/tests/test_cpuif_err_rsp/regblock.rdl b/tests/test_cpuif_err_rsp/regblock.rdl index bc684f4..6d387f2 100644 --- a/tests/test_cpuif_err_rsp/regblock.rdl +++ b/tests/test_cpuif_err_rsp/regblock.rdl @@ -3,6 +3,7 @@ addrmap top { default sw=rw; default hw=na; + // Internal registers reg { field { sw=rw; hw=na; // Storage element @@ -13,32 +14,27 @@ addrmap top { field { sw=r; hw=na; // Wire/Bus - constant value } f[31:0] = 80; - } r_r; + } r_ro; reg { field { sw=w; hw=r; // Storage element } f[31:0] = 100; - } r_w; + } r_wo; - external reg { - field { - sw=rw; hw=na; // Storage element - } f[31:0]; - } er_rw; - - external reg { - field { - sw=r; hw=na; // Wire/Bus - constant value - } f[31:0]; - } er_r; - - external reg { - field { - sw=w; hw=na; // Storage element - } f[31:0]; - } er_w; + // Combined read-only and write-only register + reg { + default sw = w; + default hw = r; + field {} wvalue[32] = 0; + } writeonly @ 0x1C; + reg { + default sw = r; + default hw = na; + field {} rvalue[32] = 200; + } readonly @ 0x1C; + // External memories external mem { memwidth = 32; mementries = 2; @@ -48,12 +44,33 @@ addrmap top { memwidth = 32; mementries = 2; sw=r; - } mem_r @ 0x28; + } mem_ro @ 0x28; external mem { memwidth = 32; mementries = 2; sw=w; - } mem_w @ 0x30; + } mem_wo @ 0x30; + // External block + external regfile { + // Placeholder registers + reg { + field { + sw=rw; hw=na; + } f[31:0] = 40; + } r_rw; + + reg { + field { + sw=r; hw=na; + } f[31:0] = 80; + } r_ro; + + reg { + field { + sw=w; hw=r; + } f[31:0] = 100; + } r_wo; + } external_rf @ 0x40; }; diff --git a/tests/test_cpuif_err_rsp/tb_template.sv b/tests/test_cpuif_err_rsp/tb_template.sv index 462b856..50e5620 100644 --- a/tests/test_cpuif_err_rsp/tb_template.sv +++ b/tests/test_cpuif_err_rsp/tb_template.sv @@ -3,45 +3,6 @@ {%- block dut_support %} {% sv_line_anchor %} - external_reg ext_reg_inst ( - .clk(clk), - .rst(rst), - - .req(hwif_out.er_rw.req), - .req_is_wr(hwif_out.er_rw.req_is_wr), - .wr_data(hwif_out.er_rw.wr_data), - .wr_biten(hwif_out.er_rw.wr_biten), - .rd_ack(hwif_in.er_rw.rd_ack), - .rd_data(hwif_in.er_rw.rd_data), - .wr_ack(hwif_in.er_rw.wr_ack) - ); - - external_reg ro_reg_inst ( - .clk(clk), - .rst(rst), - - .req(hwif_out.er_r.req), - .req_is_wr(hwif_out.er_r.req_is_wr), - .wr_data(32'b0), - .wr_biten(32'b0), - .rd_ack(hwif_in.er_r.rd_ack), - .rd_data(hwif_in.er_r.rd_data), - .wr_ack() - ); - - external_reg wo_reg_inst ( - .clk(clk), - .rst(rst), - - .req(hwif_out.er_w.req), - .req_is_wr(hwif_out.er_w.req_is_wr), - .wr_data(hwif_out.er_w.wr_data), - .wr_biten(hwif_out.er_w.wr_biten), - .rd_ack(), - .rd_data(), - .wr_ack(hwif_in.er_w.wr_ack) - ); - external_block #( .ADDR_WIDTH(3) ) mem_rw_inst ( @@ -64,14 +25,14 @@ .clk(clk), .rst(rst), - .req(hwif_out.mem_r.req), - .req_is_wr(hwif_out.mem_r.req_is_wr), - .addr(hwif_out.mem_r.addr), + .req(hwif_out.mem_ro.req), + .req_is_wr(hwif_out.mem_ro.req_is_wr), + .addr(hwif_out.mem_ro.addr), .wr_data(32'b0), .wr_biten(32'b0), - .rd_ack(hwif_in.mem_r.rd_ack), - .rd_data(hwif_in.mem_r.rd_data), - .wr_ack(hwif_in.mem_r.wr_ack) + .rd_ack(hwif_in.mem_ro.rd_ack), + .rd_data(hwif_in.mem_ro.rd_data), + .wr_ack(hwif_in.mem_ro.wr_ack) ); external_block #( @@ -80,17 +41,33 @@ .clk(clk), .rst(rst), - .req(hwif_out.mem_w.req), - .req_is_wr(hwif_out.mem_w.req_is_wr), - .addr(hwif_out.mem_w.addr), - .wr_data(hwif_out.mem_w.wr_data), - .wr_biten(hwif_out.mem_w.wr_biten), + .req(hwif_out.mem_wo.req), + .req_is_wr(hwif_out.mem_wo.req_is_wr), + .addr(hwif_out.mem_wo.addr), + .wr_data(hwif_out.mem_wo.wr_data), + .wr_biten(hwif_out.mem_wo.wr_biten), .rd_ack(), .rd_data(), - .wr_ack(hwif_in.mem_w.wr_ack) + .wr_ack(hwif_in.mem_wo.wr_ack) + ); + assign hwif_in.mem_wo.rd_ack = '0; + assign hwif_in.mem_wo.rd_data = '0; + + external_block #( + .ADDR_WIDTH(4) + ) external_rf_inst ( + .clk(clk), + .rst(rst), + + .req(hwif_out.external_rf.req), + .req_is_wr(hwif_out.external_rf.req_is_wr), + .addr(hwif_out.external_rf.addr), + .wr_data(hwif_out.external_rf.wr_data), + .wr_biten(hwif_out.external_rf.wr_biten), + .rd_ack(hwif_in.external_rf.rd_ack), + .rd_data(hwif_in.external_rf.rd_data), + .wr_ack(hwif_in.external_rf.wr_ack) ); - assign hwif_in.mem_w.rd_ack = '0; - assign hwif_in.mem_w.rd_data = '0; {%- endblock %} @@ -102,7 +79,7 @@ logic expected_rd_err; logic bad_addr_expected_err; logic bad_rw_expected_wr_err; logic bad_rw_expected_rd_err; -logic [5:0] addr; +logic [7:0] addr; {% sv_line_anchor %} ##1; @@ -139,53 +116,29 @@ logic [5:0] addr; cpuif.write(addr, 81, .expects_err(expected_wr_err)); cpuif.assert_read(addr, 80, .expects_err(expected_rd_err)); - // r_w - sw=w; hw=r; // Storage element + // r_wo - sw=w; hw=r; // Storage element addr = 'h8; expected_rd_err = bad_rw_expected_rd_err; expected_wr_err = 'h0; cpuif.assert_read(addr, 0, .expects_err(expected_rd_err)); - assert(cb.hwif_out.r_w.f.value == 100); + assert(cb.hwif_out.r_wo.f.value == 100); cpuif.write(addr, 101, .expects_err(expected_wr_err)); cpuif.assert_read(addr, 0, .expects_err(expected_rd_err)); - assert(cb.hwif_out.r_w.f.value == 101); - - // External registers - // er_rw - sw=rw; hw=na; // Storage element - addr = 'hC; - expected_rd_err = 'h0; - expected_wr_err = 'h0; - ext_reg_inst.value = 'h8C; - cpuif.assert_read(addr, 'h8C, .expects_err(expected_rd_err)); - cpuif.write(addr, 'h8D, .expects_err(expected_wr_err)); - cpuif.assert_read(addr, 'h8D, .expects_err(expected_rd_err)); - - // er_r - sw=r; hw=na; // Wire/Bus - constant value - addr = 'h10; - expected_rd_err = 'h0; - expected_wr_err = bad_rw_expected_wr_err; - ro_reg_inst.value = 'hB4; - cpuif.assert_read(addr, 'hB4, .expects_err(expected_rd_err)); - cpuif.write(addr, 'hB5, .expects_err(expected_wr_err)); - cpuif.assert_read(addr, 'hB4, .expects_err(expected_rd_err)); - - // er_w - sw=w; hw=r; // Storage element - addr = 'h14; - expected_rd_err = bad_rw_expected_rd_err; - expected_wr_err = 'h0; - wo_reg_inst.value = 'hC8; - cpuif.assert_read(addr, 0, .expects_err(expected_rd_err)); - assert(wo_reg_inst.value == 'hC8); - - cpuif.write(addr, 'hC9, .expects_err(expected_wr_err)); - cpuif.assert_read(addr, 0, .expects_err(expected_rd_err)); - assert(wo_reg_inst.value == 'hC9); + assert(cb.hwif_out.r_wo.f.value == 101); // Reading/writing from/to non existing register addr = 'h18; cpuif.assert_read(addr, 0, .expects_err(bad_addr_expected_err)); cpuif.write(addr, 'h8C, .expects_err(bad_addr_expected_err)); + // Reading/writing from/to combined read AND write only register + addr = 'h1C; + expected_rd_err = 'h0; + expected_wr_err = 'h0; + cpuif.assert_read(addr, 200, .expects_err(expected_rd_err)); + cpuif.write(addr, 'h8C, .expects_err(expected_wr_err)); + // External memories // mem_rw - sw=rw; addr = 'h20; @@ -218,4 +171,11 @@ logic [5:0] addr; cpuif.assert_read(addr, 0, .expects_err(expected_rd_err)); assert(mem_wo_inst.mem[0] == 'hC9); + // External rf; + addr = 'h40; + expected_rd_err = 'h0; + expected_wr_err = 'h0; + cpuif.assert_read(addr, 'h0, .expects_err(expected_rd_err)); + cpuif.write(addr, 'hD0, .expects_err(expected_wr_err)); + cpuif.assert_read(addr, 'hD0, .expects_err(expected_rd_err)); {% endblock %}