pussuw commented on code in PR #6162:
URL: https://github.com/apache/incubator-nuttx/pull/6162#discussion_r859810549


##########
arch/risc-v/src/common/riscv_pmp.c:
##########
@@ -600,9 +600,14 @@ int riscv_config_pmp_region(uintptr_t region, uintptr_t 
attr,
 #   error "XLEN of risc-v not supported"
 # endif
 
-  /* fence is needed when page-based virtual memory is implemented */
+#ifdef CONFIG_ARCH_HAVE_S_MODE

Review Comment:
   Requirement for sfence.vma here is based on chip capabilities right ? Not 
utilization of address translations / MMU ? 
   
   **TL;DR I think this change is fine as-is.**
   
   A longer read:
   `CONFIG_ARCH_HAVE_S_MODE` was originally intended to indicate software 
capability, not chip capability. But maybe I could add ARCH_RV_ISA_S like the 
other chip capabilities are listed for other chips and do 
`CONFIG_ARCH_HAVE_S_MODE` via another config parameter which depends 
ARCH_RV_ISA_S. You don't have to wait for this I can do the refactoring.
   
   Volume II: RISC-V Privileged Architectures V1.10 table Table 3.2: Encoding 
of Extensions field in misa: This table conveniently lists how the capabilities 
are encoded.



-- 
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