gpoulios commented on PR #16729:
URL: https://github.com/apache/nuttx/pull/16729#issuecomment-3077365957

   @xiaoxiang781216 you merged it right before I was about to send this:
   
   The problem I see now is that I overlooked that most arm defconfigs do not 
explicitly enable `CONFIG_ARCH_USE_MMU`, and it defaults to `n`. The only 
configs that enable it are:
    - `avaota-a1:nsh`
    - `qemu-armv8a:knsh`
    - `imx93-evk:knsh`
    - `imx93-evk:koptee`
   
   Then most boards (even some of the above) just go on and initialize the MMU 
without guarding on any kconfig 
([eg](https://github.com/apache/nuttx/blob/4a765b557ffc0f7b6471483b266869016161be2f/arch/arm64/src/qemu/qemu_boot.c#L162)).
 And that is even though the 
[pgalloc()](https://github.com/apache/nuttx/blob/4a765b557ffc0f7b6471483b266869016161be2f/include/nuttx/arch.h#L781)
 and 
[sbrk()](https://github.com/apache/nuttx/blob/4a765b557ffc0f7b6471483b266869016161be2f/include/unistd.h#L375)
 still depend on `CONFIG_ARCH_USE_MMU` being set!? What a mess.
   
   So for OP-TEE driver, that means users of the OP-TEE driver on other configs 
will enter the cache maintenance blocks even though they don't need to.
   
   Ideally, we would want:
   1. all arm configs to explicitly set `CONFIG_ARCH_USE_MMU=y`
   2. wrap all calls to `arm{64}_init_mmu(true);` around `CONFIG_ARCH_USE_MMU=y`
   3. fix 
[this](https://github.com/apache/nuttx/blob/4a765b557ffc0f7b6471483b266869016161be2f/arch/arm64/src/common/arm64_cpustart.c#L224-L226)
 to wrap around `CONFIG_ARCH_USE_MMU` instead of `CONFIG_ARCH_HAVE_MMU`.
    
   That might be quite invasive though. But still, current use of  
`CONFIG_ARCH_HAVE_MMU` vs `CONFIG_ARCH_USE_MMU` in nuttx arm(64) is wrong, even 
semantically wrong (`HAVE` is always assumed to be `USE=y`, and `USE` is not 
used almost at all). And this is inconsistent with the use of `USE_MMU` in 
other architectures 
([x86-64](https://github.com/apache/nuttx/blob/4a765b557ffc0f7b6471483b266869016161be2f/arch/x86_64/src/common/CMakeLists.txt#L57),
 
[risc-v](https://github.com/apache/nuttx/blob/4a765b557ffc0f7b6471483b266869016161be2f/arch/risc-v/src/common/CMakeLists.txt#L119)).
   
   If you think it's a good idea to fix how `CONFIG_ARCH_USE_MMU` is used in 
arm(64) I could give it a go.


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