Re: [PATCH v8 03/27] lib: Adapt digest header files to MbedTLS

11 Oct 2024

On Fri, Oct 11, 2024 at 02:25:20PM -0400, Raymond Mao wrote:
...
Hi Tom,
On Wed, 9 Oct 2024 at 13:52, Tom Rini trini@konsulko.com wrote:
...
On Wed, Oct 09, 2024 at 08:32:29PM +0300, Ilias Apalodimas wrote:
...
On Wed, 9 Oct 2024 at 18:32, Raymond Mao raymond.mao@linaro.org wrote:
...
Hi Tom,
On Wed, 9 Oct 2024 at 10:38, Tom Rini trini@konsulko.com wrote:
...
On Thu, Oct 03, 2024 at 02:50:16PM -0700, Raymond Mao wrote:
...
Adapt digest header files to support both original libs and MbedTLS
by switching on/off MBEDTLS_LIB_CRYPTO.
Introduce <alg>_LEGACY kconfig for legacy hash implementations.
sha256.o should depend on SHA256 kconfig only but not
SUPPORT_EMMC_RPMB,
...
...
...
...
SHA256 should be selected when SUPPORT_EMMC_RPMB is enabled instead.
`IS_ENABLED` or `CONFIG_IS_ENABLED` is not applicable here, since
including <linux/kconfig.h> causes undefined reference on schedule()
with sandbox build, as <linux/kconfig.h> includes
<generated/autoconf.h>
...
...
...
...
which enables `CONFIG_HW_WATCHDOG` and `CONFIG_WATCHDOG` but no
schedule()
...
...
...
...
are defined in sandbox build,
Thus we use `#if defined(CONFIG_MBEDTLS_LIB_CRYPTO)` instead.
Signed-off-by: Raymond Mao raymond.mao@linaro.org
Reviewed-by: Ilias Apalodimas ilias.apalodimas@linaro.org
There's three platforms where we see something like:
       arm: (for 1/1 boards) all +5651.0 data +112.0 rodata +139.0
text +5400.0
...
...
...
        o4-imx6ull-nano: all +5651 data +112 rodata +139 text

+5400
...
...
...
           u-boot: add: 23/0, grow: 1/0 bytes: 1172/0 (1172)
             function                                   old

new   delta
...
...
...
             hash_command                               108

296    +188
...
...
...
             sha1_finish                                  -

156    +156
...
...
...
             static.sha1_update                           -

114    +114
...
...
...
             hash_algo                                    -

112    +112
...
...
...
             sha1_padding                                 -

64     +64
...
...
...
             hash_lookup_algo                             -

60     +60
...
...
...
             sha1_starts                                  -

52     +52
...
...
...
             crc16_ccitt_wd_buf                           -

36     +36
...
...
...
             sha256_csum_wd                               -

34     +34
...
...
...
             sha1_csum_wd                                 -

34     +34
...
...
...
             hash_finish_sha256                           -

34     +34
...
...
...
             hash_finish_sha1                             -

34     +34
...
...
...
             crc32_wd_buf                                 -

34     +34
...
...
...
             hash_finish_crc32                            -

28     +28
...
...
...
             hash_finish_crc16_ccitt                      -

28     +28
...
...
...
             hash_init_sha256                             -

22     +22
...
...
...
             hash_init_sha1                               -

22     +22
...
...
...
             hash_update_crc32                            -

20     +20
...
...
...
             hash_update_crc16_ccitt                      -

20     +20
...
...
...
             hash_init_crc32                              -

20     +20
...
...
...
             hash_init_crc16_ccitt                        -

20     +20
...
...
...
             hash_update_sha256                           -

16     +16
...
...
...
             hash_update_sha1                             -

16     +16
...
...
...
             sha1_update                                  -

8      +8
...
...
...
This is because:
...

diff --git a/drivers/mmc/Kconfig b/drivers/mmc/Kconfig
index 982e84dc3bc..5d7fd904950 100644
--- a/drivers/mmc/Kconfig
+++ b/drivers/mmc/Kconfig
@@ -119,6 +119,7 @@ config MMC_HW_PARTITIONING
 config SUPPORT_EMMC_RPMB
      bool "Support eMMC replay protected memory block (RPMB)"
      imply CMD_MMC_RPMB

select SHA256
help
  Enable support for reading, writing and programming the
  key for the Replay Protection Memory Block partition in



eMMC.
...
...
...
Wasn't true / required before now, no hashing algorithms were enabled.
This was fine because:
[snip]
...
diff --git a/lib/Makefile b/lib/Makefile
index c4950b78a29..33755778283 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -50,7 +50,6 @@ obj-$(CONFIG_XXHASH) += xxhash.o
 obj-y += net_utils.o
 obj-$(CONFIG_PHYSMEM) += physmem.o
 obj-y += rc4.o
-obj-$(CONFIG_SUPPORT_EMMC_RPMB) += sha256.o
 obj-$(CONFIG_RBTREE) += rbtree.o
 obj-$(CONFIG_BITREVERSE) += bitrev.o
 obj-y += list_sort.o
Got us the library access without bringing in everything else. And
since
...
...
...
two of the platforms that are hitting this now are "nano" this is an
important thing to figure out how to continue to support. If there's
just no way around it, we can likely live with the size increase, but
I'd like to see this looked in to specifically first, thanks!
To address this, I think there are two options.

Introduce SUPPORT_EMMC_RPMB into the MbedTLS sub makefile.
Make MBEDTLS_LIB_CRYPTO depends on !SUPPORT_EMMC_RPMB.


looks to be ugly, I prefer 2) if you agree.

2 is not a good idea either. We can't just drop RPMB support when
mbedTLS is enabled
To be clear, while I hope we can do something about this growth, I would
rather live with it (as it's not an unreasonable amount) than do 2, and
if 1 is too ugly, probably skip that as well. If it's not a matter of
loosening some select statements, or maybe introducing a library type
symbol we can see if anyone else more motivated has a better idea as
it's literally 3 platforms (ev-imx280-nano-x-mb is the other nano, and
then uniphier_v8 where it's arguably a missing feature anyhow) rather
than a large number of them. And not even other "mini" or "nano"
configs.
Yes. I figured out what the problem is.
The "select SHA256" change doesn't matter, but size growth was introduced by
the inline function in sha1_alt.h I added in patch #2 of v8.
I already fixed this in my working branch:
aarch64: (for 1/1 boards) all +8502.0 data +224.0 rodata +218.0 text
+8060.0
            uniphier_v8    : all +8502 data +224 rodata +218 text +8060
       arm: (for 2/2 boards) all +5649.0 data +112.0 rodata +137.0 text
+5400.0
            o4-imx6ull-nano: all +5651 data +112 rodata +139 text +5400
            ev-imx280-nano-x-mb: all +5647 data +112 rodata +135 text +5400
Now there is no "u-boot: add" any more on these three boards.
I will update v9 with this fix, and we don't need either 1) or 2) options
mentioned
in my previous reply.
Erm, it looks like you just don't have the flag passed to show the
functions that changed? That's pretty close to the text change I saw.
For reference I do:
-----8>
#!/bin/bash
# Initial and constant buildman args
ARGS="-devl -PEWM"
ALL=0
KEEP=0
# Find our arguments
while test $# -ne 0; do
    if [ "$1" == "--all" ]; then
    	ALL=1
    	shift 1
    elif [ "$1" == "--branch" ]; then
    	BRANCH=$2
    	shift 2
    elif [ "$1" == "--keep" ]; then
    	KEEP=1
    	ARGS="$ARGS -k"
    	shift 1
    elif [ "$1" == "--board" ]; then
    	MACHINE="--board $2"
    	OUTDIR=/tmp/$2
    	shift 2
    else
    	MACHINE=$1
    	shift 1
    fi
done
OUTDIR=${OUTDIR:-/tmp/$MACHINE}
if [ -z "$MACHINE" ]; then
    echo Usage: $0 MACHINE [--all] [--keep] [--branch BRANCH]
    exit 1
fi
# If not all, then only first/last
if [ $ALL -ne 1 ]; then
    ARGS="$ARGS --step 0"
fi
if [ ! -z $BRANCH ]; then
    ARGS="$ARGS -b $BRANCH"
else
    ARGS="$ARGS -b `git rev-parse --abbrev-ref HEAD`"
fi
mkdir -p ${OUTDIR}
export SOURCE_DATE_EPOCH=`date +%s`
./tools/buildman/buildman -o ${OUTDIR} $ARGS -SBC $MACHINE
./tools/buildman/buildman -o ${OUTDIR} $ARGS -SsB $MACHINE
[ $KEEP -eq 0 ] && rm -rf ${OUTDIR}
<----8
-- 
Tom

    