Hi Tom
On Wed, 9 Oct 2024 at 20: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.
Yep, I agree. That was pretty clear from your first email as well. I just pointed out that option 2 was just as bad
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.
-- Tom
Cheers /Ilias