[PATCH] crypto: caam - fix setting IV after decrypt

2018-12-07 Thread Sascha Hauer
The crypto API wants the updated IV in req->info after decryption. The
updated IV used to be copied correctly to req->info after running the
decryption job. Since 115957bb3e59 this is done before running the job
so instead of the updated IV only the unmodified input IV is given back
to the crypto API.

This was observed running the gcm(aes) selftest which internally uses
ctr(aes) implemented by the CAAM engine.

Fixes: 115957bb3e59 ("crypto: caam - fix IV DMA mapping and updating")

Signed-off-by: Sascha Hauer 
Cc: sta...@vger.kernel.org
---
 drivers/crypto/caam/caamalg.c | 17 +
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/crypto/caam/caamalg.c b/drivers/crypto/caam/caamalg.c
index 869f092432de..c05c7938439c 100644
--- a/drivers/crypto/caam/caamalg.c
+++ b/drivers/crypto/caam/caamalg.c
@@ -917,10 +917,10 @@ static void skcipher_decrypt_done(struct device *jrdev, 
u32 *desc, u32 err,
 {
struct skcipher_request *req = context;
struct skcipher_edesc *edesc;
-#ifdef DEBUG
struct crypto_skcipher *skcipher = crypto_skcipher_reqtfm(req);
int ivsize = crypto_skcipher_ivsize(skcipher);
 
+#ifdef DEBUG
dev_err(jrdev, "%s %d: err 0x%x\n", __func__, __LINE__, err);
 #endif
 
@@ -937,6 +937,14 @@ static void skcipher_decrypt_done(struct device *jrdev, 
u32 *desc, u32 err,
 edesc->dst_nents > 1 ? 100 : req->cryptlen, 1);
 
skcipher_unmap(jrdev, edesc, req);
+
+   /*
+* The crypto API expects us to set the IV (req->iv) to the last
+* ciphertext block.
+*/
+   scatterwalk_map_and_copy(req->iv, req->src, req->cryptlen - ivsize,
+ivsize, 0);
+
kfree(edesc);
 
skcipher_request_complete(req, err);
@@ -1588,13 +1596,6 @@ static int skcipher_decrypt(struct skcipher_request *req)
if (IS_ERR(edesc))
return PTR_ERR(edesc);
 
-   /*
-* The crypto API expects us to set the IV (req->iv) to the last
-* ciphertext block.
-*/
-   scatterwalk_map_and_copy(req->iv, req->src, req->cryptlen - ivsize,
-ivsize, 0);
-
/* Create and submit job descriptor*/
init_skcipher_job(req, edesc, false);
desc = edesc->hw_desc;
-- 
2.19.1



Re: [PATCH 03/12] crypto: caam - Enable and disable clocks on Freescale i.MX platforms

2015-07-31 Thread Sascha Hauer
On Thu, Jul 30, 2015 at 11:32:44PM -0700, Victoria Milhoan wrote:
 Hi Sascha,
 
 Thank you for the responses. Comments inline. Changes will be 
 in the next version of the patch set.
 
 -Victoria
 
 On Thu, 30 Jul 2015 08:02:14 +0200
 Sascha Hauer s.ha...@pengutronix.de wrote:
 
  Hi Victoria,
  
  comments inline.
  
  On Wed, Jul 29, 2015 at 08:58:20PM -0700, Victoria Milhoan wrote:
   ARM-based systems may disable clocking to the CAAM device on the
   Freescale i.MX platform for power management purposes.  This patch
   enables the required clocks when the CAAM module is initialized and
   disables the required clocks when the CAAM module is shut down.
   
   Signed-off-by: Victoria Milhoan vicki.milh...@freescale.com
   ---
drivers/crypto/caam/compat.h |   1 +
drivers/crypto/caam/ctrl.c   | 191 
   +++
drivers/crypto/caam/intern.h |   5 ++
3 files changed, 197 insertions(+)
   
   diff --git a/drivers/crypto/caam/compat.h b/drivers/crypto/caam/compat.h
   index f57f395..b6955ec 100644
   --- a/drivers/crypto/caam/compat.h
   +++ b/drivers/crypto/caam/compat.h
   @@ -23,6 +23,7 @@
#include linux/types.h
#include linux/debugfs.h
#include linux/circ_buf.h
   +#include linux/clk.h
#include net/xfrm.h

#include crypto/algapi.h
   diff --git a/drivers/crypto/caam/ctrl.c b/drivers/crypto/caam/ctrl.c
   index 660cc3e..cfd8c9e 100644
   --- a/drivers/crypto/caam/ctrl.c
   +++ b/drivers/crypto/caam/ctrl.c
   @@ -16,6 +16,121 @@
#include error.h

/*
   + * ARM targets tend to have clock control subsystems that can
   + * enable/disable clocking to our device. Support clocking
   + * with the following functions.
   + */
   +#ifdef CONFIG_ARM
   +static inline struct clk *caam_drv_get_clk_ipg(struct caam_drv_private 
   *drv)
   +{
   + return drv-caam_ipg;
   +}
  
  You return drv-caam_ipg on ARM and NULL on powerpc. drv-caam_ipg is
  NULL on powerpc anyway which makes this different implementations for
  ARM and powerpc unnecessary. Just access drv-caam_ipg directly where
  needed.
 
 Agreed. I've reworked the patch to use direct references to the clocks.
 
  
   +
   +static inline struct clk *caam_drv_get_clk_mem(struct caam_drv_private 
   *drv)
   +{
   + return drv-caam_mem;
   +}
   +
   +static inline struct clk *caam_drv_get_clk_aclk(struct caam_drv_private 
   *drv)
   +{
   + return drv-caam_aclk;
   +}
   +
   +static inline struct clk *caam_drv_get_clk_emislow(struct 
   caam_drv_private *drv)
   +{
   + return drv-caam_emi_slow;
   +}
   +
   +static inline void caam_drv_set_clk_ipg(struct caam_drv_private *drv,
   + struct clk *clk)
   +{
   + drv-caam_ipg = clk;
   +}
  
  Ditto, just access drv-caam_ipg when needed.
  
   +
   +static inline void caam_drv_set_clk_mem(struct caam_drv_private *drv,
   + struct clk *clk)
   +{
   + drv-caam_mem = clk;
   +}
   +
   +static inline void caam_drv_set_clk_aclk(struct caam_drv_private *drv,
   +  struct clk *clk)
   +{
   + drv-caam_aclk = clk;
   +}
   +
   +static inline void caam_drv_set_clk_emislow(struct caam_drv_private *drv,
   + struct clk *clk)
   +{
   + drv-caam_emi_slow = clk;
   +}
   +
   +static inline struct clk *caam_drv_identify_clk(struct device *dev,
   + char *clk_name)
   +{
   + return devm_clk_get(dev, clk_name);
   +}
  
  devm_clk_get() returns NULL when the architecture does not have clk
  support, so it also seems unnecessary to have architecture specific
  implementations for this.
  
 
 Some of the QorIQ architectures have clock support enabled but do not
 support clocking to CAAM. This causes devm_clk_get() to return an error
 for these clocks instead of NULL. So, the only architecture-specific code
 left in the reworked patch is caam_drv_identify_clk().

I see. Wouldn't it be better to add CAAM clk support for QorIQ then?

Sascha

-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 08/12] ARM: clk-imx6q: Add CAAM clock support

2015-07-30 Thread Sascha Hauer
On Wed, Jul 29, 2015 at 08:58:25PM -0700, Victoria Milhoan wrote:
 Add CAAM clock support to the i.MX6 clocking infrastructure.
 
 Signed-off-by: Victoria Milhoan vicki.milh...@freescale.com
 ---
  drivers/clk/imx/clk-imx6q.c   | 3 +++
  include/dt-bindings/clock/imx6qdl-clock.h | 5 -
  2 files changed, 7 insertions(+), 1 deletion(-)
 
 diff --git a/drivers/clk/imx/clk-imx6q.c b/drivers/clk/imx/clk-imx6q.c
 index d046f8e..4de4943 100644
 --- a/drivers/clk/imx/clk-imx6q.c
 +++ b/drivers/clk/imx/clk-imx6q.c
 @@ -381,6 +381,9 @@ static void __init imx6q_clocks_init(struct device_node 
 *ccm_node)
   clk[IMX6QDL_CLK_ASRC] = imx_clk_gate2_shared(asrc, 
 asrc_podf,   base + 0x68, 6, share_count_asrc);
   clk[IMX6QDL_CLK_ASRC_IPG] = imx_clk_gate2_shared(asrc_ipg, 
 ahb, base + 0x68, 6, share_count_asrc);
   clk[IMX6QDL_CLK_ASRC_MEM] = imx_clk_gate2_shared(asrc_mem, 
 ahb, base + 0x68, 6, share_count_asrc);
 + clk[IMX6QDL_CAAM_MEM] = imx_clk_gate2(caam_mem,  ahb,   
 base + 0x68, 8);
 + clk[IMX6QDL_CAAM_ACLK]= imx_clk_gate2(caam_aclk, ahb,   
 base + 0x68, 10);
 + clk[IMX6QDL_CAAM_IPG] = imx_clk_gate2(caam_ipg,  ipg,   
 base + 0x68, 12);
   clk[IMX6QDL_CLK_CAN1_IPG] = imx_clk_gate2(can1_ipg,  ipg,   
 base + 0x68, 14);
   clk[IMX6QDL_CLK_CAN1_SERIAL]  = imx_clk_gate2(can1_serial,   
 can_root,  base + 0x68, 16);
   clk[IMX6QDL_CLK_CAN2_IPG] = imx_clk_gate2(can2_ipg,  ipg,   
 base + 0x68, 18);
 diff --git a/include/dt-bindings/clock/imx6qdl-clock.h 
 b/include/dt-bindings/clock/imx6qdl-clock.h
 index 8780868..f68e925 100644
 --- a/include/dt-bindings/clock/imx6qdl-clock.h
 +++ b/include/dt-bindings/clock/imx6qdl-clock.h
 @@ -251,6 +251,9 @@
  #define IMX6QDL_CLK_VIDEO_27M238
  #define IMX6QDL_CLK_MIPI_CORE_CFG239
  #define IMX6QDL_CLK_MIPI_IPG 240
 -#define IMX6QDL_CLK_END  241
 +#define IMX6QDL_CAAM_MEM 241
 +#define IMX6QDL_CAAM_ACLK242
 +#define IMX6QDL_CAAM_IPG 243

IMX6QDL_CLK_* please.

Sascha

-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 09/12] ARM: dts: mx6qdl: Add CAAM device node

2015-07-30 Thread Sascha Hauer
On Wed, Jul 29, 2015 at 08:58:26PM -0700, Victoria Milhoan wrote:
 Add CAAM device node to the i.MX6 device tree.
 
 Signed-off-by: Victoria Milhoan vicki.milh...@freescale.com
 ---
  arch/arm/boot/dts/imx6qdl.dtsi | 30 ++
  1 file changed, 26 insertions(+), 4 deletions(-)
 
 diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi
 index e6d1359..4df9f1e 100644
 --- a/arch/arm/boot/dts/imx6qdl.dtsi
 +++ b/arch/arm/boot/dts/imx6qdl.dtsi
 @@ -836,10 +836,32 @@
   reg = 0x0210 0x10;
   ranges;
  
 - caam@0210 {
 - reg = 0x0210 0x4;
 - interrupts = 0 105 IRQ_TYPE_LEVEL_HIGH,
 -  0 106 IRQ_TYPE_LEVEL_HIGH;
 + crypto: caam@210 {
 + compatible = fsl,sec-v4.0;
 + fsl,sec-era = 4;
 + #address-cells = 1;
 + #size-cells = 1;
 + reg = 0x210 0x1;
 + ranges = 0 0x210 0x1;
 + interrupt-parent = intc;
 + clocks = clks IMX6QDL_CAAM_MEM,
 +  clks IMX6QDL_CAAM_ACLK,
 +  clks IMX6QDL_CAAM_IPG,
 +  clks IMX6QDL_CLK_EIM_SLOW;
 + clock-names = caam_mem, caam_aclk,
 +   caam_ipg, caam_emi_slow;

The binding document should be updated for these additional properties.
The namespace caam_ is already clear from the context, you can drop
these prefixes.

Sascha


-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 03/12] crypto: caam - Enable and disable clocks on Freescale i.MX platforms

2015-07-30 Thread Sascha Hauer
Hi Victoria,

comments inline.

On Wed, Jul 29, 2015 at 08:58:20PM -0700, Victoria Milhoan wrote:
 ARM-based systems may disable clocking to the CAAM device on the
 Freescale i.MX platform for power management purposes.  This patch
 enables the required clocks when the CAAM module is initialized and
 disables the required clocks when the CAAM module is shut down.
 
 Signed-off-by: Victoria Milhoan vicki.milh...@freescale.com
 ---
  drivers/crypto/caam/compat.h |   1 +
  drivers/crypto/caam/ctrl.c   | 191 
 +++
  drivers/crypto/caam/intern.h |   5 ++
  3 files changed, 197 insertions(+)
 
 diff --git a/drivers/crypto/caam/compat.h b/drivers/crypto/caam/compat.h
 index f57f395..b6955ec 100644
 --- a/drivers/crypto/caam/compat.h
 +++ b/drivers/crypto/caam/compat.h
 @@ -23,6 +23,7 @@
  #include linux/types.h
  #include linux/debugfs.h
  #include linux/circ_buf.h
 +#include linux/clk.h
  #include net/xfrm.h
  
  #include crypto/algapi.h
 diff --git a/drivers/crypto/caam/ctrl.c b/drivers/crypto/caam/ctrl.c
 index 660cc3e..cfd8c9e 100644
 --- a/drivers/crypto/caam/ctrl.c
 +++ b/drivers/crypto/caam/ctrl.c
 @@ -16,6 +16,121 @@
  #include error.h
  
  /*
 + * ARM targets tend to have clock control subsystems that can
 + * enable/disable clocking to our device. Support clocking
 + * with the following functions.
 + */
 +#ifdef CONFIG_ARM
 +static inline struct clk *caam_drv_get_clk_ipg(struct caam_drv_private *drv)
 +{
 + return drv-caam_ipg;
 +}

You return drv-caam_ipg on ARM and NULL on powerpc. drv-caam_ipg is
NULL on powerpc anyway which makes this different implementations for
ARM and powerpc unnecessary. Just access drv-caam_ipg directly where
needed.

 +
 +static inline struct clk *caam_drv_get_clk_mem(struct caam_drv_private *drv)
 +{
 + return drv-caam_mem;
 +}
 +
 +static inline struct clk *caam_drv_get_clk_aclk(struct caam_drv_private *drv)
 +{
 + return drv-caam_aclk;
 +}
 +
 +static inline struct clk *caam_drv_get_clk_emislow(struct caam_drv_private 
 *drv)
 +{
 + return drv-caam_emi_slow;
 +}
 +
 +static inline void caam_drv_set_clk_ipg(struct caam_drv_private *drv,
 + struct clk *clk)
 +{
 + drv-caam_ipg = clk;
 +}

Ditto, just access drv-caam_ipg when needed.

 +
 +static inline void caam_drv_set_clk_mem(struct caam_drv_private *drv,
 + struct clk *clk)
 +{
 + drv-caam_mem = clk;
 +}
 +
 +static inline void caam_drv_set_clk_aclk(struct caam_drv_private *drv,
 +  struct clk *clk)
 +{
 + drv-caam_aclk = clk;
 +}
 +
 +static inline void caam_drv_set_clk_emislow(struct caam_drv_private *drv,
 + struct clk *clk)
 +{
 + drv-caam_emi_slow = clk;
 +}
 +
 +static inline struct clk *caam_drv_identify_clk(struct device *dev,
 + char *clk_name)
 +{
 + return devm_clk_get(dev, clk_name);
 +}

devm_clk_get() returns NULL when the architecture does not have clk
support, so it also seems unnecessary to have architecture specific
implementations for this.

 +
 +static inline void caam_drv_show_clk(struct device *dev, struct clk *clk,
 +  char *clk_name)
 +{
 + dev_info(dev, %s clock:%d\n, clk_name, (int)clk_get_rate(clk));
 +}

The correct format specifier for unsigned long is %lu, no need to cast
it to int. Besides, this information is not generally interesting, you
can drop this.

 +
 +#else
 +static inline struct clk *caam_drv_get_clk_ipg(struct caam_drv_private *drv)
 +{
 + return NULL;
 +}
 +
 +static inline struct clk *caam_drv_get_clk_mem(struct caam_drv_private *drv)
 +{
 + return NULL;
 +}
 +
 +static inline struct clk *caam_drv_get_clk_aclk(struct caam_drv_private *drv)
 +{
 + return NULL;
 +}
 +
 +static inline struct clk *caam_drv_get_clk_emislow(struct caam_drv_private 
 *drv)
 +{
 + return NULL;
 +}
 +
 +static inline void caam_drv_set_clk_ipg(struct caam_drv_private *drv,
 + struct clk *clk)
 +{
 +}
 +
 +static inline void caam_drv_set_clk_mem(struct caam_drv_private *drv,
 + struct clk *clk)
 +{
 +}
 +
 +static inline void caam_drv_set_clk_aclk(struct caam_drv_private *drv,
 +  struct clk *clk)
 +{
 +}
 +
 +static inline void caam_drv_set_clk_emislow(struct caam_drv_private *drv,
 + struct clk *clk)
 +{
 +}
 +
 +static inline struct clk *caam_drv_identify_clk(struct device *dev,
 + char *clk_name)
 +{
 + return 0;
 +}
 +
 +static inline void caam_drv_show_clk(struct device *dev, struct clk *clk,
 +  char *clk_name)
 +{
 +}
 +#endif
 +
 +/*
   * Descriptor to instantiate RNG State Handle 0 in normal mode and
   * load the JDKEK, TDKEK and TDSK registers
   */
 @@ 

Re: [PATCH v3 1/2] i.MX27: Add clock support for SAHARA2.

2013-03-11 Thread Sascha Hauer
On Mon, Mar 11, 2013 at 09:19:26AM +0800, Herbert Xu wrote:
 On Mon, Mar 11, 2013 at 12:08:56AM +0100, Sascha Hauer wrote:
  On Sun, Mar 10, 2013 at 04:34:01PM +0800, Herbert Xu wrote:

 https://patchwork.kernel.org/patch/1817741/

 So the change above becomes unnecessary

Very good. Then this patch can be safely dropped.
   
   So should I take this patch or not?
  
  This clk patch, no. The sahara patch, yes, if it is fine for you.
 
 But will the second patch work fine without the first?

It will work once a device is registered. The necessary clocks for it
will be provided by the devicetree then.

Sascha

-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 3/3] crypto: sahara: Add device tree binding for SAHARA2.

2013-02-27 Thread Sascha Hauer
Hi Javier,

On Wed, Feb 27, 2013 at 11:41:51AM +0100, Javier Martin wrote:
 
 Signed-off-by: Javier Martin javier.mar...@vista-silicon.com
 ---
  .../devicetree/bindings/crypto/fsl-imx-sahara.txt  |   14 ++
  1 file changed, 14 insertions(+)
  create mode 100644 
 Documentation/devicetree/bindings/crypto/fsl-imx-sahara.txt
 
 diff --git a/Documentation/devicetree/bindings/crypto/fsl-imx-sahara.txt 
 b/Documentation/devicetree/bindings/crypto/fsl-imx-sahara.txt
 new file mode 100644
 index 000..44abf11
 --- /dev/null
 +++ b/Documentation/devicetree/bindings/crypto/fsl-imx-sahara.txt
 @@ -0,0 +1,14 @@
 +* Freescale i.MX SAHARA Cryptographic Accelerator
 +
 +Required properties:
 +- compatible : Should be fsl,soc-sahara

Please add explicitly which SoCs are supported.

You can fold this patch into the driver patch. This way people reading
the commit history have a direct cross link between the driver and the
documentation.

Sascha

-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html