[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[PATCH v2 02/18] target/riscv: Add some comments to clarify the priority
From: |
Bin Meng |
Subject: |
[PATCH v2 02/18] target/riscv: Add some comments to clarify the priority policy of riscv_csrrw_check() |
Date: |
Tue, 28 Feb 2023 18:40:18 +0800 |
The priority policy of riscv_csrrw_check() was once adjusted in
commit eacaf4401956 ("target/riscv: Fix priority of csr related check in
riscv_csrrw_check")
whose commit message says the CSR existence check should come before
the access control check, but the code changes did not agree with
the commit message, that the predicate() check actually came after
the read / write check.
In fact this was intentional. Add some comments there so that people
won't bother trying to change it without a solid reason.
Signed-off-by: Bin Meng <bmeng@tinylab.org>
---
Changes in v2:
- Keep the original priority policy, instead add some comments for clarification
target/riscv/csr.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 75a540bfcb..4cc2c6370f 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -3776,11 +3776,12 @@ static inline RISCVException
riscv_csrrw_check(CPURISCVState *env,
int read_only = get_field(csrno, 0xC00) == 3;
int csr_min_priv = csr_ops[csrno].min_priv_ver;
- /* ensure the CSR extension is enabled. */
+ /* ensure the CSR extension is enabled */
if (!cpu->cfg.ext_icsr) {
return RISCV_EXCP_ILLEGAL_INST;
}
+ /* privileged spec version check */
if (env->priv_ver < csr_min_priv) {
return RISCV_EXCP_ILLEGAL_INST;
}
@@ -3790,10 +3791,18 @@ static inline RISCVException
riscv_csrrw_check(CPURISCVState *env,
return RISCV_EXCP_ILLEGAL_INST;
}
+ /* read / write check */
if (write_mask && read_only) {
return RISCV_EXCP_ILLEGAL_INST;
}
+ /*
+ * The predicate() not only does existence check but also does some
+ * access control check which triggers for example virtual instruction
+ * exception in some cases. When writing read-only CSRs in those cases
+ * illegal instruction exception should be triggered instead of virtual
+ * instruction exception. Hence this comes after the read / write check.
+ */
RISCVException ret = csr_ops[csrno].predicate(env, csrno);
if (ret != RISCV_EXCP_NONE) {
return ret;
--
2.25.1
- [PATCH v2 00/18] target/riscv: Various fixes to gdbstub and CSR access, Bin Meng, 2023/02/28
- [PATCH v2 01/18] target/riscv: gdbstub: Check priv spec version before reporting CSR, Bin Meng, 2023/02/28
- [PATCH v2 02/18] target/riscv: Add some comments to clarify the priority policy of riscv_csrrw_check(),
Bin Meng <=
- [PATCH v2 03/18] target/riscv: Use g_assert() for the predicate() NULL check, Bin Meng, 2023/02/28
- [PATCH v2 04/18] target/riscv: gdbstub: Minor change for better readability, Bin Meng, 2023/02/28
- [PATCH v2 05/18] target/riscv: gdbstub: Do not generate CSR XML if Zicsr is disabled, Bin Meng, 2023/02/28
- [PATCH v2 06/18] target/riscv: Coding style fixes in csr.c, Bin Meng, 2023/02/28
- [PATCH v2 07/18] target/riscv: Use 'bool' type for read_only, Bin Meng, 2023/02/28
- [PATCH v2 08/18] target/riscv: Simplify {read, write}_pmpcfg() a little bit, Bin Meng, 2023/02/28
- [PATCH v2 09/18] target/riscv: Simplify getting RISCVCPU pointer from env, Bin Meng, 2023/02/28
- [PATCH v2 10/18] target/riscv: Avoid reporting odd-numbered pmpcfgX in the CSR XML for RV64, Bin Meng, 2023/02/28