[PATCH v2 4/5] lib: sha256: Add support for hardware specific sha256_process

[Loic Poulain]

Hi Simon,

On Tue, 7 Feb 2023 at 05:05, Simon Glass <sjg at chromium.org> wrote:
>
> Hi Loic,
>
> On Mon, 6 Feb 2023 at 15:12, Loic Poulain <loic.poulain at linaro.org> wrote:
> >
> > Hi Simon,
> >
> > Le lun. 6 févr. 2023 à 18:12, Simon Glass <sjg at chromium.org> a écrit :
> >>
> >> Hi Loic,
> >>
> >> On Wed, 1 Jun 2022 at 12:27, Loic Poulain <loic.poulain at linaro.org> wrote:
> >> >
> >> > Mark sha256_process as weak to allow hardware specific implementation.
> >> > Add parameter for supporting multiple blocks processing.
> >> >
> >> > Signed-off-by: Loic Poulain <loic.poulain at linaro.org>
> >> > ---
> >> >  lib/sha256.c | 26 +++++++++++++++++++-------
> >> >  1 file changed, 19 insertions(+), 7 deletions(-)
> >> >
[...]
> >> > +__weak void sha256_process(sha256_context *ctx, const unsigned char *data,
> >> > +                          unsigned int blocks)
> >> > +{
> >> > +       if (!blocks)
> >> > +               return;
> >> > +
> >> > +       while (blocks--) {
> >> > +               sha256_process_one(ctx, data);
> >> > +               data += 64;
> >> > +       }
> >> > +}
> >> > +
> >> >  void sha256_update(sha256_context *ctx, const uint8_t *input, uint32_t length)
> >> >  {
> >> >         uint32_t left, fill;
> >> > @@ -204,17 +218,15 @@ void sha256_update(sha256_context *ctx, const uint8_t *input, uint32_t length)
> >> >
> >> >         if (left && length >= fill) {
> >> >                 memcpy((void *) (ctx->buffer + left), (void *) input, fill);
> >> > -               sha256_process(ctx, ctx->buffer);
> >> > +               sha256_process(ctx, ctx->buffer, 1);
> >> >                 length -= fill;
> >> >                 input += fill;
> >> >                 left = 0;
> >> >         }
> >> >
> >> > -       while (length >= 64) {
> >> > -               sha256_process(ctx, input);
> >> > -               length -= 64;
> >> > -               input += 64;
> >> > -       }
> >> > +       sha256_process(ctx, input, length / 64);
> >> > +       input += length / 64 * 64;
> >> > +       length = length % 64;
> >> >
> >> >         if (length)
> >> >                 memcpy((void *) (ctx->buffer + left), (void *) input, length);
> >> > --
> >> > 2.7.4
> >> >
> >>
> >> I just came across this patch as it broke minnowmax.
> >
> >
> > Ok, is it a build time or runtime break?
>
> Build, but you need the binary blobs to see it :-(
> >>
> >> This should be using driver model, not weak functions. Please can you
> >> take a look?

Just tested the minnowmax build (b69026c91f2e; minnowmax_defconfig;
gcc-11.3.0), and I've not observed any issue (but I had to fake some
of the binary blobs...). Could you share the build problem/error you
encountered? As you mentioned it, Is the error specifically related to
_weak function linking? Would like to have a simple and quick fix
before trying to move on to a more proper DM based solution.

Thanks,
Loic


> >
> >
> > Yes I can look at it in the next few days. I have used weak function because it’s an architecture feature offered by armv8 instructions, It’s not strictly speaking an internal device/IP.
>
> Thanks.
>
> Right, same as hardware-accelerated hashing hardware in my book.
>
> See hash.c which has become a mess. We have been trying to make do
> with a list of algos, but given all the updates I think needs a new
> UCLASS_HASH with the same operations as in hash.h
>
> Regards,
> Simon