Hi Tom,
On Sat, 20 Jul 2024 at 13:13, Tom Rini trini@konsulko.com wrote:
On Sat, Jul 20, 2024 at 01:36:02PM +0100, Simon Glass wrote:
Hi Tom,
On Fri, 19 Jul 2024 at 16:25, Tom Rini trini@konsulko.com wrote:
On Fri, Jul 19, 2024 at 04:05:09PM +0100, Simon Glass wrote:
Hi Raymond,
On Thu, 18 Jul 2024 at 17:46, Raymond Mao raymond.mao@linaro.org
wrote:
Hi Simon,
On Fri, 5 Jul 2024 at 04:36, Simon Glass sjg@chromium.org wrote:
Hi,
On Wed, Jul 3, 2024, 09:56 Ilias Apalodimas <
ilias.apalodimas@linaro.org> wrote:
> > Hi Raymond > > On Tue, 2 Jul 2024 at 21:27, Raymond Mao <
raymond.mao@linaro.org> wrote:
> > > > Integrate common/hash.c on the hash shim layer so that hash
APIs
> > from mbedtls can be leveraged by boot/image and efi_loader. > > > > Signed-off-by: Raymond Mao raymond.mao@linaro.org > > --- > > Changes in v2 > > - Use the original head files instead of creating new ones. > > Changes in v3 > > - Add handle checkers for malloc. > > Changes in v4 > > - None. > > > > common/hash.c | 143
++++++++++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 143 insertions(+) > > > > diff --git a/common/hash.c b/common/hash.c > > index ac63803fed9..96caf074374 100644 > > --- a/common/hash.c > > +++ b/common/hash.c > > @@ -35,6 +35,141 @@ > > #include <u-boot/sha512.h> > > #include <u-boot/md5.h> > > > > +#if CONFIG_IS_ENABLED(MBEDTLS_LIB_CRYPTO) > > + > > +static int hash_init_sha1(struct hash_algo *algo, void
**ctxp)
> > +{ > > + int ret; > > + mbedtls_sha1_context *ctx =
malloc(sizeof(mbedtls_sha1_context));
Why do we need allocation here? We should avoid it where possible.
The API "hash_init_sha1(struct hash_algo *algo, void **ctxp)" is
passing a pointer
address and expecting to get the context from the pointer, it is
reasonable to do the
allocation. On top of that, this patch doesn't make changes on this API
itself, but just adapted
it to MbedTLS stacks, thus you can see the allocation is needed by
the original API
as well.
Oh dear., I see Now I am looking at the code. It is full of #ifdefs for different cases.
The whole thing needs a bit of a rationalisation before adding
another case.
If you're referring too the hash_algo struct, I'm not sure we can do something different that doesn't in turn increase size globally. And long term some of this may be able to go away if we can remove non-mbedTLS options.
Well, at least using the existing functions rather than writing entirely new ones would help.
The real culprit here is the hardware-acceleration stuff, but making the software side messy too is not nice. Hashing should move to a linker-list approach for the implementation, and probably driver model for the hardware acceleration.
I'm fine with looking at that, after mbedtls is merged and we're looking at what can be removed / cleaned up. Perhaps we'll be able to shift most/all of the hardware assisted algorithms to being handled within mbedtls, I don't know.
Yes, I think to handle this within mbedtls is straight forward.
Regards, Raymond