drivers: tee: optee: Use arch_page_phys_get for address conversion #83
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
oleksiimoisieiev
force-pushed
the
optee_z_mem_fix
branch
2 times, most recently
from
ec20348 to
a2781b4
Compare
lorc
reviewed
Feb 11, 2024
drivers/tee/optee/optee.c
Outdated
| static uintptr_t optee_mem_phys_addr(void *virt) | ||
| { | ||
| uintptr_t phys; | ||
| uint32_t offt = (uintptr_t)virt % CONFIG_MMU_PAGE_SIZE; |
There was a problem hiding this comment.
It's a pity that zephyr does not provide PAGE_MASK. I just hope that compiller will be smart enough to replace division with bitwise AND.
drivers/tee/optee/optee.c
Outdated
| unsigned long a4, unsigned long a5, unsigned long a6, unsigned long a7, | ||
| struct arm_smccc_res *res) | ||
| static void | ||
| optee_smccc_smc(unsigned long a0, unsigned long a1, unsigned long a2, unsigned long a3, |
drivers/tee/optee/optee.c
Outdated
| @@ -1113,7 +1146,7 @@ static int optee_suppl_send(const struct device *dev, unsigned int ret, unsigned | |||
| break; | |||
| case TEE_PARAM_ATTR_TYPE_MEMREF_OUTPUT: | |||
| case TEE_PARAM_ATTR_TYPE_MEMREF_INOUT: | |||
| LOG_WRN("Memref params are not fully tested"); | |||
| /* LOG_WRN("Memcpyref params are not fully tested"); */ | |||
oleksiimoisieiev
force-pushed
the
optee_z_mem_fix
branch
4 times, most recently
from
dc6441d to
dcf0c67
Compare
|
|
oleksiimoisieiev
force-pushed
the
optee_z_mem_fix
branch
2 times, most recently
from
8fdb182 to
26502a4
Compare
oleksiimoisieiev
changed the title
According to the description of z_mem_phys_addr it uses Z_MEM_VM_OFFSET to make a conversion from virtual to physical address. And these macros are intended for assembly, linker code, and static initializers. So when external libraries, such as PICOLIBC or NEWLIB_LIBC uses internal mappings for dynamic memory allocation z_mem_phys_addr may not work properly and return wrong physical address. We encountered this problem while testing the generation of a certificate using PKCS11 in libckteec with MBEDTLS. Mbedtls allocates buffer dynamically and ivokes tee command to request certificate from PKCS11 TA which should place result to the allocated buffer. The result is the MBEDTLS is used with minimal libc - then everything works fine. But when CONFIG_PICOLIBC or CONFIG_NEWLIB_LIBC is enabled - it returns empty buffer. This happens because z_mem_phys_addr returns incorrect physical address. Signed-off-by: Oleksii Moisieiev <[email protected]> Reviewed-by: Volodymyr Babchuk <[email protected]>
oleksiimoisieiev
force-pushed
the
optee_z_mem_fix
branch
from
26502a4 to
81cbffd
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
According to the description of z_mem_phys_addr it uses Z_MEM_VM_OFFSET to make a conversion from virtual to physical address. And these macros are intended for assembly, linker code, and static initializers.
So when external libraries, such as PICOLIBC or NEWLIB_LIBC uses internal mappings for dynamic memory allocation z_mem_phys_addr may not work properly and return wrong physical address.
We have met this problem when testing work with MBEDTLS to generate certificate using PKCS11 in libckteec.
Mbedtls allocates buffer dynamically and ivokes tee command to request certificate from PKCS11 TA which should place result to the allocated buffer.
The result is the MBEDTLS is used with minimal libc - then everything works fine. But when CONFIG_PICOLIBC or CONFIG_NEWLIB_LIBC is enabled