diff --git a/dv/uvm/core_ibex/fcov/core_ibex_pmp_fcov_if.sv b/dv/uvm/core_ibex/fcov/core_ibex_pmp_fcov_if.sv index c565ead7..f28190e5 100644 --- a/dv/uvm/core_ibex/fcov/core_ibex_pmp_fcov_if.sv +++ b/dv/uvm/core_ibex/fcov/core_ibex_pmp_fcov_if.sv @@ -23,6 +23,8 @@ interface core_ibex_pmp_fcov_if import ibex_pkg::*; #( input logic lsu_req_done ); + localparam int unsigned PMPNumChan = 3; + `include "dv_fcov_macros.svh" import uvm_pkg::*; @@ -94,6 +96,15 @@ interface core_ibex_pmp_fcov_if import ibex_pkg::*; #( csr_pmp_cfg[i_region].exec, csr_pmp_cfg[i_region].write, csr_pmp_cfg[i_region].read}); + // Do the permission check for Data channel with the privilege level from Instruction channels. + // This way we can check the effect of mstatus.mprv changing privilege levels for LSU related + // operations. + assign current_priv_perm_check[i_region] = + g_pmp.pmp_i.perm_check_wrapper(csr_pmp_mseccfg.mml, + csr_pmp_cfg[i_region], + g_pmp.pmp_i.pmp_req_type_i[PMP_D], + cs_registers_i.priv_mode_id_o, + g_pmp.pmp_i.region_basic_perm_check[PMP_D][i_region]); covergroup pmp_region_cg @(posedge clk_i); option.per_instance = 1; diff --git a/rtl/ibex_core.sv b/rtl/ibex_core.sv index d9e4615e..f240eb5b 100644 --- a/rtl/ibex_core.sv +++ b/rtl/ibex_core.sv @@ -151,7 +151,7 @@ module ibex_core import ibex_pkg::*; #( output logic core_busy_o ); - localparam int unsigned PMP_NUM_CHAN = 3; + localparam int unsigned PMPNumChan = 3; // SEC_CM: CORE.DATA_REG_SW.SCA localparam bit DataIndTiming = SecureIbex; localparam bit PCIncrCheck = SecureIbex; @@ -309,7 +309,7 @@ module ibex_core import ibex_pkg::*; #( logic [33:0] csr_pmp_addr [PMPNumRegions]; pmp_cfg_t csr_pmp_cfg [PMPNumRegions]; pmp_mseccfg_t csr_pmp_mseccfg; - logic pmp_req_err [PMP_NUM_CHAN]; + logic pmp_req_err [PMPNumChan]; logic data_req_out; logic csr_save_if; @@ -1043,9 +1043,9 @@ module ibex_core import ibex_pkg::*; #( `ASSERT_KNOWN_IF(IbexCsrWdataIntKnown, cs_registers_i.csr_wdata_int, csr_op_en) if (PMPEnable) begin : g_pmp - logic [33:0] pmp_req_addr [PMP_NUM_CHAN]; - pmp_req_e pmp_req_type [PMP_NUM_CHAN]; - priv_lvl_e pmp_priv_lvl [PMP_NUM_CHAN]; + logic [33:0] pmp_req_addr [PMPNumChan]; + pmp_req_e pmp_req_type [PMPNumChan]; + priv_lvl_e pmp_priv_lvl [PMPNumChan]; assign pmp_req_addr[PMP_I] = {2'b00, pc_if}; assign pmp_req_type[PMP_I] = PMP_ACC_EXEC; @@ -1059,7 +1059,7 @@ module ibex_core import ibex_pkg::*; #( ibex_pmp #( .PMPGranularity(PMPGranularity), - .PMPNumChan (PMP_NUM_CHAN), + .PMPNumChan (PMPNumChan), .PMPNumRegions (PMPNumRegions) ) pmp_i ( // Interface to CSRs diff --git a/rtl/ibex_pmp.sv b/rtl/ibex_pmp.sv index 70e4c442..52a2b95e 100644 --- a/rtl/ibex_pmp.sv +++ b/rtl/ibex_pmp.sv @@ -36,10 +36,112 @@ module ibex_pmp #( logic [PMPNumChan-1:0][PMPNumRegions-1:0] region_match_eq; logic [PMPNumChan-1:0][PMPNumRegions-1:0] region_match_all; logic [PMPNumChan-1:0][PMPNumRegions-1:0] region_basic_perm_check; - logic [PMPNumChan-1:0][PMPNumRegions-1:0] region_mml_perm_check; - logic [PMPNumChan-1:0][PMPNumRegions-1:0] region_orig_perm_check; - logic [PMPNumChan-1:0] access_fault; + logic [PMPNumChan-1:0][PMPNumRegions-1:0] region_perm_check; + /////////////////////// + // Functions for PMP // + /////////////////////// + + // Flow of the PMP checking operation follows as below + // + // basic_perm_check ---> perm_check_wrapper ---> mml_perm_check/orig_perm_check ---/ + // | + // region_match_all --------------------------------> access_fault_check <---------- + // | + // \--> pmp_req_err_o + + // 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_req_e pmp_req_type, + ibex_pkg::priv_lvl_e priv_mode, + logic permission_check); + if (csr_pmp_mseccfg_mml) begin + return mml_perm_check(csr_pmp_cfg, + pmp_req_type, + priv_mode, + permission_check); + end else begin + return orig_perm_check(csr_pmp_cfg.lock, + priv_mode, + permission_check); + end + endfunction + + // 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, + 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; + + if (!csr_pmp_cfg.read && csr_pmp_cfg.write) begin + // Special-case shared regions where R = 0, W = 1 + unique case ({csr_pmp_cfg.lock, csr_pmp_cfg.exec}) + // Read/write in M, read only in S/U + 2'b00: result = + (pmp_req_type == PMP_ACC_READ) | + ((pmp_req_type == PMP_ACC_WRITE) & (priv_mode == PRIV_LVL_M)); + // Read/write in M/S/U + 2'b01: result = + (pmp_req_type == PMP_ACC_READ) | (pmp_req_type == PMP_ACC_WRITE); + // Execute only on M/S/U + 2'b10: result = (pmp_req_type == PMP_ACC_EXEC); + // Read/execute in M, execute only on S/U + 2'b11: result = + (pmp_req_type == PMP_ACC_EXEC) | + ((pmp_req_type == PMP_ACC_READ) & (priv_mode == PRIV_LVL_M)); + default: ; + endcase + end else begin + if (csr_pmp_cfg.read & csr_pmp_cfg.write & csr_pmp_cfg.exec & 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); + end + end + return result; + endfunction + + // Compute permissions checks that apply when MSECCFG.MML is unset. This is the original PMP + // behaviour before Smepmp was added. + function automatic logic orig_perm_check(logic pmp_cfg_lock, + ibex_pkg::priv_lvl_e priv_mode, + logic permission_check); + return (priv_mode == PRIV_LVL_M) ? + // For M-mode, any region which matches with the L-bit clear, or with sufficient + // access permissions will be allowed + (~pmp_cfg_lock | permission_check) : + // For other modes, the lock bit doesn't matter + permission_check; + endfunction + + // Access fault determination / prioritization + function automatic logic access_fault_check (logic csr_pmp_mseccfg_mmwp, + logic [PMPNumRegions-1:0] match_all, + ibex_pkg::priv_lvl_e priv_mode, + logic [PMPNumRegions-1:0] final_perm_check); + + + // When MSECCFG.MMWP is set default deny always, otherwise allow for M-mode, deny for other + // modes + logic access_fail = csr_pmp_mseccfg_mmwp | (priv_mode != PRIV_LVL_M); + + // PMP entries are statically prioritized, from 0 to N-1 + // The lowest-numbered PMP entry which matches an address determines accessibility + for (int r = 0; r < PMPNumRegions; r++) begin + if (match_all[r]) begin + access_fail = ~final_perm_check[r]; + break; + end + end + return access_fail; + endfunction // --------------- // Access checking @@ -102,88 +204,32 @@ module ibex_pmp #( endcase end - // Check specific required permissions + // Basic permission check compares cfg register only. assign region_basic_perm_check[c][r] = ((pmp_req_type_i[c] == PMP_ACC_EXEC) & csr_pmp_cfg_i[r].exec) | ((pmp_req_type_i[c] == PMP_ACC_WRITE) & csr_pmp_cfg_i[r].write) | ((pmp_req_type_i[c] == PMP_ACC_READ) & csr_pmp_cfg_i[r].read); - // Compute permissions checks that apply when MSECCFG.MML is unset. This is the original PMP - // behaviour before Smepmp was added. - assign region_orig_perm_check[c][r] = (priv_mode_i[c] == PRIV_LVL_M) ? - // For M-mode, any region which matches with the L-bit clear, or with sufficient - // access permissions will be allowed - (~csr_pmp_cfg_i[r].lock | region_basic_perm_check[c][r]) : - // For other modes, the lock bit doesn't matter - region_basic_perm_check[c][r]; - - // Compute permission checks that apply when MSECCFG.MML is set. - always_comb begin - region_mml_perm_check[c][r] = 1'b0; - - if (!csr_pmp_cfg_i[r].read && csr_pmp_cfg_i[r].write) begin - // Special-case shared regions where R = 0, W = 1 - unique case ({csr_pmp_cfg_i[r].lock, csr_pmp_cfg_i[r].exec}) - // Read/write in M, read only in S/U - 2'b00: region_mml_perm_check[c][r] = - (pmp_req_type_i[c] == PMP_ACC_READ) | - ((pmp_req_type_i[c] == PMP_ACC_WRITE) & (priv_mode_i[c] == PRIV_LVL_M)); - // Read/write in M/S/U - 2'b01: region_mml_perm_check[c][r] = - (pmp_req_type_i[c] == PMP_ACC_READ) | (pmp_req_type_i[c] == PMP_ACC_WRITE); - // Execute only on M/S/U - 2'b10: region_mml_perm_check[c][r] = (pmp_req_type_i[c] == PMP_ACC_EXEC); - // Read/execute in M, execute only on S/U - 2'b11: region_mml_perm_check[c][r] = - (pmp_req_type_i[c] == PMP_ACC_EXEC) | - ((pmp_req_type_i[c] == PMP_ACC_READ) & (priv_mode_i[c] == PRIV_LVL_M)); - default: ; - endcase - end else begin - if (csr_pmp_cfg_i[r].read & csr_pmp_cfg_i[r].write & csr_pmp_cfg_i[r].exec - & csr_pmp_cfg_i[r].lock) begin - // Special-case shared read only region when R = 1, W = 1, X = 1, L = 1 - region_mml_perm_check[c][r] = pmp_req_type_i[c] == 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. - region_mml_perm_check[c][r] = - priv_mode_i[c] == PRIV_LVL_M ? csr_pmp_cfg_i[r].lock & region_basic_perm_check[c][r] : - ~csr_pmp_cfg_i[r].lock & region_basic_perm_check[c][r]; - end - end - end + // Check specific required permissions since the behaviour is different + // between Smepmp implementation and original PMP. + assign region_perm_check[c][r] = perm_check_wrapper(csr_pmp_mseccfg_i.mml, + csr_pmp_cfg_i[r], + pmp_req_type_i[c], + priv_mode_i[c], + region_basic_perm_check[c][r]); end - // Access fault determination / prioritization - always_comb begin - // When MSECCFG.MMWP is set default deny always, otherwise allow for M-mode, deny for other - // modes - access_fault[c] = csr_pmp_mseccfg_i.mmwp | (priv_mode_i[c] != PRIV_LVL_M); - - // PMP entries are statically prioritized, from 0 to N-1 - // The lowest-numbered PMP entry which matches an address determines accessibility - for (int r = PMPNumRegions - 1; r >= 0; r--) begin - if (region_match_all[c][r]) begin - if (csr_pmp_mseccfg_i.mml) begin - // When MSECCFG.MML is set use MML specific permission check - access_fault[c] = ~region_mml_perm_check[c][r]; - end else begin - // Otherwise use original (non Smepmp) PMP behaviour - access_fault[c] = ~region_orig_perm_check[c][r]; - end - end - end - end - - assign pmp_req_err_o[c] = access_fault[c]; + // Once the permission checks of the regions are done, decide if the access is + // denied by figuring out the matching region and its permission check. + assign pmp_req_err_o[c] = access_fault_check(csr_pmp_mseccfg_i.mmwp, + region_match_all[c], + priv_mode_i[c], + region_perm_check[c]); // Access fails check against one region but access allowed due to another higher-priority // region. `DV_FCOV_SIGNAL(logic, pmp_region_override, - ~pmp_req_err_o[c] & - |(region_match_all[c] & (csr_pmp_mseccfg_i.mml ? ~region_mml_perm_check[c] : - ~region_orig_perm_check[c]))) + ~pmp_req_err_o[c] & |(region_match_all[c] & ~region_perm_check[c])) end // RLB, rule locking bypass, is only relevant to ibex_cs_registers which controls writes to the