[v1] target/riscv: don't read write-only CSR

[PATCH] target/riscv: don't read write-only CSR

Nikita Shubin posted 1 patch 9 months, 1 week ago

Diff against v2
Download mbox

Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20230725053806.9463-1-nikita.shubin@maquefel.me

Maintainers: Palmer Dabbelt <palmer@dabbelt.com>, Alistair Francis <alistair.francis@wdc.com>, Bin Meng <bin.meng@windriver.com>, Weiwei Li <liweiwei@iscas.ac.cn>, Daniel Henrique Barboza <dbarboza@ventanamicro.com>, Liu Zhiwei <zhiwei_liu@linux.alibaba.com>

There is a newer version of this series

target/riscv/csr.c | 19 +++++++++++--------
1 file changed, 11 insertions(+), 8 deletions(-)

Expand all Fold all

[PATCH] target/riscv: don't read write-only CSR

Posted by Nikita Shubin 9 months, 1 week ago

From: Nikita Shubin <n.shubin@yadro.com>

In case of write-only CSR don't return illegal inst error when CSR is
written and lacks read op.

Signed-off-by: Nikita Shubin <n.shubin@yadro.com>
---
Anyway it not might be a good idea to read CSR when we are not asked
for, during CSRRW or CSRRWI:

"For CSRRWI, if rd=x0, then the instruction shall not read the CSR and 
shall not cause any of the side effects that might occur on a CSR read."

May be i am missing something of course.
---
 target/riscv/csr.c | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index ea7585329e..6d6a8821a8 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -3915,14 +3915,17 @@ static RISCVException riscv_csrrw_do64(CPURISCVState *env, int csrno,
         return csr_ops[csrno].op(env, csrno, ret_value, new_value, write_mask);
     }
 
-    /* if no accessor exists then return failure */
-    if (!csr_ops[csrno].read) {
-        return RISCV_EXCP_ILLEGAL_INST;
-    }
-    /* read old value */
-    ret = csr_ops[csrno].read(env, csrno, &old_value);
-    if (ret != RISCV_EXCP_NONE) {
-        return ret;
+    /* don't read if ret_value==NULL */
+    if (ret_value) {
+        /* if no accessor exists then return failure */
+        if (!csr_ops[csrno].read) {
+            return RISCV_EXCP_ILLEGAL_INST;
+        }
+        /* read old value */
+        ret = csr_ops[csrno].read(env, csrno, &old_value);
+        if (ret != RISCV_EXCP_NONE) {
+            return ret;
+        }
     }
 
     /* write value if writable and write mask set, otherwise drop writes */
-- 
2.39.2