2019-01-22 15:26:36

by Kamil Konieczny

[permalink] [raw]
Subject: [PATCH v1 0/3] add AES support for Exynos5433

Add slimSSS node to DT and crypto AES support for Exynos5433. Tested on
Exynos5433 board with crypto run-time self tests and with tcrypt with
command insmod tcrypt.ko mode=500 sec=1

Kamil Konieczny (3):
arm64: dts: exynos: add SlimSSS for Exynos5433
dt-bindings: crypto: document Exynos5433 SlimSSS
crypto: s5p: add AES support for Exynos5433

.../bindings/crypto/samsung-sss.txt | 13 +++--
arch/arm64/boot/dts/exynos/exynos5433.dtsi | 9 ++++
drivers/crypto/s5p-sss.c | 50 +++++++++++++++++--
3 files changed, 65 insertions(+), 7 deletions(-)

--
2.20.0



2019-01-22 15:26:17

by Kamil Konieczny

[permalink] [raw]
Subject: [PATCH v1 3/3] crypto: s5p: add AES support for Exynos5433

Add AES crypto HW acceleration for Exynos5433, with the help of SlimSSS IP.

Signed-off-by: Kamil Konieczny <[email protected]>
---
drivers/crypto/s5p-sss.c | 50 ++++++++++++++++++++++++++++++++++++----
1 file changed, 46 insertions(+), 4 deletions(-)

diff --git a/drivers/crypto/s5p-sss.c b/drivers/crypto/s5p-sss.c
index 0064be0e3941..1ae570ad4391 100644
--- a/drivers/crypto/s5p-sss.c
+++ b/drivers/crypto/s5p-sss.c
@@ -232,6 +232,7 @@
* struct samsung_aes_variant - platform specific SSS driver data
* @aes_offset: AES register offset from SSS module's base.
* @hash_offset: HASH register offset from SSS module's base.
+ * @clk_names: names of clocks needed to run SSS IP
*
* Specifies platform specific configuration of SSS module.
* Note: A structure for driver specific platform data is used for future
@@ -240,6 +241,7 @@
struct samsung_aes_variant {
unsigned int aes_offset;
unsigned int hash_offset;
+ char *clk_names[];
};

struct s5p_aes_reqctx {
@@ -296,6 +298,7 @@ struct s5p_aes_ctx {
struct s5p_aes_dev {
struct device *dev;
struct clk *clk;
+ struct clk *pclk;
void __iomem *ioaddr;
void __iomem *aes_ioaddr;
int irq_fc;
@@ -384,11 +387,19 @@ struct s5p_hash_ctx {
static const struct samsung_aes_variant s5p_aes_data = {
.aes_offset = 0x4000,
.hash_offset = 0x6000,
+ .clk_names = { "secss", }
};

static const struct samsung_aes_variant exynos_aes_data = {
.aes_offset = 0x200,
.hash_offset = 0x400,
+ .clk_names = { "secss", }
+};
+
+static const struct samsung_aes_variant exynos5433_slim_aes_data = {
+ .aes_offset = 0x400,
+ .hash_offset = 0x800,
+ .clk_names = { "pclk", "aclk", }
};

static const struct of_device_id s5p_sss_dt_match[] = {
@@ -400,6 +411,10 @@ static const struct of_device_id s5p_sss_dt_match[] = {
.compatible = "samsung,exynos4210-secss",
.data = &exynos_aes_data,
},
+ {
+ .compatible = "samsung,exynos5433-slim-sss",
+ .data = &exynos5433_slim_aes_data,
+ },
{ },
};
MODULE_DEVICE_TABLE(of, s5p_sss_dt_match);
@@ -2208,18 +2223,40 @@ static int s5p_aes_probe(struct platform_device *pdev)
return PTR_ERR(pdata->ioaddr);
}

- pdata->clk = devm_clk_get(dev, "secss");
+ pdata->clk = devm_clk_get(dev, variant->clk_names[0]);
if (IS_ERR(pdata->clk)) {
- dev_err(dev, "failed to find secss clock source\n");
+ dev_err(dev, "failed to find secss clock %s\n",
+ variant->clk_names[0]);
return -ENOENT;
}

err = clk_prepare_enable(pdata->clk);
if (err < 0) {
- dev_err(dev, "Enabling SSS clk failed, err %d\n", err);
+ dev_err(dev, "Enabling clock %s failed, err %d\n",
+ variant->clk_names[0], err);
return err;
}

+ if (variant->clk_names[1]) {
+ pdata->pclk = devm_clk_get(dev, variant->clk_names[1]);
+ if (IS_ERR(pdata->pclk)) {
+ dev_err(dev, "failed to find clock %s\n",
+ variant->clk_names[1]);
+ clk_disable_unprepare(pdata->clk);
+ return -ENOENT;
+ }
+
+ err = clk_prepare_enable(pdata->pclk);
+ if (err < 0) {
+ dev_err(dev, "Enabling clock %s failed, err %d\n",
+ variant->clk_names[0], err);
+ clk_disable_unprepare(pdata->clk);
+ return err;
+ }
+ } else {
+ pdata->pclk = NULL;
+ }
+
spin_lock_init(&pdata->lock);
spin_lock_init(&pdata->hash_lock);

@@ -2295,8 +2332,10 @@ static int s5p_aes_probe(struct platform_device *pdev)
tasklet_kill(&pdata->tasklet);

err_irq:
- clk_disable_unprepare(pdata->clk);
+ if (pdata->pclk)
+ clk_disable_unprepare(pdata->pclk);

+ clk_disable_unprepare(pdata->clk);
s5p_dev = NULL;

return err;
@@ -2323,6 +2362,9 @@ static int s5p_aes_remove(struct platform_device *pdev)
pdata->use_hash = false;
}

+ if (pdata->pclk)
+ clk_disable_unprepare(pdata->pclk);
+
clk_disable_unprepare(pdata->clk);
s5p_dev = NULL;

--
2.20.0


2019-01-22 15:26:49

by Kamil Konieczny

[permalink] [raw]
Subject: [PATCH v1 2/3] dt-bindings: crypto: document Exynos5433 SlimSSS

Document DT bindings for crypto Samsung Exynos5433 SlimSSS (Slim Security
SubSystem) IP.

Signed-off-by: Kamil Konieczny <[email protected]>
---
.../devicetree/bindings/crypto/samsung-sss.txt | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/crypto/samsung-sss.txt b/Documentation/devicetree/bindings/crypto/samsung-sss.txt
index 7a5ca56683cc..75718f86fe50 100644
--- a/Documentation/devicetree/bindings/crypto/samsung-sss.txt
+++ b/Documentation/devicetree/bindings/crypto/samsung-sss.txt
@@ -1,4 +1,4 @@
-Samsung SoC SSS (Security SubSystem) module
+Samsung SoC SSS (Security SubSystem) and SlimSSS module

The SSS module in S5PV210 SoC supports the following:
-- Feeder (FeedCtrl)
@@ -15,6 +15,11 @@ supports the following also:
-- True Random Number Generator (TRNG)
-- Secure Key Manager

+The SlimSSS module in Exynos5 (Exynos5433) supports:
+-- Feeder (FeedCtrl)
+-- Advanced Encryption Standard (AES)
+-- SHA-1/SHA-256/HMAC (SHA-1/SHA-256)
+
Required properties:

- compatible : Should contain entries for this and backward compatible
@@ -22,11 +27,13 @@ Required properties:
- "samsung,s5pv210-secss" for S5PV210 SoC.
- "samsung,exynos4210-secss" for Exynos4210, Exynos4212, Exynos4412, Exynos5250,
Exynos5260 and Exynos5420 SoCs.
+ - "samsung,exynos5433-slim-sss" for Exynos542x and Exynos5433 SoCs.
- reg : Offset and length of the register set for the module
- interrupts : interrupt specifiers of SSS module interrupts (one feed
control interrupt).

- clocks : list of clock phandle and specifier pairs for all clocks listed in
clock-names property.
-- clock-names : list of device clock input names; should contain one entry
- "secss".
+- clock-names : list of device clock input names; should contain "pclk" and
+ "aclk" for slim-sss in Exynos5433, and one entry "secss" for
+ other compatibles.
--
2.20.0


2019-01-23 08:00:12

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v1 2/3] dt-bindings: crypto: document Exynos5433 SlimSSS

On Tue, 22 Jan 2019 at 16:26, Kamil Konieczny
<[email protected]> wrote:
>
> Document DT bindings for crypto Samsung Exynos5433 SlimSSS (Slim Security
> SubSystem) IP.
>
> Signed-off-by: Kamil Konieczny <[email protected]>
> ---
> .../devicetree/bindings/crypto/samsung-sss.txt | 13 ++++++++++---
> 1 file changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/crypto/samsung-sss.txt b/Documentation/devicetree/bindings/crypto/samsung-sss.txt
> index 7a5ca56683cc..75718f86fe50 100644
> --- a/Documentation/devicetree/bindings/crypto/samsung-sss.txt
> +++ b/Documentation/devicetree/bindings/crypto/samsung-sss.txt
> @@ -1,4 +1,4 @@
> -Samsung SoC SSS (Security SubSystem) module
> +Samsung SoC SSS (Security SubSystem) and SlimSSS module
>
> The SSS module in S5PV210 SoC supports the following:
> -- Feeder (FeedCtrl)
> @@ -15,6 +15,11 @@ supports the following also:
> -- True Random Number Generator (TRNG)
> -- Secure Key Manager
>
> +The SlimSSS module in Exynos5 (Exynos5433) supports:
> +-- Feeder (FeedCtrl)
> +-- Advanced Encryption Standard (AES)
> +-- SHA-1/SHA-256/HMAC (SHA-1/SHA-256)
> +
> Required properties:
>
> - compatible : Should contain entries for this and backward compatible
> @@ -22,11 +27,13 @@ Required properties:
> - "samsung,s5pv210-secss" for S5PV210 SoC.
> - "samsung,exynos4210-secss" for Exynos4210, Exynos4212, Exynos4412, Exynos5250,
> Exynos5260 and Exynos5420 SoCs.
> + - "samsung,exynos5433-slim-sss" for Exynos542x and Exynos5433 SoCs.

For Exynos5420 there is a driver for regular/full SSS module. It would
be nice to explain what is the difference between them and/or mention
potential conflicts of usage.

The point is that previously we added drivers/bindings for SSS and now
you use the other interface. It might be pretty confusing for users...

Best regards,
Krzysztof

> - reg : Offset and length of the register set for the module
> - interrupts : interrupt specifiers of SSS module interrupts (one feed
> control interrupt).
>
> - clocks : list of clock phandle and specifier pairs for all clocks listed in
> clock-names property.
> -- clock-names : list of device clock input names; should contain one entry
> - "secss".
> +- clock-names : list of device clock input names; should contain "pclk" and
> + "aclk" for slim-sss in Exynos5433, and one entry "secss" for
> + other compatibles.
> --
> 2.20.0
>

2019-01-23 08:13:44

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v1 3/3] crypto: s5p: add AES support for Exynos5433

On Tue, 22 Jan 2019 at 16:26, Kamil Konieczny
<[email protected]> wrote:
>
> Add AES crypto HW acceleration for Exynos5433, with the help of SlimSSS IP.
>
> Signed-off-by: Kamil Konieczny <[email protected]>
> ---
> drivers/crypto/s5p-sss.c | 50 ++++++++++++++++++++++++++++++++++++----
> 1 file changed, 46 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/crypto/s5p-sss.c b/drivers/crypto/s5p-sss.c
> index 0064be0e3941..1ae570ad4391 100644
> --- a/drivers/crypto/s5p-sss.c
> +++ b/drivers/crypto/s5p-sss.c
> @@ -232,6 +232,7 @@
> * struct samsung_aes_variant - platform specific SSS driver data
> * @aes_offset: AES register offset from SSS module's base.
> * @hash_offset: HASH register offset from SSS module's base.
> + * @clk_names: names of clocks needed to run SSS IP
> *
> * Specifies platform specific configuration of SSS module.
> * Note: A structure for driver specific platform data is used for future
> @@ -240,6 +241,7 @@
> struct samsung_aes_variant {
> unsigned int aes_offset;
> unsigned int hash_offset;
> + char *clk_names[];
> };
>
> struct s5p_aes_reqctx {
> @@ -296,6 +298,7 @@ struct s5p_aes_ctx {
> struct s5p_aes_dev {
> struct device *dev;
> struct clk *clk;
> + struct clk *pclk;
> void __iomem *ioaddr;
> void __iomem *aes_ioaddr;
> int irq_fc;
> @@ -384,11 +387,19 @@ struct s5p_hash_ctx {
> static const struct samsung_aes_variant s5p_aes_data = {
> .aes_offset = 0x4000,
> .hash_offset = 0x6000,
> + .clk_names = { "secss", }

Put trailing comma so it can be extended without touching previous
line (just like old code).

> };
>
> static const struct samsung_aes_variant exynos_aes_data = {
> .aes_offset = 0x200,
> .hash_offset = 0x400,
> + .clk_names = { "secss", }

Ditto.

> +};
> +
> +static const struct samsung_aes_variant exynos5433_slim_aes_data = {
> + .aes_offset = 0x400,
> + .hash_offset = 0x800,
> + .clk_names = { "pclk", "aclk", }

Ditto.

> };
>
> static const struct of_device_id s5p_sss_dt_match[] = {
> @@ -400,6 +411,10 @@ static const struct of_device_id s5p_sss_dt_match[] = {
> .compatible = "samsung,exynos4210-secss",
> .data = &exynos_aes_data,
> },
> + {
> + .compatible = "samsung,exynos5433-slim-sss",
> + .data = &exynos5433_slim_aes_data,
> + },
> { },
> };
> MODULE_DEVICE_TABLE(of, s5p_sss_dt_match);
> @@ -2208,18 +2223,40 @@ static int s5p_aes_probe(struct platform_device *pdev)
> return PTR_ERR(pdata->ioaddr);
> }
>
> - pdata->clk = devm_clk_get(dev, "secss");
> + pdata->clk = devm_clk_get(dev, variant->clk_names[0]);
> if (IS_ERR(pdata->clk)) {
> - dev_err(dev, "failed to find secss clock source\n");
> + dev_err(dev, "failed to find secss clock %s\n",
> + variant->clk_names[0]);
> return -ENOENT;
> }
>
> err = clk_prepare_enable(pdata->clk);
> if (err < 0) {
> - dev_err(dev, "Enabling SSS clk failed, err %d\n", err);
> + dev_err(dev, "Enabling clock %s failed, err %d\n",
> + variant->clk_names[0], err);
> return err;
> }
>
> + if (variant->clk_names[1]) {
> + pdata->pclk = devm_clk_get(dev, variant->clk_names[1]);
> + if (IS_ERR(pdata->pclk)) {
> + dev_err(dev, "failed to find clock %s\n",
> + variant->clk_names[1]);
> + clk_disable_unprepare(pdata->clk);
> + return -ENOENT;

goto err_irq (eventually rename the label)?

> + }

Indentation looks wrong here.

> +
> + err = clk_prepare_enable(pdata->pclk);
> + if (err < 0) {
> + dev_err(dev, "Enabling clock %s failed, err %d\n",
> + variant->clk_names[0], err);
> + clk_disable_unprepare(pdata->clk);
> + return err;

goto...

Best regards,
Krzysztof

2019-01-23 14:27:20

by Kamil Konieczny

[permalink] [raw]
Subject: Re: [PATCH v1 3/3] crypto: s5p: add AES support for Exynos5433

Hi Krzysztof,

On 23.01.2019 09:13, Krzysztof Kozlowski wrote:
> On Tue, 22 Jan 2019 at 16:26, Kamil Konieczny
> <[email protected]> wrote:
>>
>> Add AES crypto HW acceleration for Exynos5433, with the help of SlimSSS IP.
>>
>> Signed-off-by: Kamil Konieczny <[email protected]>
>> [...]
>> @@ -384,11 +387,19 @@ struct s5p_hash_ctx {
>> static const struct samsung_aes_variant s5p_aes_data = {
>> .aes_offset = 0x4000,
>> .hash_offset = 0x6000,
>> + .clk_names = { "secss", }
>
> Put trailing comma so it can be extended without touching previous
> line (just like old code).
>

ok, I will do this and two following commas.

>> [...]
>> + if (variant->clk_names[1]) {
>> + pdata->pclk = devm_clk_get(dev, variant->clk_names[1]);
>> + if (IS_ERR(pdata->pclk)) {
>> + dev_err(dev, "failed to find clock %s\n",
>> + variant->clk_names[1]);
>> + clk_disable_unprepare(pdata->clk);
>> + return -ENOENT;
>
> goto err_irq (eventually rename the label)?

I will add 'goto err_clk' with proper cleanup.

>> + }
>
> Indentation looks wrong here.

ok

>> +
>> + err = clk_prepare_enable(pdata->pclk);
>> + if (err < 0) {
>> + dev_err(dev, "Enabling clock %s failed, err %d\n",
>> + variant->clk_names[0], err);
>> + clk_disable_unprepare(pdata->clk);
>> + return err;
>
> goto...

Yes, I will put the same goto here.

--
Best regards,
Kamil Konieczny
Samsung R&D Institute Poland


2019-01-23 15:23:45

by Kamil Konieczny

[permalink] [raw]
Subject: Re: [PATCH v1 2/3] dt-bindings: crypto: document Exynos5433 SlimSSS

Hi Krzysztof,

On 23.01.2019 08:59, Krzysztof Kozlowski wrote:
> On Tue, 22 Jan 2019 at 16:26, Kamil Konieczny
> <[email protected]> wrote:
>>
>> Document DT bindings for crypto Samsung Exynos5433 SlimSSS (Slim Security
>> SubSystem) IP.
>>
>> Signed-off-by: Kamil Konieczny <[email protected]>
>> ---
>> .../devicetree/bindings/crypto/samsung-sss.txt | 13 ++++++++++---
>> 1 file changed, 10 insertions(+), 3 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/crypto/samsung-sss.txt b/Documentation/devicetree/bindings/crypto/samsung-sss.txt
>> index 7a5ca56683cc..75718f86fe50 100644
>> --- a/Documentation/devicetree/bindings/crypto/samsung-sss.txt
>> +++ b/Documentation/devicetree/bindings/crypto/samsung-sss.txt
>> @@ -1,4 +1,4 @@
>> -Samsung SoC SSS (Security SubSystem) module
>> +Samsung SoC SSS (Security SubSystem) and SlimSSS module
>>
>> The SSS module in S5PV210 SoC supports the following:
>> -- Feeder (FeedCtrl)
>> @@ -15,6 +15,11 @@ supports the following also:
>> -- True Random Number Generator (TRNG)
>> -- Secure Key Manager
>>
>> +The SlimSSS module in Exynos5 (Exynos5433) supports:
>> +-- Feeder (FeedCtrl)
>> +-- Advanced Encryption Standard (AES)
>> +-- SHA-1/SHA-256/HMAC (SHA-1/SHA-256)
>> +
>> Required properties:
>>
>> - compatible : Should contain entries for this and backward compatible
>> @@ -22,11 +27,13 @@ Required properties:
>> - "samsung,s5pv210-secss" for S5PV210 SoC.
>> - "samsung,exynos4210-secss" for Exynos4210, Exynos4212, Exynos4412, Exynos5250,
>> Exynos5260 and Exynos5420 SoCs.
>> + - "samsung,exynos5433-slim-sss" for Exynos542x and Exynos5433 SoCs.
>
> For Exynos5420 there is a driver for regular/full SSS module. It would
> be nice to explain what is the difference between them and/or mention
> potential conflicts of usage.

If Exynos has SlimSSS, it has also SSS. SlimSSS is separated from SSS, it is like SSS
limited to only AES and HASH, and HASH got limited in capabilities. In Exynos5420 SSS
HASH has SHA-1,SHA-256,md5 with/without HMAC, but slimSSS has only SHA-1,SHA-256/HMAC.
SlimSSS AES has all modes present in SSS, these are ECB, CBC, CTR, XTS and CMAC.
The same goes for Exynos5433.

There are no conflicts between SlimSSS and SSS.

> The point is that previously we added drivers/bindings for SSS and now
> you use the other interface. It might be pretty confusing for users...

SlimSSS has same AES registers offsets from base as in SSS, so the s5p-sss driver can
reuse code. It is preferred to use SSS but on Exynos5433 I was unable to read its register
(FEEDCTRL) with all clocks enabled, maybe because firmware loaded secureOS (trustzone).

>[...]
>>[...]

--
Best regards,
Kamil Konieczny
Samsung R&D Institute Poland


2019-01-23 16:18:14

by Kamil Konieczny

[permalink] [raw]
Subject: Re: [PATCH v1 2/3] dt-bindings: crypto: document Exynos5433 SlimSSS

On 23.01.2019 08:59, Krzysztof Kozlowski wrote:
> On Tue, 22 Jan 2019 at 16:26, Kamil Konieczny
> <[email protected]> wrote:
>>
>> Document DT bindings for crypto Samsung Exynos5433 SlimSSS (Slim Security
>> SubSystem) IP.
>>
>> Signed-off-by: Kamil Konieczny <[email protected]>
>> ---
>> .../devicetree/bindings/crypto/samsung-sss.txt | 13 ++++++++++---
>> 1 file changed, 10 insertions(+), 3 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/crypto/samsung-sss.txt b/Documentation/devicetree/bindings/crypto/samsung-sss.txt
>> [...]
>> - compatible : Should contain entries for this and backward compatible
>> @@ -22,11 +27,13 @@ Required properties:
>> - "samsung,s5pv210-secss" for S5PV210 SoC.
>> - "samsung,exynos4210-secss" for Exynos4210, Exynos4212, Exynos4412, Exynos5250,
>> Exynos5260 and Exynos5420 SoCs.
>> + - "samsung,exynos5433-slim-sss" for Exynos542x and Exynos5433 SoCs.
>
> For Exynos5420 there is a driver for regular/full SSS module. It would
> be nice to explain what is the difference between them and/or mention
> potential conflicts of usage.
>
> The point is that previously we added drivers/bindings for SSS and now
> you use the other interface. It might be pretty confusing for users...

By the way, Exynos4 (Exynos4210, 4412) SSS do not have ARCFOUR (ARC4), nor
TRNG nor Secure Key Manager.

--
Best regards,
Kamil Konieczny
Samsung R&D Institute Poland