Hi Tom, On Tue, 16 Apr 2024 at 19:50, Tom Rini <tr...@konsulko.com> wrote:
> On Tue, Apr 16, 2024 at 07:47:44PM -0400, Raymond Mao wrote: > > Hi Tom, > > > > On Tue, 16 Apr 2024 at 19:12, Tom Rini <tr...@konsulko.com> wrote: > > > > > On Tue, Apr 16, 2024 at 11:59:56AM -0700, Raymond Mao wrote: > > > > > > [snip] > > > > [1]: bloat-o-meter output between disabling/enabling MbedTLS > > > > ``` > > > > add/remove: 231/69 grow/shrink: 12/5 up/down: 60196/-11166 (49030) > > > > > > I don't know if this is qemu_arm64 or sandbox. With buildman's size > > > comparison functions we see: > > > qemu_arm64 : all +75537 bss -88 data +24 rodata +6349 > text > > > +69252 > > > u-boot: add: 274/-12, grow: 12/-4 bytes: 72002/-3800 > (68202) > > > ... > > > sandbox : all +57008 bss +32 data +1632 rodata +352 > > > text +54992 > > > u-boot: add: 143/-75, grow: 21/-18 bytes: 64058/-16523 > > > (47535) > > > > > > So please look in to using buildman to get more details about what's > > > changing, size-wise. Also, my goodness, that's far too much growth, and > > > we aren't removing anything? This needs to be a whole switch, not just > > > an addition. And then look in to what we can tweak / remove. Are we > > > perhaps not getting our usual link time garbage collection done? > > > > > As stated in the cover letter, with this patch set, we still build the > > original libs > > (lib/rsa, lib/asn1_decoder.c, lib/crypto/rsa_helper.c, lib/md5.c, > > lib/sha1.c, > > lib/sha256.c, lib/sha512.c) for the components outside of EFI loader. > > Eventually all these will be completely switched and removed. > > But I think we should do this in a sparated patch set - It is too big for > > one > > patch set. > > So, as the first patch set, this one will introduce MbedTLS and enable it > > with > > EFI loader, after they are merged, the next patch set will switch other > > components > > to use MbedTLS and remove the original libs. > > What are your thoughts? > > My thoughts are that we need to implement this as the optional > replacement for the existing libraries. We already have abstractions for > most if not all of the algorithms I believe. And we need to be able to > see how much size growth we get from this change because long term we > don't really want to have two sets of libraries for this functionality > (as a SW rather than HW implementation). > > I see. With this version most of the lib/crypto that are used by EFI loader (pkcs7, x509, public_key, mscode_parser, etc) are sharing the same abstractions except the ones for hash. I can replace them in v2, so that we can compare the final growth by switching on/off the MbedTLS build options. Thanks and regards, Raymond