tee: optee: making OPTEE_SHM_NUM_PRIV_PAGES configurable via Kconfig #62
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
Fixes the static checker warning in optee_release(). error: uninitialized symbol 'parg'. Reported-by: Dan Carpenter <[email protected]> Signed-off-by: Jens Wiklander <[email protected]>
tee_drv.h references struct device, but does not include device.h nor platform_device.h. Therefore, if tee_drv.h is included by some file that does not pull device.h nor platform_device.h beforehand, we have a compile warning. Fix this by adding a forward declaration. Signed-off-by: Jerome Forissier <[email protected]> Signed-off-by: Jens Wiklander <[email protected]>
dma_buf_ops are not supposed to change at runtime. All functions working with dma_buf_ops provided by <linux/dma-buf.h> work with const dma_buf_ops. So mark the non-const structs as const. File size before: text data bss dec hex filename 2026 112 0 2138 85a drivers/tee/tee_shm.o File size After adding 'const': text data bss dec hex filename 2138 0 0 2138 85a drivers/tee/tee_shm.o Signed-off-by: Arvind Yadav <[email protected]> Signed-off-by: Jens Wiklander <[email protected]>
Add const to tee_desc structures as they are only passed as an argument to the function tee_device_alloc. This argument is of type const, so declare these structures as const too. Add const to tee_driver_ops structures as they are only stored in the ops field of a tee_desc structure. This field is of type const, so declare these structure types as const. Signed-off-by: Bhumika Goyal <[email protected]> Signed-off-by: Jens Wiklander <[email protected]>
Prior to this patch RPC sleep was uninterruptible since msleep() is uninterruptible. Change to use msleep_interruptible() instead. Signed-off-by: Tiger Yu <[email protected]> Reviewed-by: Joakim Bech <[email protected]> Signed-off-by: Jerome Forissier <[email protected]> Signed-off-by: Jens Wiklander <[email protected]>
Mirrors the TEE_DESC_PRIVILEGED bit of struct tee_desc:flags into struct tee_ioctl_version_data:gen_caps as TEE_GEN_CAP_PRIVILEGED in tee_ioctl_version() Reviewed-by: Jerome Forissier <[email protected]> Signed-off-by: Jens Wiklander <[email protected]>
In the latest changes of optee_os, the interrupts' names are changed to "native" and "foreign" interrupts. Signed-off-by: David Wang <[email protected]> Signed-off-by: Jerome Forissier <[email protected]> Signed-off-by: Jens Wiklander <[email protected]>
The first node supplied to of_find_matching_node() has its reference counter decreased as part of call to that function. In optee_driver_init() after calling of_find_matching_node() it's invalid to call of_node_put() on the supplied node again. So remove the invalid call to of_node_put(). Reported-by: Alex Shi <[email protected]> Signed-off-by: Jens Wiklander <[email protected]>
Reviewed-by: Etienne Carriere <[email protected]> Signed-off-by: Jens Wiklander <[email protected]>
Adds TEE_IOCTL_PARAM_ATTR_META which can be used to indicate meta parameters when communicating with user space. These meta parameters can be used by supplicant support multiple parallel requests at a time. Reviewed-by: Etienne Carriere <[email protected]> Signed-off-by: Jens Wiklander <[email protected]>
Adds support for asynchronous supplicant requests, meaning that the supplicant can process several requests in parallel or block in a request for some time. Acked-by: Etienne Carriere <[email protected]> Tested-by: Etienne Carriere <[email protected]> (b2260 pager=y/n) Signed-off-by: Jens Wiklander <[email protected]>
Makes creation of shm pools more flexible by adding new more primitive functions to allocate a shm pool. This makes it easier to add driver specific shm pool management. Signed-off-by: Jens Wiklander <[email protected]> Signed-off-by: Volodymyr Babchuk <[email protected]>
Added new ioctl to allow users register own buffers as a shared memory. Signed-off-by: Volodymyr Babchuk <[email protected]> [jw: moved tee_shm_is_registered() declaration] [jw: added space after __tee_shm_alloc() implementation] Signed-off-by: Jens Wiklander <[email protected]>
These two function will be needed for shared memory registration in OP-TEE Signed-off-by: Volodymyr Babchuk <[email protected]> Signed-off-by: Jens Wiklander <[email protected]>
In order to register a shared buffer in TEE, we need accessor function that return list of pages for that buffer. Signed-off-by: Volodymyr Babchuk <[email protected]> Signed-off-by: Jens Wiklander <[email protected]>
There were changes in REE<->OP-TEE ABI recently. Now ABI allows us to pass non-contiguous memory buffers as list of pages to OP-TEE. This can be achieved by using new parameter attribute OPTEE_MSG_ATTR_NONCONTIG. OP-TEE also is able to use all non-secure RAM for shared buffers. This new capability is enabled with OPTEE_SMC_SEC_CAP_DYNAMIC_SHM flag. This patch adds necessary definitions to the protocol definition files at Linux side. Signed-off-by: Volodymyr Babchuk <[email protected]> Signed-off-by: Jens Wiklander <[email protected]>
These functions will be used to pass information about shared buffers to OP-TEE. ABI between Linux and OP-TEE is defined in optee_msg.h and optee_smc.h. optee_msg.h defines OPTEE_MSG_ATTR_NONCONTIG attribute for shared memory references and describes how such references should be passed. Note that it uses 64-bit page addresses even on 32 bit systems. This is done to support LPAE and to unify interface. Signed-off-by: Volodymyr Babchuk <[email protected]> [jw: replacing uint64_t with u64 in optee_fill_pages_list()] Signed-off-by: Jens Wiklander <[email protected]>
This change adds ops for shm_(un)register functions in tee interface. Client application can use these functions to (un)register an own shared buffer in OP-TEE address space. This allows zero copy data sharing between Normal and Secure Worlds. Please note that while those functions were added to optee code, it does not report to userspace that those functions are available. OP-TEE code does not set TEE_GEN_CAP_REG_MEM flag. This flag will be enabled only after all other features of dynamic shared memory will be implemented in subsequent patches. Of course user can ignore presence of TEE_GEN_CAP_REG_MEM flag and try do call those functions. This is okay, driver will register shared buffer in OP-TEE, but any attempts to use this shared buffer will fail. Signed-off-by: Volodymyr Babchuk <[email protected]> Signed-off-by: Jens Wiklander <[email protected]>
Now, when client applications can register own shared buffers in OP-TEE, we need to extend ABI for parameter passing to/from OP-TEE. So, if OP-TEE core detects that parameter belongs to registered shared memory, it will use corresponding parameter attribute. Signed-off-by: Volodymyr Babchuk <[email protected]> Signed-off-by: Jens Wiklander <[email protected]>
With latest changes to OP-TEE we can use any buffers as a shared memory. Thus, it is possible for supplicant to provide part of own memory when OP-TEE asks to allocate a shared buffer. This patch adds support for such feature into RPC handling code. Now when OP-TEE asks supplicant to allocate shared buffer, supplicant can use TEE_IOC_SHM_REGISTER to provide such buffer. RPC handler is aware of this, so it will pass list of allocated pages to OP-TEE. Signed-off-by: Volodymyr Babchuk <[email protected]> [jw: fix parenthesis alignment in free_pages_list()] Signed-off-by: Jens Wiklander <[email protected]>
Those capabilities will be used in subsequent patches. Signed-off-by: Volodymyr Babchuk <[email protected]> Signed-off-by: Jens Wiklander <[email protected]>
This is simple pool that uses kernel page allocator. This pool can be used in case OP-TEE supports dynamic shared memory. Signed-off-by: Volodymyr Babchuk <[email protected]> Signed-off-by: Jens Wiklander <[email protected]>
Previous patches added various features that are needed for dynamic SHM. Dynamic SHM allows Normal World to share any buffers with OP-TEE. While original design suggested to use pre-allocated region (usually of 1M to 2M of size), this new approach allows to use all non-secure RAM for command buffers, RPC allocations and TA parameters. This patch checks capability OPTEE_SMC_SEC_CAP_DYNAMIC_SHM. If it was set by OP-TEE, then kernel part of OP-TEE will use kernel page allocator to allocate command buffers. Also it will set TEE_GEN_CAP_REG_MEM capability to tell userspace that it supports shared memory registration. Signed-off-by: Volodymyr Babchuk <[email protected]> Signed-off-by: Jens Wiklander <[email protected]>
We need to ensure that tee_context is present until last shared buffer will be freed. Signed-off-by: Volodymyr Babchuk <[email protected]> Signed-off-by: Jens Wiklander <[email protected]>
Now, when struct tee_shm is defined in public header, we can inline small getter functions like this one. Signed-off-by: Volodymyr Babchuk <[email protected]> Signed-off-by: Jens Wiklander <[email protected]>
The optee driver includes the header files in an unusual order, with asm/pgtable.h before the linux/*.h headers. For some reason this seems to trigger a build failure: drivers/tee/optee/call.c: In function 'optee_fill_pages_list': include/asm-generic/memory_model.h:64:14: error: implicit declaration of function 'page_to_section'; did you mean '__nr_to_section'? [-Werror=implicit-function-declaration] int __sec = page_to_section(__pg); \ drivers/tee/optee/call.c:494:15: note: in expansion of macro 'page_to_phys' optee_page = page_to_phys(*pages) + Let's just include linux/mm.h, which will then get the other header implicitly. Fixes: 3bb48ba ("tee: optee: add page list manipulation functions") Signed-off-by: Arnd Bergmann <[email protected]>
Adds a start argument to the shm_register callback to allow the callback to check memory type of the passed pages. Signed-off-by: Jens Wiklander <[email protected]>
Checks the memory type of the pages to be registered as shared memory. Only normal cached memory is allowed. Signed-off-by: Jens Wiklander <[email protected]>
The function __tee_shm_alloc is local to the source and does not need to be in global scope, so make it static. Cleans up sparse warning: symbol '__tee_shm_alloc' was not declared. Should it be static? Signed-off-by: Colin Ian King <[email protected]> Signed-off-by: Jens Wiklander <[email protected]>
In the case that shm->pages fails to allocate, the current exit
error path will try to put_page on a null shm->pages and cause
a null pointer dereference when accessing shm->pages[n]. Fix this
by only performing the put_page and kfree on shm->pages if it
is not null.
Detected by CoverityScan, CID#1463283 ("Dereference after null check")
Fixes: 033ddf1 ("tee: add register user memory")
Signed-off-by: Colin Ian King <[email protected]>
Signed-off-by: Jens Wiklander <[email protected]>
The privileged dev id range is [TEE_NUM_DEVICES / 2, TEE_NUM_DEVICES). The non-privileged dev id range is [0, TEE_NUM_DEVICES / 2). So when finding a slot for them, need to use different max value. Signed-off-by: Peng Fan <[email protected]> Signed-off-by: Jens Wiklander <[email protected]>
In the OPTEE_SMC_CALL_GET_OS_REVISION request, the previously reserved parameter a2 is now documented as being an optional build identifier (such as an SCM revision or commit ID, for instance). A new structure optee_smc_call_get_os_revision_result is introduced to be used when querying the secure OS version, instead of re-using the struct defined for OPTEE_SMC_CALLS_REVISION. Signed-off-by: Jerome Forissier <[email protected]> Reviewed-by: Matthias Brugger <[email protected]> Signed-off-by: Jens Wiklander <[email protected]>
When the driver initializes, report the following information about the OP-TEE OS: - major and minor version, - build identifier (if available). Signed-off-by: Jerome Forissier <[email protected]> Reviewed-by: Matthias Brugger <[email protected]> Signed-off-by: Jens Wiklander <[email protected]>
Suggested-by: Jerome Forissier <[email protected]> Signed-off-by: Victor Chong <[email protected]> Reviewed-by: Jerome Forissier <[email protected]>
Successful registration of a memory reference in the scope of a TEE content must increase the context refcount. This change adds this missing refcount increase. The context refcount is already decremented when such shm reference is freed by its owner, in tee_shm_release(), hence current unbalance refcount before this path is applied. Fixes: 9f9806e ("tee: new ioctl to a register tee_shm from a dmabuf file descriptor") Signed-off-by: Etienne Carriere <[email protected]> Tested-by: Etienne Carriere <[email protected]> (Qemu armv7/v8) Acked-by: Jens Wiklander <[email protected]>
This change prevents userland from referencing TEE shared memory outside the area initially allocated by its owner. Prior this change an application could not reference or access memory it did not own but it could reference memory not explicitly allocated by owner but still allocated to the owner due to the memory allocation granule. Reported-by: Alexandre Jutras <[email protected]> Signed-off-by: Etienne Carriere <[email protected]> Reviewed-by: Jens Wiklander <[email protected]>
jforissier
reviewed
May 31, 2018
drivers/tee/optee/Kconfig
Outdated
| depends on OPTEE | ||
| help | ||
| This sets the number of private shared memory pages to be | ||
| used by OP-TEE Trusted Execute (TEE) driver. |
There was a problem hiding this comment.
"by the OP-TEE driver".
What are private memory pages used for? How should the value be selected? When does it need to be increased? By how much? I think all that has to be documented here, no?
There was a problem hiding this comment.
s/Trusted Execute (TEE)/Trusted Execution Environment (TEE)/
But I think I would just have written
... by the OP-TEE TEE driver.
There was a problem hiding this comment.
I have written following description for --help-- section.
Please check if it is right or something needs to be corrected.
Thanks.
This sets the number of private shared memory pages to be
used by OP-TEE TEE driver.This shared memory is used for communication between Secure World
& Non Secure World (Structure optee_msg_arg allocated for passing
arguments b/w Secure and Non-Secure World).For more information please check Shared Memory section in
OPTEE Design Documentation.Some use cases needs this value to be increased.
e.g. In a system where multiple applications are running
simultaneously trying to get services from TA.This value needs to be carefully changed based on platform and
use case.
There was a problem hiding this comment.
Please check this @jbech-linaro @jforissier
Thanks.
There was a problem hiding this comment.
Please update to as jbech-linaro "would just have written"
This change adds KCONFIG option to set number of pages out of whole shared memory to be used for OP-TEE driver private data structures. Signed-off-by: Sahil Malhotra <[email protected]>
|
Pull request updated. Please check. |
| depends on OPTEE | ||
| help | ||
| This sets the number of private shared memory pages to be | ||
| used by OP-TEE TEE driver. |
There was a problem hiding this comment.
s/OP-TEE TEE driver/OP-TEE driver/
There was a problem hiding this comment.
But jbech-linaro "would just have written"
... by the OP-TEE TEE driver.
There was a problem hiding this comment.
Sorry missed that. I guess it's good as it is then.
There was a problem hiding this comment.
I will post it on LKML now.
Thanks all for reviewing.
|
Posted on LKML |
jforissier
force-pushed
the
optee
branch
3 times, most recently
from
221a1ac
to
0fd2deb
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.
This change adds KCONFIG option to set number of pages out of
whole shared memory to be used for OP-TEE driver private data
structures.
Signed-off-by: Sahil Malhotra [email protected]