Re: [RFC PATCH v2 1/9] crypto: qce: Add core driver implementation
On Fri, May 09, 2014 at 12:57:39AM +0300, Stanimir Vabanov wrote: Hi Herbert, On 04/28/2014 11:59 AM, Herbert Xu wrote: On Mon, Apr 14, 2014 at 03:48:37PM +0300, Stanimir Varbanov wrote: +#define QCE_MAJOR_VERSION50x05 +#define QCE_QUEUE_LENGTH 50 What is the purpose of this software queue? Why can't you directly feed the requests to the hardware? If the hardware can't handle more than 50 requests in-flight, then your software queue has failed to handle this since you're taking requests off the queue before you touch the hardware so you're not really limiting it to 50. That is, for users that can wait you're potentially dropping their requests instead of letting them wait through the backlog mechanism. My assumption was that crypto_ablkcipher_encrypt/decrypt couldn't sleep and I should take the request almost immediately and return the appropriate error value - EINPROGRESS if the hardware is idle and EBUSY if the hardware working on some previous request. Thus if the returned error is EBUSY and the request could be backlogged I should call backlog-complete() when this request is taken actually for processing. What I've done in practice is another story. Is that assumption correct? If so, is crypto_enqueue|dequeue_request() are the proper tools to implement this behaviour? Technically you are allowed to sleep if the MAY_SLEEP bit is set but it's safe to just not sleep if that makes things easier for you. The enqueue/dequeue functions implement a software queue. Typically you would have a software queue in addition to whatever requests you have in flight on the actual hardware. For example, if your hardware is only able to handle one outstanding request, then your software queue should only be dequeued once the outstanding request has completed. Cheers, -- Email: Herbert Xu herb...@gondor.apana.org.au Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH v2 1/9] crypto: qce: Add core driver implementation
Hi Herbert, On 04/28/2014 11:59 AM, Herbert Xu wrote: On Mon, Apr 14, 2014 at 03:48:37PM +0300, Stanimir Varbanov wrote: +#define QCE_MAJOR_VERSION5 0x05 +#define QCE_QUEUE_LENGTH50 What is the purpose of this software queue? Why can't you directly feed the requests to the hardware? If the hardware can't handle more than 50 requests in-flight, then your software queue has failed to handle this since you're taking requests off the queue before you touch the hardware so you're not really limiting it to 50. That is, for users that can wait you're potentially dropping their requests instead of letting them wait through the backlog mechanism. My assumption was that crypto_ablkcipher_encrypt/decrypt couldn't sleep and I should take the request almost immediately and return the appropriate error value - EINPROGRESS if the hardware is idle and EBUSY if the hardware working on some previous request. Thus if the returned error is EBUSY and the request could be backlogged I should call backlog-complete() when this request is taken actually for processing. What I've done in practice is another story. Is that assumption correct? If so, is crypto_enqueue|dequeue_request() are the proper tools to implement this behaviour? regards, Stan -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH v2 1/9] crypto: qce: Add core driver implementation
Hi Herbert, On 04/28/2014 11:59 AM, Herbert Xu wrote: On Mon, Apr 14, 2014 at 03:48:37PM +0300, Stanimir Varbanov wrote: +#define QCE_MAJOR_VERSION5 0x05 +#define QCE_QUEUE_LENGTH50 What is the purpose of this software queue? Why can't you directly feed the requests to the hardware? Good question. This is a leftover from original driver. The hardware can handle one request at a time. After you raise the question I think the queue length should be 1 or remove it completely. I don't know why the original codeaurora's driver use 50. If the hardware can't handle more than 50 requests in-flight, then your software queue has failed to handle this since you're taking requests off the queue before you touch the hardware so you're not really limiting it to 50. That is, for users that can wait you're potentially dropping their requests instead of letting them wait through the backlog mechanism. -- regards, Stan -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH v2 1/9] crypto: qce: Add core driver implementation
Thanks for the review! On 04/28/2014 11:50 AM, Herbert Xu wrote: On Mon, Apr 14, 2014 at 03:48:37PM +0300, Stanimir Varbanov wrote: +if (backlog) +backlog-complete(backlog, -EINPROGRESS); The completion function needs to be called with BH disabled. Cheers, This is new for me because I saw similar code in cryptd.c where in cryptd_queue_worker() (workqueue context) the backlog-complete() is called outside of local_bh_disable(). -- regards, Stan -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH v2 1/9] crypto: qce: Add core driver implementation
On Tue, Apr 29, 2014 at 05:38:14PM +0300, Stanimir Varbanov wrote: This is new for me because I saw similar code in cryptd.c where in cryptd_queue_worker() (workqueue context) the backlog-complete() is called outside of local_bh_disable(). That's what I thought :) If you dig deeper you'll find that when cryptd calls the actual completion functions (rather than its own) it disables BH. Cheers, -- Email: Herbert Xu herb...@gondor.apana.org.au Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH v2 1/9] crypto: qce: Add core driver implementation
On Mon, Apr 14, 2014 at 03:48:37PM +0300, Stanimir Varbanov wrote: + if (backlog) + backlog-complete(backlog, -EINPROGRESS); The completion function needs to be called with BH disabled. Cheers, -- Email: Herbert Xu herb...@gondor.apana.org.au Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH v2 1/9] crypto: qce: Add core driver implementation
On Mon, Apr 14, 2014 at 03:48:37PM +0300, Stanimir Varbanov wrote: +#define QCE_MAJOR_VERSION5 0x05 +#define QCE_QUEUE_LENGTH 50 What is the purpose of this software queue? Why can't you directly feed the requests to the hardware? If the hardware can't handle more than 50 requests in-flight, then your software queue has failed to handle this since you're taking requests off the queue before you touch the hardware so you're not really limiting it to 50. That is, for users that can wait you're potentially dropping their requests instead of letting them wait through the backlog mechanism. Cheers, -- Email: Herbert Xu herb...@gondor.apana.org.au Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC PATCH v2 1/9] crypto: qce: Add core driver implementation
This adds core driver files. The core part is implementing a
platform driver probe and remove callbaks, the probe enables
clocks, checks crypto version, initialize and request dma
channels, create done tasklet and work queue and finally
register the algorithms into crypto subsystem.
Signed-off-by: Stanimir Varbanov svarba...@mm-sol.com
---
drivers/crypto/qce/core.c | 295 ++
drivers/crypto/qce/core.h | 73
2 files changed, 368 insertions(+)
create mode 100644 drivers/crypto/qce/core.c
create mode 100644 drivers/crypto/qce/core.h
diff --git a/drivers/crypto/qce/core.c b/drivers/crypto/qce/core.c
new file mode 100644
index ..61d08c5ff5b9
--- /dev/null
+++ b/drivers/crypto/qce/core.c
@@ -0,0 +1,295 @@
+/*
+ * Copyright (c) 2010-2014, The Linux Foundation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include linux/clk.h
+#include linux/interrupt.h
+#include linux/module.h
+#include linux/platform_device.h
+#include linux/spinlock.h
+#include linux/types.h
+#include crypto/algapi.h
+#include crypto/internal/hash.h
+#include crypto/sha.h
+
+#include core.h
+#include cipher.h
+#include sha.h
+
+#define QCE_MAJOR_VERSION5 0x05
+#define QCE_QUEUE_LENGTH 50
+
+static int qce_async_request_queue(struct qce_device *qce,
+ struct crypto_async_request *req)
+{
+ unsigned long flags;
+ int ret;
+
+ spin_lock_irqsave(qce-lock, flags);
+ ret = crypto_enqueue_request(qce-queue, req);
+ spin_unlock_irqrestore(qce-lock, flags);
+
+ queue_work(qce-queue_wq, qce-queue_work);
+
+ return ret;
+}
+
+static void qce_async_request_done(struct qce_device *qce, int ret)
+{
+ qce-result = ret;
+ tasklet_schedule(qce-done_tasklet);
+}
+
+static struct qce_algo_ops *qce_ops[] = {
+ ablkcipher_ops,
+ ahash_ops,
+};
+
+static void qce_unregister_algs(struct qce_device *qce)
+{
+ struct qce_algo_ops *ops;
+ int i;
+
+ for (i = 0; i ARRAY_SIZE(qce_ops); i++) {
+ ops = qce_ops[i];
+ ops-unregister_algs(qce);
+ }
+}
+
+static int qce_register_algs(struct qce_device *qce)
+{
+ struct qce_algo_ops *ops;
+ int i, ret = -ENODEV;
+
+ qce-async_req_queue = qce_async_request_queue;
+ qce-async_req_done = qce_async_request_done;
+
+ for (i = 0; i ARRAY_SIZE(qce_ops); i++) {
+ ops = qce_ops[i];
+ ret = ops-register_algs(qce);
+ if (ret)
+ break;
+ }
+
+ return ret;
+}
+
+static int qce_handle_request(struct crypto_async_request *async_req)
+{
+ int ret = -EINVAL, i;
+ struct qce_algo_ops *ops;
+ u32 type = crypto_tfm_alg_type(async_req-tfm);
+
+ for (i = 0; i ARRAY_SIZE(qce_ops); i++) {
+ ops = qce_ops[i];
+ if (type != ops-type)
+ continue;
+ ret = ops-async_req_handle(async_req);
+ break;
+ }
+
+ return ret;
+}
+
+static void qce_reqqueue_handler(struct work_struct *work)
+{
+ struct qce_device *qce =
+ container_of(work, struct qce_device, queue_work);
+ struct crypto_async_request *async_req = NULL, *backlog = NULL;
+ unsigned long flags;
+ int ret;
+
+ spin_lock_irqsave(qce-lock, flags);
+ if (!qce-req) {
+ backlog = crypto_get_backlog(qce-queue);
+ async_req = crypto_dequeue_request(qce-queue);
+ qce-req = async_req;
+ }
+ spin_unlock_irqrestore(qce-lock, flags);
+
+ if (!async_req)
+ return;
+
+ if (backlog)
+ backlog-complete(backlog, -EINPROGRESS);
+
+ ret = qce_handle_request(async_req);
+ if (ret) {
+ spin_lock_irqsave(qce-lock, flags);
+ qce-req = NULL;
+ spin_unlock_irqrestore(qce-lock, flags);
+
+ async_req-complete(async_req, ret);
+ }
+}
+
+static void qce_tasklet_req_done(unsigned long data)
+{
+ struct qce_device *qce = (struct qce_device *)data;
+ struct crypto_async_request *areq;
+ unsigned long flags;
+
+ spin_lock_irqsave(qce-lock, flags);
+ areq = qce-req;
+ qce-req = NULL;
+ spin_unlock_irqrestore(qce-lock, flags);
+
+ if (areq)
+ areq-complete(areq, qce-result);
+
+ queue_work(qce-queue_wq, qce-queue_work);
+}
+
+static int
