[fcov] Add fcov for the interaction of pmp with debug module accesses

hcallahan-lowrisc · hcallahan-lowrisc · commit ab9bca09e9f7 · 2025-05-22T10:40:31.000+01:00
Accesses to the debug module in debug mode should never be denied by the PMP
unit. This commit implements fcov to confirm we have stimulated this particular
behaviour in relevant related states.

Illegal bins are used for incorrect behaviour (e.g. denied access in debug mode)
Other behaviours such as debug module accesses outside of debug mode are left as
ignore_bins for now. This is not explicitly disallowed by the specification, and
our implementation does not have any opinion about its validity, but external
debug modules opine that it should not be allowed. We could possibly expand the
stimulus in the future to test this condition, but it is low priority.
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
@@ -20,6 +20,8 @@ interface core_ibex_pmp_fcov_if import ibex_pkg::*; #(
   input logic                pmp_req_err     [3],
   input pmp_mseccfg_t        csr_pmp_mseccfg,
 
+  input logic debug_mode,
+
   input logic data_req_out,
 
   input fcov_csr_write
@@ -136,6 +138,14 @@ interface core_ibex_pmp_fcov_if import ibex_pkg::*; #(
     assign pmp_dside_boundary_cross = |(pmp_dside_match ^ pmp_dside_match_last) &
                                       load_store_unit_i.fcov_ls_second_req;
 
+    logic pmp_iside_access_fault_check;
+    logic pmp_iside2_access_fault_check;
+    logic pmp_dside_access_fault_check;
+
+    assign pmp_iside_access_fault_check  = g_pmp.pmp_i.access_fault_check_res[PMP_I];
+    assign pmp_iside2_access_fault_check = g_pmp.pmp_i.access_fault_check_res[PMP_I2];
+    assign pmp_dside_access_fault_check  = g_pmp.pmp_i.access_fault_check_res[PMP_D];
+
     for (genvar i_region = 0; i_region < PMPNumRegions; i_region += 1) begin : g_pmp_region_fcov
       pmp_priv_bits_e pmp_region_priv_bits;
       pmp_priv_bits_e pmp_region_priv_bits_wr;
@@ -237,7 +247,8 @@ interface core_ibex_pmp_fcov_if import ibex_pkg::*; #(
           cross cp_region_priv_bits, cp_req_type_iside, cp_priv_lvl_iside, pmp_iside_req_err
             iff (g_pmp_fcov_signals.g_pmp_region_fcov[i_region].fcov_pmp_region_ichan_access &&
                  g_pmp_fcov_signals.fcov_pmp_region_ichan_priority[i_region]                 &&
-                 csr_pmp_cfg[i_region].mode != PMP_MODE_OFF) {
+                 csr_pmp_cfg[i_region].mode != PMP_MODE_OFF                                  &&
+                 !(g_pmp_fcov_signals.fcov_access_attempted_into_dm[PMP_I])) {
 
             // Will never see a succesful exec access when execute is disallowed
             illegal_bins illegal_allow_exec =
@@ -294,7 +305,8 @@ interface core_ibex_pmp_fcov_if import ibex_pkg::*; #(
           cross cp_region_priv_bits, cp_req_type_iside2, cp_priv_lvl_iside2, pmp_iside2_req_err
             iff (g_pmp_fcov_signals.g_pmp_region_fcov[i_region].fcov_pmp_region_ichan2_access &&
                  g_pmp_fcov_signals.fcov_pmp_region_ichan2_priority[i_region]                 &&
-                 csr_pmp_cfg[i_region].mode != PMP_MODE_OFF) {
+                 csr_pmp_cfg[i_region].mode != PMP_MODE_OFF                                   &&
+                   !(g_pmp_fcov_signals.fcov_access_attempted_into_dm[PMP_I2])) {
 
           // Will never see a succesful exec access when execute is disallowed
           illegal_bins illegal_allow_exec =
@@ -351,7 +363,8 @@ interface core_ibex_pmp_fcov_if import ibex_pkg::*; #(
           cross cp_region_priv_bits, cp_req_type_dside, cp_priv_lvl_dside, pmp_dside_req_err
             iff (g_pmp_fcov_signals.g_pmp_region_fcov[i_region].fcov_pmp_region_dchan_access &&
                  g_pmp_fcov_signals.fcov_pmp_region_dchan_priority[i_region]                 &&
-                 csr_pmp_cfg[i_region].mode != PMP_MODE_OFF) {
+                 csr_pmp_cfg[i_region].mode != PMP_MODE_OFF                                  &&
+                 !(g_pmp_fcov_signals.fcov_access_attempted_into_dm[PMP_D])) {
 
           // Will never see a succesful read access when read is disallowed
           illegal_bins illegal_allow_read =
@@ -709,6 +722,108 @@ interface core_ibex_pmp_fcov_if import ibex_pkg::*; #(
       misaligned_lsu_access_cross: cross misaligned_pmp_err_last,
                                          load_store_unit_i.fcov_ls_mis_pmp_err_2
                                    iff (pmp_dside_boundary_cross);
+
+      // Debug Module Accesses
+
+      // Coverpoints that we have attempted to access the debug module address range
+      //
+      // The riscv-dbg module states the following:
+      // > riscv-dbg/doc/debug-system.md (# Debug Memory)
+      // > The Debug Memory should only be accessible from the CPU if it is in debug mode.
+      // However, The RISC-V Debug Specification does not mandate this. Bins for accesses in normal
+      // mode are ignored for the purpose of this coverage.
+
+      dm_fetch_iside_debug_mode_cp: coverpoint debug_mode
+        iff (g_pmp_fcov_signals.fcov_access_attempted_into_dm[PMP_I])
+      {
+        bins dm_debug_mode_fetch = {1'b1};
+        // We shouldn't be fetching from the debug module unless in debug mode.
+        ignore_bins dm_normal_mode_fetch = {1'b0};
+      }
+      dm_fetch_iside2_debug_mode_cp: coverpoint debug_mode
+        iff (g_pmp_fcov_signals.fcov_access_attempted_into_dm[PMP_I2])
+      {
+        bins dm_debug_mode_fetch = {1'b1};
+        // We shouldn't be fetching from the debug module unless in debug mode.
+        ignore_bins dm_normal_mode_fetch = {1'b0};
+      }
+      dm_load_store_debug_mode_cp: coverpoint debug_mode
+        iff (g_pmp_fcov_signals.fcov_access_attempted_into_dm[PMP_D])
+          {
+        bins dm_debug_mode_load_store = {1'b1};
+        // We shouldn't be loading/storing from/to the debug module unless in debug mode.
+        ignore_bins dm_normal_mode_load_store = {1'b0};
+      }
+
+      cp_ls_pmp_exception: coverpoint load_store_unit_i.fcov_ls_pmp_exception;
+
+      // Cross access attempts with the result of the access check
+
+      dm_fetch_access_iside_cross: cross
+        dm_fetch_iside_debug_mode_cp,
+        pmp_iside_req_err,
+        pmp_iside_access_fault_check
+      {
+        // Fetches should never fail the access check in debug mode
+        illegal_bins dm_debug_mode_disallowed_fetch =
+          binsof(pmp_iside_req_err) intersect {1'b1} &&
+          binsof(dm_fetch_iside_debug_mode_cp) intersect {1'b1};
+
+        // Allowed fetches in debug mode may or may not override a denying PMP region.
+        // Create a bin for each possibility.
+        bins dm_debug_mode_pmp_allow_fetch_allowed =
+          binsof(dm_fetch_iside_debug_mode_cp) intersect {1'b1} &&
+          binsof(pmp_iside_access_fault_check) intersect {1'b0} &&
+          binsof(pmp_iside_req_err) intersect {1'b0};
+        bins dm_debug_mode_pmp_deny_fetch_allowed =
+          binsof(dm_fetch_iside_debug_mode_cp) intersect {1'b1} &&
+          binsof(pmp_iside_access_fault_check) intersect {1'b1} &&
+          binsof(pmp_iside_req_err) intersect {1'b0};
+      }
+
+      dm_fetch_access_iside2_cross: cross
+        dm_fetch_iside2_debug_mode_cp,
+        pmp_iside2_req_err,
+        pmp_iside2_access_fault_check
+      {
+        // Fetches should never fail the access check in debug mode
+        illegal_bins dm_debug_mode_disallowed_fetch =
+          binsof(pmp_iside2_req_err) intersect {1'b1} &&
+          binsof(dm_fetch_iside2_debug_mode_cp) intersect {1'b1};
+
+        // Allowed fetches in debug mode may or may not override a denying PMP region.
+        // Create a bin for each possibility.
+        bins dm_debug_mode_pmp_allow_fetch_allowed =
+          binsof(dm_fetch_iside2_debug_mode_cp) intersect {1'b1} &&
+          binsof(pmp_iside2_access_fault_check) intersect {1'b0} &&
+          binsof(pmp_iside2_req_err) intersect {1'b0};
+        bins dm_debug_mode_pmp_deny_fetch_allowed =
+          binsof(dm_fetch_iside2_debug_mode_cp) intersect {1'b1} &&
+          binsof(pmp_iside2_access_fault_check) intersect {1'b1} &&
+          binsof(pmp_iside2_req_err) intersect {1'b0};
+      }
+
+      dm_load_store_access_cross: cross
+        dm_load_store_debug_mode_cp,
+        cp_ls_pmp_exception,
+        pmp_dside_access_fault_check
+      {
+        // Loads/Stores should never fail the access check in debug mode
+        illegal_bins dm_debug_mode_disallowed_load_store =
+          binsof(cp_ls_pmp_exception) intersect {1'b1} &&
+          binsof(dm_load_store_debug_mode_cp) intersect {1'b1};
+
+        // Allowed loads/stores in debug mode may or may not override a denying PMP region.
+        // Create a bin for each possibility.
+        bins dm_debug_mode_pmp_allow_allowed_load_store =
+          binsof(dm_load_store_debug_mode_cp) intersect {1'b1} &&
+          binsof(pmp_dside_access_fault_check) intersect {1'b1} &&
+          binsof(cp_ls_pmp_exception) intersect {1'b0};
+        bins dm_debug_mode_pmp_deny_allowed_load_store =
+          binsof(dm_load_store_debug_mode_cp) intersect {1'b1} &&
+          binsof(pmp_dside_access_fault_check) intersect {1'b0} &&
+          binsof(cp_ls_pmp_exception) intersect {1'b0};
+      }
     endgroup
 
     `DV_FCOV_INSTANTIATE_CG(pmp_top_cg, en_pmp_fcov)
diff --git a/rtl/ibex_core.sv b/rtl/ibex_core.sv
@@ -1944,6 +1944,20 @@ module ibex_core import ibex_pkg::*; #(
           g_pmp.pmp_i.region_match_all[PMP_D][i_region];
       end
     end
+
+    // Create a signal 'fcov_access_attempted_into_dm' for each PMP channel, which becomes true at
+    // the point when a memory access into the debug_module address space via that channel is
+    // accesses-checked by it's PMP unit. If true we know that the current instruction at least
+    // attempted to make an access, and we can then cross this signal with the result of the pmp
+    // check and the current debug_mode state to capture all appropriate coverage.
+    logic [PMPNumChan-1:0] access_check_into_dm;
+    for (genvar c = 0; c < PMPNumChan; c++) begin : g_pmp_channel_access_check
+      assign access_check_into_dm[c] = (g_pmp.pmp_req_addr[c][31:0] & ~DmAddrMask) == DmBaseAddr;
+    end
+    `DV_FCOV_SIGNAL(logic [PMPNumChan-1:0], access_attempted_into_dm,
+                    {access_check_into_dm[PMP_D]  & data_req_out,                  // [2]
+                     access_check_into_dm[PMP_I2] & if_stage_i.if_id_pipe_reg_we,  // [1]
+                     access_check_into_dm[PMP_I]  & if_stage_i.if_id_pipe_reg_we}) // [0]
   end
 `endif
 
diff --git a/rtl/ibex_pmp.sv b/rtl/ibex_pmp.sv
@@ -41,6 +41,7 @@ module ibex_pmp #(
   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_perm_check;
+  logic [PMPNumChan-1:0]                      access_fault_check_res;
   logic [PMPNumChan-1:0]                      debug_mode_allowed_access;
 
   ///////////////////////
@@ -241,14 +242,14 @@ module ibex_pmp #(
 
     // 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.
-    // No error is raised if the access is allowed as Debug Module access (first term).
-    assign pmp_req_err_o[c] = ~debug_mode_allowed_access[c] &
-                              access_fault_check(csr_pmp_mseccfg_i.mmwp,
-                                                 csr_pmp_mseccfg_i.mml,
-                                                 pmp_req_type_i[c],
-                                                 region_match_all[c],
-                                                 priv_mode_i[c],
-                                                 region_perm_check[c]);
+    // No error is raised if the access is allowed via the Debug Module in Debug Mode exception.
+    assign access_fault_check_res[c] = access_fault_check(csr_pmp_mseccfg_i.mmwp,
+                                                          csr_pmp_mseccfg_i.mml,
+                                                          pmp_req_type_i[c],
+                                                          region_match_all[c],
+                                                          priv_mode_i[c],
+                                                          region_perm_check[c]);
+    assign pmp_req_err_o[c] = ~debug_mode_allowed_access[c] & access_fault_check_res[c];
 
     // Access fails check against one region but access allowed due to another higher-priority
     // region.
