Skip to content

Commit

Permalink
[rtl] Avoid name collision in ibex_pmp.sv
Browse files Browse the repository at this point in the history
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!
  • Loading branch information
rswarbrick committed Nov 24, 2023
1 parent fe84d64 commit e81c0c8
Showing 1 changed file with 10 additions and 9 deletions.
19 changes: 10 additions & 9 deletions rtl/ibex_pmp.sv
Original file line number Diff line number Diff line change
Expand Up @@ -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) |
Expand All @@ -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;
Expand All @@ -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
Expand Down

0 comments on commit e81c0c8

Please sign in to comment.