Re: [U-Boot] [PATCH 1/4 v7] Exynos: Add hardware accelerated SHA256 and SHA1
Hi Kim,
On Mon, 18 Mar 2013 02:06:15 -0400 Akshay Saraswat akshay.s@samsung.com wrote:
SHA-256 and SHA-1 accelerated using ACE hardware.
Signed-off-by: ARUN MANKUZHI arun.m@samsung.com Signed-off-by: Akshay Saraswat akshay.s@samsung.com Acked-by: Simon Glass sjg@chromium.org
+++ b/arch/arm/include/asm/arch-exynos/ace_sha.h
ace_sha.h belongs in drivers/crypto/
+#define ACE_FC_SELBC_AES (0 << 2) /* AES */ +#define ACE_FC_SELBC_DES (1 << 2) /* DES */
nit: comments seem a bit redundant
Few comments look same but they differ.
For example: "Feed control - BTDMA control" and "Feed control - BRDMA control"
+#define ACE_FC_BRDMACARPROT_OFS (2) +#define ACE_FC_BRDMACARCACHE_OFS (5) +#define ACE_FC_BTDMACAWPROT_OFS (2) +#define ACE_FC_BTDMACAWCACHE_OFS (5) +#define ACE_FC_HRDMACARPROT_OFS (2) +#define ACE_FC_HRDMACARCACHE_OFS (5) +#define ACE_FC_SRAMOFFSET_MASK (0xfff)
no parens.
+/**
- Computes hash value of input pbuf using ACE
- @param in_addr A pointer to the input buffer
- @param bufleni Byte length of input buffer
- @param out_addr A pointer to the output buffer. When complete
32 bytes are copied to pout[0]...pout[31]. Thus, a user
should allocate at least 32 bytes at pOut in advance.
- @param hash_type SHA1 or SHA256
- @return 0 on Success, -1 on Failure (Timeout)
...
- /* Check if status changes within given time limit */
leftovers from timeout implementation?
- while ((readl(&ace_sha_reg->hash_status) & ACE_HASH_MSGDONE_MASK) ==
ACE_HASH_MSGDONE_OFF) {/** PRNG error bit goes HIGH if a PRNG request occurs without* a complete seed setup. We are using this bit to check h/w* because proper setup is not expected in that case.*/if ((readl(&ace_sha_reg->hash_status)& ACE_HASH_PRNGERROR_MASK) == ACE_HASH_PRNGERROR_ON)break;- }
so we have:
whilst (not_done) if (PRNGERROR_ON) break;
...and then success-assuming code flow continues after this. What value is this check for PRNGERROR_ON, if the code isn't going to do anything differently? And what does the status of the RNG have to do with keyless hashing? It sounds like a check for proper initialization (RNG got seeded) is in order here, but only if it's required for the given hash operations (I doubt it, but the h/w might be fussy).
Kim
PRNG ERROR means setup was not proper which should be the case when h/w is faulty because driver does everything in order. Earlier break was happening which was not correct so changing it to "return -EBUSY" which means code flow should not be the same as the one being successfull.
Regards, Akshay Saraswat
On Thu, 21 Mar 2013 03:12:54 -0400 Akshay Saraswat akshay.s@samsung.com wrote:
On Mon, 18 Mar 2013 02:06:15 -0400 Akshay Saraswat akshay.s@samsung.com wrote:
- while ((readl(&ace_sha_reg->hash_status) & ACE_HASH_MSGDONE_MASK) ==
ACE_HASH_MSGDONE_OFF) {/** PRNG error bit goes HIGH if a PRNG request occurs without* a complete seed setup. We are using this bit to check h/w* because proper setup is not expected in that case.*/if ((readl(&ace_sha_reg->hash_status)& ACE_HASH_PRNGERROR_MASK) == ACE_HASH_PRNGERROR_ON)break;- }
so we have:
whilst (not_done) if (PRNGERROR_ON) break;
...and then success-assuming code flow continues after this. What value is this check for PRNGERROR_ON, if the code isn't going to do anything differently? And what does the status of the RNG have to do with keyless hashing? It sounds like a check for proper initialization (RNG got seeded) is in order here, but only if it's required for the given hash operations (I doubt it, but the h/w might be fussy).
PRNG ERROR means setup was not proper which should be the case when h/w is faulty because driver does everything in order.
The comment above says:
"PRNG error bit goes HIGH if a PRNG request occurs without a complete seed setup"
But a SHA request isn't a PRNG request. The h/w shouldn't need the PRNG at all to perform a simple SHA.
If this is a general initialization/"setup" check, then do it then. If not, can you do it immediately prior to the wait-for-done after the "setup"? A PRNG error shouldn't be going off in the middle of the wait-for-done loop for a SHA.
Earlier break was happening which was not correct so changing it to "return -EBUSY" which means code flow should not be the same as the one being successfull.
-EBUSY? that means the h/w was busy, and carries a "try again" implication (much like -EAGAIN) - I doubt that's what's intended here.
What happened to reverting to s/w, btw?
Kim
participants (2)
-
Akshay Saraswat -
Kim Phillips