Re: [PR] drivers/misc/optee: Cache coherency when MMU is disabled [nuttx]
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] drivers/misc/optee: Cache coherency when MMU is disabled [nuttx]
xiaoxiang781216 merged PR #16729: URL: https://github.com/apache/nuttx/pull/16729 -- 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]
Re: [PR] drivers/misc/optee: Cache coherency when MMU is disabled [nuttx]
gpoulios commented on code in PR #16729: URL: https://github.com/apache/nuttx/pull/16729#discussion_r2209402819 ## drivers/misc/optee.c: ## @@ -345,6 +346,10 @@ optee_shm_to_page_list(FAR struct optee_shm *shm, FAR uintptr_t *list_pa) *list_pa = optee_va_to_pa(list) | pgoff; } +#ifndef CONFIG_ARCH_USE_MMU Review Comment: It **_is_** a supported configuration, even though uncommon: 1) The document you cite states that in the case of MMU off (emphasis mine): > _Unexpected data cache hit behavior is **implementation defined**._ 2) Please have a look at [this thread](https://community.arm.com/support-forums/f/architectures-and-processors-forum/8908/a8-keeping-cache-enabled-and-mmu-disabled), and in particular the [selected answer](https://community.arm.com/support-forums/f/architectures-and-processors-forum/8908/a8-keeping-cache-enabled-and-mmu-disabled/29759) by an Arm employee at the bottom (again, emphasis mine): > _There is absolutely no point in running without the MMU enabled, **except if you are EXTREMELY resource-constrained and can't spare the memory to write the tables** (you only need 16KiB, though, for the bare essentials at 1MB and 16MB granularity)._ Well, in our case, we **_are_** extremely resource constrained (and we've found that in NuttX, the tables take up ~~slightly~~ significantly more than 16KiB, but even if so, we can't spare those 16KiB). and: > _Whether the caches are 'enabled' [...] or not, **every request goes through the memory system hierarchy in order a non-cacheable access passes through the L1 cache controller, which may pass it to the L2 cache controller, and so on**._ -- 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]
Re: [PR] drivers/misc/optee: Cache coherency when MMU is disabled [nuttx]
gpoulios commented on code in PR #16729: URL: https://github.com/apache/nuttx/pull/16729#discussion_r2209402819 ## drivers/misc/optee.c: ## @@ -345,6 +346,10 @@ optee_shm_to_page_list(FAR struct optee_shm *shm, FAR uintptr_t *list_pa) *list_pa = optee_va_to_pa(list) | pgoff; } +#ifndef CONFIG_ARCH_USE_MMU Review Comment: It **_is_** a supported configuration, even though uncommon: 1) The document you cite states that in the case of MMU off (emphasis mine): > _Unexpected data cache hit behavior is **implementation defined**._ 2) Please have a look at [this thread](https://community.arm.com/support-forums/f/architectures-and-processors-forum/8908/a8-keeping-cache-enabled-and-mmu-disabled#:~:text=If%20I%20enable%20the%20L1,Is%20this%20the%20expected%20behavior?), and in particular the [selected answer](https://community.arm.com/support-forums/f/architectures-and-processors-forum/8908/a8-keeping-cache-enabled-and-mmu-disabled/29759) by an Arm employee at the bottom (again, emphasis mine): > _There is absolutely no point in running without the MMU enabled, **except if you are EXTREMELY resource-constrained and can't spare the memory to write the tables** (you only need 16KiB, though, for the bare essentials at 1MB and 16MB granularity)._ Well, in our case, we **_are_** extremely resource constrained (and we've found that in NuttX, the tables take up ~~slightly~~ significantly more than 16KiB, but even if so, we can't spare those 16KiB). and: > _Whether the caches are 'enabled' [...] or not, **every request goes through the memory system hierarchy in order a non-cacheable access passes through the L1 cache controller, which may pass it to the L2 cache controller, and so on**._ -- 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]
Re: [PR] drivers/misc/optee: Cache coherency when MMU is disabled [nuttx]
gpoulios commented on code in PR #16729: URL: https://github.com/apache/nuttx/pull/16729#discussion_r2209402819 ## drivers/misc/optee.c: ## @@ -345,6 +346,10 @@ optee_shm_to_page_list(FAR struct optee_shm *shm, FAR uintptr_t *list_pa) *list_pa = optee_va_to_pa(list) | pgoff; } +#ifndef CONFIG_ARCH_USE_MMU Review Comment: It **_is_** a supported configuration, even though uncommon: 1) The document you cite states that in the case of MMU off (emphasis mine): > _Unexpected data cache hit behavior is **implementation defined**._ 2) Please have a look at this [thread](https://community.arm.com/support-forums/f/architectures-and-processors-forum/8908/a8-keeping-cache-enabled-and-mmu-disabled#:~:text=If%20I%20enable%20the%20L1,Is%20this%20the%20expected%20behavior?), and in particular the [selected answer](https://community.arm.com/support-forums/f/architectures-and-processors-forum/8908/a8-keeping-cache-enabled-and-mmu-disabled/29759) by an Arm employee at the bottom (again, emphasis mine): > _There is absolutely no point in running without the MMU enabled, **except if you are EXTREMELY resource-constrained and can't spare the memory to write the tables** (you only need 16KiB, though, for the bare essentials at 1MB and 16MB granularity)._ Well, in our case, we **_are_** extremely resource constrained (and we've found that in NuttX, the tables take up ~~slightly~~ significantly more than 16KiB, but even if so, we can't spare those 16KiB). and: > _Whether the caches are 'enabled' [...] or not, **every request goes through the memory system hierarchy in order a non-cacheable access passes through the L1 cache controller, which may pass it to the L2 cache controller, and so on**._ -- 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]
Re: [PR] drivers/misc/optee: Cache coherency when MMU is disabled [nuttx]
gpoulios commented on code in PR #16729: URL: https://github.com/apache/nuttx/pull/16729#discussion_r2209402819 ## drivers/misc/optee.c: ## @@ -345,6 +346,10 @@ optee_shm_to_page_list(FAR struct optee_shm *shm, FAR uintptr_t *list_pa) *list_pa = optee_va_to_pa(list) | pgoff; } +#ifndef CONFIG_ARCH_USE_MMU Review Comment: It **_is_** a supported configuration, even though uncommon: 1) The document you cite states that in the case of MMU off (emphasis mine): > _Unexpected data cache hit behavior is **implementation defined**._ 2) Please have a look at the selected answer by the Arm employee [here](https://community.arm.com/support-forums/f/architectures-and-processors-forum/8908/a8-keeping-cache-enabled-and-mmu-disabled#:~:text=If%20I%20enable%20the%20L1,Is%20this%20the%20expected%20behavior?) (again, emphasis mine): > _There is absolutely no point in running without the MMU enabled, **except if you are EXTREMELY resource-constrained and can't spare the memory to write the tables** (you only need 16KiB, though, for the bare essentials at 1MB and 16MB granularity)._ Well, in our case, we **_are_** extremely resource constrained (and we've found that in NuttX, the tables take up ~~slightly~~ significantly more than 16KiB, but even if so, we can't spare those 16KiB). and: > _Whether the caches are 'enabled' [...] or not, **every request goes through the memory system hierarchy in order a non-cacheable access passes through the L1 cache controller, which may pass it to the L2 cache controller, and so on**._ -- 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]
Re: [PR] drivers/misc/optee: Cache coherency when MMU is disabled [nuttx]
gpoulios commented on code in PR #16729: URL: https://github.com/apache/nuttx/pull/16729#discussion_r2209402819 ## drivers/misc/optee.c: ## @@ -345,6 +346,10 @@ optee_shm_to_page_list(FAR struct optee_shm *shm, FAR uintptr_t *list_pa) *list_pa = optee_va_to_pa(list) | pgoff; } +#ifndef CONFIG_ARCH_USE_MMU Review Comment: It **_is_** a supported configuration, even though uncommon: 1) The document you cite states that in the case of MMU off (emphasis mine): > _Unexpected data cache hit behavior is **implementation defined**._ 2) Please have a look at the selected answer by the Arm employee [here](https://community.arm.com/support-forums/f/architectures-and-processors-forum/8908/a8-keeping-cache-enabled-and-mmu-disabled#:~:text=If%20I%20enable%20the%20L1,Is%20this%20the%20expected%20behavior?) (again, emphasis mine): > _There is absolutely no point in running without the MMU enabled, **except if you are EXTREMELY resource-constrained and can't spare the memory to write the tables** (you only need 16KiB, though, for the bare essentials at 1MB and 16MB granularity)._ Well, in our case, we **_are_** extremely resource constrained (and we've found that in NuttX, the tables take up slightly more than 16KiB, but even if so, we can't spare those 16KiB). and: > _Whether the caches are 'enabled' [...] or not, **every request goes through the memory system hierarchy in order a non-cacheable access passes through the L1 cache controller, which may pass it to the L2 cache controller, and so on**._ -- 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]
Re: [PR] drivers/misc/optee: Cache coherency when MMU is disabled [nuttx]
gpoulios commented on code in PR #16729: URL: https://github.com/apache/nuttx/pull/16729#discussion_r2209402819 ## drivers/misc/optee.c: ## @@ -345,6 +346,10 @@ optee_shm_to_page_list(FAR struct optee_shm *shm, FAR uintptr_t *list_pa) *list_pa = optee_va_to_pa(list) | pgoff; } +#ifndef CONFIG_ARCH_USE_MMU Review Comment: It **_is_** a supported configuration, even though uncommon: 1) The document you cite states that in the case of MMU off (emphasis mine): > _Unexpected data cache hit behavior is **implementation defined**._ 2) Please have a look at the selected answer by the Arm employee [here](https://community.arm.com/support-forums/f/architectures-and-processors-forum/8908/a8-keeping-cache-enabled-and-mmu-disabled#:~:text=If%20I%20enable%20the%20L1,Is%20this%20the%20expected%20behavior?) (again, emphasis mine): > _There is absolutely no point in running without the MMU enabled, **except if you are EXTREMELY resource-constrained and can't spare the memory to write the tables** (you only need 16KiB, though, for the bare essentials at 1MB and 16MB granularity)._ Well, in our case, we **_are_** extremely resource constrained (and we've found that in NuttX, the tables take up slightly more than 16KiB, but even if so, we can't spare those 16KiB). -- 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]
Re: [PR] drivers/misc/optee: Cache coherency when MMU is disabled [nuttx]
xiaoxiang781216 commented on code in PR #16729: URL: https://github.com/apache/nuttx/pull/16729#discussion_r2209342978 ## drivers/misc/optee.c: ## @@ -345,6 +346,10 @@ optee_shm_to_page_list(FAR struct optee_shm *shm, FAR uintptr_t *list_pa) *list_pa = optee_va_to_pa(list) | pgoff; } +#ifndef CONFIG_ARCH_USE_MMU Review Comment: Ok, I understand your config: disable MMU, but enable cache. But it mayn't a support combination from: https://developer.arm.com/documentation/ddi0406/b/System-Level-Architecture/Virtual-Memory-System-Architecture--VMSA-/Memory-access-sequence/Enabling-and-disabling-the-MMU this statement is for armv7-a, but the similar limitation may exist on armv8-a too. -- 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]
Re: [PR] drivers/misc/optee: Cache coherency when MMU is disabled [nuttx]
gpoulios commented on code in PR #16729: URL: https://github.com/apache/nuttx/pull/16729#discussion_r2209184982 ## drivers/misc/optee.c: ## @@ -345,6 +346,10 @@ optee_shm_to_page_list(FAR struct optee_shm *shm, FAR uintptr_t *list_pa) *list_pa = optee_va_to_pa(list) | pgoff; } +#ifndef CONFIG_ARCH_USE_MMU Review Comment: > but MMU is still enable, otherwise the code doesn't need program MMU related registers. Sorry I don't follow. Cache maintenance code I added is guarded around "MMU not being enabled". > I never saw Cortex-A chip doesn't enable MMU in proudction [...] We're running a custom bootloader that has it disabled to reduce size. https://github.com/apache/nuttx/blob/a754b73e4c5faf50b30e9cf84de54d5bb99c3b45/arch/arm64/src/imx9/imx9_boot.c#L182-L186 > [...] since the cachable configuration come from MMU entry and disable cache forcely when MMU is off. You mean overall cache config (`SCTLR_C_BIT`) is forced off when MMU is off? That's not what I see in the code but it doesn't really matter for the purposes of this discussion. > The real difference is whether config MMU to flat virtual address space(flat build) or overlapped virtual address space(kernel build). No, I explained what my tests show, and what I think the root cause is (see my previous message). Difference is when the MMU is off vs on, **not** when I build flat vs kernel. > But either config is the pure software favorite, I am wondering why it impact the cache coherence between normal and secure world. Is my previous message not a satisfactory explanation? -- 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]
Re: [PR] drivers/misc/optee: Cache coherency when MMU is disabled [nuttx]
gpoulios commented on code in PR #16729: URL: https://github.com/apache/nuttx/pull/16729#discussion_r2209184982 ## drivers/misc/optee.c: ## @@ -345,6 +346,10 @@ optee_shm_to_page_list(FAR struct optee_shm *shm, FAR uintptr_t *list_pa) *list_pa = optee_va_to_pa(list) | pgoff; } +#ifndef CONFIG_ARCH_USE_MMU Review Comment: > but MMU is still enable, otherwise the code doesn't need program MMU related registers. Sorry I don't follow. Cache maintenance code I added is guarded around "MMU not being enabled". > I never saw Cortex-A chip doesn't enable MMU in proudction [...] We're running a custom bootloader that has it disabled to reduce size. https://github.com/apache/nuttx/blob/a754b73e4c5faf50b30e9cf84de54d5bb99c3b45/arch/arm64/src/imx9/imx9_boot.c#L182-L186 > [...] since the cachable configuration come from MMU entry and disable cache forcely when MMU is off. You mean overall cache config (`SCTLR_C_BIT`) is forced off when MMU is off? That's not what I see in the code but it doesn't really matter for the purposes of this discussion. > The real difference is whether config MMU to flat virtual address space(flat build) or overlapped virtual address space(kernel build). No, I explained what my tests show, and what I think the root cause is (see my previous message). Difference is when the MMU is off vs on, **not** when I build flat vs kernel. > But either config is the pure software favorite, I am wondering why it impact the cache coherence between normal and secure world. Is my previous message not a satisfactory explanation or did you just skip it? -- 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]
Re: [PR] drivers/misc/optee: Cache coherency when MMU is disabled [nuttx]
gpoulios commented on code in PR #16729: URL: https://github.com/apache/nuttx/pull/16729#discussion_r2209184982 ## drivers/misc/optee.c: ## @@ -345,6 +346,10 @@ optee_shm_to_page_list(FAR struct optee_shm *shm, FAR uintptr_t *list_pa) *list_pa = optee_va_to_pa(list) | pgoff; } +#ifndef CONFIG_ARCH_USE_MMU Review Comment: > but MMU is still enable, otherwise the code doesn't need program MMU related registers. Sorry I don't follow. Cache maintenance code I added is guarded around "MMU not being enabled". > I never saw Cortex-A chip doesn't enable MMU in proudction [...] We're running a custom bootloader that has it disabled to reduce size. https://github.com/apache/nuttx/blob/a754b73e4c5faf50b30e9cf84de54d5bb99c3b45/arch/arm64/src/imx9/imx9_boot.c#L182-L186 > [...] since the cachable configuration come from MMU entry and disable cache forcely when MMU is off. You mean overall cache config (`SCTLR_C_BIT`) is forced off when MMU is off? That's not what I see in the code but it doesn't really matter for the purposes of this discussion. > The real difference is whether config MMU to flat virtual address space(flat build) or overlapped virtual address space(kernel build). No, I explained what my tests show, and what I think the root cause is (see my previous message). Difference is when the MMU is off vs on, **not** when I build flat vs kernel. > But either config is the pure software favorite, I am wondering why it impact the cache coherence between normal and secure world. Is my previous message not a satisfactory explanation or did you just miss it? -- 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]
Re: [PR] drivers/misc/optee: Cache coherency when MMU is disabled [nuttx]
gpoulios commented on code in PR #16729: URL: https://github.com/apache/nuttx/pull/16729#discussion_r2209184982 ## drivers/misc/optee.c: ## @@ -345,6 +346,10 @@ optee_shm_to_page_list(FAR struct optee_shm *shm, FAR uintptr_t *list_pa) *list_pa = optee_va_to_pa(list) | pgoff; } +#ifndef CONFIG_ARCH_USE_MMU Review Comment: > but MMU is still enable, otherwise the code doesn't need program MMU related registers. Sorry I don't follow. Cache maintenance code I added is guarded around "MMU not being enabled". > I never saw Cortex-A chip doesn't enable MMU in proudction [...] We're running a custom bootloader that has it disabled to reduce size. > [...] since the cachable configuration come from MMU entry and disable cache forcely when MMU is off. You mean overall cache config (`SCTLR_C_BIT`) is forced off when MMU is off? That's not what I see in the code but it doesn't really matter for the purposes of this discussion. > The real difference is whether config MMU to flat virtual address space(flat build) or overlapped virtual address space(kernel build). No, I explained what my tests show, and what I think the root cause is (see my previous message). Difference is when the MMU is off vs on, **not** when I build flat vs kernel. > But either config is the pure software favorite, I am wondering why it impact the cache coherence between normal and secure world. Is my previous message not a satisfactory explanation or did you just miss it? -- 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]
Re: [PR] drivers/misc/optee: Cache coherency when MMU is disabled [nuttx]
xiaoxiang781216 commented on code in PR #16729: URL: https://github.com/apache/nuttx/pull/16729#discussion_r2209008333 ## drivers/misc/optee.c: ## @@ -345,6 +346,10 @@ optee_shm_to_page_list(FAR struct optee_shm *shm, FAR uintptr_t *list_pa) *list_pa = optee_va_to_pa(list) | pgoff; } +#ifndef CONFIG_ARCH_USE_MMU Review Comment: but MMU is still enable, otherwise the code doesn't need program MMU related registers. I never saw Cortex-A chip doesn't enable MMU in proudction since the cachable configuration come from MMU entry and disable cache forcely when MMU is off. The real difference is whether config MMU to flat virtual address space(flat build) or overlapped virtual address space(kernel build). But either config is the pure software favorite, I am wondering why it impact the cache coherence between normal and secure world. -- 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]
Re: [PR] drivers/misc/optee: Cache coherency when MMU is disabled [nuttx]
xiaoxiang781216 commented on code in PR #16729: URL: https://github.com/apache/nuttx/pull/16729#discussion_r2209008333 ## drivers/misc/optee.c: ## @@ -345,6 +346,10 @@ optee_shm_to_page_list(FAR struct optee_shm *shm, FAR uintptr_t *list_pa) *list_pa = optee_va_to_pa(list) | pgoff; } +#ifndef CONFIG_ARCH_USE_MMU Review Comment: but MMU is still enable, otherwise the code doesn't need program MMU related registers. I never saw Cortex-A chip doesn't enable MMU in proudction since the cachable configuration come from MMU entry. The real difference is whether config MMU to flat virtual address space(flat build) or overlapped virtual address space(kernel build). But either config is the pure software favorite, I am wondering why it impact the cache coherence between normal and secure world. -- 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]
Re: [PR] drivers/misc/optee: Cache coherency when MMU is disabled [nuttx]
gpoulios commented on code in PR #16729: URL: https://github.com/apache/nuttx/pull/16729#discussion_r2208318032 ## drivers/misc/optee.c: ## @@ -345,6 +346,10 @@ optee_shm_to_page_list(FAR struct optee_shm *shm, FAR uintptr_t *list_pa) *list_pa = optee_va_to_pa(list) | pgoff; } +#ifndef CONFIG_ARCH_USE_MMU Review Comment: If you’re asking why the MMU has this behaviour, I believe it is because the DRAM regions are typically configured as `MT_NORMAL` ([eg1](https://github.com/apache/nuttx/blob/master/arch/arm64/src/qemu/qemu_boot.c#L65), [eg2](https://github.com/apache/nuttx/blob/master/arch/arm64/src/common/arm64_mmu.c#L201), [eg3](https://github.com/apache/nuttx/blob/master/arch/arm64/src/imx9/imx9_boot.c#L65)) which translates in page tables [marked as inner-shareable](https://github.com/apache/nuttx/blob/3c8859b509f22c4e7811d45fb065137718d817fa/arch/arm64/src/common/arm64_mmu.c#L386). According to A-profile ref. manual, B2.10.1.1.1: > Each Inner Shareability domain contains a set of observers that are data coherent for each member of that set for data accesses with the Inner Shareable attribute made by any member of that set. -- 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]
Re: [PR] drivers/misc/optee: Cache coherency when MMU is disabled [nuttx]
gpoulios commented on code in PR #16729: URL: https://github.com/apache/nuttx/pull/16729#discussion_r2208318032 ## drivers/misc/optee.c: ## @@ -345,6 +346,10 @@ optee_shm_to_page_list(FAR struct optee_shm *shm, FAR uintptr_t *list_pa) *list_pa = optee_va_to_pa(list) | pgoff; } +#ifndef CONFIG_ARCH_USE_MMU Review Comment: If you’re asking why the MMU has this behaviour, I believe it is because the DRAM regions are typically configured as `MT_NORMAL` ([eg1](https://github.com/apache/nuttx/blob/master/arch/arm64/src/qemu/qemu_boot.c#L65), [eg2](https://github.com/apache/nuttx/blob/master/arch/arm64/src/common/arm64_mmu.c#L201), [eg3](https://github.com/apache/nuttx/blob/master/arch/arm64/src/imx9/imx9_boot.c#L65)) which translates in page tables [marked as inner-shareable](https://github.com/apache/nuttx/blob/def76c90d5648f64b2ef51175fb3327e2d8d9352/arch/arm64/src/imx9/imx9_boot.c#L65). According to A-profile ref. manual, B2.10.1.1.1: > Each Inner Shareability domain contains a set of observers that are data coherent for each member of that set for data accesses with the Inner Shareable attribute made by any member of that set. -- 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]
Re: [PR] drivers/misc/optee: Cache coherency when MMU is disabled [nuttx]
gpoulios commented on code in PR #16729: URL: https://github.com/apache/nuttx/pull/16729#discussion_r2208318032 ## drivers/misc/optee.c: ## @@ -345,6 +346,10 @@ optee_shm_to_page_list(FAR struct optee_shm *shm, FAR uintptr_t *list_pa) *list_pa = optee_va_to_pa(list) | pgoff; } +#ifndef CONFIG_ARCH_USE_MMU Review Comment: If your asking why the MMU has this behaviour, I believe it is because the DRAM regions are typically configured as `MT_NORMAL` ([eg1](https://github.com/apache/nuttx/blob/master/arch/arm64/src/qemu/qemu_boot.c#L65), [eg2](https://github.com/apache/nuttx/blob/master/arch/arm64/src/common/arm64_mmu.c#L201), [eg3](https://github.com/apache/nuttx/blob/master/arch/arm64/src/imx9/imx9_boot.c#L65)) which translates in page tables [marked as inner-shareable](https://github.com/apache/nuttx/blob/def76c90d5648f64b2ef51175fb3327e2d8d9352/arch/arm64/src/imx9/imx9_boot.c#L65). According to A-profile ref. manual, B2.10.1.1.1: > Each Inner Shareability domain contains a set of observers that are data coherent for each member of that set for data accesses with the Inner Shareable attribute made by any member of that set. -- 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]
Re: [PR] drivers/misc/optee: Cache coherency when MMU is disabled [nuttx]
gpoulios commented on code in PR #16729: URL: https://github.com/apache/nuttx/pull/16729#discussion_r2208133516 ## drivers/misc/optee.c: ## @@ -345,6 +346,10 @@ optee_shm_to_page_list(FAR struct optee_shm *shm, FAR uintptr_t *list_pa) *list_pa = optee_va_to_pa(list) | pgoff; } +#ifndef CONFIG_ARCH_USE_MMU Review Comment: I explain it in the description. With MMU, there is no need to sync. Without MMU, we have to sync. -- 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]
Re: [PR] drivers/misc/optee: Cache coherency when MMU is disabled [nuttx]
xiaoxiang781216 commented on code in PR #16729: URL: https://github.com/apache/nuttx/pull/16729#discussion_r2208099010 ## drivers/misc/optee.c: ## @@ -345,6 +346,10 @@ optee_shm_to_page_list(FAR struct optee_shm *shm, FAR uintptr_t *list_pa) *list_pa = optee_va_to_pa(list) | pgoff; } +#ifndef CONFIG_ARCH_USE_MMU Review Comment: but why check CONFIG_ARCH_USE_MMU? -- 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]
