2019-11-14 14:49:10

by Corentin Labbe

[permalink] [raw]
Subject: [PATCH 0/3] crypto: sun4i-ss: fix SHA1 on A33 SecuritySystem

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


2019-11-14 14:49:53

by Corentin Labbe

[permalink] [raw]
Subject: [PATCH 1/3] dt-bindings: crypto: add new compatible for A33 SecuritySystem

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 <[email protected]>
---
.../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

2019-11-14 14:51:57

by Corentin Labbe

[permalink] [raw]
Subject: [PATCH 2/3] ARM: dts: sun8i: a33: add the new SecuritySystem compatible

Add the new A33 SecuritySystem compatible to the crypto node.

Signed-off-by: Corentin Labbe <[email protected]>
---
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

2019-11-19 07:41:15

by Corentin Labbe

[permalink] [raw]
Subject: Re: [PATCH 2/3] ARM: dts: sun8i: a33: add the new SecuritySystem compatible

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 <[email protected]>
> > ---
> > 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

2019-11-20 14:01:21

by Maxime Ripard

[permalink] [raw]
Subject: Re: [PATCH 2/3] ARM: dts: sun8i: a33: add the new SecuritySystem compatible

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 <[email protected]>
> > > ---
> > > 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


Attachments:
(No filename) (1.61 kB)
signature.asc (235.00 B)
Download all attachments