qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 1/4] target/riscv: Add CSR tcontrol of debug trigger module


From: Daniel Henrique Barboza
Subject: Re: [PATCH 1/4] target/riscv: Add CSR tcontrol of debug trigger module
Date: Fri, 16 Feb 2024 12:04:50 -0300
User-agent: Mozilla Thunderbird



On 2/16/24 10:42, Alvin Che-Chia Chang(張哲嘉) wrote:
Hi Daniel,

-----Original Message-----
From: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
Sent: Friday, February 16, 2024 8:51 PM
To: Alvin Che-Chia Chang(張哲嘉) <alvinga@andestech.com>;
qemu-riscv@nongnu.org; qemu-devel@nongnu.org
Cc: alistair.francis@wdc.com; bin.meng@windriver.com;
liwei1518@gmail.com; zhiwei_liu@linux.alibaba.com
Subject: Re: [PATCH 1/4] target/riscv: Add CSR tcontrol of debug trigger module



On 2/16/24 03:13, Alvin Chang wrote:
The RISC-V debug specification defines an optional CSR "tcontrol"
within the trigger module. This commit adds its read/write operations
and related bit-field definitions.

Signed-off-by: Alvin Chang <alvinga@andestech.com>
---
   target/riscv/cpu.h      |  1 +
   target/riscv/cpu_bits.h |  3 +++
   target/riscv/csr.c      | 15 +++++++++++++++
   3 files changed, 19 insertions(+)

diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h index
f52dce78ba..f9ae3e3025 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -364,6 +364,7 @@ struct CPUArchState {
       target_ulong tdata1[RV_MAX_TRIGGERS];
       target_ulong tdata2[RV_MAX_TRIGGERS];
       target_ulong tdata3[RV_MAX_TRIGGERS];
+    target_ulong tcontrol;
       target_ulong mcontext;
       struct CPUBreakpoint *cpu_breakpoint[RV_MAX_TRIGGERS];
       struct CPUWatchpoint *cpu_watchpoint[RV_MAX_TRIGGERS]; diff
--git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h index
fc2068ee4d..3b3a7a0fa4 100644
--- a/target/riscv/cpu_bits.h
+++ b/target/riscv/cpu_bits.h
@@ -353,6 +353,7 @@
   #define CSR_TDATA2          0x7a2
   #define CSR_TDATA3          0x7a3
   #define CSR_TINFO           0x7a4
+#define CSR_TCONTROL        0x7a5
   #define CSR_MCONTEXT        0x7a8

   /* Debug Mode Registers */
@@ -900,6 +901,8 @@ typedef enum RISCVException {
   #define JVT_BASE                           (~0x3F)

   /* Debug Sdtrig CSR masks */
+#define TCONTROL_MTE                       BIT(3)
+#define TCONTROL_MPTE                      BIT(7)
   #define MCONTEXT32                         0x0000003F
   #define MCONTEXT64
0x0000000000001FFFULL
   #define MCONTEXT32_HCONTEXT                0x0000007F
diff --git a/target/riscv/csr.c b/target/riscv/csr.c index
d4e8ac13b9..816c530481 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -3937,6 +3937,20 @@ static RISCVException read_tinfo(CPURISCVState
*env, int csrno,
       return RISCV_EXCP_NONE;
   }

+static RISCVException read_tcontrol(CPURISCVState *env, int csrno,
+                                    target_ulong *val) {
+    *val = env->tcontrol;
+    return RISCV_EXCP_NONE;
+}
+
+static RISCVException write_tcontrol(CPURISCVState *env, int csrno,
+                                     target_ulong val) {
+    env->tcontrol = val & (TCONTROL_MPTE | TCONTROL_MTE);
+    return RISCV_EXCP_NONE;
+}
+
   static RISCVException read_mcontext(CPURISCVState *env, int csrno,
                                       target_ulong *val)
   {
@@ -4861,6 +4875,7 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
       [CSR_TDATA2]    =  { "tdata2",   debug, read_tdata,
write_tdata    },
       [CSR_TDATA3]    =  { "tdata3",   debug, read_tdata,
write_tdata    },
       [CSR_TINFO]     =  { "tinfo",    debug, read_tinfo,
write_ignore   },
+    [CSR_TCONTROL]  =  { "tcontrol", debug, read_tcontrol,
+ write_tcontrol },

The spec reads:

"This optional register is only accessible in M-mode and Debug Mode and
provides
   various control bits related to triggers."

"debug()" is checking only if we have the 'debug' cpu option enabled:


static RISCVException debug(CPURISCVState *env, int csrno) {
      if (riscv_cpu_cfg(env)->debug) {
          return RISCV_EXCP_NONE;
      }

      return RISCV_EXCP_ILLEGAL_INST;
}


It looks like we don't have a "Debug Mode" model.

Yes, currently RISC-V QEMU doesn't have "Debug Mode", so I just ignore it and 
only consider M-mode here.


Section 4.1 of the spec mentions the following about "Debug Mode":

"1. All implemented instructions operate just as they do in M-mode, unless an
   exception is mentioned in this list.
   2. All operations are executed with machine mode privilege, except that
additional
   Debug Mode CSRs are accessible and MPRV in mstatus may be ignored
according to
   mprven. Full permission checks, or a relaxed set of permission checks, will
apply
   according to relaxedpriv (...)"


So, if the operations are "executed with machine mode privilege" then can we
expect
env->priv == PRV_M ? As it is now tcontrol will execute in any mode, so
env->checking
for PRV_M seems reasonable.

The riscv_csrrw_check() function has checked the privilege level by the 
accessed CSR address.
Please see https://github.com/qemu/qemu/blob/master/target/riscv/csr.c#L4360.
If the current privilege level is lower than the accessed CSR, the illegal 
instruction exception is raised.

According to privilege spec, each CSR address has two bits to encode the lowest 
privilege level to access that CSR.
Sdtrig CSRs (tselect, tdata{1~3}, tinfo, mcontext, and tcontrol) are encoded 
within 0x7A0~0x7AF,
Therefore, their lowest required privilege level is M-mode.
If env->priv < PRV_M, the checking on those CSR fails in riscv_csrrw_check().

Is this enough, or do I miss something?


This is enough, thanks for clarifying.

And since we're not modelling a "Debug Mode" I think this is already the best 
we can
do ATM.


Thanks,

Daniel


Sincerely,
Alvin Chang



Thanks,


Daniel





       [CSR_MCONTEXT]  =  { "mcontext", debug, read_mcontext,
write_mcontext },

       /* User Pointer Masking */



reply via email to

[Prev in Thread] Current Thread [Next in Thread]