Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1672629 > unrolled thread

[PATCH v2 0/7] staging: ccree: bug fixes and TODO items for 4.13

Back to article view | Back to linux.kernel

Contents

  [PATCH v2 0/7] staging: ccree: bug fixes and TODO items for 4.13 Gilad Ben-Yossef <gilad@benyossef.com> - 2017-06-22 15:40 +0200
    [PATCH v2 7/7] staging: ccree: use signal safe completion wait Gilad Ben-Yossef <gilad@benyossef.com> - 2017-06-22 15:40 +0200
    [PATCH v2 4/7] staging: ccree: remove unused function Gilad Ben-Yossef <gilad@benyossef.com> - 2017-06-22 15:40 +0200
    [PATCH v2 6/7] staging: ccree: add DT bus coherency detection Gilad Ben-Yossef <gilad@benyossef.com> - 2017-06-22 15:40 +0200
    [PATCH v2 5/7] staging: ccree: add clock management support Gilad Ben-Yossef <gilad@benyossef.com> - 2017-06-22 15:40 +0200
      Re: [PATCH v2 5/7] staging: ccree: add clock management support Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-06-23 22:30 +0200
    [PATCH v2 2/7] staging: ccree: register setkey for none hash macs Gilad Ben-Yossef <gilad@benyossef.com> - 2017-06-22 15:40 +0200
      Re: [PATCH v2 2/7] staging: ccree: register setkey for none hash macs Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-06-23 22:30 +0200
        Re: [PATCH v2 2/7] staging: ccree: register setkey for none hash macs Gilad Ben-Yossef <gilad@benyossef.com> - 2017-06-25 08:30 +0200
    [PATCH v2 1/7] staging: ccree: fix hash import/export Gilad Ben-Yossef <gilad@benyossef.com> - 2017-06-22 15:40 +0200
      Re: [PATCH v2 1/7] staging: ccree: fix hash import/export Dan Carpenter <dan.carpenter@oracle.com> - 2017-06-22 16:00 +0200
        Re: [PATCH v2 1/7] staging: ccree: fix hash import/export Gilad Ben-Yossef <gilad@benyossef.com> - 2017-06-25 08:30 +0200
    Re: [PATCH v2 3/7] staging: ccree: add support for older HW revisions Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-06-23 22:30 +0200

#1672629 — [PATCH v2 0/7] staging: ccree: bug fixes and TODO items for 4.13

An assortment of bug fixes and staging TODO items.
Highlights includes the driver passing crypto testmgr boot tests
and support of multiple HW revs. without build time changes.

CC: Dan Carpenter <dan.carpenter@oracle.com>

Changes from v1:
- Rebased on top of latest patches sent to mailing
  list but not yet marged for same driver.
- Address Dan Carpenter style review comments
- Fix clk de-init being called on clk init failure
  error path pointed out by Dan Carpenter.

Gilad Ben-Yossef (7):
  staging: ccree: fix hash import/export
  staging: ccree: register setkey for none hash macs
  staging: ccree: add support for older HW revisions
  staging: ccree: remove unused function
  staging: ccree: add clock management support
  staging: ccree: add DT bus coherency detection
  staging: ccree: use signal safe completion wait

 drivers/staging/ccree/Kconfig            |   7 +-
 drivers/staging/ccree/Makefile           |   2 +-
 drivers/staging/ccree/cc_crypto_ctx.h    |  16 --
 drivers/staging/ccree/cc_hw_queue_defs.h |  14 +-
 drivers/staging/ccree/cc_regs.h          |   7 +-
 drivers/staging/ccree/dx_crys_kernel.h   |   1 +
 drivers/staging/ccree/dx_host.h          |   3 +
 drivers/staging/ccree/dx_reg_common.h    |   2 -
 drivers/staging/ccree/ssi_aead.c         |  36 ++-
 drivers/staging/ccree/ssi_buffer_mgr.c   |  37 +--
 drivers/staging/ccree/ssi_cipher.c       |  27 +-
 drivers/staging/ccree/ssi_config.h       |  20 --
 drivers/staging/ccree/ssi_driver.c       | 167 +++++++++---
 drivers/staging/ccree/ssi_driver.h       |  32 ++-
 drivers/staging/ccree/ssi_fips_ll.c      |  59 +++--
 drivers/staging/ccree/ssi_hash.c         | 441 ++++++++++++++++++-------------
 drivers/staging/ccree/ssi_hash.h         |  12 +-
 drivers/staging/ccree/ssi_pm.c           |  13 +-
 drivers/staging/ccree/ssi_pm_ext.c       |  60 -----
 drivers/staging/ccree/ssi_pm_ext.h       |  33 ---
 drivers/staging/ccree/ssi_request_mgr.c  |  22 +-
 drivers/staging/ccree/ssi_sram_mgr.c     |  15 +-
 22 files changed, 561 insertions(+), 465 deletions(-)
 delete mode 100644 drivers/staging/ccree/ssi_pm_ext.c
 delete mode 100644 drivers/staging/ccree/ssi_pm_ext.h

-- 
2.1.4

[toc] | [next] | [standalone]

#1672630 — [PATCH v2 7/7] staging: ccree: use signal safe completion wait

We were waiting for a completion notification of HW DMA
operation using an interruptible wait which can result
in data corruption if a signal interrupted us while
DMA was not yet completed.

Fix this by moving to uninterrupted wait.

Fixes: abefd6741d ("staging: ccree: introduce CryptoCell HW driver").

Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
---
 drivers/staging/ccree/ssi_request_mgr.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/ccree/ssi_request_mgr.c b/drivers/staging/ccree/ssi_request_mgr.c
index cffc8de..87f5ab6 100644
--- a/drivers/staging/ccree/ssi_request_mgr.c
+++ b/drivers/staging/ccree/ssi_request_mgr.c
@@ -382,7 +382,8 @@ int send_request(
 		/* Wait upon sequence completion.
 		 *  Return "0" -Operation done successfully.
 		 */
-		return wait_for_completion_interruptible(&ssi_req->seq_compl);
+		wait_for_completion(&ssi_req->seq_compl);
+		return 0;
 	} else {
 		/* Operation still in process */
 		return -EINPROGRESS;
-- 
2.1.4

[toc] | [prev] | [next] | [standalone]

#1672632 — [PATCH v2 4/7] staging: ccree: remove unused function

The function set_ack_last was not used anywhere. Remove it.

Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
---
 drivers/staging/ccree/cc_hw_queue_defs.h | 12 ------------
 1 file changed, 12 deletions(-)

diff --git a/drivers/staging/ccree/cc_hw_queue_defs.h b/drivers/staging/ccree/cc_hw_queue_defs.h
index c730c3c..ac0eb97 100644
--- a/drivers/staging/ccree/cc_hw_queue_defs.h
+++ b/drivers/staging/ccree/cc_hw_queue_defs.h
@@ -226,18 +226,6 @@ static inline void set_queue_last_ind_bit(struct cc_hw_desc *pdesc)
 }
 
 /*
- * Signs the end of HW descriptors flow by asking for completion ack,
- * and release the HW engines
- *
- * @pdesc: pointer HW descriptor struct
- */
-static inline void set_ack_last(struct cc_hw_desc *pdesc)
-{
-	pdesc->word[3] |= FIELD_PREP(WORD3_QUEUE_LAST_IND, 1);
-	pdesc->word[4] |= FIELD_PREP(WORD4_ACK_NEEDED, 1);
-}
-
-/*
  * Set the DIN field of a HW descriptors
  *
  * @pdesc: pointer HW descriptor struct
-- 
2.1.4

[toc] | [prev] | [next] | [standalone]

#1672633 — [PATCH v2 6/7] staging: ccree: add DT bus coherency detection

The ccree driver has build time configurable support
to work on top of coherent (e.g. ACP) vs. none coherent bus
connections. Turn it to run-time configurable option
based on device tree.

Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
---
 drivers/staging/ccree/ssi_buffer_mgr.c | 37 ++++++++++++++++++----------------
 drivers/staging/ccree/ssi_config.h     | 20 ------------------
 drivers/staging/ccree/ssi_driver.c     | 12 ++++++++---
 drivers/staging/ccree/ssi_driver.h     |  3 +++
 4 files changed, 32 insertions(+), 40 deletions(-)

diff --git a/drivers/staging/ccree/ssi_buffer_mgr.c b/drivers/staging/ccree/ssi_buffer_mgr.c
index 88ebda8..4373d1d 100644
--- a/drivers/staging/ccree/ssi_buffer_mgr.c
+++ b/drivers/staging/ccree/ssi_buffer_mgr.c
@@ -627,6 +627,7 @@ void ssi_buffer_mgr_unmap_aead_request(
 	struct aead_req_ctx *areq_ctx = aead_request_ctx(req);
 	unsigned int hw_iv_size = areq_ctx->hw_iv_size;
 	struct crypto_aead *tfm = crypto_aead_reqtfm(req);
+	struct ssi_drvdata *drvdata = dev_get_drvdata(dev);
 	u32 dummy;
 	bool chained;
 	u32 size_to_unmap = 0;
@@ -700,8 +701,8 @@ void ssi_buffer_mgr_unmap_aead_request(
 		dma_unmap_sg(dev, req->dst, ssi_buffer_mgr_get_sgl_nents(req->dst,size_to_unmap,&dummy,&chained),
 			DMA_BIDIRECTIONAL);
 	}
-#if DX_HAS_ACP
-	if ((areq_ctx->gen_ctx.op_type == DRV_CRYPTO_DIRECTION_DECRYPT) &&
+	if (drvdata->coherent &&
+	    (areq_ctx->gen_ctx.op_type == DRV_CRYPTO_DIRECTION_DECRYPT) &&
 	    likely(req->src == req->dst))
 	{
 		u32 size_to_skip = req->assoclen;
@@ -716,7 +717,6 @@ void ssi_buffer_mgr_unmap_aead_request(
 			size_to_skip+ req->cryptlen - areq_ctx->req_authsize,
 			size_to_skip+ req->cryptlen, SSI_SG_FROM_BUF);
 	}
-#endif
 }
 
 static inline int ssi_buffer_mgr_get_aead_icv_nents(
@@ -981,20 +981,24 @@ static inline int ssi_buffer_mgr_prepare_aead_data_mlli(
 			 * MAC verification upon request completion
 			 */
 			if (direct == DRV_CRYPTO_DIRECTION_DECRYPT) {
-#if !DX_HAS_ACP
-				/* In ACP platform we already copying ICV
-				 * for any INPLACE-DECRYPT operation, hence
+				if (!drvdata->coherent) {
+				/* In coherent platforms (e.g. ACP)
+				 * already copying ICV for any
+				 * INPLACE-DECRYPT operation, hence
 				 * we must neglect this code.
 				 */
-				u32 size_to_skip = req->assoclen;
-				if (areq_ctx->is_gcm4543) {
-					size_to_skip += crypto_aead_ivsize(tfm);
+					u32 skip = req->assoclen;
+
+					if (areq_ctx->is_gcm4543)
+						skip += crypto_aead_ivsize(tfm);
+
+					ssi_buffer_mgr_copy_scatterlist_portion(
+						areq_ctx->backup_mac, req->src,
+						(skip + req->cryptlen -
+						 areq_ctx->req_authsize),
+						skip + req->cryptlen,
+						SSI_SG_TO_BUF);
 				}
-				ssi_buffer_mgr_copy_scatterlist_portion(
-					areq_ctx->backup_mac, req->src,
-					size_to_skip+ req->cryptlen - areq_ctx->req_authsize,
-					size_to_skip+ req->cryptlen, SSI_SG_TO_BUF);
-#endif
 				areq_ctx->icv_virt_addr = areq_ctx->backup_mac;
 			} else {
 				areq_ctx->icv_virt_addr = areq_ctx->mac_buf;
@@ -1281,8 +1285,8 @@ int ssi_buffer_mgr_map_aead_request(
 	mlli_params->curr_pool = NULL;
 	sg_data.num_of_buffers = 0;
 
-#if DX_HAS_ACP
-	if ((areq_ctx->gen_ctx.op_type == DRV_CRYPTO_DIRECTION_DECRYPT) &&
+	if (drvdata->coherent &&
+	    (areq_ctx->gen_ctx.op_type == DRV_CRYPTO_DIRECTION_DECRYPT) &&
 	    likely(req->src == req->dst))
 	{
 		u32 size_to_skip = req->assoclen;
@@ -1297,7 +1301,6 @@ int ssi_buffer_mgr_map_aead_request(
 			size_to_skip+ req->cryptlen - areq_ctx->req_authsize,
 			size_to_skip+ req->cryptlen, SSI_SG_TO_BUF);
 	}
-#endif
 
 	/* cacluate the size for cipher remove ICV in decrypt*/
 	areq_ctx->cryptlen = (areq_ctx->gen_ctx.op_type ==
diff --git a/drivers/staging/ccree/ssi_config.h b/drivers/staging/ccree/ssi_config.h
index 2484a06..ff7597c 100644
--- a/drivers/staging/ccree/ssi_config.h
+++ b/drivers/staging/ccree/ssi_config.h
@@ -23,7 +23,6 @@
 
 #include <linux/version.h>
 
-//#define DISABLE_COHERENT_DMA_OPS
 //#define FLUSH_CACHE_ALL
 //#define COMPLETION_DELAY
 //#define DX_DUMP_DESCS
@@ -33,24 +32,5 @@
 //#define DX_IRQ_DELAY 100000
 #define DMA_BIT_MASK_LEN	48	/* was 32 bit, but for juno's sake it was enlarged to 48 bit */
 
-#if defined (CONFIG_ARM64)	// TODO currently only this mode was test on Juno (which is ARM64), need to enable coherent also.
-#define DISABLE_COHERENT_DMA_OPS
-#endif
-
-/* Define the CryptoCell DMA cache coherency signals configuration */
-#if defined (DISABLE_COHERENT_DMA_OPS)
-	/* Software Controlled Cache Coherency (SCCC) */
-	#define SSI_CACHE_PARAMS (0x000)
-	/* CC attached to NONE-ACP such as HPP/ACE/AMBA4.
-	 * The customer is responsible to enable/disable this feature
-	 * according to his platform type.
-	 */
-	#define DX_HAS_ACP 0
-#else
-	#define SSI_CACHE_PARAMS (0xEEE)
-	/* CC attached to ACP */
-	#define DX_HAS_ACP 1
-#endif
-
 #endif /*__DX_CONFIG_H__*/
 
diff --git a/drivers/staging/ccree/ssi_driver.c b/drivers/staging/ccree/ssi_driver.c
index 2e251a4..a606ab4 100644
--- a/drivers/staging/ccree/ssi_driver.c
+++ b/drivers/staging/ccree/ssi_driver.c
@@ -58,6 +58,7 @@
 #include <linux/random.h>
 #include <linux/of.h>
 #include <linux/clk.h>
+#include <linux/of_address.h>
 
 #include "ssi_config.h"
 #include "ssi_driver.h"
@@ -199,7 +200,7 @@ static irqreturn_t cc_isr(int irq, void *dev_id)
 
 int init_cc_regs(struct ssi_drvdata *drvdata, bool is_probe)
 {
-	unsigned int val;
+	unsigned int val, cache_params;
 	void __iomem *cc_base = drvdata->cc_base;
 
 	/* Unmask all AXI interrupt sources AXI_CFG1 register */
@@ -232,14 +233,18 @@ int init_cc_regs(struct ssi_drvdata *drvdata, bool is_probe)
 	}
 #endif
 
+	cache_params = (drvdata->coherent ? CC_COHERENT_CACHE_PARAMS : 0x0);
+
 	val = CC_HAL_READ_REGISTER(CC_REG_OFFSET(CRY_KERNEL, AXIM_CACHE_PARAMS));
 	if (is_probe == true) {
 		SSI_LOG_INFO("Cache params previous: 0x%08X\n", val);
 	}
-	CC_HAL_WRITE_REGISTER(CC_REG_OFFSET(CRY_KERNEL, AXIM_CACHE_PARAMS), SSI_CACHE_PARAMS);
+	CC_HAL_WRITE_REGISTER(CC_REG_OFFSET(CRY_KERNEL, AXIM_CACHE_PARAMS),
+			      cache_params);
 	val = CC_HAL_READ_REGISTER(CC_REG_OFFSET(CRY_KERNEL, AXIM_CACHE_PARAMS));
 	if (is_probe == true) {
-		SSI_LOG_INFO("Cache params current: 0x%08X  (expected: 0x%08X)\n", val, SSI_CACHE_PARAMS);
+		SSI_LOG_INFO("Cache params current: 0x%08X (expect: 0x%08X)\n",
+			     val, cache_params);
 	}
 
 	return 0;
@@ -271,6 +276,7 @@ static int init_cc_resources(struct platform_device *plat_dev)
 	new_drvdata->hw_rev_name = hw_rev->name;
 	new_drvdata->hw_rev = hw_rev->rev;
 	new_drvdata->clk = of_clk_get(np, 0);
+	new_drvdata->coherent = of_dma_is_coherent(np);
 
 	if (hw_rev->rev >= CC_HW_REV_712) {
 		new_drvdata->hash_len_sz = HASH_LEN_SIZE_712;
diff --git a/drivers/staging/ccree/ssi_driver.h b/drivers/staging/ccree/ssi_driver.h
index a9333892..bdbf1c7 100644
--- a/drivers/staging/ccree/ssi_driver.h
+++ b/drivers/staging/ccree/ssi_driver.h
@@ -59,6 +59,8 @@ enum cc_hw_rev {
 	CC_HW_REV_712 = 712
 };
 
+#define CC_COHERENT_CACHE_PARAMS 0xEEE
+
 #define SSI_CC_HAS_AES_CCM 1
 #define SSI_CC_HAS_AES_GCM 1
 #define SSI_CC_HAS_AES_XTS 1
@@ -158,6 +160,7 @@ struct ssi_drvdata {
 	u32 hash_len_sz;
 	u32 axim_mon_offset;
 	struct clk *clk;
+	bool coherent;
 };
 
 struct ssi_crypto_alg {
-- 
2.1.4

[toc] | [prev] | [next] | [standalone]

#1672634 — [PATCH v2 5/7] staging: ccree: add clock management support

Some SoC which implement CryptoCell have a dedicated clock
tied to it, some do not. Implement clock support if exists
based on device tree data and tie power management to it.

Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
---
 drivers/staging/ccree/Makefile     |  2 +-
 drivers/staging/ccree/ssi_driver.c | 40 +++++++++++++++++++++----
 drivers/staging/ccree/ssi_driver.h |  4 +++
 drivers/staging/ccree/ssi_pm.c     | 13 +++++----
 drivers/staging/ccree/ssi_pm_ext.c | 60 --------------------------------------
 drivers/staging/ccree/ssi_pm_ext.h | 33 ---------------------
 6 files changed, 47 insertions(+), 105 deletions(-)
 delete mode 100644 drivers/staging/ccree/ssi_pm_ext.c
 delete mode 100644 drivers/staging/ccree/ssi_pm_ext.h

diff --git a/drivers/staging/ccree/Makefile b/drivers/staging/ccree/Makefile
index 44f3e3e..318c2b3 100644
--- a/drivers/staging/ccree/Makefile
+++ b/drivers/staging/ccree/Makefile
@@ -1,3 +1,3 @@
 obj-$(CONFIG_CRYPTO_DEV_CCREE) := ccree.o
-ccree-y := ssi_driver.o ssi_sysfs.o ssi_buffer_mgr.o ssi_request_mgr.o ssi_cipher.o ssi_hash.o ssi_aead.o ssi_ivgen.o ssi_sram_mgr.o ssi_pm.o ssi_pm_ext.o
+ccree-y := ssi_driver.o ssi_sysfs.o ssi_buffer_mgr.o ssi_request_mgr.o ssi_cipher.o ssi_hash.o ssi_aead.o ssi_ivgen.o ssi_sram_mgr.o ssi_pm.o
 ccree-$(CCREE_FIPS_SUPPORT) += ssi_fips.o ssi_fips_ll.o ssi_fips_ext.o ssi_fips_local.o
diff --git a/drivers/staging/ccree/ssi_driver.c b/drivers/staging/ccree/ssi_driver.c
index 131bfc4..2e251a4 100644
--- a/drivers/staging/ccree/ssi_driver.c
+++ b/drivers/staging/ccree/ssi_driver.c
@@ -57,6 +57,7 @@
 #include <linux/sched.h>
 #include <linux/random.h>
 #include <linux/of.h>
+#include <linux/clk.h>
 
 #include "ssi_config.h"
 #include "ssi_driver.h"
@@ -269,6 +270,7 @@ static int init_cc_resources(struct platform_device *plat_dev)
 	hw_rev = (struct cc_hw_data *)dev_id->data;
 	new_drvdata->hw_rev_name = hw_rev->name;
 	new_drvdata->hw_rev = hw_rev->rev;
+	new_drvdata->clk = of_clk_get(np, 0);
 
 	if (hw_rev->rev >= CC_HW_REV_712) {
 		new_drvdata->hash_len_sz = HASH_LEN_SIZE_712;
@@ -338,6 +340,10 @@ static int init_cc_resources(struct platform_device *plat_dev)
 
 	new_drvdata->plat_dev = plat_dev;
 
+	rc = cc_clk_on(new_drvdata);
+	if (rc)
+		goto init_cc_res_err;
+
 	if(new_drvdata->plat_dev->dev.dma_mask == NULL)
 	{
 		new_drvdata->plat_dev->dev.dma_mask = & new_drvdata->plat_dev->dev.coherent_dma_mask;
@@ -504,14 +510,11 @@ static void cleanup_cc_resources(struct platform_device *plat_dev)
 	ssi_sysfs_fini();
 #endif
 
-	/* Mask all interrupts */
-	WRITE_REGISTER(drvdata->cc_base + CC_REG_OFFSET(HOST_RGF, HOST_IMR),
-		0xFFFFFFFF);
+	fini_cc_regs(drvdata);
+	cc_clk_off(drvdata);
 	free_irq(drvdata->res_irq->start, drvdata);
 	drvdata->res_irq = NULL;
 
-	fini_cc_regs(drvdata);
-
 	if (drvdata->cc_base != NULL) {
 		iounmap(drvdata->cc_base);
 		release_mem_region(drvdata->res_mem->start,
@@ -524,6 +527,33 @@ static void cleanup_cc_resources(struct platform_device *plat_dev)
 	dev_set_drvdata(&plat_dev->dev, NULL);
 }
 
+int cc_clk_on(struct ssi_drvdata *drvdata)
+{
+	struct clk *clk = drvdata->clk;
+	int rc;
+
+	if (IS_ERR(clk))
+		/* Not all devices have a clock associated with CCREE  */
+		return 0;
+
+	rc = clk_prepare_enable(clk);
+	if (rc)
+		return rc;
+
+	return 0;
+}
+
+void cc_clk_off(struct ssi_drvdata *drvdata)
+{
+	struct clk *clk = drvdata->clk;
+
+	if (IS_ERR(clk))
+		/* Not all devices have a clock associated with CCREE */
+		return;
+
+	clk_disable_unprepare(clk);
+}
+
 static int ccree_probe(struct platform_device *plat_dev)
 {
 	int rc;
diff --git a/drivers/staging/ccree/ssi_driver.h b/drivers/staging/ccree/ssi_driver.h
index 3c27fd8..a9333892 100644
--- a/drivers/staging/ccree/ssi_driver.h
+++ b/drivers/staging/ccree/ssi_driver.h
@@ -36,6 +36,7 @@
 #include <crypto/authenc.h>
 #include <crypto/hash.h>
 #include <linux/version.h>
+#include <linux/clk.h>
 
 /* Registers definitions from shared/hw/ree_include */
 #include "dx_reg_base_host.h"
@@ -156,6 +157,7 @@ struct ssi_drvdata {
 	enum cc_hw_rev hw_rev;
 	u32 hash_len_sz;
 	u32 axim_mon_offset;
+	struct clk *clk;
 };
 
 struct ssi_crypto_alg {
@@ -201,6 +203,8 @@ void dump_byte_array(const char *name, const u8 *the_array, unsigned long size);
 
 int init_cc_regs(struct ssi_drvdata *drvdata, bool is_probe);
 void fini_cc_regs(struct ssi_drvdata *drvdata);
+int cc_clk_on(struct ssi_drvdata *drvdata);
+void cc_clk_off(struct ssi_drvdata *drvdata);
 
 static inline void set_queue_last_ind(struct ssi_drvdata *drvdata,
 				      struct cc_hw_desc *pdesc)
diff --git a/drivers/staging/ccree/ssi_pm.c b/drivers/staging/ccree/ssi_pm.c
index 5bfbdd0..67ae1dc 100644
--- a/drivers/staging/ccree/ssi_pm.c
+++ b/drivers/staging/ccree/ssi_pm.c
@@ -29,7 +29,6 @@
 #include "ssi_ivgen.h"
 #include "ssi_hash.h"
 #include "ssi_pm.h"
-#include "ssi_pm_ext.h"
 
 
 #if defined (CONFIG_PM_RUNTIME) || defined (CONFIG_PM_SLEEP)
@@ -52,9 +51,7 @@ int ssi_power_mgr_runtime_suspend(struct device *dev)
 		return rc;
 	}
 	fini_cc_regs(drvdata);
-
-	/* Specific HW suspend code */
-	ssi_pm_ext_hw_suspend(dev);
+	cc_clk_off(drvdata);
 	return 0;
 }
 
@@ -66,8 +63,12 @@ int ssi_power_mgr_runtime_resume(struct device *dev)
 
 	SSI_LOG_DEBUG("ssi_power_mgr_runtime_resume , unset HOST_POWER_DOWN_EN\n");
 	WRITE_REGISTER(drvdata->cc_base + CC_REG_OFFSET(HOST_RGF, HOST_POWER_DOWN_EN), POWER_DOWN_DISABLE);
-	/* Specific HW resume code */
-	ssi_pm_ext_hw_resume(dev);
+
+	rc = cc_clk_on(drvdata);
+	if (rc) {
+		SSI_LOG_ERR("failed getting clock back on. We're toast.\n");
+		return rc;
+	}
 
 	rc = init_cc_regs(drvdata, false);
 	if (rc !=0) {
diff --git a/drivers/staging/ccree/ssi_pm_ext.c b/drivers/staging/ccree/ssi_pm_ext.c
deleted file mode 100644
index 453151c..0000000
--- a/drivers/staging/ccree/ssi_pm_ext.c
+++ /dev/null
@@ -1,60 +0,0 @@
-/*
- * Copyright (C) 2012-2017 ARM Limited or its affiliates.
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 as
- * published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, see <http://www.gnu.org/licenses/>.
- */
-
-
-#include "ssi_config.h"
-#include <linux/kernel.h>
-#include <linux/platform_device.h>
-#include <linux/interrupt.h>
-#include <crypto/ctr.h>
-#include <linux/pm_runtime.h>
-#include "ssi_driver.h"
-#include "ssi_sram_mgr.h"
-#include "ssi_pm_ext.h"
-
-/*
- * This function should suspend the HW (if possiable), It should be implemented by
- * the driver user.
- * The reference code clears the internal SRAM to imitate lose of state.
- */
-void ssi_pm_ext_hw_suspend(struct device *dev)
-{
-	struct ssi_drvdata *drvdata =
-		(struct ssi_drvdata *)dev_get_drvdata(dev);
-	unsigned int val;
-	void __iomem *cc_base = drvdata->cc_base;
-	unsigned int  sram_addr = 0;
-
-	CC_HAL_WRITE_REGISTER(CC_REG_OFFSET(HOST_RGF, SRAM_ADDR), sram_addr);
-
-	for (;sram_addr < SSI_CC_SRAM_SIZE ; sram_addr+=4) {
-		CC_HAL_WRITE_REGISTER(CC_REG_OFFSET(HOST_RGF, SRAM_DATA), 0x0);
-
-		do {
-			val = CC_HAL_READ_REGISTER(CC_REG_OFFSET(HOST_RGF, SRAM_DATA_READY));
-		} while (!(val &0x1));
-	}
-}
-
-/*
- * This function should resume the HW (if possiable).It should be implemented by
- * the driver user.
- */
-void ssi_pm_ext_hw_resume(struct device *dev)
-{
-	return;
-}
-
diff --git a/drivers/staging/ccree/ssi_pm_ext.h b/drivers/staging/ccree/ssi_pm_ext.h
deleted file mode 100644
index dbe658b..0000000
--- a/drivers/staging/ccree/ssi_pm_ext.h
+++ /dev/null
@@ -1,33 +0,0 @@
-/*
- * Copyright (C) 2012-2017 ARM Limited or its affiliates.
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 as
- * published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, see <http://www.gnu.org/licenses/>.
- */
-
-/* \file ssi_pm_ext.h
- */
-
-#ifndef __PM_EXT_H__
-#define __PM_EXT_H__
-
-
-#include "ssi_config.h"
-#include "ssi_driver.h"
-
-void ssi_pm_ext_hw_suspend(struct device *dev);
-
-void ssi_pm_ext_hw_resume(struct device *dev);
-
-
-#endif /*__POWER_MGR_H__*/
-
-- 
2.1.4

[toc] | [prev] | [next] | [standalone]

#1673847 — Re: [PATCH v2 5/7] staging: ccree: add clock management support

On Thu, Jun 22, 2017 at 04:36:59PM +0300, Gilad Ben-Yossef wrote:
> Some SoC which implement CryptoCell have a dedicated clock
> tied to it, some do not. Implement clock support if exists
> based on device tree data and tie power management to it.
> 
> Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
> ---
>  drivers/staging/ccree/Makefile     |  2 +-
>  drivers/staging/ccree/ssi_driver.c | 40 +++++++++++++++++++++----
>  drivers/staging/ccree/ssi_driver.h |  4 +++
>  drivers/staging/ccree/ssi_pm.c     | 13 +++++----
>  drivers/staging/ccree/ssi_pm_ext.c | 60 --------------------------------------
>  drivers/staging/ccree/ssi_pm_ext.h | 33 ---------------------
>  6 files changed, 47 insertions(+), 105 deletions(-)
>  delete mode 100644 drivers/staging/ccree/ssi_pm_ext.c
>  delete mode 100644 drivers/staging/ccree/ssi_pm_ext.h

Ok, adding new features that make the code smaller is ok, this is nice :)

[toc] | [prev] | [next] | [standalone]

#1672637 — [PATCH v2 2/7] staging: ccree: register setkey for none hash macs

Fix a bug where the transformation init code did
not register a setkey method for none hash based MACs.

Fixes commit 50cfbbb7e627 ("staging: ccree: add ahash support").

Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
---
 drivers/staging/ccree/ssi_hash.c | 83 ++++++++++++++++++++--------------------
 1 file changed, 42 insertions(+), 41 deletions(-)

diff --git a/drivers/staging/ccree/ssi_hash.c b/drivers/staging/ccree/ssi_hash.c
index c39e3be..f52e1af 100644
--- a/drivers/staging/ccree/ssi_hash.c
+++ b/drivers/staging/ccree/ssi_hash.c
@@ -1874,8 +1874,8 @@ static int ssi_ahash_setkey(struct crypto_ahash *ahash,
 struct ssi_hash_template {
 	char name[CRYPTO_MAX_ALG_NAME];
 	char driver_name[CRYPTO_MAX_ALG_NAME];
-	char hmac_name[CRYPTO_MAX_ALG_NAME];
-	char hmac_driver_name[CRYPTO_MAX_ALG_NAME];
+	char mac_name[CRYPTO_MAX_ALG_NAME];
+	char mac_driver_name[CRYPTO_MAX_ALG_NAME];
 	unsigned int blocksize;
 	bool synchronize;
 	struct ahash_alg template_ahash;
@@ -1894,8 +1894,8 @@ static struct ssi_hash_template driver_hash[] = {
 	{
 		.name = "sha1",
 		.driver_name = "sha1-dx",
-		.hmac_name = "hmac(sha1)",
-		.hmac_driver_name = "hmac-sha1-dx",
+		.mac_name = "hmac(sha1)",
+		.mac_driver_name = "hmac-sha1-dx",
 		.blocksize = SHA1_BLOCK_SIZE,
 		.synchronize = false,
 		.template_ahash = {
@@ -1919,8 +1919,8 @@ static struct ssi_hash_template driver_hash[] = {
 	{
 		.name = "sha256",
 		.driver_name = "sha256-dx",
-		.hmac_name = "hmac(sha256)",
-		.hmac_driver_name = "hmac-sha256-dx",
+		.mac_name = "hmac(sha256)",
+		.mac_driver_name = "hmac-sha256-dx",
 		.blocksize = SHA256_BLOCK_SIZE,
 		.template_ahash = {
 			.init = ssi_ahash_init,
@@ -1943,8 +1943,8 @@ static struct ssi_hash_template driver_hash[] = {
 	{
 		.name = "sha224",
 		.driver_name = "sha224-dx",
-		.hmac_name = "hmac(sha224)",
-		.hmac_driver_name = "hmac-sha224-dx",
+		.mac_name = "hmac(sha224)",
+		.mac_driver_name = "hmac-sha224-dx",
 		.blocksize = SHA224_BLOCK_SIZE,
 		.template_ahash = {
 			.init = ssi_ahash_init,
@@ -1968,8 +1968,8 @@ static struct ssi_hash_template driver_hash[] = {
 	{
 		.name = "sha384",
 		.driver_name = "sha384-dx",
-		.hmac_name = "hmac(sha384)",
-		.hmac_driver_name = "hmac-sha384-dx",
+		.mac_name = "hmac(sha384)",
+		.mac_driver_name = "hmac-sha384-dx",
 		.blocksize = SHA384_BLOCK_SIZE,
 		.template_ahash = {
 			.init = ssi_ahash_init,
@@ -1992,8 +1992,8 @@ static struct ssi_hash_template driver_hash[] = {
 	{
 		.name = "sha512",
 		.driver_name = "sha512-dx",
-		.hmac_name = "hmac(sha512)",
-		.hmac_driver_name = "hmac-sha512-dx",
+		.mac_name = "hmac(sha512)",
+		.mac_driver_name = "hmac-sha512-dx",
 		.blocksize = SHA512_BLOCK_SIZE,
 		.template_ahash = {
 			.init = ssi_ahash_init,
@@ -2017,8 +2017,8 @@ static struct ssi_hash_template driver_hash[] = {
 	{
 		.name = "md5",
 		.driver_name = "md5-dx",
-		.hmac_name = "hmac(md5)",
-		.hmac_driver_name = "hmac-md5-dx",
+		.mac_name = "hmac(md5)",
+		.mac_driver_name = "hmac-md5-dx",
 		.blocksize = MD5_HMAC_BLOCK_SIZE,
 		.template_ahash = {
 			.init = ssi_ahash_init,
@@ -2039,8 +2039,8 @@ static struct ssi_hash_template driver_hash[] = {
 		.inter_digestsize = MD5_DIGEST_SIZE,
 	},
 	{
-		.name = "xcbc(aes)",
-		.driver_name = "xcbc-aes-dx",
+		.mac_name = "xcbc(aes)",
+		.mac_driver_name = "xcbc-aes-dx",
 		.blocksize = AES_BLOCK_SIZE,
 		.template_ahash = {
 			.init = ssi_ahash_init,
@@ -2062,8 +2062,8 @@ static struct ssi_hash_template driver_hash[] = {
 	},
 #if SSI_CC_HAS_CMAC
 	{
-		.name = "cmac(aes)",
-		.driver_name = "cmac-aes-dx",
+		.mac_name = "cmac(aes)",
+		.mac_driver_name = "cmac-aes-dx",
 		.blocksize = AES_BLOCK_SIZE,
 		.template_ahash = {
 			.init = ssi_ahash_init,
@@ -2106,9 +2106,9 @@ ssi_hash_create_alg(struct ssi_hash_template *template, bool keyed)
 
 	if (keyed) {
 		snprintf(alg->cra_name, CRYPTO_MAX_ALG_NAME, "%s",
-			 template->hmac_name);
+			 template->mac_name);
 		snprintf(alg->cra_driver_name, CRYPTO_MAX_ALG_NAME, "%s",
-			 template->hmac_driver_name);
+			 template->mac_driver_name);
 	} else {
 		halg->setkey = NULL;
 		snprintf(alg->cra_name, CRYPTO_MAX_ALG_NAME, "%s",
@@ -2297,32 +2297,33 @@ int ssi_hash_alloc(struct ssi_drvdata *drvdata)
 	/* ahash registration */
 	for (alg = 0; alg < ARRAY_SIZE(driver_hash); alg++) {
 		struct ssi_hash_alg *t_alg;
+		int hw_mode = driver_hash[alg].hw_mode;
 
 		/* register hmac version */
+		t_alg = ssi_hash_create_alg(&driver_hash[alg], true);
+		if (IS_ERR(t_alg)) {
+			rc = PTR_ERR(t_alg);
+			SSI_LOG_ERR("%s alg allocation failed\n",
+				    driver_hash[alg].driver_name);
+			goto fail;
+		}
+		t_alg->drvdata = drvdata;
 
-		if ((((struct ssi_hash_template *)&driver_hash[alg])->hw_mode != DRV_CIPHER_XCBC_MAC) &&
-		    (((struct ssi_hash_template *)&driver_hash[alg])->hw_mode != DRV_CIPHER_CMAC)) {
-			t_alg = ssi_hash_create_alg(&driver_hash[alg], true);
-			if (IS_ERR(t_alg)) {
-				rc = PTR_ERR(t_alg);
-				SSI_LOG_ERR("%s alg allocation failed\n",
-					 driver_hash[alg].driver_name);
-				goto fail;
-			}
-			t_alg->drvdata = drvdata;
-
-			rc = crypto_register_ahash(&t_alg->ahash_alg);
-			if (unlikely(rc)) {
-				SSI_LOG_ERR("%s alg registration failed\n",
-					    driver_hash[alg].driver_name);
-				kfree(t_alg);
-				goto fail;
-			} else {
-				list_add_tail(&t_alg->entry,
-					      &hash_handle->hash_list);
-			}
+		rc = crypto_register_ahash(&t_alg->ahash_alg);
+		if (unlikely(rc)) {
+			SSI_LOG_ERR("%s alg registration failed\n",
+				    driver_hash[alg].driver_name);
+			kfree(t_alg);
+			goto fail;
+		} else {
+			list_add_tail(&t_alg->entry,
+				      &hash_handle->hash_list);
 		}
 
+		if ((hw_mode == DRV_CIPHER_XCBC_MAC) ||
+		    (hw_mode == DRV_CIPHER_CMAC))
+			continue;
+
 		/* register hash version */
 		t_alg = ssi_hash_create_alg(&driver_hash[alg], false);
 		if (IS_ERR(t_alg)) {
-- 
2.1.4

[toc] | [prev] | [next] | [standalone]

#1673834 — Re: [PATCH v2 2/7] staging: ccree: register setkey for none hash macs

On Thu, Jun 22, 2017 at 04:36:56PM +0300, Gilad Ben-Yossef wrote:
> Fix a bug where the transformation init code did
> not register a setkey method for none hash based MACs.

"none hash based MACs"?  Is that the correct language, I don't
understand it, sorry, can you expand on it a bit in your v3 series?

> Fixes commit 50cfbbb7e627 ("staging: ccree: add ahash support").

This line should be written as:
Fixes: 50cfbbb7e627 ("staging: ccree: add ahash support").

thanks,

greg k-h

[toc] | [prev] | [next] | [standalone]

#1674181 — Re: [PATCH v2 2/7] staging: ccree: register setkey for none hash macs

On Fri, Jun 23, 2017 at 7:29 PM, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> On Thu, Jun 22, 2017 at 04:36:56PM +0300, Gilad Ben-Yossef wrote:
>> Fix a bug where the transformation init code did
>> not register a setkey method for none hash based MACs.
>
> "none hash based MACs"?  Is that the correct language, I don't
> understand it, sorry, can you expand on it a bit in your v3 series?

My apologies. English is not my mother tongue. This should have been
"non-hash based MACs".

>
>> Fixes commit 50cfbbb7e627 ("staging: ccree: add ahash support").
>
> This line should be written as:
> Fixes: 50cfbbb7e627 ("staging: ccree: add ahash support").
>

I'll fix both.

Thanks,
Gilad


-- 
Gilad Ben-Yossef
Chief Coffee Drinker

"If you take a class in large-scale robotics, can you end up in a
situation where the homework eats your dog?"
 -- Jean-Baptiste Queru

[toc] | [prev] | [next] | [standalone]

#1672638 — [PATCH v2 1/7] staging: ccree: fix hash import/export

Hash import and export was saving and restoring the wrong context
and therefore disabled. Fix it by restoring intermediate digest
and additional state needed.

The hash and mac transform now pass testmgr partial hash tests.

Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
---
 drivers/staging/ccree/ssi_hash.c | 140 +++++++++++++++++++++++++++------------
 drivers/staging/ccree/ssi_hash.h |   2 +
 2 files changed, 98 insertions(+), 44 deletions(-)

diff --git a/drivers/staging/ccree/ssi_hash.c b/drivers/staging/ccree/ssi_hash.c
index ed1c672..c39e3be 100644
--- a/drivers/staging/ccree/ssi_hash.c
+++ b/drivers/staging/ccree/ssi_hash.c
@@ -976,22 +976,6 @@ static int ssi_hash_init(struct ahash_req_ctx *state, struct ssi_hash_ctx *ctx)
 	return 0;
 }
 
-#ifdef EXPORT_FIXED
-static int ssi_hash_export(struct ssi_hash_ctx *ctx, void *out)
-{
-	CHECK_AND_RETURN_UPON_FIPS_ERROR();
-	memcpy(out, ctx, sizeof(struct ssi_hash_ctx));
-	return 0;
-}
-
-static int ssi_hash_import(struct ssi_hash_ctx *ctx, const void *in)
-{
-	CHECK_AND_RETURN_UPON_FIPS_ERROR();
-	memcpy(ctx, in, sizeof(struct ssi_hash_ctx));
-	return 0;
-}
-#endif
-
 static int ssi_hash_setkey(void *hash,
 			   const u8 *key,
 			   unsigned int keylen,
@@ -1782,23 +1766,104 @@ static int ssi_ahash_init(struct ahash_request *req)
 	return ssi_hash_init(state, ctx);
 }
 
-#ifdef EXPORT_FIXED
 static int ssi_ahash_export(struct ahash_request *req, void *out)
 {
 	struct crypto_ahash *ahash = crypto_ahash_reqtfm(req);
 	struct ssi_hash_ctx *ctx = crypto_ahash_ctx(ahash);
+	struct device *dev = &ctx->drvdata->plat_dev->dev;
+	struct ahash_req_ctx *state = ahash_request_ctx(req);
+	u8 *curr_buff = state->buff_index ? state->buff1 : state->buff0;
+	u32 curr_buff_cnt = state->buff_index ? state->buff1_cnt :
+				state->buff0_cnt;
+	const u32 tmp = CC_EXPORT_MAGIC;
+
+	CHECK_AND_RETURN_UPON_FIPS_ERROR();
 
-	return ssi_hash_export(ctx, out);
+	memcpy(out, &tmp, sizeof(u32));
+	out += sizeof(u32);
+
+	dma_sync_single_for_cpu(dev, state->digest_buff_dma_addr,
+				ctx->inter_digestsize, DMA_BIDIRECTIONAL);
+	memcpy(out, state->digest_buff, ctx->inter_digestsize);
+	out += ctx->inter_digestsize;
+
+	if (state->digest_bytes_len_dma_addr) {
+		dma_sync_single_for_cpu(dev, state->digest_bytes_len_dma_addr,
+					HASH_LEN_SIZE, DMA_BIDIRECTIONAL);
+		memcpy(out, state->digest_bytes_len, HASH_LEN_SIZE);
+	}
+	out += HASH_LEN_SIZE;
+
+	memcpy(out, &curr_buff_cnt, sizeof(u32));
+	out += sizeof(u32);
+
+	memcpy(out, curr_buff, curr_buff_cnt);
+
+	/* No sync for device ineeded since we did not change the data,
+	 * we only copy it
+	 */
+
+	return 0;
 }
 
 static int ssi_ahash_import(struct ahash_request *req, const void *in)
 {
 	struct crypto_ahash *ahash = crypto_ahash_reqtfm(req);
 	struct ssi_hash_ctx *ctx = crypto_ahash_ctx(ahash);
+	struct device *dev = &ctx->drvdata->plat_dev->dev;
+	struct ahash_req_ctx *state = ahash_request_ctx(req);
+	u32 tmp;
+	int rc;
+
+	CHECK_AND_RETURN_UPON_FIPS_ERROR();
 
-	return ssi_hash_import(ctx, in);
+	memcpy(&tmp, in, sizeof(u32));
+	if (tmp != CC_EXPORT_MAGIC) {
+		rc = -EINVAL;
+		goto out;
+	}
+	in += sizeof(u32);
+
+	rc = ssi_hash_init(state, ctx);
+	if (rc)
+		goto out;
+
+	dma_sync_single_for_cpu(dev, state->digest_buff_dma_addr,
+				ctx->inter_digestsize, DMA_BIDIRECTIONAL);
+	memcpy(state->digest_buff, in, ctx->inter_digestsize);
+	in += ctx->inter_digestsize;
+
+	if (state->digest_bytes_len_dma_addr) {
+		dma_sync_single_for_cpu(dev, state->digest_bytes_len_dma_addr,
+					HASH_LEN_SIZE, DMA_BIDIRECTIONAL);
+		memcpy(state->digest_bytes_len, in, HASH_LEN_SIZE);
+	}
+	in += HASH_LEN_SIZE;
+
+	dma_sync_single_for_device(dev, state->digest_buff_dma_addr,
+				   ctx->inter_digestsize, DMA_BIDIRECTIONAL);
+
+	if (state->digest_bytes_len_dma_addr)
+		dma_sync_single_for_device(dev,
+					   state->digest_bytes_len_dma_addr,
+					   HASH_LEN_SIZE, DMA_BIDIRECTIONAL);
+
+	state->buff_index = 0;
+
+	/* Sanity check the data as much as possible */
+	memcpy(&tmp, in, sizeof(u32));
+	if (tmp > SSI_MAX_HASH_BLCK_SIZE) {
+		rc = -EINVAL;
+		goto out;
+	}
+	in += sizeof(u32);
+
+	state->buff0_cnt = tmp;
+	memcpy(state->buff0, in, state->buff0_cnt);
+
+out:
+	return rc;
 }
-#endif
 
 static int ssi_ahash_setkey(struct crypto_ahash *ahash,
 			const u8 *key, unsigned int keylen)
@@ -1820,6 +1885,9 @@ struct ssi_hash_template {
 	struct ssi_drvdata *drvdata;
 };
 
+#define CC_STATE_SIZE(_x) \
+	((_x) + HASH_LEN_SIZE + SSI_MAX_HASH_BLCK_SIZE + (2 * sizeof(u32)))
+
 /* hash descriptors */
 static struct ssi_hash_template driver_hash[] = {
 	//Asynchronize hash template
@@ -1836,14 +1904,12 @@ static struct ssi_hash_template driver_hash[] = {
 			.final = ssi_ahash_final,
 			.finup = ssi_ahash_finup,
 			.digest = ssi_ahash_digest,
-#ifdef EXPORT_FIXED
 			.export = ssi_ahash_export,
 			.import = ssi_ahash_import,
-#endif
 			.setkey = ssi_ahash_setkey,
 			.halg = {
 				.digestsize = SHA1_DIGEST_SIZE,
-				.statesize = sizeof(struct sha1_state),
+				.statesize = CC_STATE_SIZE(SHA1_DIGEST_SIZE),
 			},
 		},
 		.hash_mode = DRV_HASH_SHA1,
@@ -1862,14 +1928,12 @@ static struct ssi_hash_template driver_hash[] = {
 			.final = ssi_ahash_final,
 			.finup = ssi_ahash_finup,
 			.digest = ssi_ahash_digest,
-#ifdef EXPORT_FIXED
 			.export = ssi_ahash_export,
 			.import = ssi_ahash_import,
-#endif
 			.setkey = ssi_ahash_setkey,
 			.halg = {
 				.digestsize = SHA256_DIGEST_SIZE,
-				.statesize = sizeof(struct sha256_state),
+				.statesize = CC_STATE_SIZE(SHA256_DIGEST_SIZE)
 			},
 		},
 		.hash_mode = DRV_HASH_SHA256,
@@ -1888,14 +1952,12 @@ static struct ssi_hash_template driver_hash[] = {
 			.final = ssi_ahash_final,
 			.finup = ssi_ahash_finup,
 			.digest = ssi_ahash_digest,
-#ifdef EXPORT_FIXED
 			.export = ssi_ahash_export,
 			.import = ssi_ahash_import,
-#endif
 			.setkey = ssi_ahash_setkey,
 			.halg = {
 				.digestsize = SHA224_DIGEST_SIZE,
-				.statesize = sizeof(struct sha256_state),
+				.statesize = CC_STATE_SIZE(SHA224_DIGEST_SIZE),
 			},
 		},
 		.hash_mode = DRV_HASH_SHA224,
@@ -1915,14 +1977,12 @@ static struct ssi_hash_template driver_hash[] = {
 			.final = ssi_ahash_final,
 			.finup = ssi_ahash_finup,
 			.digest = ssi_ahash_digest,
-#ifdef EXPORT_FIXED
 			.export = ssi_ahash_export,
 			.import = ssi_ahash_import,
-#endif
 			.setkey = ssi_ahash_setkey,
 			.halg = {
 				.digestsize = SHA384_DIGEST_SIZE,
-				.statesize = sizeof(struct sha512_state),
+				.statesize = CC_STATE_SIZE(SHA384_DIGEST_SIZE),
 			},
 		},
 		.hash_mode = DRV_HASH_SHA384,
@@ -1941,14 +2001,12 @@ static struct ssi_hash_template driver_hash[] = {
 			.final = ssi_ahash_final,
 			.finup = ssi_ahash_finup,
 			.digest = ssi_ahash_digest,
-#ifdef EXPORT_FIXED
 			.export = ssi_ahash_export,
 			.import = ssi_ahash_import,
-#endif
 			.setkey = ssi_ahash_setkey,
 			.halg = {
 				.digestsize = SHA512_DIGEST_SIZE,
-				.statesize = sizeof(struct sha512_state),
+				.statesize = CC_STATE_SIZE(SHA512_DIGEST_SIZE),
 			},
 		},
 		.hash_mode = DRV_HASH_SHA512,
@@ -1968,14 +2026,12 @@ static struct ssi_hash_template driver_hash[] = {
 			.final = ssi_ahash_final,
 			.finup = ssi_ahash_finup,
 			.digest = ssi_ahash_digest,
-#ifdef EXPORT_FIXED
 			.export = ssi_ahash_export,
 			.import = ssi_ahash_import,
-#endif
 			.setkey = ssi_ahash_setkey,
 			.halg = {
 				.digestsize = MD5_DIGEST_SIZE,
-				.statesize = sizeof(struct md5_state),
+				.statesize = CC_STATE_SIZE(MD5_DIGEST_SIZE),
 			},
 		},
 		.hash_mode = DRV_HASH_MD5,
@@ -1993,13 +2049,11 @@ static struct ssi_hash_template driver_hash[] = {
 			.finup = ssi_mac_finup,
 			.digest = ssi_mac_digest,
 			.setkey = ssi_xcbc_setkey,
-#ifdef EXPORT_FIXED
 			.export = ssi_ahash_export,
 			.import = ssi_ahash_import,
-#endif
 			.halg = {
 				.digestsize = AES_BLOCK_SIZE,
-				.statesize = sizeof(struct aeshash_state),
+				.statesize = CC_STATE_SIZE(AES_BLOCK_SIZE),
 			},
 		},
 		.hash_mode = DRV_HASH_NULL,
@@ -2018,13 +2072,11 @@ static struct ssi_hash_template driver_hash[] = {
 			.finup = ssi_mac_finup,
 			.digest = ssi_mac_digest,
 			.setkey = ssi_cmac_setkey,
-#ifdef EXPORT_FIXED
 			.export = ssi_ahash_export,
 			.import = ssi_ahash_import,
-#endif
 			.halg = {
 				.digestsize = AES_BLOCK_SIZE,
-				.statesize = sizeof(struct aeshash_state),
+				.statesize = CC_STATE_SIZE(AES_BLOCK_SIZE),
 			},
 		},
 		.hash_mode = DRV_HASH_NULL,
diff --git a/drivers/staging/ccree/ssi_hash.h b/drivers/staging/ccree/ssi_hash.h
index 7c94661..0bb99cb 100644
--- a/drivers/staging/ccree/ssi_hash.h
+++ b/drivers/staging/ccree/ssi_hash.h
@@ -39,6 +39,8 @@
 #define XCBC_MAC_K2_OFFSET 16
 #define XCBC_MAC_K3_OFFSET 32
 
+#define CC_EXPORT_MAGIC 0xC2EE1070U
+
 // this struct was taken from drivers/crypto/nx/nx-aes-xcbc.c and it is used for xcbc/cmac statesize
 struct aeshash_state {
 	u8 state[AES_BLOCK_SIZE];
-- 
2.1.4

[toc] | [prev] | [next] | [standalone]

#1672650 — Re: [PATCH v2 1/7] staging: ccree: fix hash import/export

On Thu, Jun 22, 2017 at 04:36:55PM +0300, Gilad Ben-Yossef wrote:
>  static int ssi_ahash_export(struct ahash_request *req, void *out)
>  {
>  	struct crypto_ahash *ahash = crypto_ahash_reqtfm(req);
>  	struct ssi_hash_ctx *ctx = crypto_ahash_ctx(ahash);
> +	struct device *dev = &ctx->drvdata->plat_dev->dev;
> +	struct ahash_req_ctx *state = ahash_request_ctx(req);
> +	u8 *curr_buff = state->buff_index ? state->buff1 : state->buff0;
> +	u32 curr_buff_cnt = state->buff_index ? state->buff1_cnt :
> +				state->buff0_cnt;
> +	const u32 tmp = CC_EXPORT_MAGIC;
> +
> +	CHECK_AND_RETURN_UPON_FIPS_ERROR();
>  
> -	return ssi_hash_export(ctx, out);
> +	memcpy(out, &tmp, sizeof(u32));
> +	out += sizeof(u32);
> +
> +	dma_sync_single_for_cpu(dev, state->digest_buff_dma_addr,
> +				ctx->inter_digestsize, DMA_BIDIRECTIONAL);
> +	memcpy(out, state->digest_buff, ctx->inter_digestsize);
> +	out += ctx->inter_digestsize;
> +
> +	if (state->digest_bytes_len_dma_addr) {
> +		dma_sync_single_for_cpu(dev, state->digest_bytes_len_dma_addr,
> +					HASH_LEN_SIZE, DMA_BIDIRECTIONAL);
> +		memcpy(out, state->digest_bytes_len, HASH_LEN_SIZE);
> +	}
> +	out += HASH_LEN_SIZE;

I'm sorry to ask this question now instead of on my first review.  I was
waiting for my other computer to get ready so I could look up how this
is called.  But now that I've looked at it, I'm still not sure how this
is called.

Anyway, my question is, is out zeroed memory?  Should we have something
like:
	} else {
		memset(out, 0, HASH_LEN_SIZE);
	}
	out += HASH_LEN_SIZE;

The ->import() function has a similar snippet.

regards,
dan carpenter

[toc] | [prev] | [next] | [standalone]

#1674180 — Re: [PATCH v2 1/7] staging: ccree: fix hash import/export

On Thu, Jun 22, 2017 at 4:58 PM, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> On Thu, Jun 22, 2017 at 04:36:55PM +0300, Gilad Ben-Yossef wrote:
>>  static int ssi_ahash_export(struct ahash_request *req, void *out)
>>  {
>>       struct crypto_ahash *ahash = crypto_ahash_reqtfm(req);
>>       struct ssi_hash_ctx *ctx = crypto_ahash_ctx(ahash);
>> +     struct device *dev = &ctx->drvdata->plat_dev->dev;
>> +     struct ahash_req_ctx *state = ahash_request_ctx(req);
>> +     u8 *curr_buff = state->buff_index ? state->buff1 : state->buff0;
>> +     u32 curr_buff_cnt = state->buff_index ? state->buff1_cnt :
>> +                             state->buff0_cnt;
>> +     const u32 tmp = CC_EXPORT_MAGIC;
>> +
>> +     CHECK_AND_RETURN_UPON_FIPS_ERROR();
>>
>> -     return ssi_hash_export(ctx, out);
>> +     memcpy(out, &tmp, sizeof(u32));
>> +     out += sizeof(u32);
>> +
>> +     dma_sync_single_for_cpu(dev, state->digest_buff_dma_addr,
>> +                             ctx->inter_digestsize, DMA_BIDIRECTIONAL);
>> +     memcpy(out, state->digest_buff, ctx->inter_digestsize);
>> +     out += ctx->inter_digestsize;
>> +
>> +     if (state->digest_bytes_len_dma_addr) {
>> +             dma_sync_single_for_cpu(dev, state->digest_bytes_len_dma_addr,
>> +                                     HASH_LEN_SIZE, DMA_BIDIRECTIONAL);
>> +             memcpy(out, state->digest_bytes_len, HASH_LEN_SIZE);
>> +     }
>> +     out += HASH_LEN_SIZE;
>
> I'm sorry to ask this question now instead of on my first review.  I was
> waiting for my other computer to get ready so I could look up how this
> is called.  But now that I've looked at it, I'm still not sure how this
> is called.

No reason to be sorry. I value every review I get, no matter the timing :-)

>
> Anyway, my question is, is out zeroed memory?  Should we have something
> like:
>         } else {
>                 memset(out, 0, HASH_LEN_SIZE);
>         }
>         out += HASH_LEN_SIZE;
>

The export/import function serialize internal hash state into/from a
user supplied
buffer, which may or may not be zeroed.

The import/output is an opaque struct to the user, and in these
specific circumstances
that field (location in hashed data) is simply not used.

After thinking about it, I think the correct thing to do here is to
poison the unused field,
rather than zero it to make it easier to debug cases where someone
passes to export
an import of a different hash/parameters.

Thanks for pointing it out.


> The ->import() function has a similar snippet.

That one is easier as the internal data that is being imported is
simply not allocated
in this scenario at all.

Thanks!
Gilad

-- 
Gilad Ben-Yossef
Chief Coffee Drinker

"If you take a class in large-scale robotics, can you end up in a
situation where the homework eats your dog?"
 -- Jean-Baptiste Queru

[toc] | [prev] | [next] | [standalone]

#1673836 — Re: [PATCH v2 3/7] staging: ccree: add support for older HW revisions

On Thu, Jun 22, 2017 at 04:36:57PM +0300, Gilad Ben-Yossef wrote:
> Add support for the older CryptoCell 710 and 630P hardware revisions.

No, I do not want to add new features to staging drivers where ever
possible.  I want you to spend your time fixing up the code to be good
enough to get it out of staging, then you can add any new hardware
support you want to it without having to go back and clean up anything
else.

That is one of the requirements of code in staging, sorry.

thanks,

greg k-h

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web