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 <[email protected]> wrote: > > > On Wed, Oct 09, 2024 at 08:32:29PM +0300, Ilias Apalodimas wrote: > > > On Wed, 9 Oct 2024 at 18:32, Raymond Mao <[email protected]> wrote: > > > > > > > > Hi Tom, > > > > > > > > On Wed, 9 Oct 2024 at 10:38, Tom Rini <[email protected]> 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 <[email protected]> > > > >> > Reviewed-by: Ilias Apalodimas <[email protected]> > > > >> > > > >> 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. > > > > 1. Introduce SUPPORT_EMMC_RPMB into the MbedTLS sub makefile. > > > > 2. Make MBEDTLS_LIB_CRYPTO depends on !SUPPORT_EMMC_RPMB. > > > > 1) 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
signature.asc
Description: PGP signature
