optee: support asynchronous supplicant requests

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 <etienne.carriere@linaro.org>
Tested-by: Etienne Carriere <etienne.carriere@linaro.org> (b2260 pager=y/n)
Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>

4 files changed, 243 insertions, 173 deletions

diff --git a/drivers/tee/optee/core.c b/drivers/tee/optee/core.c
index 7952357df9c8..b7492da92567 100644
--- a/drivers/tee/optee/core.c
+++ b/drivers/tee/optee/core.c
@@ -187,12 +187,12 @@ static int optee_open(struct tee_context *ctx)
         if (teedev == optee->supp_teedev) {
                 bool busy = true;
 
-                mutex_lock(&optee->supp.ctx_mutex);
+                mutex_lock(&optee->supp.mutex);
                 if (!optee->supp.ctx) {
                         busy = false;
                         optee->supp.ctx = ctx;
                 }
-                mutex_unlock(&optee->supp.ctx_mutex);
+                mutex_unlock(&optee->supp.mutex);
                 if (busy) {
                         kfree(ctxdata);
                         return -EBUSY;
@@ -252,11 +252,8 @@ static void optee_release(struct tee_context *ctx)
 
         ctx->data = NULL;
 
-        if (teedev == optee->supp_teedev) {
-                mutex_lock(&optee->supp.ctx_mutex);
-                optee->supp.ctx = NULL;
-                mutex_unlock(&optee->supp.ctx_mutex);
-        }
+        if (teedev == optee->supp_teedev)
+                optee_supp_release(&optee->supp);
 }
 
 static const struct tee_driver_ops optee_ops = {
diff --git a/drivers/tee/optee/optee_private.h b/drivers/tee/optee/optee_private.h
index c374cd594314..3e7da187acbe 100644
--- a/drivers/tee/optee/optee_private.h
+++ b/drivers/tee/optee/optee_private.h
@@ -53,36 +53,24 @@ struct optee_wait_queue {
  * @ctx                 the context of current connected supplicant.
  *                      if !NULL the supplicant device is available for use,
  *                      else busy
- * @ctx_mutex:          held while accessing @ctx
- * @func:               supplicant function id to call
- * @ret:                call return value
- * @num_params:         number of elements in @param
- * @param:              parameters for @func
- * @req_posted:         if true, a request has been posted to the supplicant
- * @supp_next_send:     if true, next step is for supplicant to send response
- * @thrd_mutex:         held by the thread doing a request to supplicant
- * @supp_mutex:         held by supplicant while operating on this struct
- * @data_to_supp:       supplicant is waiting on this for next request
- * @data_from_supp:     requesting thread is waiting on this to get the result
+ * @mutex:              held while accessing content of this struct
+ * @req_id:             current request id if supplicant is doing synchronous
+ *                      communication, else -1
+ * @reqs:               queued request not yet retrieved by supplicant
+ * @idr:                IDR holding all requests currently being processed
+ *                      by supplicant
+ * @reqs_c:             completion used by supplicant when waiting for a
+ *                      request to be queued.
  */
 struct optee_supp {
+        /* Serializes access to this struct */
+        struct mutex mutex;
         struct tee_context *ctx;
-        /* Serializes access of ctx */
-        struct mutex ctx_mutex;
-
-        u32 func;
-        u32 ret;
-        size_t num_params;
-        struct tee_param *param;
-
-        bool req_posted;
-        bool supp_next_send;
-        /* Serializes access to this struct for requesting thread */
-        struct mutex thrd_mutex;
-        /* Serializes access to this struct for supplicant threads */
-        struct mutex supp_mutex;
-        struct completion data_to_supp;
-        struct completion data_from_supp;
+
+        int req_id;
+        struct list_head reqs;
+        struct idr idr;
+        struct completion reqs_c;
 };
 
 /**
@@ -142,6 +130,7 @@ int optee_supp_read(struct tee_context *ctx, void __user *buf, size_t len);
 int optee_supp_write(struct tee_context *ctx, void __user *buf, size_t len);
 void optee_supp_init(struct optee_supp *supp);
 void optee_supp_uninit(struct optee_supp *supp);
+void optee_supp_release(struct optee_supp *supp);
 
 int optee_supp_recv(struct tee_context *ctx, u32 *func, u32 *num_params,
                     struct tee_param *param);
diff --git a/drivers/tee/optee/rpc.c b/drivers/tee/optee/rpc.c
index cef417f4f4d2..c6df4317ca9f 100644
--- a/drivers/tee/optee/rpc.c
+++ b/drivers/tee/optee/rpc.c
@@ -192,10 +192,10 @@ static struct tee_shm *cmd_alloc_suppl(struct tee_context *ctx, size_t sz)
         if (ret)
                 return ERR_PTR(-ENOMEM);
 
-        mutex_lock(&optee->supp.ctx_mutex);
+        mutex_lock(&optee->supp.mutex);
         /* Increases count as secure world doesn't have a reference */
         shm = tee_shm_get_from_id(optee->supp.ctx, param.u.value.c);
-        mutex_unlock(&optee->supp.ctx_mutex);
+        mutex_unlock(&optee->supp.mutex);
         return shm;
 }
 
diff --git a/drivers/tee/optee/supp.c b/drivers/tee/optee/supp.c
index 56aa8b929b8c..df35fc01fd3e 100644
--- a/drivers/tee/optee/supp.c
+++ b/drivers/tee/optee/supp.c
@@ -16,21 +16,61 @@
 #include <linux/uaccess.h>
 #include "optee_private.h"
 
+struct optee_supp_req {
+        struct list_head link;
+
+        bool busy;
+        u32 func;
+        u32 ret;
+        size_t num_params;
+        struct tee_param *param;
+
+        struct completion c;
+};
+
 void optee_supp_init(struct optee_supp *supp)
 {
         memset(supp, 0, sizeof(*supp));
-        mutex_init(&supp->ctx_mutex);
-        mutex_init(&supp->thrd_mutex);
-        mutex_init(&supp->supp_mutex);
-        init_completion(&supp->data_to_supp);
-        init_completion(&supp->data_from_supp);
+        mutex_init(&supp->mutex);
+        init_completion(&supp->reqs_c);
+        idr_init(&supp->idr);
+        INIT_LIST_HEAD(&supp->reqs);
+        supp->req_id = -1;
 }
 
 void optee_supp_uninit(struct optee_supp *supp)
 {
-        mutex_destroy(&supp->ctx_mutex);
-        mutex_destroy(&supp->thrd_mutex);
-        mutex_destroy(&supp->supp_mutex);
+        mutex_destroy(&supp->mutex);
+        idr_destroy(&supp->idr);
+}
+
+void optee_supp_release(struct optee_supp *supp)
+{
+        int id;
+        struct optee_supp_req *req;
+        struct optee_supp_req *req_tmp;
+
+        mutex_lock(&supp->mutex);
+
+        /* Abort all request retrieved by supplicant */
+        idr_for_each_entry(&supp->idr, req, id) {
+                req->busy = false;
+                idr_remove(&supp->idr, id);
+                req->ret = TEEC_ERROR_COMMUNICATION;
+                complete(&req->c);
+        }
+
+        /* Abort all queued requests */
+        list_for_each_entry_safe(req, req_tmp, &supp->reqs, link) {
+                list_del(&req->link);
+                req->ret = TEEC_ERROR_COMMUNICATION;
+                complete(&req->c);
+        }
+
+        supp->ctx = NULL;
+        supp->req_id = -1;
+
+        mutex_unlock(&supp->mutex);
 }
 
 /**
@@ -44,53 +84,42 @@ void optee_supp_uninit(struct optee_supp *supp)
  */
 u32 optee_supp_thrd_req(struct tee_context *ctx, u32 func, size_t num_params,
                         struct tee_param *param)
+
 {
-        bool interruptable;
         struct optee *optee = tee_get_drvdata(ctx->teedev);
         struct optee_supp *supp = &optee->supp;
+        struct optee_supp_req *req = kzalloc(sizeof(*req), GFP_KERNEL);
+        bool interruptable;
         u32 ret;
 
-        /*
-         * Other threads blocks here until we've copied our answer from
-         * supplicant.
-         */
-        while (mutex_lock_interruptible(&supp->thrd_mutex)) {
-                /* See comment below on when the RPC can be interrupted. */
-                mutex_lock(&supp->ctx_mutex);
-                interruptable = !supp->ctx;
-                mutex_unlock(&supp->ctx_mutex);
-                if (interruptable)
-                        return TEEC_ERROR_COMMUNICATION;
-        }
+        if (!req)
+                return TEEC_ERROR_OUT_OF_MEMORY;
 
-        /*
-         * We have exclusive access now since the supplicant at this
-         * point is either doing a
-         * wait_for_completion_interruptible(&supp->data_to_supp) or is in
-         * userspace still about to do the ioctl() to enter
-         * optee_supp_recv() below.
-         */
+        init_completion(&req->c);
+        req->func = func;
+        req->num_params = num_params;
+        req->param = param;
 
-        supp->func = func;
-        supp->num_params = num_params;
-        supp->param = param;
-        supp->req_posted = true;
+        /* Insert the request in the request list */
+        mutex_lock(&supp->mutex);
+        list_add_tail(&req->link, &supp->reqs);
+        mutex_unlock(&supp->mutex);
 
-        /* Let supplicant get the data */
-        complete(&supp->data_to_supp);
+        /* Tell an eventual waiter there's a new request */
+        complete(&supp->reqs_c);
 
         /*
          * Wait for supplicant to process and return result, once we've
-         * returned from wait_for_completion(data_from_supp) we have
+         * returned from wait_for_completion(&req->c) successfully we have
          * exclusive access again.
          */
-        while (wait_for_completion_interruptible(&supp->data_from_supp)) {
-                mutex_lock(&supp->ctx_mutex);
+        while (wait_for_completion_interruptible(&req->c)) {
+                mutex_lock(&supp->mutex);
                 interruptable = !supp->ctx;
                 if (interruptable) {
                         /*
                          * There's no supplicant available and since the
-                         * supp->ctx_mutex currently is held none can
+                         * supp->mutex currently is held none can
                          * become available until the mutex released
                          * again.
                          *
@@ -101,28 +130,65 @@ u32 optee_supp_thrd_req(struct tee_context *ctx, u32 func, size_t num_params,
                          * will serve all requests in a timely manner and
                          * interrupting then wouldn't make sense.
                          */
-                        supp->ret = TEEC_ERROR_COMMUNICATION;
-                        init_completion(&supp->data_to_supp);
+                        interruptable = !req->busy;
+                        if (!req->busy)
+                                list_del(&req->link);
                 }
-                mutex_unlock(&supp->ctx_mutex);
-                if (interruptable)
+                mutex_unlock(&supp->mutex);
+
+                if (interruptable) {
+                        req->ret = TEEC_ERROR_COMMUNICATION;
                         break;
+                }
         }
 
-        ret = supp->ret;
-        supp->param = NULL;
-        supp->req_posted = false;
-
-        /* We're done, let someone else talk to the supplicant now. */
-        mutex_unlock(&supp->thrd_mutex);
+        ret = req->ret;
+        kfree(req);
 
         return ret;
 }
 
-static int supp_check_recv_params(size_t num_params, struct tee_param *params)
+static struct optee_supp_req  *supp_pop_entry(struct optee_supp *supp,
+                                              int num_params, int *id)
+{
+        struct optee_supp_req *req;
+
+        if (supp->req_id != -1) {
+                /*
+                 * Supplicant should not mix synchronous and asnynchronous
+                 * requests.
+                 */
+                return ERR_PTR(-EINVAL);
+        }
+
+        if (list_empty(&supp->reqs))
+                return NULL;
+
+        req = list_first_entry(&supp->reqs, struct optee_supp_req, link);
+
+        if (num_params < req->num_params) {
+                /* Not enough room for parameters */
+                return ERR_PTR(-EINVAL);
+        }
+
+        *id = idr_alloc(&supp->idr, req, 1, 0, GFP_KERNEL);
+        if (*id < 0)
+                return ERR_PTR(-ENOMEM);
+
+        list_del(&req->link);
+        req->busy = true;
+
+        return req;
+}
+
+static int supp_check_recv_params(size_t num_params, struct tee_param *params,
+                                  size_t *num_meta)
 {
         size_t n;
 
+        if (!num_params)
+                return -EINVAL;
+
         /*
          * If there's memrefs we need to decrease those as they where
          * increased earlier and we'll even refuse to accept any below.
@@ -132,11 +198,20 @@ static int supp_check_recv_params(size_t num_params, struct tee_param *params)
                         tee_shm_put(params[n].u.memref.shm);
 
         /*
-         * We only expect parameters as TEE_IOCTL_PARAM_ATTR_TYPE_NONE (0).
+         * We only expect parameters as TEE_IOCTL_PARAM_ATTR_TYPE_NONE with
+         * or without the TEE_IOCTL_PARAM_ATTR_META bit set.
          */
         for (n = 0; n < num_params; n++)
-                if (params[n].attr)
+                if (params[n].attr &&
+                    params[n].attr != TEE_IOCTL_PARAM_ATTR_META)
                         return -EINVAL;
+
+        /* At most we'll need one meta parameter so no need to check for more */
+        if (params->attr == TEE_IOCTL_PARAM_ATTR_META)
+                *num_meta = 1;
+        else
+                *num_meta = 0;
+
         return 0;
 }
 
@@ -156,69 +231,99 @@ int optee_supp_recv(struct tee_context *ctx, u32 *func, u32 *num_params,
         struct tee_device *teedev = ctx->teedev;
         struct optee *optee = tee_get_drvdata(teedev);
         struct optee_supp *supp = &optee->supp;
+        struct optee_supp_req *req = NULL;
+        int id;
+        size_t num_meta;
         int rc;
 
-        rc = supp_check_recv_params(*num_params, param);
+        rc = supp_check_recv_params(*num_params, param, &num_meta);
         if (rc)
                 return rc;
 
-        /*
-         * In case two threads in one supplicant is calling this function
-         * simultaneously we need to protect the data with a mutex which
-         * we'll release before returning.
-         */
-        mutex_lock(&supp->supp_mutex);
+        while (true) {
+                mutex_lock(&supp->mutex);
+                req = supp_pop_entry(supp, *num_params - num_meta, &id);
+                mutex_unlock(&supp->mutex);
+
+                if (req) {
+                        if (IS_ERR(req))
+                                return PTR_ERR(req);
+                        break;
+                }
 
-        if (supp->supp_next_send) {
                 /*
-                 * optee_supp_recv() has been called again without
-                 * a optee_supp_send() in between. Supplicant has
-                 * probably been restarted before it was able to
-                 * write back last result. Abort last request and
-                 * wait for a new.
+                 * If we didn't get a request we'll block in
+                 * wait_for_completion() to avoid needless spinning.
+                 *
+                 * This is where supplicant will be hanging most of
+                 * the time, let's make this interruptable so we
+                 * can easily restart supplicant if needed.
                  */
-                if (supp->req_posted) {
-                        supp->ret = TEEC_ERROR_COMMUNICATION;
-                        supp->supp_next_send = false;
-                        complete(&supp->data_from_supp);
-                }
+                if (wait_for_completion_interruptible(&supp->reqs_c))
+                        return -ERESTARTSYS;
         }
 
-        /*
-         * This is where supplicant will be hanging most of the
-         * time, let's make this interruptable so we can easily
-         * restart supplicant if needed.
-         */
-        if (wait_for_completion_interruptible(&supp->data_to_supp)) {
-                rc = -ERESTARTSYS;
-                goto out;
+        if (num_meta) {
+                /*
+                 * tee-supplicant support meta parameters -> requsts can be
+                 * processed asynchronously.
+                 */
+                param->attr = TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_INOUT |
+                              TEE_IOCTL_PARAM_ATTR_META;
+                param->u.value.a = id;
+                param->u.value.b = 0;
+                param->u.value.c = 0;
+        } else {
+                mutex_lock(&supp->mutex);
+                supp->req_id = id;
+                mutex_unlock(&supp->mutex);
         }
 
-        /* We have exlusive access to the data */
+        *func = req->func;
+        *num_params = req->num_params + num_meta;
+        memcpy(param + num_meta, req->param,
+               sizeof(struct tee_param) * req->num_params);
 
-        if (*num_params < supp->num_params) {
-                /*
-                 * Not enough room for parameters, tell supplicant
-                 * it failed and abort last request.
-                 */
-                supp->ret = TEEC_ERROR_COMMUNICATION;
-                rc = -EINVAL;
-                complete(&supp->data_from_supp);
-                goto out;
+        return 0;
+}
+
+static struct optee_supp_req *supp_pop_req(struct optee_supp *supp,
+                                           size_t num_params,
+                                           struct tee_param *param,
+                                           size_t *num_meta)
+{
+        struct optee_supp_req *req;
+        int id;
+        size_t nm;
+        const u32 attr = TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_INOUT |
+                         TEE_IOCTL_PARAM_ATTR_META;
+
+        if (!num_params)
+                return ERR_PTR(-EINVAL);
+
+        if (supp->req_id == -1) {
+                if (param->attr != attr)
+                        return ERR_PTR(-EINVAL);
+                id = param->u.value.a;
+                nm = 1;
+        } else {
+                id = supp->req_id;
+                nm = 0;
         }
 
-        *func = supp->func;
-        *num_params = supp->num_params;
-        memcpy(param, supp->param,
-               sizeof(struct tee_param) * supp->num_params);
+        req = idr_find(&supp->idr, id);
+        if (!req)
+                return ERR_PTR(-ENOENT);
+
+        if ((num_params - nm) != req->num_params)
+                return ERR_PTR(-EINVAL);
 
-        /* Allow optee_supp_send() below to do its work */
-        supp->supp_next_send = true;
+        req->busy = false;
+        idr_remove(&supp->idr, id);
+        supp->req_id = -1;
+        *num_meta = nm;
 
-        rc = 0;
-out:
-        mutex_unlock(&supp->supp_mutex);
-        return rc;
+        return req;
 }
 
 /**
@@ -236,63 +341,42 @@ int optee_supp_send(struct tee_context *ctx, u32 ret, u32 num_params,
         struct tee_device *teedev = ctx->teedev;
         struct optee *optee = tee_get_drvdata(teedev);
         struct optee_supp *supp = &optee->supp;
+        struct optee_supp_req *req;
         size_t n;
-        int rc = 0;
+        size_t num_meta;
 
-        /*
-         * We still have exclusive access to the data since that's how we
-         * left it when returning from optee_supp_read().
-         */
+        mutex_lock(&supp->mutex);
+        req = supp_pop_req(supp, num_params, param, &num_meta);
+        mutex_unlock(&supp->mutex);
 
-        /* See comment on mutex in optee_supp_read() above */
-        mutex_lock(&supp->supp_mutex);
-
-        if (!supp->supp_next_send) {
-                /*
-                 * Something strange is going on, supplicant shouldn't
-                 * enter optee_supp_send() in this state
-                 */
-                rc = -ENOENT;
-                goto out;
-        }
-
-        if (num_params != supp->num_params) {
-                /*
-                 * Something is wrong, let supplicant restart. Next call to
-                 * optee_supp_recv() will give an error to the requesting
-                 * thread and release it.
-                 */
-                rc = -EINVAL;
-                goto out;
+        if (IS_ERR(req)) {
+                /* Something is wrong, let supplicant restart. */
+                return PTR_ERR(req);
         }
 
         /* Update out and in/out parameters */
-        for (n = 0; n < num_params; n++) {
-                struct tee_param *p = supp->param + n;
+        for (n = 0; n < req->num_params; n++) {
+                struct tee_param *p = req->param + n;
 
-                switch (p->attr) {
+                switch (p->attr & TEE_IOCTL_PARAM_ATTR_TYPE_MASK) {
                 case TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_OUTPUT:
                 case TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_INOUT:
-                        p->u.value.a = param[n].u.value.a;
-                        p->u.value.b = param[n].u.value.b;
-                        p->u.value.c = param[n].u.value.c;
+                        p->u.value.a = param[n + num_meta].u.value.a;
+                        p->u.value.b = param[n + num_meta].u.value.b;
+                        p->u.value.c = param[n + num_meta].u.value.c;
                         break;
                 case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_OUTPUT:
                 case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INOUT:
-                        p->u.memref.size = param[n].u.memref.size;
+                        p->u.memref.size = param[n + num_meta].u.memref.size;
                         break;
                 default:
                         break;
                 }
         }
-        supp->ret = ret;
-
-        /* Allow optee_supp_recv() above to do its work */
-        supp->supp_next_send = false;
+        req->ret = ret;
 
         /* Let the requesting thread continue */
-        complete(&supp->data_from_supp);
-out:
-        mutex_unlock(&supp->supp_mutex);
-        return rc;
+        complete(&req->c);
+
+        return 0;
 }