Hello Jens,
Here are some comments on patch V4.
*smccc_call32* Do we only target aarch32 optee_os?
*Buffer management* It looks that temporary buffers are now managed in the client API, not in the driver anymore. The positive point is that it makes the driver lighter. But the drawbacks is the number of extra ioctl used when using a temporary buffer: 1 to allocate, 1 to map, and 1 to unmap. So a TEEC_InvokeCommand could lead to 13 ioctls. What about the performances? Could not it be optimized in some way? Could we manage temporary buffers, dma buffers and "other" buffers in different ways?
*Kernel API* The GP API looks managed in the Client part now, without a 1-to-1 mapping in kernel side. That means that we have lost the Kernel API, which is useful when a given driver wants to use a TA. It looks we are really missing this Kernel API. This is the most critical point of our review.
*Non-zero copy* When using RegisterSharedMemory(), it looks we do not benefit from the fact that the original buffer was allocated using dma_buf. Hence we never have zero-copy in the current version when using RegisterSharedMemory as we will furher use the so-called shadowbuffer. Can you confirm this limitation can be removed in further versions of the patch?
By the way, we think xtest 5006, which calls TEEC_RegisterSharedBuffer() twice may lead to memory leaks (it could be the same with the current optee_linuxdriver).
*List of SHM* The list of the SHM allocated is now contained in the device, whereas it used to be in the context in optee_linuxdriver. This is not clear to us whether this is an issue or not.
*tee_shm_pool_alloc_res_mem()* If we'd like to use the generic function for a LX backend, we would need to add the following arguments to the function:
- the alignment of the private region, and the one of the dma region - a way to give the limits between both region (the size of the 1st region, or anything else).
Emmanuel and I would like to check few other points in the proposal. We'll come back to you if we have any other remarks.
Best regards, Jean Michel / Emmanuel / Etienne / Pascal