pussuw commented on a change in pull request #5758:
URL: https://github.com/apache/incubator-nuttx/pull/5758#discussion_r829100509



##########
File path: arch/risc-v/src/common/riscv_cpuindex.c
##########
@@ -48,10 +50,14 @@
  *
  ****************************************************************************/
 
-int up_cpu_index(void)
+uintptr_t riscv_cpuindex(void)
 {
-  int mhartid;
+  return READ_CSR(mhartid);
+}
 
-  asm volatile ("csrr %0, mhartid": "=r" (mhartid));
-  return mhartid;
+#ifdef CONFIG_SMP
+int up_cpu_index(void)
+{
+  return (int)riscv_cpuindex();

Review comment:
       mhartid is XLEN wide. IMO there is no need to strip or splice it.
   
   The reason for having a separate function for obtaining hartid is because 
this function will look like this eventually:
   
   ```
   uintptr_t riscv_cpuindex(void)
   {
   #ifdef CONFIG_ARCH_USE_S_MODE
     /* Kernel is in S-mode */
   
     return riscv_mcall_mhartid();
   
   #else
     /* Kernel is in M-mode */
   
     return READ_CSR(mhartid);
   #endif
   }
   ```
   So depending on which mode the kernel runs in the same method can be used to 
either directly read the hartid register, or obtain the value via ecall to 
machine mode.
   
   I personally don't mind the extra internal function, riscv_mhartid is 
provided by riscv_internal for internal use. The NuttX kernel needs 
up_cpu_index which just becomes a tailcall to riscv_mhartid. Also the SMP 
dependency is avoided **internally** in RISC-V now. We need the hart's id 
regardless of SMP due to external interrupt handling among other things.




-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to