OP-TEE mediator tracks cookie value for a last buffer which was requested by OP-TEE. This tracked value serves one important purpose: if OP-TEE wants to request another buffer, we know that it finished importing previous one and we can free page list associated with it.
Also, we had false premise that OP_TEE will free requested buffers in reversed order. So we checked rpc_data_cookie during handling OPTEE_RPC_CMD_SHM_FREE call and printed warning if cookie of buffer which is requested to be freed differs from last allocated one.
During testing RPMB FS services I discovered, that RPMB code frees request and response buffers in the same order is it allocated them. And this is perfectly fine, actually.
So, we are removing mentioned warning message in Xen, as it is perfectly normal to free buffers in arbitrary order.
Signed-off-by: Volodymyr Babchuk volodymyr_babchuk@epam.com --- xen/arch/arm/tee/optee.c | 20 +------------------- 1 file changed, 1 insertion(+), 19 deletions(-)
diff --git a/xen/arch/arm/tee/optee.c b/xen/arch/arm/tee/optee.c index 8a39fe33b0..ee85359742 100644 --- a/xen/arch/arm/tee/optee.c +++ b/xen/arch/arm/tee/optee.c @@ -1127,25 +1127,7 @@ static int handle_rpc_return(struct optee_domain *ctx, */ if ( shm_rpc->xen_arg->cmd == OPTEE_RPC_CMD_SHM_FREE ) { - uint64_t cookie = shm_rpc->xen_arg->params[0].u.value.b; - - free_optee_shm_buf(ctx, cookie); - - /* - * OP-TEE asks to free the buffer, but this is not the same - * buffer we previously allocated for it. While nothing - * prevents OP-TEE from asking this, it is the strange - * situation. This may or may not be caused by a bug in - * OP-TEE or mediator. But is better to print warning. - */ - if ( call->rpc_data_cookie && call->rpc_data_cookie != cookie ) - { - gprintk(XENLOG_ERR, - "Saved RPC cookie does not corresponds to OP-TEE's (%"PRIx64" != %"PRIx64")\n", - call->rpc_data_cookie, cookie); - - WARN(); - } + free_optee_shm_buf(ctx, shm_rpc->xen_arg->params[0].u.value.b); call->rpc_data_cookie = 0; } unmap_domain_page(shm_rpc->xen_arg);
On Mon, 5 Oct 2020, Volodymyr Babchuk wrote:
OP-TEE mediator tracks cookie value for a last buffer which was requested by OP-TEE. This tracked value serves one important purpose: if OP-TEE wants to request another buffer, we know that it finished importing previous one and we can free page list associated with it.
Also, we had false premise that OP_TEE will free requested buffers in reversed order. So we checked rpc_data_cookie during handling OPTEE_RPC_CMD_SHM_FREE call and printed warning if cookie of buffer which is requested to be freed differs from last allocated one.
During testing RPMB FS services I discovered, that RPMB code frees request and response buffers in the same order is it allocated them. And this is perfectly fine, actually.
So, we are removing mentioned warning message in Xen, as it is perfectly normal to free buffers in arbitrary order.
Signed-off-by: Volodymyr Babchuk volodymyr_babchuk@epam.com
Reviewed-by: Stefano Stabellini sstabellini@kernel.org
I am going to fix the grammar on commit
xen/arch/arm/tee/optee.c | 20 +------------------- 1 file changed, 1 insertion(+), 19 deletions(-)
diff --git a/xen/arch/arm/tee/optee.c b/xen/arch/arm/tee/optee.c index 8a39fe33b0..ee85359742 100644 --- a/xen/arch/arm/tee/optee.c +++ b/xen/arch/arm/tee/optee.c @@ -1127,25 +1127,7 @@ static int handle_rpc_return(struct optee_domain *ctx, */ if ( shm_rpc->xen_arg->cmd == OPTEE_RPC_CMD_SHM_FREE ) {
uint64_t cookie = shm_rpc->xen_arg->params[0].u.value.b;free_optee_shm_buf(ctx, cookie);/** OP-TEE asks to free the buffer, but this is not the same* buffer we previously allocated for it. While nothing* prevents OP-TEE from asking this, it is the strange* situation. This may or may not be caused by a bug in* OP-TEE or mediator. But is better to print warning.*/if ( call->rpc_data_cookie && call->rpc_data_cookie != cookie ){gprintk(XENLOG_ERR,"Saved RPC cookie does not corresponds to OP-TEE's (%"PRIx64" != %"PRIx64")\n",call->rpc_data_cookie, cookie);WARN();}
free_optee_shm_buf(ctx, shm_rpc->xen_arg->params[0].u.value.b); call->rpc_data_cookie = 0; } unmap_domain_page(shm_rpc->xen_arg);-- 2.27.0
-
Stefano Stabellini -
Volodymyr Babchuk