linux-crypto.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] crypto: sun4i-ss: fix SHA1 on A33 SecuritySystem
@ 2019-11-14 14:48 Corentin Labbe
  2019-11-14 14:48 ` [PATCH 1/3] dt-bindings: crypto: add new compatible for " Corentin Labbe
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Corentin Labbe @ 2019-11-14 14:48 UTC (permalink / raw)
  To: davem, herbert, mark.rutland, mripard, robh+dt, wens
  Cc: devicetree, linux-arm-kernel, linux-crypto, linux-kernel,
	linux-sunxi, Corentin Labbe

Thanks to Igor Pecovnik, I have now in my kernelCI lab, a sun8i-a33-olinuxino.
Strange behavour, crypto selftests was failling but only for SHA1 on
this A33 SoC.

This is due to the A33 SS having a difference with all other SS, it give SHA1 digest directly in BE.
This serie handle this difference.

Corentin Labbe (3):
  dt-bindings: crypto: add new compatible for A33 SS
  ARM: dts: sun8i: a33: add the new SS compatible
  crypto: sun4i-ss: add the A33 variant of SS

 .../crypto/allwinner,sun4i-a10-crypto.yaml    |  3 +++
 arch/arm/boot/dts/sun8i-a33.dtsi              |  3 ++-
 .../crypto/allwinner/sun4i-ss/sun4i-ss-core.c | 22 ++++++++++++++++++-
 .../crypto/allwinner/sun4i-ss/sun4i-ss-hash.c |  5 ++++-
 drivers/crypto/allwinner/sun4i-ss/sun4i-ss.h  |  9 ++++++++
 5 files changed, 39 insertions(+), 3 deletions(-)

-- 
2.23.0


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 1/3] dt-bindings: crypto: add new compatible for A33 SecuritySystem
  2019-11-14 14:48 [PATCH 0/3] crypto: sun4i-ss: fix SHA1 on A33 SecuritySystem Corentin Labbe
@ 2019-11-14 14:48 ` Corentin Labbe
  2019-11-14 14:48 ` [PATCH 2/3] ARM: dts: sun8i: a33: add the new SecuritySystem compatible Corentin Labbe
  2019-11-14 14:48 ` [PATCH 3/3] crypto: sun4i-ss: add the A33 variant of SecuritySystem Corentin Labbe
  2 siblings, 0 replies; 7+ messages in thread
From: Corentin Labbe @ 2019-11-14 14:48 UTC (permalink / raw)
  To: davem, herbert, mark.rutland, mripard, robh+dt, wens
  Cc: devicetree, linux-arm-kernel, linux-crypto, linux-kernel,
	linux-sunxi, Corentin Labbe

The A33 SecuritySystem has a difference with all other SS, it give SHA1 digest
directly in BE.
This difference need to be handlded by the driver and so need a new
compatible.

Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
---
 .../devicetree/bindings/crypto/allwinner,sun4i-a10-crypto.yaml | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/crypto/allwinner,sun4i-a10-crypto.yaml b/Documentation/devicetree/bindings/crypto/allwinner,sun4i-a10-crypto.yaml
index 80b3e7350a73..ae6dcfa795d1 100644
--- a/Documentation/devicetree/bindings/crypto/allwinner,sun4i-a10-crypto.yaml
+++ b/Documentation/devicetree/bindings/crypto/allwinner,sun4i-a10-crypto.yaml
@@ -23,6 +23,9 @@ properties:
       - items:
         - const: allwinner,sun7i-a20-crypto
         - const: allwinner,sun4i-a10-crypto
+      - items:
+        - const: allwinner,sun8i-a33-crypto
+        - const: allwinner,sun4i-a10-crypto
 
   reg:
     maxItems: 1
-- 
2.23.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 2/3] ARM: dts: sun8i: a33: add the new SecuritySystem compatible
  2019-11-14 14:48 [PATCH 0/3] crypto: sun4i-ss: fix SHA1 on A33 SecuritySystem Corentin Labbe
  2019-11-14 14:48 ` [PATCH 1/3] dt-bindings: crypto: add new compatible for " Corentin Labbe
@ 2019-11-14 14:48 ` Corentin Labbe
  2019-11-18 11:11   ` Maxime Ripard
  2019-11-14 14:48 ` [PATCH 3/3] crypto: sun4i-ss: add the A33 variant of SecuritySystem Corentin Labbe
  2 siblings, 1 reply; 7+ messages in thread
From: Corentin Labbe @ 2019-11-14 14:48 UTC (permalink / raw)
  To: davem, herbert, mark.rutland, mripard, robh+dt, wens
  Cc: devicetree, linux-arm-kernel, linux-crypto, linux-kernel,
	linux-sunxi, Corentin Labbe

Add the new A33 SecuritySystem compatible to the crypto node.

Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
---
 arch/arm/boot/dts/sun8i-a33.dtsi | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/sun8i-a33.dtsi b/arch/arm/boot/dts/sun8i-a33.dtsi
index 1532a0e59af4..5680fa1de102 100644
--- a/arch/arm/boot/dts/sun8i-a33.dtsi
+++ b/arch/arm/boot/dts/sun8i-a33.dtsi
@@ -215,7 +215,8 @@
 		};
 
 		crypto: crypto-engine@1c15000 {
-			compatible = "allwinner,sun4i-a10-crypto";
+			compatible = "allwinner,sun8i-a33-crypto",
+				     "allwinner,sun4i-a10-crypto";
 			reg = <0x01c15000 0x1000>;
 			interrupts = <GIC_SPI 80 IRQ_TYPE_LEVEL_HIGH>;
 			clocks = <&ccu CLK_BUS_SS>, <&ccu CLK_SS>;
-- 
2.23.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 3/3] crypto: sun4i-ss: add the A33 variant of SecuritySystem
  2019-11-14 14:48 [PATCH 0/3] crypto: sun4i-ss: fix SHA1 on A33 SecuritySystem Corentin Labbe
  2019-11-14 14:48 ` [PATCH 1/3] dt-bindings: crypto: add new compatible for " Corentin Labbe
  2019-11-14 14:48 ` [PATCH 2/3] ARM: dts: sun8i: a33: add the new SecuritySystem compatible Corentin Labbe
@ 2019-11-14 14:48 ` Corentin Labbe
  2 siblings, 0 replies; 7+ messages in thread
From: Corentin Labbe @ 2019-11-14 14:48 UTC (permalink / raw)
  To: davem, herbert, mark.rutland, mripard, robh+dt, wens
  Cc: devicetree, linux-arm-kernel, linux-crypto, linux-kernel,
	linux-sunxi, Corentin Labbe

The A33 SecuritySystem has a difference with all other SS, it give SHA1 digest
directly in BE.
So this patch adds variant support in sun4i-ss.

Fixes: 6298e948215f ("crypto: sunxi-ss - Add Allwinner Security System crypto accelerator")
Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
---
 .../crypto/allwinner/sun4i-ss/sun4i-ss-core.c | 22 ++++++++++++++++++-
 .../crypto/allwinner/sun4i-ss/sun4i-ss-hash.c |  5 ++++-
 drivers/crypto/allwinner/sun4i-ss/sun4i-ss.h  |  9 ++++++++
 3 files changed, 34 insertions(+), 2 deletions(-)

diff --git a/drivers/crypto/allwinner/sun4i-ss/sun4i-ss-core.c b/drivers/crypto/allwinner/sun4i-ss/sun4i-ss-core.c
index 814cd12149a9..d35a05843c22 100644
--- a/drivers/crypto/allwinner/sun4i-ss/sun4i-ss-core.c
+++ b/drivers/crypto/allwinner/sun4i-ss/sun4i-ss-core.c
@@ -13,6 +13,7 @@
 #include <linux/io.h>
 #include <linux/module.h>
 #include <linux/of.h>
+#include <linux/of_device.h>
 #include <linux/platform_device.h>
 #include <crypto/scatterwalk.h>
 #include <linux/scatterlist.h>
@@ -22,6 +23,14 @@
 
 #include "sun4i-ss.h"
 
+static const struct ss_variant ss_a10_variant = {
+	.sha1_in_be = false,
+};
+
+static const struct ss_variant ss_a33_variant = {
+	.sha1_in_be = true,
+};
+
 static struct sun4i_ss_alg_template ss_algs[] = {
 {       .type = CRYPTO_ALG_TYPE_AHASH,
 	.mode = SS_OP_MD5,
@@ -323,6 +332,12 @@ static int sun4i_ss_probe(struct platform_device *pdev)
 		return PTR_ERR(ss->base);
 	}
 
+	ss->variant = of_device_get_match_data(&pdev->dev);
+	if (!ss->variant) {
+		dev_err(&pdev->dev, "Missing Security System variant\n");
+		return -EINVAL;
+	}
+
 	ss->ssclk = devm_clk_get(&pdev->dev, "mod");
 	if (IS_ERR(ss->ssclk)) {
 		err = PTR_ERR(ss->ssclk);
@@ -484,7 +499,12 @@ static int sun4i_ss_remove(struct platform_device *pdev)
 }
 
 static const struct of_device_id a20ss_crypto_of_match_table[] = {
-	{ .compatible = "allwinner,sun4i-a10-crypto" },
+	{ .compatible = "allwinner,sun4i-a10-crypto",
+	  .data = &ss_a10_variant
+	},
+	{ .compatible = "allwinner,sun8i-a33-crypto",
+	  .data = &ss_a33_variant
+	},
 	{}
 };
 MODULE_DEVICE_TABLE(of, a20ss_crypto_of_match_table);
diff --git a/drivers/crypto/allwinner/sun4i-ss/sun4i-ss-hash.c b/drivers/crypto/allwinner/sun4i-ss/sun4i-ss-hash.c
index 91cf58db3845..c791d6935c65 100644
--- a/drivers/crypto/allwinner/sun4i-ss/sun4i-ss-hash.c
+++ b/drivers/crypto/allwinner/sun4i-ss/sun4i-ss-hash.c
@@ -478,7 +478,10 @@ static int sun4i_hash(struct ahash_request *areq)
 	/* Get the hash from the device */
 	if (op->mode == SS_OP_SHA1) {
 		for (i = 0; i < 5; i++) {
-			v = cpu_to_be32(readl(ss->base + SS_MD0 + i * 4));
+			if (ss->variant->sha1_in_be)
+				v = cpu_to_le32(readl(ss->base + SS_MD0 + i * 4));
+			else
+				v = cpu_to_be32(readl(ss->base + SS_MD0 + i * 4));
 			memcpy(areq->result + i * 4, &v, 4);
 		}
 	} else {
diff --git a/drivers/crypto/allwinner/sun4i-ss/sun4i-ss.h b/drivers/crypto/allwinner/sun4i-ss/sun4i-ss.h
index 60425ac75d90..2b4c6333eb67 100644
--- a/drivers/crypto/allwinner/sun4i-ss/sun4i-ss.h
+++ b/drivers/crypto/allwinner/sun4i-ss/sun4i-ss.h
@@ -131,7 +131,16 @@
 #define SS_SEED_LEN 192
 #define SS_DATA_LEN 160
 
+/*
+ * struct ss_variant - Describe SS hardware variant
+ * @sha1_in_be:		The SHA1 digest is given by SS in BE, and so need to be inverted.
+ */
+struct ss_variant {
+	bool sha1_in_be;
+};
+
 struct sun4i_ss_ctx {
+	const struct ss_variant *variant;
 	void __iomem *base;
 	int irq;
 	struct clk *busclk;
-- 
2.23.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/3] ARM: dts: sun8i: a33: add the new SecuritySystem compatible
  2019-11-14 14:48 ` [PATCH 2/3] ARM: dts: sun8i: a33: add the new SecuritySystem compatible Corentin Labbe
@ 2019-11-18 11:11   ` Maxime Ripard
  2019-11-19  7:39     ` Corentin Labbe
  0 siblings, 1 reply; 7+ messages in thread
From: Maxime Ripard @ 2019-11-18 11:11 UTC (permalink / raw)
  To: Corentin Labbe
  Cc: davem, herbert, mark.rutland, robh+dt, wens, devicetree,
	linux-arm-kernel, linux-crypto, linux-kernel, linux-sunxi

[-- Attachment #1: Type: text/plain, Size: 838 bytes --]

Hi,

On Thu, Nov 14, 2019 at 03:48:11PM +0100, Corentin Labbe wrote:
> Add the new A33 SecuritySystem compatible to the crypto node.
>
> Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
> ---
>  arch/arm/boot/dts/sun8i-a33.dtsi | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/boot/dts/sun8i-a33.dtsi b/arch/arm/boot/dts/sun8i-a33.dtsi
> index 1532a0e59af4..5680fa1de102 100644
> --- a/arch/arm/boot/dts/sun8i-a33.dtsi
> +++ b/arch/arm/boot/dts/sun8i-a33.dtsi
> @@ -215,7 +215,8 @@
>  		};
>
>  		crypto: crypto-engine@1c15000 {
> -			compatible = "allwinner,sun4i-a10-crypto";
> +			compatible = "allwinner,sun8i-a33-crypto",
> +				     "allwinner,sun4i-a10-crypto";

If some algorithms aren't working properly, we can't really fall back
to it, we should just use the a33 compatible.

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/3] ARM: dts: sun8i: a33: add the new SecuritySystem compatible
  2019-11-18 11:11   ` Maxime Ripard
@ 2019-11-19  7:39     ` Corentin Labbe
  2019-11-20 13:45       ` Maxime Ripard
  0 siblings, 1 reply; 7+ messages in thread
From: Corentin Labbe @ 2019-11-19  7:39 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: davem, herbert, mark.rutland, robh+dt, wens, devicetree,
	linux-arm-kernel, linux-crypto, linux-kernel, linux-sunxi

On Mon, Nov 18, 2019 at 12:11:43PM +0100, Maxime Ripard wrote:
> Hi,
> 
> On Thu, Nov 14, 2019 at 03:48:11PM +0100, Corentin Labbe wrote:
> > Add the new A33 SecuritySystem compatible to the crypto node.
> >
> > Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
> > ---
> >  arch/arm/boot/dts/sun8i-a33.dtsi | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/arm/boot/dts/sun8i-a33.dtsi b/arch/arm/boot/dts/sun8i-a33.dtsi
> > index 1532a0e59af4..5680fa1de102 100644
> > --- a/arch/arm/boot/dts/sun8i-a33.dtsi
> > +++ b/arch/arm/boot/dts/sun8i-a33.dtsi
> > @@ -215,7 +215,8 @@
> >  		};
> >
> >  		crypto: crypto-engine@1c15000 {
> > -			compatible = "allwinner,sun4i-a10-crypto";
> > +			compatible = "allwinner,sun8i-a33-crypto",
> > +				     "allwinner,sun4i-a10-crypto";
> 
> If some algorithms aren't working properly, we can't really fall back
> to it, we should just use the a33 compatible.
> 

Since crypto selftest detect the problem, the fallback could be used and SS will just be in degraded mode (no sha1).
But since nobody reported this problem since 4 years (when SS was added in a33 dts), the absence of sha1 is clearly not an issue.

Regards

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/3] ARM: dts: sun8i: a33: add the new SecuritySystem compatible
  2019-11-19  7:39     ` Corentin Labbe
@ 2019-11-20 13:45       ` Maxime Ripard
  0 siblings, 0 replies; 7+ messages in thread
From: Maxime Ripard @ 2019-11-20 13:45 UTC (permalink / raw)
  To: Corentin Labbe
  Cc: Maxime Ripard, davem, herbert, mark.rutland, robh+dt, wens,
	devicetree, linux-arm-kernel, linux-crypto, linux-kernel,
	linux-sunxi

[-- Attachment #1: Type: text/plain, Size: 1606 bytes --]

Hi,

On Tue, Nov 19, 2019 at 08:39:24AM +0100, Corentin Labbe wrote:
> On Mon, Nov 18, 2019 at 12:11:43PM +0100, Maxime Ripard wrote:
> > Hi,
> >
> > On Thu, Nov 14, 2019 at 03:48:11PM +0100, Corentin Labbe wrote:
> > > Add the new A33 SecuritySystem compatible to the crypto node.
> > >
> > > Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
> > > ---
> > >  arch/arm/boot/dts/sun8i-a33.dtsi | 3 ++-
> > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/arch/arm/boot/dts/sun8i-a33.dtsi b/arch/arm/boot/dts/sun8i-a33.dtsi
> > > index 1532a0e59af4..5680fa1de102 100644
> > > --- a/arch/arm/boot/dts/sun8i-a33.dtsi
> > > +++ b/arch/arm/boot/dts/sun8i-a33.dtsi
> > > @@ -215,7 +215,8 @@
> > >  		};
> > >
> > >  		crypto: crypto-engine@1c15000 {
> > > -			compatible = "allwinner,sun4i-a10-crypto";
> > > +			compatible = "allwinner,sun8i-a33-crypto",
> > > +				     "allwinner,sun4i-a10-crypto";
> >
> > If some algorithms aren't working properly, we can't really fall back
> > to it, we should just use the a33 compatible.
>
> Since crypto selftest detect the problem, the fallback could be used
> and SS will just be in degraded mode (no sha1).
>
> But since nobody reported this problem since 4 years (when SS was
> added in a33 dts), the absence of sha1 is clearly not an issue.

It's not really the point though. There's a bug, it's something that
was overlooked and it's unfortunate. The bug is still there though,
and the only option to fix it properly is to simply fix, not claim
that it's somewhat ok to keep it around since no one really uses it
anyway.

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2019-11-20 13:45 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-14 14:48 [PATCH 0/3] crypto: sun4i-ss: fix SHA1 on A33 SecuritySystem Corentin Labbe
2019-11-14 14:48 ` [PATCH 1/3] dt-bindings: crypto: add new compatible for " Corentin Labbe
2019-11-14 14:48 ` [PATCH 2/3] ARM: dts: sun8i: a33: add the new SecuritySystem compatible Corentin Labbe
2019-11-18 11:11   ` Maxime Ripard
2019-11-19  7:39     ` Corentin Labbe
2019-11-20 13:45       ` Maxime Ripard
2019-11-14 14:48 ` [PATCH 3/3] crypto: sun4i-ss: add the A33 variant of SecuritySystem 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).