Possible ISA incompliance in PMP

[kangoojim]

Hi,
I possibly found behaviour in the PMP that is not compliant with the Privileged ISA Specification.

The current implementation (if I understand it correctly) first checks for each rule if it is enabled and if the R/W permissions of the rule fit the write enable signal of the current access. In case the rule permits the requested access, it is checked whether the access matches the rule's region. (See the following code snippet, lines 577 - 582:)

  for(j=0;j<N_PMP_ENTRIES;j++)
      begin

         if( EN_rule[j] & ((~data_we_i & R_rule[j]) | (data_we_i & W_rule[j])) )
         begin
             case(MODE_rule[j])
                  //checking region of rule for TOR, NA4 and NAPOT modes

If at least one region is matched this way, the access is granted and no error is raised. If no region is matched and the current privilege level is not machine mode, the PMP raises an error and denies the access. (See lines 644 - 668:)

   always_comb
   begin
      if(pmp_privil_mode_i == PRIV_LVL_M)
      begin
         data_req_o   = data_req_i;
         data_gnt_o   = data_gnt_i;
         data_err_int   = 1'b0;

      end
      else
      begin
            if(|data_match_region == 1'b0)
            begin
               data_req_o   = 1'b0;
               data_err_int   = data_req_i;
               data_gnt_o   = 1'b0;
            end
            else
            begin
               data_req_o   =  data_req_i;
               data_err_int =  1'b0;
               data_gnt_o   =  data_gnt_i;
            end
      end
   end

This behaviour is not compliant with the Privileged ISA Specification (20190608-Priv-MSU-Ratified) in the following two points:

1. If no PMP rule is enabled, all user privilege mode accesses are denied. The ISA requires the opposite, which is implied in the following quote from the ISA (page 52):

If no PMP entry matches an S-mode or U-mode access, but at least one PMP entry is implemented, the access fails.

2. In the current implementation, one matching region which permits the access overrules all other rules that would match the requested access and deny it. The ISA, however, specifies that in case of multiple matches, the one with the lowest index has priority. (See quote from page 52:)

PMP entries are statically prioritized. The lowest-numbered PMP entry that matches any byte of an access determines whether that access succeeds or fails.

The same issues exist analogously for instruction accesses.

For a fix, the region matching could be conducted independently of the permission checking for each rule. In this way, the result from the permission checking of the highest priority match could be used to determine whether the access is granted or denied - even if the result is negative.

[Silabs-ArjanB]

Hi @kangoojim Thank you for reporting this issue. Please be aware that PMP is not included in the CV32E40P (even though the RTL file itself is present). We will of course still look into this issue, but it will most likely be during the follow-up project CV32E40 (which will re-introduce the PMP functionality).

[kangoojim]

Thanks for the quick reply! I was not aware of this. I found the issue while working with the pulpissimo platform (which fetches its core from this repo, an older commit though). Maybe it is worthwhile noting, that pulpissimo still uses this core with the PMP enabled by default.

[Silabs-ArjanB]

Hi @kangoojim Yes, when this core was still called RI5CY the PMP was supported, so it makes sense that you can use it if you are on an older commit. The good news is that it will be supported again; the bad news is that it will take some time (and it will be on another core, CV32E40 instead of CV32E40P).