Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RFC] Another attempt at Provision TEE threads for system invocation #110

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions drivers/firmware/arm_scmi/optee.c
Original file line number Diff line number Diff line change
Expand Up @@ -440,6 +440,10 @@ static int scmi_optee_chan_setup(struct scmi_chan_info *cinfo, struct device *de
if (ret)
goto err_free_shm;

ret = tee_client_system_session(scmi_optee_private->tee_ctx, channel->tee_session);
if (ret)
dev_warn(dev, "Could not switch to system session, do best effort\n");

ret = get_channel(channel);
if (ret)
goto err_close_sess;
Expand Down
150 changes: 135 additions & 15 deletions drivers/tee/optee/call.c
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,26 @@ struct optee_shm_arg_entry {
DECLARE_BITMAP(map, MAX_ARG_COUNT_PER_ENTRY);
};

void optee_cq_init(struct optee_call_queue *cq, int thread_count)
{
mutex_init(&cq->mutex);
INIT_LIST_HEAD(&cq->normal_waiters);
INIT_LIST_HEAD(&cq->sys_waiters);
/*
* If cq->total_thread_count is 0 then we're not trying to keep
* track of how many free threads we have, instead we're relying on
* the secure world to tell us when we're out of thread and have to
* wait for another thread to become available.
*/
cq->total_thread_count = thread_count;
cq->free_normal_thread_count = thread_count;
}

void optee_cq_wait_init(struct optee_call_queue *cq,
struct optee_call_waiter *w)
struct optee_call_waiter *w, bool sys_thread)
{
bool need_wait = false;

/*
* We're preparing to make a call to secure world. In case we can't
* allocate a thread in secure world we'll end up waiting in
Expand All @@ -53,15 +70,40 @@ void optee_cq_wait_init(struct optee_call_queue *cq,
mutex_lock(&cq->mutex);

/*
* We add ourselves to the queue, but we don't wait. This
* guarantees that we don't lose a completion if secure world
* returns busy and another thread just exited and try to complete
* someone.
* We add ourselves to a queue, but we don't wait. This guarantees
* that we don't lose a completion if secure world returns busy and
* another thread just exited and try to complete someone.
*/
init_completion(&w->c);
list_add_tail(&w->list_node, &cq->waiters);
w->sys_thread = sys_thread;
if (sys_thread) {
list_add_tail(&w->list_node, &cq->sys_waiters);
} else {
list_add_tail(&w->list_node, &cq->normal_waiters);
if (cq->total_thread_count) {
/*
* Claim a normal thread if one is available, else
* we'll need to wait for a normal thread to be
* released.
*/
if (cq->free_normal_thread_count > 0)
cq->free_normal_thread_count--;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

some thread protection around this incrementation?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're holding cq->mutex.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

right, sorry.

else
need_wait = true;
}
}

mutex_unlock(&cq->mutex);

while (need_wait) {
optee_cq_wait_for_completion(cq, w);
mutex_lock(&cq->mutex);
if (cq->free_normal_thread_count > 0) {
cq->free_normal_thread_count--;
need_wait = false;
}
mutex_unlock(&cq->mutex);
}
}

void optee_cq_wait_for_completion(struct optee_call_queue *cq,
Expand All @@ -74,7 +116,10 @@ void optee_cq_wait_for_completion(struct optee_call_queue *cq,
/* Move to end of list to get out of the way for other waiters */
list_del(&w->list_node);
reinit_completion(&w->c);
list_add_tail(&w->list_node, &cq->waiters);
if (w->sys_thread)
list_add_tail(&w->list_node, &cq->sys_waiters);
else
list_add_tail(&w->list_node, &cq->normal_waiters);

mutex_unlock(&cq->mutex);
}
Expand All @@ -83,10 +128,19 @@ static void optee_cq_complete_one(struct optee_call_queue *cq)
{
struct optee_call_waiter *w;

list_for_each_entry(w, &cq->waiters, list_node) {
list_for_each_entry(w, &cq->sys_waiters, list_node) {
if (!completion_done(&w->c)) {
complete(&w->c);
break;
return;
}
}

if (!cq->total_thread_count || cq->free_normal_thread_count > 0) {
list_for_each_entry(w, &cq->normal_waiters, list_node) {
if (!completion_done(&w->c)) {
complete(&w->c);
break;
}
}
}
}
Expand All @@ -104,6 +158,9 @@ void optee_cq_wait_final(struct optee_call_queue *cq,
/* Get out of the list */
list_del(&w->list_node);

if (!w->sys_thread)
cq->free_normal_thread_count++; /* Release a normal thread */

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mutex?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're holding cq->mutex.


/* Wake up one eventual waiting task */
optee_cq_complete_one(cq);

Expand All @@ -119,6 +176,36 @@ void optee_cq_wait_final(struct optee_call_queue *cq,
mutex_unlock(&cq->mutex);
}

bool optee_cq_inc_sys_thread_count(struct optee_call_queue *cq)
{
bool rc = false;

mutex_lock(&cq->mutex);

/* Leave at least 1 normal (non-system) thread */
if (cq->res_sys_thread_count + 1 < cq->total_thread_count) {
cq->free_normal_thread_count--;
cq->res_sys_thread_count++;
rc = true;
}

mutex_unlock(&cq->mutex);

return rc;
}

void optee_cq_dec_sys_thread_count(struct optee_call_queue *cq)
{
mutex_lock(&cq->mutex);
if (cq->res_sys_thread_count > 0) {
cq->res_sys_thread_count--;
cq->free_normal_thread_count++;
/* If there's someone waiting, let it resume */
optee_cq_complete_one(cq);
}
mutex_unlock(&cq->mutex);
}

/* Requires the filpstate mutex to be held */
static struct optee_session *find_session(struct optee_context_data *ctxdata,
u32 session_id)
Expand Down Expand Up @@ -328,7 +415,8 @@ int optee_open_session(struct tee_context *ctx,
goto out;
}

if (optee->ops->do_call_with_arg(ctx, shm, offs)) {
if (optee->ops->do_call_with_arg(ctx, shm, offs,
sess->use_sys_thread)) {
msg_arg->ret = TEEC_ERROR_COMMUNICATION;
msg_arg->ret_origin = TEEC_ORIGIN_COMMS;
}
Expand Down Expand Up @@ -360,7 +448,29 @@ int optee_open_session(struct tee_context *ctx,
return rc;
}

int optee_close_session_helper(struct tee_context *ctx, u32 session)
int optee_system_session(struct tee_context *ctx, u32 session)
{
struct optee_context_data *ctxdata = ctx->data;
struct optee *optee = tee_get_drvdata(ctx->teedev);
struct optee_session *sess;
int rc = -EINVAL;

mutex_lock(&ctxdata->mutex);

sess = find_session(ctxdata, session);
if (sess && !sess->use_sys_thread &&
optee_cq_inc_sys_thread_count(&optee->call_queue)) {
rc = 0;
sess->use_sys_thread = true;
}

mutex_unlock(&ctxdata->mutex);

return rc;
}

int optee_close_session_helper(struct tee_context *ctx, u32 session,
bool system_thread)
{
struct optee *optee = tee_get_drvdata(ctx->teedev);
struct optee_shm_arg_entry *entry;
Expand All @@ -374,8 +484,10 @@ int optee_close_session_helper(struct tee_context *ctx, u32 session)

msg_arg->cmd = OPTEE_MSG_CMD_CLOSE_SESSION;
msg_arg->session = session;
optee->ops->do_call_with_arg(ctx, shm, offs);
optee->ops->do_call_with_arg(ctx, shm, offs, system_thread);

if (system_thread)
optee_cq_dec_sys_thread_count(&optee->call_queue);
optee_free_msg_arg(ctx, entry, offs);

return 0;
Expand All @@ -385,6 +497,7 @@ int optee_close_session(struct tee_context *ctx, u32 session)
{
struct optee_context_data *ctxdata = ctx->data;
struct optee_session *sess;
bool system_thread;

/* Check that the session is valid and remove it from the list */
mutex_lock(&ctxdata->mutex);
Expand All @@ -394,9 +507,10 @@ int optee_close_session(struct tee_context *ctx, u32 session)
mutex_unlock(&ctxdata->mutex);
if (!sess)
return -EINVAL;
system_thread = sess->use_sys_thread;
kfree(sess);

return optee_close_session_helper(ctx, session);
return optee_close_session_helper(ctx, session, system_thread);
}

int optee_invoke_func(struct tee_context *ctx, struct tee_ioctl_invoke_arg *arg,
Expand All @@ -408,12 +522,15 @@ int optee_invoke_func(struct tee_context *ctx, struct tee_ioctl_invoke_arg *arg,
struct optee_msg_arg *msg_arg;
struct optee_session *sess;
struct tee_shm *shm;
bool system_thread;
u_int offs;
int rc;

/* Check that the session is valid */
mutex_lock(&ctxdata->mutex);
sess = find_session(ctxdata, arg->session);
if (sess)
system_thread = sess->use_sys_thread;
mutex_unlock(&ctxdata->mutex);
if (!sess)
return -EINVAL;
Expand All @@ -432,7 +549,7 @@ int optee_invoke_func(struct tee_context *ctx, struct tee_ioctl_invoke_arg *arg,
if (rc)
goto out;

if (optee->ops->do_call_with_arg(ctx, shm, offs)) {
if (optee->ops->do_call_with_arg(ctx, shm, offs, system_thread)) {
msg_arg->ret = TEEC_ERROR_COMMUNICATION;
msg_arg->ret_origin = TEEC_ORIGIN_COMMS;
}
Expand All @@ -457,12 +574,15 @@ int optee_cancel_req(struct tee_context *ctx, u32 cancel_id, u32 session)
struct optee_shm_arg_entry *entry;
struct optee_msg_arg *msg_arg;
struct optee_session *sess;
bool system_thread;
struct tee_shm *shm;
u_int offs;

/* Check that the session is valid */
mutex_lock(&ctxdata->mutex);
sess = find_session(ctxdata, session);
if (sess)
system_thread = sess->use_sys_thread;
mutex_unlock(&ctxdata->mutex);
if (!sess)
return -EINVAL;
Expand All @@ -474,7 +594,7 @@ int optee_cancel_req(struct tee_context *ctx, u32 cancel_id, u32 session)
msg_arg->cmd = OPTEE_MSG_CMD_CANCEL;
msg_arg->session = session;
msg_arg->cancel_id = cancel_id;
optee->ops->do_call_with_arg(ctx, shm, offs);
optee->ops->do_call_with_arg(ctx, shm, offs, system_thread);

optee_free_msg_arg(ctx, entry, offs);
return 0;
Expand Down
5 changes: 3 additions & 2 deletions drivers/tee/optee/core.c
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,8 @@ int optee_open(struct tee_context *ctx, bool cap_memref_null)

static void optee_release_helper(struct tee_context *ctx,
int (*close_session)(struct tee_context *ctx,
u32 session))
u32 session,
bool system_thread))
{
struct optee_context_data *ctxdata = ctx->data;
struct optee_session *sess;
Expand All @@ -141,7 +142,7 @@ static void optee_release_helper(struct tee_context *ctx,
list_for_each_entry_safe(sess, sess_tmp, &ctxdata->sess_list,
list_node) {
list_del(&sess->list_node);
close_session(ctx, sess->session_id);
close_session(ctx, sess->session_id, sess->use_sys_thread);
kfree(sess);
}
kfree(ctxdata);
Expand Down
13 changes: 7 additions & 6 deletions drivers/tee/optee/ffa_abi.c
Original file line number Diff line number Diff line change
Expand Up @@ -528,7 +528,8 @@ static void optee_handle_ffa_rpc(struct tee_context *ctx, struct optee *optee,

static int optee_ffa_yielding_call(struct tee_context *ctx,
struct ffa_send_direct_data *data,
struct optee_msg_arg *rpc_arg)
struct optee_msg_arg *rpc_arg,
bool system_thread)
{
struct optee *optee = tee_get_drvdata(ctx->teedev);
struct ffa_device *ffa_dev = optee->ffa.ffa_dev;
Expand All @@ -541,7 +542,7 @@ static int optee_ffa_yielding_call(struct tee_context *ctx,
int rc;

/* Initialize waiter */
optee_cq_wait_init(&optee->call_queue, &w);
optee_cq_wait_init(&optee->call_queue, &w, system_thread);
while (true) {
rc = msg_ops->sync_send_receive(ffa_dev, data);
if (rc)
Expand Down Expand Up @@ -612,7 +613,8 @@ static int optee_ffa_yielding_call(struct tee_context *ctx,
*/

static int optee_ffa_do_call_with_arg(struct tee_context *ctx,
struct tee_shm *shm, u_int offs)
struct tee_shm *shm, u_int offs,
bool system_thread)
{
struct ffa_send_direct_data data = {
.data0 = OPTEE_FFA_YIELDING_CALL_WITH_ARG,
Expand Down Expand Up @@ -642,7 +644,7 @@ static int optee_ffa_do_call_with_arg(struct tee_context *ctx,
if (IS_ERR(rpc_arg))
return PTR_ERR(rpc_arg);

return optee_ffa_yielding_call(ctx, &data, rpc_arg);
return optee_ffa_yielding_call(ctx, &data, rpc_arg, system_thread);
}

/*
Expand Down Expand Up @@ -850,8 +852,7 @@ static int optee_ffa_probe(struct ffa_device *ffa_dev)
if (rc)
goto err_unreg_supp_teedev;
mutex_init(&optee->ffa.mutex);
mutex_init(&optee->call_queue.mutex);
INIT_LIST_HEAD(&optee->call_queue.waiters);
optee_cq_init(&optee->call_queue, 0);
optee_supp_init(&optee->supp);
optee_shm_arg_cache_init(optee, arg_cache_flags);
ffa_dev_set_drvdata(ffa_dev, optee);
Expand Down
Loading