Patchwork [U-Boot,3/4,v3] gen: Add ACE acceleration to hash

login
register
mail settings
Submitter Akshay Saraswat
Date March 1, 2013, 4:16 p.m.
Message ID <1362154585-16216-4-git-send-email-akshay.s@samsung.com>
Download mbox | patch
Permalink /patch/224402/
State Changes Requested
Delegated to: Minkyu Kang
Headers show

Comments

Akshay Saraswat - March 1, 2013, 4:16 p.m.
ACE H/W acceleration support is added to hash
which can be used to test SHA 256 hash algorithm.

Tested with command "hash sha256 0x40008000 0x2B 0x40009000".
Used mm and md to write a standard string to memory location
0x40008000 and ran the above command to verify the output.

Change-Id: If76820057763e833a6150e55bffea3d010b86721
Signed-off-by: ARUN MANKUZHI <arun.m@samsung.com>
Signed-off-by: Akshay Saraswat <akshay.s@samsung.com>
---
Changes sice v2:
	- Added new nodes for SHA1 and SHA256 in struct hash_algo for the case when ACE is enabled.
	- Added new declaration for function pointer hash_func_ws with different return type.

 common/hash.c  | 15 +++++++++++++++
 include/hash.h |  5 +++++
 2 files changed, 20 insertions(+)
Simon Glass - March 2, 2013, 6:05 a.m.
Hi Akshay,

On Fri, Mar 1, 2013 at 8:16 AM, Akshay Saraswat <akshay.s@samsung.com> wrote:
> ACE H/W acceleration support is added to hash
> which can be used to test SHA 256 hash algorithm.
>
> Tested with command "hash sha256 0x40008000 0x2B 0x40009000".
> Used mm and md to write a standard string to memory location
> 0x40008000 and ran the above command to verify the output.
>
> Change-Id: If76820057763e833a6150e55bffea3d010b86721
> Signed-off-by: ARUN MANKUZHI <arun.m@samsung.com>
> Signed-off-by: Akshay Saraswat <akshay.s@samsung.com>
> ---
> Changes sice v2:
>         - Added new nodes for SHA1 and SHA256 in struct hash_algo for the case when ACE is enabled.
>         - Added new declaration for function pointer hash_func_ws with different return type.
>
>  common/hash.c  | 15 +++++++++++++++
>  include/hash.h |  5 +++++
>  2 files changed, 20 insertions(+)
>
> diff --git a/common/hash.c b/common/hash.c
> index e3a6e43..b8f9d29 100644
> --- a/common/hash.c
> +++ b/common/hash.c
> @@ -28,12 +28,26 @@
>  #include <hash.h>
>  #include <sha1.h>
>  #include <sha256.h>
> +#include <ace_sha.h>
>
>  /*
>   * These are the hash algorithms we support. Chips which support accelerated
>   * crypto could perhaps add named version of these algorithms here.
>   */
>  static struct hash_algo hash_algo[] = {
> +#ifdef CONFIG_EXYNOS_ACE_SHA
> +       {
> +               "SHA1",

Please use lower case here, since there is a pending patch that
changes all these to lower case

> +               SHA1_SUM_LEN,
> +               ace_sha_hash_digest,
> +               ACE_SHA_TYPE_SHA1,
> +       }, {
> +               "SHA256",

lower case

> +               SHA256_SUM_LEN,
> +               ace_sha_hash_digest,
> +               ACE_SHA_TYPE_SHA256,
> +       },
> +#else

I don't think you need the #else. It is OK to have multiple SHA
implements - the hash module will just use the first.

>  #ifdef CONFIG_SHA1
>         {
>                 "SHA1",
> @@ -50,6 +64,7 @@ static struct hash_algo hash_algo[] = {
>                 CHUNKSZ_SHA256,
>         },
>  #endif
> +#endif
>  };
>
>  /**
> diff --git a/include/hash.h b/include/hash.h
> index 34ba558..83e1fb6 100644
> --- a/include/hash.h
> +++ b/include/hash.h
> @@ -40,8 +40,13 @@ struct hash_algo {
>          * @output:     Checksum result (length depends on algorithm)
>          * @chunk_sz:   Trigger watchdog after processing this many bytes
>          */
> +#ifdef CONFIG_EXYNOS_ACE_SHA
> +       int (*hash_func_ws)(const unsigned char *input, unsigned int ilen,
> +               unsigned char *output, unsigned int chunk_sz);

Hmmm you can't do that :-) Let's change hash_func_ws to return an
integer. You should create a separate patch to change over existing
functions (sha1, sha256) to return 0. Then this patch doesn't need the
#ifdef.

> +#else
>         void (*hash_func_ws)(const unsigned char *input, unsigned int ilen,
>                 unsigned char *output, unsigned int chunk_sz);
> +#endif
>         int chunk_size;                         /* Watchdog chunk size */
>  };
>
> --
> 1.8.0
>

Regards,
Simon
Kim Phillips - March 4, 2013, 10:16 p.m.
On Fri, 1 Mar 2013 11:16:24 -0500
Akshay Saraswat <akshay.s@samsung.com> wrote:

> +#include <ace_sha.h>
>  
>  /*
>   * These are the hash algorithms we support. Chips which support accelerated
>   * crypto could perhaps add named version of these algorithms here.
>   */
>  static struct hash_algo hash_algo[] = {
> +#ifdef CONFIG_EXYNOS_ACE_SHA
> +	{
> +		"SHA1",
> +		SHA1_SUM_LEN,
> +		ace_sha_hash_digest,
> +		ACE_SHA_TYPE_SHA1,
> +	}, {
> +		"SHA256",
> +		SHA256_SUM_LEN,
> +		ace_sha_hash_digest,
> +		ACE_SHA_TYPE_SHA256,
> +	},

Can we make this a generic "hardware accelerated SHA", not tied to
vendor-SoC-specific defines?  I don't see more than one h/w
implementation being used on any one instance of u-boot...

> +#ifdef CONFIG_EXYNOS_ACE_SHA
> +	int (*hash_func_ws)(const unsigned char *input, unsigned int ilen,
> +		unsigned char *output, unsigned int chunk_sz);
> +#else
>  	void (*hash_func_ws)(const unsigned char *input, unsigned int ilen,
>  		unsigned char *output, unsigned int chunk_sz);
> +#endif

function signature mismatch, but I see Simon already got this.

Kim

Patch

diff --git a/common/hash.c b/common/hash.c
index e3a6e43..b8f9d29 100644
--- a/common/hash.c
+++ b/common/hash.c
@@ -28,12 +28,26 @@ 
 #include <hash.h>
 #include <sha1.h>
 #include <sha256.h>
+#include <ace_sha.h>
 
 /*
  * These are the hash algorithms we support. Chips which support accelerated
  * crypto could perhaps add named version of these algorithms here.
  */
 static struct hash_algo hash_algo[] = {
+#ifdef CONFIG_EXYNOS_ACE_SHA
+	{
+		"SHA1",
+		SHA1_SUM_LEN,
+		ace_sha_hash_digest,
+		ACE_SHA_TYPE_SHA1,
+	}, {
+		"SHA256",
+		SHA256_SUM_LEN,
+		ace_sha_hash_digest,
+		ACE_SHA_TYPE_SHA256,
+	},
+#else
 #ifdef CONFIG_SHA1
 	{
 		"SHA1",
@@ -50,6 +64,7 @@  static struct hash_algo hash_algo[] = {
 		CHUNKSZ_SHA256,
 	},
 #endif
+#endif
 };
 
 /**
diff --git a/include/hash.h b/include/hash.h
index 34ba558..83e1fb6 100644
--- a/include/hash.h
+++ b/include/hash.h
@@ -40,8 +40,13 @@  struct hash_algo {
 	 * @output:	Checksum result (length depends on algorithm)
 	 * @chunk_sz:	Trigger watchdog after processing this many bytes
 	 */
+#ifdef CONFIG_EXYNOS_ACE_SHA
+	int (*hash_func_ws)(const unsigned char *input, unsigned int ilen,
+		unsigned char *output, unsigned int chunk_sz);
+#else
 	void (*hash_func_ws)(const unsigned char *input, unsigned int ilen,
 		unsigned char *output, unsigned int chunk_sz);
+#endif
 	int chunk_size;				/* Watchdog chunk size */
 };