Side effects of executing mret with mcause.minhv==1

[silabs-oysteink]

In the CLIC specification there is pseudo code for what happens when an mret instruction is executed with and without minhv set. In case of a trap on the resulting table fetch, additional state changes will occur. I added some more code to the "take table-fetch trap" to highlight where things seem unclear and added numbered arcs which I will reference here:

1. In the pseudo code mstatus.MIE is set to mstatus.MPIE (as for a regular mret). Then the updated MIE is used to set MPIE if a trap occurs.
2. The privilege level is updated to mstatus.MPP before the table fetch for permission checking (this is well explained in the spec). If we take an exception for the table fetch this new privilege is what we store in mstatus.mpp. This could be a different privilege than what the mret instruction runs in, can this be a problem?
3. Changing privilege before performing the table fetch is well explained in the spec.

Also, the 'prev_priv' is not used the original pseudo code?

It seems like the mret should update CSRs only once, would the following pseudo code work?

function exception_handler(cur_priv, xret, pc) { match (xret) {

  MRET =>  {
  let xepc = prepare_xret_target(cur_priv);
  let xinhv = prepare_xinhv_target(cur_priv);

  let prev_priv  = cur_priv;   // Unused?
  cur_priv       = mstatus.MPP;

  if xinhv
  then {
    if (check_fetch_permissions(xepc) = Addr_OK) {
        next_pc = mem_read(xepc) & ~1; /* xepc contains an address of a table entry */

        // mret side effects
        mstatus.MIE    = mstatus.MPIE;
        mstatus.MPIE   = 1;
        mstatus.MPP    = lowest_supported_priv;
        // other side effects of mret (mepc etc)
      } else {
        /* take table-fetch trap */
        mstatus.MPIE = mstatus.MIE;
        mstatus.MIE  = 0;
        mstatus.MPP  = cur_priv;
        cur_priv     = Machine;
        mcause.minhv = 1

        // other side effects of taking a trap
      }
    } else { /* Standard xRET behavior - xepc becomes next_pc */
      next_pc = xepc & ~1;

      // mret side effects
      mstatus.MIE    = mstatus.MPIE;
      mstatus.MPIE   = 1;
      mstatus.MPP    = lowest_supported_priv;
      // other side effects of mret (mepc etc)
    }
  }
}

}

[dansmathers]

For reference: Here is the source sail code I was trying to leverage for pseudo-code:

https://github.com/riscv/sail-riscv/blob/master/model/riscv_sys_control.sail function exception_handler(cur_priv : Privilege, ctl : ctl_result, pc: xlenbits) -> xlenbits = { match (curpriv, ctl) { ... (, CTL_MRET()) => { let prev_priv = cur_privilege; mstatus->MIE() = mstatus.MPIE(); mstatus->MPIE() = 0b1; cur_privilege = privLevel_of_bits(mstatus.MPP()); mstatus->MPP() = privLevel_to_bits(if haveUsrMode() then User else Machine); if cur_privilege != Machine then mstatus->MPRV() = 0b0; cancel_reservation(); prepare_xret_target(Machine) & pc_alignment_mask() },

[kasanovic]

Pseudo-code needs to align xepc value to XLEN/8 byte boundary by ignoring low-order bits. Code is only for M-mode (MRET cannot be executed in other modes) so pseudo-code could maybe clarified to indicate where things will actually be M-mode and not a parameter.

[Silabs-ArjanB]

Hi @dansmathers , the Sail code you reference is essentially different in the fact that this original MRET specification does not deal with taking a trap during an MRET-sub-operation. In CLIC we have this 'sub-operation' that deals with performing a table look-up i xinhv is set and that sub-operation can cause a trap.

The part of the currently specified MRET behavior in the CLIC specification that we question is that if an MRET experiences such a (xinhv table jump related) trap, that part of the architectural state (i.e. mstatus.mie and cur_priv in this case) has already been updated (according to the current CLIC spec) after which the trap is taken (and the 'take the trap code' relies on the already updated mstatus.mie and cur_priv instead of on the mstatus.mie and cur_priv from before the execution attempt of the MRET instruction).

Is that 'partial execution' behavior really what you intend to happen? (I think it is cleaner if instructions either fully execute or trap, and never 'partially execute and trap'. Of course there are exceptions to that such as for example the Zc pop instruction; we simply wonder if MRET is purposely defined to allow this 'partial execute and trap' behavior. If so, then that would at least deserve a note in the CLIC specification and also the part in the pseudocode that currently states /take table-fetch trap / should be written out so that the partial execution behavior becomes really clear.

Best regards, Arjan

[dansmathers]

I think the idea is that MRET fully executes. mepc on a trap due to failing fetch_permissions should contain the mepc_inhv/mepc value, not the MRET instruction location. I think that is correct that the take-the-trap pseudo-code relies on the already updated mstatus.mie and cur_priv. those are the settings from before taking an exception that is being tried again (context has been resumed) so I don't think I would describe that as partial execution. Am I understanding your point? (we will try to discuss tomorrow in our TG meeting).

[Silabs-ArjanB]

Hello @dansmathers, thank you for your reply. Øystein and I discussed your repsonse and we agree with you (and the current CLIC spec). @silabs-oysteink could you please close this issue if agreed that the CLIC documentation is actually correct here?

[silabs-oysteink]

Yes, I think the CLIC spec is correct as it is, closing issue.

[dansmathers]

reopening for tracking of pull #363. I think we are planning on discussing this in today's TG.