From e81c0c839687405b3b99555f0beac0a3e2f81870 Mon Sep 17 00:00:00 2001 From: Rupert Swarbrick Date: Mon, 20 Nov 2023 16:49:38 +0000 Subject: [PATCH] [rtl] Avoid name collision in ibex_pmp.sv Recent versions of Verilator complain about the code that was there because the csr_pmp_cfg argument clashes with a name in ibex_core.sv. What's more, they mean different things! In ibex_core.sv, it was the PMP configuration for the entire core. In the functions, it's the PMP configuration for a single region. This patch adds a "region_" prefix to the names, which fixes both the Verilator warning and my confusion! --- rtl/ibex_pmp.sv | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/rtl/ibex_pmp.sv b/rtl/ibex_pmp.sv index 6bda6e03d3..48c3a7ed67 100644 --- a/rtl/ibex_pmp.sv +++ b/rtl/ibex_pmp.sv @@ -51,16 +51,16 @@ module ibex_pmp #( // \--> pmp_req_err_o // Compute permissions checks that apply when MSECCFG.MML is set. Added for Smepmp support. - function automatic logic mml_perm_check(ibex_pkg::pmp_cfg_t csr_pmp_cfg, + function automatic logic mml_perm_check(ibex_pkg::pmp_cfg_t region_csr_pmp_cfg, ibex_pkg::pmp_req_e pmp_req_type, ibex_pkg::priv_lvl_e priv_mode, logic permission_check); logic result = 1'b0; - logic unused_cfg = |csr_pmp_cfg.mode; + logic unused_cfg = |region_csr_pmp_cfg.mode; - if (!csr_pmp_cfg.read && csr_pmp_cfg.write) begin + if (!region_csr_pmp_cfg.read && region_csr_pmp_cfg.write) begin // Special-case shared regions where R = 0, W = 1 - unique case ({csr_pmp_cfg.lock, csr_pmp_cfg.exec}) + unique case ({region_csr_pmp_cfg.lock, region_csr_pmp_cfg.exec}) // Read/write in M, read only in S/U 2'b00: result = (pmp_req_type == PMP_ACC_READ) | @@ -77,14 +77,15 @@ module ibex_pmp #( default: ; endcase end else begin - if (csr_pmp_cfg.read & csr_pmp_cfg.write & csr_pmp_cfg.exec & csr_pmp_cfg.lock) begin + if (region_csr_pmp_cfg.read & region_csr_pmp_cfg.write & + region_csr_pmp_cfg.exec & region_csr_pmp_cfg.lock) begin // Special-case shared read only region when R = 1, W = 1, X = 1, L = 1 result = pmp_req_type == PMP_ACC_READ; end else begin // Otherwise use basic permission check. Permission is always denied if in S/U mode and // L is set or if in M mode and L is unset. result = permission_check & - (priv_mode == PRIV_LVL_M ? csr_pmp_cfg.lock : ~csr_pmp_cfg.lock); + (priv_mode == PRIV_LVL_M ? region_csr_pmp_cfg.lock : ~region_csr_pmp_cfg.lock); end end return result; @@ -105,15 +106,15 @@ module ibex_pmp #( // A wrapper function in which it is decided which form of permission check function gets called function automatic logic perm_check_wrapper(logic csr_pmp_mseccfg_mml, - ibex_pkg::pmp_cfg_t csr_pmp_cfg, + ibex_pkg::pmp_cfg_t region_csr_pmp_cfg, ibex_pkg::pmp_req_e pmp_req_type, ibex_pkg::priv_lvl_e priv_mode, logic permission_check); - return csr_pmp_mseccfg_mml ? mml_perm_check(csr_pmp_cfg, + return csr_pmp_mseccfg_mml ? mml_perm_check(region_csr_pmp_cfg, pmp_req_type, priv_mode, permission_check) : - orig_perm_check(csr_pmp_cfg.lock, + orig_perm_check(region_csr_pmp_cfg.lock, priv_mode, permission_check); endfunction