Hi Tom, On Wed, 29 May 2024 at 14:01, Tom Rini <tr...@konsulko.com> wrote:
> On Wed, May 29, 2024 at 01:42:16PM -0400, Raymond Mao wrote: > > Hi Tom, > > > > On Wed, 29 May 2024 at 12:58, Tom Rini <tr...@konsulko.com> wrote: > > > > > On Tue, May 28, 2024 at 07:09:14AM -0700, Raymond Mao wrote: > > > > > > > Port mbedtls with dummy libc header files. > > > > Add mbedtls default config header file. > > > > Optimize mbedtls default config by disabling unused features to > > > > reduce the target size. > > > > Add mbedtls kbuild makefile. > > > > Add Kconfig and mbedtls config submenu. > > > [snip] > > > > diff --git a/lib/mbedtls/Kconfig b/lib/mbedtls/Kconfig > > > > new file mode 100644 > > > > index 00000000000..d6e77d56871 > > > > --- /dev/null > > > > +++ b/lib/mbedtls/Kconfig > > > > @@ -0,0 +1,25 @@ > > > > +menuconfig MBEDTLS_LIB > > > > + bool "Use mbedtls libraries" > > > > + select MBEDTLS_LIB_CRYPTO > > > > + select MBEDTLS_LIB_X509 > > > > + help > > > > + Enable mbedtls libraries > > > > + > > > > +if MBEDTLS_LIB > > > > + > > > > +config MBEDTLS_LIB_CRYPTO > > > > + bool "Crypto library" > > > > + help > > > > + Enable mbedtls crypto library > > > > + > > > > +config MBEDTLS_LIB_X509 > > > > + bool "X509 library" > > > > + help > > > > + Enable mbedtls X509 library > > > > + > > > > +config MBEDTLS_LIB_TLS > > > > + bool "TLS library" > > > > + help > > > > + Enable mbedtls TLS library > > > > + > > > > +endif # MBEDTLS_LIB > > > > > > We need much more granularity here, and to re-think some existing > > > symbols too perhaps. What we should be able to do is pick mbedTLS or > > > "legacy SW implementation" or "HW implementation" for the various > > > algorithms, and that in turn can have some higher level grouping to it. > > > This should then negate a bunch of the Makefile work you're doing as we > > > won't have CONFIG_SHA256 enabled as we'll have > CONFIG_MBEDTLS_LIB_SHA256 > > > or whatever enabled. > > > > > > > I think we should use CONFIG_MBEDTLS_LIB_[CRYPTO,X509,TLS] for high-level > > grouping. > > Underneath, the CONFIG_SHA[1,256,512] switches (and other crypto options) > > can be > > used as sub build options in both MbedTLS and "legacy libs". > > > > Take hash as an example, if the users prefer to use MbedTLS other than > > "legacy libs" for > > hash operation, CONFIG_MBEDTLS_LIB_CRYPTO should be defined as the main > > switch > > (the users can still prefer to use "legacy libs" for X509 by > > keeping CONFIG_MBEDTLS_LIB_X509 > > disabled). > > Then enable the algorithms they need (e.g. CONFIG_SHA256) - the algorithm > > options works > > for both MbedTLS and "legacy libs". > > > > HW implementations with MbedTLS (aka, Alternative algorithms in MbedTLS) > is > > another > > topic which is not covered in this patch set (It needs to migrate each > > vendor's solution under > > MbedTLS alternative algorithm). > > Current patch set is focused on SW implementation with MbedTLS. > > The "easy" problem with what's in v3 is that X509 and CRYPTO are > select'd under the main heading. Not sure if I get what you mentioned, currently all MbedTLS options are under Library routines > Security support Do you think we should keep them in other places? > The harder problem is that we > intentionally have granularity for SHA256, SHA512, etc, etc and all of > that goes away with the current Kconfig option if you select mbedTLS. We > need to bring that back. And we shouldn't need to have all of the ifneq > statements in Makefiles because both CONFIG_SHA256 and > CONFIG_MBEDLTS_LIB_CRYPTO_SHA256 will not be true (Or possibly, > CONFIG_SHA256 gates things U-Boot's internal API for sha256'ing > something and CONFIG_LEGACY_SHA256 controls building lib/sha256.c). > > I think we should not introduce new ones like CONFIG_LEGACY_, CONFIG_SHA[1,256,512] should be used no matter whether MbedTLS is enabled or not. I understand your concern, I will bring CONFIG_SHA[1,256,512] back when MbedTLS is enabled. Those options should control the options in the MbedTLS default config file. Regards, Raymond