xiaoxiang781216 commented on a change in pull request #5782:
URL: https://github.com/apache/incubator-nuttx/pull/5782#discussion_r837389096



##########
File path: arch/risc-v/src/common/supervisor/riscv_sbi.c
##########
@@ -41,7 +47,11 @@
 
 void riscv_sbi_ack_timer(void)
 {
+#ifdef CONFIG_NUTTSBI
+  riscv_mcall_ack_timer();

Review comment:
       > I think I might be wrong on this. This detail was VERY well hidden in 
the RISC-V privileged spec. But according to this: 
[riscv-software-src/riscv-pk#184](https://github.com/riscv-software-src/riscv-pk/issues/184)
   > 
   
   Good finding.
   
   > mtime apparently is accessible to the supervisor, via CSR called "time" 
which apparently, implicitly equals mtime, or at least can be used as a seed 
for calculating values for sbi_set_timer(abs_time). This detail was not at all 
apparent to me and I won't believe it works until I have tried.
   >
   
   I think we can add a general macro to mode.h, so the same macro can be used 
with in S-mode and M-mode.
    
   > The reason I'm thoroughly confused, is because the mtime/mtimecmp 
registers are not really CSRs accessible with the csr instructions. They are 
memory mapped registers that reside in the CLINT memory space, and are accessed 
via the normal ld/sd instructions, which reference memory (not registers).
   > 
   
   From my previous experience with IC designer, the source of timer CSR may 
come from(connect to) the external timer.
   
   > An example of reading / writing their values is like this:
   > 
   > ```
   > static void c906_reload_mtimecmp(void)
   > {
   >   irqstate_t flags = spin_lock_irqsave(NULL);
   > 
   >   uint64_t current;
   >   uint64_t next;
   > 
   >   if (!_b_tick_started)
   >     {
   >       _b_tick_started = true;
   >       current = getreg64(C906_CLINT_MTIME);
   >     }
   >   else
   >     {
   >       current = getreg64(C906_CLINT_MTIMECMP);
   >     }
   > 
   >   uint64_t tick = TICK_COUNT;
   >   next = current + tick;
   > 
   >   putreg64(next, C906_CLINT_MTIMECMP);
   > 
   >   spin_unlock_irqrestore(NULL, flags);
   > }
   > ```
   > 
   > However, in the risc-v privileged ISA spec there indeed is a CSR called 
"time", which lives in the user ISA. If the value of time == value of mtime, 
then using riscv_mcall_ack_timer() becomes unnecessary and we can do it like 
the RISC-V SBI standard indicates.
   
   




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscr...@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


Reply via email to