* [PATCH 0/6] target/riscv: Add support for Control Transfer Records Ext.
@ 2024-05-29 16:09 Rajnesh Kanwal
2024-05-29 16:09 ` [PATCH 1/6] target/riscv: Remove obsolete sfence.vm instruction Rajnesh Kanwal
` (6 more replies)
0 siblings, 7 replies; 22+ messages in thread
From: Rajnesh Kanwal @ 2024-05-29 16:09 UTC (permalink / raw)
To: qemu-riscv, qemu-devel
Cc: alistair.francis, bin.meng, liweiwei, dbarboza, zhiwei_liu,
atishp, apatel, rkanwal, beeman, tech-control-transfer-records
This series enables Control Transfer Records extension support on riscv
platform. This extension is similar to Arch LBR in x86 and BRBE in ARM.
The Extension has been stable and the latest release can be found here [0]
CTR extension depends on couple of other extensions:
1. S[m|s]csrind : The indirect CSR extension [1] which defines additional
([M|S|VS]IREG2-[M|S|VS]IREG6) register to address size limitation of
RISC-V CSR address space. CTR access ctrsource, ctrtartget and ctrdata
CSRs using sscsrind extension.
2. Smstateen: The mstateen bit[54] controls the access to the CTR ext to
S-mode.
3. Sscofpmf: Counter overflow and privilege mode filtering. [2]
The series is based on Smcdeleg/Ssccfg counter delegation extension [3]
patches. CTR itself doesn't depend on counter delegation support. This
rebase is basically to include the Smcsrind patches.
Due to the dependency of these extensions, the following extensions must be
enabled to use the control transfer records feature.
"smstateen=true,sscofpmf=true,smcsrind=true,sscsrind=true,smctr=true,ssctr=true"
Here is the link to a quick guide [5] to setup and run a basic perf demo on
Linux to use CTR Ext.
The Qemu patches can be found here:
https://github.com/rajnesh-kanwal/qemu/tree/ctr_upstream
The opensbi patch can be found here:
https://github.com/rajnesh-kanwal/opensbi/tree/ctr_upstream
The Linux kernel patches can be found here:
https://github.com/rajnesh-kanwal/linux/tree/ctr_upstream
[0]: https://github.com/riscv/riscv-control-transfer-records/release
[1]: https://github.com/riscv/riscv-indirect-csr-access
[2]: https://github.com/riscvarchive/riscv-count-overflow/tree/main
[3]: https://github.com/riscv/riscv-smcdeleg-ssccfg
[4]: https://lore.kernel.org/all/20240217000134.3634191-1-atishp@rivosinc.com/
[5]: https://github.com/rajnesh-kanwal/linux/wiki/Running-CTR-basic-demo-on-QEMU-RISC%E2%80%90V-Virt-machine
Rajnesh Kanwal (6):
target/riscv: Remove obsolete sfence.vm instruction
target/riscv: Add Control Transfer Records CSR definitions.
target/riscv: Add support for Control Transfer Records extension CSRs.
target/riscv: Add support to record CTR entries.
target/riscv: Add CTR sctrclr instruction.
target/riscv: Add support to access ctrsource, ctrtarget, ctrdata
regs.
target/riscv/cpu.c | 4 +
target/riscv/cpu.h | 14 +
target/riscv/cpu_bits.h | 154 +++++++++
target/riscv/cpu_cfg.h | 2 +
target/riscv/cpu_helper.c | 213 ++++++++++++
target/riscv/csr.c | 312 +++++++++++++++++-
target/riscv/helper.h | 8 +-
target/riscv/insn32.decode | 2 +-
.../riscv/insn_trans/trans_privileged.c.inc | 21 +-
target/riscv/insn_trans/trans_rvi.c.inc | 27 ++
target/riscv/op_helper.c | 117 ++++++-
target/riscv/translate.c | 9 +
12 files changed, 870 insertions(+), 13 deletions(-)
--
2.34.1
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 1/6] target/riscv: Remove obsolete sfence.vm instruction
2024-05-29 16:09 [PATCH 0/6] target/riscv: Add support for Control Transfer Records Ext Rajnesh Kanwal
@ 2024-05-29 16:09 ` Rajnesh Kanwal
2024-06-05 0:46 ` Alistair Francis
2024-05-29 16:09 ` [PATCH 2/6] target/riscv: Add Control Transfer Records CSR definitions Rajnesh Kanwal
` (5 subsequent siblings)
6 siblings, 1 reply; 22+ messages in thread
From: Rajnesh Kanwal @ 2024-05-29 16:09 UTC (permalink / raw)
To: qemu-riscv, qemu-devel
Cc: alistair.francis, bin.meng, liweiwei, dbarboza, zhiwei_liu,
atishp, apatel, rkanwal, beeman, tech-control-transfer-records
Signed-off-by: Rajnesh Kanwal <rkanwal@rivosinc.com>
---
target/riscv/insn32.decode | 1 -
target/riscv/insn_trans/trans_privileged.c.inc | 5 -----
2 files changed, 6 deletions(-)
diff --git a/target/riscv/insn32.decode b/target/riscv/insn32.decode
index f22df04cfd..9cb1a1b4ec 100644
--- a/target/riscv/insn32.decode
+++ b/target/riscv/insn32.decode
@@ -112,7 +112,6 @@ sret 0001000 00010 00000 000 00000 1110011
mret 0011000 00010 00000 000 00000 1110011
wfi 0001000 00101 00000 000 00000 1110011
sfence_vma 0001001 ..... ..... 000 00000 1110011 @sfence_vma
-sfence_vm 0001000 00100 ..... 000 00000 1110011 @sfence_vm
# *** RV32I Base Instruction Set ***
lui .................... ..... 0110111 @u
diff --git a/target/riscv/insn_trans/trans_privileged.c.inc b/target/riscv/insn_trans/trans_privileged.c.inc
index bc5263a4e0..4eccdddeaa 100644
--- a/target/riscv/insn_trans/trans_privileged.c.inc
+++ b/target/riscv/insn_trans/trans_privileged.c.inc
@@ -127,8 +127,3 @@ static bool trans_sfence_vma(DisasContext *ctx, arg_sfence_vma *a)
#endif
return false;
}
-
-static bool trans_sfence_vm(DisasContext *ctx, arg_sfence_vm *a)
-{
- return false;
-}
--
2.34.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 2/6] target/riscv: Add Control Transfer Records CSR definitions.
2024-05-29 16:09 [PATCH 0/6] target/riscv: Add support for Control Transfer Records Ext Rajnesh Kanwal
2024-05-29 16:09 ` [PATCH 1/6] target/riscv: Remove obsolete sfence.vm instruction Rajnesh Kanwal
@ 2024-05-29 16:09 ` Rajnesh Kanwal
2024-05-29 16:09 ` [PATCH 3/6] target/riscv: Add support for Control Transfer Records extension CSRs Rajnesh Kanwal
` (4 subsequent siblings)
6 siblings, 0 replies; 22+ messages in thread
From: Rajnesh Kanwal @ 2024-05-29 16:09 UTC (permalink / raw)
To: qemu-riscv, qemu-devel
Cc: alistair.francis, bin.meng, liweiwei, dbarboza, zhiwei_liu,
atishp, apatel, rkanwal, beeman, tech-control-transfer-records
The Control Transfer Records (CTR) extension provides a method to
record a limited branch history in register-accessible internal chip
storage.
This extension is similar to Arch LBR in x86 and BRBE in ARM.
The Extension has been stable and the latest release can be found here
https://github.com/riscv/riscv-control-transfer-records/release
Signed-off-by: Rajnesh Kanwal <rkanwal@rivosinc.com>
---
target/riscv/cpu_bits.h | 154 ++++++++++++++++++++++++++++++++++++++++
1 file changed, 154 insertions(+)
diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
index 86e15381c8..71ddccaf1a 100644
--- a/target/riscv/cpu_bits.h
+++ b/target/riscv/cpu_bits.h
@@ -242,6 +242,17 @@
#define CSR_SIEH 0x114
#define CSR_SIPH 0x154
+/* Machine-Level Control transfer records CSRs */
+#define CSR_MCTRCTL 0x34e
+
+/* Supervisor-Level Control transfer records CSRs */
+#define CSR_SCTRCTL 0x14e
+#define CSR_SCTRSTATUS 0x14f
+#define CSR_SCTRDEPTH 0x15f
+
+/* VS-Level Control transfer records CSRs */
+#define CSR_VSCTRCTL 0x24e
+
/* Hpervisor CSRs */
#define CSR_HSTATUS 0x600
#define CSR_HEDELEG 0x602
@@ -339,6 +350,7 @@
#define SMSTATEEN0_CS (1ULL << 0)
#define SMSTATEEN0_FCSR (1ULL << 1)
#define SMSTATEEN0_JVT (1ULL << 2)
+#define SMSTATEEN0_CTR (1ULL << 54)
#define SMSTATEEN0_HSCONTXT (1ULL << 57)
#define SMSTATEEN0_IMSIC (1ULL << 58)
#define SMSTATEEN0_AIA (1ULL << 59)
@@ -854,6 +866,148 @@ typedef enum RISCVException {
#define UMTE_U_PM_INSN U_PM_INSN
#define UMTE_MASK (UMTE_U_PM_ENABLE | MMTE_U_PM_CURRENT | UMTE_U_PM_INSN)
+/* mctrctl CSR bits. */
+#define MCTRCTL_U_ENABLE BIT(0)
+#define MCTRCTL_S_ENABLE BIT(1)
+#define MCTRCTL_M_ENABLE BIT(2)
+#define MCTRCTL_RASEMU BIT(7)
+#define MCTRCTL_STE BIT(8)
+#define MCTRCTL_MTE BIT(9)
+#define MCTRCTL_BPFRZ BIT(11)
+#define MCTRCTL_LCOFIFRZ BIT(12)
+#define MCTRCTL_EXCINH BIT(33)
+#define MCTRCTL_INTRINH BIT(34)
+#define MCTRCTL_TRETINH BIT(35)
+#define MCTRCTL_NTBREN BIT(36)
+#define MCTRCTL_TKBRINH BIT(37)
+#define MCTRCTL_INDCALL_INH BIT(40)
+#define MCTRCTL_DIRCALL_INH BIT(41)
+#define MCTRCTL_INDJUMP_INH BIT(42)
+#define MCTRCTL_DIRJUMP_INH BIT(43)
+#define MCTRCTL_CORSWAP_INH BIT(44)
+#define MCTRCTL_RET_INH BIT(45)
+#define MCTRCTL_INDOJUMP_INH BIT(46)
+#define MCTRCTL_DIROJUMP_INH BIT(47)
+
+#define MCTRCTL_INH_START 32U
+
+#define MCTRCTL_MASK (MCTRCTL_M_ENABLE | MCTRCTL_S_ENABLE | \
+ MCTRCTL_U_ENABLE | MCTRCTL_RASEMU | \
+ MCTRCTL_MTE | MCTRCTL_STE | \
+ MCTRCTL_BPFRZ | MCTRCTL_LCOFIFRZ | \
+ MCTRCTL_EXCINH | MCTRCTL_INTRINH | \
+ MCTRCTL_TRETINH | MCTRCTL_NTBREN | \
+ MCTRCTL_TKBRINH | MCTRCTL_INDCALL_INH | \
+ MCTRCTL_DIRCALL_INH | MCTRCTL_INDJUMP_INH | \
+ MCTRCTL_DIRJUMP_INH | MCTRCTL_CORSWAP_INH | \
+ MCTRCTL_RET_INH | MCTRCTL_INDOJUMP_INH | \
+ MCTRCTL_DIROJUMP_INH)
+
+/* sctrctl CSR bits. */
+#define SCTRCTL_U_ENABLE MCTRCTL_U_ENABLE
+#define SCTRCTL_S_ENABLE MCTRCTL_S_ENABLE
+#define SCTRCTL_RASEMU MCTRCTL_RASEMU
+#define SCTRCTL_STE MCTRCTL_STE
+#define SCTRCTL_BPFRZ MCTRCTL_BPFRZ
+#define SCTRCTL_LCOFIFRZ MCTRCTL_LCOFIFRZ
+#define SCTRCTL_EXCINH MCTRCTL_EXCINH
+#define SCTRCTL_INTRINH MCTRCTL_INTRINH
+#define SCTRCTL_TRETINH MCTRCTL_TRETINH
+#define SCTRCTL_NTBREN MCTRCTL_NTBREN
+#define SCTRCTL_TKBRINH MCTRCTL_TKBRINH
+#define SCTRCTL_INDCALL_INH MCTRCTL_INDCALL_INH
+#define SCTRCTL_DIRCALL_INH MCTRCTL_DIRCALL_INH
+#define SCTRCTL_INDJUMP_INH MCTRCTL_INDJUMP_INH
+#define SCTRCTL_DIRJUMP_INH MCTRCTL_DIRJUMP_INH
+#define SCTRCTL_CORSWAP_INH MCTRCTL_CORSWAP_INH
+#define SCTRCTL_RET_INH MCTRCTL_RET_INH
+#define SCTRCTL_INDOJUMP_INH MCTRCTL_INDOJUMP_INH
+#define SCTRCTL_DIROJUMP_INH MCTRCTL_DIROJUMP_INH
+
+#define SCTRCTL_MASK (SCTRCTL_S_ENABLE | SCTRCTL_U_ENABLE | \
+ SCTRCTL_RASEMU | SCTRCTL_STE | \
+ SCTRCTL_BPFRZ | SCTRCTL_LCOFIFRZ | \
+ SCTRCTL_EXCINH | SCTRCTL_INTRINH | \
+ SCTRCTL_TRETINH | SCTRCTL_NTBREN | \
+ SCTRCTL_TKBRINH | SCTRCTL_INDCALL_INH | \
+ SCTRCTL_DIRCALL_INH | SCTRCTL_INDJUMP_INH | \
+ SCTRCTL_DIRJUMP_INH | SCTRCTL_CORSWAP_INH | \
+ SCTRCTL_RET_INH | SCTRCTL_INDOJUMP_INH | \
+ SCTRCTL_DIROJUMP_INH)
+
+/* sctrstatus CSR bits. */
+#define SCTRSTATUS_WRPTR_MASK 0xFF
+#define SCTRSTATUS_FROZEN BIT(31)
+#define SCTRSTATUS_MASK (SCTRSTATUS_WRPTR_MASK | SCTRSTATUS_FROZEN)
+
+/* sctrdepth CSR bits. */
+#define SCTRDEPTH_MASK 0x7
+#define SCTRDEPTH_MIN 0U /* 16 Entries. */
+#define SCTRDEPTH_MAX 4U /* 256 Entries. */
+
+/* vsctrctl CSR bits. */
+#define VSCTRCTL_VU_ENABLE MCTRCTL_U_ENABLE
+#define VSCTRCTL_VS_ENABLE MCTRCTL_S_ENABLE
+#define VSCTRCTL_RASEMU MCTRCTL_RASEMU
+#define VSCTRCTL_VSTE MCTRCTL_STE
+#define VSCTRCTL_BPFRZ MCTRCTL_BPFRZ
+#define VSCTRCTL_LCOFIFRZ MCTRCTL_LCOFIFRZ
+#define VSCTRCTL_EXCINH MCTRCTL_EXCINH
+#define VSCTRCTL_INTRINH MCTRCTL_INTRINH
+#define VSCTRCTL_TRETINH MCTRCTL_TRETINH
+#define VSCTRCTL_NTBREN MCTRCTL_NTBREN
+#define VSCTRCTL_TKBRINH MCTRCTL_TKBRINH
+#define VSCTRCTL_INDCALL_INH MCTRCTL_INDCALL_INH
+#define VSCTRCTL_DIRCALL_INH MCTRCTL_DIRCALL_INH
+#define VSCTRCTL_INDJUMP_INH MCTRCTL_INDJUMP_INH
+#define VSCTRCTL_DIRJUMP_INH MCTRCTL_DIRJUMP_INH
+#define VSCTRCTL_CORSWAP_INH MCTRCTL_CORSWAP_INH
+#define VSCTRCTL_RET_INH MCTRCTL_RET_INH
+#define VSCTRCTL_INDOJUMP_INH MCTRCTL_INDOJUMP_INH
+#define VSCTRCTL_DIROJUMP_INH MCTRCTL_DIROJUMP_INH
+
+#define VSCTRCTL_MASK (VSCTRCTL_VS_ENABLE | VSCTRCTL_VU_ENABLE | \
+ VSCTRCTL_RASEMU | VSCTRCTL_VSTE | \
+ VSCTRCTL_BPFRZ | VSCTRCTL_LCOFIFRZ | \
+ VSCTRCTL_EXCINH | VSCTRCTL_INTRINH | \
+ VSCTRCTL_TRETINH | VSCTRCTL_NTBREN | \
+ VSCTRCTL_TKBRINH | VSCTRCTL_INDCALL_INH | \
+ VSCTRCTL_DIRCALL_INH | VSCTRCTL_INDJUMP_INH | \
+ VSCTRCTL_DIRJUMP_INH | VSCTRCTL_CORSWAP_INH | \
+ VSCTRCTL_RET_INH | VSCTRCTL_INDOJUMP_INH | \
+ VSCTRCTL_DIROJUMP_INH)
+
+#define CTR_ENTRIES_FIRST 0x200
+#define CTR_ENTRIES_LAST 0x2ff
+
+#define CTRSOURCE_VALID BIT(0)
+#define CTRTARGET_MISP BIT(0)
+
+#define CTRDATA_TYPE_MASK 0xF
+#define CTRDATA_CCV BIT(15)
+#define CTRDATA_CCM_MASK 0xFFF0000
+#define CTRDATA_CCE_MASK 0xF0000000
+
+#define CTRDATA_MASK (CTRDATA_TYPE_MASK | CTRDATA_CCV | \
+ CTRDATA_CCM_MASK | CTRDATA_CCE_MASK)
+
+#define CTRDATA_TYPE_NONE 0
+#define CTRDATA_TYPE_EXCEPTION 1
+#define CTRDATA_TYPE_INTERRUPT 2
+#define CTRDATA_TYPE_EXCEP_INT_RET 3
+#define CTRDATA_TYPE_NONTAKEN_BRANCH 4
+#define CTRDATA_TYPE_TAKEN_BRANCH 5
+#define CTRDATA_TYPE_RESERVED_0 6
+#define CTRDATA_TYPE_RESERVED_1 7
+#define CTRDATA_TYPE_INDIRECT_CALL 8
+#define CTRDATA_TYPE_DIRECT_CALL 9
+#define CTRDATA_TYPE_INDIRECT_JUMP 10
+#define CTRDATA_TYPE_DIRECT_JUMP 11
+#define CTRDATA_TYPE_CO_ROUTINE_SWAP 12
+#define CTRDATA_TYPE_RETURN 13
+#define CTRDATA_TYPE_OTHER_INDIRECT_JUMP 14
+#define CTRDATA_TYPE_OTHER_DIRECT_JUMP 15
+
/* MISELECT, SISELECT, and VSISELECT bits (AIA) */
#define ISELECT_IPRIO0 0x30
#define ISELECT_IPRIO15 0x3f
--
2.34.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 3/6] target/riscv: Add support for Control Transfer Records extension CSRs.
2024-05-29 16:09 [PATCH 0/6] target/riscv: Add support for Control Transfer Records Ext Rajnesh Kanwal
2024-05-29 16:09 ` [PATCH 1/6] target/riscv: Remove obsolete sfence.vm instruction Rajnesh Kanwal
2024-05-29 16:09 ` [PATCH 2/6] target/riscv: Add Control Transfer Records CSR definitions Rajnesh Kanwal
@ 2024-05-29 16:09 ` Rajnesh Kanwal
2024-06-04 10:14 ` Jason Chien
2024-05-29 16:09 ` [PATCH 4/6] target/riscv: Add support to record CTR entries Rajnesh Kanwal
` (3 subsequent siblings)
6 siblings, 1 reply; 22+ messages in thread
From: Rajnesh Kanwal @ 2024-05-29 16:09 UTC (permalink / raw)
To: qemu-riscv, qemu-devel
Cc: alistair.francis, bin.meng, liweiwei, dbarboza, zhiwei_liu,
atishp, apatel, rkanwal, beeman, tech-control-transfer-records
This commit adds support for [m|s|vs]ctrcontrol, sctrstatus and
sctrdepth CSRs handling.
Signed-off-by: Rajnesh Kanwal <rkanwal@rivosinc.com>
---
target/riscv/cpu.h | 5 ++
target/riscv/cpu_cfg.h | 2 +
target/riscv/csr.c | 159 +++++++++++++++++++++++++++++++++++++++++
3 files changed, 166 insertions(+)
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index a185e2d494..3d4d5172b8 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -263,6 +263,11 @@ struct CPUArchState {
target_ulong mcause;
target_ulong mtval; /* since: priv-1.10.0 */
+ uint64_t mctrctl;
+ uint32_t sctrdepth;
+ uint32_t sctrstatus;
+ uint64_t vsctrctl;
+
/* Machine and Supervisor interrupt priorities */
uint8_t miprio[64];
uint8_t siprio[64];
diff --git a/target/riscv/cpu_cfg.h b/target/riscv/cpu_cfg.h
index d9354dc80a..d329a65811 100644
--- a/target/riscv/cpu_cfg.h
+++ b/target/riscv/cpu_cfg.h
@@ -123,6 +123,8 @@ struct RISCVCPUConfig {
bool ext_zvfhmin;
bool ext_smaia;
bool ext_ssaia;
+ bool ext_smctr;
+ bool ext_ssctr;
bool ext_sscofpmf;
bool ext_smepmp;
bool rvv_ta_all_1s;
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 2f92e4b717..888084d8e5 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -621,6 +621,61 @@ static RISCVException pointer_masking(CPURISCVState *env, int csrno)
return RISCV_EXCP_ILLEGAL_INST;
}
+/*
+ * M-mode:
+ * Without ext_smctr raise illegal inst excep.
+ * Otherwise everything is accessible to m-mode.
+ *
+ * S-mode:
+ * Without ext_ssctr or mstateen.ctr raise illegal inst excep.
+ * Otherwise everything other than mctrctl is accessible.
+ *
+ * VS-mode:
+ * Without ext_ssctr or mstateen.ctr raise illegal inst excep.
+ * Without hstateen.ctr raise virtual illegal inst excep.
+ * Otherwise allow vsctrctl, sctrstatus, 0x200-0x2ff entry range.
+ * Always raise illegal instruction exception for sctrdepth.
+ */
+static RISCVException ctr_mmode(CPURISCVState *env, int csrno)
+{
+ /* Check if smctr-ext is present */
+ if (riscv_cpu_cfg(env)->ext_smctr) {
+ return RISCV_EXCP_NONE;
+ }
+
+ return RISCV_EXCP_ILLEGAL_INST;
+}
+
+static RISCVException ctr_smode(CPURISCVState *env, int csrno)
+{
+ if ((env->priv == PRV_M && riscv_cpu_cfg(env)->ext_smctr) ||
+ (env->priv == PRV_S && !env->virt_enabled &&
+ riscv_cpu_cfg(env)->ext_ssctr)) {
+ return smstateen_acc_ok(env, 0, SMSTATEEN0_CTR);
+ }
+
+ if (env->priv == PRV_S && env->virt_enabled &&
+ riscv_cpu_cfg(env)->ext_ssctr) {
+ if (csrno == CSR_SCTRSTATUS) {
+ return smstateen_acc_ok(env, 0, SMSTATEEN0_CTR);
+ }
+
+ return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
+ }
+
+ return RISCV_EXCP_ILLEGAL_INST;
+}
+
+static RISCVException ctr_vsmode(CPURISCVState *env, int csrno)
+{
+ if (env->priv == PRV_S && env->virt_enabled &&
+ riscv_cpu_cfg(env)->ext_ssctr) {
+ return smstateen_acc_ok(env, 0, SMSTATEEN0_CTR);
+ }
+
+ return ctr_smode(env, csrno);
+}
+
static RISCVException aia_hmode(CPURISCVState *env, int csrno)
{
int ret;
@@ -3835,6 +3890,100 @@ static RISCVException write_satp(CPURISCVState *env, int csrno,
return RISCV_EXCP_NONE;
}
+static RISCVException rmw_sctrdepth(CPURISCVState *env, int csrno,
+ target_ulong *ret_val,
+ target_ulong new_val, target_ulong wr_mask)
+{
+ uint64_t mask = wr_mask & SCTRDEPTH_MASK;
+
+ if (ret_val) {
+ *ret_val = env->sctrdepth & SCTRDEPTH_MASK;
+ }
+
+ env->sctrdepth = (env->sctrdepth & ~mask) | (new_val & mask);
+
+ /* Correct depth. */
+ if (wr_mask & SCTRDEPTH_MASK) {
+ uint64_t depth = get_field(env->sctrdepth, SCTRDEPTH_MASK);
+
+ if (depth > SCTRDEPTH_MAX) {
+ env->sctrdepth =
+ set_field(env->sctrdepth, SCTRDEPTH_MASK, SCTRDEPTH_MAX);
+ }
+
+ /* Update sctrstatus.WRPTR with a legal value */
+ depth = 16 << depth;
+ env->sctrstatus =
+ env->sctrstatus & (~SCTRSTATUS_WRPTR_MASK | (depth - 1));
+ }
+
+ return RISCV_EXCP_NONE;
+}
+
+static RISCVException rmw_mctrctl(CPURISCVState *env, int csrno,
+ target_ulong *ret_val,
+ target_ulong new_val, target_ulong wr_mask)
+{
+ uint64_t mask = wr_mask & MCTRCTL_MASK;
+
+ if (ret_val) {
+ *ret_val = env->mctrctl & MCTRCTL_MASK;
+ }
+
+ env->mctrctl = (env->mctrctl & ~mask) | (new_val & mask);
+
+ return RISCV_EXCP_NONE;
+}
+
+static RISCVException rmw_sctrctl(CPURISCVState *env, int csrno,
+ target_ulong *ret_val,
+ target_ulong new_val, target_ulong wr_mask)
+{
+ uint64_t mask = wr_mask & SCTRCTL_MASK;
+ RISCVException ret;
+
+ ret = rmw_mctrctl(env, csrno, ret_val, new_val, mask);
+ if (ret_val) {
+ *ret_val &= SCTRCTL_MASK;
+ }
+
+ return ret;
+}
+
+static RISCVException rmw_sctrstatus(CPURISCVState *env, int csrno,
+ target_ulong *ret_val,
+ target_ulong new_val, target_ulong wr_mask)
+{
+ uint32_t depth = 16 << get_field(env->sctrdepth, SCTRDEPTH_MASK);
+ uint32_t mask = wr_mask & SCTRSTATUS_MASK;
+
+ if (ret_val) {
+ *ret_val = env->sctrstatus & SCTRSTATUS_MASK;
+ }
+
+ env->sctrstatus = (env->sctrstatus & ~mask) | (new_val & mask);
+
+ /* Update sctrstatus.WRPTR with a legal value */
+ env->sctrstatus = env->sctrstatus & (~SCTRSTATUS_WRPTR_MASK | (depth - 1));
+
+ return RISCV_EXCP_NONE;
+}
+
+static RISCVException rmw_vsctrctl(CPURISCVState *env, int csrno,
+ target_ulong *ret_val,
+ target_ulong new_val, target_ulong wr_mask)
+{
+ uint64_t mask = wr_mask & VSCTRCTL_MASK;
+
+ if (ret_val) {
+ *ret_val = env->vsctrctl & VSCTRCTL_MASK;
+ }
+
+ env->vsctrctl = (env->vsctrctl & ~mask) | (new_val & mask);
+
+ return RISCV_EXCP_NONE;
+}
+
static RISCVException read_vstopi(CPURISCVState *env, int csrno,
target_ulong *val)
{
@@ -5771,6 +5920,16 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
[CSR_SPMBASE] = { "spmbase", pointer_masking, read_spmbase,
write_spmbase },
+ [CSR_MCTRCTL] = { "mctrctl", ctr_mmode, NULL, NULL,
+ rmw_mctrctl },
+ [CSR_SCTRCTL] = { "sctrctl", ctr_smode, NULL, NULL,
+ rmw_sctrctl },
+ [CSR_SCTRDEPTH] = { "sctrdepth", ctr_smode, NULL, NULL,
+ rmw_sctrdepth },
+ [CSR_SCTRSTATUS] = { "sctrstatus", ctr_smode, NULL, NULL,
+ rmw_sctrstatus },
+ [CSR_VSCTRCTL] = { "vsctrctl", ctr_vsmode, NULL, NULL,
+ rmw_vsctrctl },
/* Performance Counters */
[CSR_HPMCOUNTER3] = { "hpmcounter3", ctr, read_hpmcounter },
[CSR_HPMCOUNTER4] = { "hpmcounter4", ctr, read_hpmcounter },
--
2.34.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 4/6] target/riscv: Add support to record CTR entries.
2024-05-29 16:09 [PATCH 0/6] target/riscv: Add support for Control Transfer Records Ext Rajnesh Kanwal
` (2 preceding siblings ...)
2024-05-29 16:09 ` [PATCH 3/6] target/riscv: Add support for Control Transfer Records extension CSRs Rajnesh Kanwal
@ 2024-05-29 16:09 ` Rajnesh Kanwal
2024-06-04 16:30 ` Jason Chien
2024-05-29 16:09 ` [PATCH 5/6] target/riscv: Add CTR sctrclr instruction Rajnesh Kanwal
` (2 subsequent siblings)
6 siblings, 1 reply; 22+ messages in thread
From: Rajnesh Kanwal @ 2024-05-29 16:09 UTC (permalink / raw)
To: qemu-riscv, qemu-devel
Cc: alistair.francis, bin.meng, liweiwei, dbarboza, zhiwei_liu,
atishp, apatel, rkanwal, beeman, tech-control-transfer-records
This commit adds logic to records CTR entries of different types
and adds required hooks in TCG and interrupt/Exception logic to
record events.
This commit also adds support to invoke freeze CTR logic for breakpoint
exceptions and counter overflow interrupts.
Signed-off-by: Rajnesh Kanwal <rkanwal@rivosinc.com>
---
target/riscv/cpu.h | 8 +
target/riscv/cpu_helper.c | 206 ++++++++++++++++++
target/riscv/helper.h | 8 +-
.../riscv/insn_trans/trans_privileged.c.inc | 6 +-
target/riscv/insn_trans/trans_rvi.c.inc | 27 +++
target/riscv/op_helper.c | 112 +++++++++-
target/riscv/translate.c | 9 +
7 files changed, 370 insertions(+), 6 deletions(-)
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 3d4d5172b8..a294a5372a 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -268,6 +268,10 @@ struct CPUArchState {
uint32_t sctrstatus;
uint64_t vsctrctl;
+ uint64_t ctr_src[16 << SCTRDEPTH_MAX];
+ uint64_t ctr_dst[16 << SCTRDEPTH_MAX];
+ uint64_t ctr_data[16 << SCTRDEPTH_MAX];
+
/* Machine and Supervisor interrupt priorities */
uint8_t miprio[64];
uint8_t siprio[64];
@@ -565,6 +569,10 @@ RISCVException smstateen_acc_ok(CPURISCVState *env, int index, uint64_t bit);
#endif
void riscv_cpu_set_mode(CPURISCVState *env, target_ulong newpriv, bool virt_en);
+void riscv_ctr_freeze(CPURISCVState *env, uint64_t freeze_mask);
+void riscv_ctr_add_entry(CPURISCVState *env, target_long src, target_long dst,
+ uint64_t type, target_ulong prev_priv, bool prev_virt);
+
void riscv_translate_init(void);
G_NORETURN void riscv_raise_exception(CPURISCVState *env,
uint32_t exception, uintptr_t pc);
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index a441a03ef4..e064a7306e 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -663,6 +663,10 @@ uint64_t riscv_cpu_update_mip(CPURISCVState *env, uint64_t mask, uint64_t value)
BQL_LOCK_GUARD();
+ if (MIP_LCOFIP & value & mask) {
+ riscv_ctr_freeze(env, MCTRCTL_LCOFIFRZ);
+ }
+
env->mip = (env->mip & ~mask) | (value & mask);
riscv_cpu_interrupt(env);
@@ -691,6 +695,197 @@ void riscv_cpu_set_aia_ireg_rmw_fn(CPURISCVState *env, uint32_t priv,
}
}
+void riscv_ctr_freeze(CPURISCVState *env, uint64_t freeze_mask)
+{
+ assert((freeze_mask & (~(MCTRCTL_BPFRZ | MCTRCTL_LCOFIFRZ))) == 0);
+
+ if (env->mctrctl & freeze_mask) {
+ env->sctrstatus |= SCTRSTATUS_FROZEN;
+ }
+}
+
+static uint64_t riscv_ctr_priv_to_mask(target_ulong priv, bool virt)
+{
+ switch (priv) {
+ case PRV_M:
+ return MCTRCTL_M_ENABLE;
+ case PRV_S:
+ if (virt) {
+ return VSCTRCTL_VS_ENABLE;
+ }
+ return MCTRCTL_S_ENABLE;
+ case PRV_U:
+ if (virt) {
+ return VSCTRCTL_VU_ENABLE;
+ }
+ return MCTRCTL_U_ENABLE;
+ }
+
+ g_assert_not_reached();
+}
+
+static uint64_t riscv_ctr_get_control(CPURISCVState *env, target_long priv,
+ bool virt)
+{
+ switch (priv) {
+ case PRV_M:
+ return env->mctrctl;
+ case PRV_S:
+ case PRV_U:
+ if (virt) {
+ return env->vsctrctl;
+ }
+ return env->mctrctl;
+ }
+
+ g_assert_not_reached();
+}
+
+/*
+ * Special cases for traps and trap returns:
+ *
+ * 1- Traps, and trap returns, between enabled modes are recorded as normal.
+ * 2- Traps from an inhibited mode to an enabled mode, and trap returns from an
+ * enabled mode back to an inhibited mode, are partially recorded. In such
+ * cases, the PC from the inhibited mode (source PC for traps, and target PC
+ * for trap returns) is 0.
+ *
+ * 3- Trap returns from an inhibited mode to an enabled mode are not recorded.
+ * Traps from an enabled mode to an inhibited mode, known as external traps,
+ * receive special handling.
+ * By default external traps are not recorded, but a handshake mechanism exists
+ * to allow partial recording. Software running in the target mode of the trap
+ * can opt-in to allowing CTR to record traps into that mode even when the mode
+ * is inhibited. The MTE, STE, and VSTE bits allow M-mode, S-mode, and VS-mode,
+ * respectively, to opt-in. When an External Trap occurs, and xTE=1, such that
+ * x is the target privilege mode of the trap, will CTR record the trap. In such
+ * cases, the target PC is 0.
+ */
+/*
+ * CTR arrays are implemented as circular buffers and new entry is stored at
+ * sctrstatus.WRPTR, but they are presented to software as moving circular
+ * buffers. Which means, software get's the illusion that whenever a new entry
+ * is added the whole buffer is moved by one place and the new entry is added at
+ * the start keeping new entry at idx 0 and older ones follow.
+ *
+ * Depth = 16.
+ *
+ * buffer [0] [1] [2] [3] [4] [5] [6] [7] [8] [9] [A] [B] [C] [D] [E] [F]
+ * WRPTR W
+ * entry 7 6 5 4 3 2 1 0 F E D C B A 9 8
+ *
+ * When a new entry is added:
+ * buffer [0] [1] [2] [3] [4] [5] [6] [7] [8] [9] [A] [B] [C] [D] [E] [F]
+ * WRPTR W
+ * entry 8 7 6 5 4 3 2 1 0 F E D C B A 9
+ *
+ * entry here denotes the logical entry number that software can access
+ * using ctrsource, ctrtarget and ctrdata registers. So xiselect 0x200
+ * will return entry 0 i-e buffer[8] and 0x201 will return entry 1 i-e
+ * buffer[7]. Here is how we convert entry to buffer idx.
+ *
+ * entry = isel - CTR_ENTRIES_FIRST;
+ * idx = (sctrstatus.WRPTR - entry - 1) & (depth - 1);
+ */
+void riscv_ctr_add_entry(CPURISCVState *env, target_long src, target_long dst,
+ uint64_t type, target_ulong src_priv, bool src_virt)
+{
+ bool tgt_virt = env->virt_enabled;
+ uint64_t src_mask = riscv_ctr_priv_to_mask(src_priv, src_virt);
+ uint64_t tgt_mask = riscv_ctr_priv_to_mask(env->priv, tgt_virt);
+ uint64_t src_ctrl = riscv_ctr_get_control(env, src_priv, src_virt);
+ uint64_t tgt_ctrl = riscv_ctr_get_control(env, env->priv, tgt_virt);
+ uint64_t depth, head;
+ bool ext_trap = false;
+
+ if (env->sctrstatus & SCTRSTATUS_FROZEN) {
+ return;
+ }
+
+ /*
+ * With RAS Emul enabled, only allow Indirect, drirect calls, Function
+ * returns and Co-routine swap types.
+ */
+ if (env->mctrctl & MCTRCTL_RASEMU &&
+ type != CTRDATA_TYPE_INDIRECT_CALL &&
+ type != CTRDATA_TYPE_DIRECT_CALL &&
+ type != CTRDATA_TYPE_RETURN &&
+ type != CTRDATA_TYPE_CO_ROUTINE_SWAP) {
+ return;
+ }
+
+ if (type == CTRDATA_TYPE_EXCEPTION || type == CTRDATA_TYPE_INTERRUPT) {
+ /* Case 2 for traps. */
+ if (!(src_ctrl & src_mask) && (tgt_ctrl & tgt_mask)) {
+ src = 0;
+ } else if ((src_ctrl & src_mask) && !(tgt_ctrl & tgt_mask)) {
+ /* Check if target priv-mode has allowed external trap recording. */
+ if ((env->priv == PRV_M && !(tgt_ctrl & MCTRCTL_MTE)) ||
+ (env->priv == PRV_S && !(tgt_ctrl & MCTRCTL_STE))) {
+ return;
+ }
+
+ ext_trap = true;
+ dst = 0;
+ } else if (!(src_ctrl & src_mask) && !(tgt_ctrl & tgt_mask)) {
+ return;
+ }
+ } else if (type == CTRDATA_TYPE_EXCEP_INT_RET) {
+ /*
+ * Case 3 for trap returns. Trap returns from inhibited mode are not
+ * recorded.
+ */
+ if (!(src_ctrl & src_mask)) {
+ return;
+ }
+
+ /* Case 2 for trap returns. */
+ if (!(tgt_ctrl & tgt_mask)) {
+ dst = 0;
+ }
+ } else if (!(tgt_ctrl & tgt_mask)) {
+ return;
+ }
+
+ /* Ignore filters in case of RASEMU mode or External trap. */
+ if (!(tgt_ctrl & MCTRCTL_RASEMU) && !ext_trap) {
+ /*
+ * Check if the specific type is inhibited. Not taken branch filter is
+ * an enable bit and needs to be checked separatly.
+ */
+ bool check = tgt_ctrl & BIT_ULL(type + MCTRCTL_INH_START);
+ if ((type == CTRDATA_TYPE_NONTAKEN_BRANCH && !check) ||
+ (type != CTRDATA_TYPE_NONTAKEN_BRANCH && check)) {
+ return;
+ }
+ }
+
+ head = get_field(env->sctrstatus, SCTRSTATUS_WRPTR_MASK);
+
+ depth = 16 << get_field(env->sctrdepth, SCTRDEPTH_MASK);
+ if (tgt_ctrl & MCTRCTL_RASEMU && type == CTRDATA_TYPE_RETURN) {
+ head = (head - 1) & (depth - 1);
+
+ env->ctr_src[head] &= ~CTRSOURCE_VALID;
+ env->sctrstatus =
+ set_field(env->sctrstatus, SCTRSTATUS_WRPTR_MASK, head);
+ return;
+ }
+
+ /* In case of Co-routine SWAP we overwrite latest entry. */
+ if (tgt_ctrl & MCTRCTL_RASEMU && type == CTRDATA_TYPE_CO_ROUTINE_SWAP) {
+ head = (head - 1) & (depth - 1);
+ }
+
+ env->ctr_src[head] = src | CTRSOURCE_VALID;
+ env->ctr_dst[head] = dst & ~CTRTARGET_MISP;
+ env->ctr_data[head] = set_field(0, CTRDATA_TYPE_MASK, type);
+
+ head = (head + 1) & (depth - 1);
+
+ env->sctrstatus = set_field(env->sctrstatus, SCTRSTATUS_WRPTR_MASK, head);
+}
+
void riscv_cpu_set_mode(CPURISCVState *env, target_ulong newpriv, bool virt_en)
{
g_assert(newpriv <= PRV_M && newpriv != PRV_RESERVED);
@@ -1669,10 +1864,13 @@ void riscv_cpu_do_interrupt(CPUState *cs)
!(env->mip & (1 << cause));
bool vs_injected = env->hvip & (1 << cause) & env->hvien &&
!(env->mip & (1 << cause));
+ const bool prev_virt = env->virt_enabled;
+ const target_ulong prev_priv = env->priv;
target_ulong tval = 0;
target_ulong tinst = 0;
target_ulong htval = 0;
target_ulong mtval2 = 0;
+ target_ulong src;
if (!async) {
/* set tval to badaddr for traps with address information */
@@ -1729,6 +1927,7 @@ void riscv_cpu_do_interrupt(CPUState *cs)
tval = cs->watchpoint_hit->hitaddr;
cs->watchpoint_hit = NULL;
}
+ riscv_ctr_freeze(env, MCTRCTL_BPFRZ);
break;
default:
break;
@@ -1807,6 +2006,8 @@ void riscv_cpu_do_interrupt(CPUState *cs)
env->pc = (env->stvec >> 2 << 2) +
((async && (env->stvec & 3) == 1) ? cause * 4 : 0);
riscv_cpu_set_mode(env, PRV_S, virt);
+
+ src = env->sepc;
} else {
/* handle the trap in M-mode */
if (riscv_has_ext(env, RVH)) {
@@ -1838,8 +2039,13 @@ void riscv_cpu_do_interrupt(CPUState *cs)
env->pc = (env->mtvec >> 2 << 2) +
((async && (env->mtvec & 3) == 1) ? cause * 4 : 0);
riscv_cpu_set_mode(env, PRV_M, virt);
+ src = env->mepc;
}
+ riscv_ctr_add_entry(env, src, env->pc,
+ async ? CTRDATA_TYPE_INTERRUPT : CTRDATA_TYPE_EXCEPTION,
+ prev_priv, prev_virt);
+
/*
* NOTE: it is not necessary to yield load reservations here. It is only
* necessary for an SC from "another hart" to cause a load reservation
diff --git a/target/riscv/helper.h b/target/riscv/helper.h
index 451261ce5a..0a9a545d87 100644
--- a/target/riscv/helper.h
+++ b/target/riscv/helper.h
@@ -129,12 +129,16 @@ DEF_HELPER_2(csrr_i128, tl, env, int)
DEF_HELPER_4(csrw_i128, void, env, int, tl, tl)
DEF_HELPER_6(csrrw_i128, tl, env, int, tl, tl, tl, tl)
#ifndef CONFIG_USER_ONLY
-DEF_HELPER_1(sret, tl, env)
-DEF_HELPER_1(mret, tl, env)
+DEF_HELPER_2(sret, tl, env, tl)
+DEF_HELPER_2(mret, tl, env, tl)
+DEF_HELPER_1(ctr_clear, void, env)
DEF_HELPER_1(wfi, void, env)
DEF_HELPER_1(wrs_nto, void, env)
DEF_HELPER_1(tlb_flush, void, env)
DEF_HELPER_1(tlb_flush_all, void, env)
+DEF_HELPER_4(ctr_branch, void, env, tl, tl, tl)
+DEF_HELPER_4(ctr_jal, void, env, tl, tl, tl)
+DEF_HELPER_5(ctr_jalr, void, env, tl, tl, tl, tl)
/* Native Debug */
DEF_HELPER_1(itrigger_match, void, env)
#endif
diff --git a/target/riscv/insn_trans/trans_privileged.c.inc b/target/riscv/insn_trans/trans_privileged.c.inc
index 4eccdddeaa..339d659151 100644
--- a/target/riscv/insn_trans/trans_privileged.c.inc
+++ b/target/riscv/insn_trans/trans_privileged.c.inc
@@ -78,9 +78,10 @@ static bool trans_sret(DisasContext *ctx, arg_sret *a)
{
#ifndef CONFIG_USER_ONLY
if (has_ext(ctx, RVS)) {
+ TCGv src = tcg_constant_tl(ctx->base.pc_next);
decode_save_opc(ctx);
translator_io_start(&ctx->base);
- gen_helper_sret(cpu_pc, tcg_env);
+ gen_helper_sret(cpu_pc, tcg_env, src);
exit_tb(ctx); /* no chaining */
ctx->base.is_jmp = DISAS_NORETURN;
} else {
@@ -95,9 +96,10 @@ static bool trans_sret(DisasContext *ctx, arg_sret *a)
static bool trans_mret(DisasContext *ctx, arg_mret *a)
{
#ifndef CONFIG_USER_ONLY
+ TCGv src = tcg_constant_tl(ctx->base.pc_next);
decode_save_opc(ctx);
translator_io_start(&ctx->base);
- gen_helper_mret(cpu_pc, tcg_env);
+ gen_helper_mret(cpu_pc, tcg_env, src);
exit_tb(ctx); /* no chaining */
ctx->base.is_jmp = DISAS_NORETURN;
return true;
diff --git a/target/riscv/insn_trans/trans_rvi.c.inc b/target/riscv/insn_trans/trans_rvi.c.inc
index ad40d3e87f..7f95362b66 100644
--- a/target/riscv/insn_trans/trans_rvi.c.inc
+++ b/target/riscv/insn_trans/trans_rvi.c.inc
@@ -55,6 +55,11 @@ static bool trans_jalr(DisasContext *ctx, arg_jalr *a)
TCGLabel *misaligned = NULL;
TCGv target_pc = tcg_temp_new();
TCGv succ_pc = dest_gpr(ctx, a->rd);
+#ifndef CONFIG_USER_ONLY
+ TCGv rd = tcg_constant_tl(a->rd);
+ TCGv rs1 = tcg_constant_tl(a->rs1);
+ TCGv src = tcg_constant_tl(ctx->base.pc_next);
+#endif
tcg_gen_addi_tl(target_pc, get_gpr(ctx, a->rs1, EXT_NONE), a->imm);
tcg_gen_andi_tl(target_pc, target_pc, (target_ulong)-2);
@@ -75,6 +80,9 @@ static bool trans_jalr(DisasContext *ctx, arg_jalr *a)
gen_set_gpr(ctx, a->rd, succ_pc);
tcg_gen_mov_tl(cpu_pc, target_pc);
+#ifndef CONFIG_USER_ONLY
+ gen_helper_ctr_jalr(tcg_env, src, cpu_pc, rd, rs1);
+#endif
lookup_and_goto_ptr(ctx);
if (misaligned) {
@@ -164,6 +172,11 @@ static bool gen_branch(DisasContext *ctx, arg_b *a, TCGCond cond)
TCGv src1 = get_gpr(ctx, a->rs1, EXT_SIGN);
TCGv src2 = get_gpr(ctx, a->rs2, EXT_SIGN);
target_ulong orig_pc_save = ctx->pc_save;
+#ifndef CONFIG_USER_ONLY
+ TCGv src = tcg_constant_tl(ctx->base.pc_next);
+ TCGv taken;
+ TCGv dest;
+#endif
if (get_xl(ctx) == MXL_RV128) {
TCGv src1h = get_gprh(ctx, a->rs1);
@@ -176,6 +189,14 @@ static bool gen_branch(DisasContext *ctx, arg_b *a, TCGCond cond)
} else {
tcg_gen_brcond_tl(cond, src1, src2, l);
}
+
+#ifndef CONFIG_USER_ONLY
+ dest = tcg_constant_tl(ctx->base.pc_next + ctx->cur_insn_len);
+ taken = tcg_constant_tl(0);
+
+ gen_helper_ctr_branch(tcg_env, src, dest, taken);
+#endif
+
gen_goto_tb(ctx, 1, ctx->cur_insn_len);
ctx->pc_save = orig_pc_save;
@@ -188,6 +209,12 @@ static bool gen_branch(DisasContext *ctx, arg_b *a, TCGCond cond)
gen_pc_plus_diff(target_pc, ctx, a->imm);
gen_exception_inst_addr_mis(ctx, target_pc);
} else {
+#ifndef CONFIG_USER_ONLY
+ dest = tcg_constant_tl(ctx->base.pc_next + a->imm);
+ taken = tcg_constant_tl(1);
+
+ gen_helper_ctr_branch(tcg_env, src, dest, taken);
+#endif
gen_goto_tb(ctx, 0, a->imm);
}
ctx->pc_save = -1;
diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
index 25a5263573..c8053d9c2f 100644
--- a/target/riscv/op_helper.c
+++ b/target/riscv/op_helper.c
@@ -259,10 +259,12 @@ void helper_cbo_inval(CPURISCVState *env, target_ulong address)
#ifndef CONFIG_USER_ONLY
-target_ulong helper_sret(CPURISCVState *env)
+target_ulong helper_sret(CPURISCVState *env, target_ulong curr_pc)
{
uint64_t mstatus;
target_ulong prev_priv, prev_virt = env->virt_enabled;
+ const target_ulong src_priv = env->priv;
+ const bool src_virt = env->virt_enabled;
if (!(env->priv >= PRV_S)) {
riscv_raise_exception(env, RISCV_EXCP_ILLEGAL_INST, GETPC());
@@ -309,11 +311,17 @@ target_ulong helper_sret(CPURISCVState *env)
riscv_cpu_set_mode(env, prev_priv, prev_virt);
+ riscv_ctr_add_entry(env, curr_pc, retpc, CTRDATA_TYPE_EXCEP_INT_RET,
+ src_priv, src_virt);
+
return retpc;
}
-target_ulong helper_mret(CPURISCVState *env)
+target_ulong helper_mret(CPURISCVState *env, target_ulong curr_pc)
{
+ const target_ulong src_priv = env->priv;
+ const bool src_virt = env->virt_enabled;
+
if (!(env->priv >= PRV_M)) {
riscv_raise_exception(env, RISCV_EXCP_ILLEGAL_INST, GETPC());
}
@@ -350,9 +358,109 @@ target_ulong helper_mret(CPURISCVState *env)
riscv_cpu_set_mode(env, prev_priv, prev_virt);
+ riscv_ctr_add_entry(env, curr_pc, retpc, CTRDATA_TYPE_EXCEP_INT_RET,
+ src_priv, src_virt);
+
return retpc;
}
+/*
+ * Indirect calls
+ * – jalr x1, rs where rs != x5;
+ * – jalr x5, rs where rs != x1;
+ * – c.jalr rs1 where rs1 != x5;
+ *
+ * Indirect jumps
+ * – jalr x0, rs where rs != x1 and rs != x5;
+ * – c.jr rs1 where rs1 != x1 and rs1 != x5.
+ *
+ * Returns
+ * – jalr rd, rs where (rs == x1 or rs == x5) and rd != x1 and rd != x5;
+ * – c.jr rs1 where rs1 == x1 or rs1 == x5.
+ *
+ * Co-routine swap
+ * – jalr x1, x5;
+ * – jalr x5, x1;
+ * – c.jalr x5.
+ *
+ * Other indirect jumps
+ * – jalr rd, rs where rs != x1, rs != x5, rd != x0, rd != x1 and rd != x5.
+ */
+void helper_ctr_jalr(CPURISCVState *env, target_ulong src, target_ulong dest,
+ target_ulong rd, target_ulong rs1)
+{
+ target_ulong curr_priv = env->priv;
+ bool curr_virt = env->virt_enabled;
+
+ if ((rd == 1 && rs1 != 5) || (rd == 5 && rs1 != 1)) {
+ riscv_ctr_add_entry(env, src, dest, CTRDATA_TYPE_INDIRECT_CALL,
+ curr_priv, curr_virt);
+ } else if (rd == 0 && rs1 != 1 && rs1 != 5) {
+ riscv_ctr_add_entry(env, src, dest, CTRDATA_TYPE_INDIRECT_JUMP,
+ curr_priv, curr_virt);
+ } else if ((rs1 == 1 || rs1 == 5) && (rd != 1 && rd != 5)) {
+ riscv_ctr_add_entry(env, src, dest, CTRDATA_TYPE_RETURN,
+ curr_priv, curr_virt);
+ } else if ((rs1 == 1 && rd == 5) || (rs1 == 5 && rd == 1)) {
+ riscv_ctr_add_entry(env, src, dest, CTRDATA_TYPE_CO_ROUTINE_SWAP,
+ curr_priv, curr_virt);
+ } else {
+ riscv_ctr_add_entry(env, src, dest,
+ CTRDATA_TYPE_OTHER_INDIRECT_JUMP, curr_priv,
+ curr_virt);
+ }
+}
+
+/*
+ * Direct calls
+ * – jal x1;
+ * – jal x5;
+ * – c.jal.
+ *
+ * Direct jumps
+ * – jal x0;
+ * – c.j;
+ *
+ * Other direct jumps
+ * – jal rd where rd != x1 and rd != x5 and rd != x0;
+ */
+void helper_ctr_jal(CPURISCVState *env, target_ulong src, target_ulong dest,
+ target_ulong rd)
+{
+ target_ulong priv = env->priv;
+ bool virt = env->virt_enabled;
+
+ /*
+ * If rd is x1 or x5 link registers, treat this as direct call otherwise
+ * its a direct jump.
+ */
+ if (rd == 1 || rd == 5) {
+ riscv_ctr_add_entry(env, src, dest, CTRDATA_TYPE_DIRECT_CALL, priv,
+ virt);
+ } else if (rd == 0) {
+ riscv_ctr_add_entry(env, src, dest, CTRDATA_TYPE_DIRECT_JUMP, priv,
+ virt);
+ } else {
+ riscv_ctr_add_entry(env, src, dest, CTRDATA_TYPE_OTHER_DIRECT_JUMP,
+ priv, virt);
+ }
+}
+
+void helper_ctr_branch(CPURISCVState *env, target_ulong src, target_ulong dest,
+ target_ulong branch_taken)
+{
+ target_ulong curr_priv = env->priv;
+ bool curr_virt = env->virt_enabled;
+
+ if (branch_taken) {
+ riscv_ctr_add_entry(env, src, dest, CTRDATA_TYPE_TAKEN_BRANCH,
+ curr_priv, curr_virt);
+ } else {
+ riscv_ctr_add_entry(env, src, dest, CTRDATA_TYPE_NONTAKEN_BRANCH,
+ curr_priv, curr_virt);
+ }
+}
+
void helper_wfi(CPURISCVState *env)
{
CPUState *cs = env_cpu(env);
diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index 15e7123a68..8b0492991d 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -561,6 +561,11 @@ static void gen_set_fpr_d(DisasContext *ctx, int reg_num, TCGv_i64 t)
static void gen_jal(DisasContext *ctx, int rd, target_ulong imm)
{
TCGv succ_pc = dest_gpr(ctx, rd);
+#ifndef CONFIG_USER_ONLY
+ TCGv dest = tcg_constant_tl(ctx->base.pc_next + imm);
+ TCGv src = tcg_constant_tl(ctx->base.pc_next);
+ TCGv tcg_rd = tcg_constant_tl((target_ulong)rd);
+#endif
/* check misaligned: */
if (!has_ext(ctx, RVC) && !ctx->cfg_ptr->ext_zca) {
@@ -572,6 +577,10 @@ static void gen_jal(DisasContext *ctx, int rd, target_ulong imm)
}
}
+#ifndef CONFIG_USER_ONLY
+ gen_helper_ctr_jal(tcg_env, src, dest, tcg_rd);
+#endif
+
gen_pc_plus_diff(succ_pc, ctx, ctx->cur_insn_len);
gen_set_gpr(ctx, rd, succ_pc);
--
2.34.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 5/6] target/riscv: Add CTR sctrclr instruction.
2024-05-29 16:09 [PATCH 0/6] target/riscv: Add support for Control Transfer Records Ext Rajnesh Kanwal
` (3 preceding siblings ...)
2024-05-29 16:09 ` [PATCH 4/6] target/riscv: Add support to record CTR entries Rajnesh Kanwal
@ 2024-05-29 16:09 ` Rajnesh Kanwal
2024-06-04 17:19 ` Jason Chien
2024-05-29 16:09 ` [PATCH 6/6] target/riscv: Add support to access ctrsource, ctrtarget, ctrdata regs Rajnesh Kanwal
2024-06-04 7:29 ` [PATCH 0/6] target/riscv: Add support for Control Transfer Records Ext Jason Chien
6 siblings, 1 reply; 22+ messages in thread
From: Rajnesh Kanwal @ 2024-05-29 16:09 UTC (permalink / raw)
To: qemu-riscv, qemu-devel
Cc: alistair.francis, bin.meng, liweiwei, dbarboza, zhiwei_liu,
atishp, apatel, rkanwal, beeman, tech-control-transfer-records
CTR extension adds a new instruction sctrclr to quickly
clear the recorded entries buffer.
Signed-off-by: Rajnesh Kanwal <rkanwal@rivosinc.com>
---
target/riscv/cpu.h | 1 +
target/riscv/cpu_helper.c | 7 +++++++
target/riscv/insn32.decode | 1 +
target/riscv/insn_trans/trans_privileged.c.inc | 10 ++++++++++
target/riscv/op_helper.c | 5 +++++
5 files changed, 24 insertions(+)
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index a294a5372a..fade71aa09 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -572,6 +572,7 @@ void riscv_cpu_set_mode(CPURISCVState *env, target_ulong newpriv, bool virt_en);
void riscv_ctr_freeze(CPURISCVState *env, uint64_t freeze_mask);
void riscv_ctr_add_entry(CPURISCVState *env, target_long src, target_long dst,
uint64_t type, target_ulong prev_priv, bool prev_virt);
+void riscv_ctr_clear(CPURISCVState *env);
void riscv_translate_init(void);
G_NORETURN void riscv_raise_exception(CPURISCVState *env,
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index e064a7306e..45502f50a7 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -704,6 +704,13 @@ void riscv_ctr_freeze(CPURISCVState *env, uint64_t freeze_mask)
}
}
+void riscv_ctr_clear(CPURISCVState *env)
+{
+ memset(env->ctr_src, 0x0, sizeof(env->ctr_src));
+ memset(env->ctr_dst, 0x0, sizeof(env->ctr_dst));
+ memset(env->ctr_data, 0x0, sizeof(env->ctr_data));
+}
+
static uint64_t riscv_ctr_priv_to_mask(target_ulong priv, bool virt)
{
switch (priv) {
diff --git a/target/riscv/insn32.decode b/target/riscv/insn32.decode
index 9cb1a1b4ec..d3d38c7c68 100644
--- a/target/riscv/insn32.decode
+++ b/target/riscv/insn32.decode
@@ -107,6 +107,7 @@
# *** Privileged Instructions ***
ecall 000000000000 00000 000 00000 1110011
ebreak 000000000001 00000 000 00000 1110011
+sctrclr 000100000100 00000 000 00000 1110011
uret 0000000 00010 00000 000 00000 1110011
sret 0001000 00010 00000 000 00000 1110011
mret 0011000 00010 00000 000 00000 1110011
diff --git a/target/riscv/insn_trans/trans_privileged.c.inc b/target/riscv/insn_trans/trans_privileged.c.inc
index 339d659151..dd9da8651f 100644
--- a/target/riscv/insn_trans/trans_privileged.c.inc
+++ b/target/riscv/insn_trans/trans_privileged.c.inc
@@ -69,6 +69,16 @@ static bool trans_ebreak(DisasContext *ctx, arg_ebreak *a)
return true;
}
+static bool trans_sctrclr(DisasContext *ctx, arg_sctrclr *a)
+{
+#ifndef CONFIG_USER_ONLY
+ gen_helper_ctr_clear(tcg_env);
+ return true;
+#else
+ return false;
+#endif
+}
+
static bool trans_uret(DisasContext *ctx, arg_uret *a)
{
return false;
diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
index c8053d9c2f..89423c04b3 100644
--- a/target/riscv/op_helper.c
+++ b/target/riscv/op_helper.c
@@ -461,6 +461,11 @@ void helper_ctr_branch(CPURISCVState *env, target_ulong src, target_ulong dest,
}
}
+void helper_ctr_clear(CPURISCVState *env)
+{
+ riscv_ctr_clear(env);
+}
+
void helper_wfi(CPURISCVState *env)
{
CPUState *cs = env_cpu(env);
--
2.34.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 6/6] target/riscv: Add support to access ctrsource, ctrtarget, ctrdata regs.
2024-05-29 16:09 [PATCH 0/6] target/riscv: Add support for Control Transfer Records Ext Rajnesh Kanwal
` (4 preceding siblings ...)
2024-05-29 16:09 ` [PATCH 5/6] target/riscv: Add CTR sctrclr instruction Rajnesh Kanwal
@ 2024-05-29 16:09 ` Rajnesh Kanwal
2024-06-04 17:07 ` Jason Chien
2024-06-04 7:29 ` [PATCH 0/6] target/riscv: Add support for Control Transfer Records Ext Jason Chien
6 siblings, 1 reply; 22+ messages in thread
From: Rajnesh Kanwal @ 2024-05-29 16:09 UTC (permalink / raw)
To: qemu-riscv, qemu-devel
Cc: alistair.francis, bin.meng, liweiwei, dbarboza, zhiwei_liu,
atishp, apatel, rkanwal, beeman, tech-control-transfer-records
CTR entries are accessed using ctrsource, ctrtarget and ctrdata
registers using smcsrind/sscsrind extension. This commits extends
the csrind extension to support CTR registers.
ctrsource is accessible through xireg CSR, ctrtarget is accessible
through xireg1 and ctrdata is accessible through xireg2 CSR.
CTR supports maximum depth of 256 entries which are accessed using
xiselect range 0x200 to 0x2ff.
This commits also adds properties to enable CTR extension. CTR can be
enabled using smctr=true and ssctr=true now.
Signed-off-by: Rajnesh Kanwal <rkanwal@rivosinc.com>
---
target/riscv/cpu.c | 4 ++
target/riscv/csr.c | 153 ++++++++++++++++++++++++++++++++++++++++++++-
2 files changed, 156 insertions(+), 1 deletion(-)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 30bdfc22ae..a77b1d5caf 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -193,6 +193,8 @@ const RISCVIsaExtData isa_edata_arr[] = {
ISA_EXT_DATA_ENTRY(sstvala, PRIV_VERSION_1_12_0, has_priv_1_12),
ISA_EXT_DATA_ENTRY(sstvecd, PRIV_VERSION_1_12_0, has_priv_1_12),
ISA_EXT_DATA_ENTRY(svade, PRIV_VERSION_1_11_0, ext_svade),
+ ISA_EXT_DATA_ENTRY(smctr, PRIV_VERSION_1_12_0, ext_smctr),
+ ISA_EXT_DATA_ENTRY(ssctr, PRIV_VERSION_1_12_0, ext_ssctr),
ISA_EXT_DATA_ENTRY(svadu, PRIV_VERSION_1_12_0, ext_svadu),
ISA_EXT_DATA_ENTRY(svinval, PRIV_VERSION_1_12_0, ext_svinval),
ISA_EXT_DATA_ENTRY(svnapot, PRIV_VERSION_1_12_0, ext_svnapot),
@@ -1473,6 +1475,8 @@ const RISCVCPUMultiExtConfig riscv_cpu_extensions[] = {
MULTI_EXT_CFG_BOOL("sscsrind", ext_sscsrind, false),
MULTI_EXT_CFG_BOOL("smcdeleg", ext_smcdeleg, false),
MULTI_EXT_CFG_BOOL("ssccfg", ext_ssccfg, false),
+ MULTI_EXT_CFG_BOOL("smctr", ext_smctr, false),
+ MULTI_EXT_CFG_BOOL("ssctr", ext_ssctr, false),
MULTI_EXT_CFG_BOOL("zifencei", ext_zifencei, true),
MULTI_EXT_CFG_BOOL("zicsr", ext_zicsr, true),
MULTI_EXT_CFG_BOOL("zihintntl", ext_zihintntl, true),
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 888084d8e5..15b953f268 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -2291,6 +2291,11 @@ static bool xiselect_cd_range(target_ulong isel)
return (ISELECT_CD_FIRST <= isel && isel <= ISELECT_CD_LAST);
}
+static bool xiselect_ctr_range(target_ulong isel)
+{
+ return (CTR_ENTRIES_FIRST <= isel && isel <= CTR_ENTRIES_LAST);
+}
+
static int rmw_iprio(target_ulong xlen,
target_ulong iselect, uint8_t *iprio,
target_ulong *val, target_ulong new_val,
@@ -2336,6 +2341,118 @@ static int rmw_iprio(target_ulong xlen,
return 0;
}
+static int rmw_xctrsource(CPURISCVState *env, int isel, target_ulong *val,
+ target_ulong new_val, target_ulong wr_mask)
+{
+ /*
+ * CTR arrays are treated as circular buffers and TOS always points to next
+ * empty slot, keeping TOS - 1 always pointing to latest entry. Given entry
+ * 0 is always the latest one, traversal is a bit different here. See the
+ * below example.
+ *
+ * Depth = 16.
+ *
+ * idx [0] [1] [2] [3] [4] [5] [6] [7] [8] [9] [A] [B] [C] [D] [E] [F]
+ * TOS H
+ * entry 6 5 4 3 2 1 0 F E D C B A 9 8 7
+ */
+ const uint64_t entry = isel - CTR_ENTRIES_FIRST;
+ const uint64_t depth = 16 << get_field(env->sctrdepth, SCTRDEPTH_MASK);
+ uint64_t idx;
+
+ /* Entry greater than depth-1 is read-only zero */
+ if (entry >= depth) {
+ *val = 0;
+ return 0;
+ }
+
+ idx = get_field(env->sctrstatus, SCTRSTATUS_WRPTR_MASK);
+ idx = (idx - entry - 1) & (depth - 1);
+
+ if (val) {
+ *val = env->ctr_src[idx];
+ }
+
+ env->ctr_src[idx] = (env->ctr_src[idx] & ~wr_mask) | (new_val & wr_mask);
+
+ return 0;
+}
+
+static int rmw_xctrtarget(CPURISCVState *env, int isel, target_ulong *val,
+ target_ulong new_val, target_ulong wr_mask)
+{
+ /*
+ * CTR arrays are treated as circular buffers and TOS always points to next
+ * empty slot, keeping TOS - 1 always pointing to latest entry. Given entry
+ * 0 is always the latest one, traversal is a bit different here. See the
+ * below example.
+ *
+ * Depth = 16.
+ *
+ * idx [0] [1] [2] [3] [4] [5] [6] [7] [8] [9] [A] [B] [C] [D] [E] [F]
+ * head H
+ * entry 6 5 4 3 2 1 0 F E D C B A 9 8 7
+ */
+ const uint64_t entry = isel - CTR_ENTRIES_FIRST;
+ const uint64_t depth = 16 << get_field(env->sctrdepth, SCTRDEPTH_MASK);
+ uint64_t idx;
+
+ /* Entry greater than depth-1 is read-only zero */
+ if (entry >= depth) {
+ *val = 0;
+ return 0;
+ }
+
+ idx = get_field(env->sctrstatus, SCTRSTATUS_WRPTR_MASK);
+ idx = (idx - entry - 1) & (depth - 1);
+
+ if (val) {
+ *val = env->ctr_dst[idx];
+ }
+
+ env->ctr_dst[idx] = (env->ctr_dst[idx] & ~wr_mask) | (new_val & wr_mask);
+
+ return 0;
+}
+
+static int rmw_xctrdata(CPURISCVState *env, int isel, target_ulong *val,
+ target_ulong new_val, target_ulong wr_mask)
+{
+ /*
+ * CTR arrays are treated as circular buffers and TOS always points to next
+ * empty slot, keeping TOS - 1 always pointing to latest entry. Given entry
+ * 0 is always the latest one, traversal is a bit different here. See the
+ * below example.
+ *
+ * Depth = 16.
+ *
+ * idx [0] [1] [2] [3] [4] [5] [6] [7] [8] [9] [A] [B] [C] [D] [E] [F]
+ * head H
+ * entry 6 5 4 3 2 1 0 F E D C B A 9 8 7
+ */
+ const uint64_t entry = isel - CTR_ENTRIES_FIRST;
+ const uint64_t mask = wr_mask & CTRDATA_MASK;
+ const uint64_t depth = 16 << get_field(env->sctrdepth, SCTRDEPTH_MASK);
+ uint64_t idx;
+
+ /* Entry greater than depth-1 is read-only zero */
+ if (entry >= depth) {
+ *val = 0;
+ return 0;
+ }
+
+ idx = get_field(env->sctrstatus, SCTRSTATUS_WRPTR_MASK);
+ idx = (idx - entry - 1) & (depth - 1);
+
+ if (val) {
+ *val = env->ctr_data[idx];
+ }
+
+ env->ctr_data[idx] = (env->ctr_data[idx] & ~mask) | (new_val & mask);
+
+ return 0;
+}
+
static RISCVException rmw_xireg_aia(CPURISCVState *env, int csrno,
target_ulong isel, target_ulong *val,
target_ulong new_val, target_ulong wr_mask)
@@ -2486,6 +2603,38 @@ done:
return ret;
}
+static int rmw_xireg_ctr(CPURISCVState *env, int csrno,
+ target_ulong isel, target_ulong *val,
+ target_ulong new_val, target_ulong wr_mask)
+{
+ bool ext_sxctr = false;
+ int ret = -EINVAL;
+
+ if (CSR_MIREG <= csrno && csrno <= CSR_MIREG3) {
+ ext_sxctr = riscv_cpu_cfg(env)->ext_smctr;
+ } else if (CSR_SIREG <= csrno && csrno <= CSR_SIREG3) {
+ ext_sxctr = riscv_cpu_cfg(env)->ext_ssctr;
+ } else if (CSR_VSIREG <= csrno && csrno <= CSR_VSIREG3) {
+ ext_sxctr = riscv_cpu_cfg(env)->ext_ssctr;
+ }
+
+ if (!ext_sxctr) {
+ return -EINVAL;
+ }
+
+ if (csrno == CSR_MIREG || csrno == CSR_SIREG || csrno == CSR_VSIREG) {
+ ret = rmw_xctrsource(env, isel, val, new_val, wr_mask);
+ } else if (csrno == CSR_MIREG2 || csrno == CSR_SIREG2 ||
+ csrno == CSR_VSIREG2) {
+ ret = rmw_xctrtarget(env, isel, val, new_val, wr_mask);
+ } else if (csrno == CSR_MIREG3 || csrno == CSR_SIREG3 ||
+ csrno == CSR_VSIREG3) {
+ ret = rmw_xctrdata(env, isel, val, new_val, wr_mask);
+ }
+
+ return ret;
+}
+
/*
* rmw_xireg_sxcsrind: Perform indirect access to xireg and xireg2-xireg6
*
@@ -2497,11 +2646,13 @@ static int rmw_xireg_sxcsrind(CPURISCVState *env, int csrno,
target_ulong isel, target_ulong *val,
target_ulong new_val, target_ulong wr_mask)
{
- int ret = -EINVAL;
bool virt = csrno == CSR_VSIREG ? true : false;
+ int ret = -EINVAL;
if (xiselect_cd_range(isel)) {
ret = rmw_xireg_cd(env, csrno, isel, val, new_val, wr_mask);
+ } else if (xiselect_ctr_range(isel)) {
+ ret = rmw_xireg_ctr(env, csrno, isel, val, new_val, wr_mask);
} else {
/*
* As per the specification, access to unimplented region is undefined
--
2.34.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH 0/6] target/riscv: Add support for Control Transfer Records Ext.
2024-05-29 16:09 [PATCH 0/6] target/riscv: Add support for Control Transfer Records Ext Rajnesh Kanwal
` (5 preceding siblings ...)
2024-05-29 16:09 ` [PATCH 6/6] target/riscv: Add support to access ctrsource, ctrtarget, ctrdata regs Rajnesh Kanwal
@ 2024-06-04 7:29 ` Jason Chien
2024-06-04 22:32 ` Beeman Strong
6 siblings, 1 reply; 22+ messages in thread
From: Jason Chien @ 2024-06-04 7:29 UTC (permalink / raw)
To: Rajnesh Kanwal, qemu-riscv, qemu-devel
Cc: alistair.francis, bin.meng, liweiwei, dbarboza, zhiwei_liu,
atishp, apatel, beeman, tech-control-transfer-records
Smctr depends on the Smcsrind extension, Ssctr depends on the Sscsrind
extension, and both Smctr and Ssctr depend upon implementation of S-mode.
There should be a dependency check in riscv_cpu_validate_set_extensions().
Rajnesh Kanwal 於 2024/5/30 上午 12:09 寫道:
> This series enables Control Transfer Records extension support on riscv
> platform. This extension is similar to Arch LBR in x86 and BRBE in ARM.
> The Extension has been stable and the latest release can be found here [0]
>
> CTR extension depends on couple of other extensions:
>
> 1. S[m|s]csrind : The indirect CSR extension [1] which defines additional
> ([M|S|VS]IREG2-[M|S|VS]IREG6) register to address size limitation of
> RISC-V CSR address space. CTR access ctrsource, ctrtartget and ctrdata
> CSRs using sscsrind extension.
>
> 2. Smstateen: The mstateen bit[54] controls the access to the CTR ext to
> S-mode.
>
> 3. Sscofpmf: Counter overflow and privilege mode filtering. [2]
>
> The series is based on Smcdeleg/Ssccfg counter delegation extension [3]
> patches. CTR itself doesn't depend on counter delegation support. This
> rebase is basically to include the Smcsrind patches.
>
> Due to the dependency of these extensions, the following extensions must be
> enabled to use the control transfer records feature.
>
> "smstateen=true,sscofpmf=true,smcsrind=true,sscsrind=true,smctr=true,ssctr=true"
>
> Here is the link to a quick guide [5] to setup and run a basic perf demo on
> Linux to use CTR Ext.
>
> The Qemu patches can be found here:
> https://github.com/rajnesh-kanwal/qemu/tree/ctr_upstream
>
> The opensbi patch can be found here:
> https://github.com/rajnesh-kanwal/opensbi/tree/ctr_upstream
>
> The Linux kernel patches can be found here:
> https://github.com/rajnesh-kanwal/linux/tree/ctr_upstream
>
> [0]:https://github.com/riscv/riscv-control-transfer-records/release
> [1]:https://github.com/riscv/riscv-indirect-csr-access
> [2]:https://github.com/riscvarchive/riscv-count-overflow/tree/main
> [3]:https://github.com/riscv/riscv-smcdeleg-ssccfg
> [4]:https://lore.kernel.org/all/20240217000134.3634191-1-atishp@rivosinc.com/
> [5]:https://github.com/rajnesh-kanwal/linux/wiki/Running-CTR-basic-demo-on-QEMU-RISC%E2%80%90V-Virt-machine
>
> Rajnesh Kanwal (6):
> target/riscv: Remove obsolete sfence.vm instruction
> target/riscv: Add Control Transfer Records CSR definitions.
> target/riscv: Add support for Control Transfer Records extension CSRs.
> target/riscv: Add support to record CTR entries.
> target/riscv: Add CTR sctrclr instruction.
> target/riscv: Add support to access ctrsource, ctrtarget, ctrdata
> regs.
>
> target/riscv/cpu.c | 4 +
> target/riscv/cpu.h | 14 +
> target/riscv/cpu_bits.h | 154 +++++++++
> target/riscv/cpu_cfg.h | 2 +
> target/riscv/cpu_helper.c | 213 ++++++++++++
> target/riscv/csr.c | 312 +++++++++++++++++-
> target/riscv/helper.h | 8 +-
> target/riscv/insn32.decode | 2 +-
> .../riscv/insn_trans/trans_privileged.c.inc | 21 +-
> target/riscv/insn_trans/trans_rvi.c.inc | 27 ++
> target/riscv/op_helper.c | 117 ++++++-
> target/riscv/translate.c | 9 +
> 12 files changed, 870 insertions(+), 13 deletions(-)
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 3/6] target/riscv: Add support for Control Transfer Records extension CSRs.
2024-05-29 16:09 ` [PATCH 3/6] target/riscv: Add support for Control Transfer Records extension CSRs Rajnesh Kanwal
@ 2024-06-04 10:14 ` Jason Chien
2024-06-10 14:11 ` Rajnesh Kanwal
0 siblings, 1 reply; 22+ messages in thread
From: Jason Chien @ 2024-06-04 10:14 UTC (permalink / raw)
To: Rajnesh Kanwal, qemu-riscv, qemu-devel
Cc: alistair.francis, bin.meng, liweiwei, dbarboza, zhiwei_liu,
atishp, apatel, beeman, tech-control-transfer-records
[-- Attachment #1: Type: text/plain, Size: 10771 bytes --]
Rajnesh Kanwal 於 2024/5/30 上午 12:09 寫道:
> This commit adds support for [m|s|vs]ctrcontrol, sctrstatus and
> sctrdepth CSRs handling.
>
> Signed-off-by: Rajnesh Kanwal<rkanwal@rivosinc.com>
> ---
> target/riscv/cpu.h | 5 ++
> target/riscv/cpu_cfg.h | 2 +
> target/riscv/csr.c | 159 +++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 166 insertions(+)
>
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index a185e2d494..3d4d5172b8 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -263,6 +263,11 @@ struct CPUArchState {
> target_ulong mcause;
> target_ulong mtval; /* since: priv-1.10.0 */
>
> + uint64_t mctrctl;
> + uint32_t sctrdepth;
> + uint32_t sctrstatus;
> + uint64_t vsctrctl;
> +
> /* Machine and Supervisor interrupt priorities */
> uint8_t miprio[64];
> uint8_t siprio[64];
> diff --git a/target/riscv/cpu_cfg.h b/target/riscv/cpu_cfg.h
> index d9354dc80a..d329a65811 100644
> --- a/target/riscv/cpu_cfg.h
> +++ b/target/riscv/cpu_cfg.h
> @@ -123,6 +123,8 @@ struct RISCVCPUConfig {
> bool ext_zvfhmin;
> bool ext_smaia;
> bool ext_ssaia;
> + bool ext_smctr;
> + bool ext_ssctr;
> bool ext_sscofpmf;
> bool ext_smepmp;
> bool rvv_ta_all_1s;
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index 2f92e4b717..888084d8e5 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -621,6 +621,61 @@ static RISCVException pointer_masking(CPURISCVState *env, int csrno)
> return RISCV_EXCP_ILLEGAL_INST;
> }
>
> +/*
> + * M-mode:
> + * Without ext_smctr raise illegal inst excep.
> + * Otherwise everything is accessible to m-mode.
> + *
> + * S-mode:
> + * Without ext_ssctr or mstateen.ctr raise illegal inst excep.
> + * Otherwise everything other than mctrctl is accessible.
> + *
> + * VS-mode:
> + * Without ext_ssctr or mstateen.ctr raise illegal inst excep.
> + * Without hstateen.ctr raise virtual illegal inst excep.
> + * Otherwise allow vsctrctl, sctrstatus, 0x200-0x2ff entry range.
> + * Always raise illegal instruction exception for sctrdepth.
> + */
> +static RISCVException ctr_mmode(CPURISCVState *env, int csrno)
> +{
> + /* Check if smctr-ext is present */
> + if (riscv_cpu_cfg(env)->ext_smctr) {
> + return RISCV_EXCP_NONE;
> + }
> +
> + return RISCV_EXCP_ILLEGAL_INST;
> +}
> +
> +static RISCVException ctr_smode(CPURISCVState *env, int csrno)
> +{
> + if ((env->priv == PRV_M && riscv_cpu_cfg(env)->ext_smctr) ||
> + (env->priv == PRV_S && !env->virt_enabled &&
> + riscv_cpu_cfg(env)->ext_ssctr)) {
> + return smstateen_acc_ok(env, 0, SMSTATEEN0_CTR);
> + }
> +
> + if (env->priv == PRV_S && env->virt_enabled &&
> + riscv_cpu_cfg(env)->ext_ssctr) {
> + if (csrno == CSR_SCTRSTATUS) {
missing sctrctl?
> + return smstateen_acc_ok(env, 0, SMSTATEEN0_CTR);
> + }
> +
> + return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
> + }
> +
> + return RISCV_EXCP_ILLEGAL_INST;
> +}
I think there is no need to bind M-mode with ext_smctr, S-mode with
ext_ssctr and VS-mode with ext_ssctr, since this predicate function is
for S-mode CSRs, which are defined in both smctr and ssctr, we just need
to check at least one of ext_ssctr or ext_smctr is true.
The spec states that:
Attempts to access sctrdepth from VS-mode or VU-mode raise a
virtual-instruction exception, unless CTR state enable access
restrictions apply.
In my understanding, we should check the presence of smstateen extension
first, and
if smstateen is implemented:
* for sctrctl and sctrstatus, call smstateen_acc_ok()
* for sctrdepth, call smstateen_acc_ok(), and if there is any
exception returned, always report virtual-instruction exception.
If smstateen is not implemented:
* for sctrctl and sctrstatus, there is no check.
* for sctrdepth, I think the spec is ambiguous. What does "CTR state
enable access restrictions apply" mean when smstateen is not
implemented?
Here is the code to better understand my description.
static RISCVException ctr_smode(CPURISCVState *env, int csrno)
{
const RISCVCPUConfig *cfg = riscv_cpu_cfg(env);
if (!cfg->ext_ssctr && !cfg->ext_smctr) {
return RISCV_EXCP_ILLEGAL_INST;
}
if (riscv_cpu_cfg(env)->ext_smstateen) {
RISCVException ret = smstateen_acc_ok(env, 0, SMSTATEEN0_CTR);
if (ret != RISCV_EXCP_NONE) {
if (csrno == CSR_SCTRDEPTH && env->virt_enabled) {
return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
}
return ret;
}
} else {
/* The spec is ambiguous. */
if (csrno == CSR_SCTRDEPTH && env->virt_enabled) {
return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
}
}
return RISCV_EXCP_NONE;
}
> +
> +static RISCVException ctr_vsmode(CPURISCVState *env, int csrno)
> +{
> + if (env->priv == PRV_S && env->virt_enabled &&
> + riscv_cpu_cfg(env)->ext_ssctr) {
> + return smstateen_acc_ok(env, 0, SMSTATEEN0_CTR);
In riscv_csrrw_check(), an virtual-instruction exception is always
reported no matter what. Do we need this check?
> + }
> +
> + return ctr_smode(env, csrno);
> +}
> +
> static RISCVException aia_hmode(CPURISCVState *env, int csrno)
> {
> int ret;
> @@ -3835,6 +3890,100 @@ static RISCVException write_satp(CPURISCVState *env, int csrno,
> return RISCV_EXCP_NONE;
> }
>
> +static RISCVException rmw_sctrdepth(CPURISCVState *env, int csrno,
> + target_ulong *ret_val,
> + target_ulong new_val, target_ulong wr_mask)
> +{
> + uint64_t mask = wr_mask & SCTRDEPTH_MASK;
> +
> + if (ret_val) {
> + *ret_val = env->sctrdepth & SCTRDEPTH_MASK;
We don't need to do bitwise and with SCTRDEPTH_MASK on read accesses
when we always do bitwise and with SCTRDEPTH_MASK on write accesses.
> + }
> +
> + env->sctrdepth = (env->sctrdepth & ~mask) | (new_val & mask);
> +
> + /* Correct depth. */
> + if (wr_mask & SCTRDEPTH_MASK) {
> + uint64_t depth = get_field(env->sctrdepth, SCTRDEPTH_MASK);
> +
> + if (depth > SCTRDEPTH_MAX) {
> + env->sctrdepth =
> + set_field(env->sctrdepth, SCTRDEPTH_MASK, SCTRDEPTH_MAX);
> + }
> +
> + /* Update sctrstatus.WRPTR with a legal value */
> + depth = 16 << depth;
The "depth" on the right side may exceed SCTRDEPTH_MAX.
> + env->sctrstatus =
> + env->sctrstatus & (~SCTRSTATUS_WRPTR_MASK | (depth - 1));
> + }
> +
> + return RISCV_EXCP_NONE;
> +}
> +
> +static RISCVException rmw_mctrctl(CPURISCVState *env, int csrno,
> + target_ulong *ret_val,
> + target_ulong new_val, target_ulong wr_mask)
> +{
> + uint64_t mask = wr_mask & MCTRCTL_MASK;
> +
> + if (ret_val) {
> + *ret_val = env->mctrctl & MCTRCTL_MASK;
There is no need to do bitwise and with the mask on read accesses when
we always do bitwise and with the mask on write accesses.
> + }
> +
> + env->mctrctl = (env->mctrctl & ~mask) | (new_val & mask);
> +
> + return RISCV_EXCP_NONE;
> +}
> +
> +static RISCVException rmw_sctrctl(CPURISCVState *env, int csrno,
> + target_ulong *ret_val,
> + target_ulong new_val, target_ulong wr_mask)
> +{
> + uint64_t mask = wr_mask & SCTRCTL_MASK;
> + RISCVException ret;
> +
> + ret = rmw_mctrctl(env, csrno, ret_val, new_val, mask);
When V=1, vsctrctl substitutes for sctrctl.
> + if (ret_val) {
> + *ret_val &= SCTRCTL_MASK;
> + }
> +
> + return ret;
> +}
> +
> +static RISCVException rmw_sctrstatus(CPURISCVState *env, int csrno,
> + target_ulong *ret_val,
> + target_ulong new_val, target_ulong wr_mask)
> +{
> + uint32_t depth = 16 << get_field(env->sctrdepth, SCTRDEPTH_MASK);
> + uint32_t mask = wr_mask & SCTRSTATUS_MASK;
> +
> + if (ret_val) {
> + *ret_val = env->sctrstatus & SCTRSTATUS_MASK;
There is no need to do bitwise and with the mask on read accesses when
we always do bitwise and with the mask on write accesses.
> + }
> +
> + env->sctrstatus = (env->sctrstatus & ~mask) | (new_val & mask);
> +
> + /* Update sctrstatus.WRPTR with a legal value */
> + env->sctrstatus = env->sctrstatus & (~SCTRSTATUS_WRPTR_MASK | (depth - 1));
> +
> + return RISCV_EXCP_NONE;
> +}
> +
> +static RISCVException rmw_vsctrctl(CPURISCVState *env, int csrno,
> + target_ulong *ret_val,
> + target_ulong new_val, target_ulong wr_mask)
> +{
> + uint64_t mask = wr_mask & VSCTRCTL_MASK;
> +
> + if (ret_val) {
> + *ret_val = env->vsctrctl & VSCTRCTL_MASK;
There is no need to do bitwise and with the mask on read accesses when
we always do bitwise and with the mask on write accesses.
> + }
> +
> + env->vsctrctl = (env->vsctrctl & ~mask) | (new_val & mask);
> +
> + return RISCV_EXCP_NONE;
> +}
Is it possible to define rmw_xctrctl() instead of three individual rmw
functions and use a switch case to select the mask and the CSR for the
purpose of reducing code size?
> +
> static RISCVException read_vstopi(CPURISCVState *env, int csrno,
> target_ulong *val)
> {
> @@ -5771,6 +5920,16 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
> [CSR_SPMBASE] = { "spmbase", pointer_masking, read_spmbase,
> write_spmbase },
>
> + [CSR_MCTRCTL] = { "mctrctl", ctr_mmode, NULL, NULL,
> + rmw_mctrctl },
I think this can be one line.
> + [CSR_SCTRCTL] = { "sctrctl", ctr_smode, NULL, NULL,
> + rmw_sctrctl },
same here
> + [CSR_SCTRDEPTH] = { "sctrdepth", ctr_smode, NULL, NULL,
> + rmw_sctrdepth },
same here
> + [CSR_SCTRSTATUS] = { "sctrstatus", ctr_smode, NULL, NULL,
> + rmw_sctrstatus },
same here
> + [CSR_VSCTRCTL] = { "vsctrctl", ctr_vsmode, NULL, NULL,
> + rmw_vsctrctl },
same here
> /* Performance Counters */
> [CSR_HPMCOUNTER3] = { "hpmcounter3", ctr, read_hpmcounter },
> [CSR_HPMCOUNTER4] = { "hpmcounter4", ctr, read_hpmcounter },
[-- Attachment #2: Type: text/html, Size: 13997 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 4/6] target/riscv: Add support to record CTR entries.
2024-05-29 16:09 ` [PATCH 4/6] target/riscv: Add support to record CTR entries Rajnesh Kanwal
@ 2024-06-04 16:30 ` Jason Chien
0 siblings, 0 replies; 22+ messages in thread
From: Jason Chien @ 2024-06-04 16:30 UTC (permalink / raw)
To: Rajnesh Kanwal, qemu-riscv, qemu-devel
Cc: alistair.francis, bin.meng, liweiwei, dbarboza, zhiwei_liu,
atishp, apatel, beeman, tech-control-transfer-records
This commit is missing CTR for cm.jalt, cm.jt, cm.popret, cm.popretz.
Rajnesh Kanwal 於 2024/5/30 上午 12:09 寫道:
> This commit adds logic to records CTR entries of different types
> and adds required hooks in TCG and interrupt/Exception logic to
> record events.
>
> This commit also adds support to invoke freeze CTR logic for breakpoint
> exceptions and counter overflow interrupts.
>
> Signed-off-by: Rajnesh Kanwal <rkanwal@rivosinc.com>
> ---
> target/riscv/cpu.h | 8 +
> target/riscv/cpu_helper.c | 206 ++++++++++++++++++
> target/riscv/helper.h | 8 +-
> .../riscv/insn_trans/trans_privileged.c.inc | 6 +-
> target/riscv/insn_trans/trans_rvi.c.inc | 27 +++
> target/riscv/op_helper.c | 112 +++++++++-
> target/riscv/translate.c | 9 +
> 7 files changed, 370 insertions(+), 6 deletions(-)
>
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 3d4d5172b8..a294a5372a 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -268,6 +268,10 @@ struct CPUArchState {
> uint32_t sctrstatus;
> uint64_t vsctrctl;
>
> + uint64_t ctr_src[16 << SCTRDEPTH_MAX];
> + uint64_t ctr_dst[16 << SCTRDEPTH_MAX];
> + uint64_t ctr_data[16 << SCTRDEPTH_MAX];
> +
> /* Machine and Supervisor interrupt priorities */
> uint8_t miprio[64];
> uint8_t siprio[64];
> @@ -565,6 +569,10 @@ RISCVException smstateen_acc_ok(CPURISCVState *env, int index, uint64_t bit);
> #endif
> void riscv_cpu_set_mode(CPURISCVState *env, target_ulong newpriv, bool virt_en);
>
> +void riscv_ctr_freeze(CPURISCVState *env, uint64_t freeze_mask);
> +void riscv_ctr_add_entry(CPURISCVState *env, target_long src, target_long dst,
> + uint64_t type, target_ulong prev_priv, bool prev_virt);
> +
> void riscv_translate_init(void);
> G_NORETURN void riscv_raise_exception(CPURISCVState *env,
> uint32_t exception, uintptr_t pc);
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index a441a03ef4..e064a7306e 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -663,6 +663,10 @@ uint64_t riscv_cpu_update_mip(CPURISCVState *env, uint64_t mask, uint64_t value)
>
> BQL_LOCK_GUARD();
>
> + if (MIP_LCOFIP & value & mask) {
> + riscv_ctr_freeze(env, MCTRCTL_LCOFIFRZ);
> + }
> +
> env->mip = (env->mip & ~mask) | (value & mask);
>
> riscv_cpu_interrupt(env);
> @@ -691,6 +695,197 @@ void riscv_cpu_set_aia_ireg_rmw_fn(CPURISCVState *env, uint32_t priv,
> }
> }
>
> +void riscv_ctr_freeze(CPURISCVState *env, uint64_t freeze_mask)
> +{
If both smctr and ssctr are not enabled, return immediately. Or, do the
check before invoking riscv_ctr_freeze().
> + assert((freeze_mask & (~(MCTRCTL_BPFRZ | MCTRCTL_LCOFIFRZ))) == 0);
> +
> + if (env->mctrctl & freeze_mask) {
If the trap is handled in VS-mode, we should check vsctrctl instead:
if (((env->virt_enabled) ? env->vsctrctl : env->mctrctl) & freeze_mask)
> + env->sctrstatus |= SCTRSTATUS_FROZEN;
> + }
> +}
> +
> +static uint64_t riscv_ctr_priv_to_mask(target_ulong priv, bool virt)
> +{
> + switch (priv) {
> + case PRV_M:
> + return MCTRCTL_M_ENABLE;
> + case PRV_S:
> + if (virt) {
> + return VSCTRCTL_VS_ENABLE;
> + }
> + return MCTRCTL_S_ENABLE;
> + case PRV_U:
> + if (virt) {
> + return VSCTRCTL_VU_ENABLE;
> + }
> + return MCTRCTL_U_ENABLE;
> + }
> +
> + g_assert_not_reached();
> +}
> +
> +static uint64_t riscv_ctr_get_control(CPURISCVState *env, target_long priv,
> + bool virt)
> +{
> + switch (priv) {
> + case PRV_M:
> + return env->mctrctl;
> + case PRV_S:
> + case PRV_U:
> + if (virt) {
> + return env->vsctrctl;
> + }
> + return env->mctrctl;
> + }
> +
> + g_assert_not_reached();
> +}
> +
> +/*
> + * Special cases for traps and trap returns:
> + *
> + * 1- Traps, and trap returns, between enabled modes are recorded as normal.
> + * 2- Traps from an inhibited mode to an enabled mode, and trap returns from an
> + * enabled mode back to an inhibited mode, are partially recorded. In such
> + * cases, the PC from the inhibited mode (source PC for traps, and target PC
> + * for trap returns) is 0.
> + *
> + * 3- Trap returns from an inhibited mode to an enabled mode are not recorded.
> + * Traps from an enabled mode to an inhibited mode, known as external traps,
> + * receive special handling.
> + * By default external traps are not recorded, but a handshake mechanism exists
> + * to allow partial recording. Software running in the target mode of the trap
> + * can opt-in to allowing CTR to record traps into that mode even when the mode
> + * is inhibited. The MTE, STE, and VSTE bits allow M-mode, S-mode, and VS-mode,
> + * respectively, to opt-in. When an External Trap occurs, and xTE=1, such that
> + * x is the target privilege mode of the trap, will CTR record the trap. In such
> + * cases, the target PC is 0.
> + */
> +/*
> + * CTR arrays are implemented as circular buffers and new entry is stored at
> + * sctrstatus.WRPTR, but they are presented to software as moving circular
> + * buffers. Which means, software get's the illusion that whenever a new entry
> + * is added the whole buffer is moved by one place and the new entry is added at
> + * the start keeping new entry at idx 0 and older ones follow.
> + *
> + * Depth = 16.
> + *
> + * buffer [0] [1] [2] [3] [4] [5] [6] [7] [8] [9] [A] [B] [C] [D] [E] [F]
> + * WRPTR W
> + * entry 7 6 5 4 3 2 1 0 F E D C B A 9 8
> + *
> + * When a new entry is added:
> + * buffer [0] [1] [2] [3] [4] [5] [6] [7] [8] [9] [A] [B] [C] [D] [E] [F]
> + * WRPTR W
> + * entry 8 7 6 5 4 3 2 1 0 F E D C B A 9
> + *
> + * entry here denotes the logical entry number that software can access
> + * using ctrsource, ctrtarget and ctrdata registers. So xiselect 0x200
> + * will return entry 0 i-e buffer[8] and 0x201 will return entry 1 i-e
> + * buffer[7]. Here is how we convert entry to buffer idx.
> + *
> + * entry = isel - CTR_ENTRIES_FIRST;
> + * idx = (sctrstatus.WRPTR - entry - 1) & (depth - 1);
> + */
> +void riscv_ctr_add_entry(CPURISCVState *env, target_long src, target_long dst,
> + uint64_t type, target_ulong src_priv, bool src_virt)
> +{
> + bool tgt_virt = env->virt_enabled;
> + uint64_t src_mask = riscv_ctr_priv_to_mask(src_priv, src_virt);
> + uint64_t tgt_mask = riscv_ctr_priv_to_mask(env->priv, tgt_virt);
> + uint64_t src_ctrl = riscv_ctr_get_control(env, src_priv, src_virt);
> + uint64_t tgt_ctrl = riscv_ctr_get_control(env, env->priv, tgt_virt);
> + uint64_t depth, head;
> + bool ext_trap = false;
> +
If both smctr and ssctr are disabled, return immediately. Or, do the
check before invoking riscv_ctr_add_entry().
> + if (env->sctrstatus & SCTRSTATUS_FROZEN) {
> + return;
> + }
> +
if both source mode and target mode are disabled for CTR recording,
return immediately.
> + /*
> + * With RAS Emul enabled, only allow Indirect, drirect calls, Function
> + * returns and Co-routine swap types.
> + */
> + if (env->mctrctl & MCTRCTL_RASEMU &&
> + type != CTRDATA_TYPE_INDIRECT_CALL &&
> + type != CTRDATA_TYPE_DIRECT_CALL &&
> + type != CTRDATA_TYPE_RETURN &&
> + type != CTRDATA_TYPE_CO_ROUTINE_SWAP) {
> + return;
> + }
> +
> + if (type == CTRDATA_TYPE_EXCEPTION || type == CTRDATA_TYPE_INTERRUPT) {
> + /* Case 2 for traps. */
> + if (!(src_ctrl & src_mask) && (tgt_ctrl & tgt_mask)) {
Since we have check that at least one of the source mode and target mode
is enabled, we only need to check:
if (!(src_ctrl & src_mask))
> + src = 0;
> + } else if ((src_ctrl & src_mask) && !(tgt_ctrl & tgt_mask)) {
Since we have check that at least one of the source mode and target mode
is enabled, we only need to check:
if (!(tgt_ctrl & tgt_mask))
> + /* Check if target priv-mode has allowed external trap recording. */
> + if ((env->priv == PRV_M && !(tgt_ctrl & MCTRCTL_MTE)) ||
> + (env->priv == PRV_S && !(tgt_ctrl & MCTRCTL_STE))) {
External trap recording depends not only on the target mode, but on any
intervening modes, which are modes that are more privileged than the
source mode but less privileged than the target mode.
Please refer to section 6.1.2.
> + return;
> + }
> +
> + ext_trap = true;
> + dst = 0;
> + } else if (!(src_ctrl & src_mask) && !(tgt_ctrl & tgt_mask)) {
This scope can be removed.
> + return;
> + }
> + } else if (type == CTRDATA_TYPE_EXCEP_INT_RET) {
> + /*
> + * Case 3 for trap returns. Trap returns from inhibited mode are not
> + * recorded.
> + */
> + if (!(src_ctrl & src_mask)) {
> + return;
> + }
> +
> + /* Case 2 for trap returns. */
> + if (!(tgt_ctrl & tgt_mask)) {
> + dst = 0;
> + }
> + } else if (!(tgt_ctrl & tgt_mask)) {
Only trap and trap return change modes. The target mode is the source
mode in this scope, and they are both disabled. Thus this scope can be
removed.
> + return;
> + }
> +
> + /* Ignore filters in case of RASEMU mode or External trap. */
> + if (!(tgt_ctrl & MCTRCTL_RASEMU) && !ext_trap) {
> + /*
> + * Check if the specific type is inhibited. Not taken branch filter is
> + * an enable bit and needs to be checked separatly.
> + */
> + bool check = tgt_ctrl & BIT_ULL(type + MCTRCTL_INH_START);
> + if ((type == CTRDATA_TYPE_NONTAKEN_BRANCH && !check) ||
> + (type != CTRDATA_TYPE_NONTAKEN_BRANCH && check)) {
> + return;
> + }
> + }
> +
> + head = get_field(env->sctrstatus, SCTRSTATUS_WRPTR_MASK);
> +
> + depth = 16 << get_field(env->sctrdepth, SCTRDEPTH_MASK);
> + if (tgt_ctrl & MCTRCTL_RASEMU && type == CTRDATA_TYPE_RETURN) {
> + head = (head - 1) & (depth - 1);
> +
> + env->ctr_src[head] &= ~CTRSOURCE_VALID;
> + env->sctrstatus =
> + set_field(env->sctrstatus, SCTRSTATUS_WRPTR_MASK, head);
> + return;
> + }
> +
> + /* In case of Co-routine SWAP we overwrite latest entry. */
> + if (tgt_ctrl & MCTRCTL_RASEMU && type == CTRDATA_TYPE_CO_ROUTINE_SWAP) {
> + head = (head - 1) & (depth - 1);
> + }
> +
> + env->ctr_src[head] = src | CTRSOURCE_VALID;
> + env->ctr_dst[head] = dst & ~CTRTARGET_MISP;
> + env->ctr_data[head] = set_field(0, CTRDATA_TYPE_MASK, type);
> +
> + head = (head + 1) & (depth - 1);
> +
> + env->sctrstatus = set_field(env->sctrstatus, SCTRSTATUS_WRPTR_MASK, head);
> +}
> +
> void riscv_cpu_set_mode(CPURISCVState *env, target_ulong newpriv, bool virt_en)
> {
> g_assert(newpriv <= PRV_M && newpriv != PRV_RESERVED);
> @@ -1669,10 +1864,13 @@ void riscv_cpu_do_interrupt(CPUState *cs)
> !(env->mip & (1 << cause));
> bool vs_injected = env->hvip & (1 << cause) & env->hvien &&
> !(env->mip & (1 << cause));
> + const bool prev_virt = env->virt_enabled;
> + const target_ulong prev_priv = env->priv;
> target_ulong tval = 0;
> target_ulong tinst = 0;
> target_ulong htval = 0;
> target_ulong mtval2 = 0;
> + target_ulong src;
>
> if (!async) {
> /* set tval to badaddr for traps with address information */
> @@ -1729,6 +1927,7 @@ void riscv_cpu_do_interrupt(CPUState *cs)
> tval = cs->watchpoint_hit->hitaddr;
> cs->watchpoint_hit = NULL;
> }
> + riscv_ctr_freeze(env, MCTRCTL_BPFRZ);
> break;
> default:
> break;
> @@ -1807,6 +2006,8 @@ void riscv_cpu_do_interrupt(CPUState *cs)
> env->pc = (env->stvec >> 2 << 2) +
> ((async && (env->stvec & 3) == 1) ? cause * 4 : 0);
> riscv_cpu_set_mode(env, PRV_S, virt);
> +
> + src = env->sepc;
> } else {
> /* handle the trap in M-mode */
> if (riscv_has_ext(env, RVH)) {
> @@ -1838,8 +2039,13 @@ void riscv_cpu_do_interrupt(CPUState *cs)
> env->pc = (env->mtvec >> 2 << 2) +
> ((async && (env->mtvec & 3) == 1) ? cause * 4 : 0);
> riscv_cpu_set_mode(env, PRV_M, virt);
> + src = env->mepc;
> }
I think it is more clear to doing the freeze check here for both the
breakpoint exception and the local counter overflow interrupt than doing
the freeze check seperately.
if (async && cause == IRQ_PMU_OVF) {
riscv_ctr_freeze(env, CTRCTL_LCOFIFRZ);
} else if (!async && RISCV_EXCP_BREAKPOINT) {
riscv_ctr_freeze(env, CTRCTL_BPFRZ);
}
>
> + riscv_ctr_add_entry(env, src, env->pc,
> + async ? CTRDATA_TYPE_INTERRUPT : CTRDATA_TYPE_EXCEPTION,
> + prev_priv, prev_virt);
> +
We should check that at least one of smctr and ssctr is enabled before
invoking riscv_ctr_add_entry().
> /*
> * NOTE: it is not necessary to yield load reservations here. It is only
> * necessary for an SC from "another hart" to cause a load reservation
> diff --git a/target/riscv/helper.h b/target/riscv/helper.h
> index 451261ce5a..0a9a545d87 100644
> --- a/target/riscv/helper.h
> +++ b/target/riscv/helper.h
> @@ -129,12 +129,16 @@ DEF_HELPER_2(csrr_i128, tl, env, int)
> DEF_HELPER_4(csrw_i128, void, env, int, tl, tl)
> DEF_HELPER_6(csrrw_i128, tl, env, int, tl, tl, tl, tl)
> #ifndef CONFIG_USER_ONLY
> -DEF_HELPER_1(sret, tl, env)
> -DEF_HELPER_1(mret, tl, env)
> +DEF_HELPER_2(sret, tl, env, tl)
> +DEF_HELPER_2(mret, tl, env, tl)
> +DEF_HELPER_1(ctr_clear, void, env)
"ctr_clear" should be in the next commit.
> DEF_HELPER_1(wfi, void, env)
> DEF_HELPER_1(wrs_nto, void, env)
> DEF_HELPER_1(tlb_flush, void, env)
> DEF_HELPER_1(tlb_flush_all, void, env)
> +DEF_HELPER_4(ctr_branch, void, env, tl, tl, tl)
> +DEF_HELPER_4(ctr_jal, void, env, tl, tl, tl)
> +DEF_HELPER_5(ctr_jalr, void, env, tl, tl, tl, tl)
> /* Native Debug */
> DEF_HELPER_1(itrigger_match, void, env)
> #endif
> diff --git a/target/riscv/insn_trans/trans_privileged.c.inc b/target/riscv/insn_trans/trans_privileged.c.inc
> index 4eccdddeaa..339d659151 100644
> --- a/target/riscv/insn_trans/trans_privileged.c.inc
> +++ b/target/riscv/insn_trans/trans_privileged.c.inc
> @@ -78,9 +78,10 @@ static bool trans_sret(DisasContext *ctx, arg_sret *a)
> {
> #ifndef CONFIG_USER_ONLY
> if (has_ext(ctx, RVS)) {
> + TCGv src = tcg_constant_tl(ctx->base.pc_next);
> decode_save_opc(ctx);
> translator_io_start(&ctx->base);
> - gen_helper_sret(cpu_pc, tcg_env);
> + gen_helper_sret(cpu_pc, tcg_env, src);
> exit_tb(ctx); /* no chaining */
> ctx->base.is_jmp = DISAS_NORETURN;
> } else {
> @@ -95,9 +96,10 @@ static bool trans_sret(DisasContext *ctx, arg_sret *a)
> static bool trans_mret(DisasContext *ctx, arg_mret *a)
> {
> #ifndef CONFIG_USER_ONLY
> + TCGv src = tcg_constant_tl(ctx->base.pc_next);
> decode_save_opc(ctx);
> translator_io_start(&ctx->base);
> - gen_helper_mret(cpu_pc, tcg_env);
> + gen_helper_mret(cpu_pc, tcg_env, src);
> exit_tb(ctx); /* no chaining */
> ctx->base.is_jmp = DISAS_NORETURN;
> return true;
> diff --git a/target/riscv/insn_trans/trans_rvi.c.inc b/target/riscv/insn_trans/trans_rvi.c.inc
> index ad40d3e87f..7f95362b66 100644
> --- a/target/riscv/insn_trans/trans_rvi.c.inc
> +++ b/target/riscv/insn_trans/trans_rvi.c.inc
> @@ -55,6 +55,11 @@ static bool trans_jalr(DisasContext *ctx, arg_jalr *a)
> TCGLabel *misaligned = NULL;
> TCGv target_pc = tcg_temp_new();
> TCGv succ_pc = dest_gpr(ctx, a->rd);
> +#ifndef CONFIG_USER_ONLY
> + TCGv rd = tcg_constant_tl(a->rd);
> + TCGv rs1 = tcg_constant_tl(a->rs1);
> + TCGv src = tcg_constant_tl(ctx->base.pc_next);
> +#endif
>
> tcg_gen_addi_tl(target_pc, get_gpr(ctx, a->rs1, EXT_NONE), a->imm);
> tcg_gen_andi_tl(target_pc, target_pc, (target_ulong)-2);
> @@ -75,6 +80,9 @@ static bool trans_jalr(DisasContext *ctx, arg_jalr *a)
> gen_set_gpr(ctx, a->rd, succ_pc);
>
> tcg_gen_mov_tl(cpu_pc, target_pc);
> +#ifndef CONFIG_USER_ONLY
> + gen_helper_ctr_jalr(tcg_env, src, cpu_pc, rd, rs1);
A call to the helper function should be generated only when at least one
of smctr and ssctr is enabled. Otherwise, the helper function is always
executed even if both smctr and ssctr are disabled, which impacts QEMU
performance.
> +#endif
> lookup_and_goto_ptr(ctx);
>
> if (misaligned) {
> @@ -164,6 +172,11 @@ static bool gen_branch(DisasContext *ctx, arg_b *a, TCGCond cond)
> TCGv src1 = get_gpr(ctx, a->rs1, EXT_SIGN);
> TCGv src2 = get_gpr(ctx, a->rs2, EXT_SIGN);
> target_ulong orig_pc_save = ctx->pc_save;
> +#ifndef CONFIG_USER_ONLY
> + TCGv src = tcg_constant_tl(ctx->base.pc_next);
> + TCGv taken;
> + TCGv dest;
> +#endif
>
> if (get_xl(ctx) == MXL_RV128) {
> TCGv src1h = get_gprh(ctx, a->rs1);
> @@ -176,6 +189,14 @@ static bool gen_branch(DisasContext *ctx, arg_b *a, TCGCond cond)
> } else {
> tcg_gen_brcond_tl(cond, src1, src2, l);
> }
> +
> +#ifndef CONFIG_USER_ONLY
> + dest = tcg_constant_tl(ctx->base.pc_next + ctx->cur_insn_len);
> + taken = tcg_constant_tl(0);
> +
> + gen_helper_ctr_branch(tcg_env, src, dest, taken);
A call to the helper function should be generated only when at least one
of smctr and ssctr is enabled. Otherwise, the helper function is always
executed even if both smctr and ssctr are disabled, which impacts QEMU
performance.
> +#endif
> +
> gen_goto_tb(ctx, 1, ctx->cur_insn_len);
> ctx->pc_save = orig_pc_save;
>
> @@ -188,6 +209,12 @@ static bool gen_branch(DisasContext *ctx, arg_b *a, TCGCond cond)
> gen_pc_plus_diff(target_pc, ctx, a->imm);
> gen_exception_inst_addr_mis(ctx, target_pc);
> } else {
> +#ifndef CONFIG_USER_ONLY
> + dest = tcg_constant_tl(ctx->base.pc_next + a->imm);
> + taken = tcg_constant_tl(1);
> +
> + gen_helper_ctr_branch(tcg_env, src, dest, taken);
A call to the helper function should be generated only when at least one
of smctr and ssctr is enabled. Otherwise, the helper function is always
executed even if both smctr and ssctr are disabled, which impacts QEMU
performance.
> +#endif
> gen_goto_tb(ctx, 0, a->imm);
> }
> ctx->pc_save = -1;
> diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
> index 25a5263573..c8053d9c2f 100644
> --- a/target/riscv/op_helper.c
> +++ b/target/riscv/op_helper.c
> @@ -259,10 +259,12 @@ void helper_cbo_inval(CPURISCVState *env, target_ulong address)
>
> #ifndef CONFIG_USER_ONLY
>
> -target_ulong helper_sret(CPURISCVState *env)
> +target_ulong helper_sret(CPURISCVState *env, target_ulong curr_pc)
> {
> uint64_t mstatus;
> target_ulong prev_priv, prev_virt = env->virt_enabled;
> + const target_ulong src_priv = env->priv;
> + const bool src_virt = env->virt_enabled;
>
> if (!(env->priv >= PRV_S)) {
> riscv_raise_exception(env, RISCV_EXCP_ILLEGAL_INST, GETPC());
> @@ -309,11 +311,17 @@ target_ulong helper_sret(CPURISCVState *env)
>
> riscv_cpu_set_mode(env, prev_priv, prev_virt);
>
> + riscv_ctr_add_entry(env, curr_pc, retpc, CTRDATA_TYPE_EXCEP_INT_RET,
> + src_priv, src_virt);
We should check that at least one of smctr and ssctr is enabled before
invoking riscv_ctr_add_entry().
> +
> return retpc;
> }
>
> -target_ulong helper_mret(CPURISCVState *env)
> +target_ulong helper_mret(CPURISCVState *env, target_ulong curr_pc)
> {
> + const target_ulong src_priv = env->priv;
> + const bool src_virt = env->virt_enabled;
> +
> if (!(env->priv >= PRV_M)) {
> riscv_raise_exception(env, RISCV_EXCP_ILLEGAL_INST, GETPC());
> }
> @@ -350,9 +358,109 @@ target_ulong helper_mret(CPURISCVState *env)
>
> riscv_cpu_set_mode(env, prev_priv, prev_virt);
>
> + riscv_ctr_add_entry(env, curr_pc, retpc, CTRDATA_TYPE_EXCEP_INT_RET,
> + src_priv, src_virt);
> +
We should check that at least one of smctr and ssctr is enabled before
invoking riscv_ctr_add_entry().
src_priv can be replaced with "PRV_M" and src_virt can be replaced with
"false".
Thus there is no need to declare src_priv and src_virt.
> return retpc;
> }
>
> +/*
> + * Indirect calls
> + * �� jalr x1, rs where rs != x5;
> + * �� jalr x5, rs where rs != x1;
> + * �� c.jalr rs1 where rs1 != x5;
> + *
> + * Indirect jumps
> + * �� jalr x0, rs where rs != x1 and rs != x5;
> + * �� c.jr rs1 where rs1 != x1 and rs1 != x5.
> + *
> + * Returns
> + * �� jalr rd, rs where (rs == x1 or rs == x5) and rd != x1 and rd != x5;
> + * �� c.jr rs1 where rs1 == x1 or rs1 == x5.
> + *
> + * Co-routine swap
> + * �� jalr x1, x5;
> + * �� jalr x5, x1;
> + * �� c.jalr x5.
> + *
> + * Other indirect jumps
> + * �� jalr rd, rs where rs != x1, rs != x5, rd != x0, rd != x1 and rd != x5.
> + */
> +void helper_ctr_jalr(CPURISCVState *env, target_ulong src, target_ulong dest,
> + target_ulong rd, target_ulong rs1)
> +{
> + target_ulong curr_priv = env->priv;
> + bool curr_virt = env->virt_enabled;
> +
> + if ((rd == 1 && rs1 != 5) || (rd == 5 && rs1 != 1)) {
> + riscv_ctr_add_entry(env, src, dest, CTRDATA_TYPE_INDIRECT_CALL,
> + curr_priv, curr_virt);
> + } else if (rd == 0 && rs1 != 1 && rs1 != 5) {
> + riscv_ctr_add_entry(env, src, dest, CTRDATA_TYPE_INDIRECT_JUMP,
> + curr_priv, curr_virt);
> + } else if ((rs1 == 1 || rs1 == 5) && (rd != 1 && rd != 5)) {
> + riscv_ctr_add_entry(env, src, dest, CTRDATA_TYPE_RETURN,
> + curr_priv, curr_virt);
> + } else if ((rs1 == 1 && rd == 5) || (rs1 == 5 && rd == 1)) {
> + riscv_ctr_add_entry(env, src, dest, CTRDATA_TYPE_CO_ROUTINE_SWAP,
> + curr_priv, curr_virt);
> + } else {
> + riscv_ctr_add_entry(env, src, dest,
> + CTRDATA_TYPE_OTHER_INDIRECT_JUMP, curr_priv,
> + curr_virt);
> + }
> +}
> +
> +/*
> + * Direct calls
> + * �� jal x1;
> + * �� jal x5;
> + * �� c.jal.
> + *
> + * Direct jumps
> + * �� jal x0;
> + * �� c.j;
> + *
> + * Other direct jumps
> + * �� jal rd where rd != x1 and rd != x5 and rd != x0;
> + */
> +void helper_ctr_jal(CPURISCVState *env, target_ulong src, target_ulong dest,
> + target_ulong rd)
> +{
> + target_ulong priv = env->priv;
> + bool virt = env->virt_enabled;
> +
> + /*
> + * If rd is x1 or x5 link registers, treat this as direct call otherwise
> + * its a direct jump.
> + */
> + if (rd == 1 || rd == 5) {
> + riscv_ctr_add_entry(env, src, dest, CTRDATA_TYPE_DIRECT_CALL, priv,
> + virt);
> + } else if (rd == 0) {
> + riscv_ctr_add_entry(env, src, dest, CTRDATA_TYPE_DIRECT_JUMP, priv,
> + virt);
> + } else {
> + riscv_ctr_add_entry(env, src, dest, CTRDATA_TYPE_OTHER_DIRECT_JUMP,
> + priv, virt);
> + }
> +}
> +
> +void helper_ctr_branch(CPURISCVState *env, target_ulong src, target_ulong dest,
> + target_ulong branch_taken)
> +{
> + target_ulong curr_priv = env->priv;
> + bool curr_virt = env->virt_enabled;
> +
> + if (branch_taken) {
> + riscv_ctr_add_entry(env, src, dest, CTRDATA_TYPE_TAKEN_BRANCH,
> + curr_priv, curr_virt);
> + } else {
> + riscv_ctr_add_entry(env, src, dest, CTRDATA_TYPE_NONTAKEN_BRANCH,
> + curr_priv, curr_virt);
> + }
> +}
> +
> void helper_wfi(CPURISCVState *env)
> {
> CPUState *cs = env_cpu(env);
> diff --git a/target/riscv/translate.c b/target/riscv/translate.c
> index 15e7123a68..8b0492991d 100644
> --- a/target/riscv/translate.c
> +++ b/target/riscv/translate.c
> @@ -561,6 +561,11 @@ static void gen_set_fpr_d(DisasContext *ctx, int reg_num, TCGv_i64 t)
> static void gen_jal(DisasContext *ctx, int rd, target_ulong imm)
> {
> TCGv succ_pc = dest_gpr(ctx, rd);
> +#ifndef CONFIG_USER_ONLY
> + TCGv dest = tcg_constant_tl(ctx->base.pc_next + imm);
> + TCGv src = tcg_constant_tl(ctx->base.pc_next);
> + TCGv tcg_rd = tcg_constant_tl((target_ulong)rd);
> +#endif
>
> /* check misaligned: */
> if (!has_ext(ctx, RVC) && !ctx->cfg_ptr->ext_zca) {
> @@ -572,6 +577,10 @@ static void gen_jal(DisasContext *ctx, int rd, target_ulong imm)
> }
> }
>
> +#ifndef CONFIG_USER_ONLY
> + gen_helper_ctr_jal(tcg_env, src, dest, tcg_rd);
A call to the helper function should be generated only when at least one
of smctr and ssctr is enabled. Otherwise, the helper function is always
executed even if both smctr and ssctr are disabled, which impacts QEMU
performance.
> +#endif
> +
> gen_pc_plus_diff(succ_pc, ctx, ctx->cur_insn_len);
> gen_set_gpr(ctx, rd, succ_pc);
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 6/6] target/riscv: Add support to access ctrsource, ctrtarget, ctrdata regs.
2024-05-29 16:09 ` [PATCH 6/6] target/riscv: Add support to access ctrsource, ctrtarget, ctrdata regs Rajnesh Kanwal
@ 2024-06-04 17:07 ` Jason Chien
0 siblings, 0 replies; 22+ messages in thread
From: Jason Chien @ 2024-06-04 17:07 UTC (permalink / raw)
To: Rajnesh Kanwal, qemu-riscv, qemu-devel
Cc: alistair.francis, bin.meng, liweiwei, dbarboza, zhiwei_liu,
atishp, apatel, beeman, tech-control-transfer-records
Rajnesh Kanwal 於 2024/5/30 上午 12:09 寫道:
> CTR entries are accessed using ctrsource, ctrtarget and ctrdata
> registers using smcsrind/sscsrind extension. This commits extends
> the csrind extension to support CTR registers.
>
> ctrsource is accessible through xireg CSR, ctrtarget is accessible
> through xireg1 and ctrdata is accessible through xireg2 CSR.
>
> CTR supports maximum depth of 256 entries which are accessed using
> xiselect range 0x200 to 0x2ff.
>
> This commits also adds properties to enable CTR extension. CTR can be
> enabled using smctr=true and ssctr=true now.
>
> Signed-off-by: Rajnesh Kanwal <rkanwal@rivosinc.com>
> ---
> target/riscv/cpu.c | 4 ++
> target/riscv/csr.c | 153 ++++++++++++++++++++++++++++++++++++++++++++-
> 2 files changed, 156 insertions(+), 1 deletion(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 30bdfc22ae..a77b1d5caf 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -193,6 +193,8 @@ const RISCVIsaExtData isa_edata_arr[] = {
> ISA_EXT_DATA_ENTRY(sstvala, PRIV_VERSION_1_12_0, has_priv_1_12),
> ISA_EXT_DATA_ENTRY(sstvecd, PRIV_VERSION_1_12_0, has_priv_1_12),
> ISA_EXT_DATA_ENTRY(svade, PRIV_VERSION_1_11_0, ext_svade),
> + ISA_EXT_DATA_ENTRY(smctr, PRIV_VERSION_1_12_0, ext_smctr),
> + ISA_EXT_DATA_ENTRY(ssctr, PRIV_VERSION_1_12_0, ext_ssctr),
> ISA_EXT_DATA_ENTRY(svadu, PRIV_VERSION_1_12_0, ext_svadu),
> ISA_EXT_DATA_ENTRY(svinval, PRIV_VERSION_1_12_0, ext_svinval),
> ISA_EXT_DATA_ENTRY(svnapot, PRIV_VERSION_1_12_0, ext_svnapot),
> @@ -1473,6 +1475,8 @@ const RISCVCPUMultiExtConfig riscv_cpu_extensions[] = {
> MULTI_EXT_CFG_BOOL("sscsrind", ext_sscsrind, false),
> MULTI_EXT_CFG_BOOL("smcdeleg", ext_smcdeleg, false),
> MULTI_EXT_CFG_BOOL("ssccfg", ext_ssccfg, false),
> + MULTI_EXT_CFG_BOOL("smctr", ext_smctr, false),
> + MULTI_EXT_CFG_BOOL("ssctr", ext_ssctr, false),
> MULTI_EXT_CFG_BOOL("zifencei", ext_zifencei, true),
> MULTI_EXT_CFG_BOOL("zicsr", ext_zicsr, true),
> MULTI_EXT_CFG_BOOL("zihintntl", ext_zihintntl, true),
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index 888084d8e5..15b953f268 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -2291,6 +2291,11 @@ static bool xiselect_cd_range(target_ulong isel)
> return (ISELECT_CD_FIRST <= isel && isel <= ISELECT_CD_LAST);
> }
>
> +static bool xiselect_ctr_range(target_ulong isel)
> +{
> + return (CTR_ENTRIES_FIRST <= isel && isel <= CTR_ENTRIES_LAST);
> +}
> +
> static int rmw_iprio(target_ulong xlen,
> target_ulong iselect, uint8_t *iprio,
> target_ulong *val, target_ulong new_val,
> @@ -2336,6 +2341,118 @@ static int rmw_iprio(target_ulong xlen,
> return 0;
> }
>
> +static int rmw_xctrsource(CPURISCVState *env, int isel, target_ulong *val,
> + target_ulong new_val, target_ulong wr_mask)
I prefer naming the function as rmw_ctrsource(), since this register
name does not have a mode prefix.
> +{
> + /*
> + * CTR arrays are treated as circular buffers and TOS always points to next
> + * empty slot, keeping TOS - 1 always pointing to latest entry. Given entry
> + * 0 is always the latest one, traversal is a bit different here. See the
> + * below example.
> + *
> + * Depth = 16.
> + *
> + * idx [0] [1] [2] [3] [4] [5] [6] [7] [8] [9] [A] [B] [C] [D] [E] [F]
> + * TOS H
> + * entry 6 5 4 3 2 1 0 F E D C B A 9 8 7
> + */
> + const uint64_t entry = isel - CTR_ENTRIES_FIRST;
> + const uint64_t depth = 16 << get_field(env->sctrdepth, SCTRDEPTH_MASK);
> + uint64_t idx;
> +
> + /* Entry greater than depth-1 is read-only zero */
> + if (entry >= depth) {
> + *val = 0;
val may be NULL.
> + return 0;
> + }
> +
> + idx = get_field(env->sctrstatus, SCTRSTATUS_WRPTR_MASK);
> + idx = (idx - entry - 1) & (depth - 1);
> +
> + if (val) {
> + *val = env->ctr_src[idx];
> + }
> +
> + env->ctr_src[idx] = (env->ctr_src[idx] & ~wr_mask) | (new_val & wr_mask);
> +
> + return 0;
> +}
> +
> +static int rmw_xctrtarget(CPURISCVState *env, int isel, target_ulong *val,
> + target_ulong new_val, target_ulong wr_mask)
I prefer naming the function as rmw_ctrtarget(), since this register
name does not have a mode prefix.
> +{
> + /*
> + * CTR arrays are treated as circular buffers and TOS always points to next
> + * empty slot, keeping TOS - 1 always pointing to latest entry. Given entry
> + * 0 is always the latest one, traversal is a bit different here. See the
> + * below example.
> + *
> + * Depth = 16.
> + *
> + * idx [0] [1] [2] [3] [4] [5] [6] [7] [8] [9] [A] [B] [C] [D] [E] [F]
> + * head H
> + * entry 6 5 4 3 2 1 0 F E D C B A 9 8 7
> + */
> + const uint64_t entry = isel - CTR_ENTRIES_FIRST;
> + const uint64_t depth = 16 << get_field(env->sctrdepth, SCTRDEPTH_MASK);
> + uint64_t idx;
> +
> + /* Entry greater than depth-1 is read-only zero */
> + if (entry >= depth) {
> + *val = 0;
val may be NULL.
> + return 0;
> + }
> +
> + idx = get_field(env->sctrstatus, SCTRSTATUS_WRPTR_MASK);
> + idx = (idx - entry - 1) & (depth - 1);
> +
> + if (val) {
> + *val = env->ctr_dst[idx];
> + }
> +
> + env->ctr_dst[idx] = (env->ctr_dst[idx] & ~wr_mask) | (new_val & wr_mask);
> +
> + return 0;
> +}
> +
> +static int rmw_xctrdata(CPURISCVState *env, int isel, target_ulong *val,
> + target_ulong new_val, target_ulong wr_mask)
I prefer naming the function as rmw_ctrdata(), since this register name
does not have a mode prefix.
> +{
> + /*
> + * CTR arrays are treated as circular buffers and TOS always points to next
> + * empty slot, keeping TOS - 1 always pointing to latest entry. Given entry
> + * 0 is always the latest one, traversal is a bit different here. See the
> + * below example.
> + *
> + * Depth = 16.
> + *
> + * idx [0] [1] [2] [3] [4] [5] [6] [7] [8] [9] [A] [B] [C] [D] [E] [F]
> + * head H
> + * entry 6 5 4 3 2 1 0 F E D C B A 9 8 7
> + */
> + const uint64_t entry = isel - CTR_ENTRIES_FIRST;
> + const uint64_t mask = wr_mask & CTRDATA_MASK;
> + const uint64_t depth = 16 << get_field(env->sctrdepth, SCTRDEPTH_MASK);
> + uint64_t idx;
> +
> + /* Entry greater than depth-1 is read-only zero */
> + if (entry >= depth) {
> + *val = 0;
val may be NULL.
> + return 0;
> + }
> +
> + idx = get_field(env->sctrstatus, SCTRSTATUS_WRPTR_MASK);
> + idx = (idx - entry - 1) & (depth - 1);
> +
> + if (val) {
> + *val = env->ctr_data[idx];
> + }
> +
> + env->ctr_data[idx] = (env->ctr_data[idx] & ~mask) | (new_val & mask);
> +
> + return 0;
> +}
> +
> static RISCVException rmw_xireg_aia(CPURISCVState *env, int csrno,
> target_ulong isel, target_ulong *val,
> target_ulong new_val, target_ulong wr_mask)
> @@ -2486,6 +2603,38 @@ done:
> return ret;
> }
>
> +static int rmw_xireg_ctr(CPURISCVState *env, int csrno,
> + target_ulong isel, target_ulong *val,
> + target_ulong new_val, target_ulong wr_mask)
> +{
> + bool ext_sxctr = false;
> + int ret = -EINVAL;
> +
> + if (CSR_MIREG <= csrno && csrno <= CSR_MIREG3) {
> + ext_sxctr = riscv_cpu_cfg(env)->ext_smctr;
> + } else if (CSR_SIREG <= csrno && csrno <= CSR_SIREG3) {
> + ext_sxctr = riscv_cpu_cfg(env)->ext_ssctr;
> + } else if (CSR_VSIREG <= csrno && csrno <= CSR_VSIREG3) {
> + ext_sxctr = riscv_cpu_cfg(env)->ext_ssctr;
> + }
> +
> + if (!ext_sxctr) {
I think [s|vs]ireg4/5/6 are read-only 0 and accesses on them should not
trigger exceptions.
Please refer to chapter 3.
Exceptions should be triggered when both smctr and ssctr are disabled.
> + return -EINVAL;
> + }
> +
> + if (csrno == CSR_MIREG || csrno == CSR_SIREG || csrno == CSR_VSIREG) {
MIREG* are not used by CTR.
> + ret = rmw_xctrsource(env, isel, val, new_val, wr_mask);
> + } else if (csrno == CSR_MIREG2 || csrno == CSR_SIREG2 ||
> + csrno == CSR_VSIREG2) {
MIREG* are not used by CTR.
> + ret = rmw_xctrtarget(env, isel, val, new_val, wr_mask);
> + } else if (csrno == CSR_MIREG3 || csrno == CSR_SIREG3 ||
> + csrno == CSR_VSIREG3) {
MIREG* are not used by CTR.
> + ret = rmw_xctrdata(env, isel, val, new_val, wr_mask);
> + }
> +
> + return ret;
> +}
> +
> /*
> * rmw_xireg_sxcsrind: Perform indirect access to xireg and xireg2-xireg6
> *
> @@ -2497,11 +2646,13 @@ static int rmw_xireg_sxcsrind(CPURISCVState *env, int csrno,
> target_ulong isel, target_ulong *val,
> target_ulong new_val, target_ulong wr_mask)
> {
> - int ret = -EINVAL;
> bool virt = csrno == CSR_VSIREG ? true : false;
> + int ret = -EINVAL;
>
> if (xiselect_cd_range(isel)) {
> ret = rmw_xireg_cd(env, csrno, isel, val, new_val, wr_mask);
> + } else if (xiselect_ctr_range(isel)) {
MIREG* are not used by CTR. We can check:
else if (csrno < CSR_MIREG && xiselect_ctr_range(isel))
> + ret = rmw_xireg_ctr(env, csrno, isel, val, new_val, wr_mask);
> } else {
> /*
> * As per the specification, access to unimplented region is undefined
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 5/6] target/riscv: Add CTR sctrclr instruction.
2024-05-29 16:09 ` [PATCH 5/6] target/riscv: Add CTR sctrclr instruction Rajnesh Kanwal
@ 2024-06-04 17:19 ` Jason Chien
2024-06-04 18:46 ` Beeman Strong
0 siblings, 1 reply; 22+ messages in thread
From: Jason Chien @ 2024-06-04 17:19 UTC (permalink / raw)
To: Rajnesh Kanwal, qemu-riscv, qemu-devel
Cc: alistair.francis, bin.meng, liweiwei, dbarboza, zhiwei_liu,
atishp, apatel, beeman, tech-control-transfer-records
Rajnesh Kanwal 於 2024/5/30 上午 12:09 寫道:
> CTR extension adds a new instruction sctrclr to quickly
> clear the recorded entries buffer.
>
> Signed-off-by: Rajnesh Kanwal <rkanwal@rivosinc.com>
> ---
> target/riscv/cpu.h | 1 +
> target/riscv/cpu_helper.c | 7 +++++++
> target/riscv/insn32.decode | 1 +
> target/riscv/insn_trans/trans_privileged.c.inc | 10 ++++++++++
> target/riscv/op_helper.c | 5 +++++
> 5 files changed, 24 insertions(+)
>
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index a294a5372a..fade71aa09 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -572,6 +572,7 @@ void riscv_cpu_set_mode(CPURISCVState *env, target_ulong newpriv, bool virt_en);
> void riscv_ctr_freeze(CPURISCVState *env, uint64_t freeze_mask);
> void riscv_ctr_add_entry(CPURISCVState *env, target_long src, target_long dst,
> uint64_t type, target_ulong prev_priv, bool prev_virt);
> +void riscv_ctr_clear(CPURISCVState *env);
>
> void riscv_translate_init(void);
> G_NORETURN void riscv_raise_exception(CPURISCVState *env,
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index e064a7306e..45502f50a7 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -704,6 +704,13 @@ void riscv_ctr_freeze(CPURISCVState *env, uint64_t freeze_mask)
> }
> }
>
> +void riscv_ctr_clear(CPURISCVState *env)
> +{
> + memset(env->ctr_src, 0x0, sizeof(env->ctr_src));
> + memset(env->ctr_dst, 0x0, sizeof(env->ctr_dst));
> + memset(env->ctr_data, 0x0, sizeof(env->ctr_data));
> +}
> +
> static uint64_t riscv_ctr_priv_to_mask(target_ulong priv, bool virt)
> {
> switch (priv) {
> diff --git a/target/riscv/insn32.decode b/target/riscv/insn32.decode
> index 9cb1a1b4ec..d3d38c7c68 100644
> --- a/target/riscv/insn32.decode
> +++ b/target/riscv/insn32.decode
> @@ -107,6 +107,7 @@
> # *** Privileged Instructions ***
> ecall 000000000000 00000 000 00000 1110011
> ebreak 000000000001 00000 000 00000 1110011
> +sctrclr 000100000100 00000 000 00000 1110011
> uret 0000000 00010 00000 000 00000 1110011
> sret 0001000 00010 00000 000 00000 1110011
> mret 0011000 00010 00000 000 00000 1110011
> diff --git a/target/riscv/insn_trans/trans_privileged.c.inc b/target/riscv/insn_trans/trans_privileged.c.inc
> index 339d659151..dd9da8651f 100644
> --- a/target/riscv/insn_trans/trans_privileged.c.inc
> +++ b/target/riscv/insn_trans/trans_privileged.c.inc
> @@ -69,6 +69,16 @@ static bool trans_ebreak(DisasContext *ctx, arg_ebreak *a)
> return true;
> }
>
> +static bool trans_sctrclr(DisasContext *ctx, arg_sctrclr *a)
> +{
> +#ifndef CONFIG_USER_ONLY
If both of smctr and ssctr are not enabled, it is an illegal instruction.
> + gen_helper_ctr_clear(tcg_env);
> + return true;
> +#else
> + return false;
> +#endif
> +}
> +
> static bool trans_uret(DisasContext *ctx, arg_uret *a)
> {
> return false;
> diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
> index c8053d9c2f..89423c04b3 100644
> --- a/target/riscv/op_helper.c
> +++ b/target/riscv/op_helper.c
> @@ -461,6 +461,11 @@ void helper_ctr_branch(CPURISCVState *env, target_ulong src, target_ulong dest,
> }
> }
>
> +void helper_ctr_clear(CPURISCVState *env)
> +{
There should be some checks here.
The spec states:
SCTRCLR raises an illegal-instruction exception in U-mode, and a
virtual-instruction exception in VU-mode, unless CTR state enable access
restrictions apply.
I don't quite understand "unless CTR state enable access restrictions
apply" though.
> + riscv_ctr_clear(env);
> +}
> +
> void helper_wfi(CPURISCVState *env)
> {
> CPUState *cs = env_cpu(env);
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 5/6] target/riscv: Add CTR sctrclr instruction.
2024-06-04 17:19 ` Jason Chien
@ 2024-06-04 18:46 ` Beeman Strong
2024-06-05 6:28 ` Jason Chien
0 siblings, 1 reply; 22+ messages in thread
From: Beeman Strong @ 2024-06-04 18:46 UTC (permalink / raw)
To: Jason Chien
Cc: Rajnesh Kanwal, qemu-riscv, qemu-devel, alistair.francis,
bin.meng, liweiwei, dbarboza, zhiwei_liu, atishp, apatel,
tech-control-transfer-records
[-- Attachment #1: Type: text/plain, Size: 4586 bytes --]
On Tue, Jun 4, 2024 at 10:19 AM Jason Chien <jason.chien@sifive.com> wrote:
>
> Rajnesh Kanwal 於 2024/5/30 上午 12:09 寫道:
> > CTR extension adds a new instruction sctrclr to quickly
> > clear the recorded entries buffer.
> >
> > Signed-off-by: Rajnesh Kanwal <rkanwal@rivosinc.com>
> > ---
> > target/riscv/cpu.h | 1 +
> > target/riscv/cpu_helper.c | 7 +++++++
> > target/riscv/insn32.decode | 1 +
> > target/riscv/insn_trans/trans_privileged.c.inc | 10 ++++++++++
> > target/riscv/op_helper.c | 5 +++++
> > 5 files changed, 24 insertions(+)
> >
> > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> > index a294a5372a..fade71aa09 100644
> > --- a/target/riscv/cpu.h
> > +++ b/target/riscv/cpu.h
> > @@ -572,6 +572,7 @@ void riscv_cpu_set_mode(CPURISCVState *env,
> target_ulong newpriv, bool virt_en);
> > void riscv_ctr_freeze(CPURISCVState *env, uint64_t freeze_mask);
> > void riscv_ctr_add_entry(CPURISCVState *env, target_long src,
> target_long dst,
> > uint64_t type, target_ulong prev_priv, bool
> prev_virt);
> > +void riscv_ctr_clear(CPURISCVState *env);
> >
> > void riscv_translate_init(void);
> > G_NORETURN void riscv_raise_exception(CPURISCVState *env,
> > diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> > index e064a7306e..45502f50a7 100644
> > --- a/target/riscv/cpu_helper.c
> > +++ b/target/riscv/cpu_helper.c
> > @@ -704,6 +704,13 @@ void riscv_ctr_freeze(CPURISCVState *env, uint64_t
> freeze_mask)
> > }
> > }
> >
> > +void riscv_ctr_clear(CPURISCVState *env)
> > +{
> > + memset(env->ctr_src, 0x0, sizeof(env->ctr_src));
> > + memset(env->ctr_dst, 0x0, sizeof(env->ctr_dst));
> > + memset(env->ctr_data, 0x0, sizeof(env->ctr_data));
> > +}
> > +
> > static uint64_t riscv_ctr_priv_to_mask(target_ulong priv, bool virt)
> > {
> > switch (priv) {
> > diff --git a/target/riscv/insn32.decode b/target/riscv/insn32.decode
> > index 9cb1a1b4ec..d3d38c7c68 100644
> > --- a/target/riscv/insn32.decode
> > +++ b/target/riscv/insn32.decode
> > @@ -107,6 +107,7 @@
> > # *** Privileged Instructions ***
> > ecall 000000000000 00000 000 00000 1110011
> > ebreak 000000000001 00000 000 00000 1110011
> > +sctrclr 000100000100 00000 000 00000 1110011
> > uret 0000000 00010 00000 000 00000 1110011
> > sret 0001000 00010 00000 000 00000 1110011
> > mret 0011000 00010 00000 000 00000 1110011
> > diff --git a/target/riscv/insn_trans/trans_privileged.c.inc
> b/target/riscv/insn_trans/trans_privileged.c.inc
> > index 339d659151..dd9da8651f 100644
> > --- a/target/riscv/insn_trans/trans_privileged.c.inc
> > +++ b/target/riscv/insn_trans/trans_privileged.c.inc
> > @@ -69,6 +69,16 @@ static bool trans_ebreak(DisasContext *ctx,
> arg_ebreak *a)
> > return true;
> > }
> >
> > +static bool trans_sctrclr(DisasContext *ctx, arg_sctrclr *a)
> > +{
> > +#ifndef CONFIG_USER_ONLY
> If both of smctr and ssctr are not enabled, it is an illegal instruction.
> > + gen_helper_ctr_clear(tcg_env);
> > + return true;
> > +#else
> > + return false;
> > +#endif
> > +}
> > +
> > static bool trans_uret(DisasContext *ctx, arg_uret *a)
> > {
> > return false;
> > diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
> > index c8053d9c2f..89423c04b3 100644
> > --- a/target/riscv/op_helper.c
> > +++ b/target/riscv/op_helper.c
> > @@ -461,6 +461,11 @@ void helper_ctr_branch(CPURISCVState *env,
> target_ulong src, target_ulong dest,
> > }
> > }
> >
> > +void helper_ctr_clear(CPURISCVState *env)
> > +{
>
> There should be some checks here.
> The spec states:
> SCTRCLR raises an illegal-instruction exception in U-mode, and a
> virtual-instruction exception in VU-mode, unless CTR state enable access
> restrictions apply.
>
> I don't quite understand "unless CTR state enable access restrictions
> apply" though.
>
The next sentence says "See Chapter 5", which states that execution of
SCTRCLR raises an illegal instruction exception if mstateen0.CTR=0 and the
priv mode is not M-mode, and it raises a virtual instruction exception if
mstateen0.CTR=0 && hstateen0.CTR=1 and the priv mode is VS-mode.
>
> > + riscv_ctr_clear(env);
> > +}
> > +
> > void helper_wfi(CPURISCVState *env)
> > {
> > CPUState *cs = env_cpu(env);
>
[-- Attachment #2: Type: text/html, Size: 5842 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 0/6] target/riscv: Add support for Control Transfer Records Ext.
2024-06-04 7:29 ` [PATCH 0/6] target/riscv: Add support for Control Transfer Records Ext Jason Chien
@ 2024-06-04 22:32 ` Beeman Strong
2024-06-05 3:34 ` Jason Chien
0 siblings, 1 reply; 22+ messages in thread
From: Beeman Strong @ 2024-06-04 22:32 UTC (permalink / raw)
To: Jason Chien
Cc: Rajnesh Kanwal, alistair.francis, apatel, atishp, bin.meng,
dbarboza, liweiwei, qemu-devel, qemu-riscv,
tech-control-transfer-records, zhiwei_liu
[-- Attachment #1: Type: text/plain, Size: 3845 bytes --]
There is no dependency on Smcsrind, only Sscsrind.
On Tue, Jun 4, 2024 at 12:29 AM Jason Chien <jason.chien@sifive.com> wrote:
> Smctr depends on the Smcsrind extension, Ssctr depends on the Sscsrind
> extension, and both Smctr and Ssctr depend upon implementation of S-mode.
> There should be a dependency check in riscv_cpu_validate_set_extensions().
>
> Rajnesh Kanwal 於 2024/5/30 上午 12:09 寫道:
> > This series enables Control Transfer Records extension support on riscv
> > platform. This extension is similar to Arch LBR in x86 and BRBE in ARM.
> > The Extension has been stable and the latest release can be found here
> [0]
> >
> > CTR extension depends on couple of other extensions:
> >
> > 1. S[m|s]csrind : The indirect CSR extension [1] which defines additional
> > ([M|S|VS]IREG2-[M|S|VS]IREG6) register to address size limitation of
> > RISC-V CSR address space. CTR access ctrsource, ctrtartget and
> ctrdata
> > CSRs using sscsrind extension.
> >
> > 2. Smstateen: The mstateen bit[54] controls the access to the CTR ext to
> > S-mode.
> >
> > 3. Sscofpmf: Counter overflow and privilege mode filtering. [2]
> >
> > The series is based on Smcdeleg/Ssccfg counter delegation extension [3]
> > patches. CTR itself doesn't depend on counter delegation support. This
> > rebase is basically to include the Smcsrind patches.
> >
> > Due to the dependency of these extensions, the following extensions must
> be
> > enabled to use the control transfer records feature.
> >
> >
> "smstateen=true,sscofpmf=true,smcsrind=true,sscsrind=true,smctr=true,ssctr=true"
> >
> > Here is the link to a quick guide [5] to setup and run a basic perf demo
> on
> > Linux to use CTR Ext.
> >
> > The Qemu patches can be found here:
> > https://github.com/rajnesh-kanwal/qemu/tree/ctr_upstream
> >
> > The opensbi patch can be found here:
> > https://github.com/rajnesh-kanwal/opensbi/tree/ctr_upstream
> >
> > The Linux kernel patches can be found here:
> > https://github.com/rajnesh-kanwal/linux/tree/ctr_upstream
> >
> > [0]:https://github.com/riscv/riscv-control-transfer-records/release
> > [1]:https://github.com/riscv/riscv-indirect-csr-access
> > [2]:https://github.com/riscvarchive/riscv-count-overflow/tree/main
> > [3]:https://github.com/riscv/riscv-smcdeleg-ssccfg
> > [4]:
> https://lore.kernel.org/all/20240217000134.3634191-1-atishp@rivosinc.com/
> > [5]:
> https://github.com/rajnesh-kanwal/linux/wiki/Running-CTR-basic-demo-on-QEMU-RISC%E2%80%90V-Virt-machine
> >
> > Rajnesh Kanwal (6):
> > target/riscv: Remove obsolete sfence.vm instruction
> > target/riscv: Add Control Transfer Records CSR definitions.
> > target/riscv: Add support for Control Transfer Records extension CSRs.
> > target/riscv: Add support to record CTR entries.
> > target/riscv: Add CTR sctrclr instruction.
> > target/riscv: Add support to access ctrsource, ctrtarget, ctrdata
> > regs.
> >
> > target/riscv/cpu.c | 4 +
> > target/riscv/cpu.h | 14 +
> > target/riscv/cpu_bits.h | 154 +++++++++
> > target/riscv/cpu_cfg.h | 2 +
> > target/riscv/cpu_helper.c | 213 ++++++++++++
> > target/riscv/csr.c | 312 +++++++++++++++++-
> > target/riscv/helper.h | 8 +-
> > target/riscv/insn32.decode | 2 +-
> > .../riscv/insn_trans/trans_privileged.c.inc | 21 +-
> > target/riscv/insn_trans/trans_rvi.c.inc | 27 ++
> > target/riscv/op_helper.c | 117 ++++++-
> > target/riscv/translate.c | 9 +
> > 12 files changed, 870 insertions(+), 13 deletions(-)
> >
>
[-- Attachment #2: Type: text/html, Size: 5704 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/6] target/riscv: Remove obsolete sfence.vm instruction
2024-05-29 16:09 ` [PATCH 1/6] target/riscv: Remove obsolete sfence.vm instruction Rajnesh Kanwal
@ 2024-06-05 0:46 ` Alistair Francis
0 siblings, 0 replies; 22+ messages in thread
From: Alistair Francis @ 2024-06-05 0:46 UTC (permalink / raw)
To: Rajnesh Kanwal
Cc: qemu-riscv, qemu-devel, alistair.francis, bin.meng, liweiwei,
dbarboza, zhiwei_liu, atishp, apatel, beeman,
tech-control-transfer-records
On Thu, May 30, 2024 at 2:12 AM Rajnesh Kanwal <rkanwal@rivosinc.com> wrote:
>
> Signed-off-by: Rajnesh Kanwal <rkanwal@rivosinc.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> target/riscv/insn32.decode | 1 -
> target/riscv/insn_trans/trans_privileged.c.inc | 5 -----
> 2 files changed, 6 deletions(-)
>
> diff --git a/target/riscv/insn32.decode b/target/riscv/insn32.decode
> index f22df04cfd..9cb1a1b4ec 100644
> --- a/target/riscv/insn32.decode
> +++ b/target/riscv/insn32.decode
> @@ -112,7 +112,6 @@ sret 0001000 00010 00000 000 00000 1110011
> mret 0011000 00010 00000 000 00000 1110011
> wfi 0001000 00101 00000 000 00000 1110011
> sfence_vma 0001001 ..... ..... 000 00000 1110011 @sfence_vma
> -sfence_vm 0001000 00100 ..... 000 00000 1110011 @sfence_vm
>
> # *** RV32I Base Instruction Set ***
> lui .................... ..... 0110111 @u
> diff --git a/target/riscv/insn_trans/trans_privileged.c.inc b/target/riscv/insn_trans/trans_privileged.c.inc
> index bc5263a4e0..4eccdddeaa 100644
> --- a/target/riscv/insn_trans/trans_privileged.c.inc
> +++ b/target/riscv/insn_trans/trans_privileged.c.inc
> @@ -127,8 +127,3 @@ static bool trans_sfence_vma(DisasContext *ctx, arg_sfence_vma *a)
> #endif
> return false;
> }
> -
> -static bool trans_sfence_vm(DisasContext *ctx, arg_sfence_vm *a)
> -{
> - return false;
> -}
> --
> 2.34.1
>
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 0/6] target/riscv: Add support for Control Transfer Records Ext.
2024-06-04 22:32 ` Beeman Strong
@ 2024-06-05 3:34 ` Jason Chien
2024-06-05 3:41 ` Beeman Strong
0 siblings, 1 reply; 22+ messages in thread
From: Jason Chien @ 2024-06-05 3:34 UTC (permalink / raw)
To: Beeman Strong
Cc: Rajnesh Kanwal, alistair.francis, apatel, atishp, bin.meng,
dbarboza, liweiwei, qemu-devel, qemu-riscv,
tech-control-transfer-records, zhiwei_liu
[-- Attachment #1: Type: text/plain, Size: 4727 bytes --]
Thank you for pointing that out. CTR does not use miselect and mireg*.
There is no dependency on Smcsrind. I believe the spec needs to be
corrected.
Chapter 1 states that:
Smctr depends on the Smcsrind extension, while Ssctr depends on the
Sscsrind extension. Further, both Smctr and Ssctr depend upon
implementation of S-mode.
Beeman Strong 於 2024/6/5 上午 06:32 寫道:
> There is no dependency on Smcsrind, only Sscsrind.
>
> On Tue, Jun 4, 2024 at 12:29 AM Jason Chien <jason.chien@sifive.com>
> wrote:
>
> Smctr depends on the Smcsrind extension, Ssctr depends on the
> Sscsrind
> extension, and both Smctr and Ssctr depend upon implementation of
> S-mode.
> There should be a dependency check in
> riscv_cpu_validate_set_extensions().
>
> Rajnesh Kanwal 於 2024/5/30 上午 12:09 寫道:
> > This series enables Control Transfer Records extension support
> on riscv
> > platform. This extension is similar to Arch LBR in x86 and BRBE
> in ARM.
> > The Extension has been stable and the latest release can be
> found here [0]
> >
> > CTR extension depends on couple of other extensions:
> >
> > 1. S[m|s]csrind : The indirect CSR extension [1] which defines
> additional
> > ([M|S|VS]IREG2-[M|S|VS]IREG6) register to address size
> limitation of
> > RISC-V CSR address space. CTR access ctrsource, ctrtartget
> and ctrdata
> > CSRs using sscsrind extension.
> >
> > 2. Smstateen: The mstateen bit[54] controls the access to the
> CTR ext to
> > S-mode.
> >
> > 3. Sscofpmf: Counter overflow and privilege mode filtering. [2]
> >
> > The series is based on Smcdeleg/Ssccfg counter delegation
> extension [3]
> > patches. CTR itself doesn't depend on counter delegation
> support. This
> > rebase is basically to include the Smcsrind patches.
> >
> > Due to the dependency of these extensions, the following
> extensions must be
> > enabled to use the control transfer records feature.
> >
> >
> "smstateen=true,sscofpmf=true,smcsrind=true,sscsrind=true,smctr=true,ssctr=true"
> >
> > Here is the link to a quick guide [5] to setup and run a basic
> perf demo on
> > Linux to use CTR Ext.
> >
> > The Qemu patches can be found here:
> > https://github.com/rajnesh-kanwal/qemu/tree/ctr_upstream
> >
> > The opensbi patch can be found here:
> > https://github.com/rajnesh-kanwal/opensbi/tree/ctr_upstream
> >
> > The Linux kernel patches can be found here:
> > https://github.com/rajnesh-kanwal/linux/tree/ctr_upstream
> >
> > [0]:https://github.com/riscv/riscv-control-transfer-records/release
> > [1]:https://github.com/riscv/riscv-indirect-csr-access
> > [2]:https://github.com/riscvarchive/riscv-count-overflow/tree/main
> > [3]:https://github.com/riscv/riscv-smcdeleg-ssccfg
> >
> [4]:https://lore.kernel.org/all/20240217000134.3634191-1-atishp@rivosinc.com/
> >
> [5]:https://github.com/rajnesh-kanwal/linux/wiki/Running-CTR-basic-demo-on-QEMU-RISC%E2%80%90V-Virt-machine
> >
> > Rajnesh Kanwal (6):
> > target/riscv: Remove obsolete sfence.vm instruction
> > target/riscv: Add Control Transfer Records CSR definitions.
> > target/riscv: Add support for Control Transfer Records
> extension CSRs.
> > target/riscv: Add support to record CTR entries.
> > target/riscv: Add CTR sctrclr instruction.
> > target/riscv: Add support to access ctrsource, ctrtarget, ctrdata
> > regs.
> >
> > target/riscv/cpu.c | 4 +
> > target/riscv/cpu.h | 14 +
> > target/riscv/cpu_bits.h | 154 +++++++++
> > target/riscv/cpu_cfg.h | 2 +
> > target/riscv/cpu_helper.c | 213 ++++++++++++
> > target/riscv/csr.c | 312
> +++++++++++++++++-
> > target/riscv/helper.h | 8 +-
> > target/riscv/insn32.decode | 2 +-
> > .../riscv/insn_trans/trans_privileged.c.inc | 21 +-
> > target/riscv/insn_trans/trans_rvi.c.inc | 27 ++
> > target/riscv/op_helper.c | 117 ++++++-
> > target/riscv/translate.c | 9 +
> > 12 files changed, 870 insertions(+), 13 deletions(-)
> >
>
[-- Attachment #2: Type: text/html, Size: 8600 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 0/6] target/riscv: Add support for Control Transfer Records Ext.
2024-06-05 3:34 ` Jason Chien
@ 2024-06-05 3:41 ` Beeman Strong
2024-06-06 18:36 ` Beeman Strong
0 siblings, 1 reply; 22+ messages in thread
From: Beeman Strong @ 2024-06-05 3:41 UTC (permalink / raw)
To: Jason Chien
Cc: Rajnesh Kanwal, alistair.francis, apatel, atishp, bin.meng,
dbarboza, liweiwei, qemu-devel, qemu-riscv,
tech-control-transfer-records, zhiwei_liu
[-- Attachment #1: Type: text/plain, Size: 4498 bytes --]
Ah, good catch. We removed that dependency late. I'll fix it.
On Tue, Jun 4, 2024 at 8:34 PM Jason Chien <jason.chien@sifive.com> wrote:
> Thank you for pointing that out. CTR does not use miselect and mireg*.
> There is no dependency on Smcsrind. I believe the spec needs to be
> corrected.
>
> Chapter 1 states that:
> Smctr depends on the Smcsrind extension, while Ssctr depends on the
> Sscsrind extension. Further, both Smctr and Ssctr depend upon
> implementation of S-mode.
> Beeman Strong 於 2024/6/5 上午 06:32 寫道:
>
> There is no dependency on Smcsrind, only Sscsrind.
>
> On Tue, Jun 4, 2024 at 12:29 AM Jason Chien <jason.chien@sifive.com>
> wrote:
>
>> Smctr depends on the Smcsrind extension, Ssctr depends on the Sscsrind
>> extension, and both Smctr and Ssctr depend upon implementation of S-mode.
>> There should be a dependency check in riscv_cpu_validate_set_extensions().
>>
>> Rajnesh Kanwal 於 2024/5/30 上午 12:09 寫道:
>> > This series enables Control Transfer Records extension support on riscv
>> > platform. This extension is similar to Arch LBR in x86 and BRBE in ARM.
>> > The Extension has been stable and the latest release can be found here
>> [0]
>> >
>> > CTR extension depends on couple of other extensions:
>> >
>> > 1. S[m|s]csrind : The indirect CSR extension [1] which defines
>> additional
>> > ([M|S|VS]IREG2-[M|S|VS]IREG6) register to address size limitation of
>> > RISC-V CSR address space. CTR access ctrsource, ctrtartget and
>> ctrdata
>> > CSRs using sscsrind extension.
>> >
>> > 2. Smstateen: The mstateen bit[54] controls the access to the CTR ext to
>> > S-mode.
>> >
>> > 3. Sscofpmf: Counter overflow and privilege mode filtering. [2]
>> >
>> > The series is based on Smcdeleg/Ssccfg counter delegation extension [3]
>> > patches. CTR itself doesn't depend on counter delegation support. This
>> > rebase is basically to include the Smcsrind patches.
>> >
>> > Due to the dependency of these extensions, the following extensions
>> must be
>> > enabled to use the control transfer records feature.
>> >
>> >
>> "smstateen=true,sscofpmf=true,smcsrind=true,sscsrind=true,smctr=true,ssctr=true"
>> >
>> > Here is the link to a quick guide [5] to setup and run a basic perf
>> demo on
>> > Linux to use CTR Ext.
>> >
>> > The Qemu patches can be found here:
>> > https://github.com/rajnesh-kanwal/qemu/tree/ctr_upstream
>> >
>> > The opensbi patch can be found here:
>> > https://github.com/rajnesh-kanwal/opensbi/tree/ctr_upstream
>> >
>> > The Linux kernel patches can be found here:
>> > https://github.com/rajnesh-kanwal/linux/tree/ctr_upstream
>> >
>> > [0]:https://github.com/riscv/riscv-control-transfer-records/release
>> > [1]:https://github.com/riscv/riscv-indirect-csr-access
>> > [2]:https://github.com/riscvarchive/riscv-count-overflow/tree/main
>> > [3]:https://github.com/riscv/riscv-smcdeleg-ssccfg
>> > [4]:
>> https://lore.kernel.org/all/20240217000134.3634191-1-atishp@rivosinc.com/
>> > [5]:
>> https://github.com/rajnesh-kanwal/linux/wiki/Running-CTR-basic-demo-on-QEMU-RISC%E2%80%90V-Virt-machine
>> >
>> > Rajnesh Kanwal (6):
>> > target/riscv: Remove obsolete sfence.vm instruction
>> > target/riscv: Add Control Transfer Records CSR definitions.
>> > target/riscv: Add support for Control Transfer Records extension
>> CSRs.
>> > target/riscv: Add support to record CTR entries.
>> > target/riscv: Add CTR sctrclr instruction.
>> > target/riscv: Add support to access ctrsource, ctrtarget, ctrdata
>> > regs.
>> >
>> > target/riscv/cpu.c | 4 +
>> > target/riscv/cpu.h | 14 +
>> > target/riscv/cpu_bits.h | 154 +++++++++
>> > target/riscv/cpu_cfg.h | 2 +
>> > target/riscv/cpu_helper.c | 213 ++++++++++++
>> > target/riscv/csr.c | 312 +++++++++++++++++-
>> > target/riscv/helper.h | 8 +-
>> > target/riscv/insn32.decode | 2 +-
>> > .../riscv/insn_trans/trans_privileged.c.inc | 21 +-
>> > target/riscv/insn_trans/trans_rvi.c.inc | 27 ++
>> > target/riscv/op_helper.c | 117 ++++++-
>> > target/riscv/translate.c | 9 +
>> > 12 files changed, 870 insertions(+), 13 deletions(-)
>> >
>>
>
[-- Attachment #2: Type: text/html, Size: 8070 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 5/6] target/riscv: Add CTR sctrclr instruction.
2024-06-04 18:46 ` Beeman Strong
@ 2024-06-05 6:28 ` Jason Chien
2024-06-05 15:16 ` Beeman Strong
0 siblings, 1 reply; 22+ messages in thread
From: Jason Chien @ 2024-06-05 6:28 UTC (permalink / raw)
To: Beeman Strong
Cc: Rajnesh Kanwal, qemu-riscv, qemu-devel, alistair.francis,
bin.meng, liweiwei, dbarboza, zhiwei_liu, atishp, apatel,
tech-control-transfer-records
[-- Attachment #1: Type: text/plain, Size: 9563 bytes --]
Chapter 5 describes the CTR behavior when Smstaten is enabled, but it
does not talk about the CTR behavior when Smstateen is disabled, that
is, there is no mstateen0 and hstateen0 CSR.
The spec states:
* Attempts to access sctrdepth from VS-mode or VU-mode raise a
virtual-instruction exception, unless CTR state enable access
restrictions apply.
* sctrdepth is not included in the above list of supervisor CTR state
controlled by hstateen0.CTR since accesses to sctrdepth from VS-mode
raise a virtual-instruction exception regardless of the value of
hstateen0.CTR.
Below is my understanding:
* If Smstateen is enabled:
o If mstateen0.CTR=0:
+ Attempts to access sctrctl, vsctrctl, sctrdepth, or
sctrstatus raise an illegal-instruction exception for
privilege modes less privileged than M-mode.
+ Attempts to access sireg* when siselect is in 0x200..0x2FF,
or vsireg* when vsiselect is in 0x200..0x2FF raise an
illegal-instruction exception for privilege modes less
privileged than M-mode.
+ Execution of the SCTRCLR instruction raises an
illegal-instruction exception for privilege modes less
privileged than M-mode.
o If mstateen.CTR=1:
+ Attempts to access sctrctl, vsctrctl, sctrdepth, or
sctrstatus raise an illegal-instruction exception for U-mode.
+ Attempts to access sireg* when siselect is in 0x200..0x2FF,
or vsireg* when vsiselect is in 0x200..0x2FF raise an
illegal-instruction exception for U-mode.
+ Execution of the SCTRCLR instruction raises an
illegal-instruction exception for U-mode.
+ If H extension is enabled:
# If hstateen0.CTR = 0:
* Attempts to access sctrctl, vsctrctl, sctrdepth, or
sctrstatus raise an virtual-instruction exception
for VU-mode and VS-mode.
* Attempts to access sireg* when siselect is in
0x200..0x2FF, or vsireg* when vsiselect is in
0x200..0x2FF raise an virtual-instruction exception
for VU-mode and VS-mode.
* Execution of the SCTRCLR instruction raises an
virtual-instruction exception for VU-mode and VS-mode.
# If hstateen0.CTR = 1:
* Attempts to access sctrctl, vsctrctl, sctrdepth, or
sctrstatus raise an virtual-instruction exception
for VU-mode.
* *_/Attempts to access sctrdepth raise an
virtual-instruction exception for "VS-mode"/._*
* Attempts to access sireg* when siselect is in
0x200..0x2FF, or vsireg* when vsiselect is in
0x200..0x2FF raise an virtual-instruction exception
for VU-mode.
* Execution of the SCTRCLR instruction raises an
virtual-instruction exception for VU-mode.
* If Smstateen is disabled:
o Attempts to access sctrctl, vsctrctl, sctrdepth, or sctrstatus
raise an illegal-instruction exception for U-mode.
o Attempts to access sireg* when siselect is in 0x200..0x2FF, or
vsireg* when vsiselect is in 0x200..0x2FF raise an
illegal-instruction exception for U-mode.
o Execution of the SCTRCLR instruction raises an
illegal-instruction exception for U-mode.
o If H extension is enabled:
+ Attempts to access sctrctl, vsctrctl, sctrdepth, or
sctrstatus raise an virtual-instruction exception for VU-mode.
+ */_Attempts to access sctrdepth raise an virtual-instruction
exception for "VS-mode"._/*
+ Attempts to access sireg* when siselect is in 0x200..0x2FF,
or vsireg* when vsiselect is in 0x200..0x2FF raise an
virtual-instruction exception for VU-mode.
+ Execution of the SCTRCLR instruction raises an
virtual-instruction exception for VU-mode.
If there is any misunderstanding, please let me know. Also Sstateen0
does not involve in CTR. Am I correct?
Thanks in advance.
Beeman Strong 於 2024/6/5 上午 02:46 寫道:
>
>
> On Tue, Jun 4, 2024 at 10:19 AM Jason Chien <jason.chien@sifive.com>
> wrote:
>
>
> Rajnesh Kanwal 於 2024/5/30 上午 12:09 寫道:
> > CTR extension adds a new instruction sctrclr to quickly
> > clear the recorded entries buffer.
> >
> > Signed-off-by: Rajnesh Kanwal <rkanwal@rivosinc.com>
> > ---
> > target/riscv/cpu.h | 1 +
> > target/riscv/cpu_helper.c | 7 +++++++
> > target/riscv/insn32.decode | 1 +
> > target/riscv/insn_trans/trans_privileged.c.inc | 10 ++++++++++
> > target/riscv/op_helper.c | 5 +++++
> > 5 files changed, 24 insertions(+)
> >
> > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> > index a294a5372a..fade71aa09 100644
> > --- a/target/riscv/cpu.h
> > +++ b/target/riscv/cpu.h
> > @@ -572,6 +572,7 @@ void riscv_cpu_set_mode(CPURISCVState *env,
> target_ulong newpriv, bool virt_en);
> > void riscv_ctr_freeze(CPURISCVState *env, uint64_t freeze_mask);
> > void riscv_ctr_add_entry(CPURISCVState *env, target_long src,
> target_long dst,
> > uint64_t type, target_ulong
> prev_priv, bool prev_virt);
> > +void riscv_ctr_clear(CPURISCVState *env);
> >
> > void riscv_translate_init(void);
> > G_NORETURN void riscv_raise_exception(CPURISCVState *env,
> > diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> > index e064a7306e..45502f50a7 100644
> > --- a/target/riscv/cpu_helper.c
> > +++ b/target/riscv/cpu_helper.c
> > @@ -704,6 +704,13 @@ void riscv_ctr_freeze(CPURISCVState *env,
> uint64_t freeze_mask)
> > }
> > }
> >
> > +void riscv_ctr_clear(CPURISCVState *env)
> > +{
> > + memset(env->ctr_src, 0x0, sizeof(env->ctr_src));
> > + memset(env->ctr_dst, 0x0, sizeof(env->ctr_dst));
> > + memset(env->ctr_data, 0x0, sizeof(env->ctr_data));
> > +}
> > +
> > static uint64_t riscv_ctr_priv_to_mask(target_ulong priv, bool
> virt)
> > {
> > switch (priv) {
> > diff --git a/target/riscv/insn32.decode b/target/riscv/insn32.decode
> > index 9cb1a1b4ec..d3d38c7c68 100644
> > --- a/target/riscv/insn32.decode
> > +++ b/target/riscv/insn32.decode
> > @@ -107,6 +107,7 @@
> > # *** Privileged Instructions ***
> > ecall 000000000000 00000 000 00000 1110011
> > ebreak 000000000001 00000 000 00000 1110011
> > +sctrclr 000100000100 00000 000 00000 1110011
> > uret 0000000 00010 00000 000 00000 1110011
> > sret 0001000 00010 00000 000 00000 1110011
> > mret 0011000 00010 00000 000 00000 1110011
> > diff --git a/target/riscv/insn_trans/trans_privileged.c.inc
> b/target/riscv/insn_trans/trans_privileged.c.inc
> > index 339d659151..dd9da8651f 100644
> > --- a/target/riscv/insn_trans/trans_privileged.c.inc
> > +++ b/target/riscv/insn_trans/trans_privileged.c.inc
> > @@ -69,6 +69,16 @@ static bool trans_ebreak(DisasContext *ctx,
> arg_ebreak *a)
> > return true;
> > }
> >
> > +static bool trans_sctrclr(DisasContext *ctx, arg_sctrclr *a)
> > +{
> > +#ifndef CONFIG_USER_ONLY
> If both of smctr and ssctr are not enabled, it is an illegal
> instruction.
> > + gen_helper_ctr_clear(tcg_env);
> > + return true;
> > +#else
> > + return false;
> > +#endif
> > +}
> > +
> > static bool trans_uret(DisasContext *ctx, arg_uret *a)
> > {
> > return false;
> > diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
> > index c8053d9c2f..89423c04b3 100644
> > --- a/target/riscv/op_helper.c
> > +++ b/target/riscv/op_helper.c
> > @@ -461,6 +461,11 @@ void helper_ctr_branch(CPURISCVState *env,
> target_ulong src, target_ulong dest,
> > }
> > }
> >
> > +void helper_ctr_clear(CPURISCVState *env)
> > +{
>
> There should be some checks here.
> The spec states:
> SCTRCLR raises an illegal-instruction exception in U-mode, and a
> virtual-instruction exception in VU-mode, unless CTR state enable
> access
> restrictions apply.
>
> I don't quite understand "unless CTR state enable access restrictions
> apply" though.
>
>
> The next sentence says "See Chapter 5", which states that execution of
> SCTRCLR raises an illegal instruction exception if mstateen0.CTR=0 and
> the priv mode is not M-mode, and it raises a virtual instruction
> exception if mstateen0.CTR=0 && hstateen0.CTR=1 and the priv mode is
> VS-mode.
When mstateen0.CTR=0, hstateen0.CTR is read-only 0. I guess you meant
"mstateen0.CTR=1 && hstateen0.CTR=0" here.
>
>
> > + riscv_ctr_clear(env);
> > +}
> > +
> > void helper_wfi(CPURISCVState *env)
> > {
> > CPUState *cs = env_cpu(env);
>
[-- Attachment #2: Type: text/html, Size: 13402 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 5/6] target/riscv: Add CTR sctrclr instruction.
2024-06-05 6:28 ` Jason Chien
@ 2024-06-05 15:16 ` Beeman Strong
0 siblings, 0 replies; 22+ messages in thread
From: Beeman Strong @ 2024-06-05 15:16 UTC (permalink / raw)
To: Jason Chien
Cc: Rajnesh Kanwal, qemu-riscv, qemu-devel, alistair.francis,
bin.meng, liweiwei, dbarboza, zhiwei_liu, atishp, apatel,
tech-control-transfer-records
[-- Attachment #1: Type: text/plain, Size: 9733 bytes --]
On Tue, Jun 4, 2024 at 11:28 PM Jason Chien <jason.chien@sifive.com> wrote:
> Chapter 5 describes the CTR behavior when Smstaten is enabled, but it does
> not talk about the CTR behavior when Smstateen is disabled, that is, there
> is no mstateen0 and hstateen0 CSR.
>
If Smstateen is not implemented then chapter 5 can be ignored, and the
behavior is as described in the other chapters.
> The spec states:
>
> - Attempts to access sctrdepth from VS-mode or VU-mode raise a
> virtual-instruction exception, unless CTR state enable access restrictions
> apply.
>
>
> - sctrdepth is not included in the above list of supervisor CTR state
> controlled by hstateen0.CTR since accesses to sctrdepth from VS-mode raise
> a virtual-instruction exception regardless of the value of hstateen0.CTR.
>
> Below is my understanding:
>
> - If Smstateen is enabled:
> - If mstateen0.CTR=0:
> - Attempts to access sctrctl, vsctrctl, sctrdepth, or sctrstatus
> raise an illegal-instruction exception for privilege modes less privileged
> than M-mode.
> - Attempts to access sireg* when siselect is in 0x200..0x2FF, or
> vsireg* when vsiselect is in 0x200..0x2FF raise an illegal-instruction
> exception for privilege modes less privileged than M-mode.
> - Execution of the SCTRCLR instruction raises an
> illegal-instruction exception for privilege modes less privileged than
> M-mode.
> - If mstateen.CTR=1:
> - Attempts to access sctrctl, vsctrctl, sctrdepth, or sctrstatus
> raise an illegal-instruction exception for U-mode.
> - Attempts to access sireg* when siselect is in 0x200..0x2FF, or
> vsireg* when vsiselect is in 0x200..0x2FF raise an illegal-instruction
> exception for U-mode.
> - Execution of the SCTRCLR instruction raises an
> illegal-instruction exception for U-mode.
> - If H extension is enabled:
> - If hstateen0.CTR = 0:
> - Attempts to access sctrctl, vsctrctl, sctrdepth, or
> sctrstatus raise an virtual-instruction exception for VU-mode and VS-mode.
> - Attempts to access sireg* when siselect is in
> 0x200..0x2FF, or vsireg* when vsiselect is in 0x200..0x2FF raise an
> virtual-instruction exception for VU-mode and VS-mode.
> - Execution of the SCTRCLR instruction raises an
> virtual-instruction exception for VU-mode and VS-mode.
> - If hstateen0.CTR = 1:
> - Attempts to access sctrctl, vsctrctl, sctrdepth, or
> sctrstatus raise an virtual-instruction exception for VU-mode.
> - *Attempts to access sctrdepth raise an
> virtual-instruction exception for "VS-mode".*
> - Attempts to access sireg* when siselect is in
> 0x200..0x2FF, or vsireg* when vsiselect is in 0x200..0x2FF raise an
> virtual-instruction exception for VU-mode.
> - Execution of the SCTRCLR instruction raises an
> virtual-instruction exception for VU-mode.
> - If Smstateen is disabled:
> - Attempts to access sctrctl, vsctrctl, sctrdepth, or sctrstatus
> raise an illegal-instruction exception for U-mode.
> - Attempts to access sireg* when siselect is in 0x200..0x2FF, or
> vsireg* when vsiselect is in 0x200..0x2FF raise an illegal-instruction
> exception for U-mode.
> - Execution of the SCTRCLR instruction raises an
> illegal-instruction exception for U-mode.
> - If H extension is enabled:
> - Attempts to access sctrctl, vsctrctl, sctrdepth, or sctrstatus
> raise an virtual-instruction exception for VU-mode.
> - *Attempts to access sctrdepth raise an virtual-instruction
> exception for "VS-mode".*
> - Attempts to access sireg* when siselect is in 0x200..0x2FF, or
> vsireg* when vsiselect is in 0x200..0x2FF raise an virtual-instruction
> exception for VU-mode.
> - Execution of the SCTRCLR instruction raises an
> virtual-instruction exception for VU-mode.
>
> If there is any misunderstanding, please let me know. Also Sstateen0 does
> not involve in CTR. Am I correct?
>
That all looks correct. sctrdepth gets that special treatment in VS-mode
(bold and underlined above) because it is expected that a hypervisor will
wish to limit the depth options available to a guest, for the purposes of
VM-migration.
And yes, there is no U/VU-mode access to CTR, so Ssstaten does not apply to
CTR.
> Thanks in advance.
> Beeman Strong 於 2024/6/5 上午 02:46 寫道:
>
>
>
> On Tue, Jun 4, 2024 at 10:19 AM Jason Chien <jason.chien@sifive.com>
> wrote:
>
>>
>> Rajnesh Kanwal 於 2024/5/30 上午 12:09 寫道:
>> > CTR extension adds a new instruction sctrclr to quickly
>> > clear the recorded entries buffer.
>> >
>> > Signed-off-by: Rajnesh Kanwal <rkanwal@rivosinc.com>
>> > ---
>> > target/riscv/cpu.h | 1 +
>> > target/riscv/cpu_helper.c | 7 +++++++
>> > target/riscv/insn32.decode | 1 +
>> > target/riscv/insn_trans/trans_privileged.c.inc | 10 ++++++++++
>> > target/riscv/op_helper.c | 5 +++++
>> > 5 files changed, 24 insertions(+)
>> >
>> > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
>> > index a294a5372a..fade71aa09 100644
>> > --- a/target/riscv/cpu.h
>> > +++ b/target/riscv/cpu.h
>> > @@ -572,6 +572,7 @@ void riscv_cpu_set_mode(CPURISCVState *env,
>> target_ulong newpriv, bool virt_en);
>> > void riscv_ctr_freeze(CPURISCVState *env, uint64_t freeze_mask);
>> > void riscv_ctr_add_entry(CPURISCVState *env, target_long src,
>> target_long dst,
>> > uint64_t type, target_ulong prev_priv, bool
>> prev_virt);
>> > +void riscv_ctr_clear(CPURISCVState *env);
>> >
>> > void riscv_translate_init(void);
>> > G_NORETURN void riscv_raise_exception(CPURISCVState *env,
>> > diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
>> > index e064a7306e..45502f50a7 100644
>> > --- a/target/riscv/cpu_helper.c
>> > +++ b/target/riscv/cpu_helper.c
>> > @@ -704,6 +704,13 @@ void riscv_ctr_freeze(CPURISCVState *env, uint64_t
>> freeze_mask)
>> > }
>> > }
>> >
>> > +void riscv_ctr_clear(CPURISCVState *env)
>> > +{
>> > + memset(env->ctr_src, 0x0, sizeof(env->ctr_src));
>> > + memset(env->ctr_dst, 0x0, sizeof(env->ctr_dst));
>> > + memset(env->ctr_data, 0x0, sizeof(env->ctr_data));
>> > +}
>> > +
>> > static uint64_t riscv_ctr_priv_to_mask(target_ulong priv, bool virt)
>> > {
>> > switch (priv) {
>> > diff --git a/target/riscv/insn32.decode b/target/riscv/insn32.decode
>> > index 9cb1a1b4ec..d3d38c7c68 100644
>> > --- a/target/riscv/insn32.decode
>> > +++ b/target/riscv/insn32.decode
>> > @@ -107,6 +107,7 @@
>> > # *** Privileged Instructions ***
>> > ecall 000000000000 00000 000 00000 1110011
>> > ebreak 000000000001 00000 000 00000 1110011
>> > +sctrclr 000100000100 00000 000 00000 1110011
>> > uret 0000000 00010 00000 000 00000 1110011
>> > sret 0001000 00010 00000 000 00000 1110011
>> > mret 0011000 00010 00000 000 00000 1110011
>> > diff --git a/target/riscv/insn_trans/trans_privileged.c.inc
>> b/target/riscv/insn_trans/trans_privileged.c.inc
>> > index 339d659151..dd9da8651f 100644
>> > --- a/target/riscv/insn_trans/trans_privileged.c.inc
>> > +++ b/target/riscv/insn_trans/trans_privileged.c.inc
>> > @@ -69,6 +69,16 @@ static bool trans_ebreak(DisasContext *ctx,
>> arg_ebreak *a)
>> > return true;
>> > }
>> >
>> > +static bool trans_sctrclr(DisasContext *ctx, arg_sctrclr *a)
>> > +{
>> > +#ifndef CONFIG_USER_ONLY
>> If both of smctr and ssctr are not enabled, it is an illegal instruction.
>> > + gen_helper_ctr_clear(tcg_env);
>> > + return true;
>> > +#else
>> > + return false;
>> > +#endif
>> > +}
>> > +
>> > static bool trans_uret(DisasContext *ctx, arg_uret *a)
>> > {
>> > return false;
>> > diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
>> > index c8053d9c2f..89423c04b3 100644
>> > --- a/target/riscv/op_helper.c
>> > +++ b/target/riscv/op_helper.c
>> > @@ -461,6 +461,11 @@ void helper_ctr_branch(CPURISCVState *env,
>> target_ulong src, target_ulong dest,
>> > }
>> > }
>> >
>> > +void helper_ctr_clear(CPURISCVState *env)
>> > +{
>>
>> There should be some checks here.
>> The spec states:
>> SCTRCLR raises an illegal-instruction exception in U-mode, and a
>> virtual-instruction exception in VU-mode, unless CTR state enable access
>> restrictions apply.
>>
>> I don't quite understand "unless CTR state enable access restrictions
>> apply" though.
>>
>
> The next sentence says "See Chapter 5", which states that execution of
> SCTRCLR raises an illegal instruction exception if mstateen0.CTR=0 and the
> priv mode is not M-mode, and it raises a virtual instruction exception if
> mstateen0.CTR=0 && hstateen0.CTR=1 and the priv mode is VS-mode.
>
> When mstateen0.CTR=0, hstateen0.CTR is read-only 0. I guess you meant
> "mstateen0.CTR=1 && hstateen0.CTR=0" here.
>
You're right, thanks.
>
>
>>
>> > + riscv_ctr_clear(env);
>> > +}
>> > +
>> > void helper_wfi(CPURISCVState *env)
>> > {
>> > CPUState *cs = env_cpu(env);
>>
>
[-- Attachment #2: Type: text/html, Size: 14625 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 0/6] target/riscv: Add support for Control Transfer Records Ext.
2024-06-05 3:41 ` Beeman Strong
@ 2024-06-06 18:36 ` Beeman Strong
0 siblings, 0 replies; 22+ messages in thread
From: Beeman Strong @ 2024-06-06 18:36 UTC (permalink / raw)
To: Jason Chien
Cc: Rajnesh Kanwal, alistair.francis, apatel, atishp, bin.meng,
dbarboza, liweiwei, qemu-devel, qemu-riscv,
tech-control-transfer-records, zhiwei_liu
[-- Attachment #1: Type: text/plain, Size: 4894 bytes --]
PR for the spec fix, in case anyone is interested. Found a couple of other
references to Smcsrind that I also removed.
https://github.com/riscv/riscv-control-transfer-records/pull/29
On Tue, Jun 4, 2024 at 8:41 PM Beeman Strong <beeman@rivosinc.com> wrote:
> Ah, good catch. We removed that dependency late. I'll fix it.
>
> On Tue, Jun 4, 2024 at 8:34 PM Jason Chien <jason.chien@sifive.com> wrote:
>
>> Thank you for pointing that out. CTR does not use miselect and mireg*.
>> There is no dependency on Smcsrind. I believe the spec needs to be
>> corrected.
>>
>> Chapter 1 states that:
>> Smctr depends on the Smcsrind extension, while Ssctr depends on the
>> Sscsrind extension. Further, both Smctr and Ssctr depend upon
>> implementation of S-mode.
>> Beeman Strong 於 2024/6/5 上午 06:32 寫道:
>>
>> There is no dependency on Smcsrind, only Sscsrind.
>>
>> On Tue, Jun 4, 2024 at 12:29 AM Jason Chien <jason.chien@sifive.com>
>> wrote:
>>
>>> Smctr depends on the Smcsrind extension, Ssctr depends on the Sscsrind
>>> extension, and both Smctr and Ssctr depend upon implementation of S-mode.
>>> There should be a dependency check in
>>> riscv_cpu_validate_set_extensions().
>>>
>>> Rajnesh Kanwal 於 2024/5/30 上午 12:09 寫道:
>>> > This series enables Control Transfer Records extension support on riscv
>>> > platform. This extension is similar to Arch LBR in x86 and BRBE in ARM.
>>> > The Extension has been stable and the latest release can be found here
>>> [0]
>>> >
>>> > CTR extension depends on couple of other extensions:
>>> >
>>> > 1. S[m|s]csrind : The indirect CSR extension [1] which defines
>>> additional
>>> > ([M|S|VS]IREG2-[M|S|VS]IREG6) register to address size limitation
>>> of
>>> > RISC-V CSR address space. CTR access ctrsource, ctrtartget and
>>> ctrdata
>>> > CSRs using sscsrind extension.
>>> >
>>> > 2. Smstateen: The mstateen bit[54] controls the access to the CTR ext
>>> to
>>> > S-mode.
>>> >
>>> > 3. Sscofpmf: Counter overflow and privilege mode filtering. [2]
>>> >
>>> > The series is based on Smcdeleg/Ssccfg counter delegation extension [3]
>>> > patches. CTR itself doesn't depend on counter delegation support. This
>>> > rebase is basically to include the Smcsrind patches.
>>> >
>>> > Due to the dependency of these extensions, the following extensions
>>> must be
>>> > enabled to use the control transfer records feature.
>>> >
>>> >
>>> "smstateen=true,sscofpmf=true,smcsrind=true,sscsrind=true,smctr=true,ssctr=true"
>>> >
>>> > Here is the link to a quick guide [5] to setup and run a basic perf
>>> demo on
>>> > Linux to use CTR Ext.
>>> >
>>> > The Qemu patches can be found here:
>>> > https://github.com/rajnesh-kanwal/qemu/tree/ctr_upstream
>>> >
>>> > The opensbi patch can be found here:
>>> > https://github.com/rajnesh-kanwal/opensbi/tree/ctr_upstream
>>> >
>>> > The Linux kernel patches can be found here:
>>> > https://github.com/rajnesh-kanwal/linux/tree/ctr_upstream
>>> >
>>> > [0]:https://github.com/riscv/riscv-control-transfer-records/release
>>> > [1]:https://github.com/riscv/riscv-indirect-csr-access
>>> > [2]:https://github.com/riscvarchive/riscv-count-overflow/tree/main
>>> > [3]:https://github.com/riscv/riscv-smcdeleg-ssccfg
>>> > [4]:
>>> https://lore.kernel.org/all/20240217000134.3634191-1-atishp@rivosinc.com/
>>> > [5]:
>>> https://github.com/rajnesh-kanwal/linux/wiki/Running-CTR-basic-demo-on-QEMU-RISC%E2%80%90V-Virt-machine
>>> >
>>> > Rajnesh Kanwal (6):
>>> > target/riscv: Remove obsolete sfence.vm instruction
>>> > target/riscv: Add Control Transfer Records CSR definitions.
>>> > target/riscv: Add support for Control Transfer Records extension
>>> CSRs.
>>> > target/riscv: Add support to record CTR entries.
>>> > target/riscv: Add CTR sctrclr instruction.
>>> > target/riscv: Add support to access ctrsource, ctrtarget, ctrdata
>>> > regs.
>>> >
>>> > target/riscv/cpu.c | 4 +
>>> > target/riscv/cpu.h | 14 +
>>> > target/riscv/cpu_bits.h | 154 +++++++++
>>> > target/riscv/cpu_cfg.h | 2 +
>>> > target/riscv/cpu_helper.c | 213 ++++++++++++
>>> > target/riscv/csr.c | 312
>>> +++++++++++++++++-
>>> > target/riscv/helper.h | 8 +-
>>> > target/riscv/insn32.decode | 2 +-
>>> > .../riscv/insn_trans/trans_privileged.c.inc | 21 +-
>>> > target/riscv/insn_trans/trans_rvi.c.inc | 27 ++
>>> > target/riscv/op_helper.c | 117 ++++++-
>>> > target/riscv/translate.c | 9 +
>>> > 12 files changed, 870 insertions(+), 13 deletions(-)
>>> >
>>>
>>
[-- Attachment #2: Type: text/html, Size: 8750 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 3/6] target/riscv: Add support for Control Transfer Records extension CSRs.
2024-06-04 10:14 ` Jason Chien
@ 2024-06-10 14:11 ` Rajnesh Kanwal
2024-06-12 3:13 ` Jason Chien
0 siblings, 1 reply; 22+ messages in thread
From: Rajnesh Kanwal @ 2024-06-10 14:11 UTC (permalink / raw)
To: Jason Chien
Cc: qemu-riscv, qemu-devel, alistair.francis, bin.meng, liweiwei,
dbarboza, zhiwei_liu, atishp, apatel, beeman,
tech-control-transfer-records
[-- Attachment #1: Type: text/plain, Size: 14237 bytes --]
Thanks Jason for your review.
On Tue, Jun 4, 2024 at 11:14 AM Jason Chien <jason.chien@sifive.com> wrote:
>
>
> Rajnesh Kanwal 於 2024/5/30 上午 12:09 寫道:
>
> This commit adds support for [m|s|vs]ctrcontrol, sctrstatus and
> sctrdepth CSRs handling.
>
> Signed-off-by: Rajnesh Kanwal <rkanwal@rivosinc.com>
> ---
> target/riscv/cpu.h | 5 ++
> target/riscv/cpu_cfg.h | 2 +
> target/riscv/csr.c | 159 +++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 166 insertions(+)
>
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index a185e2d494..3d4d5172b8 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -263,6 +263,11 @@ struct CPUArchState {
> target_ulong mcause;
> target_ulong mtval; /* since: priv-1.10.0 */
>
> + uint64_t mctrctl;
> + uint32_t sctrdepth;
> + uint32_t sctrstatus;
> + uint64_t vsctrctl;
> +
> /* Machine and Supervisor interrupt priorities */
> uint8_t miprio[64];
> uint8_t siprio[64];
> diff --git a/target/riscv/cpu_cfg.h b/target/riscv/cpu_cfg.h
> index d9354dc80a..d329a65811 100644
> --- a/target/riscv/cpu_cfg.h
> +++ b/target/riscv/cpu_cfg.h
> @@ -123,6 +123,8 @@ struct RISCVCPUConfig {
> bool ext_zvfhmin;
> bool ext_smaia;
> bool ext_ssaia;
> + bool ext_smctr;
> + bool ext_ssctr;
> bool ext_sscofpmf;
> bool ext_smepmp;
> bool rvv_ta_all_1s;
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index 2f92e4b717..888084d8e5 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -621,6 +621,61 @@ static RISCVException pointer_masking(CPURISCVState
*env, int csrno)
> return RISCV_EXCP_ILLEGAL_INST;
> }
>
> +/*
> + * M-mode:
> + * Without ext_smctr raise illegal inst excep.
> + * Otherwise everything is accessible to m-mode.
> + *
> + * S-mode:
> + * Without ext_ssctr or mstateen.ctr raise illegal inst excep.
> + * Otherwise everything other than mctrctl is accessible.
> + *
> + * VS-mode:
> + * Without ext_ssctr or mstateen.ctr raise illegal inst excep.
> + * Without hstateen.ctr raise virtual illegal inst excep.
> + * Otherwise allow vsctrctl, sctrstatus, 0x200-0x2ff entry range.
> + * Always raise illegal instruction exception for sctrdepth.
> + */
> +static RISCVException ctr_mmode(CPURISCVState *env, int csrno)
> +{
> + /* Check if smctr-ext is present */
> + if (riscv_cpu_cfg(env)->ext_smctr) {
> + return RISCV_EXCP_NONE;
> + }
> +
> + return RISCV_EXCP_ILLEGAL_INST;
> +}
> +
> +static RISCVException ctr_smode(CPURISCVState *env, int csrno)
> +{
> + if ((env->priv == PRV_M && riscv_cpu_cfg(env)->ext_smctr) ||
> + (env->priv == PRV_S && !env->virt_enabled &&
> + riscv_cpu_cfg(env)->ext_ssctr)) {
> + return smstateen_acc_ok(env, 0, SMSTATEEN0_CTR);
> + }
> +
> + if (env->priv == PRV_S && env->virt_enabled &&
> + riscv_cpu_cfg(env)->ext_ssctr) {
> + if (csrno == CSR_SCTRSTATUS) {
>
> missing sctrctl?
>
> + return smstateen_acc_ok(env, 0, SMSTATEEN0_CTR);
> + }
> +
> + return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
> + }
> +
> + return RISCV_EXCP_ILLEGAL_INST;
> +}
>
> I think there is no need to bind M-mode with ext_smctr, S-mode with
ext_ssctr and VS-mode with ext_ssctr, since this predicate function is for
S-mode CSRs, which are defined in both smctr and ssctr, we just need to
check at least one of ext_ssctr or ext_smctr is true.
>
> The spec states that:
> Attempts to access sctrdepth from VS-mode or VU-mode raise a
virtual-instruction exception, unless CTR state enable access restrictions
apply.
>
> In my understanding, we should check the presence of smstateen extension
first, and
>
> if smstateen is implemented:
>
> for sctrctl and sctrstatus, call smstateen_acc_ok()
> for sctrdepth, call smstateen_acc_ok(), and if there is any exception
returned, always report virtual-instruction exception.
For sctrdepth, we are supposed to always return a virt-inst exception in
case of
VS-VU mode unless CTR state enable access restrictions apply.
So for sctrdepth, call smstateen_acc_ok(), and if there is no exception
returned
(mstateen.CTR=1 and hstateen.CTR=1 for virt mode), check if we are in
virtual
mode and return virtual-instruction exception otherwise return
RISCV_EXCP_NONE.
Note that if hstateen.CTR=0, smstateen_acc_ok() will return
virtual-instruction
exception which means regardless of the hstateen.CTR state, we will always
return virtual-instruction exception for VS/VU mode access to sctrdepth.
Basically this covers following rules for sctrdepth:
if mstateen.ctr == 0
return RISCV_EXCP_ILLEGAL_INST; // For all modes lower than M-mode.
else if in virt-mode // regardless of the state of hstateen.CTR
return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
else
return RISCV_EXCP_NONE
>
> If smstateen is not implemented:
>
> for sctrctl and sctrstatus, there is no check.
> for sctrdepth, I think the spec is ambiguous. What does "CTR state enable
access restrictions apply" mean when smstateen is not implemented?
As per my understanding, this means if mstateen.CTR=0 then we return an
illegal instruction exception regardless if it's virtual mode or not. This
is
the only effect of CTR state enable on sctrdepth CSR. If mstateen.CTR=1,
sctrdepth access from VS-mode results in virtual-instruction exception
regardless of hstateen.CTR.
Based on this, we have following model for predicate checks:
if smstateen is implemented:
for sctrctl and sctrstatus, call smstateen_acc_ok()
for sctrdepth, call smstateen_acc_ok(), and if there is no exception,
check if we are in virtual mode and return virtual-instruction exception
otherwise return RISCV_EXCP_NONE.
If smstateen is not implemented:
for sctrctl and sctrstatus, there is no check.
for sctrdepth, if in VS/VU mode return virtual-instruction exception
otherwise
no check.
Here is the code to better understand this.
static RISCVException ctr_smode(CPURISCVState *env, int csrno)
{
const RISCVCPUConfig *cfg = riscv_cpu_cfg(env);
if (!cfg->ext_ssctr && !cfg->ext_smctr) {
return RISCV_EXCP_ILLEGAL_INST;
}
if (riscv_cpu_cfg(env)->ext_smstateen) {
RISCVException ret = smstateen_acc_ok(env, 0, SMSTATEEN0_CTR);
if (ret == RISCV_EXCP_NONE && csrno == CSR_SCTRDEPTH &&
env->virt_enabled) {
return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
}
return ret;
} else {
if (csrno == CSR_SCTRDEPTH && env->virt_enabled) {
return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
}
}
return RISCV_EXCP_NONE;
}
Given smstateen_acc_ok() returns RISCV_EXCP_NONE in case if ext_smstateen
is not
implemented, this can be further simplified to:
static RISCVException ctr_smode(CPURISCVState *env, int csrno)
{
const RISCVCPUConfig *cfg = riscv_cpu_cfg(env);
if (!cfg->ext_ssctr && !cfg->ext_smctr) {
return RISCV_EXCP_ILLEGAL_INST;
}
RISCVException ret = smstateen_acc_ok(env, 0, SMSTATEEN0_CTR);
if (ret == RISCV_EXCP_NONE && csrno == CSR_SCTRDEPTH &&
env->virt_enabled) {
return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
}
return ret;
}
>
> Here is the code to better understand my description.
>
> static RISCVException ctr_smode(CPURISCVState *env, int csrno)
> {
> const RISCVCPUConfig *cfg = riscv_cpu_cfg(env);
>
> if (!cfg->ext_ssctr && !cfg->ext_smctr) {
> return RISCV_EXCP_ILLEGAL_INST;
> }
>
> if (riscv_cpu_cfg(env)->ext_smstateen) {
> RISCVException ret = smstateen_acc_ok(env, 0, SMSTATEEN0_CTR);
> if (ret != RISCV_EXCP_NONE) {
> if (csrno == CSR_SCTRDEPTH && env->virt_enabled) {
> return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
> }
>
> return ret;
> }
> } else {
> /* The spec is ambiguous. */
> if (csrno == CSR_SCTRDEPTH && env->virt_enabled) {
> return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
> }
> }
>
> return RISCV_EXCP_NONE;
> }
>
> +
> +static RISCVException ctr_vsmode(CPURISCVState *env, int csrno)
> +{
> + if (env->priv == PRV_S && env->virt_enabled &&
> + riscv_cpu_cfg(env)->ext_ssctr) {
> + return smstateen_acc_ok(env, 0, SMSTATEEN0_CTR);
>
> In riscv_csrrw_check(), an virtual-instruction exception is always
reported no matter what. Do we need this check?
>
> + }
> +
> + return ctr_smode(env, csrno);
> +}
> +
> static RISCVException aia_hmode(CPURISCVState *env, int csrno)
> {
> int ret;
> @@ -3835,6 +3890,100 @@ static RISCVException write_satp(CPURISCVState
*env, int csrno,
> return RISCV_EXCP_NONE;
> }
>
> +static RISCVException rmw_sctrdepth(CPURISCVState *env, int csrno,
> + target_ulong *ret_val,
> + target_ulong new_val, target_ulong
wr_mask)
> +{
> + uint64_t mask = wr_mask & SCTRDEPTH_MASK;
> +
> + if (ret_val) {
> + *ret_val = env->sctrdepth & SCTRDEPTH_MASK;
>
> We don't need to do bitwise and with SCTRDEPTH_MASK on read accesses when
we always do bitwise and with SCTRDEPTH_MASK on write accesses.
>
> + }
> +
> + env->sctrdepth = (env->sctrdepth & ~mask) | (new_val & mask);
> +
> + /* Correct depth. */
> + if (wr_mask & SCTRDEPTH_MASK) {
> + uint64_t depth = get_field(env->sctrdepth, SCTRDEPTH_MASK);
> +
> + if (depth > SCTRDEPTH_MAX) {
> + env->sctrdepth =
> + set_field(env->sctrdepth, SCTRDEPTH_MASK, SCTRDEPTH_MAX);
> + }
> +
> + /* Update sctrstatus.WRPTR with a legal value */
> + depth = 16 << depth;
>
> The "depth" on the right side may exceed SCTRDEPTH_MAX.
>
> + env->sctrstatus =
> + env->sctrstatus & (~SCTRSTATUS_WRPTR_MASK | (depth - 1));
> + }
> +
> + return RISCV_EXCP_NONE;
> +}
> +
> +static RISCVException rmw_mctrctl(CPURISCVState *env, int csrno,
> + target_ulong *ret_val,
> + target_ulong new_val, target_ulong
wr_mask)
> +{
> + uint64_t mask = wr_mask & MCTRCTL_MASK;
> +
> + if (ret_val) {
> + *ret_val = env->mctrctl & MCTRCTL_MASK;
>
> There is no need to do bitwise and with the mask on read accesses when we
always do bitwise and with the mask on write accesses.
>
> + }
> +
> + env->mctrctl = (env->mctrctl & ~mask) | (new_val & mask);
> +
> + return RISCV_EXCP_NONE;
> +}
> +
> +static RISCVException rmw_sctrctl(CPURISCVState *env, int csrno,
> + target_ulong *ret_val,
> + target_ulong new_val, target_ulong
wr_mask)
> +{
> + uint64_t mask = wr_mask & SCTRCTL_MASK;
> + RISCVException ret;
> +
> + ret = rmw_mctrctl(env, csrno, ret_val, new_val, mask);
>
> When V=1, vsctrctl substitutes for sctrctl.
>
> + if (ret_val) {
> + *ret_val &= SCTRCTL_MASK;
> + }
> +
> + return ret;
> +}
> +
> +static RISCVException rmw_sctrstatus(CPURISCVState *env, int csrno,
> + target_ulong *ret_val,
> + target_ulong new_val, target_ulong
wr_mask)
> +{
> + uint32_t depth = 16 << get_field(env->sctrdepth, SCTRDEPTH_MASK);
> + uint32_t mask = wr_mask & SCTRSTATUS_MASK;
> +
> + if (ret_val) {
> + *ret_val = env->sctrstatus & SCTRSTATUS_MASK;
>
> There is no need to do bitwise and with the mask on read accesses when we
always do bitwise and with the mask on write accesses.
>
> + }
> +
> + env->sctrstatus = (env->sctrstatus & ~mask) | (new_val & mask);
> +
> + /* Update sctrstatus.WRPTR with a legal value */
> + env->sctrstatus = env->sctrstatus & (~SCTRSTATUS_WRPTR_MASK | (depth
- 1));
> +
> + return RISCV_EXCP_NONE;
> +}
> +
> +static RISCVException rmw_vsctrctl(CPURISCVState *env, int csrno,
> + target_ulong *ret_val,
> + target_ulong new_val, target_ulong
wr_mask)
> +{
> + uint64_t mask = wr_mask & VSCTRCTL_MASK;
> +
> + if (ret_val) {
> + *ret_val = env->vsctrctl & VSCTRCTL_MASK;
>
> There is no need to do bitwise and with the mask on read accesses when we
always do bitwise and with the mask on write accesses.
>
> + }
> +
> + env->vsctrctl = (env->vsctrctl & ~mask) | (new_val & mask);
> +
> + return RISCV_EXCP_NONE;
> +}
>
> Is it possible to define rmw_xctrctl() instead of three individual rmw
functions and use a switch case to select the mask and the CSR for the
purpose of reducing code size?
>
> +
> static RISCVException read_vstopi(CPURISCVState *env, int csrno,
> target_ulong *val)
> {
> @@ -5771,6 +5920,16 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
> [CSR_SPMBASE] = { "spmbase", pointer_masking, read_spmbase,
> write_spmbase
},
>
> + [CSR_MCTRCTL] = { "mctrctl", ctr_mmode, NULL, NULL,
> + rmw_mctrctl },
>
> I think this can be one line.
>
> + [CSR_SCTRCTL] = { "sctrctl", ctr_smode, NULL, NULL,
> + rmw_sctrctl },
>
> same here
>
> + [CSR_SCTRDEPTH] = { "sctrdepth", ctr_smode, NULL, NULL,
> + rmw_sctrdepth },
>
> same here
>
> + [CSR_SCTRSTATUS] = { "sctrstatus", ctr_smode, NULL, NULL,
> + rmw_sctrstatus },
>
> same here
>
> + [CSR_VSCTRCTL] = { "vsctrctl", ctr_vsmode, NULL, NULL,
> + rmw_vsctrctl },
>
> same here
>
> /* Performance Counters */
> [CSR_HPMCOUNTER3] = { "hpmcounter3", ctr, read_hpmcounter },
> [CSR_HPMCOUNTER4] = { "hpmcounter4", ctr, read_hpmcounter },
[-- Attachment #2: Type: text/html, Size: 17443 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 3/6] target/riscv: Add support for Control Transfer Records extension CSRs.
2024-06-10 14:11 ` Rajnesh Kanwal
@ 2024-06-12 3:13 ` Jason Chien
0 siblings, 0 replies; 22+ messages in thread
From: Jason Chien @ 2024-06-12 3:13 UTC (permalink / raw)
To: Rajnesh Kanwal
Cc: qemu-riscv, qemu-devel, alistair.francis, bin.meng, liweiwei,
dbarboza, zhiwei_liu, atishp, apatel, beeman,
tech-control-transfer-records
[-- Attachment #1: Type: text/plain, Size: 15257 bytes --]
It makes sense. Thank you for the explanation.
Rajnesh Kanwal <rkanwal@rivosinc.com> 於 2024年6月10日 週一 下午10:12寫道：
>
> Thanks Jason for your review.
>
> On Tue, Jun 4, 2024 at 11:14 AM Jason Chien <jason.chien@sifive.com>
> wrote:
> >
> >
> > Rajnesh Kanwal 於 2024/5/30 上午 12:09 寫道:
> >
> > This commit adds support for [m|s|vs]ctrcontrol, sctrstatus and
> > sctrdepth CSRs handling.
> >
> > Signed-off-by: Rajnesh Kanwal <rkanwal@rivosinc.com>
> > ---
> > target/riscv/cpu.h | 5 ++
> > target/riscv/cpu_cfg.h | 2 +
> > target/riscv/csr.c | 159 +++++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 166 insertions(+)
> >
> > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> > index a185e2d494..3d4d5172b8 100644
> > --- a/target/riscv/cpu.h
> > +++ b/target/riscv/cpu.h
> > @@ -263,6 +263,11 @@ struct CPUArchState {
> > target_ulong mcause;
> > target_ulong mtval; /* since: priv-1.10.0 */
> >
> > + uint64_t mctrctl;
> > + uint32_t sctrdepth;
> > + uint32_t sctrstatus;
> > + uint64_t vsctrctl;
> > +
> > /* Machine and Supervisor interrupt priorities */
> > uint8_t miprio[64];
> > uint8_t siprio[64];
> > diff --git a/target/riscv/cpu_cfg.h b/target/riscv/cpu_cfg.h
> > index d9354dc80a..d329a65811 100644
> > --- a/target/riscv/cpu_cfg.h
> > +++ b/target/riscv/cpu_cfg.h
> > @@ -123,6 +123,8 @@ struct RISCVCPUConfig {
> > bool ext_zvfhmin;
> > bool ext_smaia;
> > bool ext_ssaia;
> > + bool ext_smctr;
> > + bool ext_ssctr;
> > bool ext_sscofpmf;
> > bool ext_smepmp;
> > bool rvv_ta_all_1s;
> > diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> > index 2f92e4b717..888084d8e5 100644
> > --- a/target/riscv/csr.c
> > +++ b/target/riscv/csr.c
> > @@ -621,6 +621,61 @@ static RISCVException pointer_masking(CPURISCVState
> *env, int csrno)
> > return RISCV_EXCP_ILLEGAL_INST;
> > }
> >
> > +/*
> > + * M-mode:
> > + * Without ext_smctr raise illegal inst excep.
> > + * Otherwise everything is accessible to m-mode.
> > + *
> > + * S-mode:
> > + * Without ext_ssctr or mstateen.ctr raise illegal inst excep.
> > + * Otherwise everything other than mctrctl is accessible.
> > + *
> > + * VS-mode:
> > + * Without ext_ssctr or mstateen.ctr raise illegal inst excep.
> > + * Without hstateen.ctr raise virtual illegal inst excep.
> > + * Otherwise allow vsctrctl, sctrstatus, 0x200-0x2ff entry range.
> > + * Always raise illegal instruction exception for sctrdepth.
> > + */
> > +static RISCVException ctr_mmode(CPURISCVState *env, int csrno)
> > +{
> > + /* Check if smctr-ext is present */
> > + if (riscv_cpu_cfg(env)->ext_smctr) {
> > + return RISCV_EXCP_NONE;
> > + }
> > +
> > + return RISCV_EXCP_ILLEGAL_INST;
> > +}
> > +
> > +static RISCVException ctr_smode(CPURISCVState *env, int csrno)
> > +{
> > + if ((env->priv == PRV_M && riscv_cpu_cfg(env)->ext_smctr) ||
> > + (env->priv == PRV_S && !env->virt_enabled &&
> > + riscv_cpu_cfg(env)->ext_ssctr)) {
> > + return smstateen_acc_ok(env, 0, SMSTATEEN0_CTR);
> > + }
> > +
> > + if (env->priv == PRV_S && env->virt_enabled &&
> > + riscv_cpu_cfg(env)->ext_ssctr) {
> > + if (csrno == CSR_SCTRSTATUS) {
> >
> > missing sctrctl?
> >
> > + return smstateen_acc_ok(env, 0, SMSTATEEN0_CTR);
> > + }
> > +
> > + return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
> > + }
> > +
> > + return RISCV_EXCP_ILLEGAL_INST;
> > +}
> >
> > I think there is no need to bind M-mode with ext_smctr, S-mode with
> ext_ssctr and VS-mode with ext_ssctr, since this predicate function is for
> S-mode CSRs, which are defined in both smctr and ssctr, we just need to
> check at least one of ext_ssctr or ext_smctr is true.
> >
> > The spec states that:
> > Attempts to access sctrdepth from VS-mode or VU-mode raise a
> virtual-instruction exception, unless CTR state enable access restrictions
> apply.
> >
> > In my understanding, we should check the presence of smstateen extension
> first, and
> >
> > if smstateen is implemented:
> >
> > for sctrctl and sctrstatus, call smstateen_acc_ok()
> > for sctrdepth, call smstateen_acc_ok(), and if there is any exception
> returned, always report virtual-instruction exception.
>
> For sctrdepth, we are supposed to always return a virt-inst exception in
> case of
> VS-VU mode unless CTR state enable access restrictions apply.
>
> So for sctrdepth, call smstateen_acc_ok(), and if there is no exception
> returned
> (mstateen.CTR=1 and hstateen.CTR=1 for virt mode), check if we are in
> virtual
> mode and return virtual-instruction exception otherwise return
> RISCV_EXCP_NONE.
> Note that if hstateen.CTR=0, smstateen_acc_ok() will return
> virtual-instruction
> exception which means regardless of the hstateen.CTR state, we will always
> return virtual-instruction exception for VS/VU mode access to sctrdepth.
>
> Basically this covers following rules for sctrdepth:
>
> if mstateen.ctr == 0
> return RISCV_EXCP_ILLEGAL_INST; // For all modes lower than M-mode.
> else if in virt-mode // regardless of the state of hstateen.CTR
> return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
> else
> return RISCV_EXCP_NONE
>
> >
> > If smstateen is not implemented:
> >
> > for sctrctl and sctrstatus, there is no check.
> > for sctrdepth, I think the spec is ambiguous. What does "CTR state
> enable access restrictions apply" mean when smstateen is not implemented?
>
> As per my understanding, this means if mstateen.CTR=0 then we return an
> illegal instruction exception regardless if it's virtual mode or not. This
> is
> the only effect of CTR state enable on sctrdepth CSR. If mstateen.CTR=1,
> sctrdepth access from VS-mode results in virtual-instruction exception
> regardless of hstateen.CTR.
>
> Based on this, we have following model for predicate checks:
>
> if smstateen is implemented:
>
> for sctrctl and sctrstatus, call smstateen_acc_ok()
> for sctrdepth, call smstateen_acc_ok(), and if there is no exception,
> check if we are in virtual mode and return virtual-instruction
> exception
> otherwise return RISCV_EXCP_NONE.
>
> If smstateen is not implemented:
>
> for sctrctl and sctrstatus, there is no check.
> for sctrdepth, if in VS/VU mode return virtual-instruction exception
> otherwise
> no check.
>
> Here is the code to better understand this.
>
> static RISCVException ctr_smode(CPURISCVState *env, int csrno)
> {
> const RISCVCPUConfig *cfg = riscv_cpu_cfg(env);
>
> if (!cfg->ext_ssctr && !cfg->ext_smctr) {
> return RISCV_EXCP_ILLEGAL_INST;
> }
>
> if (riscv_cpu_cfg(env)->ext_smstateen) {
> RISCVException ret = smstateen_acc_ok(env, 0, SMSTATEEN0_CTR);
> if (ret == RISCV_EXCP_NONE && csrno == CSR_SCTRDEPTH &&
> env->virt_enabled) {
> return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
> }
> return ret;
> } else {
> if (csrno == CSR_SCTRDEPTH && env->virt_enabled) {
> return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
> }
> }
>
> return RISCV_EXCP_NONE;
> }
>
> Given smstateen_acc_ok() returns RISCV_EXCP_NONE in case if ext_smstateen
> is not
> implemented, this can be further simplified to:
>
> static RISCVException ctr_smode(CPURISCVState *env, int csrno)
> {
> const RISCVCPUConfig *cfg = riscv_cpu_cfg(env);
>
> if (!cfg->ext_ssctr && !cfg->ext_smctr) {
> return RISCV_EXCP_ILLEGAL_INST;
> }
>
> RISCVException ret = smstateen_acc_ok(env, 0, SMSTATEEN0_CTR);
> if (ret == RISCV_EXCP_NONE && csrno == CSR_SCTRDEPTH &&
> env->virt_enabled) {
> return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
> }
> return ret;
> }
>
> >
> > Here is the code to better understand my description.
> >
> > static RISCVException ctr_smode(CPURISCVState *env, int csrno)
> > {
> > const RISCVCPUConfig *cfg = riscv_cpu_cfg(env);
> >
> > if (!cfg->ext_ssctr && !cfg->ext_smctr) {
> > return RISCV_EXCP_ILLEGAL_INST;
> > }
> >
> > if (riscv_cpu_cfg(env)->ext_smstateen) {
> > RISCVException ret = smstateen_acc_ok(env, 0, SMSTATEEN0_CTR);
> > if (ret != RISCV_EXCP_NONE) {
> > if (csrno == CSR_SCTRDEPTH && env->virt_enabled) {
> > return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
> > }
> >
> > return ret;
> > }
> > } else {
> > /* The spec is ambiguous. */
> > if (csrno == CSR_SCTRDEPTH && env->virt_enabled) {
> > return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
> > }
> > }
> >
> > return RISCV_EXCP_NONE;
> > }
> >
> > +
> > +static RISCVException ctr_vsmode(CPURISCVState *env, int csrno)
> > +{
> > + if (env->priv == PRV_S && env->virt_enabled &&
> > + riscv_cpu_cfg(env)->ext_ssctr) {
> > + return smstateen_acc_ok(env, 0, SMSTATEEN0_CTR);
> >
> > In riscv_csrrw_check(), an virtual-instruction exception is always
> reported no matter what. Do we need this check?
> >
> > + }
> > +
> > + return ctr_smode(env, csrno);
> > +}
> > +
> > static RISCVException aia_hmode(CPURISCVState *env, int csrno)
> > {
> > int ret;
> > @@ -3835,6 +3890,100 @@ static RISCVException write_satp(CPURISCVState
> *env, int csrno,
> > return RISCV_EXCP_NONE;
> > }
> >
> > +static RISCVException rmw_sctrdepth(CPURISCVState *env, int csrno,
> > + target_ulong *ret_val,
> > + target_ulong new_val, target_ulong
> wr_mask)
> > +{
> > + uint64_t mask = wr_mask & SCTRDEPTH_MASK;
> > +
> > + if (ret_val) {
> > + *ret_val = env->sctrdepth & SCTRDEPTH_MASK;
> >
> > We don't need to do bitwise and with SCTRDEPTH_MASK on read accesses
> when we always do bitwise and with SCTRDEPTH_MASK on write accesses.
> >
> > + }
> > +
> > + env->sctrdepth = (env->sctrdepth & ~mask) | (new_val & mask);
> > +
> > + /* Correct depth. */
> > + if (wr_mask & SCTRDEPTH_MASK) {
> > + uint64_t depth = get_field(env->sctrdepth, SCTRDEPTH_MASK);
> > +
> > + if (depth > SCTRDEPTH_MAX) {
> > + env->sctrdepth =
> > + set_field(env->sctrdepth, SCTRDEPTH_MASK,
> SCTRDEPTH_MAX);
> > + }
> > +
> > + /* Update sctrstatus.WRPTR with a legal value */
> > + depth = 16 << depth;
> >
> > The "depth" on the right side may exceed SCTRDEPTH_MAX.
> >
> > + env->sctrstatus =
> > + env->sctrstatus & (~SCTRSTATUS_WRPTR_MASK | (depth - 1));
> > + }
> > +
> > + return RISCV_EXCP_NONE;
> > +}
> > +
> > +static RISCVException rmw_mctrctl(CPURISCVState *env, int csrno,
> > + target_ulong *ret_val,
> > + target_ulong new_val, target_ulong
> wr_mask)
> > +{
> > + uint64_t mask = wr_mask & MCTRCTL_MASK;
> > +
> > + if (ret_val) {
> > + *ret_val = env->mctrctl & MCTRCTL_MASK;
> >
> > There is no need to do bitwise and with the mask on read accesses when
> we always do bitwise and with the mask on write accesses.
> >
> > + }
> > +
> > + env->mctrctl = (env->mctrctl & ~mask) | (new_val & mask);
> > +
> > + return RISCV_EXCP_NONE;
> > +}
> > +
> > +static RISCVException rmw_sctrctl(CPURISCVState *env, int csrno,
> > + target_ulong *ret_val,
> > + target_ulong new_val, target_ulong
> wr_mask)
> > +{
> > + uint64_t mask = wr_mask & SCTRCTL_MASK;
> > + RISCVException ret;
> > +
> > + ret = rmw_mctrctl(env, csrno, ret_val, new_val, mask);
> >
> > When V=1, vsctrctl substitutes for sctrctl.
> >
> > + if (ret_val) {
> > + *ret_val &= SCTRCTL_MASK;
> > + }
> > +
> > + return ret;
> > +}
> > +
> > +static RISCVException rmw_sctrstatus(CPURISCVState *env, int csrno,
> > + target_ulong *ret_val,
> > + target_ulong new_val, target_ulong
> wr_mask)
> > +{
> > + uint32_t depth = 16 << get_field(env->sctrdepth, SCTRDEPTH_MASK);
> > + uint32_t mask = wr_mask & SCTRSTATUS_MASK;
> > +
> > + if (ret_val) {
> > + *ret_val = env->sctrstatus & SCTRSTATUS_MASK;
> >
> > There is no need to do bitwise and with the mask on read accesses when
> we always do bitwise and with the mask on write accesses.
> >
> > + }
> > +
> > + env->sctrstatus = (env->sctrstatus & ~mask) | (new_val & mask);
> > +
> > + /* Update sctrstatus.WRPTR with a legal value */
> > + env->sctrstatus = env->sctrstatus & (~SCTRSTATUS_WRPTR_MASK |
> (depth - 1));
> > +
> > + return RISCV_EXCP_NONE;
> > +}
> > +
> > +static RISCVException rmw_vsctrctl(CPURISCVState *env, int csrno,
> > + target_ulong *ret_val,
> > + target_ulong new_val, target_ulong
> wr_mask)
> > +{
> > + uint64_t mask = wr_mask & VSCTRCTL_MASK;
> > +
> > + if (ret_val) {
> > + *ret_val = env->vsctrctl & VSCTRCTL_MASK;
> >
> > There is no need to do bitwise and with the mask on read accesses when
> we always do bitwise and with the mask on write accesses.
> >
> > + }
> > +
> > + env->vsctrctl = (env->vsctrctl & ~mask) | (new_val & mask);
> > +
> > + return RISCV_EXCP_NONE;
> > +}
> >
> > Is it possible to define rmw_xctrctl() instead of three individual rmw
> functions and use a switch case to select the mask and the CSR for the
> purpose of reducing code size?
> >
> > +
> > static RISCVException read_vstopi(CPURISCVState *env, int csrno,
> > target_ulong *val)
> > {
> > @@ -5771,6 +5920,16 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
> > [CSR_SPMBASE] = { "spmbase", pointer_masking, read_spmbase,
> > write_spmbase
> },
> >
> > + [CSR_MCTRCTL] = { "mctrctl", ctr_mmode, NULL, NULL,
> > + rmw_mctrctl },
> >
> > I think this can be one line.
> >
> > + [CSR_SCTRCTL] = { "sctrctl", ctr_smode, NULL, NULL,
> > + rmw_sctrctl },
> >
> > same here
> >
> > + [CSR_SCTRDEPTH] = { "sctrdepth", ctr_smode, NULL, NULL,
> > + rmw_sctrdepth },
> >
> > same here
> >
> > + [CSR_SCTRSTATUS] = { "sctrstatus", ctr_smode, NULL,
> NULL,
> > + rmw_sctrstatus },
> >
> > same here
> >
> > + [CSR_VSCTRCTL] = { "vsctrctl", ctr_vsmode, NULL, NULL,
> > + rmw_vsctrctl },
> >
> > same here
> >
> > /* Performance Counters */
> > [CSR_HPMCOUNTER3] = { "hpmcounter3", ctr, read_hpmcounter
> },
> > [CSR_HPMCOUNTER4] = { "hpmcounter4", ctr, read_hpmcounter
> },
>
[-- Attachment #2: Type: text/html, Size: 17891 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2024-06-12 3:14 UTC | newest]
Thread overview: 22+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-05-29 16:09 [PATCH 0/6] target/riscv: Add support for Control Transfer Records Ext Rajnesh Kanwal
2024-05-29 16:09 ` [PATCH 1/6] target/riscv: Remove obsolete sfence.vm instruction Rajnesh Kanwal
2024-06-05 0:46 ` Alistair Francis
2024-05-29 16:09 ` [PATCH 2/6] target/riscv: Add Control Transfer Records CSR definitions Rajnesh Kanwal
2024-05-29 16:09 ` [PATCH 3/6] target/riscv: Add support for Control Transfer Records extension CSRs Rajnesh Kanwal
2024-06-04 10:14 ` Jason Chien
2024-06-10 14:11 ` Rajnesh Kanwal
2024-06-12 3:13 ` Jason Chien
2024-05-29 16:09 ` [PATCH 4/6] target/riscv: Add support to record CTR entries Rajnesh Kanwal
2024-06-04 16:30 ` Jason Chien
2024-05-29 16:09 ` [PATCH 5/6] target/riscv: Add CTR sctrclr instruction Rajnesh Kanwal
2024-06-04 17:19 ` Jason Chien
2024-06-04 18:46 ` Beeman Strong
2024-06-05 6:28 ` Jason Chien
2024-06-05 15:16 ` Beeman Strong
2024-05-29 16:09 ` [PATCH 6/6] target/riscv: Add support to access ctrsource, ctrtarget, ctrdata regs Rajnesh Kanwal
2024-06-04 17:07 ` Jason Chien
2024-06-04 7:29 ` [PATCH 0/6] target/riscv: Add support for Control Transfer Records Ext Jason Chien
2024-06-04 22:32 ` Beeman Strong
2024-06-05 3:34 ` Jason Chien
2024-06-05 3:41 ` Beeman Strong
2024-06-06 18:36 ` Beeman Strong
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).