arm64: marvell: add cryptographic engine support for 7k/8k

The Inside Secure Safexcel cryptographic engine is found on some Marvell SoCs (7k/8k). Document the bindings used by its driver.

Signed-off-by Antoine Tenart <[email protected]>

---
 .../bindings/crypto/inside-secure-safexcel.txt     | 27 ++++++++++++++++++++++
 1 file changed, 27 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/crypto/inside-secure-safexcel.txt

Documentation/devicetree/bindings/crypto/inside-secure-safexcel.txt new file +7 -0

new file mode 100644
index 000000000000..ff56b9384fcc
@@ -0,0 +1,27 @@
+Inside Secure SafeXcel cryptographic engine
+
+Required properties:
+- compatible: Should be "inside-secure,safexcel-eip197".
+- reg: Base physical address of the engine and length of memory mapped region.
+- interrupts: Interrupt numbers for the rings and engine.
+- interrupt-names: Should be "ring0", "ring1", "ring2", "ring3", "eip", "mem".

Documentation/devicetree/bindings/crypto/inside-secure-safexcel.txt line 17

Quoted 5 lines · Antoine Tenart

Documentation/devicetree/bindings/crypto/inside-secure-safexcel.txt new file +5 -0

+
+	crypto: crypto at 800000 {
+		compatible = "inside-secure,safexcel-eip197";
+		reg = <0x800000 0x200000>;
+		interrupts = <GIC_SPI 34 (IRQ_TYPE_EDGE_RISING | IRQ_TYPE_LEVEL_HIGH)>,

I'm puzzled. How can the interrupt can be both level *and* edge? That doesn't make any sense.

Hi Marc,

Quoted 9 lines · Marc Zyngier

Documentation/devicetree/bindings/crypto/inside-secure-safexcel.txt line 17

Quoted 5 lines · Antoine Tenart

Documentation/devicetree/bindings/crypto/inside-secure-safexcel.txt new file +5 -0

+
+	crypto: crypto at 800000 {
+		compatible = "inside-secure,safexcel-eip197";
+		reg = <0x800000 0x200000>;
+		interrupts = <GIC_SPI 34 (IRQ_TYPE_EDGE_RISING | IRQ_TYPE_LEVEL_HIGH)>,

I'm puzzled. How can the interrupt can be both level *and* edge? That doesn't make any sense.

I agree this looks odd. I took it from Russel's ICU mapping:
http://lists.infradead.org/pipermail/linux-arm-kernel/2017-February/489040.html

The driver does not use this interrupt (yet?). I'm not sure what is this interrupt exact configuration...

Do you want me to remove this from the documentation (and device trees)?

Quoted 11 lines · Antoine Tenart

Quoted 9 lines · Marc Zyngier

Documentation/devicetree/bindings/crypto/inside-secure-safexcel.txt line 17

Quoted 5 lines · Antoine Tenart

Documentation/devicetree/bindings/crypto/inside-secure-safexcel.txt new file +5 -0

+
+	crypto: crypto at 800000 {
+		compatible = "inside-secure,safexcel-eip197";
+		reg = <0x800000 0x200000>;
+		interrupts = <GIC_SPI 34 (IRQ_TYPE_EDGE_RISING | IRQ_TYPE_LEVEL_HIGH)>,

I'm puzzled. How can the interrupt can be both level *and* edge? That doesn't make any sense.

Quoted 2 lines

I agree this looks odd. I took it from Russel's ICU mapping:
http://lists.infradead.org/pipermail/linux-arm-kernel/2017-February/489040.html

This emails says:

ICU-irq => GIC-SPI-num Enable Edge/Level ICU-group [...]
24 => 34 En Lv 0

Quoted 15 lines · Marc Zyngier

Quoted 13 lines · Antoine Tenart

Quoted 9 lines · Marc Zyngier

Documentation/devicetree/bindings/crypto/inside-secure-safexcel.txt line 17

Quoted 5 lines · Antoine Tenart

Documentation/devicetree/bindings/crypto/inside-secure-safexcel.txt new file +5 -0

+
+	crypto: crypto at 800000 {
+		compatible = "inside-secure,safexcel-eip197";
+		reg = <0x800000 0x200000>;
+		interrupts = <GIC_SPI 34 (IRQ_TYPE_EDGE_RISING | IRQ_TYPE_LEVEL_HIGH)>,

I'm puzzled. How can the interrupt can be both level *and* edge? That doesn't make any sense.

I agree this looks odd. I took it from Russel's ICU mapping:
http://lists.infradead.org/pipermail/linux-arm-kernel/2017-February/489040.html

Quoted 5 lines

This emails says:

ICU-irq => GIC-SPI-num Enable Edge/Level ICU-group [...]
24 => 34 En Lv 0

It also says: 87 => 34 En Lv 5, which is the IRQ I'm looking for.

Quoted 1 line

It also says: 87 => 34 En Lv 5, which is the IRQ I'm looking for.

Ah, that one as well. So how is the interrupt routed? Via the ICU, and then to the GIC (with several ICU sources mapped on a single SPI)?

Hello,

Quoted 2 lines · Marc Zyngier

Quoted 1 line

It also says: 87 => 34 En Lv 5, which is the IRQ I'm looking for.

Quoted 2 lines

Ah, that one as well. So how is the interrupt routed? Via the ICU, and then to the GIC (with several ICU sources mapped on a single SPI)?

The crypto block being in the CP part, it has a wired interrupt to the ICU (also in the CP). The ICU then turns this wired interrupt into a memory write transaction to a register called GICP SPI in the AP, which triggers a SPI interrupt in the GIC.

In the current mainline kernel, the ICU is configured by the firmware and creates static associations between wired interrupts in the CP and corresponding SPI interrupts. Therefore the Device Tree currently reference such SPI interrupts directly.

However, I have a patch series that I plan to submit hopefully in the next days that adds an ICU driver, and changes the Device Tree to refer to the ICU interrupt instead.

Therefore, I don't think the binding should reference anything else than the usual info about the interrupts property.

Quoted 4 lines

The crypto block being in the CP part, it has a wired interrupt to the ICU (also in the CP). The ICU then turns this wired interrupt into a memory write transaction to a register called GICP SPI in the AP, which triggers a SPI interrupt in the GIC.

Is that some kind of Level-triggered MSI, ? la GICv3 GICD_SETSPI_NSR?

Quoted 3 lines

However, I have a patch series that I plan to submit hopefully in the next days that adds an ICU driver, and changes the Device Tree to refer to the ICU interrupt instead.

OK, I'm quite interested to see that, specially if my above hunch is right...

drivers/crypto/inside-secure/safexcel.c new file +4 -0

+	priv->clk = of_clk_get(dev->of_node, 0);
+	if (!IS_ERR(priv->clk)) {
+		ret = clk_prepare_enable(priv->clk);
+		if (ret) {

[...]

Quoted 4 lines

drivers/crypto/inside-secure/safexcel.c new file +4 -0

+	priv->clk = of_clk_get(dev->of_node, 0);
+	if (!IS_ERR(priv->clk)) {
+		ret = clk_prepare_enable(priv->clk);
+		if (ret) {

drivers/crypto/inside-secure/safexcel.c line 807

Quoted 10 lines

drivers/crypto/inside-secure/safexcel.c new file +10 -0

+			dev_err(dev, "unable to enable clk (%d)\n", ret);
+			return ret;
+		}
+	} else {
+		/* The clock isn't mandatory */
+		if (PTR_ERR(priv->clk) == -EPROBE_DEFER)
+			return -EPROBE_DEFER;
+	}
+
+	ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64));

The correct address width of the engine in Marvell SoCs is 40bit.

Hi Igal,

Quoted 18 lines · Igal Liberman

[...]

Quoted 4 lines

drivers/crypto/inside-secure/safexcel.c new file +4 -0

+	priv->clk = of_clk_get(dev->of_node, 0);
+	if (!IS_ERR(priv->clk)) {
+		ret = clk_prepare_enable(priv->clk);
+		if (ret) {

drivers/crypto/inside-secure/safexcel.c line 807

Quoted 10 lines

drivers/crypto/inside-secure/safexcel.c new file +10 -0

+			dev_err(dev, "unable to enable clk (%d)\n", ret);
+			return ret;
+		}
+	} else {
+		/* The clock isn't mandatory */
+		if (PTR_ERR(priv->clk) == -EPROBE_DEFER)
+			return -EPROBE_DEFER;
+	}
+
+	ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64));

The correct address width of the engine in Marvell SoCs is 40bit.

Oops. I'll update.

Thanks!
Antoine

-----Original Message-----
From: linux-crypto-owner at vger.kernel.org [mailto:linux-crypto- owner at vger.kernel.org] On Behalf Of Robin Murphy
Sent: Wednesday, May 03, 2017 20:15
To: Antoine Tenart; Igal Liberman
Cc: herbert at gondor.apana.org.au; davem at davemloft.net;
jason at lakedaemon.net; andrew at lunn.ch; gregory.clement at free- electrons.com; sebastian.hesselbarth at gmail.com; linux-
crypto at vger.kernel.org; linux-arm-kernel at lists.infradead.org; thomas.petazzoni at free-electrons.com; boris.brezillon at free-electrons.com; Ofer Heifetz; Nadav Haklai
Subject: Re: [PATCH v3 2/3] crypto: inside-secure: add SafeXcel EIP197 crypto engine driver

Quoted 21 lines · Antoine Tenart

Hi Igal,

Quoted 18 lines · Igal Liberman

[...]

Quoted 4 lines

drivers/crypto/inside-secure/safexcel.c new file +4 -0

+	priv->clk = of_clk_get(dev->of_node, 0);
+	if (!IS_ERR(priv->clk)) {
+		ret = clk_prepare_enable(priv->clk);
+		if (ret) {

drivers/crypto/inside-secure/safexcel.c line 807

Quoted 10 lines

drivers/crypto/inside-secure/safexcel.c new file +10 -0

+			dev_err(dev, "unable to enable clk (%d)\n", ret);
+			return ret;
+		}
+	} else {
+		/* The clock isn't mandatory */
+		if (PTR_ERR(priv->clk) == -EPROBE_DEFER)
+			return -EPROBE_DEFER;
+	}
+
+	ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64));

The correct address width of the engine in Marvell SoCs is 40bit.

Is that what the IP block itself is internally limited to, or just how many bits happen to be wired up externally in the particular Marvell integrations?

Quoted 1 line

Oops. I'll update.

If the device itself can handle full 64-bit addresses, then the driver should reflect that regardless of the integration - describing integration limitations is the job of the firmware, e.g. with DT "dma-ranges", assuming they actually matter (typically if the whole SoC interconnect and physical memory map is the same width then having an even wider DMA mask doesn't make any difference - 64 bits still just means "everything").

If it's the case that the IP is configurable and could be synthesised with various internal address widths, then that might warrant a DT property to identify the particular configuration if it's not readable from some ID register, although it may be reasonable in practice to simply assume the chosen configuration to be sane (i.e. not smaller than the rest of the system) and that the driver can set the maximal DMA mask as above.

Robin.

Quoted 4 lines

Thanks!
Antoine

drivers/crypto/inside-secure/safexcel_cipher.c new file +30 -0

+	unsigned int key_len;
+};
+
+static void safexcel_cipher_token(struct safexcel_cipher_ctx *ctx,
+				  struct crypto_async_request *async,
+				  struct safexcel_command_desc *cdesc,
+				  u32 length)
+{
+	struct ablkcipher_request *req = ablkcipher_request_cast(async);
+	struct safexcel_token *token;
+	unsigned offset = 0;
+
+	if (ctx->mode == CONTEXT_CONTROL_CRYPTO_MODE_CBC) {
+		offset = AES_BLOCK_SIZE / sizeof(u32);
+		memcpy(cdesc->control_data.token, req->info, AES_BLOCK_SIZE);
+
+		cdesc->control_data.options |= EIP197_OPTION_4_TOKEN_IV_CMD;
+	}
+
+	token = (struct safexcel_token *)(cdesc->control_data.token + offset);
+
+	token[0].opcode = EIP197_TOKEN_OPCODE_DIRECTION;
+	token[0].packet_length = length;
+	token[0].stat = EIP197_TOKEN_STAT_LAST_PACKET;
+	token[0].instructions = EIP197_TOKEN_INS_LAST |
+				EIP197_TOKEN_INS_TYPE_CRYTO |
+				EIP197_TOKEN_INS_TYPE_OUTPUT;
+}
+
+static int safexcel_aes_setkey(struct crypto_ablkcipher *ctfm, const u8

*key, + unsigned int len)
+{

drivers/crypto/inside-secure/safexcel_cipher.c new file +12 -0

+	struct crypto_tfm *tfm = crypto_ablkcipher_tfm(ctfm);
+	struct safexcel_cipher_ctx *ctx = crypto_tfm_ctx(tfm);
+	struct crypto_aes_ctx aes;
+	int ret, i;
+
+	ret = crypto_aes_expand_key(&aes, key, len);
+	if (ret) {
+		crypto_ablkcipher_set_flags(ctfm, CRYPTO_TFM_RES_BAD_KEY_LEN);
+		return ret;
+	}
+
+	for (i = 0; i < len / sizeof(u32); i++) {

drivers/crypto/inside-secure/safexcel_cipher.c new file +42 -0

+
+	return 0;
+}
+
+static int safexcel_context_control(struct safexcel_cipher_ctx *ctx,
+				    struct safexcel_command_desc *cdesc)
+{
+	struct safexcel_crypto_priv *priv = ctx->priv;
+	int ctrl_size;
+
+	if (ctx->direction == SAFEXCEL_ENCRYPT)
+		cdesc->control_data.control0 |= CONTEXT_CONTROL_TYPE_CRYPTO_OUT;
+	else
+		cdesc->control_data.control0 |= CONTEXT_CONTROL_TYPE_CRYPTO_IN;
+
+	cdesc->control_data.control0 |= CONTEXT_CONTROL_KEY_EN;
+	cdesc->control_data.control1 |= ctx->mode;
+
+	switch (ctx->key_len) {
+	case AES_KEYSIZE_128:
+		cdesc->control_data.control0 |= CONTEXT_CONTROL_CRYPTO_ALG_AES128;
+		ctrl_size = 4;
+		break;
+	case AES_KEYSIZE_192:
+		cdesc->control_data.control0 |= CONTEXT_CONTROL_CRYPTO_ALG_AES192;
+		ctrl_size = 6;
+		break;
+	case AES_KEYSIZE_256:
+		cdesc->control_data.control0 |= CONTEXT_CONTROL_CRYPTO_ALG_AES256;
+		ctrl_size = 8;
+		break;
+	default:
+		dev_err(priv->dev, "aes keysize not supported: %u\n",
+			ctx->key_len);
+		return -EINVAL;
+	}
+	cdesc->control_data.control0 |= CONTEXT_CONTROL_SIZE(ctrl_size);
+
+	return 0;
+}
+
+static int safexcel_handle_result(struct safexcel_crypto_priv *priv, int

ring, + struct crypto_async_request *async,

drivers/crypto/inside-secure/safexcel_cipher.c new file +80 -0

+				  bool *should_complete, int *ret)
+{
+	struct ablkcipher_request *req = ablkcipher_request_cast(async);
+	struct safexcel_result_desc *rdesc;
+	int ndesc = 0;
+
+	*ret = 0;
+
+	spin_lock_bh(&priv->ring[ring].egress_lock);
+	do {
+		rdesc = safexcel_ring_next_rptr(priv, &priv->ring[ring].rdr);
+		if (IS_ERR(rdesc)) {
+			dev_err(priv->dev,
+				"cipher: result: could not retrieve the result descriptor\n");
+			*ret = PTR_ERR(rdesc);
+			break;
+		}
+
+		if (rdesc->result_data.error_code) {
+			dev_err(priv->dev,
+				"cipher: result: result descriptor error (%d)\n",
+				rdesc->result_data.error_code);
+			*ret = -EIO;
+		}
+
+		ndesc++;
+	} while (!rdesc->last_seg);
+
+	safexcel_complete(priv, ring);
+	spin_unlock_bh(&priv->ring[ring].egress_lock);
+
+	if (req->src == req->dst) {
+		dma_unmap_sg(priv->dev, req->src,
+			     sg_nents_for_len(req->src, req->nbytes),
+			     DMA_BIDIRECTIONAL);
+	} else {
+		dma_unmap_sg(priv->dev, req->src,
+			     sg_nents_for_len(req->src, req->nbytes),
+			     DMA_TO_DEVICE);
+		dma_unmap_sg(priv->dev, req->dst,
+			     sg_nents_for_len(req->dst, req->nbytes),
+			     DMA_FROM_DEVICE);
+	}
+
+	*should_complete = true;
+
+	return ndesc;
+}
+
+static int safexcel_aes_send(struct crypto_async_request *async,
+			     int ring, struct safexcel_request *request,
+			     int *commands, int *results)
+{
+	struct ablkcipher_request *req = ablkcipher_request_cast(async);
+	struct safexcel_cipher_ctx *ctx = crypto_tfm_ctx(req->base.tfm);
+	struct safexcel_crypto_priv *priv = ctx->priv;
+	struct safexcel_command_desc *cdesc;
+	struct safexcel_result_desc *rdesc;
+	struct scatterlist *sg;
+	int nr_src, nr_dst, n_cdesc = 0, n_rdesc = 0, queued = req->nbytes;
+	int i, ret = 0;
+
+	request->req = &req->base;
+
+	if (req->src == req->dst) {
+		nr_src = dma_map_sg(priv->dev, req->src,
+				    sg_nents_for_len(req->src, req->nbytes),
+				    DMA_BIDIRECTIONAL);
+		nr_dst = nr_src;
+		if (!nr_src)
+			return -EINVAL;
+	} else {
+		nr_src = dma_map_sg(priv->dev, req->src,
+				    sg_nents_for_len(req->src, req->nbytes),
+				    DMA_TO_DEVICE);
+		if (!nr_src)
+			return -EINVAL;
+
+		nr_dst = dma_map_sg(priv->dev, req->dst,
+				    sg_nents_for_len(req->dst, req->nbytes),

Am Montag, 24. April 2017, 09:54:06 CEST schrieb Antoine Tenart:

drivers/crypto/inside-secure/safexcel_cipher.c line 31

Quoted 8 lines

drivers/crypto/inside-secure/safexcel_cipher.c new file +8 -0

+struct safexcel_cipher_ctx {
+	struct safexcel_context base;
+	struct safexcel_crypto_priv *priv;
+
+	enum safexcel_cipher_direction direction;
+	u32 mode;
+
+	__le32 key[8];

Can you please help me find the location where this memory is zeroized when released?

Quoted 27 lines

drivers/crypto/inside-secure/safexcel_cipher.c new file +27 -0

+static void safexcel_cipher_token(struct safexcel_cipher_ctx *ctx,
+				  struct crypto_async_request *async,
+				  struct safexcel_command_desc *cdesc,
+				  u32 length)
+{
+	struct ablkcipher_request *req = ablkcipher_request_cast(async);
+	struct safexcel_token *token;
+	unsigned offset = 0;
+
+	if (ctx->mode == CONTEXT_CONTROL_CRYPTO_MODE_CBC) {
+		offset = AES_BLOCK_SIZE / sizeof(u32);
+		memcpy(cdesc->control_data.token, req->info, AES_BLOCK_SIZE);
+
+		cdesc->control_data.options |= EIP197_OPTION_4_TOKEN_IV_CMD;
+	}
+
+	token = (struct safexcel_token *)(cdesc->control_data.token + offset);
+
+	token[0].opcode = EIP197_TOKEN_OPCODE_DIRECTION;
+	token[0].packet_length = length;
+	token[0].stat = EIP197_TOKEN_STAT_LAST_PACKET;
+	token[0].instructions = EIP197_TOKEN_INS_LAST |
+				EIP197_TOKEN_INS_TYPE_CRYTO |
+				EIP197_TOKEN_INS_TYPE_OUTPUT;
+}
+
+static int safexcel_aes_setkey(struct crypto_ablkcipher *ctfm, const u8

Quoted 2 lines

*key, + unsigned int len)
+{

You still use ablkcipher. I thought that it is on its way out in favor of the skcipher API. Why do you stick to ablkcipher?

Note, a change could be as simple as s/ablkcipher/skcipher/g

Quoted 16 lines

drivers/crypto/inside-secure/safexcel_cipher.c new file +16 -0

+	struct crypto_tfm *tfm = crypto_ablkcipher_tfm(ctfm);
+	struct safexcel_cipher_ctx *ctx = crypto_tfm_ctx(tfm);
+	struct crypto_aes_ctx aes;
+	int ret, i;
+
+	ret = crypto_aes_expand_key(&aes, key, len);
+	if (ret) {
+		crypto_ablkcipher_set_flags(ctfm, CRYPTO_TFM_RES_BAD_KEY_LEN);
+		return ret;
+	}
+
+	for (i = 0; i < len / sizeof(u32); i++) {
+		if (ctx->key[i] != cpu_to_le32(aes.key_enc[i])) {
+			ctx->base.needs_inv = true;
+			break;
+		}

drivers/crypto/inside-secure/safexcel_cipher.c line 85

Quoted 6 lines

drivers/crypto/inside-secure/safexcel_cipher.c new file +6 -0

+	}
+
+	for (i = 0; i < len / sizeof(u32); i++)
+		ctx->key[i] = cpu_to_le32(aes.key_enc[i]);
+
+	ctx->key_len = len;

memzero_explicit(aes)?

Quoted 35 lines

drivers/crypto/inside-secure/safexcel_cipher.c new file +35 -0

+static int safexcel_aes_send(struct crypto_async_request *async,
+			     int ring, struct safexcel_request *request,
+			     int *commands, int *results)
+{
+	struct ablkcipher_request *req = ablkcipher_request_cast(async);
+	struct safexcel_cipher_ctx *ctx = crypto_tfm_ctx(req->base.tfm);
+	struct safexcel_crypto_priv *priv = ctx->priv;
+	struct safexcel_command_desc *cdesc;
+	struct safexcel_result_desc *rdesc;
+	struct scatterlist *sg;
+	int nr_src, nr_dst, n_cdesc = 0, n_rdesc = 0, queued = req->nbytes;
+	int i, ret = 0;
+
+	request->req = &req->base;
+
+	if (req->src == req->dst) {
+		nr_src = dma_map_sg(priv->dev, req->src,
+				    sg_nents_for_len(req->src, req->nbytes),
+				    DMA_BIDIRECTIONAL);
+		nr_dst = nr_src;
+		if (!nr_src)
+			return -EINVAL;
+	} else {
+		nr_src = dma_map_sg(priv->dev, req->src,
+				    sg_nents_for_len(req->src, req->nbytes),
+				    DMA_TO_DEVICE);
+		if (!nr_src)
+			return -EINVAL;
+
+		nr_dst = dma_map_sg(priv->dev, req->dst,
+				    sg_nents_for_len(req->dst, req->nbytes),
+				    DMA_FROM_DEVICE);
+		if (!nr_dst) {
+			dma_unmap_sg(priv->dev, req->src,
+				     sg_nents_for_len(req->src, req->nbytes),

drivers/crypto/inside-secure/safexcel_cipher.c line 218

Quoted 6 lines

drivers/crypto/inside-secure/safexcel_cipher.c new file +6 -0

+				     DMA_TO_DEVICE);
+			return -EINVAL;
+		}
+	}
+
+	memcpy(ctx->base.ctxr->data, ctx->key, ctx->key_len);

Is ctxr->data properly zeroized?

Add support for Inside Secure SafeXcel EIP197 cryptographic engine, which can be found on Marvell Armada 7k and 8k boards. This driver currently implements: ecb(aes), cbc(aes), sha1, sha224, sha256 and hmac(sah1) algorithms.

Two firmwares are needed for this engine to work. Their are mostly used for more advanced operations than the ones supported (as of now), but we still need them to pass the data to the internal cryptographic engine.

Signed-off-by Antoine Tenart <[email protected]>

drivers/crypto/inside-secure/safexcel.c new file +12 -0

new file mode 100644
index 000000000000..9cb3d0832835
@@ -0,0 +1,925 @@
+/*
+ * Copyright (C) 2017 Marvell
+ *
+ * Antoine Tenart <[email protected]>
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2. This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+
+#include <linux/clk.h>
+#include <linux/device.h>

Review

Quoted 2 lines · Antoine Tenart

(quoted diff) +2 -0

+
+#include <asm/dma-mapping.h>

<linux/dma-mapping.h> everywhere, please.

Other than that, the DMA aspects all look much nicer now, thanks.