* [PATCH 00/18] target/riscv: Various fixes to gdbstub and CSR access
@ 2023-02-13 18:01 Bin Meng
2023-02-13 18:01 ` [PATCH 01/18] target/riscv: gdbstub: Check priv spec version before reporting CSR Bin Meng
` (18 more replies)
0 siblings, 19 replies; 57+ messages in thread
From: Bin Meng @ 2023-02-13 18:01 UTC (permalink / raw)
To: qemu-devel
Cc: Alistair Francis, Bin Meng, Daniel Henrique Barboza, Liu Zhiwei,
Palmer Dabbelt, Weiwei Li, qemu-riscv
At present gdbstub reports an incorrect / incomplete CSR list in the
target description XML, for example:
- menvcfg is reported in 'sifive_u' machine
- fcsr is missing in a F/D enabled processor
The issue is caused by:
- priv spec version check is missing when reporting CSRs
- CSR predicate() routine is called without turning on the debugger flag
This series aims to generate a correct and complete CSR list for gdbstub.
Bin Meng (18):
target/riscv: gdbstub: Check priv spec version before reporting CSR
target/riscv: Correct the priority policy of riscv_csrrw_check()
target/riscv: gdbstub: Minor change for better readability
target/riscv: gdbstub: Do not generate CSR XML if Zicsr is disabled
target/riscv: Coding style fixes in csr.c
target/riscv: Use 'bool' type for read_only
target/riscv: Simplify {read,write}_pmpcfg() a little bit
target/riscv: Simplify getting RISCVCPU pointer from env
target/riscv: Avoid reporting odd-numbered pmpcfgX in the CSR XML for
RV64
target/riscv: gdbstub: Turn on debugger mode before calling CSR
predicate()
target/riscv: gdbstub: Drop the vector CSRs in riscv-vector.xml
target/riscv: Allow debugger to access user timer and counter CSRs
target/riscv: Allow debugger to access seed CSR
target/riscv: Allow debugger to access {h,s}stateen CSRs
target/riscv: Allow debugger to access sstc CSRs
target/riscv: Drop priv level check in mseccfg predicate()
target/riscv: Group all predicate() routines together
target/riscv: Move configuration check to envcfg CSRs predicate()
target/riscv/csr.c | 360 +++++++++++++++++++++++------------------
target/riscv/gdbstub.c | 100 +++---------
2 files changed, 221 insertions(+), 239 deletions(-)
--
2.25.1
^ permalink raw reply [flat|nested] 57+ messages in thread
* [PATCH 01/18] target/riscv: gdbstub: Check priv spec version before reporting CSR
2023-02-13 18:01 [PATCH 00/18] target/riscv: Various fixes to gdbstub and CSR access Bin Meng
@ 2023-02-13 18:01 ` Bin Meng
2023-02-14 8:40 ` weiwei
2023-02-17 2:11 ` LIU Zhiwei
2023-02-13 18:01 ` [PATCH 02/18] target/riscv: Correct the priority policy of riscv_csrrw_check() Bin Meng
` (17 subsequent siblings)
18 siblings, 2 replies; 57+ messages in thread
From: Bin Meng @ 2023-02-13 18:01 UTC (permalink / raw)
To: qemu-devel
Cc: Alistair Francis, Bin Meng, Daniel Henrique Barboza, Liu Zhiwei,
Palmer Dabbelt, Weiwei Li, qemu-riscv
The gdbstub CSR XML is dynamically generated according to the result
of the CSR predicate() result. This has been working fine until
commit 7100fe6c2441 ("target/riscv: Enable privileged spec version 1.12")
introduced the privilege spec version check in riscv_csrrw_check().
When debugging the 'sifive_u' machine whose priv spec is at 1.10,
gdbstub reports priv spec 1.12 CSRs like menvcfg in the XML, hence
we see "remote failure reply 'E14'" message when examining all CSRs
via "info register system" from gdb.
Add the priv spec version check in the CSR XML generation logic to
fix this issue.
Fixes: 7100fe6c2441 ("target/riscv: Enable privileged spec version 1.12")
Signed-off-by: Bin Meng <bmeng@tinylab.org>
---
target/riscv/gdbstub.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/target/riscv/gdbstub.c b/target/riscv/gdbstub.c
index 6e7bbdbd5e..e57372db38 100644
--- a/target/riscv/gdbstub.c
+++ b/target/riscv/gdbstub.c
@@ -290,6 +290,9 @@ static int riscv_gen_dynamic_csr_xml(CPUState *cs, int base_reg)
g_string_append_printf(s, "<feature name=\"org.gnu.gdb.riscv.csr\">");
for (i = 0; i < CSR_TABLE_SIZE; i++) {
+ if (env->priv_ver < csr_ops[i].min_priv_ver) {
+ continue;
+ }
predicate = csr_ops[i].predicate;
if (predicate && (predicate(env, i) == RISCV_EXCP_NONE)) {
if (csr_ops[i].name) {
--
2.25.1
^ permalink raw reply related [flat|nested] 57+ messages in thread
* [PATCH 02/18] target/riscv: Correct the priority policy of riscv_csrrw_check()
2023-02-13 18:01 [PATCH 00/18] target/riscv: Various fixes to gdbstub and CSR access Bin Meng
2023-02-13 18:01 ` [PATCH 01/18] target/riscv: gdbstub: Check priv spec version before reporting CSR Bin Meng
@ 2023-02-13 18:01 ` Bin Meng
2023-02-14 8:43 ` weiwei
2023-02-17 2:15 ` LIU Zhiwei
2023-02-13 18:01 ` [PATCH 03/18] target/riscv: gdbstub: Minor change for better readability Bin Meng
` (16 subsequent siblings)
18 siblings, 2 replies; 57+ messages in thread
From: Bin Meng @ 2023-02-13 18:01 UTC (permalink / raw)
To: qemu-devel
Cc: Alistair Francis, Bin Meng, Daniel Henrique Barboza, Liu Zhiwei,
Palmer Dabbelt, Weiwei Li, qemu-riscv
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 came
after the read / write check.
Fixes: eacaf4401956 ("target/riscv: Fix priority of csr related check in riscv_csrrw_check")
Signed-off-by: Bin Meng <bmeng@tinylab.org>
---
target/riscv/csr.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 1b0a0c1693..c2dd9d5af0 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -3793,15 +3793,15 @@ static inline RISCVException riscv_csrrw_check(CPURISCVState *env,
return RISCV_EXCP_ILLEGAL_INST;
}
- if (write_mask && read_only) {
- return RISCV_EXCP_ILLEGAL_INST;
- }
-
RISCVException ret = csr_ops[csrno].predicate(env, csrno);
if (ret != RISCV_EXCP_NONE) {
return ret;
}
+ if (write_mask && read_only) {
+ return RISCV_EXCP_ILLEGAL_INST;
+ }
+
#if !defined(CONFIG_USER_ONLY)
int csr_priv, effective_priv = env->priv;
--
2.25.1
^ permalink raw reply related [flat|nested] 57+ messages in thread
* [PATCH 03/18] target/riscv: gdbstub: Minor change for better readability
2023-02-13 18:01 [PATCH 00/18] target/riscv: Various fixes to gdbstub and CSR access Bin Meng
2023-02-13 18:01 ` [PATCH 01/18] target/riscv: gdbstub: Check priv spec version before reporting CSR Bin Meng
2023-02-13 18:01 ` [PATCH 02/18] target/riscv: Correct the priority policy of riscv_csrrw_check() Bin Meng
@ 2023-02-13 18:01 ` Bin Meng
2023-02-14 8:45 ` weiwei
2023-02-17 2:20 ` LIU Zhiwei
2023-02-13 18:02 ` [PATCH 04/18] target/riscv: gdbstub: Do not generate CSR XML if Zicsr is disabled Bin Meng
` (15 subsequent siblings)
18 siblings, 2 replies; 57+ messages in thread
From: Bin Meng @ 2023-02-13 18:01 UTC (permalink / raw)
To: qemu-devel
Cc: Alistair Francis, Bin Meng, Daniel Henrique Barboza, Liu Zhiwei,
Palmer Dabbelt, Weiwei Li, qemu-riscv
Use a variable 'base_reg' to represent cs->gdb_num_regs so that
the call to ricsv_gen_dynamic_vector_xml() can be placed in one
single line for better readability.
Signed-off-by: Bin Meng <bmeng@tinylab.org>
---
target/riscv/gdbstub.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/target/riscv/gdbstub.c b/target/riscv/gdbstub.c
index e57372db38..704f3d6922 100644
--- a/target/riscv/gdbstub.c
+++ b/target/riscv/gdbstub.c
@@ -385,9 +385,9 @@ void riscv_cpu_register_gdb_regs_for_features(CPUState *cs)
32, "riscv-32bit-fpu.xml", 0);
}
if (env->misa_ext & RVV) {
+ int base_reg = cs->gdb_num_regs;
gdb_register_coprocessor(cs, riscv_gdb_get_vector, riscv_gdb_set_vector,
- ricsv_gen_dynamic_vector_xml(cs,
- cs->gdb_num_regs),
+ ricsv_gen_dynamic_vector_xml(cs, base_reg),
"riscv-vector.xml", 0);
}
switch (env->misa_mxl_max) {
--
2.25.1
^ permalink raw reply related [flat|nested] 57+ messages in thread
* [PATCH 04/18] target/riscv: gdbstub: Do not generate CSR XML if Zicsr is disabled
2023-02-13 18:01 [PATCH 00/18] target/riscv: Various fixes to gdbstub and CSR access Bin Meng
` (2 preceding siblings ...)
2023-02-13 18:01 ` [PATCH 03/18] target/riscv: gdbstub: Minor change for better readability Bin Meng
@ 2023-02-13 18:02 ` Bin Meng
2023-02-14 8:46 ` weiwei
2023-02-17 2:23 ` LIU Zhiwei
2023-02-13 18:02 ` [PATCH 05/18] target/riscv: Coding style fixes in csr.c Bin Meng
` (14 subsequent siblings)
18 siblings, 2 replies; 57+ messages in thread
From: Bin Meng @ 2023-02-13 18:02 UTC (permalink / raw)
To: qemu-devel
Cc: Alistair Francis, Bin Meng, Daniel Henrique Barboza, Liu Zhiwei,
Palmer Dabbelt, Weiwei Li, qemu-riscv
There is no need to generate the CSR XML if the Zicsr extension
is not enabled.
Signed-off-by: Bin Meng <bmeng@tinylab.org>
---
target/riscv/gdbstub.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/target/riscv/gdbstub.c b/target/riscv/gdbstub.c
index 704f3d6922..294f0ceb1c 100644
--- a/target/riscv/gdbstub.c
+++ b/target/riscv/gdbstub.c
@@ -406,7 +406,10 @@ void riscv_cpu_register_gdb_regs_for_features(CPUState *cs)
g_assert_not_reached();
}
- gdb_register_coprocessor(cs, riscv_gdb_get_csr, riscv_gdb_set_csr,
- riscv_gen_dynamic_csr_xml(cs, cs->gdb_num_regs),
- "riscv-csr.xml", 0);
+ if (cpu->cfg.ext_icsr) {
+ int base_reg = cs->gdb_num_regs;
+ gdb_register_coprocessor(cs, riscv_gdb_get_csr, riscv_gdb_set_csr,
+ riscv_gen_dynamic_csr_xml(cs, base_reg),
+ "riscv-csr.xml", 0);
+ }
}
--
2.25.1
^ permalink raw reply related [flat|nested] 57+ messages in thread
* [PATCH 05/18] target/riscv: Coding style fixes in csr.c
2023-02-13 18:01 [PATCH 00/18] target/riscv: Various fixes to gdbstub and CSR access Bin Meng
` (3 preceding siblings ...)
2023-02-13 18:02 ` [PATCH 04/18] target/riscv: gdbstub: Do not generate CSR XML if Zicsr is disabled Bin Meng
@ 2023-02-13 18:02 ` Bin Meng
2023-02-14 8:48 ` weiwei
2023-02-17 2:24 ` LIU Zhiwei
2023-02-13 18:02 ` [PATCH 06/18] target/riscv: Use 'bool' type for read_only Bin Meng
` (13 subsequent siblings)
18 siblings, 2 replies; 57+ messages in thread
From: Bin Meng @ 2023-02-13 18:02 UTC (permalink / raw)
To: qemu-devel
Cc: Alistair Francis, Bin Meng, Daniel Henrique Barboza, Liu Zhiwei,
Palmer Dabbelt, Weiwei Li, qemu-riscv
Fix various places that violate QEMU coding style:
- correct multi-line comment format
- indent to opening parenthesis
Signed-off-by: Bin Meng <bmeng@tinylab.org>
---
target/riscv/csr.c | 62 ++++++++++++++++++++++++----------------------
1 file changed, 32 insertions(+), 30 deletions(-)
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index c2dd9d5af0..cc74819759 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -963,7 +963,7 @@ static RISCVException sstc_32(CPURISCVState *env, int csrno)
}
static RISCVException read_vstimecmp(CPURISCVState *env, int csrno,
- target_ulong *val)
+ target_ulong *val)
{
*val = env->vstimecmp;
@@ -971,7 +971,7 @@ static RISCVException read_vstimecmp(CPURISCVState *env, int csrno,
}
static RISCVException read_vstimecmph(CPURISCVState *env, int csrno,
- target_ulong *val)
+ target_ulong *val)
{
*val = env->vstimecmp >> 32;
@@ -979,7 +979,7 @@ static RISCVException read_vstimecmph(CPURISCVState *env, int csrno,
}
static RISCVException write_vstimecmp(CPURISCVState *env, int csrno,
- target_ulong val)
+ target_ulong val)
{
RISCVCPU *cpu = env_archcpu(env);
@@ -996,7 +996,7 @@ static RISCVException write_vstimecmp(CPURISCVState *env, int csrno,
}
static RISCVException write_vstimecmph(CPURISCVState *env, int csrno,
- target_ulong val)
+ target_ulong val)
{
RISCVCPU *cpu = env_archcpu(env);
@@ -1020,7 +1020,7 @@ static RISCVException read_stimecmp(CPURISCVState *env, int csrno,
}
static RISCVException read_stimecmph(CPURISCVState *env, int csrno,
- target_ulong *val)
+ target_ulong *val)
{
if (riscv_cpu_virt_enabled(env)) {
*val = env->vstimecmp >> 32;
@@ -1032,7 +1032,7 @@ static RISCVException read_stimecmph(CPURISCVState *env, int csrno,
}
static RISCVException write_stimecmp(CPURISCVState *env, int csrno,
- target_ulong val)
+ target_ulong val)
{
RISCVCPU *cpu = env_archcpu(env);
@@ -1055,7 +1055,7 @@ static RISCVException write_stimecmp(CPURISCVState *env, int csrno,
}
static RISCVException write_stimecmph(CPURISCVState *env, int csrno,
- target_ulong val)
+ target_ulong val)
{
RISCVCPU *cpu = env_archcpu(env);
@@ -1342,7 +1342,8 @@ static RISCVException write_misa(CPURISCVState *env, int csrno,
/* 'E' excludes all other extensions */
if (val & RVE) {
- /* when we support 'E' we can do "val = RVE;" however
+ /*
+ * when we support 'E' we can do "val = RVE;" however
* for now we just drop writes if 'E' is present.
*/
return RISCV_EXCP_NONE;
@@ -1364,7 +1365,8 @@ static RISCVException write_misa(CPURISCVState *env, int csrno,
val &= ~RVD;
}
- /* Suppress 'C' if next instruction is not aligned
+ /*
+ * Suppress 'C' if next instruction is not aligned
* TODO: this should check next_pc
*/
if ((val & RVC) && (GETPC() & ~3) != 0) {
@@ -1833,28 +1835,28 @@ static RISCVException write_mscratch(CPURISCVState *env, int csrno,
}
static RISCVException read_mepc(CPURISCVState *env, int csrno,
- target_ulong *val)
+ target_ulong *val)
{
*val = env->mepc;
return RISCV_EXCP_NONE;
}
static RISCVException write_mepc(CPURISCVState *env, int csrno,
- target_ulong val)
+ target_ulong val)
{
env->mepc = val;
return RISCV_EXCP_NONE;
}
static RISCVException read_mcause(CPURISCVState *env, int csrno,
- target_ulong *val)
+ target_ulong *val)
{
*val = env->mcause;
return RISCV_EXCP_NONE;
}
static RISCVException write_mcause(CPURISCVState *env, int csrno,
- target_ulong val)
+ target_ulong val)
{
env->mcause = val;
return RISCV_EXCP_NONE;
@@ -1876,14 +1878,14 @@ static RISCVException write_mtval(CPURISCVState *env, int csrno,
/* Execution environment configuration setup */
static RISCVException read_menvcfg(CPURISCVState *env, int csrno,
- target_ulong *val)
+ target_ulong *val)
{
*val = env->menvcfg;
return RISCV_EXCP_NONE;
}
static RISCVException write_menvcfg(CPURISCVState *env, int csrno,
- target_ulong val)
+ target_ulong val)
{
uint64_t mask = MENVCFG_FIOM | MENVCFG_CBIE | MENVCFG_CBCFE | MENVCFG_CBZE;
@@ -1896,14 +1898,14 @@ static RISCVException write_menvcfg(CPURISCVState *env, int csrno,
}
static RISCVException read_menvcfgh(CPURISCVState *env, int csrno,
- target_ulong *val)
+ target_ulong *val)
{
*val = env->menvcfg >> 32;
return RISCV_EXCP_NONE;
}
static RISCVException write_menvcfgh(CPURISCVState *env, int csrno,
- target_ulong val)
+ target_ulong val)
{
uint64_t mask = MENVCFG_PBMTE | MENVCFG_STCE;
uint64_t valh = (uint64_t)val << 32;
@@ -1914,7 +1916,7 @@ static RISCVException write_menvcfgh(CPURISCVState *env, int csrno,
}
static RISCVException read_senvcfg(CPURISCVState *env, int csrno,
- target_ulong *val)
+ target_ulong *val)
{
RISCVException ret;
@@ -1928,7 +1930,7 @@ static RISCVException read_senvcfg(CPURISCVState *env, int csrno,
}
static RISCVException write_senvcfg(CPURISCVState *env, int csrno,
- target_ulong val)
+ target_ulong val)
{
uint64_t mask = SENVCFG_FIOM | SENVCFG_CBIE | SENVCFG_CBCFE | SENVCFG_CBZE;
RISCVException ret;
@@ -1943,7 +1945,7 @@ static RISCVException write_senvcfg(CPURISCVState *env, int csrno,
}
static RISCVException read_henvcfg(CPURISCVState *env, int csrno,
- target_ulong *val)
+ target_ulong *val)
{
RISCVException ret;
@@ -1957,7 +1959,7 @@ static RISCVException read_henvcfg(CPURISCVState *env, int csrno,
}
static RISCVException write_henvcfg(CPURISCVState *env, int csrno,
- target_ulong val)
+ target_ulong val)
{
uint64_t mask = HENVCFG_FIOM | HENVCFG_CBIE | HENVCFG_CBCFE | HENVCFG_CBZE;
RISCVException ret;
@@ -1977,7 +1979,7 @@ static RISCVException write_henvcfg(CPURISCVState *env, int csrno,
}
static RISCVException read_henvcfgh(CPURISCVState *env, int csrno,
- target_ulong *val)
+ target_ulong *val)
{
RISCVException ret;
@@ -1991,7 +1993,7 @@ static RISCVException read_henvcfgh(CPURISCVState *env, int csrno,
}
static RISCVException write_henvcfgh(CPURISCVState *env, int csrno,
- target_ulong val)
+ target_ulong val)
{
uint64_t mask = HENVCFG_PBMTE | HENVCFG_STCE;
uint64_t valh = (uint64_t)val << 32;
@@ -2034,13 +2036,13 @@ static RISCVException write_mstateen0(CPURISCVState *env, int csrno,
}
static RISCVException write_mstateen_1_3(CPURISCVState *env, int csrno,
- target_ulong new_val)
+ target_ulong new_val)
{
return write_mstateen(env, csrno, SMSTATEEN_STATEEN, new_val);
}
static RISCVException read_mstateenh(CPURISCVState *env, int csrno,
- target_ulong *val)
+ target_ulong *val)
{
*val = env->mstateen[csrno - CSR_MSTATEEN0H] >> 32;
@@ -2061,7 +2063,7 @@ static RISCVException write_mstateenh(CPURISCVState *env, int csrno,
}
static RISCVException write_mstateen0h(CPURISCVState *env, int csrno,
- target_ulong new_val)
+ target_ulong new_val)
{
uint64_t wr_mask = SMSTATEEN_STATEEN | SMSTATEEN0_HSENVCFG;
@@ -2069,7 +2071,7 @@ static RISCVException write_mstateen0h(CPURISCVState *env, int csrno,
}
static RISCVException write_mstateenh_1_3(CPURISCVState *env, int csrno,
- target_ulong new_val)
+ target_ulong new_val)
{
return write_mstateenh(env, csrno, SMSTATEEN_STATEEN, new_val);
}
@@ -2106,7 +2108,7 @@ static RISCVException write_hstateen0(CPURISCVState *env, int csrno,
}
static RISCVException write_hstateen_1_3(CPURISCVState *env, int csrno,
- target_ulong new_val)
+ target_ulong new_val)
{
return write_hstateen(env, csrno, SMSTATEEN_STATEEN, new_val);
}
@@ -2145,7 +2147,7 @@ static RISCVException write_hstateen0h(CPURISCVState *env, int csrno,
}
static RISCVException write_hstateenh_1_3(CPURISCVState *env, int csrno,
- target_ulong new_val)
+ target_ulong new_val)
{
return write_hstateenh(env, csrno, SMSTATEEN_STATEEN, new_val);
}
@@ -3338,7 +3340,7 @@ static RISCVException read_mseccfg(CPURISCVState *env, int csrno,
}
static RISCVException write_mseccfg(CPURISCVState *env, int csrno,
- target_ulong val)
+ target_ulong val)
{
mseccfg_csr_write(env, val);
return RISCV_EXCP_NONE;
--
2.25.1
^ permalink raw reply related [flat|nested] 57+ messages in thread
* [PATCH 06/18] target/riscv: Use 'bool' type for read_only
2023-02-13 18:01 [PATCH 00/18] target/riscv: Various fixes to gdbstub and CSR access Bin Meng
` (4 preceding siblings ...)
2023-02-13 18:02 ` [PATCH 05/18] target/riscv: Coding style fixes in csr.c Bin Meng
@ 2023-02-13 18:02 ` Bin Meng
2023-02-14 8:48 ` weiwei
2023-02-17 2:24 ` LIU Zhiwei
2023-02-13 18:02 ` [PATCH 07/18] target/riscv: Simplify {read, write}_pmpcfg() a little bit Bin Meng
` (12 subsequent siblings)
18 siblings, 2 replies; 57+ messages in thread
From: Bin Meng @ 2023-02-13 18:02 UTC (permalink / raw)
To: qemu-devel
Cc: Alistair Francis, Bin Meng, Daniel Henrique Barboza, Liu Zhiwei,
Palmer Dabbelt, Weiwei Li, qemu-riscv
The read_only variable is currently declared as an 'int', but it
should really be a 'bool'.
Signed-off-by: Bin Meng <bmeng@tinylab.org>
---
target/riscv/csr.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index cc74819759..8bbc75cbfa 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -3778,7 +3778,7 @@ static inline RISCVException riscv_csrrw_check(CPURISCVState *env,
RISCVCPU *cpu)
{
/* check privileges and return RISCV_EXCP_ILLEGAL_INST if check fails */
- int read_only = get_field(csrno, 0xC00) == 3;
+ bool read_only = get_field(csrno, 0xC00) == 3;
int csr_min_priv = csr_ops[csrno].min_priv_ver;
/* ensure the CSR extension is enabled. */
--
2.25.1
^ permalink raw reply related [flat|nested] 57+ messages in thread
* [PATCH 07/18] target/riscv: Simplify {read, write}_pmpcfg() a little bit
2023-02-13 18:01 [PATCH 00/18] target/riscv: Various fixes to gdbstub and CSR access Bin Meng
` (5 preceding siblings ...)
2023-02-13 18:02 ` [PATCH 06/18] target/riscv: Use 'bool' type for read_only Bin Meng
@ 2023-02-13 18:02 ` Bin Meng
2023-02-14 8:50 ` [PATCH 07/18] target/riscv: Simplify {read,write}_pmpcfg() " weiwei
2023-02-17 2:26 ` LIU Zhiwei
2023-02-13 18:02 ` [PATCH 08/18] target/riscv: Simplify getting RISCVCPU pointer from env Bin Meng
` (11 subsequent siblings)
18 siblings, 2 replies; 57+ messages in thread
From: Bin Meng @ 2023-02-13 18:02 UTC (permalink / raw)
To: qemu-devel
Cc: Alistair Francis, Bin Meng, Daniel Henrique Barboza, Liu Zhiwei,
Palmer Dabbelt, Weiwei Li, qemu-riscv
Use the register index that has already been calculated in the
pmpcfg_csr_{read,write} call.
Signed-off-by: Bin Meng <bmeng@tinylab.org>
---
target/riscv/csr.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 8bbc75cbfa..da3b770894 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -3363,7 +3363,7 @@ static RISCVException read_pmpcfg(CPURISCVState *env, int csrno,
if (!check_pmp_reg_index(env, reg_index)) {
return RISCV_EXCP_ILLEGAL_INST;
}
- *val = pmpcfg_csr_read(env, csrno - CSR_PMPCFG0);
+ *val = pmpcfg_csr_read(env, reg_index);
return RISCV_EXCP_NONE;
}
@@ -3375,7 +3375,7 @@ static RISCVException write_pmpcfg(CPURISCVState *env, int csrno,
if (!check_pmp_reg_index(env, reg_index)) {
return RISCV_EXCP_ILLEGAL_INST;
}
- pmpcfg_csr_write(env, csrno - CSR_PMPCFG0, val);
+ pmpcfg_csr_write(env, reg_index, val);
return RISCV_EXCP_NONE;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 57+ messages in thread
* [PATCH 08/18] target/riscv: Simplify getting RISCVCPU pointer from env
2023-02-13 18:01 [PATCH 00/18] target/riscv: Various fixes to gdbstub and CSR access Bin Meng
` (6 preceding siblings ...)
2023-02-13 18:02 ` [PATCH 07/18] target/riscv: Simplify {read, write}_pmpcfg() a little bit Bin Meng
@ 2023-02-13 18:02 ` Bin Meng
2023-02-14 8:51 ` weiwei
2023-02-17 2:30 ` LIU Zhiwei
2023-02-13 18:02 ` [PATCH 09/18] target/riscv: Avoid reporting odd-numbered pmpcfgX in the CSR XML for RV64 Bin Meng
` (10 subsequent siblings)
18 siblings, 2 replies; 57+ messages in thread
From: Bin Meng @ 2023-02-13 18:02 UTC (permalink / raw)
To: qemu-devel
Cc: Alistair Francis, Bin Meng, Daniel Henrique Barboza, Liu Zhiwei,
Palmer Dabbelt, Weiwei Li, qemu-riscv
Use env_archcpu() to get RISCVCPU pointer from env directly.
Signed-off-by: Bin Meng <bmeng@tinylab.org>
---
target/riscv/csr.c | 36 ++++++++++++------------------------
1 file changed, 12 insertions(+), 24 deletions(-)
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index da3b770894..0a3f2bef6f 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -46,8 +46,7 @@ static RISCVException smstateen_acc_ok(CPURISCVState *env, int index,
uint64_t bit)
{
bool virt = riscv_cpu_virt_enabled(env);
- CPUState *cs = env_cpu(env);
- RISCVCPU *cpu = RISCV_CPU(cs);
+ RISCVCPU *cpu = env_archcpu(env);
if (env->priv == PRV_M || !cpu->cfg.ext_smstateen) {
return RISCV_EXCP_NONE;
@@ -90,8 +89,7 @@ static RISCVException fs(CPURISCVState *env, int csrno)
static RISCVException vs(CPURISCVState *env, int csrno)
{
- CPUState *cs = env_cpu(env);
- RISCVCPU *cpu = RISCV_CPU(cs);
+ RISCVCPU *cpu = env_archcpu(env);
if (env->misa_ext & RVV ||
cpu->cfg.ext_zve32f || cpu->cfg.ext_zve64f) {
@@ -108,8 +106,7 @@ static RISCVException vs(CPURISCVState *env, int csrno)
static RISCVException ctr(CPURISCVState *env, int csrno)
{
#if !defined(CONFIG_USER_ONLY)
- CPUState *cs = env_cpu(env);
- RISCVCPU *cpu = RISCV_CPU(cs);
+ RISCVCPU *cpu = env_archcpu(env);
int ctr_index;
target_ulong ctr_mask;
int base_csrno = CSR_CYCLE;
@@ -166,8 +163,7 @@ static RISCVException ctr32(CPURISCVState *env, int csrno)
#if !defined(CONFIG_USER_ONLY)
static RISCVException mctr(CPURISCVState *env, int csrno)
{
- CPUState *cs = env_cpu(env);
- RISCVCPU *cpu = RISCV_CPU(cs);
+ RISCVCPU *cpu = env_archcpu(env);
int ctr_index;
int base_csrno = CSR_MHPMCOUNTER3;
@@ -195,8 +191,7 @@ static RISCVException mctr32(CPURISCVState *env, int csrno)
static RISCVException sscofpmf(CPURISCVState *env, int csrno)
{
- CPUState *cs = env_cpu(env);
- RISCVCPU *cpu = RISCV_CPU(cs);
+ RISCVCPU *cpu = env_archcpu(env);
if (!cpu->cfg.ext_sscofpmf) {
return RISCV_EXCP_ILLEGAL_INST;
@@ -321,8 +316,7 @@ static RISCVException umode32(CPURISCVState *env, int csrno)
static RISCVException mstateen(CPURISCVState *env, int csrno)
{
- CPUState *cs = env_cpu(env);
- RISCVCPU *cpu = RISCV_CPU(cs);
+ RISCVCPU *cpu = env_archcpu(env);
if (!cpu->cfg.ext_smstateen) {
return RISCV_EXCP_ILLEGAL_INST;
@@ -333,8 +327,7 @@ static RISCVException mstateen(CPURISCVState *env, int csrno)
static RISCVException hstateen_pred(CPURISCVState *env, int csrno, int base)
{
- CPUState *cs = env_cpu(env);
- RISCVCPU *cpu = RISCV_CPU(cs);
+ RISCVCPU *cpu = env_archcpu(env);
if (!cpu->cfg.ext_smstateen) {
return RISCV_EXCP_ILLEGAL_INST;
@@ -363,8 +356,7 @@ static RISCVException sstateen(CPURISCVState *env, int csrno)
{
bool virt = riscv_cpu_virt_enabled(env);
int index = csrno - CSR_SSTATEEN0;
- CPUState *cs = env_cpu(env);
- RISCVCPU *cpu = RISCV_CPU(cs);
+ RISCVCPU *cpu = env_archcpu(env);
if (!cpu->cfg.ext_smstateen) {
return RISCV_EXCP_ILLEGAL_INST;
@@ -918,8 +910,7 @@ static RISCVException read_timeh(CPURISCVState *env, int csrno,
static RISCVException sstc(CPURISCVState *env, int csrno)
{
- CPUState *cs = env_cpu(env);
- RISCVCPU *cpu = RISCV_CPU(cs);
+ RISCVCPU *cpu = env_archcpu(env);
bool hmode_check = false;
if (!cpu->cfg.ext_sstc || !env->rdtime_fn) {
@@ -1152,8 +1143,7 @@ static RISCVException write_ignore(CPURISCVState *env, int csrno,
static RISCVException read_mvendorid(CPURISCVState *env, int csrno,
target_ulong *val)
{
- CPUState *cs = env_cpu(env);
- RISCVCPU *cpu = RISCV_CPU(cs);
+ RISCVCPU *cpu = env_archcpu(env);
*val = cpu->cfg.mvendorid;
return RISCV_EXCP_NONE;
@@ -1162,8 +1152,7 @@ static RISCVException read_mvendorid(CPURISCVState *env, int csrno,
static RISCVException read_marchid(CPURISCVState *env, int csrno,
target_ulong *val)
{
- CPUState *cs = env_cpu(env);
- RISCVCPU *cpu = RISCV_CPU(cs);
+ RISCVCPU *cpu = env_archcpu(env);
*val = cpu->cfg.marchid;
return RISCV_EXCP_NONE;
@@ -1172,8 +1161,7 @@ static RISCVException read_marchid(CPURISCVState *env, int csrno,
static RISCVException read_mimpid(CPURISCVState *env, int csrno,
target_ulong *val)
{
- CPUState *cs = env_cpu(env);
- RISCVCPU *cpu = RISCV_CPU(cs);
+ RISCVCPU *cpu = env_archcpu(env);
*val = cpu->cfg.mimpid;
return RISCV_EXCP_NONE;
--
2.25.1
^ permalink raw reply related [flat|nested] 57+ messages in thread
* [PATCH 09/18] target/riscv: Avoid reporting odd-numbered pmpcfgX in the CSR XML for RV64
2023-02-13 18:01 [PATCH 00/18] target/riscv: Various fixes to gdbstub and CSR access Bin Meng
` (7 preceding siblings ...)
2023-02-13 18:02 ` [PATCH 08/18] target/riscv: Simplify getting RISCVCPU pointer from env Bin Meng
@ 2023-02-13 18:02 ` Bin Meng
2023-02-14 8:56 ` weiwei
2023-02-17 2:36 ` LIU Zhiwei
2023-02-13 18:02 ` [PATCH 10/18] target/riscv: gdbstub: Turn on debugger mode before calling CSR predicate() Bin Meng
` (9 subsequent siblings)
18 siblings, 2 replies; 57+ messages in thread
From: Bin Meng @ 2023-02-13 18:02 UTC (permalink / raw)
To: qemu-devel
Cc: Alistair Francis, Bin Meng, Daniel Henrique Barboza, Liu Zhiwei,
Palmer Dabbelt, Weiwei Li, qemu-riscv
At present the odd-numbered PMP configuration registers for RV64 are
reported in the CSR XML by QEMU gdbstub. However these registers do
not exist on RV64 so trying to access them from gdb results in 'E14'.
Move the pmpcfgX index check from the actual read/write routine to
the PMP CSR predicate() routine, so that non-existent pmpcfgX won't
be reported in the CSR XML for RV64.
Signed-off-by: Bin Meng <bmeng@tinylab.org>
---
target/riscv/csr.c | 23 ++++++++---------------
1 file changed, 8 insertions(+), 15 deletions(-)
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 0a3f2bef6f..749d0ef83e 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -412,6 +412,14 @@ static int aia_hmode32(CPURISCVState *env, int csrno)
static RISCVException pmp(CPURISCVState *env, int csrno)
{
if (riscv_feature(env, RISCV_FEATURE_PMP)) {
+ if (csrno <= CSR_PMPCFG3) {
+ uint32_t reg_index = csrno - CSR_PMPCFG0;
+
+ if ((reg_index & 1) && (riscv_cpu_mxl(env) == MXL_RV64)) {
+ return RISCV_EXCP_ILLEGAL_INST;
+ }
+ }
+
return RISCV_EXCP_NONE;
}
@@ -3334,23 +3342,11 @@ static RISCVException write_mseccfg(CPURISCVState *env, int csrno,
return RISCV_EXCP_NONE;
}
-static bool check_pmp_reg_index(CPURISCVState *env, uint32_t reg_index)
-{
- /* TODO: RV128 restriction check */
- if ((reg_index & 1) && (riscv_cpu_mxl(env) == MXL_RV64)) {
- return false;
- }
- return true;
-}
-
static RISCVException read_pmpcfg(CPURISCVState *env, int csrno,
target_ulong *val)
{
uint32_t reg_index = csrno - CSR_PMPCFG0;
- if (!check_pmp_reg_index(env, reg_index)) {
- return RISCV_EXCP_ILLEGAL_INST;
- }
*val = pmpcfg_csr_read(env, reg_index);
return RISCV_EXCP_NONE;
}
@@ -3360,9 +3356,6 @@ static RISCVException write_pmpcfg(CPURISCVState *env, int csrno,
{
uint32_t reg_index = csrno - CSR_PMPCFG0;
- if (!check_pmp_reg_index(env, reg_index)) {
- return RISCV_EXCP_ILLEGAL_INST;
- }
pmpcfg_csr_write(env, reg_index, val);
return RISCV_EXCP_NONE;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 57+ messages in thread
* [PATCH 10/18] target/riscv: gdbstub: Turn on debugger mode before calling CSR predicate()
2023-02-13 18:01 [PATCH 00/18] target/riscv: Various fixes to gdbstub and CSR access Bin Meng
` (8 preceding siblings ...)
2023-02-13 18:02 ` [PATCH 09/18] target/riscv: Avoid reporting odd-numbered pmpcfgX in the CSR XML for RV64 Bin Meng
@ 2023-02-13 18:02 ` Bin Meng
2023-02-14 9:02 ` weiwei
2023-02-17 2:39 ` LIU Zhiwei
2023-02-13 18:02 ` [PATCH 11/18] target/riscv: gdbstub: Drop the vector CSRs in riscv-vector.xml Bin Meng
` (8 subsequent siblings)
18 siblings, 2 replies; 57+ messages in thread
From: Bin Meng @ 2023-02-13 18:02 UTC (permalink / raw)
To: qemu-devel
Cc: Alistair Francis, Bin Meng, Daniel Henrique Barboza, Liu Zhiwei,
Palmer Dabbelt, Weiwei Li, qemu-riscv
Since commit 94452ac4cf26 ("target/riscv: remove fflags, frm, and fcsr from riscv-*-fpu.xml")
the 3 FPU CSRs are removed from the XML target decription. The
original intent of that commit was based on the assumption that
the 3 FPU CSRs will show up in the riscv-csr.xml so the ones in
riscv-*-fpu.xml are redundant. But unforuantely that is not ture.
As the FPU CSR predicate() has a run-time check on MSTATUS.FS,
at the time when CSR XML is generated MSTATUS.FS is unset, hence
no FPU CSRs will be reported.
The FPU CSR predicate() already considered such a case of being
accessed by a debugger. All we need to do is to turn on debugger
mode before calling predicate().
Signed-off-by: Bin Meng <bmeng@tinylab.org>
---
target/riscv/gdbstub.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/target/riscv/gdbstub.c b/target/riscv/gdbstub.c
index 294f0ceb1c..ef52f41460 100644
--- a/target/riscv/gdbstub.c
+++ b/target/riscv/gdbstub.c
@@ -280,6 +280,10 @@ static int riscv_gen_dynamic_csr_xml(CPUState *cs, int base_reg)
int bitsize = 16 << env->misa_mxl_max;
int i;
+#if !defined(CONFIG_USER_ONLY)
+ env->debugger = true;
+#endif
+
/* Until gdb knows about 128-bit registers */
if (bitsize > 64) {
bitsize = 64;
@@ -308,6 +312,11 @@ static int riscv_gen_dynamic_csr_xml(CPUState *cs, int base_reg)
g_string_append_printf(s, "</feature>");
cpu->dyn_csr_xml = g_string_free(s, false);
+
+#if !defined(CONFIG_USER_ONLY)
+ env->debugger = false;
+#endif
+
return CSR_TABLE_SIZE;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 57+ messages in thread
* [PATCH 11/18] target/riscv: gdbstub: Drop the vector CSRs in riscv-vector.xml
2023-02-13 18:01 [PATCH 00/18] target/riscv: Various fixes to gdbstub and CSR access Bin Meng
` (9 preceding siblings ...)
2023-02-13 18:02 ` [PATCH 10/18] target/riscv: gdbstub: Turn on debugger mode before calling CSR predicate() Bin Meng
@ 2023-02-13 18:02 ` Bin Meng
2023-02-14 9:13 ` weiwei
2023-02-17 2:43 ` LIU Zhiwei
2023-02-13 19:19 ` [PATCH 00/18] target/riscv: Various fixes to gdbstub and CSR access Daniel Henrique Barboza
` (7 subsequent siblings)
18 siblings, 2 replies; 57+ messages in thread
From: Bin Meng @ 2023-02-13 18:02 UTC (permalink / raw)
To: qemu-devel
Cc: Alistair Francis, Bin Meng, Daniel Henrique Barboza, Liu Zhiwei,
Palmer Dabbelt, Weiwei Li, qemu-riscv
It's worth noting that the vector CSR predicate() has a similar
run-time check logic to the FPU CSR. With the previous patch our
gdbstub can correctly report these vector CSRs via the CSR xml.
Commit 719d3561b269 ("target/riscv: gdb: support vector registers for rv64 & rv32")
inserted these vector CSRs in an ad-hoc, non-standard way in the
riscv-vector.xml. Now we can treat these CSRs no different from
other CSRs.
Signed-off-by: Bin Meng <bmeng@tinylab.org>
---
target/riscv/gdbstub.c | 75 ------------------------------------------
1 file changed, 75 deletions(-)
diff --git a/target/riscv/gdbstub.c b/target/riscv/gdbstub.c
index ef52f41460..6048541606 100644
--- a/target/riscv/gdbstub.c
+++ b/target/riscv/gdbstub.c
@@ -127,40 +127,6 @@ static int riscv_gdb_set_fpu(CPURISCVState *env, uint8_t *mem_buf, int n)
return 0;
}
-/*
- * Convert register index number passed by GDB to the correspond
- * vector CSR number. Vector CSRs are defined after vector registers
- * in dynamic generated riscv-vector.xml, thus the starting register index
- * of vector CSRs is 32.
- * Return 0 if register index number is out of range.
- */
-static int riscv_gdb_vector_csrno(int num_regs)
-{
- /*
- * The order of vector CSRs in the switch case
- * should match with the order defined in csr_ops[].
- */
- switch (num_regs) {
- case 32:
- return CSR_VSTART;
- case 33:
- return CSR_VXSAT;
- case 34:
- return CSR_VXRM;
- case 35:
- return CSR_VCSR;
- case 36:
- return CSR_VL;
- case 37:
- return CSR_VTYPE;
- case 38:
- return CSR_VLENB;
- default:
- /* Unknown register. */
- return 0;
- }
-}
-
static int riscv_gdb_get_vector(CPURISCVState *env, GByteArray *buf, int n)
{
uint16_t vlenb = env_archcpu(env)->cfg.vlen >> 3;
@@ -174,19 +140,6 @@ static int riscv_gdb_get_vector(CPURISCVState *env, GByteArray *buf, int n)
return cnt;
}
- int csrno = riscv_gdb_vector_csrno(n);
-
- if (!csrno) {
- return 0;
- }
-
- target_ulong val = 0;
- int result = riscv_csrrw_debug(env, csrno, &val, 0, 0);
-
- if (result == RISCV_EXCP_NONE) {
- return gdb_get_regl(buf, val);
- }
-
return 0;
}
@@ -201,19 +154,6 @@ static int riscv_gdb_set_vector(CPURISCVState *env, uint8_t *mem_buf, int n)
return vlenb;
}
- int csrno = riscv_gdb_vector_csrno(n);
-
- if (!csrno) {
- return 0;
- }
-
- target_ulong val = ldtul_p(mem_buf);
- int result = riscv_csrrw_debug(env, csrno, NULL, val, -1);
-
- if (result == RISCV_EXCP_NONE) {
- return sizeof(target_ulong);
- }
-
return 0;
}
@@ -361,21 +301,6 @@ static int ricsv_gen_dynamic_vector_xml(CPUState *cs, int base_reg)
num_regs++;
}
- /* Define vector CSRs */
- const char *vector_csrs[7] = {
- "vstart", "vxsat", "vxrm", "vcsr",
- "vl", "vtype", "vlenb"
- };
-
- for (i = 0; i < 7; i++) {
- g_string_append_printf(s,
- "<reg name=\"%s\" bitsize=\"%d\""
- " regnum=\"%d\" group=\"vector\""
- " type=\"int\"/>",
- vector_csrs[i], TARGET_LONG_BITS, base_reg++);
- num_regs++;
- }
-
g_string_append_printf(s, "</feature>");
cpu->dyn_vreg_xml = g_string_free(s, false);
--
2.25.1
^ permalink raw reply related [flat|nested] 57+ messages in thread
* Re: [PATCH 00/18] target/riscv: Various fixes to gdbstub and CSR access
2023-02-13 18:01 [PATCH 00/18] target/riscv: Various fixes to gdbstub and CSR access Bin Meng
` (10 preceding siblings ...)
2023-02-13 18:02 ` [PATCH 11/18] target/riscv: gdbstub: Drop the vector CSRs in riscv-vector.xml Bin Meng
@ 2023-02-13 19:19 ` Daniel Henrique Barboza
2023-02-14 14:31 ` Bin Meng
2023-02-14 1:09 ` [PATCH 12/18] target/riscv: Allow debugger to access user timer and counter CSRs Bin Meng
` (6 subsequent siblings)
18 siblings, 1 reply; 57+ messages in thread
From: Daniel Henrique Barboza @ 2023-02-13 19:19 UTC (permalink / raw)
To: Bin Meng, qemu-devel
Cc: Alistair Francis, Bin Meng, Liu Zhiwei, Palmer Dabbelt, Weiwei Li,
qemu-riscv
Bin,
I received only patches 1-11. I don't see the remaining patches in patchwork:
https://patchwork.kernel.org/project/qemu-devel/list/?series=721372
or in the qemu-devel archives:
https://lists.gnu.org/archive/html/qemu-devel/2023-02/msg03461.html
Can you please verify? Thanks,
Daniel
On 2/13/23 15:01, Bin Meng wrote:
> At present gdbstub reports an incorrect / incomplete CSR list in the
> target description XML, for example:
>
> - menvcfg is reported in 'sifive_u' machine
> - fcsr is missing in a F/D enabled processor
>
> The issue is caused by:
> - priv spec version check is missing when reporting CSRs
> - CSR predicate() routine is called without turning on the debugger flag
>
> This series aims to generate a correct and complete CSR list for gdbstub.
>
>
> Bin Meng (18):
> target/riscv: gdbstub: Check priv spec version before reporting CSR
> target/riscv: Correct the priority policy of riscv_csrrw_check()
> target/riscv: gdbstub: Minor change for better readability
> target/riscv: gdbstub: Do not generate CSR XML if Zicsr is disabled
> target/riscv: Coding style fixes in csr.c
> target/riscv: Use 'bool' type for read_only
> target/riscv: Simplify {read,write}_pmpcfg() a little bit
> target/riscv: Simplify getting RISCVCPU pointer from env
> target/riscv: Avoid reporting odd-numbered pmpcfgX in the CSR XML for
> RV64
> target/riscv: gdbstub: Turn on debugger mode before calling CSR
> predicate()
> target/riscv: gdbstub: Drop the vector CSRs in riscv-vector.xml
> target/riscv: Allow debugger to access user timer and counter CSRs
> target/riscv: Allow debugger to access seed CSR
> target/riscv: Allow debugger to access {h,s}stateen CSRs
> target/riscv: Allow debugger to access sstc CSRs
> target/riscv: Drop priv level check in mseccfg predicate()
> target/riscv: Group all predicate() routines together
> target/riscv: Move configuration check to envcfg CSRs predicate()
>
> target/riscv/csr.c | 360 +++++++++++++++++++++++------------------
> target/riscv/gdbstub.c | 100 +++---------
> 2 files changed, 221 insertions(+), 239 deletions(-)
>
^ permalink raw reply [flat|nested] 57+ messages in thread
* [PATCH 12/18] target/riscv: Allow debugger to access user timer and counter CSRs
2023-02-13 18:01 [PATCH 00/18] target/riscv: Various fixes to gdbstub and CSR access Bin Meng
` (11 preceding siblings ...)
2023-02-13 19:19 ` [PATCH 00/18] target/riscv: Various fixes to gdbstub and CSR access Daniel Henrique Barboza
@ 2023-02-14 1:09 ` Bin Meng
2023-02-14 9:16 ` weiwei
2023-02-17 2:48 ` LIU Zhiwei
2023-02-14 1:09 ` [PATCH 13/18] target/riscv: Allow debugger to access seed CSR Bin Meng
` (5 subsequent siblings)
18 siblings, 2 replies; 57+ messages in thread
From: Bin Meng @ 2023-02-14 1:09 UTC (permalink / raw)
To: qemu-devel
Cc: Alistair Francis, Bin Meng, Daniel Henrique Barboza, Liu Zhiwei,
Palmer Dabbelt, Weiwei Li, qemu-riscv
At present user timer and counter CSRs are not reported in the
CSR XML hence gdb cannot access them.
Fix it by addding a debugger check in their predicate() routine.
Signed-off-by: Bin Meng <bmeng@tinylab.org>
---
target/riscv/csr.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 749d0ef83e..515b05348b 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -131,6 +131,10 @@ static RISCVException ctr(CPURISCVState *env, int csrno)
skip_ext_pmu_check:
+ if (env->debugger) {
+ return RISCV_EXCP_NONE;
+ }
+
if (env->priv < PRV_M && !get_field(env->mcounteren, ctr_mask)) {
return RISCV_EXCP_ILLEGAL_INST;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 57+ messages in thread
* [PATCH 13/18] target/riscv: Allow debugger to access seed CSR
2023-02-13 18:01 [PATCH 00/18] target/riscv: Various fixes to gdbstub and CSR access Bin Meng
` (12 preceding siblings ...)
2023-02-14 1:09 ` [PATCH 12/18] target/riscv: Allow debugger to access user timer and counter CSRs Bin Meng
@ 2023-02-14 1:09 ` Bin Meng
2023-02-14 9:18 ` weiwei
2023-02-17 2:59 ` LIU Zhiwei
2023-02-14 3:06 ` [PATCH 14/18] target/riscv: Allow debugger to access {h, s}stateen CSRs Bin Meng
` (4 subsequent siblings)
18 siblings, 2 replies; 57+ messages in thread
From: Bin Meng @ 2023-02-14 1:09 UTC (permalink / raw)
To: qemu-devel
Cc: Alistair Francis, Bin Meng, Daniel Henrique Barboza, Liu Zhiwei,
Palmer Dabbelt, Weiwei Li, qemu-riscv
At present seed CSR is not reported in the CSR XML hence gdb cannot
access it.
Fix it by addding a debugger check in its predicate() routine.
Signed-off-by: Bin Meng <bmeng@tinylab.org>
---
target/riscv/csr.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 515b05348b..f1075b5728 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -458,6 +458,10 @@ static RISCVException seed(CPURISCVState *env, int csrno)
}
#if !defined(CONFIG_USER_ONLY)
+ if (env->debugger) {
+ return RISCV_EXCP_NONE;
+ }
+
/*
* With a CSR read-write instruction:
* 1) The seed CSR is always available in machine mode as normal.
--
2.25.1
^ permalink raw reply related [flat|nested] 57+ messages in thread
* [PATCH 14/18] target/riscv: Allow debugger to access {h, s}stateen CSRs
2023-02-13 18:01 [PATCH 00/18] target/riscv: Various fixes to gdbstub and CSR access Bin Meng
` (13 preceding siblings ...)
2023-02-14 1:09 ` [PATCH 13/18] target/riscv: Allow debugger to access seed CSR Bin Meng
@ 2023-02-14 3:06 ` Bin Meng
2023-02-14 9:24 ` weiwei
2023-02-14 4:12 ` [PATCH 15/18] target/riscv: Allow debugger to access sstc CSRs Bin Meng
` (3 subsequent siblings)
18 siblings, 1 reply; 57+ messages in thread
From: Bin Meng @ 2023-02-14 3:06 UTC (permalink / raw)
To: qemu-devel
Cc: Alistair Francis, Bin Meng, Daniel Henrique Barboza, Liu Zhiwei,
Palmer Dabbelt, Weiwei Li, qemu-riscv
At present {h,s}stateen CSRs are not reported in the CSR XML
hence gdb cannot access them.
Fix it by adjusting their predicate() routine logic so that the
static config check comes before the run-time check, as well as
addding a debugger check.
Signed-off-by: Bin Meng <bmeng@tinylab.org>
---
target/riscv/csr.c | 22 ++++++++++++++++++++--
1 file changed, 20 insertions(+), 2 deletions(-)
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index f1075b5728..d6bcb7f275 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -337,13 +337,22 @@ static RISCVException hstateen_pred(CPURISCVState *env, int csrno, int base)
return RISCV_EXCP_ILLEGAL_INST;
}
+ RISCVException ret = hmode(env, csrno);
+ if (ret != RISCV_EXCP_NONE) {
+ return ret;
+ }
+
+ if (env->debugger) {
+ return RISCV_EXCP_NONE;
+ }
+
if (env->priv < PRV_M) {
if (!(env->mstateen[csrno - base] & SMSTATEEN_STATEEN)) {
return RISCV_EXCP_ILLEGAL_INST;
}
}
- return hmode(env, csrno);
+ return RISCV_EXCP_NONE;
}
static RISCVException hstateen(CPURISCVState *env, int csrno)
@@ -366,6 +375,15 @@ static RISCVException sstateen(CPURISCVState *env, int csrno)
return RISCV_EXCP_ILLEGAL_INST;
}
+ RISCVException ret = smode(env, csrno);
+ if (ret != RISCV_EXCP_NONE) {
+ return ret;
+ }
+
+ if (env->debugger) {
+ return RISCV_EXCP_NONE;
+ }
+
if (env->priv < PRV_M) {
if (!(env->mstateen[index] & SMSTATEEN_STATEEN)) {
return RISCV_EXCP_ILLEGAL_INST;
@@ -378,7 +396,7 @@ static RISCVException sstateen(CPURISCVState *env, int csrno)
}
}
- return smode(env, csrno);
+ return RISCV_EXCP_NONE;
}
/* Checks if PointerMasking registers could be accessed */
--
2.25.1
^ permalink raw reply related [flat|nested] 57+ messages in thread
* [PATCH 15/18] target/riscv: Allow debugger to access sstc CSRs
2023-02-13 18:01 [PATCH 00/18] target/riscv: Various fixes to gdbstub and CSR access Bin Meng
` (14 preceding siblings ...)
2023-02-14 3:06 ` [PATCH 14/18] target/riscv: Allow debugger to access {h, s}stateen CSRs Bin Meng
@ 2023-02-14 4:12 ` Bin Meng
2023-02-14 9:26 ` weiwei
2023-02-14 4:12 ` [PATCH 16/18] target/riscv: Drop priv level check in mseccfg predicate() Bin Meng
` (2 subsequent siblings)
18 siblings, 1 reply; 57+ messages in thread
From: Bin Meng @ 2023-02-14 4:12 UTC (permalink / raw)
To: qemu-devel
Cc: Alistair Francis, Bin Meng, Daniel Henrique Barboza, Liu Zhiwei,
Palmer Dabbelt, Weiwei Li, qemu-riscv
At present with a debugger attached sstc CSRs can only be accssed
when CPU is in M-mode, or configured correctly.
Fix it by adjusting their predicate() routine logic so that the
static config check comes before the run-time check, as well as
addding a debugger check.
Signed-off-by: Bin Meng <bmeng@tinylab.org>
---
target/riscv/csr.c | 19 ++++++++++++++-----
1 file changed, 14 insertions(+), 5 deletions(-)
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index d6bcb7f275..c6a7745cb2 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -951,6 +951,19 @@ static RISCVException sstc(CPURISCVState *env, int csrno)
return RISCV_EXCP_ILLEGAL_INST;
}
+ if ((csrno == CSR_VSTIMECMP) || (csrno == CSR_VSTIMECMPH)) {
+ hmode_check = true;
+ }
+
+ RISCVException ret = hmode_check ? hmode(env, csrno) : smode(env, csrno);
+ if (ret != RISCV_EXCP_NONE) {
+ return ret;
+ }
+
+ if (env->debugger) {
+ return RISCV_EXCP_NONE;
+ }
+
if (env->priv == PRV_M) {
return RISCV_EXCP_NONE;
}
@@ -971,11 +984,7 @@ static RISCVException sstc(CPURISCVState *env, int csrno)
}
}
- if ((csrno == CSR_VSTIMECMP) || (csrno == CSR_VSTIMECMPH)) {
- hmode_check = true;
- }
-
- return hmode_check ? hmode(env, csrno) : smode(env, csrno);
+ return RISCV_EXCP_NONE;
}
static RISCVException sstc_32(CPURISCVState *env, int csrno)
--
2.25.1
^ permalink raw reply related [flat|nested] 57+ messages in thread
* [PATCH 16/18] target/riscv: Drop priv level check in mseccfg predicate()
2023-02-13 18:01 [PATCH 00/18] target/riscv: Various fixes to gdbstub and CSR access Bin Meng
` (15 preceding siblings ...)
2023-02-14 4:12 ` [PATCH 15/18] target/riscv: Allow debugger to access sstc CSRs Bin Meng
@ 2023-02-14 4:12 ` Bin Meng
2023-02-14 9:26 ` weiwei
2023-02-14 4:31 ` [PATCH 17/18] target/riscv: Group all predicate() routines together Bin Meng
2023-02-14 14:27 ` [PATCH 18/18] target/riscv: Move configuration check to envcfg CSRs predicate() Bin Meng
18 siblings, 1 reply; 57+ messages in thread
From: Bin Meng @ 2023-02-14 4:12 UTC (permalink / raw)
To: qemu-devel
Cc: Alistair Francis, Bin Meng, Daniel Henrique Barboza, Liu Zhiwei,
Palmer Dabbelt, Weiwei Li, qemu-riscv
riscv_csrrw_check() already does the generic privilege level check
hence there is no need to do the specific M-mode access check in
the mseccfg predicate().
With this change debugger can access the mseccfg CSR anytime.
Signed-off-by: Bin Meng <bmeng@tinylab.org>
---
target/riscv/csr.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index c6a7745cb2..40aae9e7b3 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -450,7 +450,7 @@ static RISCVException pmp(CPURISCVState *env, int csrno)
static RISCVException epmp(CPURISCVState *env, int csrno)
{
- if (env->priv == PRV_M && riscv_feature(env, RISCV_FEATURE_EPMP)) {
+ if (riscv_feature(env, RISCV_FEATURE_EPMP)) {
return RISCV_EXCP_NONE;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 57+ messages in thread
* [PATCH 17/18] target/riscv: Group all predicate() routines together
2023-02-13 18:01 [PATCH 00/18] target/riscv: Various fixes to gdbstub and CSR access Bin Meng
` (16 preceding siblings ...)
2023-02-14 4:12 ` [PATCH 16/18] target/riscv: Drop priv level check in mseccfg predicate() Bin Meng
@ 2023-02-14 4:31 ` Bin Meng
2023-02-14 9:27 ` weiwei
2023-02-14 14:27 ` [PATCH 18/18] target/riscv: Move configuration check to envcfg CSRs predicate() Bin Meng
18 siblings, 1 reply; 57+ messages in thread
From: Bin Meng @ 2023-02-14 4:31 UTC (permalink / raw)
To: qemu-devel
Cc: Alistair Francis, Bin Meng, Daniel Henrique Barboza, Liu Zhiwei,
Palmer Dabbelt, Weiwei Li, qemu-riscv
Move sstc()/sstc32() to where all predicate() routines live.
Signed-off-by: Bin Meng <bmeng@tinylab.org>
---
target/riscv/csr.c | 108 ++++++++++++++++++++++-----------------------
1 file changed, 54 insertions(+), 54 deletions(-)
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 40aae9e7b3..37350b8a6d 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -399,6 +399,60 @@ static RISCVException sstateen(CPURISCVState *env, int csrno)
return RISCV_EXCP_NONE;
}
+static RISCVException sstc(CPURISCVState *env, int csrno)
+{
+ RISCVCPU *cpu = env_archcpu(env);
+ bool hmode_check = false;
+
+ if (!cpu->cfg.ext_sstc || !env->rdtime_fn) {
+ return RISCV_EXCP_ILLEGAL_INST;
+ }
+
+ if ((csrno == CSR_VSTIMECMP) || (csrno == CSR_VSTIMECMPH)) {
+ hmode_check = true;
+ }
+
+ RISCVException ret = hmode_check ? hmode(env, csrno) : smode(env, csrno);
+ if (ret != RISCV_EXCP_NONE) {
+ return ret;
+ }
+
+ if (env->debugger) {
+ return RISCV_EXCP_NONE;
+ }
+
+ if (env->priv == PRV_M) {
+ return RISCV_EXCP_NONE;
+ }
+
+ /*
+ * No need of separate function for rv32 as menvcfg stores both menvcfg
+ * menvcfgh for RV32.
+ */
+ if (!(get_field(env->mcounteren, COUNTEREN_TM) &&
+ get_field(env->menvcfg, MENVCFG_STCE))) {
+ return RISCV_EXCP_ILLEGAL_INST;
+ }
+
+ if (riscv_cpu_virt_enabled(env)) {
+ if (!(get_field(env->hcounteren, COUNTEREN_TM) &&
+ get_field(env->henvcfg, HENVCFG_STCE))) {
+ return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
+ }
+ }
+
+ return RISCV_EXCP_NONE;
+}
+
+static RISCVException sstc_32(CPURISCVState *env, int csrno)
+{
+ if (riscv_cpu_mxl(env) != MXL_RV32) {
+ return RISCV_EXCP_ILLEGAL_INST;
+ }
+
+ return sstc(env, csrno);
+}
+
/* Checks if PointerMasking registers could be accessed */
static RISCVException pointer_masking(CPURISCVState *env, int csrno)
{
@@ -942,60 +996,6 @@ static RISCVException read_timeh(CPURISCVState *env, int csrno,
return RISCV_EXCP_NONE;
}
-static RISCVException sstc(CPURISCVState *env, int csrno)
-{
- RISCVCPU *cpu = env_archcpu(env);
- bool hmode_check = false;
-
- if (!cpu->cfg.ext_sstc || !env->rdtime_fn) {
- return RISCV_EXCP_ILLEGAL_INST;
- }
-
- if ((csrno == CSR_VSTIMECMP) || (csrno == CSR_VSTIMECMPH)) {
- hmode_check = true;
- }
-
- RISCVException ret = hmode_check ? hmode(env, csrno) : smode(env, csrno);
- if (ret != RISCV_EXCP_NONE) {
- return ret;
- }
-
- if (env->debugger) {
- return RISCV_EXCP_NONE;
- }
-
- if (env->priv == PRV_M) {
- return RISCV_EXCP_NONE;
- }
-
- /*
- * No need of separate function for rv32 as menvcfg stores both menvcfg
- * menvcfgh for RV32.
- */
- if (!(get_field(env->mcounteren, COUNTEREN_TM) &&
- get_field(env->menvcfg, MENVCFG_STCE))) {
- return RISCV_EXCP_ILLEGAL_INST;
- }
-
- if (riscv_cpu_virt_enabled(env)) {
- if (!(get_field(env->hcounteren, COUNTEREN_TM) &&
- get_field(env->henvcfg, HENVCFG_STCE))) {
- return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
- }
- }
-
- return RISCV_EXCP_NONE;
-}
-
-static RISCVException sstc_32(CPURISCVState *env, int csrno)
-{
- if (riscv_cpu_mxl(env) != MXL_RV32) {
- return RISCV_EXCP_ILLEGAL_INST;
- }
-
- return sstc(env, csrno);
-}
-
static RISCVException read_vstimecmp(CPURISCVState *env, int csrno,
target_ulong *val)
{
--
2.25.1
^ permalink raw reply related [flat|nested] 57+ messages in thread
* Re: [PATCH 01/18] target/riscv: gdbstub: Check priv spec version before reporting CSR
2023-02-13 18:01 ` [PATCH 01/18] target/riscv: gdbstub: Check priv spec version before reporting CSR Bin Meng
@ 2023-02-14 8:40 ` weiwei
2023-02-17 2:11 ` LIU Zhiwei
1 sibling, 0 replies; 57+ messages in thread
From: weiwei @ 2023-02-14 8:40 UTC (permalink / raw)
To: Bin Meng, qemu-devel
Cc: liweiwei, Alistair Francis, Bin Meng, Daniel Henrique Barboza,
Liu Zhiwei, Palmer Dabbelt, qemu-riscv
On 2023/2/14 02:01, Bin Meng wrote:
> The gdbstub CSR XML is dynamically generated according to the result
> of the CSR predicate() result. This has been working fine until
> commit 7100fe6c2441 ("target/riscv: Enable privileged spec version 1.12")
> introduced the privilege spec version check in riscv_csrrw_check().
>
> When debugging the 'sifive_u' machine whose priv spec is at 1.10,
> gdbstub reports priv spec 1.12 CSRs like menvcfg in the XML, hence
> we see "remote failure reply 'E14'" message when examining all CSRs
> via "info register system" from gdb.
>
> Add the priv spec version check in the CSR XML generation logic to
> fix this issue.
>
> Fixes: 7100fe6c2441 ("target/riscv: Enable privileged spec version 1.12")
> Signed-off-by: Bin Meng <bmeng@tinylab.org>
Reviewed-by: Weiwei Li <liweiwei@iscas.ac.cn>
Regards,
Weiwei Li
> ---
>
> target/riscv/gdbstub.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/target/riscv/gdbstub.c b/target/riscv/gdbstub.c
> index 6e7bbdbd5e..e57372db38 100644
> --- a/target/riscv/gdbstub.c
> +++ b/target/riscv/gdbstub.c
> @@ -290,6 +290,9 @@ static int riscv_gen_dynamic_csr_xml(CPUState *cs, int base_reg)
> g_string_append_printf(s, "<feature name=\"org.gnu.gdb.riscv.csr\">");
>
> for (i = 0; i < CSR_TABLE_SIZE; i++) {
> + if (env->priv_ver < csr_ops[i].min_priv_ver) {
> + continue;
> + }
> predicate = csr_ops[i].predicate;
> if (predicate && (predicate(env, i) == RISCV_EXCP_NONE)) {
> if (csr_ops[i].name) {
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH 02/18] target/riscv: Correct the priority policy of riscv_csrrw_check()
2023-02-13 18:01 ` [PATCH 02/18] target/riscv: Correct the priority policy of riscv_csrrw_check() Bin Meng
@ 2023-02-14 8:43 ` weiwei
2023-02-17 2:15 ` LIU Zhiwei
1 sibling, 0 replies; 57+ messages in thread
From: weiwei @ 2023-02-14 8:43 UTC (permalink / raw)
To: Bin Meng, qemu-devel
Cc: liweiwei, Alistair Francis, Bin Meng, Daniel Henrique Barboza,
Liu Zhiwei, Palmer Dabbelt, qemu-riscv
On 2023/2/14 02:01, Bin Meng wrote:
> 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 came
> after the read / write check.
Hi Bin Meng,
Let me explain why I put read-only check before predicate() check in
commit eacaf4401956:
* The predicates don't do existence check only. They also do some
access-control check , and
will trigger virtual instruction exception in come cases. I think
read-only check should be done
before these access-control check, and trigger illegal instruction
exception instead of virtual
instruction exception when writing to read-only CSRs in these cases.
* Read-only check will trigger ILLEGAL_INST exception which is also
the exception triggered when
CSR is not existed, so put this check before existence check will
not affect the final exception.
Regards,
Weiwei Li
> Fixes: eacaf4401956 ("target/riscv: Fix priority of csr related check in riscv_csrrw_check")
> Signed-off-by: Bin Meng <bmeng@tinylab.org>
> ---
>
> target/riscv/csr.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index 1b0a0c1693..c2dd9d5af0 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -3793,15 +3793,15 @@ static inline RISCVException riscv_csrrw_check(CPURISCVState *env,
> return RISCV_EXCP_ILLEGAL_INST;
> }
>
> - if (write_mask && read_only) {
> - return RISCV_EXCP_ILLEGAL_INST;
> - }
> -
> RISCVException ret = csr_ops[csrno].predicate(env, csrno);
> if (ret != RISCV_EXCP_NONE) {
> return ret;
> }
>
> + if (write_mask && read_only) {
> + return RISCV_EXCP_ILLEGAL_INST;
> + }
> +
> #if !defined(CONFIG_USER_ONLY)
> int csr_priv, effective_priv = env->priv;
>
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH 03/18] target/riscv: gdbstub: Minor change for better readability
2023-02-13 18:01 ` [PATCH 03/18] target/riscv: gdbstub: Minor change for better readability Bin Meng
@ 2023-02-14 8:45 ` weiwei
2023-02-17 2:20 ` LIU Zhiwei
1 sibling, 0 replies; 57+ messages in thread
From: weiwei @ 2023-02-14 8:45 UTC (permalink / raw)
To: Bin Meng, qemu-devel
Cc: liweiwei, Alistair Francis, Bin Meng, Daniel Henrique Barboza,
Liu Zhiwei, Palmer Dabbelt, qemu-riscv
On 2023/2/14 02:01, Bin Meng wrote:
> Use a variable 'base_reg' to represent cs->gdb_num_regs so that
> the call to ricsv_gen_dynamic_vector_xml() can be placed in one
> single line for better readability.
>
> Signed-off-by: Bin Meng <bmeng@tinylab.org>
Reviewed-by: Weiwei Li <liweiwei@iscas.ac.cn>
Regards,
Weiwei Li
> ---
>
> target/riscv/gdbstub.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/target/riscv/gdbstub.c b/target/riscv/gdbstub.c
> index e57372db38..704f3d6922 100644
> --- a/target/riscv/gdbstub.c
> +++ b/target/riscv/gdbstub.c
> @@ -385,9 +385,9 @@ void riscv_cpu_register_gdb_regs_for_features(CPUState *cs)
> 32, "riscv-32bit-fpu.xml", 0);
> }
> if (env->misa_ext & RVV) {
> + int base_reg = cs->gdb_num_regs;
> gdb_register_coprocessor(cs, riscv_gdb_get_vector, riscv_gdb_set_vector,
> - ricsv_gen_dynamic_vector_xml(cs,
> - cs->gdb_num_regs),
> + ricsv_gen_dynamic_vector_xml(cs, base_reg),
> "riscv-vector.xml", 0);
> }
> switch (env->misa_mxl_max) {
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH 04/18] target/riscv: gdbstub: Do not generate CSR XML if Zicsr is disabled
2023-02-13 18:02 ` [PATCH 04/18] target/riscv: gdbstub: Do not generate CSR XML if Zicsr is disabled Bin Meng
@ 2023-02-14 8:46 ` weiwei
2023-02-17 2:23 ` LIU Zhiwei
1 sibling, 0 replies; 57+ messages in thread
From: weiwei @ 2023-02-14 8:46 UTC (permalink / raw)
To: Bin Meng, qemu-devel
Cc: liweiwei, Alistair Francis, Bin Meng, Daniel Henrique Barboza,
Liu Zhiwei, Palmer Dabbelt, qemu-riscv
On 2023/2/14 02:02, Bin Meng wrote:
> There is no need to generate the CSR XML if the Zicsr extension
> is not enabled.
>
> Signed-off-by: Bin Meng <bmeng@tinylab.org>
Reviewed-by: Weiwei Li <liweiwei@iscas.ac.cn>
Regards,
Weiwei Li
> ---
>
> target/riscv/gdbstub.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/target/riscv/gdbstub.c b/target/riscv/gdbstub.c
> index 704f3d6922..294f0ceb1c 100644
> --- a/target/riscv/gdbstub.c
> +++ b/target/riscv/gdbstub.c
> @@ -406,7 +406,10 @@ void riscv_cpu_register_gdb_regs_for_features(CPUState *cs)
> g_assert_not_reached();
> }
>
> - gdb_register_coprocessor(cs, riscv_gdb_get_csr, riscv_gdb_set_csr,
> - riscv_gen_dynamic_csr_xml(cs, cs->gdb_num_regs),
> - "riscv-csr.xml", 0);
> + if (cpu->cfg.ext_icsr) {
> + int base_reg = cs->gdb_num_regs;
> + gdb_register_coprocessor(cs, riscv_gdb_get_csr, riscv_gdb_set_csr,
> + riscv_gen_dynamic_csr_xml(cs, base_reg),
> + "riscv-csr.xml", 0);
> + }
> }
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH 05/18] target/riscv: Coding style fixes in csr.c
2023-02-13 18:02 ` [PATCH 05/18] target/riscv: Coding style fixes in csr.c Bin Meng
@ 2023-02-14 8:48 ` weiwei
2023-02-17 2:24 ` LIU Zhiwei
1 sibling, 0 replies; 57+ messages in thread
From: weiwei @ 2023-02-14 8:48 UTC (permalink / raw)
To: Bin Meng, qemu-devel
Cc: liweiwei, Alistair Francis, Bin Meng, Daniel Henrique Barboza,
Liu Zhiwei, Palmer Dabbelt, qemu-riscv
On 2023/2/14 02:02, Bin Meng wrote:
> Fix various places that violate QEMU coding style:
>
> - correct multi-line comment format
> - indent to opening parenthesis
>
> Signed-off-by: Bin Meng <bmeng@tinylab.org>
Reviewed-by: Weiwei Li <liweiwei@iscas.ac.cn>
Regards,
Weiwei Li
> ---
>
> target/riscv/csr.c | 62 ++++++++++++++++++++++++----------------------
> 1 file changed, 32 insertions(+), 30 deletions(-)
>
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index c2dd9d5af0..cc74819759 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -963,7 +963,7 @@ static RISCVException sstc_32(CPURISCVState *env, int csrno)
> }
>
> static RISCVException read_vstimecmp(CPURISCVState *env, int csrno,
> - target_ulong *val)
> + target_ulong *val)
> {
> *val = env->vstimecmp;
>
> @@ -971,7 +971,7 @@ static RISCVException read_vstimecmp(CPURISCVState *env, int csrno,
> }
>
> static RISCVException read_vstimecmph(CPURISCVState *env, int csrno,
> - target_ulong *val)
> + target_ulong *val)
> {
> *val = env->vstimecmp >> 32;
>
> @@ -979,7 +979,7 @@ static RISCVException read_vstimecmph(CPURISCVState *env, int csrno,
> }
>
> static RISCVException write_vstimecmp(CPURISCVState *env, int csrno,
> - target_ulong val)
> + target_ulong val)
> {
> RISCVCPU *cpu = env_archcpu(env);
>
> @@ -996,7 +996,7 @@ static RISCVException write_vstimecmp(CPURISCVState *env, int csrno,
> }
>
> static RISCVException write_vstimecmph(CPURISCVState *env, int csrno,
> - target_ulong val)
> + target_ulong val)
> {
> RISCVCPU *cpu = env_archcpu(env);
>
> @@ -1020,7 +1020,7 @@ static RISCVException read_stimecmp(CPURISCVState *env, int csrno,
> }
>
> static RISCVException read_stimecmph(CPURISCVState *env, int csrno,
> - target_ulong *val)
> + target_ulong *val)
> {
> if (riscv_cpu_virt_enabled(env)) {
> *val = env->vstimecmp >> 32;
> @@ -1032,7 +1032,7 @@ static RISCVException read_stimecmph(CPURISCVState *env, int csrno,
> }
>
> static RISCVException write_stimecmp(CPURISCVState *env, int csrno,
> - target_ulong val)
> + target_ulong val)
> {
> RISCVCPU *cpu = env_archcpu(env);
>
> @@ -1055,7 +1055,7 @@ static RISCVException write_stimecmp(CPURISCVState *env, int csrno,
> }
>
> static RISCVException write_stimecmph(CPURISCVState *env, int csrno,
> - target_ulong val)
> + target_ulong val)
> {
> RISCVCPU *cpu = env_archcpu(env);
>
> @@ -1342,7 +1342,8 @@ static RISCVException write_misa(CPURISCVState *env, int csrno,
>
> /* 'E' excludes all other extensions */
> if (val & RVE) {
> - /* when we support 'E' we can do "val = RVE;" however
> + /*
> + * when we support 'E' we can do "val = RVE;" however
> * for now we just drop writes if 'E' is present.
> */
> return RISCV_EXCP_NONE;
> @@ -1364,7 +1365,8 @@ static RISCVException write_misa(CPURISCVState *env, int csrno,
> val &= ~RVD;
> }
>
> - /* Suppress 'C' if next instruction is not aligned
> + /*
> + * Suppress 'C' if next instruction is not aligned
> * TODO: this should check next_pc
> */
> if ((val & RVC) && (GETPC() & ~3) != 0) {
> @@ -1833,28 +1835,28 @@ static RISCVException write_mscratch(CPURISCVState *env, int csrno,
> }
>
> static RISCVException read_mepc(CPURISCVState *env, int csrno,
> - target_ulong *val)
> + target_ulong *val)
> {
> *val = env->mepc;
> return RISCV_EXCP_NONE;
> }
>
> static RISCVException write_mepc(CPURISCVState *env, int csrno,
> - target_ulong val)
> + target_ulong val)
> {
> env->mepc = val;
> return RISCV_EXCP_NONE;
> }
>
> static RISCVException read_mcause(CPURISCVState *env, int csrno,
> - target_ulong *val)
> + target_ulong *val)
> {
> *val = env->mcause;
> return RISCV_EXCP_NONE;
> }
>
> static RISCVException write_mcause(CPURISCVState *env, int csrno,
> - target_ulong val)
> + target_ulong val)
> {
> env->mcause = val;
> return RISCV_EXCP_NONE;
> @@ -1876,14 +1878,14 @@ static RISCVException write_mtval(CPURISCVState *env, int csrno,
>
> /* Execution environment configuration setup */
> static RISCVException read_menvcfg(CPURISCVState *env, int csrno,
> - target_ulong *val)
> + target_ulong *val)
> {
> *val = env->menvcfg;
> return RISCV_EXCP_NONE;
> }
>
> static RISCVException write_menvcfg(CPURISCVState *env, int csrno,
> - target_ulong val)
> + target_ulong val)
> {
> uint64_t mask = MENVCFG_FIOM | MENVCFG_CBIE | MENVCFG_CBCFE | MENVCFG_CBZE;
>
> @@ -1896,14 +1898,14 @@ static RISCVException write_menvcfg(CPURISCVState *env, int csrno,
> }
>
> static RISCVException read_menvcfgh(CPURISCVState *env, int csrno,
> - target_ulong *val)
> + target_ulong *val)
> {
> *val = env->menvcfg >> 32;
> return RISCV_EXCP_NONE;
> }
>
> static RISCVException write_menvcfgh(CPURISCVState *env, int csrno,
> - target_ulong val)
> + target_ulong val)
> {
> uint64_t mask = MENVCFG_PBMTE | MENVCFG_STCE;
> uint64_t valh = (uint64_t)val << 32;
> @@ -1914,7 +1916,7 @@ static RISCVException write_menvcfgh(CPURISCVState *env, int csrno,
> }
>
> static RISCVException read_senvcfg(CPURISCVState *env, int csrno,
> - target_ulong *val)
> + target_ulong *val)
> {
> RISCVException ret;
>
> @@ -1928,7 +1930,7 @@ static RISCVException read_senvcfg(CPURISCVState *env, int csrno,
> }
>
> static RISCVException write_senvcfg(CPURISCVState *env, int csrno,
> - target_ulong val)
> + target_ulong val)
> {
> uint64_t mask = SENVCFG_FIOM | SENVCFG_CBIE | SENVCFG_CBCFE | SENVCFG_CBZE;
> RISCVException ret;
> @@ -1943,7 +1945,7 @@ static RISCVException write_senvcfg(CPURISCVState *env, int csrno,
> }
>
> static RISCVException read_henvcfg(CPURISCVState *env, int csrno,
> - target_ulong *val)
> + target_ulong *val)
> {
> RISCVException ret;
>
> @@ -1957,7 +1959,7 @@ static RISCVException read_henvcfg(CPURISCVState *env, int csrno,
> }
>
> static RISCVException write_henvcfg(CPURISCVState *env, int csrno,
> - target_ulong val)
> + target_ulong val)
> {
> uint64_t mask = HENVCFG_FIOM | HENVCFG_CBIE | HENVCFG_CBCFE | HENVCFG_CBZE;
> RISCVException ret;
> @@ -1977,7 +1979,7 @@ static RISCVException write_henvcfg(CPURISCVState *env, int csrno,
> }
>
> static RISCVException read_henvcfgh(CPURISCVState *env, int csrno,
> - target_ulong *val)
> + target_ulong *val)
> {
> RISCVException ret;
>
> @@ -1991,7 +1993,7 @@ static RISCVException read_henvcfgh(CPURISCVState *env, int csrno,
> }
>
> static RISCVException write_henvcfgh(CPURISCVState *env, int csrno,
> - target_ulong val)
> + target_ulong val)
> {
> uint64_t mask = HENVCFG_PBMTE | HENVCFG_STCE;
> uint64_t valh = (uint64_t)val << 32;
> @@ -2034,13 +2036,13 @@ static RISCVException write_mstateen0(CPURISCVState *env, int csrno,
> }
>
> static RISCVException write_mstateen_1_3(CPURISCVState *env, int csrno,
> - target_ulong new_val)
> + target_ulong new_val)
> {
> return write_mstateen(env, csrno, SMSTATEEN_STATEEN, new_val);
> }
>
> static RISCVException read_mstateenh(CPURISCVState *env, int csrno,
> - target_ulong *val)
> + target_ulong *val)
> {
> *val = env->mstateen[csrno - CSR_MSTATEEN0H] >> 32;
>
> @@ -2061,7 +2063,7 @@ static RISCVException write_mstateenh(CPURISCVState *env, int csrno,
> }
>
> static RISCVException write_mstateen0h(CPURISCVState *env, int csrno,
> - target_ulong new_val)
> + target_ulong new_val)
> {
> uint64_t wr_mask = SMSTATEEN_STATEEN | SMSTATEEN0_HSENVCFG;
>
> @@ -2069,7 +2071,7 @@ static RISCVException write_mstateen0h(CPURISCVState *env, int csrno,
> }
>
> static RISCVException write_mstateenh_1_3(CPURISCVState *env, int csrno,
> - target_ulong new_val)
> + target_ulong new_val)
> {
> return write_mstateenh(env, csrno, SMSTATEEN_STATEEN, new_val);
> }
> @@ -2106,7 +2108,7 @@ static RISCVException write_hstateen0(CPURISCVState *env, int csrno,
> }
>
> static RISCVException write_hstateen_1_3(CPURISCVState *env, int csrno,
> - target_ulong new_val)
> + target_ulong new_val)
> {
> return write_hstateen(env, csrno, SMSTATEEN_STATEEN, new_val);
> }
> @@ -2145,7 +2147,7 @@ static RISCVException write_hstateen0h(CPURISCVState *env, int csrno,
> }
>
> static RISCVException write_hstateenh_1_3(CPURISCVState *env, int csrno,
> - target_ulong new_val)
> + target_ulong new_val)
> {
> return write_hstateenh(env, csrno, SMSTATEEN_STATEEN, new_val);
> }
> @@ -3338,7 +3340,7 @@ static RISCVException read_mseccfg(CPURISCVState *env, int csrno,
> }
>
> static RISCVException write_mseccfg(CPURISCVState *env, int csrno,
> - target_ulong val)
> + target_ulong val)
> {
> mseccfg_csr_write(env, val);
> return RISCV_EXCP_NONE;
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH 06/18] target/riscv: Use 'bool' type for read_only
2023-02-13 18:02 ` [PATCH 06/18] target/riscv: Use 'bool' type for read_only Bin Meng
@ 2023-02-14 8:48 ` weiwei
2023-02-17 2:24 ` LIU Zhiwei
1 sibling, 0 replies; 57+ messages in thread
From: weiwei @ 2023-02-14 8:48 UTC (permalink / raw)
To: Bin Meng, qemu-devel
Cc: liweiwei, Alistair Francis, Bin Meng, Daniel Henrique Barboza,
Liu Zhiwei, Palmer Dabbelt, qemu-riscv
On 2023/2/14 02:02, Bin Meng wrote:
> The read_only variable is currently declared as an 'int', but it
> should really be a 'bool'.
>
> Signed-off-by: Bin Meng <bmeng@tinylab.org>
Reviewed-by: Weiwei Li <liweiwei@iscas.ac.cn>
Regards,
Weiwei Li
> ---
>
> target/riscv/csr.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index cc74819759..8bbc75cbfa 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -3778,7 +3778,7 @@ static inline RISCVException riscv_csrrw_check(CPURISCVState *env,
> RISCVCPU *cpu)
> {
> /* check privileges and return RISCV_EXCP_ILLEGAL_INST if check fails */
> - int read_only = get_field(csrno, 0xC00) == 3;
> + bool read_only = get_field(csrno, 0xC00) == 3;
> int csr_min_priv = csr_ops[csrno].min_priv_ver;
>
> /* ensure the CSR extension is enabled. */
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH 07/18] target/riscv: Simplify {read,write}_pmpcfg() a little bit
2023-02-13 18:02 ` [PATCH 07/18] target/riscv: Simplify {read, write}_pmpcfg() a little bit Bin Meng
@ 2023-02-14 8:50 ` weiwei
2023-02-17 2:26 ` LIU Zhiwei
1 sibling, 0 replies; 57+ messages in thread
From: weiwei @ 2023-02-14 8:50 UTC (permalink / raw)
To: Bin Meng, qemu-devel
Cc: liweiwei, Alistair Francis, Bin Meng, Daniel Henrique Barboza,
Liu Zhiwei, Palmer Dabbelt, qemu-riscv
On 2023/2/14 02:02, Bin Meng wrote:
> Use the register index that has already been calculated in the
> pmpcfg_csr_{read,write} call.
>
> Signed-off-by: Bin Meng <bmeng@tinylab.org>
Reviewed-by: Weiwei Li <liweiwei@iscas.ac.cn>
Regards,
Weiwei Li
> ---
>
> target/riscv/csr.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index 8bbc75cbfa..da3b770894 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -3363,7 +3363,7 @@ static RISCVException read_pmpcfg(CPURISCVState *env, int csrno,
> if (!check_pmp_reg_index(env, reg_index)) {
> return RISCV_EXCP_ILLEGAL_INST;
> }
> - *val = pmpcfg_csr_read(env, csrno - CSR_PMPCFG0);
> + *val = pmpcfg_csr_read(env, reg_index);
> return RISCV_EXCP_NONE;
> }
>
> @@ -3375,7 +3375,7 @@ static RISCVException write_pmpcfg(CPURISCVState *env, int csrno,
> if (!check_pmp_reg_index(env, reg_index)) {
> return RISCV_EXCP_ILLEGAL_INST;
> }
> - pmpcfg_csr_write(env, csrno - CSR_PMPCFG0, val);
> + pmpcfg_csr_write(env, reg_index, val);
> return RISCV_EXCP_NONE;
> }
>
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH 08/18] target/riscv: Simplify getting RISCVCPU pointer from env
2023-02-13 18:02 ` [PATCH 08/18] target/riscv: Simplify getting RISCVCPU pointer from env Bin Meng
@ 2023-02-14 8:51 ` weiwei
2023-02-17 2:30 ` LIU Zhiwei
1 sibling, 0 replies; 57+ messages in thread
From: weiwei @ 2023-02-14 8:51 UTC (permalink / raw)
To: Bin Meng, qemu-devel
Cc: liweiwei, Alistair Francis, Bin Meng, Daniel Henrique Barboza,
Liu Zhiwei, Palmer Dabbelt, qemu-riscv
On 2023/2/14 02:02, Bin Meng wrote:
> Use env_archcpu() to get RISCVCPU pointer from env directly.
>
> Signed-off-by: Bin Meng <bmeng@tinylab.org>
Reviewed-by: Weiwei Li <liweiwei@iscas.ac.cn>
Regards,
Weiwei Li
> ---
>
> target/riscv/csr.c | 36 ++++++++++++------------------------
> 1 file changed, 12 insertions(+), 24 deletions(-)
>
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index da3b770894..0a3f2bef6f 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -46,8 +46,7 @@ static RISCVException smstateen_acc_ok(CPURISCVState *env, int index,
> uint64_t bit)
> {
> bool virt = riscv_cpu_virt_enabled(env);
> - CPUState *cs = env_cpu(env);
> - RISCVCPU *cpu = RISCV_CPU(cs);
> + RISCVCPU *cpu = env_archcpu(env);
>
> if (env->priv == PRV_M || !cpu->cfg.ext_smstateen) {
> return RISCV_EXCP_NONE;
> @@ -90,8 +89,7 @@ static RISCVException fs(CPURISCVState *env, int csrno)
>
> static RISCVException vs(CPURISCVState *env, int csrno)
> {
> - CPUState *cs = env_cpu(env);
> - RISCVCPU *cpu = RISCV_CPU(cs);
> + RISCVCPU *cpu = env_archcpu(env);
>
> if (env->misa_ext & RVV ||
> cpu->cfg.ext_zve32f || cpu->cfg.ext_zve64f) {
> @@ -108,8 +106,7 @@ static RISCVException vs(CPURISCVState *env, int csrno)
> static RISCVException ctr(CPURISCVState *env, int csrno)
> {
> #if !defined(CONFIG_USER_ONLY)
> - CPUState *cs = env_cpu(env);
> - RISCVCPU *cpu = RISCV_CPU(cs);
> + RISCVCPU *cpu = env_archcpu(env);
> int ctr_index;
> target_ulong ctr_mask;
> int base_csrno = CSR_CYCLE;
> @@ -166,8 +163,7 @@ static RISCVException ctr32(CPURISCVState *env, int csrno)
> #if !defined(CONFIG_USER_ONLY)
> static RISCVException mctr(CPURISCVState *env, int csrno)
> {
> - CPUState *cs = env_cpu(env);
> - RISCVCPU *cpu = RISCV_CPU(cs);
> + RISCVCPU *cpu = env_archcpu(env);
> int ctr_index;
> int base_csrno = CSR_MHPMCOUNTER3;
>
> @@ -195,8 +191,7 @@ static RISCVException mctr32(CPURISCVState *env, int csrno)
>
> static RISCVException sscofpmf(CPURISCVState *env, int csrno)
> {
> - CPUState *cs = env_cpu(env);
> - RISCVCPU *cpu = RISCV_CPU(cs);
> + RISCVCPU *cpu = env_archcpu(env);
>
> if (!cpu->cfg.ext_sscofpmf) {
> return RISCV_EXCP_ILLEGAL_INST;
> @@ -321,8 +316,7 @@ static RISCVException umode32(CPURISCVState *env, int csrno)
>
> static RISCVException mstateen(CPURISCVState *env, int csrno)
> {
> - CPUState *cs = env_cpu(env);
> - RISCVCPU *cpu = RISCV_CPU(cs);
> + RISCVCPU *cpu = env_archcpu(env);
>
> if (!cpu->cfg.ext_smstateen) {
> return RISCV_EXCP_ILLEGAL_INST;
> @@ -333,8 +327,7 @@ static RISCVException mstateen(CPURISCVState *env, int csrno)
>
> static RISCVException hstateen_pred(CPURISCVState *env, int csrno, int base)
> {
> - CPUState *cs = env_cpu(env);
> - RISCVCPU *cpu = RISCV_CPU(cs);
> + RISCVCPU *cpu = env_archcpu(env);
>
> if (!cpu->cfg.ext_smstateen) {
> return RISCV_EXCP_ILLEGAL_INST;
> @@ -363,8 +356,7 @@ static RISCVException sstateen(CPURISCVState *env, int csrno)
> {
> bool virt = riscv_cpu_virt_enabled(env);
> int index = csrno - CSR_SSTATEEN0;
> - CPUState *cs = env_cpu(env);
> - RISCVCPU *cpu = RISCV_CPU(cs);
> + RISCVCPU *cpu = env_archcpu(env);
>
> if (!cpu->cfg.ext_smstateen) {
> return RISCV_EXCP_ILLEGAL_INST;
> @@ -918,8 +910,7 @@ static RISCVException read_timeh(CPURISCVState *env, int csrno,
>
> static RISCVException sstc(CPURISCVState *env, int csrno)
> {
> - CPUState *cs = env_cpu(env);
> - RISCVCPU *cpu = RISCV_CPU(cs);
> + RISCVCPU *cpu = env_archcpu(env);
> bool hmode_check = false;
>
> if (!cpu->cfg.ext_sstc || !env->rdtime_fn) {
> @@ -1152,8 +1143,7 @@ static RISCVException write_ignore(CPURISCVState *env, int csrno,
> static RISCVException read_mvendorid(CPURISCVState *env, int csrno,
> target_ulong *val)
> {
> - CPUState *cs = env_cpu(env);
> - RISCVCPU *cpu = RISCV_CPU(cs);
> + RISCVCPU *cpu = env_archcpu(env);
>
> *val = cpu->cfg.mvendorid;
> return RISCV_EXCP_NONE;
> @@ -1162,8 +1152,7 @@ static RISCVException read_mvendorid(CPURISCVState *env, int csrno,
> static RISCVException read_marchid(CPURISCVState *env, int csrno,
> target_ulong *val)
> {
> - CPUState *cs = env_cpu(env);
> - RISCVCPU *cpu = RISCV_CPU(cs);
> + RISCVCPU *cpu = env_archcpu(env);
>
> *val = cpu->cfg.marchid;
> return RISCV_EXCP_NONE;
> @@ -1172,8 +1161,7 @@ static RISCVException read_marchid(CPURISCVState *env, int csrno,
> static RISCVException read_mimpid(CPURISCVState *env, int csrno,
> target_ulong *val)
> {
> - CPUState *cs = env_cpu(env);
> - RISCVCPU *cpu = RISCV_CPU(cs);
> + RISCVCPU *cpu = env_archcpu(env);
>
> *val = cpu->cfg.mimpid;
> return RISCV_EXCP_NONE;
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH 09/18] target/riscv: Avoid reporting odd-numbered pmpcfgX in the CSR XML for RV64
2023-02-13 18:02 ` [PATCH 09/18] target/riscv: Avoid reporting odd-numbered pmpcfgX in the CSR XML for RV64 Bin Meng
@ 2023-02-14 8:56 ` weiwei
2023-02-17 2:36 ` LIU Zhiwei
1 sibling, 0 replies; 57+ messages in thread
From: weiwei @ 2023-02-14 8:56 UTC (permalink / raw)
To: Bin Meng, qemu-devel
Cc: liweiwei, Alistair Francis, Bin Meng, Daniel Henrique Barboza,
Liu Zhiwei, Palmer Dabbelt, qemu-riscv
[-- Attachment #1: Type: text/plain, Size: 2414 bytes --]
On 2023/2/14 02:02, Bin Meng wrote:
> At present the odd-numbered PMP configuration registers for RV64 are
> reported in the CSR XML by QEMU gdbstub. However these registers do
> not exist on RV64 so trying to access them from gdb results in 'E14'.
>
> Move the pmpcfgX index check from the actual read/write routine to
> the PMP CSR predicate() routine, so that non-existent pmpcfgX won't
> be reported in the CSR XML for RV64.
>
> Signed-off-by: Bin Meng<bmeng@tinylab.org>
Reviewed-by: Weiwei Li <liweiwei@iscas.ac.cn>
Regards,
Weiwei Li
> ---
>
> target/riscv/csr.c | 23 ++++++++---------------
> 1 file changed, 8 insertions(+), 15 deletions(-)
>
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index 0a3f2bef6f..749d0ef83e 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -412,6 +412,14 @@ static int aia_hmode32(CPURISCVState *env, int csrno)
> static RISCVException pmp(CPURISCVState *env, int csrno)
> {
> if (riscv_feature(env, RISCV_FEATURE_PMP)) {
> + if (csrno <= CSR_PMPCFG3) {
> + uint32_t reg_index = csrno - CSR_PMPCFG0;
> +
> + if ((reg_index & 1) && (riscv_cpu_mxl(env) == MXL_RV64)) {
> + return RISCV_EXCP_ILLEGAL_INST;
> + }
> + }
> +
> return RISCV_EXCP_NONE;
> }
>
> @@ -3334,23 +3342,11 @@ static RISCVException write_mseccfg(CPURISCVState *env, int csrno,
> return RISCV_EXCP_NONE;
> }
>
> -static bool check_pmp_reg_index(CPURISCVState *env, uint32_t reg_index)
> -{
> - /* TODO: RV128 restriction check */
> - if ((reg_index & 1) && (riscv_cpu_mxl(env) == MXL_RV64)) {
> - return false;
> - }
> - return true;
> -}
> -
> static RISCVException read_pmpcfg(CPURISCVState *env, int csrno,
> target_ulong *val)
> {
> uint32_t reg_index = csrno - CSR_PMPCFG0;
>
> - if (!check_pmp_reg_index(env, reg_index)) {
> - return RISCV_EXCP_ILLEGAL_INST;
> - }
> *val = pmpcfg_csr_read(env, reg_index);
> return RISCV_EXCP_NONE;
> }
> @@ -3360,9 +3356,6 @@ static RISCVException write_pmpcfg(CPURISCVState *env, int csrno,
> {
> uint32_t reg_index = csrno - CSR_PMPCFG0;
>
> - if (!check_pmp_reg_index(env, reg_index)) {
> - return RISCV_EXCP_ILLEGAL_INST;
> - }
> pmpcfg_csr_write(env, reg_index, val);
> return RISCV_EXCP_NONE;
> }
[-- Attachment #2: Type: text/html, Size: 3231 bytes --]
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH 10/18] target/riscv: gdbstub: Turn on debugger mode before calling CSR predicate()
2023-02-13 18:02 ` [PATCH 10/18] target/riscv: gdbstub: Turn on debugger mode before calling CSR predicate() Bin Meng
@ 2023-02-14 9:02 ` weiwei
2023-02-17 2:39 ` LIU Zhiwei
1 sibling, 0 replies; 57+ messages in thread
From: weiwei @ 2023-02-14 9:02 UTC (permalink / raw)
To: Bin Meng, qemu-devel
Cc: liweiwei, Alistair Francis, Bin Meng, Daniel Henrique Barboza,
Liu Zhiwei, Palmer Dabbelt, qemu-riscv
On 2023/2/14 02:02, Bin Meng wrote:
> Since commit 94452ac4cf26 ("target/riscv: remove fflags, frm, and fcsr from riscv-*-fpu.xml")
> the 3 FPU CSRs are removed from the XML target decription. The
> original intent of that commit was based on the assumption that
> the 3 FPU CSRs will show up in the riscv-csr.xml so the ones in
> riscv-*-fpu.xml are redundant. But unforuantely that is not ture.
typo here -> true
otherwise, Reviewed-by: Weiwei Li <liweiwei@iscas.ac.cn>
Regards,
Weiwei Li
> As the FPU CSR predicate() has a run-time check on MSTATUS.FS,
> at the time when CSR XML is generated MSTATUS.FS is unset, hence
> no FPU CSRs will be reported.
>
> The FPU CSR predicate() already considered such a case of being
> accessed by a debugger. All we need to do is to turn on debugger
> mode before calling predicate().
>
> Signed-off-by: Bin Meng <bmeng@tinylab.org>
> ---
>
> target/riscv/gdbstub.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/target/riscv/gdbstub.c b/target/riscv/gdbstub.c
> index 294f0ceb1c..ef52f41460 100644
> --- a/target/riscv/gdbstub.c
> +++ b/target/riscv/gdbstub.c
> @@ -280,6 +280,10 @@ static int riscv_gen_dynamic_csr_xml(CPUState *cs, int base_reg)
> int bitsize = 16 << env->misa_mxl_max;
> int i;
>
> +#if !defined(CONFIG_USER_ONLY)
> + env->debugger = true;
> +#endif
> +
> /* Until gdb knows about 128-bit registers */
> if (bitsize > 64) {
> bitsize = 64;
> @@ -308,6 +312,11 @@ static int riscv_gen_dynamic_csr_xml(CPUState *cs, int base_reg)
> g_string_append_printf(s, "</feature>");
>
> cpu->dyn_csr_xml = g_string_free(s, false);
> +
> +#if !defined(CONFIG_USER_ONLY)
> + env->debugger = false;
> +#endif
> +
> return CSR_TABLE_SIZE;
> }
>
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH 11/18] target/riscv: gdbstub: Drop the vector CSRs in riscv-vector.xml
2023-02-13 18:02 ` [PATCH 11/18] target/riscv: gdbstub: Drop the vector CSRs in riscv-vector.xml Bin Meng
@ 2023-02-14 9:13 ` weiwei
2023-02-17 2:43 ` LIU Zhiwei
1 sibling, 0 replies; 57+ messages in thread
From: weiwei @ 2023-02-14 9:13 UTC (permalink / raw)
To: Bin Meng, qemu-devel
Cc: liweiwei, Alistair Francis, Bin Meng, Daniel Henrique Barboza,
Liu Zhiwei, Palmer Dabbelt, qemu-riscv
On 2023/2/14 02:02, Bin Meng wrote:
> It's worth noting that the vector CSR predicate() has a similar
> run-time check logic to the FPU CSR. With the previous patch our
> gdbstub can correctly report these vector CSRs via the CSR xml.
>
> Commit 719d3561b269 ("target/riscv: gdb: support vector registers for rv64 & rv32")
> inserted these vector CSRs in an ad-hoc, non-standard way in the
> riscv-vector.xml. Now we can treat these CSRs no different from
> other CSRs.
>
> Signed-off-by: Bin Meng <bmeng@tinylab.org>
Reviewed-by: Weiwei Li <liweiwei@iscas.ac.cn>
Regards,
Weiwei Li
> ---
>
> target/riscv/gdbstub.c | 75 ------------------------------------------
> 1 file changed, 75 deletions(-)
>
> diff --git a/target/riscv/gdbstub.c b/target/riscv/gdbstub.c
> index ef52f41460..6048541606 100644
> --- a/target/riscv/gdbstub.c
> +++ b/target/riscv/gdbstub.c
> @@ -127,40 +127,6 @@ static int riscv_gdb_set_fpu(CPURISCVState *env, uint8_t *mem_buf, int n)
> return 0;
> }
>
> -/*
> - * Convert register index number passed by GDB to the correspond
> - * vector CSR number. Vector CSRs are defined after vector registers
> - * in dynamic generated riscv-vector.xml, thus the starting register index
> - * of vector CSRs is 32.
> - * Return 0 if register index number is out of range.
> - */
> -static int riscv_gdb_vector_csrno(int num_regs)
> -{
> - /*
> - * The order of vector CSRs in the switch case
> - * should match with the order defined in csr_ops[].
> - */
> - switch (num_regs) {
> - case 32:
> - return CSR_VSTART;
> - case 33:
> - return CSR_VXSAT;
> - case 34:
> - return CSR_VXRM;
> - case 35:
> - return CSR_VCSR;
> - case 36:
> - return CSR_VL;
> - case 37:
> - return CSR_VTYPE;
> - case 38:
> - return CSR_VLENB;
> - default:
> - /* Unknown register. */
> - return 0;
> - }
> -}
> -
> static int riscv_gdb_get_vector(CPURISCVState *env, GByteArray *buf, int n)
> {
> uint16_t vlenb = env_archcpu(env)->cfg.vlen >> 3;
> @@ -174,19 +140,6 @@ static int riscv_gdb_get_vector(CPURISCVState *env, GByteArray *buf, int n)
> return cnt;
> }
>
> - int csrno = riscv_gdb_vector_csrno(n);
> -
> - if (!csrno) {
> - return 0;
> - }
> -
> - target_ulong val = 0;
> - int result = riscv_csrrw_debug(env, csrno, &val, 0, 0);
> -
> - if (result == RISCV_EXCP_NONE) {
> - return gdb_get_regl(buf, val);
> - }
> -
> return 0;
> }
>
> @@ -201,19 +154,6 @@ static int riscv_gdb_set_vector(CPURISCVState *env, uint8_t *mem_buf, int n)
> return vlenb;
> }
>
> - int csrno = riscv_gdb_vector_csrno(n);
> -
> - if (!csrno) {
> - return 0;
> - }
> -
> - target_ulong val = ldtul_p(mem_buf);
> - int result = riscv_csrrw_debug(env, csrno, NULL, val, -1);
> -
> - if (result == RISCV_EXCP_NONE) {
> - return sizeof(target_ulong);
> - }
> -
> return 0;
> }
>
> @@ -361,21 +301,6 @@ static int ricsv_gen_dynamic_vector_xml(CPUState *cs, int base_reg)
> num_regs++;
> }
>
> - /* Define vector CSRs */
> - const char *vector_csrs[7] = {
> - "vstart", "vxsat", "vxrm", "vcsr",
> - "vl", "vtype", "vlenb"
> - };
> -
> - for (i = 0; i < 7; i++) {
> - g_string_append_printf(s,
> - "<reg name=\"%s\" bitsize=\"%d\""
> - " regnum=\"%d\" group=\"vector\""
> - " type=\"int\"/>",
> - vector_csrs[i], TARGET_LONG_BITS, base_reg++);
> - num_regs++;
> - }
> -
> g_string_append_printf(s, "</feature>");
>
> cpu->dyn_vreg_xml = g_string_free(s, false);
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH 12/18] target/riscv: Allow debugger to access user timer and counter CSRs
2023-02-14 1:09 ` [PATCH 12/18] target/riscv: Allow debugger to access user timer and counter CSRs Bin Meng
@ 2023-02-14 9:16 ` weiwei
2023-02-17 2:48 ` LIU Zhiwei
1 sibling, 0 replies; 57+ messages in thread
From: weiwei @ 2023-02-14 9:16 UTC (permalink / raw)
To: Bin Meng, qemu-devel
Cc: liweiwei, Alistair Francis, Bin Meng, Daniel Henrique Barboza,
Liu Zhiwei, Palmer Dabbelt, qemu-riscv
On 2023/2/14 09:09, Bin Meng wrote:
> At present user timer and counter CSRs are not reported in the
> CSR XML hence gdb cannot access them.
>
> Fix it by addding a debugger check in their predicate() routine.
typo: adding
Otherwise, Reviewed-by: Weiwei Li <liweiwei@iscas.ac.cn>
Regards,
Weiwei Li
> Signed-off-by: Bin Meng <bmeng@tinylab.org>
> ---
>
> target/riscv/csr.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index 749d0ef83e..515b05348b 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -131,6 +131,10 @@ static RISCVException ctr(CPURISCVState *env, int csrno)
>
> skip_ext_pmu_check:
>
> + if (env->debugger) {
> + return RISCV_EXCP_NONE;
> + }
> +
> if (env->priv < PRV_M && !get_field(env->mcounteren, ctr_mask)) {
> return RISCV_EXCP_ILLEGAL_INST;
> }
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH 13/18] target/riscv: Allow debugger to access seed CSR
2023-02-14 1:09 ` [PATCH 13/18] target/riscv: Allow debugger to access seed CSR Bin Meng
@ 2023-02-14 9:18 ` weiwei
2023-02-17 2:59 ` LIU Zhiwei
1 sibling, 0 replies; 57+ messages in thread
From: weiwei @ 2023-02-14 9:18 UTC (permalink / raw)
To: Bin Meng, qemu-devel
Cc: liweiwei, Alistair Francis, Bin Meng, Daniel Henrique Barboza,
Liu Zhiwei, Palmer Dabbelt, qemu-riscv
On 2023/2/14 09:09, Bin Meng wrote:
> At present seed CSR is not reported in the CSR XML hence gdb cannot
> access it.
>
> Fix it by addding a debugger check in its predicate() routine.
typo: adding
Otherwise, Reviewed-by: Weiwei Li <liweiwei@iscas.ac.cn>
Regards,
Weiwei Li
>
> Signed-off-by: Bin Meng <bmeng@tinylab.org>
> ---
>
> target/riscv/csr.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index 515b05348b..f1075b5728 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -458,6 +458,10 @@ static RISCVException seed(CPURISCVState *env, int csrno)
> }
>
> #if !defined(CONFIG_USER_ONLY)
> + if (env->debugger) {
> + return RISCV_EXCP_NONE;
> + }
> +
> /*
> * With a CSR read-write instruction:
> * 1) The seed CSR is always available in machine mode as normal.
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH 14/18] target/riscv: Allow debugger to access {h, s}stateen CSRs
2023-02-14 3:06 ` [PATCH 14/18] target/riscv: Allow debugger to access {h, s}stateen CSRs Bin Meng
@ 2023-02-14 9:24 ` weiwei
0 siblings, 0 replies; 57+ messages in thread
From: weiwei @ 2023-02-14 9:24 UTC (permalink / raw)
To: Bin Meng, qemu-devel
Cc: liweiwei, Alistair Francis, Bin Meng, Daniel Henrique Barboza,
Liu Zhiwei, Palmer Dabbelt, qemu-riscv
On 2023/2/14 11:06, Bin Meng wrote:
> At present {h,s}stateen CSRs are not reported in the CSR XML
> hence gdb cannot access them.
>
> Fix it by adjusting their predicate() routine logic so that the
> static config check comes before the run-time check, as well as
> addding a debugger check.
>
> Signed-off-by: Bin Meng <bmeng@tinylab.org>
Similar typo,
Otherwise, Reviewed-by: Weiwei Li <liweiwei@iscas.ac.cn>
Regards,
Weiwei Li
> ---
>
> target/riscv/csr.c | 22 ++++++++++++++++++++--
> 1 file changed, 20 insertions(+), 2 deletions(-)
>
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index f1075b5728..d6bcb7f275 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -337,13 +337,22 @@ static RISCVException hstateen_pred(CPURISCVState *env, int csrno, int base)
> return RISCV_EXCP_ILLEGAL_INST;
> }
>
> + RISCVException ret = hmode(env, csrno);
> + if (ret != RISCV_EXCP_NONE) {
> + return ret;
> + }
> +
> + if (env->debugger) {
> + return RISCV_EXCP_NONE;
> + }
> +
> if (env->priv < PRV_M) {
> if (!(env->mstateen[csrno - base] & SMSTATEEN_STATEEN)) {
> return RISCV_EXCP_ILLEGAL_INST;
> }
> }
>
> - return hmode(env, csrno);
> + return RISCV_EXCP_NONE;
> }
>
> static RISCVException hstateen(CPURISCVState *env, int csrno)
> @@ -366,6 +375,15 @@ static RISCVException sstateen(CPURISCVState *env, int csrno)
> return RISCV_EXCP_ILLEGAL_INST;
> }
>
> + RISCVException ret = smode(env, csrno);
> + if (ret != RISCV_EXCP_NONE) {
> + return ret;
> + }
> +
> + if (env->debugger) {
> + return RISCV_EXCP_NONE;
> + }
> +
> if (env->priv < PRV_M) {
> if (!(env->mstateen[index] & SMSTATEEN_STATEEN)) {
> return RISCV_EXCP_ILLEGAL_INST;
> @@ -378,7 +396,7 @@ static RISCVException sstateen(CPURISCVState *env, int csrno)
> }
> }
>
> - return smode(env, csrno);
> + return RISCV_EXCP_NONE;
> }
>
> /* Checks if PointerMasking registers could be accessed */
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH 15/18] target/riscv: Allow debugger to access sstc CSRs
2023-02-14 4:12 ` [PATCH 15/18] target/riscv: Allow debugger to access sstc CSRs Bin Meng
@ 2023-02-14 9:26 ` weiwei
0 siblings, 0 replies; 57+ messages in thread
From: weiwei @ 2023-02-14 9:26 UTC (permalink / raw)
To: Bin Meng, qemu-devel
Cc: liweiwei, Alistair Francis, Bin Meng, Daniel Henrique Barboza,
Liu Zhiwei, Palmer Dabbelt, qemu-riscv
On 2023/2/14 12:12, Bin Meng wrote:
> At present with a debugger attached sstc CSRs can only be accssed
> when CPU is in M-mode, or configured correctly.
>
> Fix it by adjusting their predicate() routine logic so that the
> static config check comes before the run-time check, as well as
> addding a debugger check.
>
> Signed-off-by: Bin Meng <bmeng@tinylab.org>
Similar typo, otherwise Reviewed-by: Weiwei Li <liweiwei@iscas.ac.cn>
Regards,
Weiwei Li
> ---
>
> target/riscv/csr.c | 19 ++++++++++++++-----
> 1 file changed, 14 insertions(+), 5 deletions(-)
>
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index d6bcb7f275..c6a7745cb2 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -951,6 +951,19 @@ static RISCVException sstc(CPURISCVState *env, int csrno)
> return RISCV_EXCP_ILLEGAL_INST;
> }
>
> + if ((csrno == CSR_VSTIMECMP) || (csrno == CSR_VSTIMECMPH)) {
> + hmode_check = true;
> + }
> +
> + RISCVException ret = hmode_check ? hmode(env, csrno) : smode(env, csrno);
> + if (ret != RISCV_EXCP_NONE) {
> + return ret;
> + }
> +
> + if (env->debugger) {
> + return RISCV_EXCP_NONE;
> + }
> +
> if (env->priv == PRV_M) {
> return RISCV_EXCP_NONE;
> }
> @@ -971,11 +984,7 @@ static RISCVException sstc(CPURISCVState *env, int csrno)
> }
> }
>
> - if ((csrno == CSR_VSTIMECMP) || (csrno == CSR_VSTIMECMPH)) {
> - hmode_check = true;
> - }
> -
> - return hmode_check ? hmode(env, csrno) : smode(env, csrno);
> + return RISCV_EXCP_NONE;
> }
>
> static RISCVException sstc_32(CPURISCVState *env, int csrno)
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH 16/18] target/riscv: Drop priv level check in mseccfg predicate()
2023-02-14 4:12 ` [PATCH 16/18] target/riscv: Drop priv level check in mseccfg predicate() Bin Meng
@ 2023-02-14 9:26 ` weiwei
0 siblings, 0 replies; 57+ messages in thread
From: weiwei @ 2023-02-14 9:26 UTC (permalink / raw)
To: Bin Meng, qemu-devel
Cc: liweiwei, Alistair Francis, Bin Meng, Daniel Henrique Barboza,
Liu Zhiwei, Palmer Dabbelt, qemu-riscv
On 2023/2/14 12:12, Bin Meng wrote:
> riscv_csrrw_check() already does the generic privilege level check
> hence there is no need to do the specific M-mode access check in
> the mseccfg predicate().
>
> With this change debugger can access the mseccfg CSR anytime.
>
> Signed-off-by: Bin Meng <bmeng@tinylab.org>
Reviewed-by: Weiwei Li <liweiwei@iscas.ac.cn>
Regards,
Weiwei Li
> ---
>
> target/riscv/csr.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index c6a7745cb2..40aae9e7b3 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -450,7 +450,7 @@ static RISCVException pmp(CPURISCVState *env, int csrno)
>
> static RISCVException epmp(CPURISCVState *env, int csrno)
> {
> - if (env->priv == PRV_M && riscv_feature(env, RISCV_FEATURE_EPMP)) {
> + if (riscv_feature(env, RISCV_FEATURE_EPMP)) {
> return RISCV_EXCP_NONE;
> }
>
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH 17/18] target/riscv: Group all predicate() routines together
2023-02-14 4:31 ` [PATCH 17/18] target/riscv: Group all predicate() routines together Bin Meng
@ 2023-02-14 9:27 ` weiwei
0 siblings, 0 replies; 57+ messages in thread
From: weiwei @ 2023-02-14 9:27 UTC (permalink / raw)
To: Bin Meng, qemu-devel
Cc: liweiwei, Alistair Francis, Bin Meng, Daniel Henrique Barboza,
Liu Zhiwei, Palmer Dabbelt, qemu-riscv
On 2023/2/14 12:31, Bin Meng wrote:
> Move sstc()/sstc32() to where all predicate() routines live.
>
> Signed-off-by: Bin Meng <bmeng@tinylab.org>
Reviewed-by: Weiwei Li <liweiwei@iscas.ac.cn>
Regards,
Weiwei Li
> ---
>
> target/riscv/csr.c | 108 ++++++++++++++++++++++-----------------------
> 1 file changed, 54 insertions(+), 54 deletions(-)
>
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index 40aae9e7b3..37350b8a6d 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -399,6 +399,60 @@ static RISCVException sstateen(CPURISCVState *env, int csrno)
> return RISCV_EXCP_NONE;
> }
>
> +static RISCVException sstc(CPURISCVState *env, int csrno)
> +{
> + RISCVCPU *cpu = env_archcpu(env);
> + bool hmode_check = false;
> +
> + if (!cpu->cfg.ext_sstc || !env->rdtime_fn) {
> + return RISCV_EXCP_ILLEGAL_INST;
> + }
> +
> + if ((csrno == CSR_VSTIMECMP) || (csrno == CSR_VSTIMECMPH)) {
> + hmode_check = true;
> + }
> +
> + RISCVException ret = hmode_check ? hmode(env, csrno) : smode(env, csrno);
> + if (ret != RISCV_EXCP_NONE) {
> + return ret;
> + }
> +
> + if (env->debugger) {
> + return RISCV_EXCP_NONE;
> + }
> +
> + if (env->priv == PRV_M) {
> + return RISCV_EXCP_NONE;
> + }
> +
> + /*
> + * No need of separate function for rv32 as menvcfg stores both menvcfg
> + * menvcfgh for RV32.
> + */
> + if (!(get_field(env->mcounteren, COUNTEREN_TM) &&
> + get_field(env->menvcfg, MENVCFG_STCE))) {
> + return RISCV_EXCP_ILLEGAL_INST;
> + }
> +
> + if (riscv_cpu_virt_enabled(env)) {
> + if (!(get_field(env->hcounteren, COUNTEREN_TM) &&
> + get_field(env->henvcfg, HENVCFG_STCE))) {
> + return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
> + }
> + }
> +
> + return RISCV_EXCP_NONE;
> +}
> +
> +static RISCVException sstc_32(CPURISCVState *env, int csrno)
> +{
> + if (riscv_cpu_mxl(env) != MXL_RV32) {
> + return RISCV_EXCP_ILLEGAL_INST;
> + }
> +
> + return sstc(env, csrno);
> +}
> +
> /* Checks if PointerMasking registers could be accessed */
> static RISCVException pointer_masking(CPURISCVState *env, int csrno)
> {
> @@ -942,60 +996,6 @@ static RISCVException read_timeh(CPURISCVState *env, int csrno,
> return RISCV_EXCP_NONE;
> }
>
> -static RISCVException sstc(CPURISCVState *env, int csrno)
> -{
> - RISCVCPU *cpu = env_archcpu(env);
> - bool hmode_check = false;
> -
> - if (!cpu->cfg.ext_sstc || !env->rdtime_fn) {
> - return RISCV_EXCP_ILLEGAL_INST;
> - }
> -
> - if ((csrno == CSR_VSTIMECMP) || (csrno == CSR_VSTIMECMPH)) {
> - hmode_check = true;
> - }
> -
> - RISCVException ret = hmode_check ? hmode(env, csrno) : smode(env, csrno);
> - if (ret != RISCV_EXCP_NONE) {
> - return ret;
> - }
> -
> - if (env->debugger) {
> - return RISCV_EXCP_NONE;
> - }
> -
> - if (env->priv == PRV_M) {
> - return RISCV_EXCP_NONE;
> - }
> -
> - /*
> - * No need of separate function for rv32 as menvcfg stores both menvcfg
> - * menvcfgh for RV32.
> - */
> - if (!(get_field(env->mcounteren, COUNTEREN_TM) &&
> - get_field(env->menvcfg, MENVCFG_STCE))) {
> - return RISCV_EXCP_ILLEGAL_INST;
> - }
> -
> - if (riscv_cpu_virt_enabled(env)) {
> - if (!(get_field(env->hcounteren, COUNTEREN_TM) &&
> - get_field(env->henvcfg, HENVCFG_STCE))) {
> - return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
> - }
> - }
> -
> - return RISCV_EXCP_NONE;
> -}
> -
> -static RISCVException sstc_32(CPURISCVState *env, int csrno)
> -{
> - if (riscv_cpu_mxl(env) != MXL_RV32) {
> - return RISCV_EXCP_ILLEGAL_INST;
> - }
> -
> - return sstc(env, csrno);
> -}
> -
> static RISCVException read_vstimecmp(CPURISCVState *env, int csrno,
> target_ulong *val)
> {
^ permalink raw reply [flat|nested] 57+ messages in thread
* [PATCH 18/18] target/riscv: Move configuration check to envcfg CSRs predicate()
2023-02-13 18:01 [PATCH 00/18] target/riscv: Various fixes to gdbstub and CSR access Bin Meng
` (17 preceding siblings ...)
2023-02-14 4:31 ` [PATCH 17/18] target/riscv: Group all predicate() routines together Bin Meng
@ 2023-02-14 14:27 ` Bin Meng
2023-02-14 14:59 ` weiwei
18 siblings, 1 reply; 57+ messages in thread
From: Bin Meng @ 2023-02-14 14:27 UTC (permalink / raw)
To: qemu-devel
Cc: Alistair Francis, Bin Meng, Daniel Henrique Barboza, Liu Zhiwei,
Palmer Dabbelt, Weiwei Li, qemu-riscv
At present the envcfg CSRs predicate() routines are generic one like
smode(), hmode. The configuration check is done in the read / write
routine. Create a new predicate routine to cover such check, so that
gdbstub can correctly report its existence.
Signed-off-by: Bin Meng <bmeng@tinylab.org>
---
target/riscv/csr.c | 98 +++++++++++++++++++++++++++++-----------------
1 file changed, 61 insertions(+), 37 deletions(-)
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 37350b8a6d..284ccc09dd 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -41,40 +41,6 @@ void riscv_set_csr_ops(int csrno, riscv_csr_operations *ops)
}
/* Predicates */
-#if !defined(CONFIG_USER_ONLY)
-static RISCVException smstateen_acc_ok(CPURISCVState *env, int index,
- uint64_t bit)
-{
- bool virt = riscv_cpu_virt_enabled(env);
- RISCVCPU *cpu = env_archcpu(env);
-
- if (env->priv == PRV_M || !cpu->cfg.ext_smstateen) {
- return RISCV_EXCP_NONE;
- }
-
- if (!(env->mstateen[index] & bit)) {
- return RISCV_EXCP_ILLEGAL_INST;
- }
-
- if (virt) {
- if (!(env->hstateen[index] & bit)) {
- return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
- }
-
- if (env->priv == PRV_U && !(env->sstateen[index] & bit)) {
- return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
- }
- }
-
- if (env->priv == PRV_U && riscv_has_ext(env, RVS)) {
- if (!(env->sstateen[index] & bit)) {
- return RISCV_EXCP_ILLEGAL_INST;
- }
- }
-
- return RISCV_EXCP_NONE;
-}
-#endif
static RISCVException fs(CPURISCVState *env, int csrno)
{
@@ -318,6 +284,32 @@ static RISCVException umode32(CPURISCVState *env, int csrno)
return umode(env, csrno);
}
+static RISCVException envcfg(CPURISCVState *env, int csrno)
+{
+ RISCVCPU *cpu = env_archcpu(env);
+ riscv_csr_predicate_fn predicate;
+
+ if (cpu->cfg.ext_smstateen) {
+ return RISCV_EXCP_ILLEGAL_INST;
+ }
+
+ switch (csrno) {
+ case CSR_SENVCFG:
+ predicate = smode;
+ break;
+ case CSR_HENVCFG:
+ predicate = hmode;
+ break;
+ case CSR_HENVCFGH:
+ predicate = hmode32;
+ break;
+ default:
+ g_assert_not_reached();
+ }
+
+ return predicate(env, csrno);
+}
+
static RISCVException mstateen(CPURISCVState *env, int csrno)
{
RISCVCPU *cpu = env_archcpu(env);
@@ -1946,6 +1938,38 @@ static RISCVException write_menvcfgh(CPURISCVState *env, int csrno,
return RISCV_EXCP_NONE;
}
+static RISCVException smstateen_acc_ok(CPURISCVState *env, int index,
+ uint64_t bit)
+{
+ bool virt = riscv_cpu_virt_enabled(env);
+
+ if (env->priv == PRV_M) {
+ return RISCV_EXCP_NONE;
+ }
+
+ if (!(env->mstateen[index] & bit)) {
+ return RISCV_EXCP_ILLEGAL_INST;
+ }
+
+ if (virt) {
+ if (!(env->hstateen[index] & bit)) {
+ return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
+ }
+
+ if (env->priv == PRV_U && !(env->sstateen[index] & bit)) {
+ return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
+ }
+ }
+
+ if (env->priv == PRV_U && riscv_has_ext(env, RVS)) {
+ if (!(env->sstateen[index] & bit)) {
+ return RISCV_EXCP_ILLEGAL_INST;
+ }
+ }
+
+ return RISCV_EXCP_NONE;
+}
+
static RISCVException read_senvcfg(CPURISCVState *env, int csrno,
target_ulong *val)
{
@@ -4087,11 +4111,11 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
.min_priv_ver = PRIV_VERSION_1_12_0 },
[CSR_MENVCFGH] = { "menvcfgh", umode32, read_menvcfgh, write_menvcfgh,
.min_priv_ver = PRIV_VERSION_1_12_0 },
- [CSR_SENVCFG] = { "senvcfg", smode, read_senvcfg, write_senvcfg,
+ [CSR_SENVCFG] = { "senvcfg", envcfg, read_senvcfg, write_senvcfg,
.min_priv_ver = PRIV_VERSION_1_12_0 },
- [CSR_HENVCFG] = { "henvcfg", hmode, read_henvcfg, write_henvcfg,
+ [CSR_HENVCFG] = { "henvcfg", envcfg, read_henvcfg, write_henvcfg,
.min_priv_ver = PRIV_VERSION_1_12_0 },
- [CSR_HENVCFGH] = { "henvcfgh", hmode32, read_henvcfgh, write_henvcfgh,
+ [CSR_HENVCFGH] = { "henvcfgh", envcfg, read_henvcfgh, write_henvcfgh,
.min_priv_ver = PRIV_VERSION_1_12_0 },
/* Smstateen extension CSRs */
--
2.25.1
^ permalink raw reply related [flat|nested] 57+ messages in thread
* Re: [PATCH 00/18] target/riscv: Various fixes to gdbstub and CSR access
2023-02-13 19:19 ` [PATCH 00/18] target/riscv: Various fixes to gdbstub and CSR access Daniel Henrique Barboza
@ 2023-02-14 14:31 ` Bin Meng
0 siblings, 0 replies; 57+ messages in thread
From: Bin Meng @ 2023-02-14 14:31 UTC (permalink / raw)
To: Daniel Henrique Barboza
Cc: Bin Meng, qemu-devel, Alistair Francis, Bin Meng, Liu Zhiwei,
Palmer Dabbelt, Weiwei Li, qemu-riscv
Hi Daniel,
On Tue, Feb 14, 2023 at 3:20 AM Daniel Henrique Barboza
<dbarboza@ventanamicro.com> wrote:
>
> Bin,
>
>
> I received only patches 1-11. I don't see the remaining patches in patchwork:
>
>
> https://patchwork.kernel.org/project/qemu-devel/list/?series=721372
>
>
> or in the qemu-devel archives:
>
>
> https://lists.gnu.org/archive/html/qemu-devel/2023-02/msg03461.html
>
>
> Can you please verify? Thanks,
>
Somehow my email service provider blocked some of my patches. I've now
resent the missing patches, and I just verified that it showed up in
the patchwork.
Regards,
Bin
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH 18/18] target/riscv: Move configuration check to envcfg CSRs predicate()
2023-02-14 14:27 ` [PATCH 18/18] target/riscv: Move configuration check to envcfg CSRs predicate() Bin Meng
@ 2023-02-14 14:59 ` weiwei
2023-02-15 2:22 ` Bin Meng
0 siblings, 1 reply; 57+ messages in thread
From: weiwei @ 2023-02-14 14:59 UTC (permalink / raw)
To: Bin Meng, qemu-devel
Cc: liweiwei, Alistair Francis, Bin Meng, Daniel Henrique Barboza,
Liu Zhiwei, Palmer Dabbelt, qemu-riscv
On 2023/2/14 22:27, Bin Meng wrote:
> At present the envcfg CSRs predicate() routines are generic one like
> smode(), hmode. The configuration check is done in the read / write
> routine. Create a new predicate routine to cover such check, so that
> gdbstub can correctly report its existence.
>
> Signed-off-by: Bin Meng <bmeng@tinylab.org>
>
> ---
>
> target/riscv/csr.c | 98 +++++++++++++++++++++++++++++-----------------
> 1 file changed, 61 insertions(+), 37 deletions(-)
>
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index 37350b8a6d..284ccc09dd 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -41,40 +41,6 @@ void riscv_set_csr_ops(int csrno, riscv_csr_operations *ops)
> }
>
> /* Predicates */
> -#if !defined(CONFIG_USER_ONLY)
> -static RISCVException smstateen_acc_ok(CPURISCVState *env, int index,
> - uint64_t bit)
> -{
> - bool virt = riscv_cpu_virt_enabled(env);
> - RISCVCPU *cpu = env_archcpu(env);
> -
> - if (env->priv == PRV_M || !cpu->cfg.ext_smstateen) {
> - return RISCV_EXCP_NONE;
> - }
> -
> - if (!(env->mstateen[index] & bit)) {
> - return RISCV_EXCP_ILLEGAL_INST;
> - }
> -
> - if (virt) {
> - if (!(env->hstateen[index] & bit)) {
> - return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
> - }
> -
> - if (env->priv == PRV_U && !(env->sstateen[index] & bit)) {
> - return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
> - }
> - }
> -
> - if (env->priv == PRV_U && riscv_has_ext(env, RVS)) {
> - if (!(env->sstateen[index] & bit)) {
> - return RISCV_EXCP_ILLEGAL_INST;
> - }
> - }
> -
> - return RISCV_EXCP_NONE;
> -}
> -#endif
>
> static RISCVException fs(CPURISCVState *env, int csrno)
> {
> @@ -318,6 +284,32 @@ static RISCVException umode32(CPURISCVState *env, int csrno)
> return umode(env, csrno);
> }
>
> +static RISCVException envcfg(CPURISCVState *env, int csrno)
> +{
> + RISCVCPU *cpu = env_archcpu(env);
> + riscv_csr_predicate_fn predicate;
> +
> + if (cpu->cfg.ext_smstateen) {
> + return RISCV_EXCP_ILLEGAL_INST;
> + }
This check seems not right here. Why ILLEGAL_INST is directly
triggered if smstateen is enabled?
It seems that smstateen related check will be done for
senvcfg/henvcfg{h} when smstateen is enabled.
Regards,
Weiwei Li
> +
> + switch (csrno) {
> + case CSR_SENVCFG:
> + predicate = smode;
> + break;
> + case CSR_HENVCFG:
> + predicate = hmode;
> + break;
> + case CSR_HENVCFGH:
> + predicate = hmode32;
> + break;
> + default:
> + g_assert_not_reached();
> + }
> +
> + return predicate(env, csrno);
> +}
> +
> static RISCVException mstateen(CPURISCVState *env, int csrno)
> {
> RISCVCPU *cpu = env_archcpu(env);
> @@ -1946,6 +1938,38 @@ static RISCVException write_menvcfgh(CPURISCVState *env, int csrno,
> return RISCV_EXCP_NONE;
> }
>
> +static RISCVException smstateen_acc_ok(CPURISCVState *env, int index,
> + uint64_t bit)
> +{
> + bool virt = riscv_cpu_virt_enabled(env);
> +
> + if (env->priv == PRV_M) {
> + return RISCV_EXCP_NONE;
> + }
> +
> + if (!(env->mstateen[index] & bit)) {
> + return RISCV_EXCP_ILLEGAL_INST;
> + }
> +
> + if (virt) {
> + if (!(env->hstateen[index] & bit)) {
> + return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
> + }
> +
> + if (env->priv == PRV_U && !(env->sstateen[index] & bit)) {
> + return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
> + }
> + }
> +
> + if (env->priv == PRV_U && riscv_has_ext(env, RVS)) {
> + if (!(env->sstateen[index] & bit)) {
> + return RISCV_EXCP_ILLEGAL_INST;
> + }
> + }
> +
> + return RISCV_EXCP_NONE;
> +}
> +
> static RISCVException read_senvcfg(CPURISCVState *env, int csrno,
> target_ulong *val)
> {
> @@ -4087,11 +4111,11 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
> .min_priv_ver = PRIV_VERSION_1_12_0 },
> [CSR_MENVCFGH] = { "menvcfgh", umode32, read_menvcfgh, write_menvcfgh,
> .min_priv_ver = PRIV_VERSION_1_12_0 },
> - [CSR_SENVCFG] = { "senvcfg", smode, read_senvcfg, write_senvcfg,
> + [CSR_SENVCFG] = { "senvcfg", envcfg, read_senvcfg, write_senvcfg,
> .min_priv_ver = PRIV_VERSION_1_12_0 },
> - [CSR_HENVCFG] = { "henvcfg", hmode, read_henvcfg, write_henvcfg,
> + [CSR_HENVCFG] = { "henvcfg", envcfg, read_henvcfg, write_henvcfg,
> .min_priv_ver = PRIV_VERSION_1_12_0 },
> - [CSR_HENVCFGH] = { "henvcfgh", hmode32, read_henvcfgh, write_henvcfgh,
> + [CSR_HENVCFGH] = { "henvcfgh", envcfg, read_henvcfgh, write_henvcfgh,
> .min_priv_ver = PRIV_VERSION_1_12_0 },
>
> /* Smstateen extension CSRs */
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH 18/18] target/riscv: Move configuration check to envcfg CSRs predicate()
2023-02-14 14:59 ` weiwei
@ 2023-02-15 2:22 ` Bin Meng
2023-02-15 2:57 ` weiwei
2023-02-16 16:40 ` Palmer Dabbelt
0 siblings, 2 replies; 57+ messages in thread
From: Bin Meng @ 2023-02-15 2:22 UTC (permalink / raw)
To: weiwei
Cc: qemu-devel, Alistair Francis, Bin Meng, Daniel Henrique Barboza,
Liu Zhiwei, Palmer Dabbelt, qemu-riscv
On Tue, Feb 14, 2023 at 10:59 PM weiwei <liweiwei@iscas.ac.cn> wrote:
>
>
> On 2023/2/14 22:27, Bin Meng wrote:
> > At present the envcfg CSRs predicate() routines are generic one like
> > smode(), hmode. The configuration check is done in the read / write
> > routine. Create a new predicate routine to cover such check, so that
> > gdbstub can correctly report its existence.
> >
> > Signed-off-by: Bin Meng <bmeng@tinylab.org>
> >
> > ---
> >
> > target/riscv/csr.c | 98 +++++++++++++++++++++++++++++-----------------
> > 1 file changed, 61 insertions(+), 37 deletions(-)
> >
> > diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> > index 37350b8a6d..284ccc09dd 100644
> > --- a/target/riscv/csr.c
> > +++ b/target/riscv/csr.c
> > @@ -41,40 +41,6 @@ void riscv_set_csr_ops(int csrno, riscv_csr_operations *ops)
> > }
> >
> > /* Predicates */
> > -#if !defined(CONFIG_USER_ONLY)
> > -static RISCVException smstateen_acc_ok(CPURISCVState *env, int index,
> > - uint64_t bit)
> > -{
> > - bool virt = riscv_cpu_virt_enabled(env);
> > - RISCVCPU *cpu = env_archcpu(env);
> > -
> > - if (env->priv == PRV_M || !cpu->cfg.ext_smstateen) {
> > - return RISCV_EXCP_NONE;
> > - }
> > -
> > - if (!(env->mstateen[index] & bit)) {
> > - return RISCV_EXCP_ILLEGAL_INST;
> > - }
> > -
> > - if (virt) {
> > - if (!(env->hstateen[index] & bit)) {
> > - return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
> > - }
> > -
> > - if (env->priv == PRV_U && !(env->sstateen[index] & bit)) {
> > - return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
> > - }
> > - }
> > -
> > - if (env->priv == PRV_U && riscv_has_ext(env, RVS)) {
> > - if (!(env->sstateen[index] & bit)) {
> > - return RISCV_EXCP_ILLEGAL_INST;
> > - }
> > - }
> > -
> > - return RISCV_EXCP_NONE;
> > -}
> > -#endif
> >
> > static RISCVException fs(CPURISCVState *env, int csrno)
> > {
> > @@ -318,6 +284,32 @@ static RISCVException umode32(CPURISCVState *env, int csrno)
> > return umode(env, csrno);
> > }
> >
> > +static RISCVException envcfg(CPURISCVState *env, int csrno)
> > +{
> > + RISCVCPU *cpu = env_archcpu(env);
> > + riscv_csr_predicate_fn predicate;
> > +
> > + if (cpu->cfg.ext_smstateen) {
> > + return RISCV_EXCP_ILLEGAL_INST;
> > + }
>
> This check seems not right here. Why ILLEGAL_INST is directly
> triggered if smstateen is enabled?
This logic was there in the original codes. I was confused when I
looked at this as well.
Anyway, if it is an issue, it should be a separate patch.
>
> It seems that smstateen related check will be done for
> senvcfg/henvcfg{h} when smstateen is enabled.
>
Regards,
Bin
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH 18/18] target/riscv: Move configuration check to envcfg CSRs predicate()
2023-02-15 2:22 ` Bin Meng
@ 2023-02-15 2:57 ` weiwei
2023-02-16 16:40 ` Palmer Dabbelt
1 sibling, 0 replies; 57+ messages in thread
From: weiwei @ 2023-02-15 2:57 UTC (permalink / raw)
To: Bin Meng
Cc: liweiwei, qemu-devel, Alistair Francis, Bin Meng,
Daniel Henrique Barboza, Liu Zhiwei, Palmer Dabbelt, qemu-riscv
On 2023/2/15 10:22, Bin Meng wrote:
> On Tue, Feb 14, 2023 at 10:59 PM weiwei <liweiwei@iscas.ac.cn> wrote:
>>
>> On 2023/2/14 22:27, Bin Meng wrote:
>>> At present the envcfg CSRs predicate() routines are generic one like
>>> smode(), hmode. The configuration check is done in the read / write
>>> routine. Create a new predicate routine to cover such check, so that
>>> gdbstub can correctly report its existence.
>>>
>>> Signed-off-by: Bin Meng <bmeng@tinylab.org>
>>>
>>> ---
>>>
>>> target/riscv/csr.c | 98 +++++++++++++++++++++++++++++-----------------
>>> 1 file changed, 61 insertions(+), 37 deletions(-)
>>>
>>> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
>>> index 37350b8a6d..284ccc09dd 100644
>>> --- a/target/riscv/csr.c
>>> +++ b/target/riscv/csr.c
>>> @@ -41,40 +41,6 @@ void riscv_set_csr_ops(int csrno, riscv_csr_operations *ops)
>>> }
>>>
>>> /* Predicates */
>>> -#if !defined(CONFIG_USER_ONLY)
>>> -static RISCVException smstateen_acc_ok(CPURISCVState *env, int index,
>>> - uint64_t bit)
>>> -{
>>> - bool virt = riscv_cpu_virt_enabled(env);
>>> - RISCVCPU *cpu = env_archcpu(env);
>>> -
>>> - if (env->priv == PRV_M || !cpu->cfg.ext_smstateen) {
>>> - return RISCV_EXCP_NONE;
>>> - }
>>> -
>>> - if (!(env->mstateen[index] & bit)) {
>>> - return RISCV_EXCP_ILLEGAL_INST;
>>> - }
>>> -
>>> - if (virt) {
>>> - if (!(env->hstateen[index] & bit)) {
>>> - return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
>>> - }
>>> -
>>> - if (env->priv == PRV_U && !(env->sstateen[index] & bit)) {
>>> - return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
>>> - }
>>> - }
>>> -
>>> - if (env->priv == PRV_U && riscv_has_ext(env, RVS)) {
>>> - if (!(env->sstateen[index] & bit)) {
>>> - return RISCV_EXCP_ILLEGAL_INST;
>>> - }
>>> - }
>>> -
>>> - return RISCV_EXCP_NONE;
>>> -}
>>> -#endif
>>>
>>> static RISCVException fs(CPURISCVState *env, int csrno)
>>> {
>>> @@ -318,6 +284,32 @@ static RISCVException umode32(CPURISCVState *env, int csrno)
>>> return umode(env, csrno);
>>> }
>>>
>>> +static RISCVException envcfg(CPURISCVState *env, int csrno)
>>> +{
>>> + RISCVCPU *cpu = env_archcpu(env);
>>> + riscv_csr_predicate_fn predicate;
>>> +
>>> + if (cpu->cfg.ext_smstateen) {
>>> + return RISCV_EXCP_ILLEGAL_INST;
>>> + }
>> This check seems not right here. Why ILLEGAL_INST is directly
>> triggered if smstateen is enabled?
> This logic was there in the original codes. I was confused when I
> looked at this as well.
Sorry, I didn't find the original codes. Do you mean this:
if (env->priv == PRV_M || !cpu->cfg.ext_smstateen) {
return RISCV_EXCP_NONE;
}
If so, I think the check here is to make the following *stateen related
check ignored when smstateen extension is disabled.
Regards,
Weiwei Li
> Anyway, if it is an issue, it should be a separate patch.
>
>> It seems that smstateen related check will be done for
>> senvcfg/henvcfg{h} when smstateen is enabled.
>>
> Regards,
> Bin
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH 18/18] target/riscv: Move configuration check to envcfg CSRs predicate()
2023-02-15 2:22 ` Bin Meng
2023-02-15 2:57 ` weiwei
@ 2023-02-16 16:40 ` Palmer Dabbelt
2023-02-17 1:59 ` Bin Meng
1 sibling, 1 reply; 57+ messages in thread
From: Palmer Dabbelt @ 2023-02-16 16:40 UTC (permalink / raw)
To: Bin Meng
Cc: liweiwei, qemu-devel, Alistair Francis, bin.meng, dbarboza,
zhiwei_liu, qemu-riscv
On Tue, 14 Feb 2023 18:22:21 PST (-0800), Bin Meng wrote:
> On Tue, Feb 14, 2023 at 10:59 PM weiwei <liweiwei@iscas.ac.cn> wrote:
>>
>>
>> On 2023/2/14 22:27, Bin Meng wrote:
>> > At present the envcfg CSRs predicate() routines are generic one like
>> > smode(), hmode. The configuration check is done in the read / write
>> > routine. Create a new predicate routine to cover such check, so that
>> > gdbstub can correctly report its existence.
>> >
>> > Signed-off-by: Bin Meng <bmeng@tinylab.org>
>> >
>> > ---
>> >
>> > target/riscv/csr.c | 98 +++++++++++++++++++++++++++++-----------------
>> > 1 file changed, 61 insertions(+), 37 deletions(-)
>> >
>> > diff --git a/target/riscv/csr.c b/target/riscv/csr.c
>> > index 37350b8a6d..284ccc09dd 100644
>> > --- a/target/riscv/csr.c
>> > +++ b/target/riscv/csr.c
>> > @@ -41,40 +41,6 @@ void riscv_set_csr_ops(int csrno, riscv_csr_operations *ops)
>> > }
>> >
>> > /* Predicates */
>> > -#if !defined(CONFIG_USER_ONLY)
>> > -static RISCVException smstateen_acc_ok(CPURISCVState *env, int index,
>> > - uint64_t bit)
>> > -{
>> > - bool virt = riscv_cpu_virt_enabled(env);
>> > - RISCVCPU *cpu = env_archcpu(env);
>> > -
>> > - if (env->priv == PRV_M || !cpu->cfg.ext_smstateen) {
>> > - return RISCV_EXCP_NONE;
>> > - }
>> > -
>> > - if (!(env->mstateen[index] & bit)) {
>> > - return RISCV_EXCP_ILLEGAL_INST;
>> > - }
>> > -
>> > - if (virt) {
>> > - if (!(env->hstateen[index] & bit)) {
>> > - return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
>> > - }
>> > -
>> > - if (env->priv == PRV_U && !(env->sstateen[index] & bit)) {
>> > - return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
>> > - }
>> > - }
>> > -
>> > - if (env->priv == PRV_U && riscv_has_ext(env, RVS)) {
>> > - if (!(env->sstateen[index] & bit)) {
>> > - return RISCV_EXCP_ILLEGAL_INST;
>> > - }
>> > - }
>> > -
>> > - return RISCV_EXCP_NONE;
>> > -}
>> > -#endif
>> >
>> > static RISCVException fs(CPURISCVState *env, int csrno)
>> > {
>> > @@ -318,6 +284,32 @@ static RISCVException umode32(CPURISCVState *env, int csrno)
>> > return umode(env, csrno);
>> > }
>> >
>> > +static RISCVException envcfg(CPURISCVState *env, int csrno)
>> > +{
>> > + RISCVCPU *cpu = env_archcpu(env);
>> > + riscv_csr_predicate_fn predicate;
>> > +
>> > + if (cpu->cfg.ext_smstateen) {
>> > + return RISCV_EXCP_ILLEGAL_INST;
>> > + }
>>
>> This check seems not right here. Why ILLEGAL_INST is directly
>> triggered if smstateen is enabled?
>
> This logic was there in the original codes. I was confused when I
> looked at this as well.
>
> Anyway, if it is an issue, it should be a separate patch.
Seems reasonable to me, it's always nice to split up the refactoring types. So
I queued this up as 4ac6c32224 ("Merge patch series "target/riscv: Various
fixes to gdbstub and CSR access"").
I had to fix up the From address on the patch you re-sent and there was a minor
merge conflict, but otherwise things look sane to me. I'll hold off on sending
anything for a bit just in case, though.
Thanks!
>
>>
>> It seems that smstateen related check will be done for
>> senvcfg/henvcfg{h} when smstateen is enabled.
>>
>
> Regards,
> Bin
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH 18/18] target/riscv: Move configuration check to envcfg CSRs predicate()
2023-02-16 16:40 ` Palmer Dabbelt
@ 2023-02-17 1:59 ` Bin Meng
2023-02-17 17:28 ` Palmer Dabbelt
0 siblings, 1 reply; 57+ messages in thread
From: Bin Meng @ 2023-02-17 1:59 UTC (permalink / raw)
To: Palmer Dabbelt
Cc: liweiwei, qemu-devel, Alistair Francis, bin.meng, dbarboza,
zhiwei_liu, qemu-riscv
Hi Palmer,
On Fri, Feb 17, 2023 at 12:40 AM Palmer Dabbelt <palmer@dabbelt.com> wrote:
>
> On Tue, 14 Feb 2023 18:22:21 PST (-0800), Bin Meng wrote:
> > On Tue, Feb 14, 2023 at 10:59 PM weiwei <liweiwei@iscas.ac.cn> wrote:
> >>
> >>
> >> On 2023/2/14 22:27, Bin Meng wrote:
> >> > At present the envcfg CSRs predicate() routines are generic one like
> >> > smode(), hmode. The configuration check is done in the read / write
> >> > routine. Create a new predicate routine to cover such check, so that
> >> > gdbstub can correctly report its existence.
> >> >
> >> > Signed-off-by: Bin Meng <bmeng@tinylab.org>
> >> >
> >> > ---
> >> >
> >> > target/riscv/csr.c | 98 +++++++++++++++++++++++++++++-----------------
> >> > 1 file changed, 61 insertions(+), 37 deletions(-)
> >> >
> >> > diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> >> > index 37350b8a6d..284ccc09dd 100644
> >> > --- a/target/riscv/csr.c
> >> > +++ b/target/riscv/csr.c
> >> > @@ -41,40 +41,6 @@ void riscv_set_csr_ops(int csrno, riscv_csr_operations *ops)
> >> > }
> >> >
> >> > /* Predicates */
> >> > -#if !defined(CONFIG_USER_ONLY)
> >> > -static RISCVException smstateen_acc_ok(CPURISCVState *env, int index,
> >> > - uint64_t bit)
> >> > -{
> >> > - bool virt = riscv_cpu_virt_enabled(env);
> >> > - RISCVCPU *cpu = env_archcpu(env);
> >> > -
> >> > - if (env->priv == PRV_M || !cpu->cfg.ext_smstateen) {
> >> > - return RISCV_EXCP_NONE;
> >> > - }
> >> > -
> >> > - if (!(env->mstateen[index] & bit)) {
> >> > - return RISCV_EXCP_ILLEGAL_INST;
> >> > - }
> >> > -
> >> > - if (virt) {
> >> > - if (!(env->hstateen[index] & bit)) {
> >> > - return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
> >> > - }
> >> > -
> >> > - if (env->priv == PRV_U && !(env->sstateen[index] & bit)) {
> >> > - return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
> >> > - }
> >> > - }
> >> > -
> >> > - if (env->priv == PRV_U && riscv_has_ext(env, RVS)) {
> >> > - if (!(env->sstateen[index] & bit)) {
> >> > - return RISCV_EXCP_ILLEGAL_INST;
> >> > - }
> >> > - }
> >> > -
> >> > - return RISCV_EXCP_NONE;
> >> > -}
> >> > -#endif
> >> >
> >> > static RISCVException fs(CPURISCVState *env, int csrno)
> >> > {
> >> > @@ -318,6 +284,32 @@ static RISCVException umode32(CPURISCVState *env, int csrno)
> >> > return umode(env, csrno);
> >> > }
> >> >
> >> > +static RISCVException envcfg(CPURISCVState *env, int csrno)
> >> > +{
> >> > + RISCVCPU *cpu = env_archcpu(env);
> >> > + riscv_csr_predicate_fn predicate;
> >> > +
> >> > + if (cpu->cfg.ext_smstateen) {
> >> > + return RISCV_EXCP_ILLEGAL_INST;
> >> > + }
> >>
> >> This check seems not right here. Why ILLEGAL_INST is directly
> >> triggered if smstateen is enabled?
> >
> > This logic was there in the original codes. I was confused when I
> > looked at this as well.
> >
> > Anyway, if it is an issue, it should be a separate patch.
>
> Seems reasonable to me, it's always nice to split up the refactoring types. So
> I queued this up as 4ac6c32224 ("Merge patch series "target/riscv: Various
> fixes to gdbstub and CSR access"").
>
> I had to fix up the From address on the patch you re-sent and there was a minor
> merge conflict, but otherwise things look sane to me. I'll hold off on sending
> anything for a bit just in case, though.
>
There are some open comments in this series I need to address. Please
drop this v1. I will send v2 soon.
Regards,
Bin
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH 01/18] target/riscv: gdbstub: Check priv spec version before reporting CSR
2023-02-13 18:01 ` [PATCH 01/18] target/riscv: gdbstub: Check priv spec version before reporting CSR Bin Meng
2023-02-14 8:40 ` weiwei
@ 2023-02-17 2:11 ` LIU Zhiwei
1 sibling, 0 replies; 57+ messages in thread
From: LIU Zhiwei @ 2023-02-17 2:11 UTC (permalink / raw)
To: Bin Meng, qemu-devel
Cc: Alistair Francis, Bin Meng, Daniel Henrique Barboza, Liu Zhiwei,
Palmer Dabbelt, Weiwei Li, qemu-riscv
On 2023/2/14 2:01, Bin Meng wrote:
> The gdbstub CSR XML is dynamically generated according to the result
> of the CSR predicate() result. This has been working fine until
> commit 7100fe6c2441 ("target/riscv: Enable privileged spec version 1.12")
> introduced the privilege spec version check in riscv_csrrw_check().
>
> When debugging the 'sifive_u' machine whose priv spec is at 1.10,
> gdbstub reports priv spec 1.12 CSRs like menvcfg in the XML, hence
> we see "remote failure reply 'E14'" message when examining all CSRs
> via "info register system" from gdb.
>
> Add the priv spec version check in the CSR XML generation logic to
> fix this issue.
>
> Fixes: 7100fe6c2441 ("target/riscv: Enable privileged spec version 1.12")
> Signed-off-by: Bin Meng <bmeng@tinylab.org>
> ---
>
> target/riscv/gdbstub.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/target/riscv/gdbstub.c b/target/riscv/gdbstub.c
> index 6e7bbdbd5e..e57372db38 100644
> --- a/target/riscv/gdbstub.c
> +++ b/target/riscv/gdbstub.c
> @@ -290,6 +290,9 @@ static int riscv_gen_dynamic_csr_xml(CPUState *cs, int base_reg)
> g_string_append_printf(s, "<feature name=\"org.gnu.gdb.riscv.csr\">");
>
> for (i = 0; i < CSR_TABLE_SIZE; i++) {
> + if (env->priv_ver < csr_ops[i].min_priv_ver) {
> + continue;
> + }
Reviewed-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>
Zhiwei
> predicate = csr_ops[i].predicate;
> if (predicate && (predicate(env, i) == RISCV_EXCP_NONE)) {
> if (csr_ops[i].name) {
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH 02/18] target/riscv: Correct the priority policy of riscv_csrrw_check()
2023-02-13 18:01 ` [PATCH 02/18] target/riscv: Correct the priority policy of riscv_csrrw_check() Bin Meng
2023-02-14 8:43 ` weiwei
@ 2023-02-17 2:15 ` LIU Zhiwei
1 sibling, 0 replies; 57+ messages in thread
From: LIU Zhiwei @ 2023-02-17 2:15 UTC (permalink / raw)
To: Bin Meng, qemu-devel
Cc: Alistair Francis, Bin Meng, Daniel Henrique Barboza, Liu Zhiwei,
Palmer Dabbelt, Weiwei Li, qemu-riscv
On 2023/2/14 2:01, Bin Meng wrote:
> 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 came
> after the read / write check.
>
> Fixes: eacaf4401956 ("target/riscv: Fix priority of csr related check in riscv_csrrw_check")
> Signed-off-by: Bin Meng <bmeng@tinylab.org>
> ---
>
> target/riscv/csr.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index 1b0a0c1693..c2dd9d5af0 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -3793,15 +3793,15 @@ static inline RISCVException riscv_csrrw_check(CPURISCVState *env,
> return RISCV_EXCP_ILLEGAL_INST;
> }
>
> - if (write_mask && read_only) {
> - return RISCV_EXCP_ILLEGAL_INST;
> - }
> -
> RISCVException ret = csr_ops[csrno].predicate(env, csrno);
> if (ret != RISCV_EXCP_NONE) {
> return ret;
> }
>
> + if (write_mask && read_only) {
> + return RISCV_EXCP_ILLEGAL_INST;
> + }
> +
Reviewed-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>
Zhiwei
> #if !defined(CONFIG_USER_ONLY)
> int csr_priv, effective_priv = env->priv;
>
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH 03/18] target/riscv: gdbstub: Minor change for better readability
2023-02-13 18:01 ` [PATCH 03/18] target/riscv: gdbstub: Minor change for better readability Bin Meng
2023-02-14 8:45 ` weiwei
@ 2023-02-17 2:20 ` LIU Zhiwei
1 sibling, 0 replies; 57+ messages in thread
From: LIU Zhiwei @ 2023-02-17 2:20 UTC (permalink / raw)
To: Bin Meng, qemu-devel
Cc: Alistair Francis, Bin Meng, Daniel Henrique Barboza,
Palmer Dabbelt, Weiwei Li, qemu-riscv
On 2023/2/14 2:01, Bin Meng wrote:
> Use a variable 'base_reg' to represent cs->gdb_num_regs so that
> the call to ricsv_gen_dynamic_vector_xml() can be placed in one
> single line for better readability.
>
> Signed-off-by: Bin Meng <bmeng@tinylab.org>
> ---
>
> target/riscv/gdbstub.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/target/riscv/gdbstub.c b/target/riscv/gdbstub.c
> index e57372db38..704f3d6922 100644
> --- a/target/riscv/gdbstub.c
> +++ b/target/riscv/gdbstub.c
> @@ -385,9 +385,9 @@ void riscv_cpu_register_gdb_regs_for_features(CPUState *cs)
> 32, "riscv-32bit-fpu.xml", 0);
> }
> if (env->misa_ext & RVV) {
> + int base_reg = cs->gdb_num_regs;
> gdb_register_coprocessor(cs, riscv_gdb_get_vector, riscv_gdb_set_vector,
> - ricsv_gen_dynamic_vector_xml(cs,
> - cs->gdb_num_regs),
> + ricsv_gen_dynamic_vector_xml(cs, base_reg),
Reviewed-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>
Zhiwei
> "riscv-vector.xml", 0);
> }
> switch (env->misa_mxl_max) {
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH 04/18] target/riscv: gdbstub: Do not generate CSR XML if Zicsr is disabled
2023-02-13 18:02 ` [PATCH 04/18] target/riscv: gdbstub: Do not generate CSR XML if Zicsr is disabled Bin Meng
2023-02-14 8:46 ` weiwei
@ 2023-02-17 2:23 ` LIU Zhiwei
1 sibling, 0 replies; 57+ messages in thread
From: LIU Zhiwei @ 2023-02-17 2:23 UTC (permalink / raw)
To: Bin Meng, qemu-devel
Cc: Alistair Francis, Bin Meng, Daniel Henrique Barboza,
Palmer Dabbelt, Weiwei Li, qemu-riscv
On 2023/2/14 2:02, Bin Meng wrote:
> There is no need to generate the CSR XML if the Zicsr extension
> is not enabled.
>
> Signed-off-by: Bin Meng <bmeng@tinylab.org>
> ---
>
> target/riscv/gdbstub.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/target/riscv/gdbstub.c b/target/riscv/gdbstub.c
> index 704f3d6922..294f0ceb1c 100644
> --- a/target/riscv/gdbstub.c
> +++ b/target/riscv/gdbstub.c
> @@ -406,7 +406,10 @@ void riscv_cpu_register_gdb_regs_for_features(CPUState *cs)
> g_assert_not_reached();
> }
>
> - gdb_register_coprocessor(cs, riscv_gdb_get_csr, riscv_gdb_set_csr,
> - riscv_gen_dynamic_csr_xml(cs, cs->gdb_num_regs),
> - "riscv-csr.xml", 0);
> + if (cpu->cfg.ext_icsr) {
> + int base_reg = cs->gdb_num_regs;
> + gdb_register_coprocessor(cs, riscv_gdb_get_csr, riscv_gdb_set_csr,
> + riscv_gen_dynamic_csr_xml(cs, base_reg),
> + "riscv-csr.xml", 0);
> + }
Reviewed-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>
Zhiwei
> }
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH 05/18] target/riscv: Coding style fixes in csr.c
2023-02-13 18:02 ` [PATCH 05/18] target/riscv: Coding style fixes in csr.c Bin Meng
2023-02-14 8:48 ` weiwei
@ 2023-02-17 2:24 ` LIU Zhiwei
1 sibling, 0 replies; 57+ messages in thread
From: LIU Zhiwei @ 2023-02-17 2:24 UTC (permalink / raw)
To: Bin Meng, qemu-devel
Cc: Alistair Francis, Bin Meng, Daniel Henrique Barboza,
Palmer Dabbelt, Weiwei Li, qemu-riscv
On 2023/2/14 2:02, Bin Meng wrote:
> Fix various places that violate QEMU coding style:
>
> - correct multi-line comment format
> - indent to opening parenthesis
>
> Signed-off-by: Bin Meng <bmeng@tinylab.org>
> ---
>
> target/riscv/csr.c | 62 ++++++++++++++++++++++++----------------------
> 1 file changed, 32 insertions(+), 30 deletions(-)
>
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index c2dd9d5af0..cc74819759 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -963,7 +963,7 @@ static RISCVException sstc_32(CPURISCVState *env, int csrno)
> }
>
> static RISCVException read_vstimecmp(CPURISCVState *env, int csrno,
> - target_ulong *val)
> + target_ulong *val)
> {
> *val = env->vstimecmp;
>
> @@ -971,7 +971,7 @@ static RISCVException read_vstimecmp(CPURISCVState *env, int csrno,
> }
>
> static RISCVException read_vstimecmph(CPURISCVState *env, int csrno,
> - target_ulong *val)
> + target_ulong *val)
> {
> *val = env->vstimecmp >> 32;
>
> @@ -979,7 +979,7 @@ static RISCVException read_vstimecmph(CPURISCVState *env, int csrno,
> }
>
> static RISCVException write_vstimecmp(CPURISCVState *env, int csrno,
> - target_ulong val)
> + target_ulong val)
> {
> RISCVCPU *cpu = env_archcpu(env);
>
> @@ -996,7 +996,7 @@ static RISCVException write_vstimecmp(CPURISCVState *env, int csrno,
> }
>
> static RISCVException write_vstimecmph(CPURISCVState *env, int csrno,
> - target_ulong val)
> + target_ulong val)
> {
> RISCVCPU *cpu = env_archcpu(env);
>
> @@ -1020,7 +1020,7 @@ static RISCVException read_stimecmp(CPURISCVState *env, int csrno,
> }
>
> static RISCVException read_stimecmph(CPURISCVState *env, int csrno,
> - target_ulong *val)
> + target_ulong *val)
> {
> if (riscv_cpu_virt_enabled(env)) {
> *val = env->vstimecmp >> 32;
> @@ -1032,7 +1032,7 @@ static RISCVException read_stimecmph(CPURISCVState *env, int csrno,
> }
>
> static RISCVException write_stimecmp(CPURISCVState *env, int csrno,
> - target_ulong val)
> + target_ulong val)
> {
> RISCVCPU *cpu = env_archcpu(env);
>
> @@ -1055,7 +1055,7 @@ static RISCVException write_stimecmp(CPURISCVState *env, int csrno,
> }
>
> static RISCVException write_stimecmph(CPURISCVState *env, int csrno,
> - target_ulong val)
> + target_ulong val)
> {
> RISCVCPU *cpu = env_archcpu(env);
>
> @@ -1342,7 +1342,8 @@ static RISCVException write_misa(CPURISCVState *env, int csrno,
>
> /* 'E' excludes all other extensions */
> if (val & RVE) {
> - /* when we support 'E' we can do "val = RVE;" however
> + /*
> + * when we support 'E' we can do "val = RVE;" however
> * for now we just drop writes if 'E' is present.
> */
> return RISCV_EXCP_NONE;
> @@ -1364,7 +1365,8 @@ static RISCVException write_misa(CPURISCVState *env, int csrno,
> val &= ~RVD;
> }
>
> - /* Suppress 'C' if next instruction is not aligned
> + /*
> + * Suppress 'C' if next instruction is not aligned
> * TODO: this should check next_pc
> */
> if ((val & RVC) && (GETPC() & ~3) != 0) {
> @@ -1833,28 +1835,28 @@ static RISCVException write_mscratch(CPURISCVState *env, int csrno,
> }
>
> static RISCVException read_mepc(CPURISCVState *env, int csrno,
> - target_ulong *val)
> + target_ulong *val)
> {
> *val = env->mepc;
> return RISCV_EXCP_NONE;
> }
>
> static RISCVException write_mepc(CPURISCVState *env, int csrno,
> - target_ulong val)
> + target_ulong val)
> {
> env->mepc = val;
> return RISCV_EXCP_NONE;
> }
>
> static RISCVException read_mcause(CPURISCVState *env, int csrno,
> - target_ulong *val)
> + target_ulong *val)
> {
> *val = env->mcause;
> return RISCV_EXCP_NONE;
> }
>
> static RISCVException write_mcause(CPURISCVState *env, int csrno,
> - target_ulong val)
> + target_ulong val)
> {
> env->mcause = val;
> return RISCV_EXCP_NONE;
> @@ -1876,14 +1878,14 @@ static RISCVException write_mtval(CPURISCVState *env, int csrno,
>
> /* Execution environment configuration setup */
> static RISCVException read_menvcfg(CPURISCVState *env, int csrno,
> - target_ulong *val)
> + target_ulong *val)
> {
> *val = env->menvcfg;
> return RISCV_EXCP_NONE;
> }
>
> static RISCVException write_menvcfg(CPURISCVState *env, int csrno,
> - target_ulong val)
> + target_ulong val)
> {
> uint64_t mask = MENVCFG_FIOM | MENVCFG_CBIE | MENVCFG_CBCFE | MENVCFG_CBZE;
>
> @@ -1896,14 +1898,14 @@ static RISCVException write_menvcfg(CPURISCVState *env, int csrno,
> }
>
> static RISCVException read_menvcfgh(CPURISCVState *env, int csrno,
> - target_ulong *val)
> + target_ulong *val)
> {
> *val = env->menvcfg >> 32;
> return RISCV_EXCP_NONE;
> }
>
> static RISCVException write_menvcfgh(CPURISCVState *env, int csrno,
> - target_ulong val)
> + target_ulong val)
> {
> uint64_t mask = MENVCFG_PBMTE | MENVCFG_STCE;
> uint64_t valh = (uint64_t)val << 32;
> @@ -1914,7 +1916,7 @@ static RISCVException write_menvcfgh(CPURISCVState *env, int csrno,
> }
>
> static RISCVException read_senvcfg(CPURISCVState *env, int csrno,
> - target_ulong *val)
> + target_ulong *val)
> {
> RISCVException ret;
>
> @@ -1928,7 +1930,7 @@ static RISCVException read_senvcfg(CPURISCVState *env, int csrno,
> }
>
> static RISCVException write_senvcfg(CPURISCVState *env, int csrno,
> - target_ulong val)
> + target_ulong val)
> {
> uint64_t mask = SENVCFG_FIOM | SENVCFG_CBIE | SENVCFG_CBCFE | SENVCFG_CBZE;
> RISCVException ret;
> @@ -1943,7 +1945,7 @@ static RISCVException write_senvcfg(CPURISCVState *env, int csrno,
> }
>
> static RISCVException read_henvcfg(CPURISCVState *env, int csrno,
> - target_ulong *val)
> + target_ulong *val)
> {
> RISCVException ret;
>
> @@ -1957,7 +1959,7 @@ static RISCVException read_henvcfg(CPURISCVState *env, int csrno,
> }
>
> static RISCVException write_henvcfg(CPURISCVState *env, int csrno,
> - target_ulong val)
> + target_ulong val)
> {
> uint64_t mask = HENVCFG_FIOM | HENVCFG_CBIE | HENVCFG_CBCFE | HENVCFG_CBZE;
> RISCVException ret;
> @@ -1977,7 +1979,7 @@ static RISCVException write_henvcfg(CPURISCVState *env, int csrno,
> }
>
> static RISCVException read_henvcfgh(CPURISCVState *env, int csrno,
> - target_ulong *val)
> + target_ulong *val)
> {
> RISCVException ret;
>
> @@ -1991,7 +1993,7 @@ static RISCVException read_henvcfgh(CPURISCVState *env, int csrno,
> }
>
> static RISCVException write_henvcfgh(CPURISCVState *env, int csrno,
> - target_ulong val)
> + target_ulong val)
> {
> uint64_t mask = HENVCFG_PBMTE | HENVCFG_STCE;
> uint64_t valh = (uint64_t)val << 32;
> @@ -2034,13 +2036,13 @@ static RISCVException write_mstateen0(CPURISCVState *env, int csrno,
> }
>
> static RISCVException write_mstateen_1_3(CPURISCVState *env, int csrno,
> - target_ulong new_val)
> + target_ulong new_val)
> {
> return write_mstateen(env, csrno, SMSTATEEN_STATEEN, new_val);
> }
>
> static RISCVException read_mstateenh(CPURISCVState *env, int csrno,
> - target_ulong *val)
> + target_ulong *val)
> {
> *val = env->mstateen[csrno - CSR_MSTATEEN0H] >> 32;
>
> @@ -2061,7 +2063,7 @@ static RISCVException write_mstateenh(CPURISCVState *env, int csrno,
> }
>
> static RISCVException write_mstateen0h(CPURISCVState *env, int csrno,
> - target_ulong new_val)
> + target_ulong new_val)
> {
> uint64_t wr_mask = SMSTATEEN_STATEEN | SMSTATEEN0_HSENVCFG;
>
> @@ -2069,7 +2071,7 @@ static RISCVException write_mstateen0h(CPURISCVState *env, int csrno,
> }
>
> static RISCVException write_mstateenh_1_3(CPURISCVState *env, int csrno,
> - target_ulong new_val)
> + target_ulong new_val)
> {
> return write_mstateenh(env, csrno, SMSTATEEN_STATEEN, new_val);
> }
> @@ -2106,7 +2108,7 @@ static RISCVException write_hstateen0(CPURISCVState *env, int csrno,
> }
>
> static RISCVException write_hstateen_1_3(CPURISCVState *env, int csrno,
> - target_ulong new_val)
> + target_ulong new_val)
> {
> return write_hstateen(env, csrno, SMSTATEEN_STATEEN, new_val);
> }
> @@ -2145,7 +2147,7 @@ static RISCVException write_hstateen0h(CPURISCVState *env, int csrno,
> }
>
> static RISCVException write_hstateenh_1_3(CPURISCVState *env, int csrno,
> - target_ulong new_val)
> + target_ulong new_val)
> {
> return write_hstateenh(env, csrno, SMSTATEEN_STATEEN, new_val);
> }
> @@ -3338,7 +3340,7 @@ static RISCVException read_mseccfg(CPURISCVState *env, int csrno,
> }
>
> static RISCVException write_mseccfg(CPURISCVState *env, int csrno,
> - target_ulong val)
> + target_ulong val)
Reviewed-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>
Zhiwei
> {
> mseccfg_csr_write(env, val);
> return RISCV_EXCP_NONE;
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH 06/18] target/riscv: Use 'bool' type for read_only
2023-02-13 18:02 ` [PATCH 06/18] target/riscv: Use 'bool' type for read_only Bin Meng
2023-02-14 8:48 ` weiwei
@ 2023-02-17 2:24 ` LIU Zhiwei
1 sibling, 0 replies; 57+ messages in thread
From: LIU Zhiwei @ 2023-02-17 2:24 UTC (permalink / raw)
To: Bin Meng, qemu-devel
Cc: Alistair Francis, Bin Meng, Daniel Henrique Barboza,
Palmer Dabbelt, Weiwei Li, qemu-riscv
On 2023/2/14 2:02, Bin Meng wrote:
> The read_only variable is currently declared as an 'int', but it
> should really be a 'bool'.
>
> Signed-off-by: Bin Meng <bmeng@tinylab.org>
> ---
>
> target/riscv/csr.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index cc74819759..8bbc75cbfa 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -3778,7 +3778,7 @@ static inline RISCVException riscv_csrrw_check(CPURISCVState *env,
> RISCVCPU *cpu)
> {
> /* check privileges and return RISCV_EXCP_ILLEGAL_INST if check fails */
> - int read_only = get_field(csrno, 0xC00) == 3;
> + bool read_only = get_field(csrno, 0xC00) == 3;
Reviewed-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>
Zhiwei
> int csr_min_priv = csr_ops[csrno].min_priv_ver;
>
> /* ensure the CSR extension is enabled. */
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH 07/18] target/riscv: Simplify {read,write}_pmpcfg() a little bit
2023-02-13 18:02 ` [PATCH 07/18] target/riscv: Simplify {read, write}_pmpcfg() a little bit Bin Meng
2023-02-14 8:50 ` [PATCH 07/18] target/riscv: Simplify {read,write}_pmpcfg() " weiwei
@ 2023-02-17 2:26 ` LIU Zhiwei
1 sibling, 0 replies; 57+ messages in thread
From: LIU Zhiwei @ 2023-02-17 2:26 UTC (permalink / raw)
To: Bin Meng, qemu-devel
Cc: Alistair Francis, Bin Meng, Daniel Henrique Barboza,
Palmer Dabbelt, Weiwei Li, qemu-riscv
On 2023/2/14 2:02, Bin Meng wrote:
> Use the register index that has already been calculated in the
> pmpcfg_csr_{read,write} call.
>
> Signed-off-by: Bin Meng <bmeng@tinylab.org>
> ---
>
> target/riscv/csr.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index 8bbc75cbfa..da3b770894 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -3363,7 +3363,7 @@ static RISCVException read_pmpcfg(CPURISCVState *env, int csrno,
> if (!check_pmp_reg_index(env, reg_index)) {
> return RISCV_EXCP_ILLEGAL_INST;
> }
> - *val = pmpcfg_csr_read(env, csrno - CSR_PMPCFG0);
> + *val = pmpcfg_csr_read(env, reg_index);
> return RISCV_EXCP_NONE;
> }
>
> @@ -3375,7 +3375,7 @@ static RISCVException write_pmpcfg(CPURISCVState *env, int csrno,
> if (!check_pmp_reg_index(env, reg_index)) {
> return RISCV_EXCP_ILLEGAL_INST;
> }
> - pmpcfg_csr_write(env, csrno - CSR_PMPCFG0, val);
> + pmpcfg_csr_write(env, reg_index, val);
Reviewed-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>
Zhiwei
> return RISCV_EXCP_NONE;
> }
>
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH 08/18] target/riscv: Simplify getting RISCVCPU pointer from env
2023-02-13 18:02 ` [PATCH 08/18] target/riscv: Simplify getting RISCVCPU pointer from env Bin Meng
2023-02-14 8:51 ` weiwei
@ 2023-02-17 2:30 ` LIU Zhiwei
1 sibling, 0 replies; 57+ messages in thread
From: LIU Zhiwei @ 2023-02-17 2:30 UTC (permalink / raw)
To: Bin Meng, qemu-devel
Cc: Alistair Francis, Bin Meng, Daniel Henrique Barboza,
Palmer Dabbelt, Weiwei Li, qemu-riscv
On 2023/2/14 2:02, Bin Meng wrote:
> Use env_archcpu() to get RISCVCPU pointer from env directly.
>
> Signed-off-by: Bin Meng <bmeng@tinylab.org>
> ---
>
> target/riscv/csr.c | 36 ++++++++++++------------------------
> 1 file changed, 12 insertions(+), 24 deletions(-)
>
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index da3b770894..0a3f2bef6f 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -46,8 +46,7 @@ static RISCVException smstateen_acc_ok(CPURISCVState *env, int index,
> uint64_t bit)
> {
> bool virt = riscv_cpu_virt_enabled(env);
> - CPUState *cs = env_cpu(env);
> - RISCVCPU *cpu = RISCV_CPU(cs);
> + RISCVCPU *cpu = env_archcpu(env);
>
> if (env->priv == PRV_M || !cpu->cfg.ext_smstateen) {
> return RISCV_EXCP_NONE;
> @@ -90,8 +89,7 @@ static RISCVException fs(CPURISCVState *env, int csrno)
>
> static RISCVException vs(CPURISCVState *env, int csrno)
> {
> - CPUState *cs = env_cpu(env);
> - RISCVCPU *cpu = RISCV_CPU(cs);
> + RISCVCPU *cpu = env_archcpu(env);
I see many RISCVCPU pointers are just for cfg in this patch. We can also
use the new interface by Dianel, which will unify the interface for same
function.
Otherwise,
Reviewed-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>
Zhiwei
>
> if (env->misa_ext & RVV ||
> cpu->cfg.ext_zve32f || cpu->cfg.ext_zve64f) {
> @@ -108,8 +106,7 @@ static RISCVException vs(CPURISCVState *env, int csrno)
> static RISCVException ctr(CPURISCVState *env, int csrno)
> {
> #if !defined(CONFIG_USER_ONLY)
> - CPUState *cs = env_cpu(env);
> - RISCVCPU *cpu = RISCV_CPU(cs);
> + RISCVCPU *cpu = env_archcpu(env);
> int ctr_index;
> target_ulong ctr_mask;
> int base_csrno = CSR_CYCLE;
> @@ -166,8 +163,7 @@ static RISCVException ctr32(CPURISCVState *env, int csrno)
> #if !defined(CONFIG_USER_ONLY)
> static RISCVException mctr(CPURISCVState *env, int csrno)
> {
> - CPUState *cs = env_cpu(env);
> - RISCVCPU *cpu = RISCV_CPU(cs);
> + RISCVCPU *cpu = env_archcpu(env);
> int ctr_index;
> int base_csrno = CSR_MHPMCOUNTER3;
>
> @@ -195,8 +191,7 @@ static RISCVException mctr32(CPURISCVState *env, int csrno)
>
> static RISCVException sscofpmf(CPURISCVState *env, int csrno)
> {
> - CPUState *cs = env_cpu(env);
> - RISCVCPU *cpu = RISCV_CPU(cs);
> + RISCVCPU *cpu = env_archcpu(env);
>
> if (!cpu->cfg.ext_sscofpmf) {
> return RISCV_EXCP_ILLEGAL_INST;
> @@ -321,8 +316,7 @@ static RISCVException umode32(CPURISCVState *env, int csrno)
>
> static RISCVException mstateen(CPURISCVState *env, int csrno)
> {
> - CPUState *cs = env_cpu(env);
> - RISCVCPU *cpu = RISCV_CPU(cs);
> + RISCVCPU *cpu = env_archcpu(env);
>
> if (!cpu->cfg.ext_smstateen) {
> return RISCV_EXCP_ILLEGAL_INST;
> @@ -333,8 +327,7 @@ static RISCVException mstateen(CPURISCVState *env, int csrno)
>
> static RISCVException hstateen_pred(CPURISCVState *env, int csrno, int base)
> {
> - CPUState *cs = env_cpu(env);
> - RISCVCPU *cpu = RISCV_CPU(cs);
> + RISCVCPU *cpu = env_archcpu(env);
>
> if (!cpu->cfg.ext_smstateen) {
> return RISCV_EXCP_ILLEGAL_INST;
> @@ -363,8 +356,7 @@ static RISCVException sstateen(CPURISCVState *env, int csrno)
> {
> bool virt = riscv_cpu_virt_enabled(env);
> int index = csrno - CSR_SSTATEEN0;
> - CPUState *cs = env_cpu(env);
> - RISCVCPU *cpu = RISCV_CPU(cs);
> + RISCVCPU *cpu = env_archcpu(env);
>
> if (!cpu->cfg.ext_smstateen) {
> return RISCV_EXCP_ILLEGAL_INST;
> @@ -918,8 +910,7 @@ static RISCVException read_timeh(CPURISCVState *env, int csrno,
>
> static RISCVException sstc(CPURISCVState *env, int csrno)
> {
> - CPUState *cs = env_cpu(env);
> - RISCVCPU *cpu = RISCV_CPU(cs);
> + RISCVCPU *cpu = env_archcpu(env);
> bool hmode_check = false;
>
> if (!cpu->cfg.ext_sstc || !env->rdtime_fn) {
> @@ -1152,8 +1143,7 @@ static RISCVException write_ignore(CPURISCVState *env, int csrno,
> static RISCVException read_mvendorid(CPURISCVState *env, int csrno,
> target_ulong *val)
> {
> - CPUState *cs = env_cpu(env);
> - RISCVCPU *cpu = RISCV_CPU(cs);
> + RISCVCPU *cpu = env_archcpu(env);
>
> *val = cpu->cfg.mvendorid;
> return RISCV_EXCP_NONE;
> @@ -1162,8 +1152,7 @@ static RISCVException read_mvendorid(CPURISCVState *env, int csrno,
> static RISCVException read_marchid(CPURISCVState *env, int csrno,
> target_ulong *val)
> {
> - CPUState *cs = env_cpu(env);
> - RISCVCPU *cpu = RISCV_CPU(cs);
> + RISCVCPU *cpu = env_archcpu(env);
>
> *val = cpu->cfg.marchid;
> return RISCV_EXCP_NONE;
> @@ -1172,8 +1161,7 @@ static RISCVException read_marchid(CPURISCVState *env, int csrno,
> static RISCVException read_mimpid(CPURISCVState *env, int csrno,
> target_ulong *val)
> {
> - CPUState *cs = env_cpu(env);
> - RISCVCPU *cpu = RISCV_CPU(cs);
> + RISCVCPU *cpu = env_archcpu(env);
>
> *val = cpu->cfg.mimpid;
> return RISCV_EXCP_NONE;
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH 09/18] target/riscv: Avoid reporting odd-numbered pmpcfgX in the CSR XML for RV64
2023-02-13 18:02 ` [PATCH 09/18] target/riscv: Avoid reporting odd-numbered pmpcfgX in the CSR XML for RV64 Bin Meng
2023-02-14 8:56 ` weiwei
@ 2023-02-17 2:36 ` LIU Zhiwei
1 sibling, 0 replies; 57+ messages in thread
From: LIU Zhiwei @ 2023-02-17 2:36 UTC (permalink / raw)
To: Bin Meng, qemu-devel
Cc: Alistair Francis, Bin Meng, Daniel Henrique Barboza,
Palmer Dabbelt, Weiwei Li, qemu-riscv
On 2023/2/14 2:02, Bin Meng wrote:
> At present the odd-numbered PMP configuration registers for RV64 are
> reported in the CSR XML by QEMU gdbstub. However these registers do
> not exist on RV64 so trying to access them from gdb results in 'E14'.
>
> Move the pmpcfgX index check from the actual read/write routine to
> the PMP CSR predicate() routine, so that non-existent pmpcfgX won't
> be reported in the CSR XML for RV64.
>
> Signed-off-by: Bin Meng <bmeng@tinylab.org>
> ---
>
> target/riscv/csr.c | 23 ++++++++---------------
> 1 file changed, 8 insertions(+), 15 deletions(-)
>
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index 0a3f2bef6f..749d0ef83e 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -412,6 +412,14 @@ static int aia_hmode32(CPURISCVState *env, int csrno)
> static RISCVException pmp(CPURISCVState *env, int csrno)
> {
> if (riscv_feature(env, RISCV_FEATURE_PMP)) {
> + if (csrno <= CSR_PMPCFG3) {
> + uint32_t reg_index = csrno - CSR_PMPCFG0;
> +
> + if ((reg_index & 1) && (riscv_cpu_mxl(env) == MXL_RV64)) {
> + return RISCV_EXCP_ILLEGAL_INST;
> + }
> + }
> +
> return RISCV_EXCP_NONE;
> }
>
> @@ -3334,23 +3342,11 @@ static RISCVException write_mseccfg(CPURISCVState *env, int csrno,
> return RISCV_EXCP_NONE;
> }
>
> -static bool check_pmp_reg_index(CPURISCVState *env, uint32_t reg_index)
> -{
> - /* TODO: RV128 restriction check */
Should keep this comment. Otherwise,
Reviewed-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>
Zhiwei
> - if ((reg_index & 1) && (riscv_cpu_mxl(env) == MXL_RV64)) {
> - return false;
> - }
> - return true;
> -}
> -
> static RISCVException read_pmpcfg(CPURISCVState *env, int csrno,
> target_ulong *val)
> {
> uint32_t reg_index = csrno - CSR_PMPCFG0;
>
> - if (!check_pmp_reg_index(env, reg_index)) {
> - return RISCV_EXCP_ILLEGAL_INST;
> - }
> *val = pmpcfg_csr_read(env, reg_index);
> return RISCV_EXCP_NONE;
> }
> @@ -3360,9 +3356,6 @@ static RISCVException write_pmpcfg(CPURISCVState *env, int csrno,
> {
> uint32_t reg_index = csrno - CSR_PMPCFG0;
>
> - if (!check_pmp_reg_index(env, reg_index)) {
> - return RISCV_EXCP_ILLEGAL_INST;
> - }
> pmpcfg_csr_write(env, reg_index, val);
> return RISCV_EXCP_NONE;
> }
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH 10/18] target/riscv: gdbstub: Turn on debugger mode before calling CSR predicate()
2023-02-13 18:02 ` [PATCH 10/18] target/riscv: gdbstub: Turn on debugger mode before calling CSR predicate() Bin Meng
2023-02-14 9:02 ` weiwei
@ 2023-02-17 2:39 ` LIU Zhiwei
1 sibling, 0 replies; 57+ messages in thread
From: LIU Zhiwei @ 2023-02-17 2:39 UTC (permalink / raw)
To: Bin Meng, qemu-devel
Cc: Alistair Francis, Bin Meng, Daniel Henrique Barboza,
Palmer Dabbelt, Weiwei Li, qemu-riscv
On 2023/2/14 2:02, Bin Meng wrote:
> Since commit 94452ac4cf26 ("target/riscv: remove fflags, frm, and fcsr from riscv-*-fpu.xml")
> the 3 FPU CSRs are removed from the XML target decription. The
> original intent of that commit was based on the assumption that
> the 3 FPU CSRs will show up in the riscv-csr.xml so the ones in
> riscv-*-fpu.xml are redundant. But unforuantely that is not ture.
> As the FPU CSR predicate() has a run-time check on MSTATUS.FS,
> at the time when CSR XML is generated MSTATUS.FS is unset, hence
> no FPU CSRs will be reported.
>
> The FPU CSR predicate() already considered such a case of being
> accessed by a debugger. All we need to do is to turn on debugger
> mode before calling predicate().
>
> Signed-off-by: Bin Meng <bmeng@tinylab.org>
> ---
>
> target/riscv/gdbstub.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/target/riscv/gdbstub.c b/target/riscv/gdbstub.c
> index 294f0ceb1c..ef52f41460 100644
> --- a/target/riscv/gdbstub.c
> +++ b/target/riscv/gdbstub.c
> @@ -280,6 +280,10 @@ static int riscv_gen_dynamic_csr_xml(CPUState *cs, int base_reg)
> int bitsize = 16 << env->misa_mxl_max;
> int i;
>
> +#if !defined(CONFIG_USER_ONLY)
> + env->debugger = true;
> +#endif
> +
> /* Until gdb knows about 128-bit registers */
> if (bitsize > 64) {
> bitsize = 64;
> @@ -308,6 +312,11 @@ static int riscv_gen_dynamic_csr_xml(CPUState *cs, int base_reg)
> g_string_append_printf(s, "</feature>");
>
> cpu->dyn_csr_xml = g_string_free(s, false);
> +
> +#if !defined(CONFIG_USER_ONLY)
> + env->debugger = false;
> +#endif
> +
Reviewed-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>
Zhiwei
> return CSR_TABLE_SIZE;
> }
>
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH 11/18] target/riscv: gdbstub: Drop the vector CSRs in riscv-vector.xml
2023-02-13 18:02 ` [PATCH 11/18] target/riscv: gdbstub: Drop the vector CSRs in riscv-vector.xml Bin Meng
2023-02-14 9:13 ` weiwei
@ 2023-02-17 2:43 ` LIU Zhiwei
1 sibling, 0 replies; 57+ messages in thread
From: LIU Zhiwei @ 2023-02-17 2:43 UTC (permalink / raw)
To: Bin Meng, qemu-devel
Cc: Alistair Francis, Bin Meng, Daniel Henrique Barboza,
Palmer Dabbelt, Weiwei Li, qemu-riscv
On 2023/2/14 2:02, Bin Meng wrote:
> It's worth noting that the vector CSR predicate() has a similar
> run-time check logic to the FPU CSR. With the previous patch our
> gdbstub can correctly report these vector CSRs via the CSR xml.
>
> Commit 719d3561b269 ("target/riscv: gdb: support vector registers for rv64 & rv32")
> inserted these vector CSRs in an ad-hoc, non-standard way in the
> riscv-vector.xml. Now we can treat these CSRs no different from
> other CSRs.
>
> Signed-off-by: Bin Meng <bmeng@tinylab.org>
> ---
>
> target/riscv/gdbstub.c | 75 ------------------------------------------
> 1 file changed, 75 deletions(-)
>
> diff --git a/target/riscv/gdbstub.c b/target/riscv/gdbstub.c
> index ef52f41460..6048541606 100644
> --- a/target/riscv/gdbstub.c
> +++ b/target/riscv/gdbstub.c
> @@ -127,40 +127,6 @@ static int riscv_gdb_set_fpu(CPURISCVState *env, uint8_t *mem_buf, int n)
> return 0;
> }
>
> -/*
> - * Convert register index number passed by GDB to the correspond
> - * vector CSR number. Vector CSRs are defined after vector registers
> - * in dynamic generated riscv-vector.xml, thus the starting register index
> - * of vector CSRs is 32.
> - * Return 0 if register index number is out of range.
> - */
> -static int riscv_gdb_vector_csrno(int num_regs)
> -{
> - /*
> - * The order of vector CSRs in the switch case
> - * should match with the order defined in csr_ops[].
> - */
> - switch (num_regs) {
> - case 32:
> - return CSR_VSTART;
> - case 33:
> - return CSR_VXSAT;
> - case 34:
> - return CSR_VXRM;
> - case 35:
> - return CSR_VCSR;
> - case 36:
> - return CSR_VL;
> - case 37:
> - return CSR_VTYPE;
> - case 38:
> - return CSR_VLENB;
> - default:
> - /* Unknown register. */
> - return 0;
> - }
> -}
> -
> static int riscv_gdb_get_vector(CPURISCVState *env, GByteArray *buf, int n)
> {
> uint16_t vlenb = env_archcpu(env)->cfg.vlen >> 3;
> @@ -174,19 +140,6 @@ static int riscv_gdb_get_vector(CPURISCVState *env, GByteArray *buf, int n)
> return cnt;
> }
>
> - int csrno = riscv_gdb_vector_csrno(n);
> -
> - if (!csrno) {
> - return 0;
> - }
> -
> - target_ulong val = 0;
> - int result = riscv_csrrw_debug(env, csrno, &val, 0, 0);
> -
> - if (result == RISCV_EXCP_NONE) {
> - return gdb_get_regl(buf, val);
> - }
> -
> return 0;
> }
>
> @@ -201,19 +154,6 @@ static int riscv_gdb_set_vector(CPURISCVState *env, uint8_t *mem_buf, int n)
> return vlenb;
> }
>
> - int csrno = riscv_gdb_vector_csrno(n);
> -
> - if (!csrno) {
> - return 0;
> - }
> -
> - target_ulong val = ldtul_p(mem_buf);
> - int result = riscv_csrrw_debug(env, csrno, NULL, val, -1);
> -
> - if (result == RISCV_EXCP_NONE) {
> - return sizeof(target_ulong);
> - }
> -
> return 0;
> }
>
> @@ -361,21 +301,6 @@ static int ricsv_gen_dynamic_vector_xml(CPUState *cs, int base_reg)
> num_regs++;
> }
>
> - /* Define vector CSRs */
> - const char *vector_csrs[7] = {
> - "vstart", "vxsat", "vxrm", "vcsr",
> - "vl", "vtype", "vlenb"
> - };
> -
> - for (i = 0; i < 7; i++) {
> - g_string_append_printf(s,
> - "<reg name=\"%s\" bitsize=\"%d\""
> - " regnum=\"%d\" group=\"vector\""
> - " type=\"int\"/>",
> - vector_csrs[i], TARGET_LONG_BITS, base_reg++);
> - num_regs++;
> - }
> -
Reviewed-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>
Zhiwei
> g_string_append_printf(s, "</feature>");
>
> cpu->dyn_vreg_xml = g_string_free(s, false);
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH 12/18] target/riscv: Allow debugger to access user timer and counter CSRs
2023-02-14 1:09 ` [PATCH 12/18] target/riscv: Allow debugger to access user timer and counter CSRs Bin Meng
2023-02-14 9:16 ` weiwei
@ 2023-02-17 2:48 ` LIU Zhiwei
1 sibling, 0 replies; 57+ messages in thread
From: LIU Zhiwei @ 2023-02-17 2:48 UTC (permalink / raw)
To: Bin Meng, qemu-devel
Cc: Alistair Francis, Bin Meng, Daniel Henrique Barboza,
Palmer Dabbelt, Weiwei Li, qemu-riscv
On 2023/2/14 9:09, Bin Meng wrote:
> At present user timer and counter CSRs are not reported in the
> CSR XML hence gdb cannot access them.
>
> Fix it by addding a debugger check in their predicate() routine.
>
> Signed-off-by: Bin Meng <bmeng@tinylab.org>
> ---
>
> target/riscv/csr.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index 749d0ef83e..515b05348b 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -131,6 +131,10 @@ static RISCVException ctr(CPURISCVState *env, int csrno)
>
> skip_ext_pmu_check:
>
> + if (env->debugger) {
> + return RISCV_EXCP_NONE;
> + }
> +
Reviewed-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>
Zhiwei
> if (env->priv < PRV_M && !get_field(env->mcounteren, ctr_mask)) {
> return RISCV_EXCP_ILLEGAL_INST;
> }
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH 13/18] target/riscv: Allow debugger to access seed CSR
2023-02-14 1:09 ` [PATCH 13/18] target/riscv: Allow debugger to access seed CSR Bin Meng
2023-02-14 9:18 ` weiwei
@ 2023-02-17 2:59 ` LIU Zhiwei
1 sibling, 0 replies; 57+ messages in thread
From: LIU Zhiwei @ 2023-02-17 2:59 UTC (permalink / raw)
To: Bin Meng, qemu-devel
Cc: Alistair Francis, Bin Meng, Daniel Henrique Barboza,
Palmer Dabbelt, Weiwei Li, qemu-riscv
On 2023/2/14 9:09, Bin Meng wrote:
> At present seed CSR is not reported in the CSR XML hence gdb cannot
> access it.
>
> Fix it by addding a debugger check in its predicate() routine.
>
> Signed-off-by: Bin Meng <bmeng@tinylab.org>
> ---
>
> target/riscv/csr.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index 515b05348b..f1075b5728 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -458,6 +458,10 @@ static RISCVException seed(CPURISCVState *env, int csrno)
> }
>
> #if !defined(CONFIG_USER_ONLY)
> + if (env->debugger) {
> + return RISCV_EXCP_NONE;
> + }
> +
Reviewed-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>
Zhiwei
> /*
> * With a CSR read-write instruction:
> * 1) The seed CSR is always available in machine mode as normal.
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH 18/18] target/riscv: Move configuration check to envcfg CSRs predicate()
2023-02-17 1:59 ` Bin Meng
@ 2023-02-17 17:28 ` Palmer Dabbelt
0 siblings, 0 replies; 57+ messages in thread
From: Palmer Dabbelt @ 2023-02-17 17:28 UTC (permalink / raw)
To: Bin Meng
Cc: liweiwei, qemu-devel, Alistair Francis, bin.meng, dbarboza,
zhiwei_liu, qemu-riscv
On Thu, 16 Feb 2023 17:59:42 PST (-0800), Bin Meng wrote:
> Hi Palmer,
>
> On Fri, Feb 17, 2023 at 12:40 AM Palmer Dabbelt <palmer@dabbelt.com> wrote:
>>
>> On Tue, 14 Feb 2023 18:22:21 PST (-0800), Bin Meng wrote:
>> > On Tue, Feb 14, 2023 at 10:59 PM weiwei <liweiwei@iscas.ac.cn> wrote:
>> >>
>> >>
>> >> On 2023/2/14 22:27, Bin Meng wrote:
>> >> > At present the envcfg CSRs predicate() routines are generic one like
>> >> > smode(), hmode. The configuration check is done in the read / write
>> >> > routine. Create a new predicate routine to cover such check, so that
>> >> > gdbstub can correctly report its existence.
>> >> >
>> >> > Signed-off-by: Bin Meng <bmeng@tinylab.org>
>> >> >
>> >> > ---
>> >> >
>> >> > target/riscv/csr.c | 98 +++++++++++++++++++++++++++++-----------------
>> >> > 1 file changed, 61 insertions(+), 37 deletions(-)
>> >> >
>> >> > diff --git a/target/riscv/csr.c b/target/riscv/csr.c
>> >> > index 37350b8a6d..284ccc09dd 100644
>> >> > --- a/target/riscv/csr.c
>> >> > +++ b/target/riscv/csr.c
>> >> > @@ -41,40 +41,6 @@ void riscv_set_csr_ops(int csrno, riscv_csr_operations *ops)
>> >> > }
>> >> >
>> >> > /* Predicates */
>> >> > -#if !defined(CONFIG_USER_ONLY)
>> >> > -static RISCVException smstateen_acc_ok(CPURISCVState *env, int index,
>> >> > - uint64_t bit)
>> >> > -{
>> >> > - bool virt = riscv_cpu_virt_enabled(env);
>> >> > - RISCVCPU *cpu = env_archcpu(env);
>> >> > -
>> >> > - if (env->priv == PRV_M || !cpu->cfg.ext_smstateen) {
>> >> > - return RISCV_EXCP_NONE;
>> >> > - }
>> >> > -
>> >> > - if (!(env->mstateen[index] & bit)) {
>> >> > - return RISCV_EXCP_ILLEGAL_INST;
>> >> > - }
>> >> > -
>> >> > - if (virt) {
>> >> > - if (!(env->hstateen[index] & bit)) {
>> >> > - return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
>> >> > - }
>> >> > -
>> >> > - if (env->priv == PRV_U && !(env->sstateen[index] & bit)) {
>> >> > - return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
>> >> > - }
>> >> > - }
>> >> > -
>> >> > - if (env->priv == PRV_U && riscv_has_ext(env, RVS)) {
>> >> > - if (!(env->sstateen[index] & bit)) {
>> >> > - return RISCV_EXCP_ILLEGAL_INST;
>> >> > - }
>> >> > - }
>> >> > -
>> >> > - return RISCV_EXCP_NONE;
>> >> > -}
>> >> > -#endif
>> >> >
>> >> > static RISCVException fs(CPURISCVState *env, int csrno)
>> >> > {
>> >> > @@ -318,6 +284,32 @@ static RISCVException umode32(CPURISCVState *env, int csrno)
>> >> > return umode(env, csrno);
>> >> > }
>> >> >
>> >> > +static RISCVException envcfg(CPURISCVState *env, int csrno)
>> >> > +{
>> >> > + RISCVCPU *cpu = env_archcpu(env);
>> >> > + riscv_csr_predicate_fn predicate;
>> >> > +
>> >> > + if (cpu->cfg.ext_smstateen) {
>> >> > + return RISCV_EXCP_ILLEGAL_INST;
>> >> > + }
>> >>
>> >> This check seems not right here. Why ILLEGAL_INST is directly
>> >> triggered if smstateen is enabled?
>> >
>> > This logic was there in the original codes. I was confused when I
>> > looked at this as well.
>> >
>> > Anyway, if it is an issue, it should be a separate patch.
>>
>> Seems reasonable to me, it's always nice to split up the refactoring types. So
>> I queued this up as 4ac6c32224 ("Merge patch series "target/riscv: Various
>> fixes to gdbstub and CSR access"").
>>
>> I had to fix up the From address on the patch you re-sent and there was a minor
>> merge conflict, but otherwise things look sane to me. I'll hold off on sending
>> anything for a bit just in case, though.
>>
>
> There are some open comments in this series I need to address. Please
> drop this v1. I will send v2 soon.
Sorry aobut that, I'd thought they were all reviewed. I've dropped it
from the queue.
Thanks!
>
> Regards,
> Bin
^ permalink raw reply [flat|nested] 57+ messages in thread
end of thread, other threads:[~2023-02-17 17:29 UTC | newest]
Thread overview: 57+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-02-13 18:01 [PATCH 00/18] target/riscv: Various fixes to gdbstub and CSR access Bin Meng
2023-02-13 18:01 ` [PATCH 01/18] target/riscv: gdbstub: Check priv spec version before reporting CSR Bin Meng
2023-02-14 8:40 ` weiwei
2023-02-17 2:11 ` LIU Zhiwei
2023-02-13 18:01 ` [PATCH 02/18] target/riscv: Correct the priority policy of riscv_csrrw_check() Bin Meng
2023-02-14 8:43 ` weiwei
2023-02-17 2:15 ` LIU Zhiwei
2023-02-13 18:01 ` [PATCH 03/18] target/riscv: gdbstub: Minor change for better readability Bin Meng
2023-02-14 8:45 ` weiwei
2023-02-17 2:20 ` LIU Zhiwei
2023-02-13 18:02 ` [PATCH 04/18] target/riscv: gdbstub: Do not generate CSR XML if Zicsr is disabled Bin Meng
2023-02-14 8:46 ` weiwei
2023-02-17 2:23 ` LIU Zhiwei
2023-02-13 18:02 ` [PATCH 05/18] target/riscv: Coding style fixes in csr.c Bin Meng
2023-02-14 8:48 ` weiwei
2023-02-17 2:24 ` LIU Zhiwei
2023-02-13 18:02 ` [PATCH 06/18] target/riscv: Use 'bool' type for read_only Bin Meng
2023-02-14 8:48 ` weiwei
2023-02-17 2:24 ` LIU Zhiwei
2023-02-13 18:02 ` [PATCH 07/18] target/riscv: Simplify {read, write}_pmpcfg() a little bit Bin Meng
2023-02-14 8:50 ` [PATCH 07/18] target/riscv: Simplify {read,write}_pmpcfg() " weiwei
2023-02-17 2:26 ` LIU Zhiwei
2023-02-13 18:02 ` [PATCH 08/18] target/riscv: Simplify getting RISCVCPU pointer from env Bin Meng
2023-02-14 8:51 ` weiwei
2023-02-17 2:30 ` LIU Zhiwei
2023-02-13 18:02 ` [PATCH 09/18] target/riscv: Avoid reporting odd-numbered pmpcfgX in the CSR XML for RV64 Bin Meng
2023-02-14 8:56 ` weiwei
2023-02-17 2:36 ` LIU Zhiwei
2023-02-13 18:02 ` [PATCH 10/18] target/riscv: gdbstub: Turn on debugger mode before calling CSR predicate() Bin Meng
2023-02-14 9:02 ` weiwei
2023-02-17 2:39 ` LIU Zhiwei
2023-02-13 18:02 ` [PATCH 11/18] target/riscv: gdbstub: Drop the vector CSRs in riscv-vector.xml Bin Meng
2023-02-14 9:13 ` weiwei
2023-02-17 2:43 ` LIU Zhiwei
2023-02-13 19:19 ` [PATCH 00/18] target/riscv: Various fixes to gdbstub and CSR access Daniel Henrique Barboza
2023-02-14 14:31 ` Bin Meng
2023-02-14 1:09 ` [PATCH 12/18] target/riscv: Allow debugger to access user timer and counter CSRs Bin Meng
2023-02-14 9:16 ` weiwei
2023-02-17 2:48 ` LIU Zhiwei
2023-02-14 1:09 ` [PATCH 13/18] target/riscv: Allow debugger to access seed CSR Bin Meng
2023-02-14 9:18 ` weiwei
2023-02-17 2:59 ` LIU Zhiwei
2023-02-14 3:06 ` [PATCH 14/18] target/riscv: Allow debugger to access {h, s}stateen CSRs Bin Meng
2023-02-14 9:24 ` weiwei
2023-02-14 4:12 ` [PATCH 15/18] target/riscv: Allow debugger to access sstc CSRs Bin Meng
2023-02-14 9:26 ` weiwei
2023-02-14 4:12 ` [PATCH 16/18] target/riscv: Drop priv level check in mseccfg predicate() Bin Meng
2023-02-14 9:26 ` weiwei
2023-02-14 4:31 ` [PATCH 17/18] target/riscv: Group all predicate() routines together Bin Meng
2023-02-14 9:27 ` weiwei
2023-02-14 14:27 ` [PATCH 18/18] target/riscv: Move configuration check to envcfg CSRs predicate() Bin Meng
2023-02-14 14:59 ` weiwei
2023-02-15 2:22 ` Bin Meng
2023-02-15 2:57 ` weiwei
2023-02-16 16:40 ` Palmer Dabbelt
2023-02-17 1:59 ` Bin Meng
2023-02-17 17:28 ` Palmer Dabbelt
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).