Re: [PATCH v12 11/20] target/riscv: introduce ssp and enabling controls for zicfiss

[Deepak Gupta <debug@rivosinc.com>]

On Fri, Aug 30, 2024 at 03:20:04PM +1000, Richard Henderson wrote:
>On 8/30/24 09:34, Deepak Gupta wrote:
>>+bool cpu_get_bcfien(CPURISCVState *env)
>
>It occurs to me that a better name would be "cpu_get_sspen".
>The backward cfi is merely a consequence of the shadow stack.

Want me to change cpu_get_fcfien as well to cpu_get_lpen ?

>
>>+{
>>+    /* no cfi extension, return false */
>>+    if (!env_archcpu(env)->cfg.ext_zicfiss) {
>>+        return false;
>>+    }
>>+
>>+    switch (env->priv) {
>>+    case PRV_U:
>>+        if (riscv_has_ext(env, RVS)) {
>>+            return env->senvcfg & SENVCFG_SSE;
>>+        }
>>+        return env->menvcfg & MENVCFG_SSE;
>>+#ifndef CONFIG_USER_ONLY
>>+    case PRV_S:
>>+        if (env->virt_enabled) {
>>+            return env->henvcfg & HENVCFG_SSE;
>>+        }
>>+        return env->menvcfg & MENVCFG_SSE;
>>+    case PRV_M: /* M-mode shadow stack is always on if hart implements */
>>+        return true;
>
>From the manual:
>
>Activating Zicfiss in M-mode is currently not supported. Additionally, when S-mode is not
>implemented, activation in U-mode is also not supported.

Hmm. This is a good catch.
It seems I was using an earlier spec and missed this. Or atleast
I didn't bother to check assuming that spec didn't change.
Thanks for catching it and pointing it out.

I'll fix the M-mode case by return false.

For case of,
If S-mode is not implemented then activation in U-mode is also not supported. I am thinking of
making this check during extensions validation (in case user turned zicfiss=true).
Below options:

Option 1
In `riscv_cpu_validate_set_extensions`, check for S and if S is not implemented then error out
saying that zicfiss can't be turned on.

OR

Option 2
In `riscv_cpu_validate_set_extensions`, check for S and if S is not implemented then silently
clear `ext_zicfiss` in cpu config.

I think option 1 is better.

>
>So two of the cases above are wrong.
>
>
>r~