[PATCH 3/3] hash: Allow for SHA512 hardware implementations

[Heinrich Schuchardt]

On 5/13/21 4:36 PM, Simon Glass wrote:
> Hi Heinrich,
>
> On Wed, 12 May 2021 at 15:27, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
>>
>> Am 12. Mai 2021 19:32:42 MESZ schrieb Simon Glass <sjg at chromium.org>:
>>> Hi Heinrich,
>>>
>>> On Wed, 12 May 2021 at 10:25, Heinrich Schuchardt <xypron.glpk at gmx.de>
>>> wrote:
>>>>
>>>> On 12.05.21 18:05, Simon Glass wrote:
>>>>> Hi Heinrich,
>>>>>
>>>>> On Wed, 12 May 2021 at 10:01, Heinrich Schuchardt
>>> <xypron.glpk at gmx.de> wrote:
>>>>>>
>>>>>> On 17.02.21 04:20, Joel Stanley wrote:
>>>>>>> Similar to support for SHA1 and SHA256, allow the use of hardware
>>> hashing
>>>>>>> engine by enabling the algorithm and setting  CONFIG_SHA_HW_ACCEL
>>> /
>>>>>>> CONFIG_SHA_PROG_HW_ACCEL.
>>>>>>>
>>>>>>> Signed-off-by: Joel Stanley <joel at jms.id.au>
>>>>>>
>>>>>> This merged patch leads to errors compiling the EFI TCG2 protocol
>>> on
>>>>>> boards with CONFIG_SHA_HW_ACCEL.
>>>>>>
>>>>>> There is not as single implementation of hw_sha384 and hw_sha512.
>>> You
>>>>>> could only use CONFIG_SHA_HW_ACCEL for selecting these functions
>>> if
>>>>>> these were implemented for *all* boards with
>>> CONFIG_SHA_HW_ACCEL=y. But
>>>>>> this will never happen.
>>>>>>
>>>>>> *This patch needs to be reverted.*
>>>>>>
>>>>>> Why do we have CONFIG_SHA_HW_ACCEL at all and don't use weak
>>> functions
>>>>>> instead?
>>>>>
>>>>> This is all a mess. We should not use weak functions IMO, but
>>> instead
>>>>> have a driver interface, like we do with filesystems.
>>>>>
>>>>> Part of the challenge is the need to keep code size small for
>>>>> platforms that only need one algorithm.
>>>>
>>>> If a function is not referenced, the linker will eliminate it. But
>>> with
>>>> a driver interface every function in the interface is referenced.
>>>
>>> Indeed, but we can still have an option for enabling the progressive
>>> interface. My point is that the implementation (software or hardware)
>>> should use a driver.
>>>
>>> Regards,
>>> Simon
>>
>> Anyway that should not stop us from reverting a problematic patch. We can work on refactoring afterwards.
>
> Well the patch was applied because tests passed. We should be careful
> about reverting a patch due to problems it causes in the future.

The code compiled because SHA384 and SHA512 were not used together with
CONFIG_SHA_HW_ACCEL=y. But we need these functions to implement the TCG2
protocol on boards with TPMv2.

Gitlab CI had no issues with reverting the patch.

Best regards

Heinrich