* [PATCH V2 0/2] Proposing custom CSR handling logic
@ 2021-05-11 10:07 Ruinland Chuan-Tzu Tsai
2021-05-11 10:07 ` [PATCH V2 1/2] Adding premliminary support for custom CSR handling mechanism Ruinland Chuan-Tzu Tsai
` (2 more replies)
0 siblings, 3 replies; 12+ messages in thread
From: Ruinland Chuan-Tzu Tsai @ 2021-05-11 10:07 UTC (permalink / raw)
To: qemu-riscv, alistair23; +Cc: dylan, alankao, Ruinland Chuan-Tzu Tsai
Hi all,
My sincere apology that I missed the patch to include our own CSR table
into the patch series and there were plenty of typos.
Thus I'm sending out V2 of these tiny patches.
I agree with Alistair's comment on not introducing intrusive code which
will interfere the generic code structure. Yet since there are
possibilities that some custom CSRs/instructions could be once drafted/
proposed by vendors at first, and made themselves into the standard
as the implementation become widely adopted.
So in this patch set, we humbly utilzed a glib hash table for inserting
the `struct riscv_custom_csr_operations`, check if the CSR is a non
standard one, and then proceed the desired behavior.
Once the non-standard CSRs make themselves into the specification,
people could easily plug-and-use the code into CSR operation table
inside `csr.c`.
Ones may have concerns regarding the check code would introduce
further overhead. For those considerations, I guess it could be solved
by introducing a build option such as '--enable-riscv-vendor-features'
to toggle the code.
Cordially yours,
Ruinland ChuanTzu Tsai
Ruinland Chuan-Tzu Tsai (2):
Adding premliminary support for custom CSR handling mechanism
Adding custom Andes CSR table.
target/riscv/cpu.c | 28 ++++++++
target/riscv/cpu.h | 12 +++-
target/riscv/cpu_bits.h | 115 ++++++++++++++++++++++++++++++++
target/riscv/csr.c | 107 ++++++++++++++++++++++++++++--
target/riscv/csr_andes.inc.c | 125 +++++++++++++++++++++++++++++++++++
5 files changed, 381 insertions(+), 6 deletions(-)
create mode 100644 target/riscv/csr_andes.inc.c
--
2.17.1
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH V2 1/2] Adding premliminary support for custom CSR handling mechanism
2021-05-11 10:07 [PATCH V2 0/2] Proposing custom CSR handling logic Ruinland Chuan-Tzu Tsai
@ 2021-05-11 10:07 ` Ruinland Chuan-Tzu Tsai
2021-05-12 6:16 ` Alistair Francis
2021-05-11 10:07 ` [PATCH V2 2/2] Adding custom Andes CSR table Ruinland Chuan-Tzu Tsai
2021-05-12 6:03 ` Alistair Francis
2 siblings, 1 reply; 12+ messages in thread
From: Ruinland Chuan-Tzu Tsai @ 2021-05-11 10:07 UTC (permalink / raw)
To: qemu-riscv, alistair23; +Cc: dylan, alankao, Ruinland Chuan-Tzu Tsai
Introduce ax25 and custom CSR handling mechanism to RISC-V platform.
This is just a POC in which we add Andes custom CSR table directly
into the generic code which is undresiable and requires overhaul.
Signed-off-by: Dylan Jhong <dylan@andestech.com>
---
target/riscv/cpu.c | 28 ++++++++++
target/riscv/cpu.h | 12 ++++-
target/riscv/cpu_bits.h | 115 ++++++++++++++++++++++++++++++++++++++++
target/riscv/csr.c | 107 +++++++++++++++++++++++++++++++++++--
4 files changed, 256 insertions(+), 6 deletions(-)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 7401325..6dbe9d9 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -34,6 +34,8 @@
static const char riscv_exts[26] = "IEMAFDQCLBJTPVNSUHKORWXYZG";
+GHashTable * custom_csr_map;
+
const char * const riscv_int_regnames[] = {
"x0/zero", "x1/ra", "x2/sp", "x3/gp", "x4/tp", "x5/t0", "x6/t1",
"x7/t2", "x8/s0", "x9/s1", "x10/a0", "x11/a1", "x12/a2", "x13/a3",
@@ -159,6 +161,31 @@ static void rv64_base_cpu_init(Object *obj)
set_misa(env, RV64);
}
+static void ax25_cpu_init(Object *obj)
+{
+ CPURISCVState *env = &RISCV_CPU(obj)->env;
+ set_misa(env, RV64 | RVI | RVM | RVA | RVF | RVD | RVC | RVS | RVU);
+ set_priv_version(env, PRIV_VERSION_1_10_0);
+
+ /* setup custom csr handler hash table */
+ setup_custom_csr();
+
+}
+
+void setup_custom_csr(void) {
+ custom_csr_map = g_hash_table_new_full(g_direct_hash, g_direct_equal, NULL, NULL);
+ int i;
+ for (i = 0; i < MAX_CUSTOM_CSR_NUM; i++) {
+ if (andes_custom_csr_table[i].csrno != 0) {
+ g_hash_table_insert(custom_csr_map,
+ GINT_TO_POINTER(andes_custom_csr_table[i].csrno),
+ &andes_custom_csr_table[i].csr_opset);
+ } else {
+ break;
+ }
+ }
+}
+
static void rv64_sifive_u_cpu_init(Object *obj)
{
CPURISCVState *env = &RISCV_CPU(obj)->env;
@@ -705,6 +732,7 @@ static const TypeInfo riscv_cpu_type_infos[] = {
DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_U34, rv32_sifive_u_cpu_init),
#elif defined(TARGET_RISCV64)
DEFINE_CPU(TYPE_RISCV_CPU_BASE64, rv64_base_cpu_init),
+ DEFINE_CPU(TYPE_RISCV_CPU_AX25, ax25_cpu_init),
DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_E51, rv64_sifive_e_cpu_init),
DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_U54, rv64_sifive_u_cpu_init),
#endif
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 0edb282..a2f656c 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -37,6 +37,7 @@
#define TYPE_RISCV_CPU_ANY RISCV_CPU_TYPE_NAME("any")
#define TYPE_RISCV_CPU_BASE32 RISCV_CPU_TYPE_NAME("rv32")
#define TYPE_RISCV_CPU_BASE64 RISCV_CPU_TYPE_NAME("rv64")
+#define TYPE_RISCV_CPU_AX25 RISCV_CPU_TYPE_NAME("andes-ax25")
#define TYPE_RISCV_CPU_IBEX RISCV_CPU_TYPE_NAME("lowrisc-ibex")
#define TYPE_RISCV_CPU_SIFIVE_E31 RISCV_CPU_TYPE_NAME("sifive-e31")
#define TYPE_RISCV_CPU_SIFIVE_E34 RISCV_CPU_TYPE_NAME("sifive-e34")
@@ -485,16 +486,25 @@ typedef struct {
riscv_csr_op_fn op;
} riscv_csr_operations;
+typedef struct {
+ int csrno;
+ riscv_csr_operations csr_opset;
+ } riscv_custom_csr_operations;
+
/* CSR function table constants */
enum {
- CSR_TABLE_SIZE = 0x1000
+ CSR_TABLE_SIZE = 0x1000,
+ MAX_CUSTOM_CSR_NUM = 100
};
/* CSR function table */
+extern riscv_custom_csr_operations andes_custom_csr_table[MAX_CUSTOM_CSR_NUM];
extern riscv_csr_operations csr_ops[CSR_TABLE_SIZE];
+extern GHashTable *custom_csr_map;
void riscv_get_csr_ops(int csrno, riscv_csr_operations *ops);
void riscv_set_csr_ops(int csrno, riscv_csr_operations *ops);
+void setup_custom_csr(void);
void riscv_cpu_register_gdb_regs_for_features(CPUState *cs);
diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
index caf4599..639bc0a 100644
--- a/target/riscv/cpu_bits.h
+++ b/target/riscv/cpu_bits.h
@@ -259,6 +259,7 @@
#define CSR_TDATA1 0x7a1
#define CSR_TDATA2 0x7a2
#define CSR_TDATA3 0x7a3
+#define CSR_TINFO 0x7a4
/* Debug Mode Registers */
#define CSR_DCSR 0x7b0
@@ -593,3 +594,117 @@
#define MIE_SSIE (1 << IRQ_S_SOFT)
#define MIE_USIE (1 << IRQ_U_SOFT)
#endif
+
+/* ========= AndeStar V5 machine mode CSRs ========= */
+/* Configuration Registers */
+#define CSR_MICM_CFG 0xfc0
+#define CSR_MDCM_CFG 0xfc1
+#define CSR_MMSC_CFG 0xfc2
+#define CSR_MMSC_CFG2 0xfc3
+#define CSR_MVEC_CFG 0xfc7
+
+/* Crash Debug CSRs */
+#define CSR_MCRASH_STATESAVE 0xfc8
+#define CSR_MSTATUS_CRASHSAVE 0xfc9
+
+/* Memory CSRs */
+#define CSR_MILMB 0x7c0
+#define CSR_MDLMB 0x7c1
+#define CSR_MECC_CODE 0x7C2
+#define CSR_MNVEC 0x7c3
+#define CSR_MCACHE_CTL 0x7ca
+#define CSR_MCCTLBEGINADDR 0x7cb
+#define CSR_MCCTLCOMMAND 0x7cc
+#define CSR_MCCTLDATA 0x7cd
+#define CSR_MPPIB 0x7f0
+#define CSR_MFIOB 0x7f1
+
+/* Hardware Stack Protection & Recording */
+#define CSR_MHSP_CTL 0x7c6
+#define CSR_MSP_BOUND 0x7c7
+#define CSR_MSP_BASE 0x7c8
+#define CSR_MXSTATUS 0x7c4
+#define CSR_MDCAUSE 0x7c9
+#define CSR_MSLIDELEG 0x7d5
+#define CSR_MSAVESTATUS 0x7d6
+#define CSR_MSAVEEPC1 0x7d7
+#define CSR_MSAVECAUSE1 0x7d8
+#define CSR_MSAVEEPC2 0x7d9
+#define CSR_MSAVECAUSE2 0x7da
+#define CSR_MSAVEDCAUSE1 0x7db
+#define CSR_MSAVEDCAUSE2 0x7dc
+
+/* Control CSRs */
+#define CSR_MPFT_CTL 0x7c5
+#define CSR_MMISC_CTL 0x7d0
+#define CSR_MCLK_CTL 0x7df
+
+/* Counter related CSRs */
+#define CSR_MCOUNTERWEN 0x7ce
+#define CSR_MCOUNTERINTEN 0x7cf
+#define CSR_MCOUNTERMASK_M 0x7d1
+#define CSR_MCOUNTERMASK_S 0x7d2
+#define CSR_MCOUNTERMASK_U 0x7d3
+#define CSR_MCOUNTEROVF 0x7d4
+
+/* Enhanced CLIC CSRs */
+#define CSR_MIRQ_ENTRY 0x7ec
+#define CSR_MINTSEL_JAL 0x7ed
+#define CSR_PUSHMCAUSE 0x7ee
+#define CSR_PUSHMEPC 0x7ef
+#define CSR_PUSHMXSTATUS 0x7eb
+
+/* Andes Physical Memory Attribute(PMA) CSRs */
+#define CSR_PMACFG0 0xbc0
+#define CSR_PMACFG1 0xbc1
+#define CSR_PMACFG2 0xbc2
+#define CSR_PMACFG3 0xbc3
+#define CSR_PMAADDR0 0xbd0
+#define CSR_PMAADDR1 0xbd1
+#define CSR_PMAADDR2 0xbd2
+#define CSR_PMAADDR3 0xbd2
+#define CSR_PMAADDR4 0xbd4
+#define CSR_PMAADDR5 0xbd5
+#define CSR_PMAADDR6 0xbd6
+#define CSR_PMAADDR7 0xbd7
+#define CSR_PMAADDR8 0xbd8
+#define CSR_PMAADDR9 0xbd9
+#define CSR_PMAADDR10 0xbda
+#define CSR_PMAADDR11 0xbdb
+#define CSR_PMAADDR12 0xbdc
+#define CSR_PMAADDR13 0xbdd
+#define CSR_PMAADDR14 0xbde
+#define CSR_PMAADDR15 0xbdf
+
+/* ========= AndeStar V5 supervisor mode CSRs ========= */
+/* Supervisor trap registers */
+#define CSR_SLIE 0x9c4
+#define CSR_SLIP 0x9c5
+#define CSR_SDCAUSE 0x9c9
+
+/* Supervisor counter registers */
+#define CSR_SCOUNTERINTEN 0x9cf
+#define CSR_SCOUNTERMASK_M 0x9d1
+#define CSR_SCOUNTERMASK_S 0x9d2
+#define CSR_SCOUNTERMASK_U 0x9d3
+#define CSR_SCOUNTEROVF 0x9d4
+#define CSR_SCOUNTINHIBIT 0x9e0
+#define CSR_SHPMEVENT3 0x9e3
+#define CSR_SHPMEVENT4 0x9e4
+#define CSR_SHPMEVENT5 0x9e5
+#define CSR_SHPMEVENT6 0x9e6
+
+/* Supervisor control registers */
+#define CSR_SCCTLDATA 0x9cd
+#define CSR_SMISC_CTL 0x9d0
+
+/* ========= AndeStar V5 user mode CSRs ========= */
+/* User mode control registers */
+#define CSR_UITB 0x800
+#define CSR_UCODE 0x801
+#define CSR_UDCAUSE 0x809
+#define CSR_UCCTLBEGINADDR 0x80b
+#define CSR_UCCTLCOMMAND 0x80c
+#define CSR_WFE 0x810
+#define CSR_SLEEPVALUE 0x811
+#define CSR_TXEVT 0x812
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index fd2e636..b81efcf 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -523,6 +523,14 @@ static int read_misa(CPURISCVState *env, int csrno, target_ulong *val)
return 0;
}
+
+// XXX: This is just a write stub for developing custom CSR handler,
+// if the behavior of writting such CSR is not presentable in QEMU and doesn't
+// affect the functionality, just stub it.
+static int write_stub(CPURISCVState *env, int csrno, target_ulong val) {
+ return 0;
+}
+
static int write_misa(CPURISCVState *env, int csrno, target_ulong val)
{
if (!riscv_feature(env, RISCV_FEATURE_MISA)) {
@@ -1264,6 +1272,76 @@ static int write_pmpaddr(CPURISCVState *env, int csrno, target_ulong val)
#endif
+
+/* Custom CSR related routines and data structures */
+
+gpointer is_custom_csr(int);
+
+gpointer is_custom_csr(int csrno) {
+ gpointer ret;
+ ret = g_hash_table_lookup(custom_csr_map, GINT_TO_POINTER(csrno));
+ return ret;
+ }
+
+int try_handle_custom_csr(CPURISCVState *env, int csrno,
+ target_ulong *ret_value, target_ulong new_value, target_ulong write_mask,
+ riscv_csr_operations *opset);
+
+// XXX: This part is mainly duplicate from riscv_csrrw, we need to redo the logic
+int try_handle_custom_csr(CPURISCVState *env, int csrno,
+ target_ulong *ret_value, target_ulong new_value, target_ulong write_mask,
+ riscv_csr_operations *opset) {
+
+ int ret = 0;
+ target_ulong old_value;
+
+ /* check predicate */
+ if (!opset->predicate) {
+ return -RISCV_EXCP_ILLEGAL_INST;
+ }
+ ret = opset->predicate(env, csrno);
+ if (ret < 0) {
+ return ret;
+ }
+
+ /* execute combined read/write operation if it exists */
+ if (opset->op) {
+ return opset->op(env, csrno, ret_value, new_value, write_mask);
+ }
+
+ /* if no accessor exists then return failure */
+ if (!opset->read) {
+ return -RISCV_EXCP_ILLEGAL_INST;
+ }
+
+ /* read old value */
+ ret = opset->read(env, csrno, &old_value);
+ if (ret < 0) {
+ return ret;
+ }
+
+ /* write value if writable and write mask set, otherwise drop writes */
+ if (write_mask) {
+ new_value = (old_value & ~write_mask) | (new_value & write_mask);
+ if (opset->write) {
+ ret = opset->write(env, csrno, new_value);
+ if (ret < 0) {
+ return ret;
+ }
+ }
+ }
+
+ /* return old value */
+ if (ret_value) {
+ *ret_value = old_value;
+ }
+
+ return 0;
+
+
+
+ }
+
/*
* riscv_csrrw - read and/or update control and status register
*
@@ -1283,7 +1361,7 @@ int riscv_csrrw(CPURISCVState *env, int csrno, target_ulong *ret_value,
/* check privileges and return -1 if check fails */
#if !defined(CONFIG_USER_ONLY)
int effective_priv = env->priv;
- int read_only = get_field(csrno, 0xC00) == 3;
+ /* int read_only = get_field(csrno, 0xC00) == 3; */
if (riscv_has_ext(env, RVH) &&
env->priv == PRV_S &&
@@ -1296,10 +1374,12 @@ int riscv_csrrw(CPURISCVState *env, int csrno, target_ulong *ret_value,
effective_priv++;
}
- if ((write_mask && read_only) ||
- (!env->debugger && (effective_priv < get_field(csrno, 0x300)))) {
- return -RISCV_EXCP_ILLEGAL_INST;
- }
+ /*
+ * if ((write_mask && read_only) ||
+ * (!env->debugger && (effective_priv < get_field(csrno, 0x300)))) {
+ * return -RISCV_EXCP_ILLEGAL_INST;
+ * }
+ */
#endif
/* ensure the CSR extension is enabled. */
@@ -1307,6 +1387,12 @@ int riscv_csrrw(CPURISCVState *env, int csrno, target_ulong *ret_value,
return -RISCV_EXCP_ILLEGAL_INST;
}
+ /* try handle_custom_csr */
+ riscv_csr_operations *custom_csr_opset = (riscv_csr_operations *) is_custom_csr(csrno);
+ if(NULL != custom_csr_opset) {
+ return try_handle_custom_csr(env, csrno, ret_value, new_value, write_mask, custom_csr_opset);
+ }
+
/* check predicate */
if (!csr_ops[csrno].predicate) {
return -RISCV_EXCP_ILLEGAL_INST;
@@ -1351,6 +1437,14 @@ int riscv_csrrw(CPURISCVState *env, int csrno, target_ulong *ret_value,
return 0;
}
+/* Andes Custom Registers */
+static int read_mmsc_cfg(CPURISCVState *env, int csrno, target_ulong *val)
+{
+ /* enable pma probe */
+ *val = 0x40000000;
+ return 0;
+}
+
/*
* Debugger support. If not in user mode, set env->debugger before the
* riscv_csrrw call and clear it after the call.
@@ -1369,6 +1463,8 @@ int riscv_csrrw_debug(CPURISCVState *env, int csrno, target_ulong *ret_value,
return ret;
}
+#include "csr_andes.inc.c"
+
/* Control and Status Register function table */
riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
/* User Floating-Point CSRs */
@@ -1645,3 +1741,4 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
[CSR_MHPMCOUNTER31H] = { "mhpmcounter31h", any32, read_zero },
#endif /* !CONFIG_USER_ONLY */
};
+
--
2.17.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH V2 2/2] Adding custom Andes CSR table.
2021-05-11 10:07 [PATCH V2 0/2] Proposing custom CSR handling logic Ruinland Chuan-Tzu Tsai
2021-05-11 10:07 ` [PATCH V2 1/2] Adding premliminary support for custom CSR handling mechanism Ruinland Chuan-Tzu Tsai
@ 2021-05-11 10:07 ` Ruinland Chuan-Tzu Tsai
2021-05-12 6:03 ` Alistair Francis
2 siblings, 0 replies; 12+ messages in thread
From: Ruinland Chuan-Tzu Tsai @ 2021-05-11 10:07 UTC (permalink / raw)
To: qemu-riscv, alistair23; +Cc: dylan, alankao, Ruinland Chuan-Tzu Tsai
---
target/riscv/csr_andes.inc.c | 125 +++++++++++++++++++++++++++++++++++
1 file changed, 125 insertions(+)
create mode 100644 target/riscv/csr_andes.inc.c
diff --git a/target/riscv/csr_andes.inc.c b/target/riscv/csr_andes.inc.c
new file mode 100644
index 0000000..c470ef8
--- /dev/null
+++ b/target/riscv/csr_andes.inc.c
@@ -0,0 +1,125 @@
+riscv_custom_csr_operations andes_custom_csr_table[MAX_CUSTOM_CSR_NUM] = {
+#if !defined(CONFIG_USER_ONLY)
+ /* ==================== AndeStar V5 machine mode CSRs ==================== */
+ /* Configuration Registers */
+ {CSR_MICM_CFG, { "micm_cfg", any, read_zero, write_stub} },
+ {CSR_MDCM_CFG, { "mdcm_cfg", any, read_zero, write_stub} },
+ {CSR_MMSC_CFG, { "mmsc_cfg", any, read_mmsc_cfg, write_stub} },
+ {CSR_MMSC_CFG2, { "mmsc_cfg2", any, read_zero, write_stub} },
+ {CSR_MVEC_CFG, { "mvec_cfg", any, read_zero, write_stub} },
+
+ /* Crash Debug CSRs */
+ {CSR_MCRASH_STATESAVE, { "mcrash_statesave", any, read_zero, write_stub} },
+ {CSR_MSTATUS_CRASHSAVE, { "mstatus_crashsave", any, read_zero, write_stub} },
+
+ /* Memory CSRs */
+ {CSR_MILMB, { "milmb", any, read_zero, write_stub} },
+ {CSR_MDLMB, { "mdlmb", any, read_zero, write_stub} },
+ {CSR_MECC_CODE, { "mecc_code", any, read_zero, write_stub} },
+ {CSR_MNVEC, { "mnvec", any, read_zero, write_stub} },
+ {CSR_MCACHE_CTL, { "mcache_ctl", any, read_zero, write_stub} },
+ {CSR_MCCTLBEGINADDR, { "mcctlbeginaddr", any, read_zero, write_stub} },
+ {CSR_MCCTLCOMMAND, { "mcctlcommand", any, read_zero, write_stub} },
+ {CSR_MCCTLDATA, { "mcctldata", any, read_zero, write_stub} },
+ {CSR_MPPIB, { "mppib", any, read_zero, write_stub} },
+ {CSR_MFIOB, { "mfiob", any, read_zero, write_stub} },
+
+ /* Hardware Stack Protection & Recording */
+ {CSR_MHSP_CTL, { "mhsp_ctl", any, read_zero, write_stub} },
+ {CSR_MSP_BOUND, { "msp_bound", any, read_zero, write_stub} },
+ {CSR_MSP_BASE, { "msp_base", any, read_zero, write_stub} },
+ {CSR_MXSTATUS, { "mxstatus", any, read_zero, write_stub} },
+ {CSR_MDCAUSE, { "mdcause", any, read_zero, write_stub} },
+ {CSR_MSLIDELEG, { "mslideleg", any, read_zero, write_stub} },
+ {CSR_MSAVESTATUS, { "msavestatus", any, read_zero, write_stub} },
+ {CSR_MSAVEEPC1, { "msaveepc1", any, read_zero, write_stub} },
+ {CSR_MSAVECAUSE1, { "msavecause1", any, read_zero, write_stub} },
+ {CSR_MSAVEEPC2, { "msaveepc2", any, read_zero, write_stub} },
+ {CSR_MSAVECAUSE2, { "msavecause2", any, read_zero, write_stub} },
+ {CSR_MSAVEDCAUSE1, { "msavedcause1", any, read_zero, write_stub} },
+ {CSR_MSAVEDCAUSE2, { "msavedcause2", any, read_zero, write_stub} },
+
+ /* Control CSRs */
+ {CSR_MPFT_CTL, { "mpft_ctl", any, read_zero, write_stub} },
+ {CSR_MMISC_CTL, { "mmisc_ctl", any, read_zero, write_stub} },
+ {CSR_MCLK_CTL, { "mclk_ctl", any, read_zero, write_stub} },
+
+ /* Counter related CSRs */
+ {CSR_MCOUNTERWEN, { "mcounterwen", any, read_zero, write_stub} },
+ {CSR_MCOUNTERINTEN, { "mcounterinten", any, read_zero, write_stub} },
+ {CSR_MCOUNTERMASK_M, { "mcountermask_m", any, read_zero, write_stub} },
+ {CSR_MCOUNTERMASK_S, { "mcountermask_s", any, read_zero, write_stub} },
+ {CSR_MCOUNTERMASK_U, { "mcountermask_u", any, read_zero, write_stub} },
+ {CSR_MCOUNTEROVF, { "mcounterovf", any, read_zero, write_stub} },
+
+ /* Enhanced CLIC CSRs */
+ {CSR_MIRQ_ENTRY, { "mirq_entry", any, read_zero, write_stub} },
+ {CSR_MINTSEL_JAL, { "mintsel_jal", any, read_zero, write_stub} },
+ {CSR_PUSHMCAUSE, { "pushmcause", any, read_zero, write_stub} },
+ {CSR_PUSHMEPC, { "pushmepc", any, read_zero, write_stub} },
+ {CSR_PUSHMXSTATUS, { "pushmxstatus", any, read_zero, write_stub} },
+
+ /* Andes Physical Memory Attribute(PMA) CSRs */
+ {CSR_PMACFG0, { "pmacfg0", any, read_zero, write_stub} },
+ {CSR_PMACFG1, { "pmacfg1", any, read_zero, write_stub} },
+ {CSR_PMACFG2, { "pmacfg2", any, read_zero, write_stub} },
+ {CSR_PMACFG3, { "pmacfg3", any, read_zero, write_stub} },
+ {CSR_PMAADDR0, { "pmaaddr0", any, read_zero, write_stub} },
+ {CSR_PMAADDR1, { "pmaaddr1", any, read_zero, write_stub} },
+ {CSR_PMAADDR2, { "pmaaddr2", any, read_zero, write_stub} },
+ {CSR_PMAADDR3, { "pmaaddr3", any, read_zero, write_stub} },
+ {CSR_PMAADDR4, { "pmaaddr4", any, read_zero, write_stub} },
+ {CSR_PMAADDR5, { "pmaaddr5", any, read_zero, write_stub} },
+ {CSR_PMAADDR6, { "pmaaddr6", any, read_zero, write_stub} },
+ {CSR_PMAADDR7, { "pmaaddr7", any, read_zero, write_stub} },
+ {CSR_PMAADDR8, { "pmaaddr8", any, read_zero, write_stub} },
+ {CSR_PMAADDR9, { "pmaaddr9", any, read_zero, write_stub} },
+ {CSR_PMAADDR10, { "pmaaddr10", any, read_zero, write_stub} },
+ {CSR_PMAADDR11, { "pmaaddr11", any, read_zero, write_stub} },
+ {CSR_PMAADDR12, { "pmaaddr12", any, read_zero, write_stub} },
+ {CSR_PMAADDR13, { "pmaaddr13", any, read_zero, write_stub} },
+ {CSR_PMAADDR14, { "pmaaddr14", any, read_zero, write_stub} },
+ {CSR_PMAADDR15, { "pmaaddr15", any, read_zero, write_stub} },
+
+ /* Debug/Trace Registers (shared with Debug Mode) */
+ {CSR_TSELECT, { "tselect", any, read_zero, write_stub} },
+ {CSR_TDATA1, { "tdata1", any, read_zero, write_stub} },
+ {CSR_TDATA2, { "tdata2", any, read_zero, write_stub} },
+ {CSR_TDATA3, { "tdata3", any, read_zero, write_stub} },
+ {CSR_TINFO, { "tinfo", any, read_zero, write_stub} },
+
+ /* ================== AndeStar V5 supervisor mode CSRs ================== */
+ /* Supervisor trap registers */
+ {CSR_SLIE, { "slie", any, read_zero, write_stub} },
+ {CSR_SLIP, { "slip", any, read_zero, write_stub} },
+ {CSR_SDCAUSE, { "sdcause", any, read_zero, write_stub} },
+
+ /* Supervisor counter registers */
+ {CSR_SCOUNTERINTEN, { "scounterinten", any, read_zero, write_stub} },
+ {CSR_SCOUNTERMASK_M, { "scountermask_m", any, read_zero, write_stub} },
+ {CSR_SCOUNTERMASK_S, { "scountermask_s", any, read_zero, write_stub} },
+ {CSR_SCOUNTERMASK_U, { "scountermask_u", any, read_zero, write_stub} },
+ {CSR_SCOUNTEROVF, { "scounterovf", any, read_zero, write_stub} },
+ {CSR_SCOUNTINHIBIT, { "scountinhibit", any, read_zero, write_stub} },
+ {CSR_SHPMEVENT3, { "shpmevent3", any, read_zero, write_stub} },
+ {CSR_SHPMEVENT4, { "shpmevent4", any, read_zero, write_stub} },
+ {CSR_SHPMEVENT5, { "shpmevent5", any, read_zero, write_stub} },
+ {CSR_SHPMEVENT6, { "shpmevent6", any, read_zero, write_stub} },
+
+ /* Supervisor control registers */
+ {CSR_SCCTLDATA, { "scctldata", any, read_zero, write_stub} },
+ {CSR_SMISC_CTL, { "smisc_ctl", any, read_zero, write_stub} },
+
+ /* ===================== AndeStar V5 user mode CSRs ===================== */
+ /* User mode control registers */
+ {CSR_UITB, { "uitb", any, read_zero, write_stub} },
+ {CSR_UCODE, { "ucode", any, read_zero, write_stub} },
+ {CSR_UDCAUSE, { "udcause", any, read_zero, write_stub} },
+ {CSR_UCCTLBEGINADDR, { "ucctlbeginaddr", any, read_zero, write_stub} },
+ {CSR_UCCTLCOMMAND, { "ucctlcommand", any, read_zero, write_stub} },
+ {CSR_WFE, { "wfe", any, read_zero, write_stub} },
+ {CSR_SLEEPVALUE, { "sleepvalue", any, read_zero, write_stub} },
+ {CSR_TXEVT, { "csr_txevt", any, read_zero, write_stub} },
+ {0, { "", NULL, NULL, NULL } },
+#endif
+ };
--
2.17.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH V2 0/2] Proposing custom CSR handling logic
2021-05-11 10:07 [PATCH V2 0/2] Proposing custom CSR handling logic Ruinland Chuan-Tzu Tsai
@ 2021-05-12 6:03 ` Alistair Francis
2021-05-11 10:07 ` [PATCH V2 2/2] Adding custom Andes CSR table Ruinland Chuan-Tzu Tsai
2021-05-12 6:03 ` Alistair Francis
2 siblings, 0 replies; 12+ messages in thread
From: Alistair Francis @ 2021-05-12 6:03 UTC (permalink / raw)
To: Ruinland Chuan-Tzu Tsai, wangjunqiang,
qemu-devel@nongnu.org Developers, Bin Meng
Cc: Dylan Jhong, open list:RISC-V, Alan Quey-Liang Kao(((((((((()
On Tue, May 11, 2021 at 8:07 PM Ruinland Chuan-Tzu Tsai
<ruinland@andestech.com> wrote:
>
> Hi all,
>
> My sincere apology that I missed the patch to include our own CSR table
> into the patch series and there were plenty of typos.
> Thus I'm sending out V2 of these tiny patches.
>
> I agree with Alistair's comment on not introducing intrusive code which
> will interfere the generic code structure. Yet since there are
> possibilities that some custom CSRs/instructions could be once drafted/
> proposed by vendors at first, and made themselves into the standard
> as the implementation become widely adopted.
>
> So in this patch set, we humbly utilzed a glib hash table for inserting
> the `struct riscv_custom_csr_operations`, check if the CSR is a non
> standard one, and then proceed the desired behavior.
>
> Once the non-standard CSRs make themselves into the specification,
> people could easily plug-and-use the code into CSR operation table
> inside `csr.c`.
>
> Ones may have concerns regarding the check code would introduce
> further overhead. For those considerations, I guess it could be solved
> by introducing a build option such as '--enable-riscv-vendor-features'
> to toggle the code.
>
> Cordially yours,
> Ruinland ChuanTzu Tsai
>
> Ruinland Chuan-Tzu Tsai (2):
> Adding premliminary support for custom CSR handling mechanism
> Adding custom Andes CSR table.
Thanks for the patches.
Can you please include:
wangjunqiang@iscas.ac.cn
qemu-devel@nongnu.org
bin.meng@windriver.com
on future patches so everyone is included.
Alistair
>
> target/riscv/cpu.c | 28 ++++++++
> target/riscv/cpu.h | 12 +++-
> target/riscv/cpu_bits.h | 115 ++++++++++++++++++++++++++++++++
> target/riscv/csr.c | 107 ++++++++++++++++++++++++++++--
> target/riscv/csr_andes.inc.c | 125 +++++++++++++++++++++++++++++++++++
> 5 files changed, 381 insertions(+), 6 deletions(-)
> create mode 100644 target/riscv/csr_andes.inc.c
>
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH V2 0/2] Proposing custom CSR handling logic
@ 2021-05-12 6:03 ` Alistair Francis
0 siblings, 0 replies; 12+ messages in thread
From: Alistair Francis @ 2021-05-12 6:03 UTC (permalink / raw)
To: Ruinland Chuan-Tzu Tsai, wangjunqiang,
qemu-devel@nongnu.org Developers, Bin Meng
Cc: open list:RISC-V, Dylan Jhong, Alan Quey-Liang Kao(((((((((()
On Tue, May 11, 2021 at 8:07 PM Ruinland Chuan-Tzu Tsai
<ruinland@andestech.com> wrote:
>
> Hi all,
>
> My sincere apology that I missed the patch to include our own CSR table
> into the patch series and there were plenty of typos.
> Thus I'm sending out V2 of these tiny patches.
>
> I agree with Alistair's comment on not introducing intrusive code which
> will interfere the generic code structure. Yet since there are
> possibilities that some custom CSRs/instructions could be once drafted/
> proposed by vendors at first, and made themselves into the standard
> as the implementation become widely adopted.
>
> So in this patch set, we humbly utilzed a glib hash table for inserting
> the `struct riscv_custom_csr_operations`, check if the CSR is a non
> standard one, and then proceed the desired behavior.
>
> Once the non-standard CSRs make themselves into the specification,
> people could easily plug-and-use the code into CSR operation table
> inside `csr.c`.
>
> Ones may have concerns regarding the check code would introduce
> further overhead. For those considerations, I guess it could be solved
> by introducing a build option such as '--enable-riscv-vendor-features'
> to toggle the code.
>
> Cordially yours,
> Ruinland ChuanTzu Tsai
>
> Ruinland Chuan-Tzu Tsai (2):
> Adding premliminary support for custom CSR handling mechanism
> Adding custom Andes CSR table.
Thanks for the patches.
Can you please include:
wangjunqiang@iscas.ac.cn
qemu-devel@nongnu.org
bin.meng@windriver.com
on future patches so everyone is included.
Alistair
>
> target/riscv/cpu.c | 28 ++++++++
> target/riscv/cpu.h | 12 +++-
> target/riscv/cpu_bits.h | 115 ++++++++++++++++++++++++++++++++
> target/riscv/csr.c | 107 ++++++++++++++++++++++++++++--
> target/riscv/csr_andes.inc.c | 125 +++++++++++++++++++++++++++++++++++
> 5 files changed, 381 insertions(+), 6 deletions(-)
> create mode 100644 target/riscv/csr_andes.inc.c
>
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH V2 1/2] Adding premliminary support for custom CSR handling mechanism
2021-05-11 10:07 ` [PATCH V2 1/2] Adding premliminary support for custom CSR handling mechanism Ruinland Chuan-Tzu Tsai
@ 2021-05-12 6:16 ` Alistair Francis
2021-05-12 23:41 ` Alistair Francis
0 siblings, 1 reply; 12+ messages in thread
From: Alistair Francis @ 2021-05-12 6:16 UTC (permalink / raw)
To: Ruinland Chuan-Tzu Tsai
Cc: open list:RISC-V, Dylan Jhong, Alan Quey-Liang Kao(((((((((()
On Tue, May 11, 2021 at 8:07 PM Ruinland Chuan-Tzu Tsai
<ruinland@andestech.com> wrote:
>
> Introduce ax25 and custom CSR handling mechanism to RISC-V platform.
> This is just a POC in which we add Andes custom CSR table directly
> into the generic code which is undresiable and requires overhaul.
>
> Signed-off-by: Dylan Jhong <dylan@andestech.com>
Thanks for the patch.
This seems like a good start. I have left some comments inline.
Why do we need a hash table? Couldn't we just have a second
riscv_csr_operations array?
> ---
> target/riscv/cpu.c | 28 ++++++++++
> target/riscv/cpu.h | 12 ++++-
> target/riscv/cpu_bits.h | 115 ++++++++++++++++++++++++++++++++++++++++
> target/riscv/csr.c | 107 +++++++++++++++++++++++++++++++++++--
> 4 files changed, 256 insertions(+), 6 deletions(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 7401325..6dbe9d9 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -34,6 +34,8 @@
>
> static const char riscv_exts[26] = "IEMAFDQCLBJTPVNSUHKORWXYZG";
>
> +GHashTable * custom_csr_map;
This should be part of CPURISCVState instead of a global
> +
> const char * const riscv_int_regnames[] = {
> "x0/zero", "x1/ra", "x2/sp", "x3/gp", "x4/tp", "x5/t0", "x6/t1",
> "x7/t2", "x8/s0", "x9/s1", "x10/a0", "x11/a1", "x12/a2", "x13/a3",
> @@ -159,6 +161,31 @@ static void rv64_base_cpu_init(Object *obj)
> set_misa(env, RV64);
> }
>
> +static void ax25_cpu_init(Object *obj)
> +{
> + CPURISCVState *env = &RISCV_CPU(obj)->env;
> + set_misa(env, RV64 | RVI | RVM | RVA | RVF | RVD | RVC | RVS | RVU);
> + set_priv_version(env, PRIV_VERSION_1_10_0);
> +
> + /* setup custom csr handler hash table */
> + setup_custom_csr();
> +
> +}
> +
> +void setup_custom_csr(void) {
> + custom_csr_map = g_hash_table_new_full(g_direct_hash, g_direct_equal, NULL, NULL);
> + int i;
> + for (i = 0; i < MAX_CUSTOM_CSR_NUM; i++) {
> + if (andes_custom_csr_table[i].csrno != 0) {
> + g_hash_table_insert(custom_csr_map,
> + GINT_TO_POINTER(andes_custom_csr_table[i].csrno),
> + &andes_custom_csr_table[i].csr_opset);
> + } else {
> + break;
> + }
> + }
> +}
> +
> static void rv64_sifive_u_cpu_init(Object *obj)
> {
> CPURISCVState *env = &RISCV_CPU(obj)->env;
> @@ -705,6 +732,7 @@ static const TypeInfo riscv_cpu_type_infos[] = {
> DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_U34, rv32_sifive_u_cpu_init),
> #elif defined(TARGET_RISCV64)
> DEFINE_CPU(TYPE_RISCV_CPU_BASE64, rv64_base_cpu_init),
> + DEFINE_CPU(TYPE_RISCV_CPU_AX25, ax25_cpu_init),
Let's add the CPU in a separate patch.
> DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_E51, rv64_sifive_e_cpu_init),
> DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_U54, rv64_sifive_u_cpu_init),
> #endif
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 0edb282..a2f656c 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -37,6 +37,7 @@
> #define TYPE_RISCV_CPU_ANY RISCV_CPU_TYPE_NAME("any")
> #define TYPE_RISCV_CPU_BASE32 RISCV_CPU_TYPE_NAME("rv32")
> #define TYPE_RISCV_CPU_BASE64 RISCV_CPU_TYPE_NAME("rv64")
> +#define TYPE_RISCV_CPU_AX25 RISCV_CPU_TYPE_NAME("andes-ax25")
> #define TYPE_RISCV_CPU_IBEX RISCV_CPU_TYPE_NAME("lowrisc-ibex")
> #define TYPE_RISCV_CPU_SIFIVE_E31 RISCV_CPU_TYPE_NAME("sifive-e31")
> #define TYPE_RISCV_CPU_SIFIVE_E34 RISCV_CPU_TYPE_NAME("sifive-e34")
> @@ -485,16 +486,25 @@ typedef struct {
> riscv_csr_op_fn op;
> } riscv_csr_operations;
>
> +typedef struct {
> + int csrno;
> + riscv_csr_operations csr_opset;
> + } riscv_custom_csr_operations;
> +
> /* CSR function table constants */
> enum {
> - CSR_TABLE_SIZE = 0x1000
> + CSR_TABLE_SIZE = 0x1000,
> + MAX_CUSTOM_CSR_NUM = 100
> };
>
> /* CSR function table */
> +extern riscv_custom_csr_operations andes_custom_csr_table[MAX_CUSTOM_CSR_NUM];
> extern riscv_csr_operations csr_ops[CSR_TABLE_SIZE];
> +extern GHashTable *custom_csr_map;
>
> void riscv_get_csr_ops(int csrno, riscv_csr_operations *ops);
> void riscv_set_csr_ops(int csrno, riscv_csr_operations *ops);
> +void setup_custom_csr(void);
>
> void riscv_cpu_register_gdb_regs_for_features(CPUState *cs);
>
> diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
> index caf4599..639bc0a 100644
> --- a/target/riscv/cpu_bits.h
> +++ b/target/riscv/cpu_bits.h
> @@ -259,6 +259,7 @@
> #define CSR_TDATA1 0x7a1
> #define CSR_TDATA2 0x7a2
> #define CSR_TDATA3 0x7a3
> +#define CSR_TINFO 0x7a4
Why add this?
>
> /* Debug Mode Registers */
> #define CSR_DCSR 0x7b0
> @@ -593,3 +594,117 @@
> #define MIE_SSIE (1 << IRQ_S_SOFT)
> #define MIE_USIE (1 << IRQ_U_SOFT)
> #endif
> +
> +/* ========= AndeStar V5 machine mode CSRs ========= */
> +/* Configuration Registers */
> +#define CSR_MICM_CFG 0xfc0
> +#define CSR_MDCM_CFG 0xfc1
> +#define CSR_MMSC_CFG 0xfc2
> +#define CSR_MMSC_CFG2 0xfc3
> +#define CSR_MVEC_CFG 0xfc7
> +
> +/* Crash Debug CSRs */
> +#define CSR_MCRASH_STATESAVE 0xfc8
> +#define CSR_MSTATUS_CRASHSAVE 0xfc9
> +
> +/* Memory CSRs */
> +#define CSR_MILMB 0x7c0
> +#define CSR_MDLMB 0x7c1
> +#define CSR_MECC_CODE 0x7C2
> +#define CSR_MNVEC 0x7c3
> +#define CSR_MCACHE_CTL 0x7ca
> +#define CSR_MCCTLBEGINADDR 0x7cb
> +#define CSR_MCCTLCOMMAND 0x7cc
> +#define CSR_MCCTLDATA 0x7cd
> +#define CSR_MPPIB 0x7f0
> +#define CSR_MFIOB 0x7f1
> +
> +/* Hardware Stack Protection & Recording */
> +#define CSR_MHSP_CTL 0x7c6
> +#define CSR_MSP_BOUND 0x7c7
> +#define CSR_MSP_BASE 0x7c8
> +#define CSR_MXSTATUS 0x7c4
> +#define CSR_MDCAUSE 0x7c9
> +#define CSR_MSLIDELEG 0x7d5
> +#define CSR_MSAVESTATUS 0x7d6
> +#define CSR_MSAVEEPC1 0x7d7
> +#define CSR_MSAVECAUSE1 0x7d8
> +#define CSR_MSAVEEPC2 0x7d9
> +#define CSR_MSAVECAUSE2 0x7da
> +#define CSR_MSAVEDCAUSE1 0x7db
> +#define CSR_MSAVEDCAUSE2 0x7dc
> +
> +/* Control CSRs */
> +#define CSR_MPFT_CTL 0x7c5
> +#define CSR_MMISC_CTL 0x7d0
> +#define CSR_MCLK_CTL 0x7df
> +
> +/* Counter related CSRs */
> +#define CSR_MCOUNTERWEN 0x7ce
> +#define CSR_MCOUNTERINTEN 0x7cf
> +#define CSR_MCOUNTERMASK_M 0x7d1
> +#define CSR_MCOUNTERMASK_S 0x7d2
> +#define CSR_MCOUNTERMASK_U 0x7d3
> +#define CSR_MCOUNTEROVF 0x7d4
> +
> +/* Enhanced CLIC CSRs */
> +#define CSR_MIRQ_ENTRY 0x7ec
> +#define CSR_MINTSEL_JAL 0x7ed
> +#define CSR_PUSHMCAUSE 0x7ee
> +#define CSR_PUSHMEPC 0x7ef
> +#define CSR_PUSHMXSTATUS 0x7eb
> +
> +/* Andes Physical Memory Attribute(PMA) CSRs */
> +#define CSR_PMACFG0 0xbc0
> +#define CSR_PMACFG1 0xbc1
> +#define CSR_PMACFG2 0xbc2
> +#define CSR_PMACFG3 0xbc3
> +#define CSR_PMAADDR0 0xbd0
> +#define CSR_PMAADDR1 0xbd1
> +#define CSR_PMAADDR2 0xbd2
> +#define CSR_PMAADDR3 0xbd2
> +#define CSR_PMAADDR4 0xbd4
> +#define CSR_PMAADDR5 0xbd5
> +#define CSR_PMAADDR6 0xbd6
> +#define CSR_PMAADDR7 0xbd7
> +#define CSR_PMAADDR8 0xbd8
> +#define CSR_PMAADDR9 0xbd9
> +#define CSR_PMAADDR10 0xbda
> +#define CSR_PMAADDR11 0xbdb
> +#define CSR_PMAADDR12 0xbdc
> +#define CSR_PMAADDR13 0xbdd
> +#define CSR_PMAADDR14 0xbde
> +#define CSR_PMAADDR15 0xbdf
> +
> +/* ========= AndeStar V5 supervisor mode CSRs ========= */
> +/* Supervisor trap registers */
> +#define CSR_SLIE 0x9c4
> +#define CSR_SLIP 0x9c5
> +#define CSR_SDCAUSE 0x9c9
> +
> +/* Supervisor counter registers */
> +#define CSR_SCOUNTERINTEN 0x9cf
> +#define CSR_SCOUNTERMASK_M 0x9d1
> +#define CSR_SCOUNTERMASK_S 0x9d2
> +#define CSR_SCOUNTERMASK_U 0x9d3
> +#define CSR_SCOUNTEROVF 0x9d4
> +#define CSR_SCOUNTINHIBIT 0x9e0
> +#define CSR_SHPMEVENT3 0x9e3
> +#define CSR_SHPMEVENT4 0x9e4
> +#define CSR_SHPMEVENT5 0x9e5
> +#define CSR_SHPMEVENT6 0x9e6
> +
> +/* Supervisor control registers */
> +#define CSR_SCCTLDATA 0x9cd
> +#define CSR_SMISC_CTL 0x9d0
> +
> +/* ========= AndeStar V5 user mode CSRs ========= */
> +/* User mode control registers */
> +#define CSR_UITB 0x800
> +#define CSR_UCODE 0x801
> +#define CSR_UDCAUSE 0x809
> +#define CSR_UCCTLBEGINADDR 0x80b
> +#define CSR_UCCTLCOMMAND 0x80c
> +#define CSR_WFE 0x810
> +#define CSR_SLEEPVALUE 0x811
> +#define CSR_TXEVT 0x812
Ideally this should be a seperate file
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index fd2e636..b81efcf 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -523,6 +523,14 @@ static int read_misa(CPURISCVState *env, int csrno, target_ulong *val)
> return 0;
> }
>
> +
> +// XXX: This is just a write stub for developing custom CSR handler,
> +// if the behavior of writting such CSR is not presentable in QEMU and doesn't
> +// affect the functionality, just stub it.
> +static int write_stub(CPURISCVState *env, int csrno, target_ulong val) {
> + return 0;
> +}
Is this needed?
> +
> static int write_misa(CPURISCVState *env, int csrno, target_ulong val)
> {
> if (!riscv_feature(env, RISCV_FEATURE_MISA)) {
> @@ -1264,6 +1272,76 @@ static int write_pmpaddr(CPURISCVState *env, int csrno, target_ulong val)
>
> #endif
>
> +
> +/* Custom CSR related routines and data structures */
> +
> +gpointer is_custom_csr(int);
Just make this function static instead of having a declaration in the C file.
> +
> +gpointer is_custom_csr(int csrno) {
> + gpointer ret;
> + ret = g_hash_table_lookup(custom_csr_map, GINT_TO_POINTER(csrno));
> + return ret;
> + }
> +
> +int try_handle_custom_csr(CPURISCVState *env, int csrno,
> + target_ulong *ret_value, target_ulong new_value, target_ulong write_mask,
> + riscv_csr_operations *opset);
> +
> +// XXX: This part is mainly duplicate from riscv_csrrw, we need to redo the logic
> +int try_handle_custom_csr(CPURISCVState *env, int csrno,
> + target_ulong *ret_value, target_ulong new_value, target_ulong write_mask,
> + riscv_csr_operations *opset) {
> +
> + int ret = 0;
> + target_ulong old_value;
> +
> + /* check predicate */
> + if (!opset->predicate) {
> + return -RISCV_EXCP_ILLEGAL_INST;
> + }
> + ret = opset->predicate(env, csrno);
> + if (ret < 0) {
> + return ret;
> + }
> +
> + /* execute combined read/write operation if it exists */
> + if (opset->op) {
> + return opset->op(env, csrno, ret_value, new_value, write_mask);
> + }
> +
> + /* if no accessor exists then return failure */
> + if (!opset->read) {
> + return -RISCV_EXCP_ILLEGAL_INST;
> + }
> +
> + /* read old value */
> + ret = opset->read(env, csrno, &old_value);
> + if (ret < 0) {
> + return ret;
> + }
> +
> + /* write value if writable and write mask set, otherwise drop writes */
> + if (write_mask) {
> + new_value = (old_value & ~write_mask) | (new_value & write_mask);
> + if (opset->write) {
> + ret = opset->write(env, csrno, new_value);
> + if (ret < 0) {
> + return ret;
> + }
> + }
> + }
> +
> + /* return old value */
> + if (ret_value) {
> + *ret_value = old_value;
> + }
> +
> + return 0;
> +
> +
> +
> + }
It would be nice if we could reuse the existing CSR access function.
Could we make the current one more generic and just call it?
> +
> /*
> * riscv_csrrw - read and/or update control and status register
> *
> @@ -1283,7 +1361,7 @@ int riscv_csrrw(CPURISCVState *env, int csrno, target_ulong *ret_value,
> /* check privileges and return -1 if check fails */
> #if !defined(CONFIG_USER_ONLY)
> int effective_priv = env->priv;
> - int read_only = get_field(csrno, 0xC00) == 3;
> + /* int read_only = get_field(csrno, 0xC00) == 3; */
Don't comment this out.
>
> if (riscv_has_ext(env, RVH) &&
> env->priv == PRV_S &&
> @@ -1296,10 +1374,12 @@ int riscv_csrrw(CPURISCVState *env, int csrno, target_ulong *ret_value,
> effective_priv++;
> }
>
> - if ((write_mask && read_only) ||
> - (!env->debugger && (effective_priv < get_field(csrno, 0x300)))) {
> - return -RISCV_EXCP_ILLEGAL_INST;
> - }
> + /*
> + * if ((write_mask && read_only) ||
> + * (!env->debugger && (effective_priv < get_field(csrno, 0x300)))) {
> + * return -RISCV_EXCP_ILLEGAL_INST;
> + * }
> + */
> #endif
>
> /* ensure the CSR extension is enabled. */
> @@ -1307,6 +1387,12 @@ int riscv_csrrw(CPURISCVState *env, int csrno, target_ulong *ret_value,
> return -RISCV_EXCP_ILLEGAL_INST;
> }
>
> + /* try handle_custom_csr */
> + riscv_csr_operations *custom_csr_opset = (riscv_csr_operations *) is_custom_csr(csrno);
> + if(NULL != custom_csr_opset) {
> + return try_handle_custom_csr(env, csrno, ret_value, new_value, write_mask, custom_csr_opset);
> + }
> +
> /* check predicate */
> if (!csr_ops[csrno].predicate) {
> return -RISCV_EXCP_ILLEGAL_INST;
> @@ -1351,6 +1437,14 @@ int riscv_csrrw(CPURISCVState *env, int csrno, target_ulong *ret_value,
> return 0;
> }
>
> +/* Andes Custom Registers */
> +static int read_mmsc_cfg(CPURISCVState *env, int csrno, target_ulong *val)
> +{
> + /* enable pma probe */
> + *val = 0x40000000;
> + return 0;
> +}
> +
> /*
> * Debugger support. If not in user mode, set env->debugger before the
> * riscv_csrrw call and clear it after the call.
> @@ -1369,6 +1463,8 @@ int riscv_csrrw_debug(CPURISCVState *env, int csrno, target_ulong *ret_value,
> return ret;
> }
>
> +#include "csr_andes.inc.c"
This doesn't seem to be used.
> +
> /* Control and Status Register function table */
> riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
> /* User Floating-Point CSRs */
> @@ -1645,3 +1741,4 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
> [CSR_MHPMCOUNTER31H] = { "mhpmcounter31h", any32, read_zero },
> #endif /* !CONFIG_USER_ONLY */
> };
> +
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH V2 0/2] Proposing custom CSR handling logic
2021-05-12 6:03 ` Alistair Francis
@ 2021-05-12 14:47 ` Bin Meng
-1 siblings, 0 replies; 12+ messages in thread
From: Bin Meng @ 2021-05-12 14:47 UTC (permalink / raw)
To: Alistair Francis
Cc: Dylan Jhong, open list:RISC-V, Alan Quey-Liang Kao((((((((((),
wangjunqiang, Bin Meng, qemu-devel@nongnu.org Developers,
Ruinland Chuan-Tzu Tsai
On Wed, May 12, 2021 at 2:03 PM Alistair Francis <alistair23@gmail.com> wrote:
>
> On Tue, May 11, 2021 at 8:07 PM Ruinland Chuan-Tzu Tsai
> <ruinland@andestech.com> wrote:
> >
> > Hi all,
> >
> > My sincere apology that I missed the patch to include our own CSR table
> > into the patch series and there were plenty of typos.
> > Thus I'm sending out V2 of these tiny patches.
> >
> > I agree with Alistair's comment on not introducing intrusive code which
> > will interfere the generic code structure. Yet since there are
> > possibilities that some custom CSRs/instructions could be once drafted/
> > proposed by vendors at first, and made themselves into the standard
> > as the implementation become widely adopted.
> >
> > So in this patch set, we humbly utilzed a glib hash table for inserting
> > the `struct riscv_custom_csr_operations`, check if the CSR is a non
> > standard one, and then proceed the desired behavior.
> >
> > Once the non-standard CSRs make themselves into the specification,
> > people could easily plug-and-use the code into CSR operation table
> > inside `csr.c`.
> >
> > Ones may have concerns regarding the check code would introduce
> > further overhead. For those considerations, I guess it could be solved
> > by introducing a build option such as '--enable-riscv-vendor-features'
> > to toggle the code.
> >
> > Cordially yours,
> > Ruinland ChuanTzu Tsai
> >
> > Ruinland Chuan-Tzu Tsai (2):
> > Adding premliminary support for custom CSR handling mechanism
> > Adding custom Andes CSR table.
>
> Thanks for the patches.
>
> Can you please include:
> wangjunqiang@iscas.ac.cn
> qemu-devel@nongnu.org
> bin.meng@windriver.com
>
> on future patches so everyone is included.
Thanks Alistair!
>
> >
> > target/riscv/cpu.c | 28 ++++++++
> > target/riscv/cpu.h | 12 +++-
> > target/riscv/cpu_bits.h | 115 ++++++++++++++++++++++++++++++++
> > target/riscv/csr.c | 107 ++++++++++++++++++++++++++++--
> > target/riscv/csr_andes.inc.c | 125 +++++++++++++++++++++++++++++++++++
> > 5 files changed, 381 insertions(+), 6 deletions(-)
I didn't see the original patch set in the ML, nor does it show up on
patchwork. I wonder is it posted on the ML?
Regards,
Bin
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH V2 0/2] Proposing custom CSR handling logic
@ 2021-05-12 14:47 ` Bin Meng
0 siblings, 0 replies; 12+ messages in thread
From: Bin Meng @ 2021-05-12 14:47 UTC (permalink / raw)
To: Alistair Francis
Cc: Ruinland Chuan-Tzu Tsai, wangjunqiang,
qemu-devel@nongnu.org Developers, Bin Meng, Dylan Jhong,
open list:RISC-V, Alan Quey-Liang Kao(((((((((()
On Wed, May 12, 2021 at 2:03 PM Alistair Francis <alistair23@gmail.com> wrote:
>
> On Tue, May 11, 2021 at 8:07 PM Ruinland Chuan-Tzu Tsai
> <ruinland@andestech.com> wrote:
> >
> > Hi all,
> >
> > My sincere apology that I missed the patch to include our own CSR table
> > into the patch series and there were plenty of typos.
> > Thus I'm sending out V2 of these tiny patches.
> >
> > I agree with Alistair's comment on not introducing intrusive code which
> > will interfere the generic code structure. Yet since there are
> > possibilities that some custom CSRs/instructions could be once drafted/
> > proposed by vendors at first, and made themselves into the standard
> > as the implementation become widely adopted.
> >
> > So in this patch set, we humbly utilzed a glib hash table for inserting
> > the `struct riscv_custom_csr_operations`, check if the CSR is a non
> > standard one, and then proceed the desired behavior.
> >
> > Once the non-standard CSRs make themselves into the specification,
> > people could easily plug-and-use the code into CSR operation table
> > inside `csr.c`.
> >
> > Ones may have concerns regarding the check code would introduce
> > further overhead. For those considerations, I guess it could be solved
> > by introducing a build option such as '--enable-riscv-vendor-features'
> > to toggle the code.
> >
> > Cordially yours,
> > Ruinland ChuanTzu Tsai
> >
> > Ruinland Chuan-Tzu Tsai (2):
> > Adding premliminary support for custom CSR handling mechanism
> > Adding custom Andes CSR table.
>
> Thanks for the patches.
>
> Can you please include:
> wangjunqiang@iscas.ac.cn
> qemu-devel@nongnu.org
> bin.meng@windriver.com
>
> on future patches so everyone is included.
Thanks Alistair!
>
> >
> > target/riscv/cpu.c | 28 ++++++++
> > target/riscv/cpu.h | 12 +++-
> > target/riscv/cpu_bits.h | 115 ++++++++++++++++++++++++++++++++
> > target/riscv/csr.c | 107 ++++++++++++++++++++++++++++--
> > target/riscv/csr_andes.inc.c | 125 +++++++++++++++++++++++++++++++++++
> > 5 files changed, 381 insertions(+), 6 deletions(-)
I didn't see the original patch set in the ML, nor does it show up on
patchwork. I wonder is it posted on the ML?
Regards,
Bin
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH V2 0/2] Proposing custom CSR handling logic
2021-05-12 14:47 ` Bin Meng
@ 2021-05-12 23:40 ` Alistair Francis
-1 siblings, 0 replies; 12+ messages in thread
From: Alistair Francis @ 2021-05-12 23:40 UTC (permalink / raw)
To: Bin Meng
Cc: Dylan Jhong, open list:RISC-V, Alan Quey-Liang Kao((((((((((),
wangjunqiang, Bin Meng, qemu-devel@nongnu.org Developers,
Ruinland Chuan-Tzu Tsai
On Thu, May 13, 2021 at 12:47 AM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> On Wed, May 12, 2021 at 2:03 PM Alistair Francis <alistair23@gmail.com> wrote:
> >
> > On Tue, May 11, 2021 at 8:07 PM Ruinland Chuan-Tzu Tsai
> > <ruinland@andestech.com> wrote:
> > >
> > > Hi all,
> > >
> > > My sincere apology that I missed the patch to include our own CSR table
> > > into the patch series and there were plenty of typos.
> > > Thus I'm sending out V2 of these tiny patches.
> > >
> > > I agree with Alistair's comment on not introducing intrusive code which
> > > will interfere the generic code structure. Yet since there are
> > > possibilities that some custom CSRs/instructions could be once drafted/
> > > proposed by vendors at first, and made themselves into the standard
> > > as the implementation become widely adopted.
> > >
> > > So in this patch set, we humbly utilzed a glib hash table for inserting
> > > the `struct riscv_custom_csr_operations`, check if the CSR is a non
> > > standard one, and then proceed the desired behavior.
> > >
> > > Once the non-standard CSRs make themselves into the specification,
> > > people could easily plug-and-use the code into CSR operation table
> > > inside `csr.c`.
> > >
> > > Ones may have concerns regarding the check code would introduce
> > > further overhead. For those considerations, I guess it could be solved
> > > by introducing a build option such as '--enable-riscv-vendor-features'
> > > to toggle the code.
> > >
> > > Cordially yours,
> > > Ruinland ChuanTzu Tsai
> > >
> > > Ruinland Chuan-Tzu Tsai (2):
> > > Adding premliminary support for custom CSR handling mechanism
> > > Adding custom Andes CSR table.
> >
> > Thanks for the patches.
> >
> > Can you please include:
> > wangjunqiang@iscas.ac.cn
> > qemu-devel@nongnu.org
> > bin.meng@windriver.com
> >
> > on future patches so everyone is included.
>
> Thanks Alistair!
>
> >
> > >
> > > target/riscv/cpu.c | 28 ++++++++
> > > target/riscv/cpu.h | 12 +++-
> > > target/riscv/cpu_bits.h | 115 ++++++++++++++++++++++++++++++++
> > > target/riscv/csr.c | 107 ++++++++++++++++++++++++++++--
> > > target/riscv/csr_andes.inc.c | 125 +++++++++++++++++++++++++++++++++++
> > > 5 files changed, 381 insertions(+), 6 deletions(-)
>
> I didn't see the original patch set in the ML, nor does it show up on
> patchwork. I wonder is it posted on the ML?
It might not have. It is an early version and needs some more work,
but it does seem to be on the right track. I left a few comments but
hopefully the next version will make it to the mailing list.
I will CC you on the response so you can see.
Alistair
>
> Regards,
> Bin
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH V2 0/2] Proposing custom CSR handling logic
@ 2021-05-12 23:40 ` Alistair Francis
0 siblings, 0 replies; 12+ messages in thread
From: Alistair Francis @ 2021-05-12 23:40 UTC (permalink / raw)
To: Bin Meng
Cc: Ruinland Chuan-Tzu Tsai, wangjunqiang,
qemu-devel@nongnu.org Developers, Bin Meng, Dylan Jhong,
open list:RISC-V, Alan Quey-Liang Kao(((((((((()
On Thu, May 13, 2021 at 12:47 AM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> On Wed, May 12, 2021 at 2:03 PM Alistair Francis <alistair23@gmail.com> wrote:
> >
> > On Tue, May 11, 2021 at 8:07 PM Ruinland Chuan-Tzu Tsai
> > <ruinland@andestech.com> wrote:
> > >
> > > Hi all,
> > >
> > > My sincere apology that I missed the patch to include our own CSR table
> > > into the patch series and there were plenty of typos.
> > > Thus I'm sending out V2 of these tiny patches.
> > >
> > > I agree with Alistair's comment on not introducing intrusive code which
> > > will interfere the generic code structure. Yet since there are
> > > possibilities that some custom CSRs/instructions could be once drafted/
> > > proposed by vendors at first, and made themselves into the standard
> > > as the implementation become widely adopted.
> > >
> > > So in this patch set, we humbly utilzed a glib hash table for inserting
> > > the `struct riscv_custom_csr_operations`, check if the CSR is a non
> > > standard one, and then proceed the desired behavior.
> > >
> > > Once the non-standard CSRs make themselves into the specification,
> > > people could easily plug-and-use the code into CSR operation table
> > > inside `csr.c`.
> > >
> > > Ones may have concerns regarding the check code would introduce
> > > further overhead. For those considerations, I guess it could be solved
> > > by introducing a build option such as '--enable-riscv-vendor-features'
> > > to toggle the code.
> > >
> > > Cordially yours,
> > > Ruinland ChuanTzu Tsai
> > >
> > > Ruinland Chuan-Tzu Tsai (2):
> > > Adding premliminary support for custom CSR handling mechanism
> > > Adding custom Andes CSR table.
> >
> > Thanks for the patches.
> >
> > Can you please include:
> > wangjunqiang@iscas.ac.cn
> > qemu-devel@nongnu.org
> > bin.meng@windriver.com
> >
> > on future patches so everyone is included.
>
> Thanks Alistair!
>
> >
> > >
> > > target/riscv/cpu.c | 28 ++++++++
> > > target/riscv/cpu.h | 12 +++-
> > > target/riscv/cpu_bits.h | 115 ++++++++++++++++++++++++++++++++
> > > target/riscv/csr.c | 107 ++++++++++++++++++++++++++++--
> > > target/riscv/csr_andes.inc.c | 125 +++++++++++++++++++++++++++++++++++
> > > 5 files changed, 381 insertions(+), 6 deletions(-)
>
> I didn't see the original patch set in the ML, nor does it show up on
> patchwork. I wonder is it posted on the ML?
It might not have. It is an early version and needs some more work,
but it does seem to be on the right track. I left a few comments but
hopefully the next version will make it to the mailing list.
I will CC you on the response so you can see.
Alistair
>
> Regards,
> Bin
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH V2 1/2] Adding premliminary support for custom CSR handling mechanism
2021-05-12 6:16 ` Alistair Francis
@ 2021-05-12 23:41 ` Alistair Francis
0 siblings, 0 replies; 12+ messages in thread
From: Alistair Francis @ 2021-05-12 23:41 UTC (permalink / raw)
To: Ruinland Chuan-Tzu Tsai, wangjunqiang,
qemu-devel@nongnu.org Developers, Bin Meng
Cc: Dylan Jhong, open list:RISC-V, Alan Quey-Liang Kao(((((((((()
On Wed, May 12, 2021 at 4:16 PM Alistair Francis <alistair23@gmail.com> wrote:
>
> On Tue, May 11, 2021 at 8:07 PM Ruinland Chuan-Tzu Tsai
> <ruinland@andestech.com> wrote:
> >
> > Introduce ax25 and custom CSR handling mechanism to RISC-V platform.
> > This is just a POC in which we add Andes custom CSR table directly
> > into the generic code which is undresiable and requires overhaul.
> >
> > Signed-off-by: Dylan Jhong <dylan@andestech.com>
+ Others so everyone knows what is going on.
Alistair
>
> Thanks for the patch.
>
> This seems like a good start. I have left some comments inline.
>
> Why do we need a hash table? Couldn't we just have a second
> riscv_csr_operations array?
>
> > ---
> > target/riscv/cpu.c | 28 ++++++++++
> > target/riscv/cpu.h | 12 ++++-
> > target/riscv/cpu_bits.h | 115 ++++++++++++++++++++++++++++++++++++++++
> > target/riscv/csr.c | 107 +++++++++++++++++++++++++++++++++++--
> > 4 files changed, 256 insertions(+), 6 deletions(-)
> >
> > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> > index 7401325..6dbe9d9 100644
> > --- a/target/riscv/cpu.c
> > +++ b/target/riscv/cpu.c
> > @@ -34,6 +34,8 @@
> >
> > static const char riscv_exts[26] = "IEMAFDQCLBJTPVNSUHKORWXYZG";
> >
> > +GHashTable * custom_csr_map;
>
> This should be part of CPURISCVState instead of a global
>
> > +
> > const char * const riscv_int_regnames[] = {
> > "x0/zero", "x1/ra", "x2/sp", "x3/gp", "x4/tp", "x5/t0", "x6/t1",
> > "x7/t2", "x8/s0", "x9/s1", "x10/a0", "x11/a1", "x12/a2", "x13/a3",
> > @@ -159,6 +161,31 @@ static void rv64_base_cpu_init(Object *obj)
> > set_misa(env, RV64);
> > }
> >
> > +static void ax25_cpu_init(Object *obj)
> > +{
> > + CPURISCVState *env = &RISCV_CPU(obj)->env;
> > + set_misa(env, RV64 | RVI | RVM | RVA | RVF | RVD | RVC | RVS | RVU);
> > + set_priv_version(env, PRIV_VERSION_1_10_0);
> > +
> > + /* setup custom csr handler hash table */
> > + setup_custom_csr();
> > +
> > +}
> > +
> > +void setup_custom_csr(void) {
> > + custom_csr_map = g_hash_table_new_full(g_direct_hash, g_direct_equal, NULL, NULL);
> > + int i;
> > + for (i = 0; i < MAX_CUSTOM_CSR_NUM; i++) {
> > + if (andes_custom_csr_table[i].csrno != 0) {
> > + g_hash_table_insert(custom_csr_map,
> > + GINT_TO_POINTER(andes_custom_csr_table[i].csrno),
> > + &andes_custom_csr_table[i].csr_opset);
> > + } else {
> > + break;
> > + }
> > + }
> > +}
> > +
> > static void rv64_sifive_u_cpu_init(Object *obj)
> > {
> > CPURISCVState *env = &RISCV_CPU(obj)->env;
> > @@ -705,6 +732,7 @@ static const TypeInfo riscv_cpu_type_infos[] = {
> > DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_U34, rv32_sifive_u_cpu_init),
> > #elif defined(TARGET_RISCV64)
> > DEFINE_CPU(TYPE_RISCV_CPU_BASE64, rv64_base_cpu_init),
> > + DEFINE_CPU(TYPE_RISCV_CPU_AX25, ax25_cpu_init),
>
> Let's add the CPU in a separate patch.
>
> > DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_E51, rv64_sifive_e_cpu_init),
> > DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_U54, rv64_sifive_u_cpu_init),
> > #endif
> > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> > index 0edb282..a2f656c 100644
> > --- a/target/riscv/cpu.h
> > +++ b/target/riscv/cpu.h
> > @@ -37,6 +37,7 @@
> > #define TYPE_RISCV_CPU_ANY RISCV_CPU_TYPE_NAME("any")
> > #define TYPE_RISCV_CPU_BASE32 RISCV_CPU_TYPE_NAME("rv32")
> > #define TYPE_RISCV_CPU_BASE64 RISCV_CPU_TYPE_NAME("rv64")
> > +#define TYPE_RISCV_CPU_AX25 RISCV_CPU_TYPE_NAME("andes-ax25")
> > #define TYPE_RISCV_CPU_IBEX RISCV_CPU_TYPE_NAME("lowrisc-ibex")
> > #define TYPE_RISCV_CPU_SIFIVE_E31 RISCV_CPU_TYPE_NAME("sifive-e31")
> > #define TYPE_RISCV_CPU_SIFIVE_E34 RISCV_CPU_TYPE_NAME("sifive-e34")
> > @@ -485,16 +486,25 @@ typedef struct {
> > riscv_csr_op_fn op;
> > } riscv_csr_operations;
> >
> > +typedef struct {
> > + int csrno;
> > + riscv_csr_operations csr_opset;
> > + } riscv_custom_csr_operations;
> > +
> > /* CSR function table constants */
> > enum {
> > - CSR_TABLE_SIZE = 0x1000
> > + CSR_TABLE_SIZE = 0x1000,
> > + MAX_CUSTOM_CSR_NUM = 100
> > };
> >
> > /* CSR function table */
> > +extern riscv_custom_csr_operations andes_custom_csr_table[MAX_CUSTOM_CSR_NUM];
> > extern riscv_csr_operations csr_ops[CSR_TABLE_SIZE];
> > +extern GHashTable *custom_csr_map;
> >
> > void riscv_get_csr_ops(int csrno, riscv_csr_operations *ops);
> > void riscv_set_csr_ops(int csrno, riscv_csr_operations *ops);
> > +void setup_custom_csr(void);
> >
> > void riscv_cpu_register_gdb_regs_for_features(CPUState *cs);
> >
> > diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
> > index caf4599..639bc0a 100644
> > --- a/target/riscv/cpu_bits.h
> > +++ b/target/riscv/cpu_bits.h
> > @@ -259,6 +259,7 @@
> > #define CSR_TDATA1 0x7a1
> > #define CSR_TDATA2 0x7a2
> > #define CSR_TDATA3 0x7a3
> > +#define CSR_TINFO 0x7a4
>
> Why add this?
>
> >
> > /* Debug Mode Registers */
> > #define CSR_DCSR 0x7b0
> > @@ -593,3 +594,117 @@
> > #define MIE_SSIE (1 << IRQ_S_SOFT)
> > #define MIE_USIE (1 << IRQ_U_SOFT)
> > #endif
> > +
> > +/* ========= AndeStar V5 machine mode CSRs ========= */
> > +/* Configuration Registers */
> > +#define CSR_MICM_CFG 0xfc0
> > +#define CSR_MDCM_CFG 0xfc1
> > +#define CSR_MMSC_CFG 0xfc2
> > +#define CSR_MMSC_CFG2 0xfc3
> > +#define CSR_MVEC_CFG 0xfc7
> > +
> > +/* Crash Debug CSRs */
> > +#define CSR_MCRASH_STATESAVE 0xfc8
> > +#define CSR_MSTATUS_CRASHSAVE 0xfc9
> > +
> > +/* Memory CSRs */
> > +#define CSR_MILMB 0x7c0
> > +#define CSR_MDLMB 0x7c1
> > +#define CSR_MECC_CODE 0x7C2
> > +#define CSR_MNVEC 0x7c3
> > +#define CSR_MCACHE_CTL 0x7ca
> > +#define CSR_MCCTLBEGINADDR 0x7cb
> > +#define CSR_MCCTLCOMMAND 0x7cc
> > +#define CSR_MCCTLDATA 0x7cd
> > +#define CSR_MPPIB 0x7f0
> > +#define CSR_MFIOB 0x7f1
> > +
> > +/* Hardware Stack Protection & Recording */
> > +#define CSR_MHSP_CTL 0x7c6
> > +#define CSR_MSP_BOUND 0x7c7
> > +#define CSR_MSP_BASE 0x7c8
> > +#define CSR_MXSTATUS 0x7c4
> > +#define CSR_MDCAUSE 0x7c9
> > +#define CSR_MSLIDELEG 0x7d5
> > +#define CSR_MSAVESTATUS 0x7d6
> > +#define CSR_MSAVEEPC1 0x7d7
> > +#define CSR_MSAVECAUSE1 0x7d8
> > +#define CSR_MSAVEEPC2 0x7d9
> > +#define CSR_MSAVECAUSE2 0x7da
> > +#define CSR_MSAVEDCAUSE1 0x7db
> > +#define CSR_MSAVEDCAUSE2 0x7dc
> > +
> > +/* Control CSRs */
> > +#define CSR_MPFT_CTL 0x7c5
> > +#define CSR_MMISC_CTL 0x7d0
> > +#define CSR_MCLK_CTL 0x7df
> > +
> > +/* Counter related CSRs */
> > +#define CSR_MCOUNTERWEN 0x7ce
> > +#define CSR_MCOUNTERINTEN 0x7cf
> > +#define CSR_MCOUNTERMASK_M 0x7d1
> > +#define CSR_MCOUNTERMASK_S 0x7d2
> > +#define CSR_MCOUNTERMASK_U 0x7d3
> > +#define CSR_MCOUNTEROVF 0x7d4
> > +
> > +/* Enhanced CLIC CSRs */
> > +#define CSR_MIRQ_ENTRY 0x7ec
> > +#define CSR_MINTSEL_JAL 0x7ed
> > +#define CSR_PUSHMCAUSE 0x7ee
> > +#define CSR_PUSHMEPC 0x7ef
> > +#define CSR_PUSHMXSTATUS 0x7eb
> > +
> > +/* Andes Physical Memory Attribute(PMA) CSRs */
> > +#define CSR_PMACFG0 0xbc0
> > +#define CSR_PMACFG1 0xbc1
> > +#define CSR_PMACFG2 0xbc2
> > +#define CSR_PMACFG3 0xbc3
> > +#define CSR_PMAADDR0 0xbd0
> > +#define CSR_PMAADDR1 0xbd1
> > +#define CSR_PMAADDR2 0xbd2
> > +#define CSR_PMAADDR3 0xbd2
> > +#define CSR_PMAADDR4 0xbd4
> > +#define CSR_PMAADDR5 0xbd5
> > +#define CSR_PMAADDR6 0xbd6
> > +#define CSR_PMAADDR7 0xbd7
> > +#define CSR_PMAADDR8 0xbd8
> > +#define CSR_PMAADDR9 0xbd9
> > +#define CSR_PMAADDR10 0xbda
> > +#define CSR_PMAADDR11 0xbdb
> > +#define CSR_PMAADDR12 0xbdc
> > +#define CSR_PMAADDR13 0xbdd
> > +#define CSR_PMAADDR14 0xbde
> > +#define CSR_PMAADDR15 0xbdf
> > +
> > +/* ========= AndeStar V5 supervisor mode CSRs ========= */
> > +/* Supervisor trap registers */
> > +#define CSR_SLIE 0x9c4
> > +#define CSR_SLIP 0x9c5
> > +#define CSR_SDCAUSE 0x9c9
> > +
> > +/* Supervisor counter registers */
> > +#define CSR_SCOUNTERINTEN 0x9cf
> > +#define CSR_SCOUNTERMASK_M 0x9d1
> > +#define CSR_SCOUNTERMASK_S 0x9d2
> > +#define CSR_SCOUNTERMASK_U 0x9d3
> > +#define CSR_SCOUNTEROVF 0x9d4
> > +#define CSR_SCOUNTINHIBIT 0x9e0
> > +#define CSR_SHPMEVENT3 0x9e3
> > +#define CSR_SHPMEVENT4 0x9e4
> > +#define CSR_SHPMEVENT5 0x9e5
> > +#define CSR_SHPMEVENT6 0x9e6
> > +
> > +/* Supervisor control registers */
> > +#define CSR_SCCTLDATA 0x9cd
> > +#define CSR_SMISC_CTL 0x9d0
> > +
> > +/* ========= AndeStar V5 user mode CSRs ========= */
> > +/* User mode control registers */
> > +#define CSR_UITB 0x800
> > +#define CSR_UCODE 0x801
> > +#define CSR_UDCAUSE 0x809
> > +#define CSR_UCCTLBEGINADDR 0x80b
> > +#define CSR_UCCTLCOMMAND 0x80c
> > +#define CSR_WFE 0x810
> > +#define CSR_SLEEPVALUE 0x811
> > +#define CSR_TXEVT 0x812
>
> Ideally this should be a seperate file
>
> > diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> > index fd2e636..b81efcf 100644
> > --- a/target/riscv/csr.c
> > +++ b/target/riscv/csr.c
> > @@ -523,6 +523,14 @@ static int read_misa(CPURISCVState *env, int csrno, target_ulong *val)
> > return 0;
> > }
> >
> > +
> > +// XXX: This is just a write stub for developing custom CSR handler,
> > +// if the behavior of writting such CSR is not presentable in QEMU and doesn't
> > +// affect the functionality, just stub it.
> > +static int write_stub(CPURISCVState *env, int csrno, target_ulong val) {
> > + return 0;
> > +}
>
> Is this needed?
>
> > +
> > static int write_misa(CPURISCVState *env, int csrno, target_ulong val)
> > {
> > if (!riscv_feature(env, RISCV_FEATURE_MISA)) {
> > @@ -1264,6 +1272,76 @@ static int write_pmpaddr(CPURISCVState *env, int csrno, target_ulong val)
> >
> > #endif
> >
> > +
> > +/* Custom CSR related routines and data structures */
> > +
> > +gpointer is_custom_csr(int);
>
> Just make this function static instead of having a declaration in the C file.
>
> > +
> > +gpointer is_custom_csr(int csrno) {
> > + gpointer ret;
> > + ret = g_hash_table_lookup(custom_csr_map, GINT_TO_POINTER(csrno));
> > + return ret;
> > + }
> > +
> > +int try_handle_custom_csr(CPURISCVState *env, int csrno,
> > + target_ulong *ret_value, target_ulong new_value, target_ulong write_mask,
> > + riscv_csr_operations *opset);
> > +
> > +// XXX: This part is mainly duplicate from riscv_csrrw, we need to redo the logic
> > +int try_handle_custom_csr(CPURISCVState *env, int csrno,
> > + target_ulong *ret_value, target_ulong new_value, target_ulong write_mask,
> > + riscv_csr_operations *opset) {
> > +
> > + int ret = 0;
> > + target_ulong old_value;
> > +
> > + /* check predicate */
> > + if (!opset->predicate) {
> > + return -RISCV_EXCP_ILLEGAL_INST;
> > + }
> > + ret = opset->predicate(env, csrno);
> > + if (ret < 0) {
> > + return ret;
> > + }
> > +
> > + /* execute combined read/write operation if it exists */
> > + if (opset->op) {
> > + return opset->op(env, csrno, ret_value, new_value, write_mask);
> > + }
> > +
> > + /* if no accessor exists then return failure */
> > + if (!opset->read) {
> > + return -RISCV_EXCP_ILLEGAL_INST;
> > + }
> > +
> > + /* read old value */
> > + ret = opset->read(env, csrno, &old_value);
> > + if (ret < 0) {
> > + return ret;
> > + }
> > +
> > + /* write value if writable and write mask set, otherwise drop writes */
> > + if (write_mask) {
> > + new_value = (old_value & ~write_mask) | (new_value & write_mask);
> > + if (opset->write) {
> > + ret = opset->write(env, csrno, new_value);
> > + if (ret < 0) {
> > + return ret;
> > + }
> > + }
> > + }
> > +
> > + /* return old value */
> > + if (ret_value) {
> > + *ret_value = old_value;
> > + }
> > +
> > + return 0;
> > +
> > +
> > +
> > + }
>
> It would be nice if we could reuse the existing CSR access function.
> Could we make the current one more generic and just call it?
>
> > +
> > /*
> > * riscv_csrrw - read and/or update control and status register
> > *
> > @@ -1283,7 +1361,7 @@ int riscv_csrrw(CPURISCVState *env, int csrno, target_ulong *ret_value,
> > /* check privileges and return -1 if check fails */
> > #if !defined(CONFIG_USER_ONLY)
> > int effective_priv = env->priv;
> > - int read_only = get_field(csrno, 0xC00) == 3;
> > + /* int read_only = get_field(csrno, 0xC00) == 3; */
>
> Don't comment this out.
>
> >
> > if (riscv_has_ext(env, RVH) &&
> > env->priv == PRV_S &&
> > @@ -1296,10 +1374,12 @@ int riscv_csrrw(CPURISCVState *env, int csrno, target_ulong *ret_value,
> > effective_priv++;
> > }
> >
> > - if ((write_mask && read_only) ||
> > - (!env->debugger && (effective_priv < get_field(csrno, 0x300)))) {
> > - return -RISCV_EXCP_ILLEGAL_INST;
> > - }
> > + /*
> > + * if ((write_mask && read_only) ||
> > + * (!env->debugger && (effective_priv < get_field(csrno, 0x300)))) {
> > + * return -RISCV_EXCP_ILLEGAL_INST;
> > + * }
> > + */
> > #endif
> >
> > /* ensure the CSR extension is enabled. */
> > @@ -1307,6 +1387,12 @@ int riscv_csrrw(CPURISCVState *env, int csrno, target_ulong *ret_value,
> > return -RISCV_EXCP_ILLEGAL_INST;
> > }
> >
> > + /* try handle_custom_csr */
> > + riscv_csr_operations *custom_csr_opset = (riscv_csr_operations *) is_custom_csr(csrno);
> > + if(NULL != custom_csr_opset) {
> > + return try_handle_custom_csr(env, csrno, ret_value, new_value, write_mask, custom_csr_opset);
> > + }
> > +
> > /* check predicate */
> > if (!csr_ops[csrno].predicate) {
> > return -RISCV_EXCP_ILLEGAL_INST;
> > @@ -1351,6 +1437,14 @@ int riscv_csrrw(CPURISCVState *env, int csrno, target_ulong *ret_value,
> > return 0;
> > }
> >
> > +/* Andes Custom Registers */
> > +static int read_mmsc_cfg(CPURISCVState *env, int csrno, target_ulong *val)
> > +{
> > + /* enable pma probe */
> > + *val = 0x40000000;
> > + return 0;
> > +}
> > +
> > /*
> > * Debugger support. If not in user mode, set env->debugger before the
> > * riscv_csrrw call and clear it after the call.
> > @@ -1369,6 +1463,8 @@ int riscv_csrrw_debug(CPURISCVState *env, int csrno, target_ulong *ret_value,
> > return ret;
> > }
> >
> > +#include "csr_andes.inc.c"
>
> This doesn't seem to be used.
>
> > +
> > /* Control and Status Register function table */
> > riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
> > /* User Floating-Point CSRs */
> > @@ -1645,3 +1741,4 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
> > [CSR_MHPMCOUNTER31H] = { "mhpmcounter31h", any32, read_zero },
> > #endif /* !CONFIG_USER_ONLY */
> > };
> > +
> > --
> > 2.17.1
> >
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH V2 1/2] Adding premliminary support for custom CSR handling mechanism
@ 2021-05-12 23:41 ` Alistair Francis
0 siblings, 0 replies; 12+ messages in thread
From: Alistair Francis @ 2021-05-12 23:41 UTC (permalink / raw)
To: Ruinland Chuan-Tzu Tsai, wangjunqiang,
qemu-devel@nongnu.org Developers, Bin Meng
Cc: open list:RISC-V, Dylan Jhong, Alan Quey-Liang Kao(((((((((()
On Wed, May 12, 2021 at 4:16 PM Alistair Francis <alistair23@gmail.com> wrote:
>
> On Tue, May 11, 2021 at 8:07 PM Ruinland Chuan-Tzu Tsai
> <ruinland@andestech.com> wrote:
> >
> > Introduce ax25 and custom CSR handling mechanism to RISC-V platform.
> > This is just a POC in which we add Andes custom CSR table directly
> > into the generic code which is undresiable and requires overhaul.
> >
> > Signed-off-by: Dylan Jhong <dylan@andestech.com>
+ Others so everyone knows what is going on.
Alistair
>
> Thanks for the patch.
>
> This seems like a good start. I have left some comments inline.
>
> Why do we need a hash table? Couldn't we just have a second
> riscv_csr_operations array?
>
> > ---
> > target/riscv/cpu.c | 28 ++++++++++
> > target/riscv/cpu.h | 12 ++++-
> > target/riscv/cpu_bits.h | 115 ++++++++++++++++++++++++++++++++++++++++
> > target/riscv/csr.c | 107 +++++++++++++++++++++++++++++++++++--
> > 4 files changed, 256 insertions(+), 6 deletions(-)
> >
> > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> > index 7401325..6dbe9d9 100644
> > --- a/target/riscv/cpu.c
> > +++ b/target/riscv/cpu.c
> > @@ -34,6 +34,8 @@
> >
> > static const char riscv_exts[26] = "IEMAFDQCLBJTPVNSUHKORWXYZG";
> >
> > +GHashTable * custom_csr_map;
>
> This should be part of CPURISCVState instead of a global
>
> > +
> > const char * const riscv_int_regnames[] = {
> > "x0/zero", "x1/ra", "x2/sp", "x3/gp", "x4/tp", "x5/t0", "x6/t1",
> > "x7/t2", "x8/s0", "x9/s1", "x10/a0", "x11/a1", "x12/a2", "x13/a3",
> > @@ -159,6 +161,31 @@ static void rv64_base_cpu_init(Object *obj)
> > set_misa(env, RV64);
> > }
> >
> > +static void ax25_cpu_init(Object *obj)
> > +{
> > + CPURISCVState *env = &RISCV_CPU(obj)->env;
> > + set_misa(env, RV64 | RVI | RVM | RVA | RVF | RVD | RVC | RVS | RVU);
> > + set_priv_version(env, PRIV_VERSION_1_10_0);
> > +
> > + /* setup custom csr handler hash table */
> > + setup_custom_csr();
> > +
> > +}
> > +
> > +void setup_custom_csr(void) {
> > + custom_csr_map = g_hash_table_new_full(g_direct_hash, g_direct_equal, NULL, NULL);
> > + int i;
> > + for (i = 0; i < MAX_CUSTOM_CSR_NUM; i++) {
> > + if (andes_custom_csr_table[i].csrno != 0) {
> > + g_hash_table_insert(custom_csr_map,
> > + GINT_TO_POINTER(andes_custom_csr_table[i].csrno),
> > + &andes_custom_csr_table[i].csr_opset);
> > + } else {
> > + break;
> > + }
> > + }
> > +}
> > +
> > static void rv64_sifive_u_cpu_init(Object *obj)
> > {
> > CPURISCVState *env = &RISCV_CPU(obj)->env;
> > @@ -705,6 +732,7 @@ static const TypeInfo riscv_cpu_type_infos[] = {
> > DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_U34, rv32_sifive_u_cpu_init),
> > #elif defined(TARGET_RISCV64)
> > DEFINE_CPU(TYPE_RISCV_CPU_BASE64, rv64_base_cpu_init),
> > + DEFINE_CPU(TYPE_RISCV_CPU_AX25, ax25_cpu_init),
>
> Let's add the CPU in a separate patch.
>
> > DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_E51, rv64_sifive_e_cpu_init),
> > DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_U54, rv64_sifive_u_cpu_init),
> > #endif
> > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> > index 0edb282..a2f656c 100644
> > --- a/target/riscv/cpu.h
> > +++ b/target/riscv/cpu.h
> > @@ -37,6 +37,7 @@
> > #define TYPE_RISCV_CPU_ANY RISCV_CPU_TYPE_NAME("any")
> > #define TYPE_RISCV_CPU_BASE32 RISCV_CPU_TYPE_NAME("rv32")
> > #define TYPE_RISCV_CPU_BASE64 RISCV_CPU_TYPE_NAME("rv64")
> > +#define TYPE_RISCV_CPU_AX25 RISCV_CPU_TYPE_NAME("andes-ax25")
> > #define TYPE_RISCV_CPU_IBEX RISCV_CPU_TYPE_NAME("lowrisc-ibex")
> > #define TYPE_RISCV_CPU_SIFIVE_E31 RISCV_CPU_TYPE_NAME("sifive-e31")
> > #define TYPE_RISCV_CPU_SIFIVE_E34 RISCV_CPU_TYPE_NAME("sifive-e34")
> > @@ -485,16 +486,25 @@ typedef struct {
> > riscv_csr_op_fn op;
> > } riscv_csr_operations;
> >
> > +typedef struct {
> > + int csrno;
> > + riscv_csr_operations csr_opset;
> > + } riscv_custom_csr_operations;
> > +
> > /* CSR function table constants */
> > enum {
> > - CSR_TABLE_SIZE = 0x1000
> > + CSR_TABLE_SIZE = 0x1000,
> > + MAX_CUSTOM_CSR_NUM = 100
> > };
> >
> > /* CSR function table */
> > +extern riscv_custom_csr_operations andes_custom_csr_table[MAX_CUSTOM_CSR_NUM];
> > extern riscv_csr_operations csr_ops[CSR_TABLE_SIZE];
> > +extern GHashTable *custom_csr_map;
> >
> > void riscv_get_csr_ops(int csrno, riscv_csr_operations *ops);
> > void riscv_set_csr_ops(int csrno, riscv_csr_operations *ops);
> > +void setup_custom_csr(void);
> >
> > void riscv_cpu_register_gdb_regs_for_features(CPUState *cs);
> >
> > diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
> > index caf4599..639bc0a 100644
> > --- a/target/riscv/cpu_bits.h
> > +++ b/target/riscv/cpu_bits.h
> > @@ -259,6 +259,7 @@
> > #define CSR_TDATA1 0x7a1
> > #define CSR_TDATA2 0x7a2
> > #define CSR_TDATA3 0x7a3
> > +#define CSR_TINFO 0x7a4
>
> Why add this?
>
> >
> > /* Debug Mode Registers */
> > #define CSR_DCSR 0x7b0
> > @@ -593,3 +594,117 @@
> > #define MIE_SSIE (1 << IRQ_S_SOFT)
> > #define MIE_USIE (1 << IRQ_U_SOFT)
> > #endif
> > +
> > +/* ========= AndeStar V5 machine mode CSRs ========= */
> > +/* Configuration Registers */
> > +#define CSR_MICM_CFG 0xfc0
> > +#define CSR_MDCM_CFG 0xfc1
> > +#define CSR_MMSC_CFG 0xfc2
> > +#define CSR_MMSC_CFG2 0xfc3
> > +#define CSR_MVEC_CFG 0xfc7
> > +
> > +/* Crash Debug CSRs */
> > +#define CSR_MCRASH_STATESAVE 0xfc8
> > +#define CSR_MSTATUS_CRASHSAVE 0xfc9
> > +
> > +/* Memory CSRs */
> > +#define CSR_MILMB 0x7c0
> > +#define CSR_MDLMB 0x7c1
> > +#define CSR_MECC_CODE 0x7C2
> > +#define CSR_MNVEC 0x7c3
> > +#define CSR_MCACHE_CTL 0x7ca
> > +#define CSR_MCCTLBEGINADDR 0x7cb
> > +#define CSR_MCCTLCOMMAND 0x7cc
> > +#define CSR_MCCTLDATA 0x7cd
> > +#define CSR_MPPIB 0x7f0
> > +#define CSR_MFIOB 0x7f1
> > +
> > +/* Hardware Stack Protection & Recording */
> > +#define CSR_MHSP_CTL 0x7c6
> > +#define CSR_MSP_BOUND 0x7c7
> > +#define CSR_MSP_BASE 0x7c8
> > +#define CSR_MXSTATUS 0x7c4
> > +#define CSR_MDCAUSE 0x7c9
> > +#define CSR_MSLIDELEG 0x7d5
> > +#define CSR_MSAVESTATUS 0x7d6
> > +#define CSR_MSAVEEPC1 0x7d7
> > +#define CSR_MSAVECAUSE1 0x7d8
> > +#define CSR_MSAVEEPC2 0x7d9
> > +#define CSR_MSAVECAUSE2 0x7da
> > +#define CSR_MSAVEDCAUSE1 0x7db
> > +#define CSR_MSAVEDCAUSE2 0x7dc
> > +
> > +/* Control CSRs */
> > +#define CSR_MPFT_CTL 0x7c5
> > +#define CSR_MMISC_CTL 0x7d0
> > +#define CSR_MCLK_CTL 0x7df
> > +
> > +/* Counter related CSRs */
> > +#define CSR_MCOUNTERWEN 0x7ce
> > +#define CSR_MCOUNTERINTEN 0x7cf
> > +#define CSR_MCOUNTERMASK_M 0x7d1
> > +#define CSR_MCOUNTERMASK_S 0x7d2
> > +#define CSR_MCOUNTERMASK_U 0x7d3
> > +#define CSR_MCOUNTEROVF 0x7d4
> > +
> > +/* Enhanced CLIC CSRs */
> > +#define CSR_MIRQ_ENTRY 0x7ec
> > +#define CSR_MINTSEL_JAL 0x7ed
> > +#define CSR_PUSHMCAUSE 0x7ee
> > +#define CSR_PUSHMEPC 0x7ef
> > +#define CSR_PUSHMXSTATUS 0x7eb
> > +
> > +/* Andes Physical Memory Attribute(PMA) CSRs */
> > +#define CSR_PMACFG0 0xbc0
> > +#define CSR_PMACFG1 0xbc1
> > +#define CSR_PMACFG2 0xbc2
> > +#define CSR_PMACFG3 0xbc3
> > +#define CSR_PMAADDR0 0xbd0
> > +#define CSR_PMAADDR1 0xbd1
> > +#define CSR_PMAADDR2 0xbd2
> > +#define CSR_PMAADDR3 0xbd2
> > +#define CSR_PMAADDR4 0xbd4
> > +#define CSR_PMAADDR5 0xbd5
> > +#define CSR_PMAADDR6 0xbd6
> > +#define CSR_PMAADDR7 0xbd7
> > +#define CSR_PMAADDR8 0xbd8
> > +#define CSR_PMAADDR9 0xbd9
> > +#define CSR_PMAADDR10 0xbda
> > +#define CSR_PMAADDR11 0xbdb
> > +#define CSR_PMAADDR12 0xbdc
> > +#define CSR_PMAADDR13 0xbdd
> > +#define CSR_PMAADDR14 0xbde
> > +#define CSR_PMAADDR15 0xbdf
> > +
> > +/* ========= AndeStar V5 supervisor mode CSRs ========= */
> > +/* Supervisor trap registers */
> > +#define CSR_SLIE 0x9c4
> > +#define CSR_SLIP 0x9c5
> > +#define CSR_SDCAUSE 0x9c9
> > +
> > +/* Supervisor counter registers */
> > +#define CSR_SCOUNTERINTEN 0x9cf
> > +#define CSR_SCOUNTERMASK_M 0x9d1
> > +#define CSR_SCOUNTERMASK_S 0x9d2
> > +#define CSR_SCOUNTERMASK_U 0x9d3
> > +#define CSR_SCOUNTEROVF 0x9d4
> > +#define CSR_SCOUNTINHIBIT 0x9e0
> > +#define CSR_SHPMEVENT3 0x9e3
> > +#define CSR_SHPMEVENT4 0x9e4
> > +#define CSR_SHPMEVENT5 0x9e5
> > +#define CSR_SHPMEVENT6 0x9e6
> > +
> > +/* Supervisor control registers */
> > +#define CSR_SCCTLDATA 0x9cd
> > +#define CSR_SMISC_CTL 0x9d0
> > +
> > +/* ========= AndeStar V5 user mode CSRs ========= */
> > +/* User mode control registers */
> > +#define CSR_UITB 0x800
> > +#define CSR_UCODE 0x801
> > +#define CSR_UDCAUSE 0x809
> > +#define CSR_UCCTLBEGINADDR 0x80b
> > +#define CSR_UCCTLCOMMAND 0x80c
> > +#define CSR_WFE 0x810
> > +#define CSR_SLEEPVALUE 0x811
> > +#define CSR_TXEVT 0x812
>
> Ideally this should be a seperate file
>
> > diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> > index fd2e636..b81efcf 100644
> > --- a/target/riscv/csr.c
> > +++ b/target/riscv/csr.c
> > @@ -523,6 +523,14 @@ static int read_misa(CPURISCVState *env, int csrno, target_ulong *val)
> > return 0;
> > }
> >
> > +
> > +// XXX: This is just a write stub for developing custom CSR handler,
> > +// if the behavior of writting such CSR is not presentable in QEMU and doesn't
> > +// affect the functionality, just stub it.
> > +static int write_stub(CPURISCVState *env, int csrno, target_ulong val) {
> > + return 0;
> > +}
>
> Is this needed?
>
> > +
> > static int write_misa(CPURISCVState *env, int csrno, target_ulong val)
> > {
> > if (!riscv_feature(env, RISCV_FEATURE_MISA)) {
> > @@ -1264,6 +1272,76 @@ static int write_pmpaddr(CPURISCVState *env, int csrno, target_ulong val)
> >
> > #endif
> >
> > +
> > +/* Custom CSR related routines and data structures */
> > +
> > +gpointer is_custom_csr(int);
>
> Just make this function static instead of having a declaration in the C file.
>
> > +
> > +gpointer is_custom_csr(int csrno) {
> > + gpointer ret;
> > + ret = g_hash_table_lookup(custom_csr_map, GINT_TO_POINTER(csrno));
> > + return ret;
> > + }
> > +
> > +int try_handle_custom_csr(CPURISCVState *env, int csrno,
> > + target_ulong *ret_value, target_ulong new_value, target_ulong write_mask,
> > + riscv_csr_operations *opset);
> > +
> > +// XXX: This part is mainly duplicate from riscv_csrrw, we need to redo the logic
> > +int try_handle_custom_csr(CPURISCVState *env, int csrno,
> > + target_ulong *ret_value, target_ulong new_value, target_ulong write_mask,
> > + riscv_csr_operations *opset) {
> > +
> > + int ret = 0;
> > + target_ulong old_value;
> > +
> > + /* check predicate */
> > + if (!opset->predicate) {
> > + return -RISCV_EXCP_ILLEGAL_INST;
> > + }
> > + ret = opset->predicate(env, csrno);
> > + if (ret < 0) {
> > + return ret;
> > + }
> > +
> > + /* execute combined read/write operation if it exists */
> > + if (opset->op) {
> > + return opset->op(env, csrno, ret_value, new_value, write_mask);
> > + }
> > +
> > + /* if no accessor exists then return failure */
> > + if (!opset->read) {
> > + return -RISCV_EXCP_ILLEGAL_INST;
> > + }
> > +
> > + /* read old value */
> > + ret = opset->read(env, csrno, &old_value);
> > + if (ret < 0) {
> > + return ret;
> > + }
> > +
> > + /* write value if writable and write mask set, otherwise drop writes */
> > + if (write_mask) {
> > + new_value = (old_value & ~write_mask) | (new_value & write_mask);
> > + if (opset->write) {
> > + ret = opset->write(env, csrno, new_value);
> > + if (ret < 0) {
> > + return ret;
> > + }
> > + }
> > + }
> > +
> > + /* return old value */
> > + if (ret_value) {
> > + *ret_value = old_value;
> > + }
> > +
> > + return 0;
> > +
> > +
> > +
> > + }
>
> It would be nice if we could reuse the existing CSR access function.
> Could we make the current one more generic and just call it?
>
> > +
> > /*
> > * riscv_csrrw - read and/or update control and status register
> > *
> > @@ -1283,7 +1361,7 @@ int riscv_csrrw(CPURISCVState *env, int csrno, target_ulong *ret_value,
> > /* check privileges and return -1 if check fails */
> > #if !defined(CONFIG_USER_ONLY)
> > int effective_priv = env->priv;
> > - int read_only = get_field(csrno, 0xC00) == 3;
> > + /* int read_only = get_field(csrno, 0xC00) == 3; */
>
> Don't comment this out.
>
> >
> > if (riscv_has_ext(env, RVH) &&
> > env->priv == PRV_S &&
> > @@ -1296,10 +1374,12 @@ int riscv_csrrw(CPURISCVState *env, int csrno, target_ulong *ret_value,
> > effective_priv++;
> > }
> >
> > - if ((write_mask && read_only) ||
> > - (!env->debugger && (effective_priv < get_field(csrno, 0x300)))) {
> > - return -RISCV_EXCP_ILLEGAL_INST;
> > - }
> > + /*
> > + * if ((write_mask && read_only) ||
> > + * (!env->debugger && (effective_priv < get_field(csrno, 0x300)))) {
> > + * return -RISCV_EXCP_ILLEGAL_INST;
> > + * }
> > + */
> > #endif
> >
> > /* ensure the CSR extension is enabled. */
> > @@ -1307,6 +1387,12 @@ int riscv_csrrw(CPURISCVState *env, int csrno, target_ulong *ret_value,
> > return -RISCV_EXCP_ILLEGAL_INST;
> > }
> >
> > + /* try handle_custom_csr */
> > + riscv_csr_operations *custom_csr_opset = (riscv_csr_operations *) is_custom_csr(csrno);
> > + if(NULL != custom_csr_opset) {
> > + return try_handle_custom_csr(env, csrno, ret_value, new_value, write_mask, custom_csr_opset);
> > + }
> > +
> > /* check predicate */
> > if (!csr_ops[csrno].predicate) {
> > return -RISCV_EXCP_ILLEGAL_INST;
> > @@ -1351,6 +1437,14 @@ int riscv_csrrw(CPURISCVState *env, int csrno, target_ulong *ret_value,
> > return 0;
> > }
> >
> > +/* Andes Custom Registers */
> > +static int read_mmsc_cfg(CPURISCVState *env, int csrno, target_ulong *val)
> > +{
> > + /* enable pma probe */
> > + *val = 0x40000000;
> > + return 0;
> > +}
> > +
> > /*
> > * Debugger support. If not in user mode, set env->debugger before the
> > * riscv_csrrw call and clear it after the call.
> > @@ -1369,6 +1463,8 @@ int riscv_csrrw_debug(CPURISCVState *env, int csrno, target_ulong *ret_value,
> > return ret;
> > }
> >
> > +#include "csr_andes.inc.c"
>
> This doesn't seem to be used.
>
> > +
> > /* Control and Status Register function table */
> > riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
> > /* User Floating-Point CSRs */
> > @@ -1645,3 +1741,4 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
> > [CSR_MHPMCOUNTER31H] = { "mhpmcounter31h", any32, read_zero },
> > #endif /* !CONFIG_USER_ONLY */
> > };
> > +
> > --
> > 2.17.1
> >
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2021-05-12 23:43 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-11 10:07 [PATCH V2 0/2] Proposing custom CSR handling logic Ruinland Chuan-Tzu Tsai
2021-05-11 10:07 ` [PATCH V2 1/2] Adding premliminary support for custom CSR handling mechanism Ruinland Chuan-Tzu Tsai
2021-05-12 6:16 ` Alistair Francis
2021-05-12 23:41 ` Alistair Francis
2021-05-12 23:41 ` Alistair Francis
2021-05-11 10:07 ` [PATCH V2 2/2] Adding custom Andes CSR table Ruinland Chuan-Tzu Tsai
2021-05-12 6:03 ` [PATCH V2 0/2] Proposing custom CSR handling logic Alistair Francis
2021-05-12 6:03 ` Alistair Francis
2021-05-12 14:47 ` Bin Meng
2021-05-12 14:47 ` Bin Meng
2021-05-12 23:40 ` Alistair Francis
2021-05-12 23:40 ` Alistair Francis
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.