* [PATCH V2 0/4] Add Xilinx's ZynqMP AES driver support
@ 2019-09-01 13:54 Kalyani Akula
2019-09-01 13:54 ` [PATCH V2 1/4] dt-bindings: crypto: Add bindings for ZynqMP AES driver Kalyani Akula
` (3 more replies)
0 siblings, 4 replies; 8+ messages in thread
From: Kalyani Akula @ 2019-09-01 13:54 UTC (permalink / raw)
To: herbert, kstewart, gregkh, tglx, pombredanne, linux-crypto,
linux-kernel, netdev
Cc: Kalyani Akula, Kalyani Akula
This patch set adds support for
- dt-binding docs for Xilinx ZynqMP AES driver
- Adds device tree node for ZynqMP AES driver
- Adds communication layer support for aes in zynqmp.c
- Adds Xilinx ZynqMP driver for AES Algorithm
V2 Changes :
- Converted RFC PATCH to PATCH
- Removed ALG_SET_KEY_TYPE that was added to support keytype
attribute. Taken using setkey interface.
- Removed deprecated BLKCIPHER in Kconfig
- Erased Key/IV from the buffer.
- Renamed zynqmp-aes driver to zynqmp-aes-gcm.
- Addressed few other review comments
Kalyani Akula (4):
dt-bindings: crypto: Add bindings for ZynqMP AES driver
ARM64: zynqmp: Add Xilinix AES node.
firmware: xilinx: Add ZynqMP aes API for AES functionality
crypto: Add Xilinx AES driver
.../devicetree/bindings/crypto/xlnx,zynqmp-aes.txt | 12 +
arch/arm64/boot/dts/xilinx/zynqmp.dtsi | 4 +
drivers/crypto/Kconfig | 11 +
drivers/crypto/Makefile | 1 +
drivers/crypto/zynqmp-aes-gcm.c | 297 +++++++++++++++++++++
drivers/firmware/xilinx/zynqmp.c | 23 ++
include/linux/firmware/xlnx-zynqmp.h | 2 +
7 files changed, 350 insertions(+)
create mode 100644 Documentation/devicetree/bindings/crypto/xlnx,zynqmp-aes.txt
create mode 100644 drivers/crypto/zynqmp-aes-gcm.c
--
1.8.3.1
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH V2 1/4] dt-bindings: crypto: Add bindings for ZynqMP AES driver 2019-09-01 13:54 [PATCH V2 0/4] Add Xilinx's ZynqMP AES driver support Kalyani Akula @ 2019-09-01 13:54 ` Kalyani Akula 2019-09-01 13:54 ` [PATCH V2 2/4] ARM64: zynqmp: Add Xilinix AES node Kalyani Akula ` (2 subsequent siblings) 3 siblings, 0 replies; 8+ messages in thread From: Kalyani Akula @ 2019-09-01 13:54 UTC (permalink / raw) To: herbert, kstewart, gregkh, tglx, pombredanne, linux-crypto, linux-kernel, netdev Cc: Kalyani Akula, Kalyani Akula Add documentation to describe Xilinx ZynqMP AES driver bindings. Signed-off-by: Kalyani Akula <kalyani.akula@xilinx.com> --- Documentation/devicetree/bindings/crypto/xlnx,zynqmp-aes.txt | 12 ++++++++++++ 1 file changed, 12 insertions(+) create mode 100644 Documentation/devicetree/bindings/crypto/xlnx,zynqmp-aes.txt diff --git a/Documentation/devicetree/bindings/crypto/xlnx,zynqmp-aes.txt b/Documentation/devicetree/bindings/crypto/xlnx,zynqmp-aes.txt new file mode 100644 index 0000000..226bfb9 --- /dev/null +++ b/Documentation/devicetree/bindings/crypto/xlnx,zynqmp-aes.txt @@ -0,0 +1,12 @@ +Xilinx ZynqMP AES hw acceleration support + +The ZynqMP PS-AES hw accelerator is used to encrypt/decrypt +the given user data. + +Required properties: +- compatible: should contain "xlnx,zynqmp-aes" + +Example: + zynqmp_aes { + compatible = "xlnx,zynqmp-aes"; + }; -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH V2 2/4] ARM64: zynqmp: Add Xilinix AES node. 2019-09-01 13:54 [PATCH V2 0/4] Add Xilinx's ZynqMP AES driver support Kalyani Akula 2019-09-01 13:54 ` [PATCH V2 1/4] dt-bindings: crypto: Add bindings for ZynqMP AES driver Kalyani Akula @ 2019-09-01 13:54 ` Kalyani Akula 2019-09-01 13:54 ` [PATCH V2 3/4] firmware: xilinx: Add ZynqMP aes API for AES functionality Kalyani Akula 2019-09-01 13:54 ` [PATCH V2 4/4] crypto: Add Xilinx AES driver Kalyani Akula 3 siblings, 0 replies; 8+ messages in thread From: Kalyani Akula @ 2019-09-01 13:54 UTC (permalink / raw) To: herbert, kstewart, gregkh, tglx, pombredanne, linux-crypto, linux-kernel, netdev Cc: Kalyani Akula, Kalyani Akula This patch adds a AES DT node for Xilinx ZynqMP SoC. Signed-off-by: Kalyani Akula <kalyani.akula@xilinx.com> --- arch/arm64/boot/dts/xilinx/zynqmp.dtsi | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/arch/arm64/boot/dts/xilinx/zynqmp.dtsi b/arch/arm64/boot/dts/xilinx/zynqmp.dtsi index 9aa6734..b41febc 100644 --- a/arch/arm64/boot/dts/xilinx/zynqmp.dtsi +++ b/arch/arm64/boot/dts/xilinx/zynqmp.dtsi @@ -124,6 +124,10 @@ <1 10 0xf08>; }; + xlnx_aes: zynqmp_aes { + compatible = "xlnx,zynqmp-aes"; + }; + amba_apu: amba-apu@0 { compatible = "simple-bus"; #address-cells = <2>; -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH V2 3/4] firmware: xilinx: Add ZynqMP aes API for AES functionality 2019-09-01 13:54 [PATCH V2 0/4] Add Xilinx's ZynqMP AES driver support Kalyani Akula 2019-09-01 13:54 ` [PATCH V2 1/4] dt-bindings: crypto: Add bindings for ZynqMP AES driver Kalyani Akula 2019-09-01 13:54 ` [PATCH V2 2/4] ARM64: zynqmp: Add Xilinix AES node Kalyani Akula @ 2019-09-01 13:54 ` Kalyani Akula 2019-09-01 13:54 ` [PATCH V2 4/4] crypto: Add Xilinx AES driver Kalyani Akula 3 siblings, 0 replies; 8+ messages in thread From: Kalyani Akula @ 2019-09-01 13:54 UTC (permalink / raw) To: herbert, kstewart, gregkh, tglx, pombredanne, linux-crypto, linux-kernel, netdev Cc: Kalyani Akula, Kalyani Akula Add ZynqMP firmware AES API to perform encryption/decryption of given data. Signed-off-by: Kalyani Akula <kalyani.akula@xilinx.com> --- drivers/firmware/xilinx/zynqmp.c | 23 +++++++++++++++++++++++ include/linux/firmware/xlnx-zynqmp.h | 2 ++ 2 files changed, 25 insertions(+) diff --git a/drivers/firmware/xilinx/zynqmp.c b/drivers/firmware/xilinx/zynqmp.c index fd3d837..74f3354 100644 --- a/drivers/firmware/xilinx/zynqmp.c +++ b/drivers/firmware/xilinx/zynqmp.c @@ -663,6 +663,28 @@ static int zynqmp_pm_set_requirement(const u32 node, const u32 capabilities, return zynqmp_pm_invoke_fn(PM_SET_REQUIREMENT, node, capabilities, qos, ack, NULL); } +/** + * zynqmp_pm_aes - Access AES hardware to encrypt/decrypt the data using + * AES-GCM core. + * @address: Address of the AesParams structure. + * @out: Returned output value + * + * Return: Returns status, either success or error code. + */ +static int zynqmp_pm_aes_engine(const u64 address, u32 *out) +{ + u32 ret_payload[PAYLOAD_ARG_CNT]; + int ret; + + if (!out) + return -EINVAL; + + ret = zynqmp_pm_invoke_fn(PM_SECURE_AES, upper_32_bits(address), + lower_32_bits(address), + 0, 0, ret_payload); + *out = ret_payload[1]; + return ret; +} static const struct zynqmp_eemi_ops eemi_ops = { .get_api_version = zynqmp_pm_get_api_version, @@ -687,6 +709,7 @@ static int zynqmp_pm_set_requirement(const u32 node, const u32 capabilities, .set_requirement = zynqmp_pm_set_requirement, .fpga_load = zynqmp_pm_fpga_load, .fpga_get_status = zynqmp_pm_fpga_get_status, + .aes = zynqmp_pm_aes_engine, }; /** diff --git a/include/linux/firmware/xlnx-zynqmp.h b/include/linux/firmware/xlnx-zynqmp.h index 778abbb..508edd7 100644 --- a/include/linux/firmware/xlnx-zynqmp.h +++ b/include/linux/firmware/xlnx-zynqmp.h @@ -77,6 +77,7 @@ enum pm_api_id { PM_CLOCK_GETRATE, PM_CLOCK_SETPARENT, PM_CLOCK_GETPARENT, + PM_SECURE_AES = 47, }; /* PMU-FW return status codes */ @@ -294,6 +295,7 @@ struct zynqmp_eemi_ops { const u32 capabilities, const u32 qos, const enum zynqmp_pm_request_ack ack); + int (*aes)(const u64 address, u32 *out); }; int zynqmp_pm_invoke_fn(u32 pm_api_id, u32 arg0, u32 arg1, -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH V2 4/4] crypto: Add Xilinx AES driver 2019-09-01 13:54 [PATCH V2 0/4] Add Xilinx's ZynqMP AES driver support Kalyani Akula ` (2 preceding siblings ...) 2019-09-01 13:54 ` [PATCH V2 3/4] firmware: xilinx: Add ZynqMP aes API for AES functionality Kalyani Akula @ 2019-09-01 13:54 ` Kalyani Akula 2019-09-02 6:58 ` Corentin Labbe 3 siblings, 1 reply; 8+ messages in thread From: Kalyani Akula @ 2019-09-01 13:54 UTC (permalink / raw) To: herbert, kstewart, gregkh, tglx, pombredanne, linux-crypto, linux-kernel, netdev Cc: Kalyani Akula, Kalyani Akula This patch adds AES driver support for the Xilinx ZynqMP SoC. Signed-off-by: Kalyani Akula <kalyani.akula@xilinx.com> --- drivers/crypto/Kconfig | 11 ++ drivers/crypto/Makefile | 1 + drivers/crypto/zynqmp-aes-gcm.c | 297 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 309 insertions(+) create mode 100644 drivers/crypto/zynqmp-aes-gcm.c diff --git a/drivers/crypto/Kconfig b/drivers/crypto/Kconfig index 603413f..a0d058a 100644 --- a/drivers/crypto/Kconfig +++ b/drivers/crypto/Kconfig @@ -677,6 +677,17 @@ config CRYPTO_DEV_ROCKCHIP This driver interfaces with the hardware crypto accelerator. Supporting cbc/ecb chainmode, and aes/des/des3_ede cipher mode. +config CRYPTO_DEV_ZYNQMP_AES + tristate "Support for Xilinx ZynqMP AES hw accelerator" + depends on ARCH_ZYNQMP || COMPILE_TEST + select CRYPTO_AES + select CRYPTO_SKCIPHER + help + Xilinx ZynqMP has AES-GCM engine used for symmetric key + encryption and decryption. This driver interfaces with AES hw + accelerator. Select this if you want to use the ZynqMP module + for AES algorithms. + config CRYPTO_DEV_MEDIATEK tristate "MediaTek's EIP97 Cryptographic Engine driver" depends on (ARM && ARCH_MEDIATEK) || COMPILE_TEST diff --git a/drivers/crypto/Makefile b/drivers/crypto/Makefile index afc4753..c99663a 100644 --- a/drivers/crypto/Makefile +++ b/drivers/crypto/Makefile @@ -48,3 +48,4 @@ obj-$(CONFIG_CRYPTO_DEV_BCM_SPU) += bcm/ obj-$(CONFIG_CRYPTO_DEV_SAFEXCEL) += inside-secure/ obj-$(CONFIG_CRYPTO_DEV_ARTPEC6) += axis/ obj-y += hisilicon/ +obj-$(CONFIG_CRYPTO_DEV_ZYNQMP_AES) += zynqmp-aes-gcm.o diff --git a/drivers/crypto/zynqmp-aes-gcm.c b/drivers/crypto/zynqmp-aes-gcm.c new file mode 100644 index 0000000..d65f038 --- /dev/null +++ b/drivers/crypto/zynqmp-aes-gcm.c @@ -0,0 +1,297 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Xilinx ZynqMP AES Driver. + * Copyright (c) 2019 Xilinx Inc. + */ + +#include <crypto/aes.h> +#include <crypto/scatterwalk.h> +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/of_device.h> +#include <linux/scatterlist.h> +#include <linux/firmware/xlnx-zynqmp.h> + +#define ZYNQMP_AES_IV_SIZE 12 +#define ZYNQMP_AES_GCM_SIZE 16 +#define ZYNQMP_AES_KEY_SIZE 32 + +#define ZYNQMP_AES_DECRYPT 0 +#define ZYNQMP_AES_ENCRYPT 1 + +#define ZYNQMP_AES_KUP_KEY 0 +#define ZYNQMP_AES_DEVICE_KEY 1 +#define ZYNQMP_AES_PUF_KEY 2 + +#define ZYNQMP_AES_GCM_TAG_MISMATCH_ERR 0x01 +#define ZYNQMP_AES_SIZE_ERR 0x06 +#define ZYNQMP_AES_WRONG_KEY_SRC_ERR 0x13 +#define ZYNQMP_AES_PUF_NOT_PROGRAMMED 0xE300 + +#define ZYNQMP_AES_BLOCKSIZE 0x04 + +static const struct zynqmp_eemi_ops *eemi_ops; +struct zynqmp_aes_dev *aes_dd; + +struct zynqmp_aes_dev { + struct device *dev; +}; + +struct zynqmp_aes_op { + struct zynqmp_aes_dev *dd; + void *src; + void *dst; + int len; + u8 key[ZYNQMP_AES_KEY_SIZE]; + u8 *iv; + u32 keylen; + u32 keytype; +}; + +struct zynqmp_aes_data { + u64 src; + u64 iv; + u64 key; + u64 dst; + u64 size; + u64 optype; + u64 keysrc; +}; + +static int zynqmp_setkey_blk(struct crypto_tfm *tfm, const u8 *key, + unsigned int len) +{ + struct zynqmp_aes_op *op = crypto_tfm_ctx(tfm); + + if (((len != 1) && (len != ZYNQMP_AES_KEY_SIZE)) || (!key)) + return -EINVAL; + + if (len == 1) { + op->keytype = *key; + + if ((op->keytype < ZYNQMP_AES_KUP_KEY) || + (op->keytype > ZYNQMP_AES_PUF_KEY)) + return -EINVAL; + + } else if (len == ZYNQMP_AES_KEY_SIZE) { + op->keytype = ZYNQMP_AES_KUP_KEY; + op->keylen = len; + memcpy(op->key, key, len); + } + + return 0; +} + +static int zynqmp_aes_xcrypt(struct blkcipher_desc *desc, + struct scatterlist *dst, + struct scatterlist *src, + unsigned int nbytes, + unsigned int flags) +{ + struct zynqmp_aes_op *op = crypto_blkcipher_ctx(desc->tfm); + struct zynqmp_aes_dev *dd = aes_dd; + int err, ret, copy_bytes, src_data = 0, dst_data = 0; + dma_addr_t dma_addr, dma_addr_buf; + struct zynqmp_aes_data *abuf; + struct blkcipher_walk walk; + unsigned int data_size; + size_t dma_size; + char *kbuf; + + if (!eemi_ops->aes) + return -ENOTSUPP; + + if (op->keytype == ZYNQMP_AES_KUP_KEY) + dma_size = nbytes + ZYNQMP_AES_KEY_SIZE + + ZYNQMP_AES_IV_SIZE; + else + dma_size = nbytes + ZYNQMP_AES_IV_SIZE; + + kbuf = dma_alloc_coherent(dd->dev, dma_size, &dma_addr, GFP_KERNEL); + if (!kbuf) + return -ENOMEM; + + abuf = dma_alloc_coherent(dd->dev, sizeof(struct zynqmp_aes_data), + &dma_addr_buf, GFP_KERNEL); + if (!abuf) { + dma_free_coherent(dd->dev, dma_size, kbuf, dma_addr); + return -ENOMEM; + } + + data_size = nbytes; + blkcipher_walk_init(&walk, dst, src, data_size); + err = blkcipher_walk_virt(desc, &walk); + op->iv = walk.iv; + + while ((nbytes = walk.nbytes)) { + op->src = walk.src.virt.addr; + memcpy(kbuf + src_data, op->src, nbytes); + src_data = src_data + nbytes; + nbytes &= (ZYNQMP_AES_BLOCKSIZE - 1); + err = blkcipher_walk_done(desc, &walk, nbytes); + } + memcpy(kbuf + data_size, op->iv, ZYNQMP_AES_IV_SIZE); + abuf->src = dma_addr; + abuf->dst = dma_addr; + abuf->iv = abuf->src + data_size; + abuf->size = data_size - ZYNQMP_AES_GCM_SIZE; + abuf->optype = flags; + abuf->keysrc = op->keytype; + + if (op->keytype == ZYNQMP_AES_KUP_KEY) { + memcpy(kbuf + data_size + ZYNQMP_AES_IV_SIZE, + op->key, ZYNQMP_AES_KEY_SIZE); + + abuf->key = abuf->src + data_size + ZYNQMP_AES_IV_SIZE; + } else { + abuf->key = 0; + } + eemi_ops->aes(dma_addr_buf, &ret); + + if (ret != 0) { + switch (ret) { + case ZYNQMP_AES_GCM_TAG_MISMATCH_ERR: + dev_err(dd->dev, "ERROR: Gcm Tag mismatch\n\r"); + break; + case ZYNQMP_AES_SIZE_ERR: + dev_err(dd->dev, "ERROR : Non word aligned data\n\r"); + break; + case ZYNQMP_AES_WRONG_KEY_SRC_ERR: + dev_err(dd->dev, "ERROR: Wrong KeySrc, enable secure mode\n\r"); + break; + case ZYNQMP_AES_PUF_NOT_PROGRAMMED: + dev_err(dd->dev, "ERROR: PUF is not registered\r\n"); + break; + default: + dev_err(dd->dev, "ERROR: Invalid"); + break; + } + goto END; + } + if (flags) + copy_bytes = data_size; + else + copy_bytes = data_size - ZYNQMP_AES_GCM_SIZE; + + blkcipher_walk_init(&walk, dst, src, copy_bytes); + err = blkcipher_walk_virt(desc, &walk); + + while ((nbytes = walk.nbytes)) { + memcpy(walk.dst.virt.addr, kbuf + dst_data, nbytes); + dst_data = dst_data + nbytes; + nbytes &= (ZYNQMP_AES_BLOCKSIZE - 1); + err = blkcipher_walk_done(desc, &walk, nbytes); + } +END: + memset(kbuf, 0, dma_size); + memset(abuf, 0, sizeof(struct zynqmp_aes_data)); + dma_free_coherent(dd->dev, dma_size, kbuf, dma_addr); + dma_free_coherent(dd->dev, sizeof(struct zynqmp_aes_data), + abuf, dma_addr_buf); + return err; +} + +static int zynqmp_aes_decrypt(struct blkcipher_desc *desc, + struct scatterlist *dst, + struct scatterlist *src, + unsigned int nbytes) +{ + return zynqmp_aes_xcrypt(desc, dst, src, nbytes, ZYNQMP_AES_DECRYPT); +} + +static int zynqmp_aes_encrypt(struct blkcipher_desc *desc, + struct scatterlist *dst, + struct scatterlist *src, + unsigned int nbytes) +{ + return zynqmp_aes_xcrypt(desc, dst, src, nbytes, ZYNQMP_AES_ENCRYPT); +} + +static struct crypto_alg zynqmp_alg = { + .cra_name = "xilinx-zynqmp-aes", + .cra_driver_name = "zynqmp-aes-gcm", + .cra_priority = 400, + .cra_flags = CRYPTO_ALG_TYPE_BLKCIPHER | + CRYPTO_ALG_KERN_DRIVER_ONLY, + .cra_blocksize = ZYNQMP_AES_BLOCKSIZE, + .cra_ctxsize = sizeof(struct zynqmp_aes_op), + .cra_alignmask = 15, + .cra_type = &crypto_blkcipher_type, + .cra_module = THIS_MODULE, + .cra_u = { + .blkcipher = { + .min_keysize = 0, + .max_keysize = ZYNQMP_AES_KEY_SIZE, + .setkey = zynqmp_setkey_blk, + .encrypt = zynqmp_aes_encrypt, + .decrypt = zynqmp_aes_decrypt, + .ivsize = ZYNQMP_AES_IV_SIZE, + } + } +}; + +static const struct of_device_id zynqmp_aes_dt_ids[] = { + { .compatible = "xlnx,zynqmp-aes" }, + { /* sentinel */ } +}; +MODULE_DEVICE_TABLE(of, zynqmp_aes_dt_ids); + +static int zynqmp_aes_probe(struct platform_device *pdev) +{ + struct device *dev = &pdev->dev; + int ret; + + eemi_ops = zynqmp_pm_get_eemi_ops(); + + if (IS_ERR(eemi_ops)) + return PTR_ERR(eemi_ops); + + aes_dd = devm_kzalloc(dev, sizeof(*aes_dd), GFP_KERNEL); + if (!aes_dd) + return -ENOMEM; + + aes_dd->dev = dev; + platform_set_drvdata(pdev, aes_dd); + + ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(44)); + if (ret < 0) { + dev_err(dev, "no usable DMA configuration"); + return ret; + } + + ret = crypto_register_alg(&zynqmp_alg); + if (ret) + goto err_algs; + + dev_info(dev, "AES Successfully Registered\n\r"); + return 0; + +err_algs: + return ret; +} + +static int zynqmp_aes_remove(struct platform_device *pdev) +{ + aes_dd = platform_get_drvdata(pdev); + if (!aes_dd) + return -ENODEV; + crypto_unregister_alg(&zynqmp_alg); + + return 0; +} + +static struct platform_driver xilinx_aes_driver = { + .probe = zynqmp_aes_probe, + .remove = zynqmp_aes_remove, + .driver = { + .name = "zynqmp_aes", + .of_match_table = of_match_ptr(zynqmp_aes_dt_ids), + }, +}; + +module_platform_driver(xilinx_aes_driver); + +MODULE_DESCRIPTION("Xilinx ZynqMP AES hw acceleration support."); +MODULE_LICENSE("GPL v2"); +MODULE_AUTHOR("Nava kishore Manne <nava.manne@xilinx.com>"); +MODULE_AUTHOR("Kalyani Akula <kalyani.akula@xilinx.com>"); -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH V2 4/4] crypto: Add Xilinx AES driver 2019-09-01 13:54 ` [PATCH V2 4/4] crypto: Add Xilinx AES driver Kalyani Akula @ 2019-09-02 6:58 ` Corentin Labbe 2019-09-04 17:40 ` Kalyani Akula 0 siblings, 1 reply; 8+ messages in thread From: Corentin Labbe @ 2019-09-02 6:58 UTC (permalink / raw) To: Kalyani Akula Cc: herbert, kstewart, gregkh, tglx, pombredanne, linux-crypto, linux-kernel, netdev, Kalyani Akula On Sun, Sep 01, 2019 at 07:24:58PM +0530, Kalyani Akula wrote: > This patch adds AES driver support for the Xilinx > ZynqMP SoC. > > Signed-off-by: Kalyani Akula <kalyani.akula@xilinx.com> > --- Hello I have some comment below > drivers/crypto/Kconfig | 11 ++ > drivers/crypto/Makefile | 1 + > drivers/crypto/zynqmp-aes-gcm.c | 297 ++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 309 insertions(+) > create mode 100644 drivers/crypto/zynqmp-aes-gcm.c > > diff --git a/drivers/crypto/Kconfig b/drivers/crypto/Kconfig > index 603413f..a0d058a 100644 > --- a/drivers/crypto/Kconfig > +++ b/drivers/crypto/Kconfig > @@ -677,6 +677,17 @@ config CRYPTO_DEV_ROCKCHIP > This driver interfaces with the hardware crypto accelerator. > Supporting cbc/ecb chainmode, and aes/des/des3_ede cipher mode. > > +config CRYPTO_DEV_ZYNQMP_AES > + tristate "Support for Xilinx ZynqMP AES hw accelerator" > + depends on ARCH_ZYNQMP || COMPILE_TEST > + select CRYPTO_AES > + select CRYPTO_SKCIPHER > + help > + Xilinx ZynqMP has AES-GCM engine used for symmetric key > + encryption and decryption. This driver interfaces with AES hw > + accelerator. Select this if you want to use the ZynqMP module > + for AES algorithms. > + > config CRYPTO_DEV_MEDIATEK > tristate "MediaTek's EIP97 Cryptographic Engine driver" > depends on (ARM && ARCH_MEDIATEK) || COMPILE_TEST > diff --git a/drivers/crypto/Makefile b/drivers/crypto/Makefile > index afc4753..c99663a 100644 > --- a/drivers/crypto/Makefile > +++ b/drivers/crypto/Makefile > @@ -48,3 +48,4 @@ obj-$(CONFIG_CRYPTO_DEV_BCM_SPU) += bcm/ > obj-$(CONFIG_CRYPTO_DEV_SAFEXCEL) += inside-secure/ > obj-$(CONFIG_CRYPTO_DEV_ARTPEC6) += axis/ > obj-y += hisilicon/ > +obj-$(CONFIG_CRYPTO_DEV_ZYNQMP_AES) += zynqmp-aes-gcm.o > diff --git a/drivers/crypto/zynqmp-aes-gcm.c b/drivers/crypto/zynqmp-aes-gcm.c > new file mode 100644 > index 0000000..d65f038 > --- /dev/null > +++ b/drivers/crypto/zynqmp-aes-gcm.c > @@ -0,0 +1,297 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Xilinx ZynqMP AES Driver. > + * Copyright (c) 2019 Xilinx Inc. > + */ > + > +#include <crypto/aes.h> > +#include <crypto/scatterwalk.h> > +#include <linux/kernel.h> > +#include <linux/module.h> > +#include <linux/of_device.h> > +#include <linux/scatterlist.h> > +#include <linux/firmware/xlnx-zynqmp.h> > + > +#define ZYNQMP_AES_IV_SIZE 12 > +#define ZYNQMP_AES_GCM_SIZE 16 > +#define ZYNQMP_AES_KEY_SIZE 32 > + > +#define ZYNQMP_AES_DECRYPT 0 > +#define ZYNQMP_AES_ENCRYPT 1 > + > +#define ZYNQMP_AES_KUP_KEY 0 > +#define ZYNQMP_AES_DEVICE_KEY 1 > +#define ZYNQMP_AES_PUF_KEY 2 > + > +#define ZYNQMP_AES_GCM_TAG_MISMATCH_ERR 0x01 > +#define ZYNQMP_AES_SIZE_ERR 0x06 > +#define ZYNQMP_AES_WRONG_KEY_SRC_ERR 0x13 > +#define ZYNQMP_AES_PUF_NOT_PROGRAMMED 0xE300 > + > +#define ZYNQMP_AES_BLOCKSIZE 0x04 > + > +static const struct zynqmp_eemi_ops *eemi_ops; > +struct zynqmp_aes_dev *aes_dd; I still think that using a global variable for storing device driver data is bad. > + > +struct zynqmp_aes_dev { > + struct device *dev; > +}; > + > +struct zynqmp_aes_op { > + struct zynqmp_aes_dev *dd; > + void *src; > + void *dst; > + int len; > + u8 key[ZYNQMP_AES_KEY_SIZE]; > + u8 *iv; > + u32 keylen; > + u32 keytype; > +}; > + > +struct zynqmp_aes_data { > + u64 src; > + u64 iv; > + u64 key; > + u64 dst; > + u64 size; > + u64 optype; > + u64 keysrc; > +}; > + > +static int zynqmp_setkey_blk(struct crypto_tfm *tfm, const u8 *key, > + unsigned int len) > +{ > + struct zynqmp_aes_op *op = crypto_tfm_ctx(tfm); > + > + if (((len != 1) && (len != ZYNQMP_AES_KEY_SIZE)) || (!key)) typo, two space > + return -EINVAL; > + > + if (len == 1) { > + op->keytype = *key; > + > + if ((op->keytype < ZYNQMP_AES_KUP_KEY) || > + (op->keytype > ZYNQMP_AES_PUF_KEY)) > + return -EINVAL; > + > + } else if (len == ZYNQMP_AES_KEY_SIZE) { > + op->keytype = ZYNQMP_AES_KUP_KEY; > + op->keylen = len; > + memcpy(op->key, key, len); > + } > + > + return 0; > +} It seems your driver does not support AES keysize of 128/196, you need to fallback in that case. You need to comment the keylen=1 usecase and use a define for this value. > + > +static int zynqmp_aes_xcrypt(struct blkcipher_desc *desc, > + struct scatterlist *dst, > + struct scatterlist *src, > + unsigned int nbytes, > + unsigned int flags) > +{ > + struct zynqmp_aes_op *op = crypto_blkcipher_ctx(desc->tfm); > + struct zynqmp_aes_dev *dd = aes_dd; > + int err, ret, copy_bytes, src_data = 0, dst_data = 0; > + dma_addr_t dma_addr, dma_addr_buf; > + struct zynqmp_aes_data *abuf; > + struct blkcipher_walk walk; > + unsigned int data_size; > + size_t dma_size; > + char *kbuf; > + > + if (!eemi_ops->aes) > + return -ENOTSUPP; > + > + if (op->keytype == ZYNQMP_AES_KUP_KEY) > + dma_size = nbytes + ZYNQMP_AES_KEY_SIZE > + + ZYNQMP_AES_IV_SIZE; > + else > + dma_size = nbytes + ZYNQMP_AES_IV_SIZE; > + > + kbuf = dma_alloc_coherent(dd->dev, dma_size, &dma_addr, GFP_KERNEL); > + if (!kbuf) > + return -ENOMEM; > + > + abuf = dma_alloc_coherent(dd->dev, sizeof(struct zynqmp_aes_data), > + &dma_addr_buf, GFP_KERNEL); > + if (!abuf) { > + dma_free_coherent(dd->dev, dma_size, kbuf, dma_addr); > + return -ENOMEM; > + } > + > + data_size = nbytes; > + blkcipher_walk_init(&walk, dst, src, data_size); > + err = blkcipher_walk_virt(desc, &walk); > + op->iv = walk.iv; > + > + while ((nbytes = walk.nbytes)) { > + op->src = walk.src.virt.addr; > + memcpy(kbuf + src_data, op->src, nbytes); > + src_data = src_data + nbytes; > + nbytes &= (ZYNQMP_AES_BLOCKSIZE - 1); > + err = blkcipher_walk_done(desc, &walk, nbytes); > + } > + memcpy(kbuf + data_size, op->iv, ZYNQMP_AES_IV_SIZE); > + abuf->src = dma_addr; > + abuf->dst = dma_addr; > + abuf->iv = abuf->src + data_size; > + abuf->size = data_size - ZYNQMP_AES_GCM_SIZE; > + abuf->optype = flags; > + abuf->keysrc = op->keytype; > + > + if (op->keytype == ZYNQMP_AES_KUP_KEY) { > + memcpy(kbuf + data_size + ZYNQMP_AES_IV_SIZE, > + op->key, ZYNQMP_AES_KEY_SIZE); > + > + abuf->key = abuf->src + data_size + ZYNQMP_AES_IV_SIZE; > + } else { > + abuf->key = 0; > + } > + eemi_ops->aes(dma_addr_buf, &ret); > + > + if (ret != 0) { > + switch (ret) { > + case ZYNQMP_AES_GCM_TAG_MISMATCH_ERR: > + dev_err(dd->dev, "ERROR: Gcm Tag mismatch\n\r"); > + break; > + case ZYNQMP_AES_SIZE_ERR: > + dev_err(dd->dev, "ERROR : Non word aligned data\n\r"); > + break; > + case ZYNQMP_AES_WRONG_KEY_SRC_ERR: > + dev_err(dd->dev, "ERROR: Wrong KeySrc, enable secure mode\n\r"); > + break; > + case ZYNQMP_AES_PUF_NOT_PROGRAMMED: > + dev_err(dd->dev, "ERROR: PUF is not registered\r\n"); > + break; > + default: > + dev_err(dd->dev, "ERROR: Invalid"); > + break; > + } > + goto END; > + } > + if (flags) > + copy_bytes = data_size; > + else > + copy_bytes = data_size - ZYNQMP_AES_GCM_SIZE; > + > + blkcipher_walk_init(&walk, dst, src, copy_bytes); > + err = blkcipher_walk_virt(desc, &walk); > + > + while ((nbytes = walk.nbytes)) { > + memcpy(walk.dst.virt.addr, kbuf + dst_data, nbytes); > + dst_data = dst_data + nbytes; > + nbytes &= (ZYNQMP_AES_BLOCKSIZE - 1); > + err = blkcipher_walk_done(desc, &walk, nbytes); > + } > +END: > + memset(kbuf, 0, dma_size); > + memset(abuf, 0, sizeof(struct zynqmp_aes_data)); > + dma_free_coherent(dd->dev, dma_size, kbuf, dma_addr); > + dma_free_coherent(dd->dev, sizeof(struct zynqmp_aes_data), > + abuf, dma_addr_buf); > + return err; > +} > + > +static int zynqmp_aes_decrypt(struct blkcipher_desc *desc, > + struct scatterlist *dst, > + struct scatterlist *src, > + unsigned int nbytes) > +{ > + return zynqmp_aes_xcrypt(desc, dst, src, nbytes, ZYNQMP_AES_DECRYPT); > +} > + > +static int zynqmp_aes_encrypt(struct blkcipher_desc *desc, > + struct scatterlist *dst, > + struct scatterlist *src, > + unsigned int nbytes) > +{ > + return zynqmp_aes_xcrypt(desc, dst, src, nbytes, ZYNQMP_AES_ENCRYPT); > +} > + > +static struct crypto_alg zynqmp_alg = { > + .cra_name = "xilinx-zynqmp-aes", > + .cra_driver_name = "zynqmp-aes-gcm", > + .cra_priority = 400, > + .cra_flags = CRYPTO_ALG_TYPE_BLKCIPHER | > + CRYPTO_ALG_KERN_DRIVER_ONLY, > + .cra_blocksize = ZYNQMP_AES_BLOCKSIZE, > + .cra_ctxsize = sizeof(struct zynqmp_aes_op), > + .cra_alignmask = 15, > + .cra_type = &crypto_blkcipher_type, > + .cra_module = THIS_MODULE, > + .cra_u = { > + .blkcipher = { > + .min_keysize = 0, Are you sure to accept this a keysize of 0 ? Regards ^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [PATCH V2 4/4] crypto: Add Xilinx AES driver 2019-09-02 6:58 ` Corentin Labbe @ 2019-09-04 17:40 ` Kalyani Akula 2019-09-06 7:04 ` Corentin Labbe 0 siblings, 1 reply; 8+ messages in thread From: Kalyani Akula @ 2019-09-04 17:40 UTC (permalink / raw) To: Corentin Labbe Cc: herbert, kstewart, gregkh, tglx, pombredanne, linux-crypto, linux-kernel, netdev, Sarat Chand Savitala Hi Corentin, Thanks for the review comments. Please find my response/queries inline. > -----Original Message----- > From: Corentin Labbe <clabbe.montjoie@gmail.com> > Sent: Monday, September 2, 2019 12:29 PM > To: Kalyani Akula <kalyania@xilinx.com> > Cc: herbert@gondor.apana.org.au; kstewart@linuxfoundation.org; > gregkh@linuxfoundation.org; tglx@linutronix.de; pombredanne@nexb.com; > linux-crypto@vger.kernel.org; linux-kernel@vger.kernel.org; > netdev@vger.kernel.org; Kalyani Akula <kalyania@xilinx.com> > Subject: Re: [PATCH V2 4/4] crypto: Add Xilinx AES driver > > On Sun, Sep 01, 2019 at 07:24:58PM +0530, Kalyani Akula wrote: > > This patch adds AES driver support for the Xilinx ZynqMP SoC. > > > > Signed-off-by: Kalyani Akula <kalyani.akula@xilinx.com> > > --- > > Hello > > I have some comment below > > > drivers/crypto/Kconfig | 11 ++ > > drivers/crypto/Makefile | 1 + > > drivers/crypto/zynqmp-aes-gcm.c | 297 > > ++++++++++++++++++++++++++++++++++++++++ > > 3 files changed, 309 insertions(+) > > create mode 100644 drivers/crypto/zynqmp-aes-gcm.c > > > > diff --git a/drivers/crypto/Kconfig b/drivers/crypto/Kconfig index > > 603413f..a0d058a 100644 > > --- a/drivers/crypto/Kconfig > > +++ b/drivers/crypto/Kconfig > > @@ -677,6 +677,17 @@ config CRYPTO_DEV_ROCKCHIP > > This driver interfaces with the hardware crypto accelerator. > > Supporting cbc/ecb chainmode, and aes/des/des3_ede cipher mode. > > > > +config CRYPTO_DEV_ZYNQMP_AES > > + tristate "Support for Xilinx ZynqMP AES hw accelerator" > > + depends on ARCH_ZYNQMP || COMPILE_TEST > > + select CRYPTO_AES > > + select CRYPTO_SKCIPHER > > + help > > + Xilinx ZynqMP has AES-GCM engine used for symmetric key > > + encryption and decryption. This driver interfaces with AES hw > > + accelerator. Select this if you want to use the ZynqMP module > > + for AES algorithms. > > + > > config CRYPTO_DEV_MEDIATEK > > tristate "MediaTek's EIP97 Cryptographic Engine driver" > > depends on (ARM && ARCH_MEDIATEK) || COMPILE_TEST diff --git > > a/drivers/crypto/Makefile b/drivers/crypto/Makefile index > > afc4753..c99663a 100644 > > --- a/drivers/crypto/Makefile > > +++ b/drivers/crypto/Makefile > > @@ -48,3 +48,4 @@ obj-$(CONFIG_CRYPTO_DEV_BCM_SPU) += bcm/ > > obj-$(CONFIG_CRYPTO_DEV_SAFEXCEL) += inside-secure/ > > obj-$(CONFIG_CRYPTO_DEV_ARTPEC6) += axis/ obj-y += hisilicon/ > > +obj-$(CONFIG_CRYPTO_DEV_ZYNQMP_AES) += zynqmp-aes-gcm.o > > diff --git a/drivers/crypto/zynqmp-aes-gcm.c > > b/drivers/crypto/zynqmp-aes-gcm.c new file mode 100644 index > > 0000000..d65f038 > > --- /dev/null > > +++ b/drivers/crypto/zynqmp-aes-gcm.c > > @@ -0,0 +1,297 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * Xilinx ZynqMP AES Driver. > > + * Copyright (c) 2019 Xilinx Inc. > > + */ > > + > > +#include <crypto/aes.h> > > +#include <crypto/scatterwalk.h> > > +#include <linux/kernel.h> > > +#include <linux/module.h> > > +#include <linux/of_device.h> > > +#include <linux/scatterlist.h> > > +#include <linux/firmware/xlnx-zynqmp.h> > > + > > +#define ZYNQMP_AES_IV_SIZE 12 > > +#define ZYNQMP_AES_GCM_SIZE 16 > > +#define ZYNQMP_AES_KEY_SIZE 32 > > + > > +#define ZYNQMP_AES_DECRYPT 0 > > +#define ZYNQMP_AES_ENCRYPT 1 > > + > > +#define ZYNQMP_AES_KUP_KEY 0 > > +#define ZYNQMP_AES_DEVICE_KEY 1 > > +#define ZYNQMP_AES_PUF_KEY 2 > > + > > +#define ZYNQMP_AES_GCM_TAG_MISMATCH_ERR 0x01 > > +#define ZYNQMP_AES_SIZE_ERR 0x06 > > +#define ZYNQMP_AES_WRONG_KEY_SRC_ERR 0x13 > > +#define ZYNQMP_AES_PUF_NOT_PROGRAMMED 0xE300 > > + > > +#define ZYNQMP_AES_BLOCKSIZE 0x04 > > + > > +static const struct zynqmp_eemi_ops *eemi_ops; struct zynqmp_aes_dev > > +*aes_dd; > > I still think that using a global variable for storing device driver data is bad. I think storing the list of dd's would solve up the issue with global variable, but there is only one AES instance here. Please suggest > > > + > > +struct zynqmp_aes_dev { > > + struct device *dev; > > +}; > > + > > +struct zynqmp_aes_op { > > + struct zynqmp_aes_dev *dd; > > + void *src; > > + void *dst; > > + int len; > > + u8 key[ZYNQMP_AES_KEY_SIZE]; > > + u8 *iv; > > + u32 keylen; > > + u32 keytype; > > +}; > > + > > +struct zynqmp_aes_data { > > + u64 src; > > + u64 iv; > > + u64 key; > > + u64 dst; > > + u64 size; > > + u64 optype; > > + u64 keysrc; > > +}; > > + > > +static int zynqmp_setkey_blk(struct crypto_tfm *tfm, const u8 *key, > > + unsigned int len) > > +{ > > + struct zynqmp_aes_op *op = crypto_tfm_ctx(tfm); > > + > > + if (((len != 1) && (len != ZYNQMP_AES_KEY_SIZE)) || (!key)) > > typo, two space Will fix in the next version > > > + return -EINVAL; > > + > > + if (len == 1) { > > + op->keytype = *key; > > + > > + if ((op->keytype < ZYNQMP_AES_KUP_KEY) || > > + (op->keytype > ZYNQMP_AES_PUF_KEY)) > > + return -EINVAL; > > + > > + } else if (len == ZYNQMP_AES_KEY_SIZE) { > > + op->keytype = ZYNQMP_AES_KUP_KEY; > > + op->keylen = len; > > + memcpy(op->key, key, len); > > + } > > + > > + return 0; > > +} > > It seems your driver does not support AES keysize of 128/196, you need to > fallback in that case. [Kalyani] In case of 128/196 keysize, returning the error would suffice ? Or still algorithm need to work ? If error is enough, it is taken care by this condition if (((len != 1) && (len != ZYNQMP_AES_KEY_SIZE)) || (!key)) > You need to comment the keylen=1 usecase and use a define for this value. > Will fix in next version > > + > > +static int zynqmp_aes_xcrypt(struct blkcipher_desc *desc, > > + struct scatterlist *dst, > > + struct scatterlist *src, > > + unsigned int nbytes, > > + unsigned int flags) > > +{ > > + struct zynqmp_aes_op *op = crypto_blkcipher_ctx(desc->tfm); > > + struct zynqmp_aes_dev *dd = aes_dd; > > + int err, ret, copy_bytes, src_data = 0, dst_data = 0; > > + dma_addr_t dma_addr, dma_addr_buf; > > + struct zynqmp_aes_data *abuf; > > + struct blkcipher_walk walk; > > + unsigned int data_size; > > + size_t dma_size; > > + char *kbuf; > > + > > + if (!eemi_ops->aes) > > + return -ENOTSUPP; > > + > > + if (op->keytype == ZYNQMP_AES_KUP_KEY) > > + dma_size = nbytes + ZYNQMP_AES_KEY_SIZE > > + + ZYNQMP_AES_IV_SIZE; > > + else > > + dma_size = nbytes + ZYNQMP_AES_IV_SIZE; > > + > > + kbuf = dma_alloc_coherent(dd->dev, dma_size, &dma_addr, > GFP_KERNEL); > > + if (!kbuf) > > + return -ENOMEM; > > + > > + abuf = dma_alloc_coherent(dd->dev, sizeof(struct zynqmp_aes_data), > > + &dma_addr_buf, GFP_KERNEL); > > + if (!abuf) { > > + dma_free_coherent(dd->dev, dma_size, kbuf, dma_addr); > > + return -ENOMEM; > > + } > > + > > + data_size = nbytes; > > + blkcipher_walk_init(&walk, dst, src, data_size); > > + err = blkcipher_walk_virt(desc, &walk); > > + op->iv = walk.iv; > > + > > + while ((nbytes = walk.nbytes)) { > > + op->src = walk.src.virt.addr; > > + memcpy(kbuf + src_data, op->src, nbytes); > > + src_data = src_data + nbytes; > > + nbytes &= (ZYNQMP_AES_BLOCKSIZE - 1); > > + err = blkcipher_walk_done(desc, &walk, nbytes); > > + } > > + memcpy(kbuf + data_size, op->iv, ZYNQMP_AES_IV_SIZE); > > + abuf->src = dma_addr; > > + abuf->dst = dma_addr; > > + abuf->iv = abuf->src + data_size; > > + abuf->size = data_size - ZYNQMP_AES_GCM_SIZE; > > + abuf->optype = flags; > > + abuf->keysrc = op->keytype; > > + > > + if (op->keytype == ZYNQMP_AES_KUP_KEY) { > > + memcpy(kbuf + data_size + ZYNQMP_AES_IV_SIZE, > > + op->key, ZYNQMP_AES_KEY_SIZE); > > + > > + abuf->key = abuf->src + data_size + ZYNQMP_AES_IV_SIZE; > > + } else { > > + abuf->key = 0; > > + } > > + eemi_ops->aes(dma_addr_buf, &ret); > > + > > + if (ret != 0) { > > + switch (ret) { > > + case ZYNQMP_AES_GCM_TAG_MISMATCH_ERR: > > + dev_err(dd->dev, "ERROR: Gcm Tag mismatch\n\r"); > > + break; > > + case ZYNQMP_AES_SIZE_ERR: > > + dev_err(dd->dev, "ERROR : Non word aligned > data\n\r"); > > + break; > > + case ZYNQMP_AES_WRONG_KEY_SRC_ERR: > > + dev_err(dd->dev, "ERROR: Wrong KeySrc, enable secure > mode\n\r"); > > + break; > > + case ZYNQMP_AES_PUF_NOT_PROGRAMMED: > > + dev_err(dd->dev, "ERROR: PUF is not registered\r\n"); > > + break; > > + default: > > + dev_err(dd->dev, "ERROR: Invalid"); > > + break; > > + } > > + goto END; > > + } > > + if (flags) > > + copy_bytes = data_size; > > + else > > + copy_bytes = data_size - ZYNQMP_AES_GCM_SIZE; > > + > > + blkcipher_walk_init(&walk, dst, src, copy_bytes); > > + err = blkcipher_walk_virt(desc, &walk); > > + > > + while ((nbytes = walk.nbytes)) { > > + memcpy(walk.dst.virt.addr, kbuf + dst_data, nbytes); > > + dst_data = dst_data + nbytes; > > + nbytes &= (ZYNQMP_AES_BLOCKSIZE - 1); > > + err = blkcipher_walk_done(desc, &walk, nbytes); > > + } > > +END: > > + memset(kbuf, 0, dma_size); > > + memset(abuf, 0, sizeof(struct zynqmp_aes_data)); > > + dma_free_coherent(dd->dev, dma_size, kbuf, dma_addr); > > + dma_free_coherent(dd->dev, sizeof(struct zynqmp_aes_data), > > + abuf, dma_addr_buf); > > + return err; > > +} > > + > > +static int zynqmp_aes_decrypt(struct blkcipher_desc *desc, > > + struct scatterlist *dst, > > + struct scatterlist *src, > > + unsigned int nbytes) > > +{ > > + return zynqmp_aes_xcrypt(desc, dst, src, nbytes, > > +ZYNQMP_AES_DECRYPT); } > > + > > +static int zynqmp_aes_encrypt(struct blkcipher_desc *desc, > > + struct scatterlist *dst, > > + struct scatterlist *src, > > + unsigned int nbytes) > > +{ > > + return zynqmp_aes_xcrypt(desc, dst, src, nbytes, > > +ZYNQMP_AES_ENCRYPT); } > > + > > +static struct crypto_alg zynqmp_alg = { > > + .cra_name = "xilinx-zynqmp-aes", > > + .cra_driver_name = "zynqmp-aes-gcm", > > + .cra_priority = 400, > > + .cra_flags = CRYPTO_ALG_TYPE_BLKCIPHER | > > + CRYPTO_ALG_KERN_DRIVER_ONLY, > > + .cra_blocksize = ZYNQMP_AES_BLOCKSIZE, > > + .cra_ctxsize = sizeof(struct zynqmp_aes_op), > > + .cra_alignmask = 15, > > + .cra_type = &crypto_blkcipher_type, > > + .cra_module = THIS_MODULE, > > + .cra_u = { > > + .blkcipher = { > > + .min_keysize = 0, > > Are you sure to accept this a keysize of 0 ? > Will correct in next version Regards kalyani > Regards ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH V2 4/4] crypto: Add Xilinx AES driver 2019-09-04 17:40 ` Kalyani Akula @ 2019-09-06 7:04 ` Corentin Labbe 0 siblings, 0 replies; 8+ messages in thread From: Corentin Labbe @ 2019-09-06 7:04 UTC (permalink / raw) To: Kalyani Akula Cc: herbert, kstewart, gregkh, tglx, pombredanne, linux-crypto, linux-kernel, netdev, Sarat Chand Savitala On Wed, Sep 04, 2019 at 05:40:22PM +0000, Kalyani Akula wrote: > Hi Corentin, > > Thanks for the review comments. > Please find my response/queries inline. > > > -----Original Message----- > > From: Corentin Labbe <clabbe.montjoie@gmail.com> > > Sent: Monday, September 2, 2019 12:29 PM > > To: Kalyani Akula <kalyania@xilinx.com> > > Cc: herbert@gondor.apana.org.au; kstewart@linuxfoundation.org; > > gregkh@linuxfoundation.org; tglx@linutronix.de; pombredanne@nexb.com; > > linux-crypto@vger.kernel.org; linux-kernel@vger.kernel.org; > > netdev@vger.kernel.org; Kalyani Akula <kalyania@xilinx.com> > > Subject: Re: [PATCH V2 4/4] crypto: Add Xilinx AES driver > > > > On Sun, Sep 01, 2019 at 07:24:58PM +0530, Kalyani Akula wrote: > > > This patch adds AES driver support for the Xilinx ZynqMP SoC. > > > > > > Signed-off-by: Kalyani Akula <kalyani.akula@xilinx.com> > > > --- > > > > Hello > > > > I have some comment below > > > > > drivers/crypto/Kconfig | 11 ++ > > > drivers/crypto/Makefile | 1 + > > > drivers/crypto/zynqmp-aes-gcm.c | 297 > > > ++++++++++++++++++++++++++++++++++++++++ > > > 3 files changed, 309 insertions(+) > > > create mode 100644 drivers/crypto/zynqmp-aes-gcm.c > > > > > > diff --git a/drivers/crypto/Kconfig b/drivers/crypto/Kconfig index > > > 603413f..a0d058a 100644 > > > --- a/drivers/crypto/Kconfig > > > +++ b/drivers/crypto/Kconfig > > > @@ -677,6 +677,17 @@ config CRYPTO_DEV_ROCKCHIP > > > This driver interfaces with the hardware crypto accelerator. > > > Supporting cbc/ecb chainmode, and aes/des/des3_ede cipher mode. > > > > > > +config CRYPTO_DEV_ZYNQMP_AES > > > + tristate "Support for Xilinx ZynqMP AES hw accelerator" > > > + depends on ARCH_ZYNQMP || COMPILE_TEST > > > + select CRYPTO_AES > > > + select CRYPTO_SKCIPHER > > > + help > > > + Xilinx ZynqMP has AES-GCM engine used for symmetric key > > > + encryption and decryption. This driver interfaces with AES hw > > > + accelerator. Select this if you want to use the ZynqMP module > > > + for AES algorithms. > > > + > > > config CRYPTO_DEV_MEDIATEK > > > tristate "MediaTek's EIP97 Cryptographic Engine driver" > > > depends on (ARM && ARCH_MEDIATEK) || COMPILE_TEST diff --git > > > a/drivers/crypto/Makefile b/drivers/crypto/Makefile index > > > afc4753..c99663a 100644 > > > --- a/drivers/crypto/Makefile > > > +++ b/drivers/crypto/Makefile > > > @@ -48,3 +48,4 @@ obj-$(CONFIG_CRYPTO_DEV_BCM_SPU) += bcm/ > > > obj-$(CONFIG_CRYPTO_DEV_SAFEXCEL) += inside-secure/ > > > obj-$(CONFIG_CRYPTO_DEV_ARTPEC6) += axis/ obj-y += hisilicon/ > > > +obj-$(CONFIG_CRYPTO_DEV_ZYNQMP_AES) += zynqmp-aes-gcm.o > > > diff --git a/drivers/crypto/zynqmp-aes-gcm.c > > > b/drivers/crypto/zynqmp-aes-gcm.c new file mode 100644 index > > > 0000000..d65f038 > > > --- /dev/null > > > +++ b/drivers/crypto/zynqmp-aes-gcm.c > > > @@ -0,0 +1,297 @@ > > > +// SPDX-License-Identifier: GPL-2.0 > > > +/* > > > + * Xilinx ZynqMP AES Driver. > > > + * Copyright (c) 2019 Xilinx Inc. > > > + */ > > > + > > > +#include <crypto/aes.h> > > > +#include <crypto/scatterwalk.h> > > > +#include <linux/kernel.h> > > > +#include <linux/module.h> > > > +#include <linux/of_device.h> > > > +#include <linux/scatterlist.h> > > > +#include <linux/firmware/xlnx-zynqmp.h> > > > + > > > +#define ZYNQMP_AES_IV_SIZE 12 > > > +#define ZYNQMP_AES_GCM_SIZE 16 > > > +#define ZYNQMP_AES_KEY_SIZE 32 > > > + > > > +#define ZYNQMP_AES_DECRYPT 0 > > > +#define ZYNQMP_AES_ENCRYPT 1 > > > + > > > +#define ZYNQMP_AES_KUP_KEY 0 > > > +#define ZYNQMP_AES_DEVICE_KEY 1 > > > +#define ZYNQMP_AES_PUF_KEY 2 > > > + > > > +#define ZYNQMP_AES_GCM_TAG_MISMATCH_ERR 0x01 > > > +#define ZYNQMP_AES_SIZE_ERR 0x06 > > > +#define ZYNQMP_AES_WRONG_KEY_SRC_ERR 0x13 > > > +#define ZYNQMP_AES_PUF_NOT_PROGRAMMED 0xE300 > > > + > > > +#define ZYNQMP_AES_BLOCKSIZE 0x04 > > > + > > > +static const struct zynqmp_eemi_ops *eemi_ops; struct zynqmp_aes_dev > > > +*aes_dd; > > > > I still think that using a global variable for storing device driver data is bad. > > I think storing the list of dd's would solve up the issue with global variable, but there is only one AES instance here. > Please suggest > Look what I do for amlogic driver https://patchwork.kernel.org/patch/11059633/. I store the device driver in the instatiation of a crypto template. [...] > > > +static int zynqmp_setkey_blk(struct crypto_tfm *tfm, const u8 *key, > > > + unsigned int len) > > > +{ > > > + struct zynqmp_aes_op *op = crypto_tfm_ctx(tfm); > > > + > > > + if (((len != 1) && (len != ZYNQMP_AES_KEY_SIZE)) || (!key)) > > > > typo, two space > > Will fix in the next version > > > > > > + return -EINVAL; > > > + > > > + if (len == 1) { > > > + op->keytype = *key; > > > + > > > + if ((op->keytype < ZYNQMP_AES_KUP_KEY) || > > > + (op->keytype > ZYNQMP_AES_PUF_KEY)) > > > + return -EINVAL; > > > + > > > + } else if (len == ZYNQMP_AES_KEY_SIZE) { > > > + op->keytype = ZYNQMP_AES_KUP_KEY; > > > + op->keylen = len; > > > + memcpy(op->key, key, len); > > > + } > > > + > > > + return 0; > > > +} > > > > It seems your driver does not support AES keysize of 128/196, you need to > > fallback in that case. > > [Kalyani] In case of 128/196 keysize, returning the error would suffice ? > Or still algorithm need to work ? > If error is enough, it is taken care by this condition > if (((len != 1) && (len != ZYNQMP_AES_KEY_SIZE)) || (!key)) I think this problem just simply show us that your driver is not tested against crypto selftest. I have tried to refuse 128/196 in my driver and I get: alg: skcipher: cbc-aes-sun8i-ce setkey failed on test vector 0; expected_error=0, actual_error=-22, flags=0x1 So if your hardware lack 128/196 keys support, you must fallback to a software version. Anyway please test your driver with crypto selftest enabled (and also CONFIG_CRYPTO_MANAGER_EXTRA_TESTS=y) Regards ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2019-09-06 7:04 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-09-01 13:54 [PATCH V2 0/4] Add Xilinx's ZynqMP AES driver support Kalyani Akula 2019-09-01 13:54 ` [PATCH V2 1/4] dt-bindings: crypto: Add bindings for ZynqMP AES driver Kalyani Akula 2019-09-01 13:54 ` [PATCH V2 2/4] ARM64: zynqmp: Add Xilinix AES node Kalyani Akula 2019-09-01 13:54 ` [PATCH V2 3/4] firmware: xilinx: Add ZynqMP aes API for AES functionality Kalyani Akula 2019-09-01 13:54 ` [PATCH V2 4/4] crypto: Add Xilinx AES driver Kalyani Akula 2019-09-02 6:58 ` Corentin Labbe 2019-09-04 17:40 ` Kalyani Akula 2019-09-06 7:04 ` Corentin Labbe
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).