[U-Boot] [PATCH 1/4 v7] Exynos: Add hardware accelerated SHA256 and SHA1

[Akshay Saraswat]

Hi Kim,

>On Thu, 21 Mar 2013 03:12:54 -0400
>Akshay Saraswat <akshay.s at samsung.com> wrote:
>
>> >On Mon, 18 Mar 2013 02:06:15 -0400
>> >Akshay Saraswat <akshay.s at 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?
>

What has to be done:
Step-1: Data input (DMA Mode). Configure the DMA unit to transfer N bytes of data.
Step-2: Poll the status register until [6] == 1; OR When MSG_DONE_IRQ is asserted, execute further code.

What is being done right now:
No support for interrupt handling in u-boot, hence, while loop.
To be on the safer side wanted to keep a check for h/w malfunction which must not happen.

We used timeout earlier, but as per the discussion timeout has to be related to frequency.
The PRNG IP is based on the SHA-1 algorithm for generating the random numbers.
Hence, I found it inapt but an alternative to timeout for h/w fault detection.
Other than these two ways there is no way for h/w fault detection in while loop.
Shall I drop the idea of h/w fault detection ?

About s/w fallback:
I think the code calling h/w acceleration for sha should do that, because there may be some
situations when developers may need not to do encryption at all instead of doing it with s/w.
It's just a driver for h/w acceleration, in my opinion, it should do it's job nothing more than that.
If we fallback to s/w, then it might mislead developers in few scenarios.

Please suggest if this is correct or not and also what needs to be done.

Thanks & Regards,
Akshay Saraswat