Integrate crypto_engine into CAAM, to make use of the engine queue.
Add support for SKCIPHER algorithms.
This is intended to be used for CAAM backlogging support.
The requests, with backlog flag (e.g. from dm-crypt) will be listed
into crypto-engine queue and processed by CAAM when free.
This changes the return codes for caam_jr_enqueue:
-EINPROGRESS if OK, -EBUSY if request is backlogged,
-ENOSPC if the queue is full, -EIO if it cannot map the caller's
descriptor, -EINVAL if crypto_tfm not supported by crypto_engine.
Signed-off-by: Iuliana Prodan <[email protected]>
Signed-off-by: Franck LENORMAND <[email protected]>
---
drivers/crypto/caam/Kconfig | 1 +
drivers/crypto/caam/caamalg.c | 84 +++++++++++++++++++++++++++++++++++--------
drivers/crypto/caam/intern.h | 2 ++
drivers/crypto/caam/jr.c | 51 ++++++++++++++++++++++++--
4 files changed, 122 insertions(+), 16 deletions(-)
diff --git a/drivers/crypto/caam/Kconfig b/drivers/crypto/caam/Kconfig
index 87053e4..1930e19 100644
--- a/drivers/crypto/caam/Kconfig
+++ b/drivers/crypto/caam/Kconfig
@@ -33,6 +33,7 @@ config CRYPTO_DEV_FSL_CAAM_DEBUG
menuconfig CRYPTO_DEV_FSL_CAAM_JR
tristate "Freescale CAAM Job Ring driver backend"
+ select CRYPTO_ENGINE
default y
help
Enables the driver module for Job Rings which are part of
diff --git a/drivers/crypto/caam/caamalg.c b/drivers/crypto/caam/caamalg.c
index abebcfc..23de94d 100644
--- a/drivers/crypto/caam/caamalg.c
+++ b/drivers/crypto/caam/caamalg.c
@@ -56,6 +56,7 @@
#include "sg_sw_sec4.h"
#include "key_gen.h"
#include "caamalg_desc.h"
+#include <crypto/engine.h>
/*
* crypto alg
@@ -101,6 +102,7 @@ struct caam_skcipher_alg {
* per-session context
*/
struct caam_ctx {
+ struct crypto_engine_ctx enginectx;
u32 sh_desc_enc[DESC_MAX_USED_LEN];
u32 sh_desc_dec[DESC_MAX_USED_LEN];
u8 key[CAAM_MAX_KEY_SIZE];
@@ -114,6 +116,12 @@ struct caam_ctx {
unsigned int authsize;
};
+struct caam_skcipher_req_ctx {
+ struct skcipher_edesc *edesc;
+ void (*skcipher_op_done)(struct device *jrdev, u32 *desc, u32 err,
+ void *context);
+};
+
static int aead_null_set_sh_desc(struct crypto_aead *aead)
{
struct caam_ctx *ctx = crypto_aead_ctx(aead);
@@ -992,13 +1000,15 @@ static void skcipher_crypt_done(struct device *jrdev, u32 *desc, u32 err,
struct caam_jr_request_entry *jrentry = context;
struct skcipher_request *req = skcipher_request_cast(jrentry->base);
struct skcipher_edesc *edesc;
+ struct caam_skcipher_req_ctx *rctx = skcipher_request_ctx(req);
struct crypto_skcipher *skcipher = crypto_skcipher_reqtfm(req);
+ struct caam_drv_private_jr *jrp = dev_get_drvdata(jrdev);
int ivsize = crypto_skcipher_ivsize(skcipher);
int ecode = 0;
dev_dbg(jrdev, "%s %d: err 0x%x\n", __func__, __LINE__, err);
- edesc = container_of(desc, struct skcipher_edesc, hw_desc[0]);
+ edesc = rctx->edesc;
if (err)
ecode = caam_jr_strstatus(jrdev, err);
@@ -1024,7 +1034,14 @@ static void skcipher_crypt_done(struct device *jrdev, u32 *desc, u32 err,
kfree(edesc);
- skcipher_request_complete(req, ecode);
+ /*
+ * If no backlog flag, the completion of the request is done
+ * by CAAM, not crypto engine.
+ */
+ if (!jrentry->bklog)
+ skcipher_request_complete(req, ecode);
+ else
+ crypto_finalize_skcipher_request(jrp->engine, req, ecode);
}
/*
@@ -1553,6 +1570,7 @@ static struct skcipher_edesc *skcipher_edesc_alloc(struct skcipher_request *req,
{
struct crypto_skcipher *skcipher = crypto_skcipher_reqtfm(req);
struct caam_ctx *ctx = crypto_skcipher_ctx(skcipher);
+ struct caam_skcipher_req_ctx *rctx = skcipher_request_ctx(req);
struct device *jrdev = ctx->jrdev;
gfp_t flags = (req->base.flags & CRYPTO_TFM_REQ_MAY_SLEEP) ?
GFP_KERNEL : GFP_ATOMIC;
@@ -1653,6 +1671,9 @@ static struct skcipher_edesc *skcipher_edesc_alloc(struct skcipher_request *req,
desc_bytes);
edesc->jrentry.base = &req->base;
+ rctx->edesc = edesc;
+ rctx->skcipher_op_done = skcipher_crypt_done;
+
/* Make sure IV is located in a DMAable area */
if (ivsize) {
iv = (u8 *)edesc->sec4_sg + sec4_sg_bytes;
@@ -1707,13 +1728,37 @@ static struct skcipher_edesc *skcipher_edesc_alloc(struct skcipher_request *req,
return edesc;
}
+static int skcipher_do_one_req(struct crypto_engine *engine, void *areq)
+{
+ struct skcipher_request *req = skcipher_request_cast(areq);
+ struct caam_ctx *ctx = crypto_skcipher_ctx(crypto_skcipher_reqtfm(req));
+ struct caam_skcipher_req_ctx *rctx = skcipher_request_ctx(req);
+ struct caam_jr_request_entry *jrentry;
+ u32 *desc = rctx->edesc->hw_desc;
+ int ret;
+
+ jrentry = &rctx->edesc->jrentry;
+ jrentry->bklog = true;
+
+ ret = caam_jr_enqueue_no_bklog(ctx->jrdev, desc,
+ rctx->skcipher_op_done, jrentry);
+
+ if (ret != -EINPROGRESS) {
+ skcipher_unmap(ctx->jrdev, rctx->edesc, req);
+ kfree(rctx->edesc);
+ } else {
+ ret = 0;
+ }
+
+ return ret;
+}
+
static inline int skcipher_crypt(struct skcipher_request *req, bool encrypt)
{
struct skcipher_edesc *edesc;
struct crypto_skcipher *skcipher = crypto_skcipher_reqtfm(req);
struct caam_ctx *ctx = crypto_skcipher_ctx(skcipher);
struct device *jrdev = ctx->jrdev;
- struct caam_jr_request_entry *jrentry;
u32 *desc;
int ret = 0;
@@ -1727,16 +1772,15 @@ static inline int skcipher_crypt(struct skcipher_request *req, bool encrypt)
/* Create and submit job descriptor*/
init_skcipher_job(req, edesc, encrypt);
+ desc = edesc->hw_desc;
print_hex_dump_debug("skcipher jobdesc@" __stringify(__LINE__)": ",
- DUMP_PREFIX_ADDRESS, 16, 4, edesc->hw_desc,
- desc_bytes(edesc->hw_desc), 1);
-
- desc = edesc->hw_desc;
- jrentry = &edesc->jrentry;
+ DUMP_PREFIX_ADDRESS, 16, 4, desc,
+ desc_bytes(desc), 1);
- ret = caam_jr_enqueue(jrdev, desc, skcipher_crypt_done, jrentry);
- if (ret != -EINPROGRESS) {
+ ret = caam_jr_enqueue(jrdev, desc, skcipher_crypt_done,
+ &edesc->jrentry);
+ if ((ret != -EINPROGRESS) && (ret != -EBUSY)) {
skcipher_unmap(jrdev, edesc, req);
kfree(edesc);
}
@@ -3272,7 +3316,9 @@ static int caam_init_common(struct caam_ctx *ctx, struct caam_alg_entry *caam,
dma_addr = dma_map_single_attrs(ctx->jrdev, ctx->sh_desc_enc,
offsetof(struct caam_ctx,
- sh_desc_enc_dma),
+ sh_desc_enc_dma) -
+ offsetof(struct caam_ctx,
+ sh_desc_enc),
ctx->dir, DMA_ATTR_SKIP_CPU_SYNC);
if (dma_mapping_error(ctx->jrdev, dma_addr)) {
dev_err(ctx->jrdev, "unable to map key, shared descriptors\n");
@@ -3282,8 +3328,12 @@ static int caam_init_common(struct caam_ctx *ctx, struct caam_alg_entry *caam,
ctx->sh_desc_enc_dma = dma_addr;
ctx->sh_desc_dec_dma = dma_addr + offsetof(struct caam_ctx,
- sh_desc_dec);
- ctx->key_dma = dma_addr + offsetof(struct caam_ctx, key);
+ sh_desc_dec) -
+ offsetof(struct caam_ctx,
+ sh_desc_enc);
+ ctx->key_dma = dma_addr + offsetof(struct caam_ctx, key) -
+ offsetof(struct caam_ctx,
+ sh_desc_enc);
/* copy descriptor header template value */
ctx->cdata.algtype = OP_TYPE_CLASS1_ALG | caam->class1_alg_type;
@@ -3297,6 +3347,11 @@ static int caam_cra_init(struct crypto_skcipher *tfm)
struct skcipher_alg *alg = crypto_skcipher_alg(tfm);
struct caam_skcipher_alg *caam_alg =
container_of(alg, typeof(*caam_alg), skcipher);
+ struct caam_ctx *ctx = crypto_skcipher_ctx(tfm);
+
+ crypto_skcipher_set_reqsize(tfm, sizeof(struct caam_skcipher_req_ctx));
+
+ ctx->enginectx.op.do_one_request = skcipher_do_one_req;
return caam_init_common(crypto_skcipher_ctx(tfm), &caam_alg->caam,
false);
@@ -3315,7 +3370,8 @@ static int caam_aead_init(struct crypto_aead *tfm)
static void caam_exit_common(struct caam_ctx *ctx)
{
dma_unmap_single_attrs(ctx->jrdev, ctx->sh_desc_enc_dma,
- offsetof(struct caam_ctx, sh_desc_enc_dma),
+ offsetof(struct caam_ctx, sh_desc_enc_dma) -
+ offsetof(struct caam_ctx, sh_desc_enc),
ctx->dir, DMA_ATTR_SKIP_CPU_SYNC);
caam_jr_free(ctx->jrdev);
}
diff --git a/drivers/crypto/caam/intern.h b/drivers/crypto/caam/intern.h
index 58be66c..31abb94 100644
--- a/drivers/crypto/caam/intern.h
+++ b/drivers/crypto/caam/intern.h
@@ -12,6 +12,7 @@
#include "ctrl.h"
#include "regs.h"
+#include <crypto/engine.h>
/* Currently comes from Kconfig param as a ^2 (driver-required) */
#define JOBR_DEPTH (1 << CONFIG_CRYPTO_DEV_FSL_CAAM_RINGSIZE)
@@ -61,6 +62,7 @@ struct caam_drv_private_jr {
int out_ring_read_index; /* Output index "tail" */
int tail; /* entinfo (s/w ring) tail index */
void *outring; /* Base of output ring, DMA-safe */
+ struct crypto_engine *engine;
};
/*
diff --git a/drivers/crypto/caam/jr.c b/drivers/crypto/caam/jr.c
index 544cafa..5c55d3d 100644
--- a/drivers/crypto/caam/jr.c
+++ b/drivers/crypto/caam/jr.c
@@ -62,6 +62,15 @@ static void unregister_algs(void)
mutex_unlock(&algs_lock);
}
+static void caam_jr_crypto_engine_exit(void *data)
+{
+ struct device *jrdev = data;
+ struct caam_drv_private_jr *jrpriv = dev_get_drvdata(jrdev);
+
+ /* Free the resources of crypto-engine */
+ crypto_engine_exit(jrpriv->engine);
+}
+
static int caam_reset_hw_jr(struct device *dev)
{
struct caam_drv_private_jr *jrp = dev_get_drvdata(dev);
@@ -418,10 +427,23 @@ int caam_jr_enqueue_no_bklog(struct device *dev, u32 *desc,
}
EXPORT_SYMBOL(caam_jr_enqueue_no_bklog);
+static int transfer_request_to_engine(struct crypto_engine *engine,
+ struct crypto_async_request *req)
+{
+ switch (crypto_tfm_alg_type(req->tfm)) {
+ case CRYPTO_ALG_TYPE_SKCIPHER:
+ return crypto_transfer_skcipher_request_to_engine(engine,
+ skcipher_request_cast(req));
+ default:
+ return -EINVAL;
+ }
+}
+
/**
* caam_jr_enqueue() - Enqueue a job descriptor head. Returns -EINPROGRESS
- * if OK, -ENOSPC if the queue is full, -EIO if it cannot map the caller's
- * descriptor.
+ * if OK, -EBUSY if request is backlogged, -ENOSPC if the queue is full,
+ * -EIO if it cannot map the caller's descriptor, -EINVAL if crypto_tfm
+ * not supported by crypto_engine.
* @dev: device of the job ring to be used. This device should have
* been assigned prior by caam_jr_register().
* @desc: points to a job descriptor that execute our request. All
@@ -451,7 +473,12 @@ int caam_jr_enqueue(struct device *dev, u32 *desc,
u32 status, void *areq),
void *areq)
{
+ struct caam_drv_private_jr *jrpriv = dev_get_drvdata(dev);
struct caam_jr_request_entry *jrentry = areq;
+ struct crypto_async_request *req = jrentry->base;
+
+ if (req->flags & CRYPTO_TFM_REQ_MAY_BACKLOG)
+ return transfer_request_to_engine(jrpriv->engine, req);
return caam_jr_enqueue_no_bklog(dev, desc, cbk, jrentry);
}
@@ -577,6 +604,26 @@ static int caam_jr_probe(struct platform_device *pdev)
return error;
}
+ /* Initialize crypto engine */
+ jrpriv->engine = crypto_engine_alloc_init(jrdev, false);
+ if (!jrpriv->engine) {
+ dev_err(jrdev, "Could not init crypto-engine\n");
+ return -ENOMEM;
+ }
+
+ /* Start crypto engine */
+ error = crypto_engine_start(jrpriv->engine);
+ if (error) {
+ dev_err(jrdev, "Could not start crypto-engine\n");
+ crypto_engine_exit(jrpriv->engine);
+ return error;
+ }
+
+ error = devm_add_action_or_reset(jrdev, caam_jr_crypto_engine_exit,
+ jrdev);
+ if (error)
+ return error;
+
/* Identify the interrupt */
jrpriv->irq = irq_of_parse_and_map(nprop, 0);
if (!jrpriv->irq) {
--
2.1.0
On 11/18/2019 12:31 AM, Iuliana Prodan wrote:
> Integrate crypto_engine into CAAM, to make use of the engine queue.
> Add support for SKCIPHER algorithms.
>
> This is intended to be used for CAAM backlogging support.
> The requests, with backlog flag (e.g. from dm-crypt) will be listed
> into crypto-engine queue and processed by CAAM when free.
> This changes the return codes for caam_jr_enqueue:
> -EINPROGRESS if OK, -EBUSY if request is backlogged,
> -ENOSPC if the queue is full, -EIO if it cannot map the caller's
> descriptor, -EINVAL if crypto_tfm not supported by crypto_engine.
>
> Signed-off-by: Iuliana Prodan <[email protected]>
> Signed-off-by: Franck LENORMAND <[email protected]>
Reviewed-by: Horia Geant? <[email protected]>
Thanks,
Horia
On 11/22/2019 12:33 PM, Herbert Xu wrote:
> On Mon, Nov 18, 2019 at 12:30:41AM +0200, Iuliana Prodan wrote:
>>
>> +static int transfer_request_to_engine(struct crypto_engine *engine,
>> + struct crypto_async_request *req)
>> +{
>> + switch (crypto_tfm_alg_type(req->tfm)) {
>> + case CRYPTO_ALG_TYPE_SKCIPHER:
>> + return crypto_transfer_skcipher_request_to_engine(engine,
>> + skcipher_request_cast(req));
>> + default:
>> + return -EINVAL;
>> + }
>> +}
>
> Please don't do this. As you can see the crypto engine interface
> wants to you to use the correct type for the request object. That's
> what you should do to.
Sorry, but I don't understand what is wrong here? I'm using the correct
type, the specific type, for the request when sending it to crypto engine.
This transfer_request_to_engine function is called from caam_jr_enqueue,
where I have all types of request, so I'm using the async_request, and
when transferring to crypto engine I cast it to the specific type.
> In fact I don't understand why you're only using the crypto engine
> for the backlog case. Wouldn't it be much simpler if you used the
> engine unconditionally?
I believe is an overhead to sent all request to crypto engine since most
of them can be directly executed by hw.
Also, in case there is no need for backlog and the hw is busy we can
drop the request.
Thanks,
Iulia
On Fri, Nov 22, 2019 at 11:05:59AM +0000, Iuliana Prodan wrote:
>
> Sorry, but I don't understand what is wrong here? I'm using the correct
> type, the specific type, for the request when sending it to crypto engine.
> This transfer_request_to_engine function is called from caam_jr_enqueue,
> where I have all types of request, so I'm using the async_request, and
> when transferring to crypto engine I cast it to the specific type.
These internal types are only for use by the crypto API and helper
code such as crypto_engine. They should not be used by drivers in
general.
> I believe is an overhead to sent all request to crypto engine since most
> of them can be directly executed by hw.
> Also, in case there is no need for backlog and the hw is busy we can
> drop the request.
If the crypto_engine has so much overhead then you should work on
fixing crypto_engine and not work around it like this.
Cheers,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
On Mon, Nov 18, 2019 at 12:30:41AM +0200, Iuliana Prodan wrote:
>
> +static int transfer_request_to_engine(struct crypto_engine *engine,
> + struct crypto_async_request *req)
> +{
> + switch (crypto_tfm_alg_type(req->tfm)) {
> + case CRYPTO_ALG_TYPE_SKCIPHER:
> + return crypto_transfer_skcipher_request_to_engine(engine,
> + skcipher_request_cast(req));
> + default:
> + return -EINVAL;
> + }
> +}
Please don't do this. As you can see the crypto engine interface
wants to you to use the correct type for the request object. That's
what you should do to.
In fact I don't understand why you're only using the crypto engine
for the backlog case. Wouldn't it be much simpler if you used the
engine unconditionally?
Cheers,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
On 11/22/2019 1:09 PM, Herbert Xu wrote:
> On Fri, Nov 22, 2019 at 11:05:59AM +0000, Iuliana Prodan wrote:
>>
>> Sorry, but I don't understand what is wrong here? I'm using the correct
>> type, the specific type, for the request when sending it to crypto engine.
>> This transfer_request_to_engine function is called from caam_jr_enqueue,
>> where I have all types of request, so I'm using the async_request, and
>> when transferring to crypto engine I cast it to the specific type.
>
> These internal types are only for use by the crypto API and helper
> code such as crypto_engine. They should not be used by drivers in
> general.
>
So, just to be clear, I shouldn't use crypto_async_request in driver code?
I see that this generic crypto request is used in multiple drivers.
>> I believe is an overhead to sent all request to crypto engine since most
>> of them can be directly executed by hw.
>> Also, in case there is no need for backlog and the hw is busy we can
>> drop the request.
>
> If the crypto_engine has so much overhead then you should work on
> fixing crypto_engine and not work around it like this.
>
I can try sending _all_ requests to crypto engine and make some
performance measurements to see which solution is best.
Thanks,
Iulia
On Fri, Nov 22, 2019 at 02:11:46PM +0000, Iuliana Prodan wrote:
>
> So, just to be clear, I shouldn't use crypto_async_request in driver code?
> I see that this generic crypto request is used in multiple drivers.
I understand that a number of drivers do this in order to share
common code. However, this is definitely not the preferred way
of handling this. Ideally such code should be abstracted into
a higher layer such as crypto_engine so that the driver itself
never references these internal types.
> I can try sending _all_ requests to crypto engine and make some
> performance measurements to see which solution is best.
Thanks,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Hi Iulia,
On 11/17/19 11:30 PM, Iuliana Prodan wrote:
> Integrate crypto_engine into CAAM, to make use of the engine queue.
> Add support for SKCIPHER algorithms.
>
> This is intended to be used for CAAM backlogging support.
> The requests, with backlog flag (e.g. from dm-crypt) will be listed
> into crypto-engine queue and processed by CAAM when free.
> This changes the return codes for caam_jr_enqueue:
> -EINPROGRESS if OK, -EBUSY if request is backlogged,
> -ENOSPC if the queue is full, -EIO if it cannot map the caller's
> descriptor, -EINVAL if crypto_tfm not supported by crypto_engine.
>
> Signed-off-by: Iuliana Prodan <[email protected]>
> Signed-off-by: Franck LENORMAND <[email protected]>
> Reviewed-by: Horia Geantă <[email protected]>
> ---
> drivers/crypto/caam/Kconfig | 1 +
> drivers/crypto/caam/caamalg.c | 84 +++++++++++++++++++++++++++++++++++--------
> drivers/crypto/caam/intern.h | 2 ++
> drivers/crypto/caam/jr.c | 51 ++++++++++++++++++++++++--
> 4 files changed, 122 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/crypto/caam/Kconfig b/drivers/crypto/caam/Kconfig
> index 87053e4..1930e19 100644
> --- a/drivers/crypto/caam/Kconfig
> +++ b/drivers/crypto/caam/Kconfig
> @@ -33,6 +33,7 @@ config CRYPTO_DEV_FSL_CAAM_DEBUG
>
> menuconfig CRYPTO_DEV_FSL_CAAM_JR
> tristate "Freescale CAAM Job Ring driver backend"
> + select CRYPTO_ENGINE
> default y
> help
> Enables the driver module for Job Rings which are part of
> diff --git a/drivers/crypto/caam/caamalg.c b/drivers/crypto/caam/caamalg.c
> index abebcfc..23de94d 100644
> --- a/drivers/crypto/caam/caamalg.c
> +++ b/drivers/crypto/caam/caamalg.c
> @@ -56,6 +56,7 @@
> #include "sg_sw_sec4.h"
> #include "key_gen.h"
> #include "caamalg_desc.h"
> +#include <crypto/engine.h>
>
> /*
> * crypto alg
> @@ -101,6 +102,7 @@ struct caam_skcipher_alg {
> * per-session context
> */
> struct caam_ctx {
> + struct crypto_engine_ctx enginectx;
> u32 sh_desc_enc[DESC_MAX_USED_LEN];
> u32 sh_desc_dec[DESC_MAX_USED_LEN];
> u8 key[CAAM_MAX_KEY_SIZE];
> @@ -114,6 +116,12 @@ struct caam_ctx {
> unsigned int authsize;
> };
>
> +struct caam_skcipher_req_ctx {
> + struct skcipher_edesc *edesc;
> + void (*skcipher_op_done)(struct device *jrdev, u32 *desc, u32 err,
> + void *context);
> +};
> +
> static int aead_null_set_sh_desc(struct crypto_aead *aead)
> {
> struct caam_ctx *ctx = crypto_aead_ctx(aead);
> @@ -992,13 +1000,15 @@ static void skcipher_crypt_done(struct device *jrdev, u32 *desc, u32 err,
> struct caam_jr_request_entry *jrentry = context;
> struct skcipher_request *req = skcipher_request_cast(jrentry->base);
> struct skcipher_edesc *edesc;
> + struct caam_skcipher_req_ctx *rctx = skcipher_request_ctx(req);
> struct crypto_skcipher *skcipher = crypto_skcipher_reqtfm(req);
> + struct caam_drv_private_jr *jrp = dev_get_drvdata(jrdev);
> int ivsize = crypto_skcipher_ivsize(skcipher);
> int ecode = 0;
>
> dev_dbg(jrdev, "%s %d: err 0x%x\n", __func__, __LINE__, err);
>
> - edesc = container_of(desc, struct skcipher_edesc, hw_desc[0]);
> + edesc = rctx->edesc;
> if (err)
> ecode = caam_jr_strstatus(jrdev, err);
>
> @@ -1024,7 +1034,14 @@ static void skcipher_crypt_done(struct device *jrdev, u32 *desc, u32 err,
>
> kfree(edesc);
>
> - skcipher_request_complete(req, ecode);
> + /*
> + * If no backlog flag, the completion of the request is done
> + * by CAAM, not crypto engine.
> + */
> + if (!jrentry->bklog)
> + skcipher_request_complete(req, ecode);
> + else
> + crypto_finalize_skcipher_request(jrp->engine, req, ecode);
> }
>
> /*
> @@ -1553,6 +1570,7 @@ static struct skcipher_edesc *skcipher_edesc_alloc(struct skcipher_request *req,
> {
> struct crypto_skcipher *skcipher = crypto_skcipher_reqtfm(req);
> struct caam_ctx *ctx = crypto_skcipher_ctx(skcipher);
> + struct caam_skcipher_req_ctx *rctx = skcipher_request_ctx(req);
> struct device *jrdev = ctx->jrdev;
> gfp_t flags = (req->base.flags & CRYPTO_TFM_REQ_MAY_SLEEP) ?
> GFP_KERNEL : GFP_ATOMIC;
> @@ -1653,6 +1671,9 @@ static struct skcipher_edesc *skcipher_edesc_alloc(struct skcipher_request *req,
> desc_bytes);
> edesc->jrentry.base = &req->base;
>
> + rctx->edesc = edesc;
> + rctx->skcipher_op_done = skcipher_crypt_done;
> +
> /* Make sure IV is located in a DMAable area */
> if (ivsize) {
> iv = (u8 *)edesc->sec4_sg + sec4_sg_bytes;
> @@ -1707,13 +1728,37 @@ static struct skcipher_edesc *skcipher_edesc_alloc(struct skcipher_request *req,
> return edesc;
> }
>
> +static int skcipher_do_one_req(struct crypto_engine *engine, void *areq)
> +{
> + struct skcipher_request *req = skcipher_request_cast(areq);
> + struct caam_ctx *ctx = crypto_skcipher_ctx(crypto_skcipher_reqtfm(req));
> + struct caam_skcipher_req_ctx *rctx = skcipher_request_ctx(req);
> + struct caam_jr_request_entry *jrentry;
> + u32 *desc = rctx->edesc->hw_desc;
> + int ret;
> +
> + jrentry = &rctx->edesc->jrentry;
> + jrentry->bklog = true;
> +
> + ret = caam_jr_enqueue_no_bklog(ctx->jrdev, desc,
> + rctx->skcipher_op_done, jrentry);
> +
> + if (ret != -EINPROGRESS) {
> + skcipher_unmap(ctx->jrdev, rctx->edesc, req);
> + kfree(rctx->edesc);
> + } else {
> + ret = 0;
> + }
> +
> + return ret;
While testing this on a i.MX6 DualLite I see -ENOSPC being returned here
after a couple of GiB of data being encrypted (via dm-crypt with LUKS
extension). This results in these messages from crypto_engine:
caam_jr 2101000.jr0: Failed to do one request from queue: -28
And later..
Buffer I/O error on device dm-0, logical block 59392
JBD2: Detected IO errors while flushing file data on dm-0-8
Reproducible with something like this:
echo "testkey" | cryptsetup luksFormat \
--cipher=aes-cbc-essiv:sha256 \
--key-file=- \
--key-size=256 \
/dev/mmcblk1p8
echo "testkey" | cryptsetup open \
--type luks \
--key-file=- \
/dev/mmcblk1p8 data
mkfs.ext4 /dev/mapper/data
mount /dev/mapper/data /mnt
set -x
while [ true ]; do
dd if=/dev/zero of=/mnt/big_file bs=1M count=1024
sync
done
Any ideas?
Regards,
Bastian
> +}
> +
> static inline int skcipher_crypt(struct skcipher_request *req, bool encrypt)
> {
> struct skcipher_edesc *edesc;
> struct crypto_skcipher *skcipher = crypto_skcipher_reqtfm(req);
> struct caam_ctx *ctx = crypto_skcipher_ctx(skcipher);
> struct device *jrdev = ctx->jrdev;
> - struct caam_jr_request_entry *jrentry;
> u32 *desc;
> int ret = 0;
>
> @@ -1727,16 +1772,15 @@ static inline int skcipher_crypt(struct skcipher_request *req, bool encrypt)
>
> /* Create and submit job descriptor*/
> init_skcipher_job(req, edesc, encrypt);
> + desc = edesc->hw_desc;
>
> print_hex_dump_debug("skcipher jobdesc@" __stringify(__LINE__)": ",
> - DUMP_PREFIX_ADDRESS, 16, 4, edesc->hw_desc,
> - desc_bytes(edesc->hw_desc), 1);
> -
> - desc = edesc->hw_desc;
> - jrentry = &edesc->jrentry;
> + DUMP_PREFIX_ADDRESS, 16, 4, desc,
> + desc_bytes(desc), 1);
>
> - ret = caam_jr_enqueue(jrdev, desc, skcipher_crypt_done, jrentry);
> - if (ret != -EINPROGRESS) {
> + ret = caam_jr_enqueue(jrdev, desc, skcipher_crypt_done,
> + &edesc->jrentry);
> + if ((ret != -EINPROGRESS) && (ret != -EBUSY)) {
> skcipher_unmap(jrdev, edesc, req);
> kfree(edesc);
> }
> @@ -3272,7 +3316,9 @@ static int caam_init_common(struct caam_ctx *ctx, struct caam_alg_entry *caam,
>
> dma_addr = dma_map_single_attrs(ctx->jrdev, ctx->sh_desc_enc,
> offsetof(struct caam_ctx,
> - sh_desc_enc_dma),
> + sh_desc_enc_dma) -
> + offsetof(struct caam_ctx,
> + sh_desc_enc),
> ctx->dir, DMA_ATTR_SKIP_CPU_SYNC);
> if (dma_mapping_error(ctx->jrdev, dma_addr)) {
> dev_err(ctx->jrdev, "unable to map key, shared descriptors\n");
> @@ -3282,8 +3328,12 @@ static int caam_init_common(struct caam_ctx *ctx, struct caam_alg_entry *caam,
>
> ctx->sh_desc_enc_dma = dma_addr;
> ctx->sh_desc_dec_dma = dma_addr + offsetof(struct caam_ctx,
> - sh_desc_dec);
> - ctx->key_dma = dma_addr + offsetof(struct caam_ctx, key);
> + sh_desc_dec) -
> + offsetof(struct caam_ctx,
> + sh_desc_enc);
> + ctx->key_dma = dma_addr + offsetof(struct caam_ctx, key) -
> + offsetof(struct caam_ctx,
> + sh_desc_enc);
>
> /* copy descriptor header template value */
> ctx->cdata.algtype = OP_TYPE_CLASS1_ALG | caam->class1_alg_type;
> @@ -3297,6 +3347,11 @@ static int caam_cra_init(struct crypto_skcipher *tfm)
> struct skcipher_alg *alg = crypto_skcipher_alg(tfm);
> struct caam_skcipher_alg *caam_alg =
> container_of(alg, typeof(*caam_alg), skcipher);
> + struct caam_ctx *ctx = crypto_skcipher_ctx(tfm);
> +
> + crypto_skcipher_set_reqsize(tfm, sizeof(struct caam_skcipher_req_ctx));
> +
> + ctx->enginectx.op.do_one_request = skcipher_do_one_req;
>
> return caam_init_common(crypto_skcipher_ctx(tfm), &caam_alg->caam,
> false);
> @@ -3315,7 +3370,8 @@ static int caam_aead_init(struct crypto_aead *tfm)
> static void caam_exit_common(struct caam_ctx *ctx)
> {
> dma_unmap_single_attrs(ctx->jrdev, ctx->sh_desc_enc_dma,
> - offsetof(struct caam_ctx, sh_desc_enc_dma),
> + offsetof(struct caam_ctx, sh_desc_enc_dma) -
> + offsetof(struct caam_ctx, sh_desc_enc),
> ctx->dir, DMA_ATTR_SKIP_CPU_SYNC);
> caam_jr_free(ctx->jrdev);
> }
> diff --git a/drivers/crypto/caam/intern.h b/drivers/crypto/caam/intern.h
> index 58be66c..31abb94 100644
> --- a/drivers/crypto/caam/intern.h
> +++ b/drivers/crypto/caam/intern.h
> @@ -12,6 +12,7 @@
>
> #include "ctrl.h"
> #include "regs.h"
> +#include <crypto/engine.h>
>
> /* Currently comes from Kconfig param as a ^2 (driver-required) */
> #define JOBR_DEPTH (1 << CONFIG_CRYPTO_DEV_FSL_CAAM_RINGSIZE)
> @@ -61,6 +62,7 @@ struct caam_drv_private_jr {
> int out_ring_read_index; /* Output index "tail" */
> int tail; /* entinfo (s/w ring) tail index */
> void *outring; /* Base of output ring, DMA-safe */
> + struct crypto_engine *engine;
> };
>
> /*
> diff --git a/drivers/crypto/caam/jr.c b/drivers/crypto/caam/jr.c
> index 544cafa..5c55d3d 100644
> --- a/drivers/crypto/caam/jr.c
> +++ b/drivers/crypto/caam/jr.c
> @@ -62,6 +62,15 @@ static void unregister_algs(void)
> mutex_unlock(&algs_lock);
> }
>
> +static void caam_jr_crypto_engine_exit(void *data)
> +{
> + struct device *jrdev = data;
> + struct caam_drv_private_jr *jrpriv = dev_get_drvdata(jrdev);
> +
> + /* Free the resources of crypto-engine */
> + crypto_engine_exit(jrpriv->engine);
> +}
> +
> static int caam_reset_hw_jr(struct device *dev)
> {
> struct caam_drv_private_jr *jrp = dev_get_drvdata(dev);
> @@ -418,10 +427,23 @@ int caam_jr_enqueue_no_bklog(struct device *dev, u32 *desc,
> }
> EXPORT_SYMBOL(caam_jr_enqueue_no_bklog);
>
> +static int transfer_request_to_engine(struct crypto_engine *engine,
> + struct crypto_async_request *req)
> +{
> + switch (crypto_tfm_alg_type(req->tfm)) {
> + case CRYPTO_ALG_TYPE_SKCIPHER:
> + return crypto_transfer_skcipher_request_to_engine(engine,
> + skcipher_request_cast(req));
> + default:
> + return -EINVAL;
> + }
> +}
> +
> /**
> * caam_jr_enqueue() - Enqueue a job descriptor head. Returns -EINPROGRESS
> - * if OK, -ENOSPC if the queue is full, -EIO if it cannot map the caller's
> - * descriptor.
> + * if OK, -EBUSY if request is backlogged, -ENOSPC if the queue is full,
> + * -EIO if it cannot map the caller's descriptor, -EINVAL if crypto_tfm
> + * not supported by crypto_engine.
> * @dev: device of the job ring to be used. This device should have
> * been assigned prior by caam_jr_register().
> * @desc: points to a job descriptor that execute our request. All
> @@ -451,7 +473,12 @@ int caam_jr_enqueue(struct device *dev, u32 *desc,
> u32 status, void *areq),
> void *areq)
> {
> + struct caam_drv_private_jr *jrpriv = dev_get_drvdata(dev);
> struct caam_jr_request_entry *jrentry = areq;
> + struct crypto_async_request *req = jrentry->base;
> +
> + if (req->flags & CRYPTO_TFM_REQ_MAY_BACKLOG)
> + return transfer_request_to_engine(jrpriv->engine, req);
>
> return caam_jr_enqueue_no_bklog(dev, desc, cbk, jrentry);
> }
> @@ -577,6 +604,26 @@ static int caam_jr_probe(struct platform_device *pdev)
> return error;
> }
>
> + /* Initialize crypto engine */
> + jrpriv->engine = crypto_engine_alloc_init(jrdev, false);
> + if (!jrpriv->engine) {
> + dev_err(jrdev, "Could not init crypto-engine\n");
> + return -ENOMEM;
> + }
> +
> + /* Start crypto engine */
> + error = crypto_engine_start(jrpriv->engine);
> + if (error) {
> + dev_err(jrdev, "Could not start crypto-engine\n");
> + crypto_engine_exit(jrpriv->engine);
> + return error;
> + }
> +
> + error = devm_add_action_or_reset(jrdev, caam_jr_crypto_engine_exit,
> + jrdev);
> + if (error)
> + return error;
> +
> /* Identify the interrupt */
> jrpriv->irq = irq_of_parse_and_map(nprop, 0);
> if (!jrpriv->irq) {
>
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
Hi,
On 12/10/2019 5:27 PM, Bastian Krause wrote:
>
> Hi Iulia,
>
> On 11/17/19 11:30 PM, Iuliana Prodan wrote:
>> Integrate crypto_engine into CAAM, to make use of the engine queue.
>> Add support for SKCIPHER algorithms.
>>
>> This is intended to be used for CAAM backlogging support.
>> The requests, with backlog flag (e.g. from dm-crypt) will be listed
>> into crypto-engine queue and processed by CAAM when free.
>> This changes the return codes for caam_jr_enqueue:
>> -EINPROGRESS if OK, -EBUSY if request is backlogged,
>> -ENOSPC if the queue is full, -EIO if it cannot map the caller's
>> descriptor, -EINVAL if crypto_tfm not supported by crypto_engine.
>>
>> Signed-off-by: Iuliana Prodan <[email protected]>
>> Signed-off-by: Franck LENORMAND <[email protected]>
>> Reviewed-by: Horia Geant? <[email protected]>
>> ---
>> drivers/crypto/caam/Kconfig | 1 +
>> drivers/crypto/caam/caamalg.c | 84 +++++++++++++++++++++++++++++++++++--------
>> drivers/crypto/caam/intern.h | 2 ++
>> drivers/crypto/caam/jr.c | 51 ++++++++++++++++++++++++--
>> 4 files changed, 122 insertions(+), 16 deletions(-)
>>
>> diff --git a/drivers/crypto/caam/Kconfig b/drivers/crypto/caam/Kconfig
>> index 87053e4..1930e19 100644
>> --- a/drivers/crypto/caam/Kconfig
>> +++ b/drivers/crypto/caam/Kconfig
>> @@ -33,6 +33,7 @@ config CRYPTO_DEV_FSL_CAAM_DEBUG
>>
...
>>
>> +static int skcipher_do_one_req(struct crypto_engine *engine, void *areq)
>> +{
>> + struct skcipher_request *req = skcipher_request_cast(areq);
>> + struct caam_ctx *ctx = crypto_skcipher_ctx(crypto_skcipher_reqtfm(req));
>> + struct caam_skcipher_req_ctx *rctx = skcipher_request_ctx(req);
>> + struct caam_jr_request_entry *jrentry;
>> + u32 *desc = rctx->edesc->hw_desc;
>> + int ret;
>> +
>> + jrentry = &rctx->edesc->jrentry;
>> + jrentry->bklog = true;
>> +
>> + ret = caam_jr_enqueue_no_bklog(ctx->jrdev, desc,
>> + rctx->skcipher_op_done, jrentry);
>> +
>> + if (ret != -EINPROGRESS) {
>> + skcipher_unmap(ctx->jrdev, rctx->edesc, req);
>> + kfree(rctx->edesc);
>> + } else {
>> + ret = 0;
>> + }
>> +
>> + return ret;
>
> While testing this on a i.MX6 DualLite I see -ENOSPC being returned here
> after a couple of GiB of data being encrypted (via dm-crypt with LUKS
> extension). This results in these messages from crypto_engine:
>
> caam_jr 2101000.jr0: Failed to do one request from queue: -28
>
> And later..
>
> Buffer I/O error on device dm-0, logical block 59392
> JBD2: Detected IO errors while flushing file data on dm-0-8
>
> Reproducible with something like this:
>
> echo "testkey" | cryptsetup luksFormat \
> --cipher=aes-cbc-essiv:sha256 \
> --key-file=- \
> --key-size=256 \
> /dev/mmcblk1p8
> echo "testkey" | cryptsetup open \
> --type luks \
> --key-file=- \
> /dev/mmcblk1p8 data
>
> mkfs.ext4 /dev/mapper/data
> mount /dev/mapper/data /mnt
>
> set -x
> while [ true ]; do
> dd if=/dev/zero of=/mnt/big_file bs=1M count=1024
> sync
> done
>
> Any ideas?
>
Thanks for testing this!
I reproduced this issue on imx6dl, _but_ only with the bypass sw queue
patch. It only reproduces on some targets, e.g. on imx7d I don't get the
-ENOSPC error. So, I believe there is a timing issue between
crypto-engine and CAAM driver, both sending requests to CAAM hw.
I'm debugging this and I'll let you know my findings.
Best regards,
Iulia
Hi,
On 12/11/19 1:20 PM, Iuliana Prodan wrote:
> On 12/10/2019 5:27 PM, Bastian Krause wrote:
>> On 11/17/19 11:30 PM, Iuliana Prodan wrote:
>>> Integrate crypto_engine into CAAM, to make use of the engine queue.
>>> Add support for SKCIPHER algorithms.
>>>
>>> This is intended to be used for CAAM backlogging support.
>>> The requests, with backlog flag (e.g. from dm-crypt) will be listed
>>> into crypto-engine queue and processed by CAAM when free.
>>> This changes the return codes for caam_jr_enqueue:
>>> -EINPROGRESS if OK, -EBUSY if request is backlogged,
>>> -ENOSPC if the queue is full, -EIO if it cannot map the caller's
>>> descriptor, -EINVAL if crypto_tfm not supported by crypto_engine.
>>>
>>> Signed-off-by: Iuliana Prodan <[email protected]>
>>> Signed-off-by: Franck LENORMAND <[email protected]>
>>> Reviewed-by: Horia Geant? <[email protected]>
>>> ---
>>> drivers/crypto/caam/Kconfig | 1 +
>>> drivers/crypto/caam/caamalg.c | 84 +++++++++++++++++++++++++++++++++++--------
>>> drivers/crypto/caam/intern.h | 2 ++
>>> drivers/crypto/caam/jr.c | 51 ++++++++++++++++++++++++--
>>> 4 files changed, 122 insertions(+), 16 deletions(-)
>>>
>>> diff --git a/drivers/crypto/caam/Kconfig b/drivers/crypto/caam/Kconfig
>>> index 87053e4..1930e19 100644
>>> --- a/drivers/crypto/caam/Kconfig
>>> +++ b/drivers/crypto/caam/Kconfig
>>> @@ -33,6 +33,7 @@ config CRYPTO_DEV_FSL_CAAM_DEBUG
>>>
> ...
>>>
>>> +static int skcipher_do_one_req(struct crypto_engine *engine, void *areq)
>>> +{
>>> + struct skcipher_request *req = skcipher_request_cast(areq);
>>> + struct caam_ctx *ctx = crypto_skcipher_ctx(crypto_skcipher_reqtfm(req));
>>> + struct caam_skcipher_req_ctx *rctx = skcipher_request_ctx(req);
>>> + struct caam_jr_request_entry *jrentry;
>>> + u32 *desc = rctx->edesc->hw_desc;
>>> + int ret;
>>> +
>>> + jrentry = &rctx->edesc->jrentry;
>>> + jrentry->bklog = true;
>>> +
>>> + ret = caam_jr_enqueue_no_bklog(ctx->jrdev, desc,
>>> + rctx->skcipher_op_done, jrentry);
>>> +
>>> + if (ret != -EINPROGRESS) {
>>> + skcipher_unmap(ctx->jrdev, rctx->edesc, req);
>>> + kfree(rctx->edesc);
>>> + } else {
>>> + ret = 0;
>>> + }
>>> +
>>> + return ret;
>>
>> While testing this on a i.MX6 DualLite I see -ENOSPC being returned here
>> after a couple of GiB of data being encrypted (via dm-crypt with LUKS
>> extension). This results in these messages from crypto_engine:
>>
>> caam_jr 2101000.jr0: Failed to do one request from queue: -28
>>
>> And later..
>>
>> Buffer I/O error on device dm-0, logical block 59392
>> JBD2: Detected IO errors while flushing file data on dm-0-8
>>
>> Reproducible with something like this:
>>
>> echo "testkey" | cryptsetup luksFormat \
>> --cipher=aes-cbc-essiv:sha256 \
>> --key-file=- \
>> --key-size=256 \
>> /dev/mmcblk1p8
>> echo "testkey" | cryptsetup open \
>> --type luks \
>> --key-file=- \
>> /dev/mmcblk1p8 data
>>
>> mkfs.ext4 /dev/mapper/data
>> mount /dev/mapper/data /mnt
>>
>> set -x
>> while [ true ]; do
>> dd if=/dev/zero of=/mnt/big_file bs=1M count=1024
>> sync
>> done
>>
>> Any ideas?
>>
>
> Thanks for testing this!
Sure :)
> I reproduced this issue on imx6dl, _but_ only with the bypass sw queue
> patch. It only reproduces on some targets, e.g. on imx7d I don't get the
> -ENOSPC error. So, I believe there is a timing issue between
> crypto-engine and CAAM driver, both sending requests to CAAM hw.
> I'm debugging this and I'll let you know my findings.
I can't even use this without the "crypto: caam - bypass crypto-engine
sw queue, if empty". The mkfs.ext4 command does not even finish and I
see hung task warnings. Am I holding it wrong?
Regards,
Bastian
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |