Hello
The rockchip crypto driver is broken and do not pass self-tests.
This serie's goal is to permit to become usable and pass self-tests.
This whole serie is tested on a rk3328-rock64 with selftests (with
CONFIG_CRYPTO_MANAGER_EXTRA_TESTS=y)
Regards
Corentin Labbe (16):
crypto: rockchip: use dev_err for error message about interrupt
crypto: rockchip: do not use uninit variable
crypto: rockchip: do not do custom power management
crypto: rockchip: fix privete/private typo
crypto: rockchip: do not store mode globally
crypto: rockchip: add fallback for cipher
crypto: rockchip: add fallback for ahash
crypto: rockchip: better handle cipher key
crypto: rockchip: remove non-aligned handling
crypto: rockchip: rework by using crypto_engine
crypto: rockhip: do not handle dma clock
ARM: dts: rk3288: crypto do not need dma clock
crypto: rockchip: rewrite type
crypto: rockchip: add debugfs
arm64: dts: rockchip: add rk3328 crypto node
crypto: rockchip: add myself as maintainer
MAINTAINERS | 6 +
arch/arm/boot/dts/rk3288.dtsi | 4 +-
arch/arm64/boot/dts/rockchip/rk3328.dtsi | 12 +
drivers/crypto/Kconfig | 10 +
drivers/crypto/rockchip/rk3288_crypto.c | 283 +++--------
drivers/crypto/rockchip/rk3288_crypto.h | 68 ++-
drivers/crypto/rockchip/rk3288_crypto_ahash.c | 214 +++++----
.../crypto/rockchip/rk3288_crypto_skcipher.c | 449 +++++++++++-------
8 files changed, 533 insertions(+), 513 deletions(-)
--
2.34.1
The clock enable/disable at tfm init/exit is fragile,
if 2 tfm are init in the same time and one is removed just after,
it will leave the hardware uncloked even if a user remains.
Instead simply enable clocks at probe time.
We will do PM later.
Fixes: ce0183cb6464b ("crypto: rockchip - switch to skcipher API")
Signed-off-by: Corentin Labbe <[email protected]>
---
drivers/crypto/rockchip/rk3288_crypto.c | 4 ++--
drivers/crypto/rockchip/rk3288_crypto.h | 2 --
drivers/crypto/rockchip/rk3288_crypto_ahash.c | 3 +--
drivers/crypto/rockchip/rk3288_crypto_skcipher.c | 5 +++--
4 files changed, 6 insertions(+), 8 deletions(-)
diff --git a/drivers/crypto/rockchip/rk3288_crypto.c b/drivers/crypto/rockchip/rk3288_crypto.c
index 21d3f1458584..4cff49b82983 100644
--- a/drivers/crypto/rockchip/rk3288_crypto.c
+++ b/drivers/crypto/rockchip/rk3288_crypto.c
@@ -394,8 +394,7 @@ static int rk_crypto_probe(struct platform_device *pdev)
rk_crypto_done_task_cb, (unsigned long)crypto_info);
crypto_init_queue(&crypto_info->queue, 50);
- crypto_info->enable_clk = rk_crypto_enable_clk;
- crypto_info->disable_clk = rk_crypto_disable_clk;
+ rk_crypto_enable_clk(crypto_info);
crypto_info->load_data = rk_load_data;
crypto_info->unload_data = rk_unload_data;
crypto_info->enqueue = rk_crypto_enqueue;
@@ -422,6 +421,7 @@ static int rk_crypto_remove(struct platform_device *pdev)
struct rk_crypto_info *crypto_tmp = platform_get_drvdata(pdev);
rk_crypto_unregister();
+ rk_crypto_disable_clk(crypto_tmp);
tasklet_kill(&crypto_tmp->done_task);
tasklet_kill(&crypto_tmp->queue_task);
return 0;
diff --git a/drivers/crypto/rockchip/rk3288_crypto.h b/drivers/crypto/rockchip/rk3288_crypto.h
index 97278c2574ff..2fa7131e4060 100644
--- a/drivers/crypto/rockchip/rk3288_crypto.h
+++ b/drivers/crypto/rockchip/rk3288_crypto.h
@@ -220,8 +220,6 @@ struct rk_crypto_info {
int (*start)(struct rk_crypto_info *dev);
int (*update)(struct rk_crypto_info *dev);
void (*complete)(struct crypto_async_request *base, int err);
- int (*enable_clk)(struct rk_crypto_info *dev);
- void (*disable_clk)(struct rk_crypto_info *dev);
int (*load_data)(struct rk_crypto_info *dev,
struct scatterlist *sg_src,
struct scatterlist *sg_dst);
diff --git a/drivers/crypto/rockchip/rk3288_crypto_ahash.c b/drivers/crypto/rockchip/rk3288_crypto_ahash.c
index ed03058497bc..49017d1fb510 100644
--- a/drivers/crypto/rockchip/rk3288_crypto_ahash.c
+++ b/drivers/crypto/rockchip/rk3288_crypto_ahash.c
@@ -301,7 +301,7 @@ static int rk_cra_hash_init(struct crypto_tfm *tfm)
sizeof(struct rk_ahash_rctx) +
crypto_ahash_reqsize(tctx->fallback_tfm));
- return tctx->dev->enable_clk(tctx->dev);
+ return 0;
}
static void rk_cra_hash_exit(struct crypto_tfm *tfm)
@@ -309,7 +309,6 @@ static void rk_cra_hash_exit(struct crypto_tfm *tfm)
struct rk_ahash_ctx *tctx = crypto_tfm_ctx(tfm);
free_page((unsigned long)tctx->dev->addr_vir);
- return tctx->dev->disable_clk(tctx->dev);
}
struct rk_crypto_tmp rk_ahash_sha1 = {
diff --git a/drivers/crypto/rockchip/rk3288_crypto_skcipher.c b/drivers/crypto/rockchip/rk3288_crypto_skcipher.c
index 5bbf0d2722e1..8c44a19eab75 100644
--- a/drivers/crypto/rockchip/rk3288_crypto_skcipher.c
+++ b/drivers/crypto/rockchip/rk3288_crypto_skcipher.c
@@ -388,8 +388,10 @@ static int rk_ablk_init_tfm(struct crypto_skcipher *tfm)
ctx->dev->update = rk_ablk_rx;
ctx->dev->complete = rk_crypto_complete;
ctx->dev->addr_vir = (char *)__get_free_page(GFP_KERNEL);
+ if (!ctx->dev->addr_vir)
+ return -ENOMEM;
- return ctx->dev->addr_vir ? ctx->dev->enable_clk(ctx->dev) : -ENOMEM;
+ return 0;
}
static void rk_ablk_exit_tfm(struct crypto_skcipher *tfm)
@@ -397,7 +399,6 @@ static void rk_ablk_exit_tfm(struct crypto_skcipher *tfm)
struct rk_cipher_ctx *ctx = crypto_skcipher_ctx(tfm);
free_page((unsigned long)ctx->dev->addr_vir);
- ctx->dev->disable_clk(ctx->dev);
}
struct rk_crypto_tmp rk_ecb_aes_alg = {
--
2.34.1
The hardware does not handle 0 size length request, let's add a
fallback.
Furthermore fallback will be used for all unaligned case the hardware
cannot handle.
Fixes: ce0183cb6464b ("crypto: rockchip - switch to skcipher API")
Signed-off-by: Corentin Labbe <[email protected]>
---
drivers/crypto/rockchip/rk3288_crypto.h | 2 +
.../crypto/rockchip/rk3288_crypto_skcipher.c | 100 +++++++++++++++---
2 files changed, 89 insertions(+), 13 deletions(-)
diff --git a/drivers/crypto/rockchip/rk3288_crypto.h b/drivers/crypto/rockchip/rk3288_crypto.h
index c919d9a43a08..8b1e15d8ddc6 100644
--- a/drivers/crypto/rockchip/rk3288_crypto.h
+++ b/drivers/crypto/rockchip/rk3288_crypto.h
@@ -246,10 +246,12 @@ struct rk_cipher_ctx {
struct rk_crypto_info *dev;
unsigned int keylen;
u8 iv[AES_BLOCK_SIZE];
+ struct crypto_skcipher *fallback_tfm;
};
struct rk_cipher_rctx {
u32 mode;
+ struct skcipher_request fallback_req; // keep at the end
};
enum alg_type {
diff --git a/drivers/crypto/rockchip/rk3288_crypto_skcipher.c b/drivers/crypto/rockchip/rk3288_crypto_skcipher.c
index bbd0bf52bf07..060d52f18b31 100644
--- a/drivers/crypto/rockchip/rk3288_crypto_skcipher.c
+++ b/drivers/crypto/rockchip/rk3288_crypto_skcipher.c
@@ -13,6 +13,63 @@
#define RK_CRYPTO_DEC BIT(0)
+static int rk_cipher_need_fallback(struct skcipher_request *req)
+{
+ struct scatterlist *sgs, *sgd;
+
+ if (!req->cryptlen)
+ return true;
+
+ sgs = req->src;
+ while (sgs) {
+ if (!IS_ALIGNED(sgs->offset, sizeof(u32))) {
+ return true;
+ }
+ if (sgs->length % 16) {
+ return true;
+ }
+ sgs = sg_next(sgs);
+ }
+ sgd = req->dst;
+ while (sgd) {
+ if (!IS_ALIGNED(sgd->offset, sizeof(u32))) {
+ return true;
+ }
+ if (sgd->length % 16) {
+ return true;
+ }
+ sgd = sg_next(sgd);
+ }
+ sgs = req->src;
+ sgd = req->dst;
+ while (sgs && sgd) {
+ if (sgs->length != sgd->length)
+ return true;
+ sgs = sg_next(sgs);
+ sgd = sg_next(sgd);
+ }
+ return false;
+}
+
+static int rk_cipher_fallback(struct skcipher_request *areq)
+{
+ struct crypto_skcipher *tfm = crypto_skcipher_reqtfm(areq);
+ struct rk_cipher_ctx *op = crypto_skcipher_ctx(tfm);
+ struct rk_cipher_rctx *rctx = skcipher_request_ctx(areq);
+ int err;
+
+ skcipher_request_set_tfm(&rctx->fallback_req, op->fallback_tfm);
+ skcipher_request_set_callback(&rctx->fallback_req, areq->base.flags,
+ areq->base.complete, areq->base.data);
+ skcipher_request_set_crypt(&rctx->fallback_req, areq->src, areq->dst,
+ areq->cryptlen, areq->iv);
+ if (rctx->mode & RK_CRYPTO_DEC)
+ err = crypto_skcipher_decrypt(&rctx->fallback_req);
+ else
+ err = crypto_skcipher_encrypt(&rctx->fallback_req);
+ return err;
+}
+
static void rk_crypto_complete(struct crypto_async_request *base, int err)
{
if (base->complete)
@@ -22,10 +79,10 @@ static void rk_crypto_complete(struct crypto_async_request *base, int err)
static int rk_handle_req(struct rk_crypto_info *dev,
struct skcipher_request *req)
{
- if (!IS_ALIGNED(req->cryptlen, dev->align_size))
- return -EINVAL;
- else
- return dev->enqueue(dev, &req->base);
+ if (rk_cipher_need_fallback(req))
+ return rk_cipher_fallback(req);
+
+ return dev->enqueue(dev, &req->base);
}
static int rk_aes_setkey(struct crypto_skcipher *cipher,
@@ -39,7 +96,8 @@ static int rk_aes_setkey(struct crypto_skcipher *cipher,
return -EINVAL;
ctx->keylen = keylen;
memcpy_toio(ctx->dev->reg + RK_CRYPTO_AES_KEY_0, key, keylen);
- return 0;
+
+ return crypto_skcipher_setkey(ctx->fallback_tfm, key, keylen);
}
static int rk_des_setkey(struct crypto_skcipher *cipher,
@@ -54,7 +112,8 @@ static int rk_des_setkey(struct crypto_skcipher *cipher,
ctx->keylen = keylen;
memcpy_toio(ctx->dev->reg + RK_CRYPTO_TDES_KEY1_0, key, keylen);
- return 0;
+
+ return crypto_skcipher_setkey(ctx->fallback_tfm, key, keylen);
}
static int rk_tdes_setkey(struct crypto_skcipher *cipher,
@@ -69,7 +128,7 @@ static int rk_tdes_setkey(struct crypto_skcipher *cipher,
ctx->keylen = keylen;
memcpy_toio(ctx->dev->reg + RK_CRYPTO_TDES_KEY1_0, key, keylen);
- return 0;
+ return crypto_skcipher_setkey(ctx->fallback_tfm, key, keylen);
}
static int rk_aes_ecb_encrypt(struct skcipher_request *req)
@@ -394,6 +453,7 @@ static int rk_ablk_init_tfm(struct crypto_skcipher *tfm)
{
struct rk_cipher_ctx *ctx = crypto_skcipher_ctx(tfm);
struct skcipher_alg *alg = crypto_skcipher_alg(tfm);
+ const char *name = crypto_tfm_alg_name(&tfm->base);
struct rk_crypto_tmp *algt;
algt = container_of(alg, struct rk_crypto_tmp, alg.skcipher);
@@ -407,6 +467,20 @@ static int rk_ablk_init_tfm(struct crypto_skcipher *tfm)
if (!ctx->dev->addr_vir)
return -ENOMEM;
+ ctx->fallback_tfm = crypto_alloc_skcipher(name, 0, CRYPTO_ALG_NEED_FALLBACK);
+ if (IS_ERR(ctx->fallback_tfm)) {
+ dev_err(ctx->dev->dev, "ERROR: Cannot allocate fallback for %s %ld\n",
+ name, PTR_ERR(ctx->fallback_tfm));
+ return PTR_ERR(ctx->fallback_tfm);
+ }
+
+ dev_info(ctx->dev->dev, "Fallback for %s is %s\n",
+ crypto_tfm_alg_driver_name(&tfm->base),
+ crypto_tfm_alg_driver_name(crypto_skcipher_tfm(ctx->fallback_tfm)));
+
+ tfm->reqsize = sizeof(struct rk_cipher_rctx) +
+ crypto_skcipher_reqsize(ctx->fallback_tfm);
+
return 0;
}
@@ -423,7 +497,7 @@ struct rk_crypto_tmp rk_ecb_aes_alg = {
.base.cra_name = "ecb(aes)",
.base.cra_driver_name = "ecb-aes-rk",
.base.cra_priority = 300,
- .base.cra_flags = CRYPTO_ALG_ASYNC,
+ .base.cra_flags = CRYPTO_ALG_ASYNC | CRYPTO_ALG_NEED_FALLBACK,
.base.cra_blocksize = AES_BLOCK_SIZE,
.base.cra_ctxsize = sizeof(struct rk_cipher_ctx),
.base.cra_alignmask = 0x0f,
@@ -445,7 +519,7 @@ struct rk_crypto_tmp rk_cbc_aes_alg = {
.base.cra_name = "cbc(aes)",
.base.cra_driver_name = "cbc-aes-rk",
.base.cra_priority = 300,
- .base.cra_flags = CRYPTO_ALG_ASYNC,
+ .base.cra_flags = CRYPTO_ALG_ASYNC | CRYPTO_ALG_NEED_FALLBACK,
.base.cra_blocksize = AES_BLOCK_SIZE,
.base.cra_ctxsize = sizeof(struct rk_cipher_ctx),
.base.cra_alignmask = 0x0f,
@@ -468,7 +542,7 @@ struct rk_crypto_tmp rk_ecb_des_alg = {
.base.cra_name = "ecb(des)",
.base.cra_driver_name = "ecb-des-rk",
.base.cra_priority = 300,
- .base.cra_flags = CRYPTO_ALG_ASYNC,
+ .base.cra_flags = CRYPTO_ALG_ASYNC | CRYPTO_ALG_NEED_FALLBACK,
.base.cra_blocksize = DES_BLOCK_SIZE,
.base.cra_ctxsize = sizeof(struct rk_cipher_ctx),
.base.cra_alignmask = 0x07,
@@ -490,7 +564,7 @@ struct rk_crypto_tmp rk_cbc_des_alg = {
.base.cra_name = "cbc(des)",
.base.cra_driver_name = "cbc-des-rk",
.base.cra_priority = 300,
- .base.cra_flags = CRYPTO_ALG_ASYNC,
+ .base.cra_flags = CRYPTO_ALG_ASYNC | CRYPTO_ALG_NEED_FALLBACK,
.base.cra_blocksize = DES_BLOCK_SIZE,
.base.cra_ctxsize = sizeof(struct rk_cipher_ctx),
.base.cra_alignmask = 0x07,
@@ -513,7 +587,7 @@ struct rk_crypto_tmp rk_ecb_des3_ede_alg = {
.base.cra_name = "ecb(des3_ede)",
.base.cra_driver_name = "ecb-des3-ede-rk",
.base.cra_priority = 300,
- .base.cra_flags = CRYPTO_ALG_ASYNC,
+ .base.cra_flags = CRYPTO_ALG_ASYNC | CRYPTO_ALG_NEED_FALLBACK,
.base.cra_blocksize = DES_BLOCK_SIZE,
.base.cra_ctxsize = sizeof(struct rk_cipher_ctx),
.base.cra_alignmask = 0x07,
@@ -535,7 +609,7 @@ struct rk_crypto_tmp rk_cbc_des3_ede_alg = {
.base.cra_name = "cbc(des3_ede)",
.base.cra_driver_name = "cbc-des3-ede-rk",
.base.cra_priority = 300,
- .base.cra_flags = CRYPTO_ALG_ASYNC,
+ .base.cra_flags = CRYPTO_ALG_ASYNC | CRYPTO_ALG_NEED_FALLBACK,
.base.cra_blocksize = DES_BLOCK_SIZE,
.base.cra_ctxsize = sizeof(struct rk_cipher_ctx),
.base.cra_alignmask = 0x07,
--
2.34.1
Adds a fallback for all case hardware cannot handle.
Fixes: ce0183cb6464b ("crypto: rockchip - switch to skcipher API")
Signed-off-by: Corentin Labbe <[email protected]>
---
drivers/crypto/rockchip/rk3288_crypto_ahash.c | 37 +++++++++++++++++++
1 file changed, 37 insertions(+)
diff --git a/drivers/crypto/rockchip/rk3288_crypto_ahash.c b/drivers/crypto/rockchip/rk3288_crypto_ahash.c
index 49017d1fb510..417cf4487514 100644
--- a/drivers/crypto/rockchip/rk3288_crypto_ahash.c
+++ b/drivers/crypto/rockchip/rk3288_crypto_ahash.c
@@ -16,6 +16,40 @@
* so we put the fixed hash out when met zero message.
*/
+static bool rk_ahash_need_fallback(struct ahash_request *req)
+{
+ struct scatterlist *sg;
+
+ sg = req->src;
+ while (sg) {
+ if (!IS_ALIGNED(sg->offset, sizeof(u32))) {
+ return true;
+ }
+ if (sg->length % 4) {
+ return true;
+ }
+ sg = sg_next(sg);
+ }
+ return false;
+}
+
+static int rk_ahash_digest_fb(struct ahash_request *areq)
+{
+ struct rk_ahash_rctx *rctx = ahash_request_ctx(areq);
+ struct crypto_ahash *tfm = crypto_ahash_reqtfm(areq);
+ struct rk_ahash_ctx *tfmctx = crypto_ahash_ctx(tfm);
+
+ ahash_request_set_tfm(&rctx->fallback_req, tfmctx->fallback_tfm);
+ rctx->fallback_req.base.flags = areq->base.flags &
+ CRYPTO_TFM_REQ_MAY_SLEEP;
+
+ rctx->fallback_req.nbytes = areq->nbytes;
+ rctx->fallback_req.src = areq->src;
+ rctx->fallback_req.result = areq->result;
+
+ return crypto_ahash_digest(&rctx->fallback_req);
+}
+
static int zero_message_process(struct ahash_request *req)
{
struct crypto_ahash *tfm = crypto_ahash_reqtfm(req);
@@ -167,6 +201,9 @@ static int rk_ahash_digest(struct ahash_request *req)
struct rk_ahash_ctx *tctx = crypto_tfm_ctx(req->base.tfm);
struct rk_crypto_info *dev = tctx->dev;
+ if (rk_ahash_need_fallback(req))
+ return rk_ahash_digest_fb(req);
+
if (!req->nbytes)
return zero_message_process(req);
else
--
2.34.1
The DMA clock is handled by the DMA controller, so the crypto does not
have to touch it.
Signed-off-by: Corentin Labbe <[email protected]>
---
drivers/crypto/rockchip/rk3288_crypto.c | 16 +---------------
drivers/crypto/rockchip/rk3288_crypto.h | 1 -
2 files changed, 1 insertion(+), 16 deletions(-)
diff --git a/drivers/crypto/rockchip/rk3288_crypto.c b/drivers/crypto/rockchip/rk3288_crypto.c
index 94ef1283789f..645855d2651b 100644
--- a/drivers/crypto/rockchip/rk3288_crypto.c
+++ b/drivers/crypto/rockchip/rk3288_crypto.c
@@ -40,15 +40,8 @@ static int rk_crypto_enable_clk(struct rk_crypto_info *dev)
__func__, __LINE__);
goto err_hclk;
}
- err = clk_prepare_enable(dev->dmaclk);
- if (err) {
- dev_err(dev->dev, "[%s:%d], Couldn't enable clock dmaclk\n",
- __func__, __LINE__);
- goto err_dmaclk;
- }
+
return err;
-err_dmaclk:
- clk_disable_unprepare(dev->hclk);
err_hclk:
clk_disable_unprepare(dev->aclk);
err_aclk:
@@ -59,7 +52,6 @@ static int rk_crypto_enable_clk(struct rk_crypto_info *dev)
static void rk_crypto_disable_clk(struct rk_crypto_info *dev)
{
- clk_disable_unprepare(dev->dmaclk);
clk_disable_unprepare(dev->hclk);
clk_disable_unprepare(dev->aclk);
clk_disable_unprepare(dev->sclk);
@@ -199,12 +191,6 @@ static int rk_crypto_probe(struct platform_device *pdev)
goto err_crypto;
}
- crypto_info->dmaclk = devm_clk_get(&pdev->dev, "apb_pclk");
- if (IS_ERR(crypto_info->dmaclk)) {
- err = PTR_ERR(crypto_info->dmaclk);
- goto err_crypto;
- }
-
crypto_info->irq = platform_get_irq(pdev, 0);
if (crypto_info->irq < 0) {
dev_err(&pdev->dev, "control Interrupt is not available.\n");
diff --git a/drivers/crypto/rockchip/rk3288_crypto.h b/drivers/crypto/rockchip/rk3288_crypto.h
index c741e97057dc..963fbfc4d14e 100644
--- a/drivers/crypto/rockchip/rk3288_crypto.h
+++ b/drivers/crypto/rockchip/rk3288_crypto.h
@@ -191,7 +191,6 @@ struct rk_crypto_info {
struct clk *aclk;
struct clk *hclk;
struct clk *sclk;
- struct clk *dmaclk;
struct reset_control *rst;
void __iomem *reg;
int irq;
--
2.34.1
Nobody is set as maintainer of rockchip crypto, I propose to do it as I
have already reworked lot of this code.
Signed-off-by: Corentin Labbe <[email protected]>
---
MAINTAINERS | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index db8052bc1d26..b1b2303ac285 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -16828,6 +16828,12 @@ F: Documentation/ABI/*/sysfs-driver-hid-roccat*
F: drivers/hid/hid-roccat*
F: include/linux/hid-roccat*
+ROCKCHIP CRYPTO DRIVERS
+M: Corentin Labbe <[email protected]>
+L: [email protected]
+S: Maintained
+F: drivers/crypto/rockchip/
+
ROCKCHIP I2S TDM DRIVER
M: Nicolas Frattaroli <[email protected]>
L: [email protected]
--
2.34.1
On Mon, Feb 28, 2022 at 07:40:21PM +0000, Corentin Labbe wrote:
> The rockchip crypto driver is broken and do not pass self-tests.
> This serie's goal is to permit to become usable and pass self-tests.
>
> This whole serie is tested on a rk3328-rock64 with selftests (with
> CONFIG_CRYPTO_MANAGER_EXTRA_TESTS=y)
I previously noticed this breakage on rk3288 but never got time to
investigate (disabling the driver was quicker).
This series fixes everything on rk3288 as well, thanks!
I hit the same warnings as the kernel test robot as well as a missing
new kconfig dependency (see separate reply to patch 10), but this is
Tested-by: John Keeping <[email protected]>
Regards,
John