Re: [PATCH v3 04/20] target/riscv: save and restore elp state on priv transitions

[Deepak Gupta <debug@rivosinc.com>]

On Thu, Aug 08, 2024 at 08:40:08AM +1000, Richard Henderson wrote:
>On 8/8/24 06:11, Deepak Gupta wrote:
>>>>+    /*
>>>>+     * If forward cfi enabled for new priv, restore elp status
>>>>+     * and clear spelp in mstatus
>>>>+     */
>>>>+    if (cpu_get_fcfien(env)) {
>>>>+        env->elp = get_field(env->mstatus, MSTATUS_SPELP);
>>>>+        env->mstatus = set_field(env->mstatus, MSTATUS_SPELP, 0);
>>>>+    }
>>>
>>>The spec is perhaps poorly written here.  I read
>>>
>>> ... if xPP holds the value y, then ELP is set to the value of xPELP if yLPE is 1;
>>> otherwise, it is set to NO_LP_EXPECTED; xPELP is set to NO_LP_EXPECTED.
>>>
>>>as xPELP always being cleared, regardless of yLPE.
>>
>>Yes that's what code above is also doing. restore elp status from SPELP field and clear
>>it at SPELP.
>
>No, my point is that the text doesn't seem to be
>
>  if (enabled) {
>    restore elp
>    clear pelp
>  }
>
>but
>
>  if (enabled) {
>    restore elp
>  }
>  clear pelp
>
>I.e. the clear is unconditional.

hmm. that's right. 
good catch here.

>
>
>r~