Re: [PATCH v5 02/29] kconfig: Add tools support to CONFIG_IS_ENABLED()

From: "Alex G." <mr.nuke.me@gmail.com>
To: Tom Rini <trini@konsulko.com>
Cc: Simon Glass <sjg@chromium.org>,
	U-Boot Mailing List <u-boot@lists.denx.de>,
	Andre Przywara <andre.przywara@arm.com>,
	Rasmus Villemoes <rasmus.villemoes@prevas.dk>,
	Robert Marko <robert.marko@sartura.hr>,
	Masahiro Yamada <masahiroy@kernel.org>,
	Masahiro Yamada <yamada.masahiro@socionext.com>
Subject: Re: [PATCH v5 02/29] kconfig: Add tools support to CONFIG_IS_ENABLED()
Date: Thu, 7 Oct 2021 15:33:32 -0500	[thread overview]
Message-ID: <35223dc3-4af4-f70d-00d0-14495426be3f@gmail.com> (raw)
In-Reply-To: <20211007193943.GW31748@bill-the-cat>

On 10/7/21 2:39 PM, Tom Rini wrote:
> On Thu, Oct 07, 2021 at 02:32:42PM -0500, Alex G. wrote:
>>
>>
>> On 10/7/21 1:50 PM, Simon Glass wrote:
>>> Hi Tom,
>>>
>>> On Thu, 7 Oct 2021 at 12:30, Tom Rini <trini@konsulko.com> wrote:
>>>>
>>>> On Thu, Oct 07, 2021 at 12:02:24PM -0600, Simon Glass wrote:
>>>>> Hi Tom,
>>>>>
>>>>> On Thu, 7 Oct 2021 at 07:42, Tom Rini <trini@konsulko.com> wrote:
>>>>>>
>>>>>> On Thu, Oct 07, 2021 at 07:32:04AM -0600, Simon Glass wrote:
>>>>>>> Hi Tom,
>>>>>>>
>>>>>>> On Wed, 6 Oct 2021 at 20:52, Tom Rini <trini@konsulko.com> wrote:
>>>>>>>>
>>>>>>>> On Wed, Oct 06, 2021 at 08:49:13PM -0600, Simon Glass wrote:
>>>>>>>>> Hi Tom,
>>>>>>>>>
>>>>>>>>> On Wed, 6 Oct 2021 at 18:26, Tom Rini <trini@konsulko.com> wrote:
>>>>>>>>>>
>>>>>>>>>> On Sat, Sep 25, 2021 at 07:43:15PM -0600, Simon Glass wrote:
>>>>>>>>>>
>>>>>>>>>>> At present we must separately test for the host build for many options,
>>>>>>>>>>> since we force them to be enabled. For example, CONFIG_FIT is always
>>>>>>>>>>> enabled in the host tools, even if CONFIG_FIT is not enabled by the
>>>>>>>>>>> board itself.
>>>>>>>>>>>
>>>>>>>>>>> It would be more convenient if we could use, for example,
>>>>>>>>>>> CONFIG_IS_ENABLED(FIT) and get CONFIG_HOST_FIT, when building for the
>>>>>>>>>>> host. Add support for this.
>>>>>>>>>>>
>>>>>>>>>>> With this and the tools_build() function, we should be able to remove all
>>>>>>>>>>> the #ifdefs currently needed in code that is build by tools and targets.
>>>>>>>>>>>
>>>>>>>>>>> This will be even nicer when we move to using CONFIG(xxx) everywhere,
>>>>>>>>>>> since all the #ifdef and IS_ENABLED/CONFIG_IS_ENABLED stuff will go away.
>>>>>>>>>>>
>>>>>>>>>>> Signed-off-by: Simon Glass <sjg@chromium.org>
>>>>>>>>>>> Suggested-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk> # b4f73886
>>>>>>>>>>> Reviewed-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
>>>>>>>>>>
>>>>>>>>>> The problem here is we don't include <linux/kconfig.h> automatically
>>>>>>>>>> when building host stuff, I believe.  This is why doing this breaks
>>>>>>>>>> test_mkimage_hashes for me on am335x_evm with:
>>>>>>>>>> /tmp/.bm-work/am335x_evm/tools/mkimage -D -I dts -O dtb -i /tmp/.bm-work/am335x_evm -f /home/trini/work/u-boot/u-boot/test/py/tests/vboot//hash-images.its /tmp/.bm-work/am335x_evm/test.fit
>>>>>>>>>> *** stack smashing detected ***: <unknown> terminated
>>>>>>>>>
>>>>>>>>> Oh dear, and no CI coverage.
>>>>>>>>>
>>>>>>>>> I was reluctant to include kconfig.h everywhere but perhaps that is
>>>>>>>>> the best approach. Will take a look ASAP.
>>>>>>>>
>>>>>>>> Maybe we need to think a bit harder too about how we structure
>>>>>>>> intentionally shared code.
>>>>>>>>
>>>>>>>> Why not, for example, for these common algorithms, rely on typical
>>>>>>>> system headers/libraries in the tooling, which means we validated U-Boot
>>>>>>>> vs common reference, rather than just our implementations?
>>>>>>>
>>>>>>> Do you mean we use openssl for sha1, for example?
>>>>>>
>>>>>> I guess, yes.  Just flat out saying we require openssl for tools, and
>>>>>> doing our best to not make compatibility with libressl difficult, seems
>>>>>> likely to cause less headaches for people than what we already require
>>>>>> in terms of Python.
>>>>>
>>>>> I'm OK with that, although I do think the problem identified here
>>>>> (CONFIG_SHA256 not enabled) is somewhat sideways from that. We already
>>>>
>>>> OK, I've taken what you posted on IRC and folded that in, continuing
>>>> tests now.
>>>>
>>>>> use separate code paths to run hashing. Perhaps we could make it
>>>>> optional?
>>>>>
>>>>> What about those people that complain about crypto libraries on their systems?
>>>>
>>>> I'm not sure how big a problem that really is, currently.  I guess one
>>>> thing would be to make a separate thread on it, and put it in the next
>>>> -rc email as well, for people to explain why it would be a hardship.
>>>> That in turn, I think, is coming down to modern vs very old openssl
>>>> support, rather than having any at all.
>>>
>>> OK I'll take a look at some point.
>>>
>>> Or perhaps Alex might like to?
>>
>> We just got a complain about OpenSSL yesterday [1]
>>
>> Alex
>>
>> [1] https://lists.denx.de/pipermail/u-boot/2021-October/462728.html
> 
> Oh goodness, LibreELC is a custom build system... I'll have to chime in
> there, thanks.

I am in favor of keeping libcrapto separate. We still need our own code 
for CRC32 and other weak or non-crypto hashes, a tidbit which makes me 
doubt the wisdom of relying entirely on an external lib.

I had to make a similar decision when writing the hashes test. 
Originally, I was going to use pyCrypto, crcelk, to re-hash everything 
and compare to mkimage. It turned out to be neither necessarry nor 
efficient.

Alex