qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Deepak Gupta
Subject: Re: [PATCH v12 11/20] target/riscv: introduce ssp and enabling controls for zicfiss
Date: Thu, 29 Aug 2024 22:56:41 -0700

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~



reply via email to

[Prev in Thread] Current Thread [Next in Thread]