Skip to contentSkip to conversation

crypto: sun8i-ce - Add AES-XTS for the H616

PATCH Superseded Arun Iyengar 2 participants 2 messages Nov 10

Subsystems (3)

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 crypto: sun8i-ce - Add AES-XTS for the H616

The H616 crypto engine has a separate RAES unit (algorithm type 0x30) that does AES-ECB, AES-CBC and AES-XTS. Its XTS mode (symmetric mode 0b1001) takes the two keys back to back at the key address, the tweak at the IV address, and an xts_first/xts_last pair of flags that mark the first and the last block of a data unit. The key size field gives the size of one of the two keys: 128 or 256 bits, there is no 192-bit XTS.

Register xts(aes) for the H616: each request is one data unit, so both flags are set and the request IV is the tweak of its first block. XTS returns no IV, so the CBC write-back of the last block is skipped. Ciphertext stealing (lengths that are not a multiple of the block size) and two 192-bit keys go to the fallback, as do the requests that the existing checks already send there.

The engine is much slower at XTS than the CPU's ARMv8 AES instructions:
4 times slower for 64 KiB requests (213 against 861 MB/s), 8 times for 4 KiB and 17 times for 512 bytes. Register it below every CPU implementation of xts(aes) (priority 90), so that the CPU stays the default, for dm-crypt too, and the engine is only used when it is asked for by driver name.

The other variants get CE_ID_NOTSUPP for the new mode, so nothing changes for them.

---
The H616 crypto engine can do AES-XTS in its separate RAES unit. This patch registers xts(aes) for the H616, below the CPU implementations because the engine is slower than the ARMv8 Crypto Extensions; it stays available by driver name (xts-aes-sun8i-ce) for users who want to offload the CPU.

Testing on an Orange Pi Zero2 (H616), v7.3-based kernel with this patch (sun8i-ce is unchanged between v7.3 and cryptodev-2.6):
- the boot-time self-test of xts-aes-sun8i-ce passes, and the testmgr vectors run on the engine (debugfs request counters);
- 21 known-answer vectors (IEEE 1619 and generated, including ciphertext stealing lengths, which go to the fallback) pass, and 3000 random requests (10% with misaligned buffers) give the same result as xts-aes-ce;
- dm-crypt with capi:xts-aes-sun8i-ce-plain64, 512 and 4096-byte
sectors: 48 MiB written and read back with matching sha256, and the raw sectors decrypt with xts-aes-ce; with this priority, xts(aes) and plain aes-xts-plain64 resolve to the CPU's xts-aes-ce;
- the existing ECB/CBC ciphers, hashes and the TRNG still match their software implementations.
Throughput, engine against xts-aes-ce: 20/352 MB/s for 512-byte requests, 98/759 MB/s for 4 KiB, 213/861 MB/s for 64 KiB.

This patch was developed with Claude (Anthropic) as a coding assistant: it investigated the problem, wrote the code and the commit messages. I reviewed it, and it was tested on an Orange Pi Zero2 (H616) as described above.

---
 .../crypto/allwinner/sun8i-ce/sun8i-ce-cipher.c    | 102 +++++++++++++++++----
 drivers/crypto/allwinner/sun8i-ce/sun8i-ce-core.c  |  44 +++++++--
 drivers/crypto/allwinner/sun8i-ce/sun8i-ce.h       |  21 ++++-
 3 files changed, 141 insertions(+), 26 deletions(-)
drivers/crypto/allwinner/sun8i-ce/sun8i-ce-cipher.c +85 -17
index 021614b65e39..b87be3123b12 100644
@@ -6,7 +6,7 @@
  * Copyright (C) 2016-2019 Corentin LABBE <[email protected]>
  *
  * This file add support for AES cipher with 128,192,256 bits keysize in
- * CBC and ECB mode.
+ * CBC and ECB mode, and for AES-XTS with 256 and 512 bits keysize.
  *
  * You could find a link for the datasheet in Documentation/arch/arm/sunxi.rst
  */
@@ -19,18 +19,34 @@
 #include <crypto/scatterwalk.h>
 #include <crypto/internal/des.h>
 #include <crypto/internal/skcipher.h>
+#include <crypto/xts.h>
 #include "sun8i-ce.h"
 
+/*
+ * XTS uses the request IV as the tweak of its first block and returns no IV,
+ * CBC returns the last ciphertext block as the IV of the next request.
+ */
+static bool sun8i_ce_cipher_iv_out(struct sun8i_ce_alg_template *algt)
+{
+	return algt->ce_blockmode != CE_ID_OP_XTS;
+}
+
 static int sun8i_ce_cipher_need_fallback(struct skcipher_request *areq)
 {
 	struct crypto_skcipher *tfm = crypto_skcipher_reqtfm(areq);
 	struct scatterlist *sg;
+	struct sun8i_cipher_tfm_ctx *op = crypto_skcipher_ctx(tfm);
 	struct skcipher_alg *alg = crypto_skcipher_alg(tfm);
 	struct sun8i_ce_alg_template *algt;
 	unsigned int todo, len;
 
 	algt = container_of(alg, struct sun8i_ce_alg_template, alg.skcipher.base);
 
+	/* The XTS unit takes two 128-bit or two 256-bit keys only */
+	if (algt->ce_blockmode == CE_ID_OP_XTS &&
+	    op->keylen == 2 * AES_KEYSIZE_192)
+		return true;
+
 	if (sg_nents_for_len(areq->src, areq->cryptlen) > MAX_SG ||
 	    sg_nents_for_len(areq->dst, areq->cryptlen) > MAX_SG) {
 		if (IS_ENABLED(CONFIG_CRYPTO_DEV_SUN8I_CE_DEBUG))
@@ -164,7 +180,10 @@ static int sun8i_ce_cipher_prepare(struct skcipher_request *areq,
 	memset(cet, 0, sizeof(struct ce_task));
 
 	cet->t_id = cpu_to_le32(rctx->flow);
-	common = ce->variant->alg_cipher[algt->ce_algo_id];
+	if (algt->ce_blockmode == CE_ID_OP_XTS)
+		common = CE_ALG_RAES;
+	else
+		common = ce->variant->alg_cipher[algt->ce_algo_id];
 	common |= rctx->op_dir | CE_COMM_INT;
 	cet->t_common_ctl = cpu_to_le32(common);
 	/* CTS and recent CE (H6) need length in bytes, in word otherwise */
@@ -175,6 +194,15 @@ static int sun8i_ce_cipher_prepare(struct skcipher_request *areq,
 
 	sym = ce->variant->op_mode[algt->ce_blockmode];
 	len = op->keylen;
+	if (algt->ce_blockmode == CE_ID_OP_XTS) {
+		/*
+		 * One request is one data unit: its first block uses the
+		 * tweak from the IV, its last block ends the data unit. The
+		 * key size field gives the size of each of the two keys.
+		 */
+		sym |= CE_SYM_XTS_FIRST | CE_SYM_XTS_LAST;
+		len /= 2;
+	}
 	switch (len) {
 	case 128 / 8:
 		sym |= CE_AES_128BITS;
@@ -200,7 +228,8 @@ static int sun8i_ce_cipher_prepare(struct skcipher_request *areq,
 
 	ivsize = crypto_skcipher_ivsize(tfm);
 	if (areq->iv && ivsize > 0) {
-		if (rctx->op_dir & CE_DECRYPTION) {
+		if (sun8i_ce_cipher_iv_out(algt) &&
+		    (rctx->op_dir & CE_DECRYPTION)) {
 			offset = areq->cryptlen - ivsize;
 			scatterwalk_map_and_copy(rctx->backup_iv, areq->src,
 						 offset, ivsize, 0);
@@ -290,13 +319,15 @@ static int sun8i_ce_cipher_prepare(struct skcipher_request *areq,
 			dma_unmap_single(ce->dev, rctx->addr_iv, ivsize,
 					 DMA_TO_DEVICE);
 
-		offset = areq->cryptlen - ivsize;
-		if (rctx->op_dir & CE_DECRYPTION) {
-			memcpy(areq->iv, rctx->backup_iv, ivsize);
-			memzero_explicit(rctx->backup_iv, ivsize);
-		} else {
-			scatterwalk_map_and_copy(areq->iv, areq->dst, offset,
-						 ivsize, 0);
+		if (sun8i_ce_cipher_iv_out(algt)) {
+			offset = areq->cryptlen - ivsize;
+			if (rctx->op_dir & CE_DECRYPTION) {
+				memcpy(areq->iv, rctx->backup_iv, ivsize);
+				memzero_explicit(rctx->backup_iv, ivsize);
+			} else {
+				scatterwalk_map_and_copy(areq->iv, areq->dst,
+							 offset, ivsize, 0);
+			}
 		}
 		memzero_explicit(rctx->bounce_iv, ivsize);
 	}
@@ -314,10 +345,13 @@ static void sun8i_ce_cipher_unprepare(struct skcipher_request *areq,
 	struct sun8i_cipher_tfm_ctx *op = crypto_skcipher_ctx(tfm);
 	struct sun8i_ce_dev *ce = op->ce;
 	struct sun8i_cipher_req_ctx *rctx = skcipher_request_ctx(areq);
+	struct skcipher_alg *alg = crypto_skcipher_alg(tfm);
+	struct sun8i_ce_alg_template *algt;
 	unsigned int ivsize, offset;
 	int nr_sgs = rctx->nr_sgs;
 	int nr_sgd = rctx->nr_sgd;
 
+	algt = container_of(alg, struct sun8i_ce_alg_template, alg.skcipher.base);
 	ivsize = crypto_skcipher_ivsize(tfm);
 
 	if (areq->src == areq->dst) {
@@ -332,13 +366,15 @@ static void sun8i_ce_cipher_unprepare(struct skcipher_request *areq,
 		if (cet->t_iv)
 			dma_unmap_single(ce->dev, rctx->addr_iv, ivsize,
 					 DMA_TO_DEVICE);
-		offset = areq->cryptlen - ivsize;
-		if (rctx->op_dir & CE_DECRYPTION) {
-			memcpy(areq->iv, rctx->backup_iv, ivsize);
-			memzero_explicit(rctx->backup_iv, ivsize);
-		} else {
-			scatterwalk_map_and_copy(areq->iv, areq->dst, offset,
-						 ivsize, 0);
+		if (sun8i_ce_cipher_iv_out(algt)) {
+			offset = areq->cryptlen - ivsize;
+			if (rctx->op_dir & CE_DECRYPTION) {
+				memcpy(areq->iv, rctx->backup_iv, ivsize);
+				memzero_explicit(rctx->backup_iv, ivsize);
+			} else {
+				scatterwalk_map_and_copy(areq->iv, areq->dst,
+							 offset, ivsize, 0);
+			}
 		}
 		memzero_explicit(rctx->bounce_iv, ivsize);
 	}
@@ -489,6 +525,38 @@ int sun8i_ce_aes_setkey(struct crypto_skcipher *tfm, const u8 *key,
 	return crypto_skcipher_setkey(op->fallback_tfm, key, keylen);
 }
 
+int sun8i_ce_xts_setkey(struct crypto_skcipher *tfm, const u8 *key,
+			unsigned int keylen)
+{
+	struct sun8i_cipher_tfm_ctx *op = crypto_skcipher_ctx(tfm);
+	struct sun8i_ce_dev *ce = op->ce;
+	int err;
+
+	err = xts_verify_key(tfm, key, keylen);
+	if (err)
+		return err;
+
+	switch (keylen) {
+	case 2 * AES_KEYSIZE_128:
+	case 2 * AES_KEYSIZE_192:
+	case 2 * AES_KEYSIZE_256:
+		break;
+	default:
+		dev_dbg(ce->dev, "ERROR: Invalid keylen %u\n", keylen);
+		return -EINVAL;
+	}
+	kfree_sensitive(op->key);
+	op->keylen = keylen;
+	op->key = kmemdup(key, keylen, GFP_KERNEL | GFP_DMA);
+	if (!op->key)
+		return -ENOMEM;
+
+	crypto_skcipher_clear_flags(op->fallback_tfm, CRYPTO_TFM_REQ_MASK);
+	crypto_skcipher_set_flags(op->fallback_tfm, tfm->base.crt_flags & CRYPTO_TFM_REQ_MASK);
+
+	return crypto_skcipher_setkey(op->fallback_tfm, key, keylen);
+}
+
 int sun8i_ce_des3_setkey(struct crypto_skcipher *tfm, const u8 *key,
 			 unsigned int keylen)
 {
drivers/crypto/allwinner/sun8i-ce/sun8i-ce-core.c +37 -7

92 lines not on this page · raw message

drivers/crypto/allwinner/sun8i-ce/sun8i-ce.h +19 -2
index 468d99bf5bf6..89689e54e5fd 100644
@@ -46,6 +46,10 @@
 
 #define CE_OP_ECB	0
 #define CE_OP_CBC	(1 << 8)
+#define CE_OP_XTS	(9 << 8)
+
+#define CE_SYM_XTS_FIRST	BIT(12)
+#define CE_SYM_XTS_LAST		BIT(13)
 
 #define CE_ALG_AES		0
 #define CE_ALG_DES		1
@@ -58,6 +62,7 @@
 #define CE_ALG_SHA512           21
 #define CE_ALG_TRNG		48
 #define CE_ALG_TRNG_V2		0x1c
+#define CE_ALG_RAES		0x30
 
 /* Used in ce_variant */
 #define CE_ID_NOTSUPP		0xFF
@@ -77,7 +82,8 @@
 
 #define CE_ID_OP_ECB	0
 #define CE_ID_OP_CBC	1
-#define CE_ID_OP_MAX	2
+#define CE_ID_OP_XTS	2
+#define CE_ID_OP_MAX	3
 
 /* Used in CE registers */
 #define CE_ERR_ALGO_NOTSUP	BIT(0)
@@ -98,6 +104,14 @@
 
 #define MAX_SG 8
 
+/*
+ * Priority of the modes that the CPU's own AES instructions run faster than
+ * the engine: below every CPU implementation of them (arm64 CE 300, NEON 250
+ * and 200, AES library 110, generic templates 100), so the CPU stays the
+ * default and the engine is used when it is asked for by driver name.
+ */
+#define CE_PRIO_BELOW_CPU	90
+
 #define CE_MAX_CLOCKS 4
 #define CE_DMA_TIMEOUT_MS	3000
 
@@ -124,7 +138,8 @@ struct ce_clock {
  *              coresponding CE_ALG_XXX value
  * @alg_hash:	list of supported hashes. for each CE_ID_ this will give the
  *              corresponding CE_ALG_XXX value
- * @op_mode:	list of supported block modes
+ * @op_mode:	list of supported block modes, CE_ID_NOTSUPP if not supported.
+ *		XTS runs on the RAES unit (algorithm CE_ALG_RAES)
  * @cipher_t_dlen_in_bytes:	Does the request size for cipher is in
  *				bytes or words
  * @hash_t_dlen_in_bytes:	Does the request size for hash is in
@@ -352,6 +367,8 @@ struct sun8i_ce_alg_template {
 
 int sun8i_ce_aes_setkey(struct crypto_skcipher *tfm, const u8 *key,
 			unsigned int keylen);
+int sun8i_ce_xts_setkey(struct crypto_skcipher *tfm, const u8 *key,
+			unsigned int keylen);
 int sun8i_ce_des3_setkey(struct crypto_skcipher *tfm, const u8 *key,
 			 unsigned int keylen);
 int sun8i_ce_cipher_init(struct crypto_tfm *tfm);

---
base-commit: 82e91fe19fb838f235f7c8829afc1eb389c93bdf
change-id: 20261110-sun8i-ce-h616-xts-8e52bb3c7b36

Best regards,

Signature · 1 line
Arun Iyengar <[email protected]>
#1 lore raw
Thank you for your contribution! Sashiko AI review found 1 potential issue to consider: - [Medium] crypto: sun8i-ce: missing CRYPTO_XTS select fails sun8i_ce_cipher_init() --

Thank you for your contribution! Sashiko AI review found 1 potential issue to consider:
- [Medium] crypto: sun8i-ce: missing CRYPTO_XTS select fails sun8i_ce_cipher_init()
--

commit 660524c38c775c4edff2e13f8df35a02b4667a78
Author: Arun Iyengar <[email protected]>

crypto: sun8i-ce - Add AES-XTS for the H616

This commit adds AES-XTS support for the Allwinner H616 crypto engine using its RAES unit and registers xts-aes-sun8i-ce with a fallback transform.

Quoted 4 lines
drivers/crypto/allwinner/sun8i-ce/sun8i-ce-core.c +0 -0
index 21762bd408e54..a7d771927b7a1 100644

[ ... ]

Quoted 11 lines
drivers/crypto/allwinner/sun8i-ce/sun8i-ce-core.c +7 -0
@@ -350,6 +350,36 @@ static struct sun8i_ce_alg_template ce_algs[] = {
 		.do_one_request = sun8i_ce_cipher_do_one,
 	},
 },
+{
+	.type = CRYPTO_ALG_TYPE_SKCIPHER,
+	.ce_algo_id = CE_ID_CIPHER_AES,
+	.ce_blockmode = CE_ID_OP_XTS,
+	.alg.skcipher.base = {
+		.base = {
+			.cra_name = "xts(aes)",

drivers/crypto/allwinner/sun8i-ce/sun8i-ce-core.c static struct sun8i_ce_alg_template ce_algs[] = { · line 369

Quoted 10 lines
drivers/crypto/allwinner/sun8i-ce/sun8i-ce-core.c +10 -0
+			.cra_driver_name = "xts-aes-sun8i-ce",
+			.cra_priority = CE_PRIO_BELOW_CPU,
+			.cra_blocksize = AES_BLOCK_SIZE,
+			.cra_flags = CRYPTO_ALG_TYPE_SKCIPHER |
+				CRYPTO_ALG_ASYNC |
+				CRYPTO_ALG_NEED_FALLBACK,
+			.cra_ctxsize = sizeof(struct sun8i_cipher_tfm_ctx),
+			.cra_module = THIS_MODULE,
+			.cra_alignmask = 0xf,
+			.cra_init = sun8i_ce_cipher_init,

[Severity: Medium]
Should CRYPTO_DEV_SUN8I_CE in drivers/crypto/allwinner/Kconfig select CRYPTO_XTS alongside CRYPTO_ECB and CRYPTO_CBC?

sun8i_ce_cipher_init() unconditionally calls crypto_alloc_skcipher("xts(aes)", 0, CRYPTO_ALG_NEED_FALLBACK) to allocate op->fallback_tfm, which fails with -ENOENT during registration self-tests or transform allocation when CONFIG_CRYPTO_XTS is disabled and no other xts(aes) implementation is built in.

Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1