Re: [PATCH v5 02/29] kconfig: Add tools support to CONFIG_IS_ENABLED()
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 > Suggested-by: Rasmus Villemoes # b4f73886 > Reviewed-by: Alexandru Gagniuc Applied to u-boot/master, thanks! -- Tom signature.asc Description: PGP signature
Re: [PATCH v5 02/29] kconfig: Add tools support to CONFIG_IS_ENABLED()
On 10/7/21 4:04 PM, Tom Rini wrote: On Thu, Oct 07, 2021 at 03:33:32PM -0500, Alex G. wrote: 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 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 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 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 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 Suggested-by: Rasmus Villemoes # b4f73886 Reviewed-by: Alexandru Gagniuc The problem here is we don't include 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 ***: 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. Is there perhaps a happy medium? Or do we just need to think harder on how to make the code U-Boot needs shared between target and host tools clean and clear and obvious enough? I think hard that's an honorable goal irrespective of the status of libcrypto. libcrypto isolation is a happy side-effect. Alex
Re: [PATCH v5 02/29] kconfig: Add tools support to CONFIG_IS_ENABLED()
On Thu, Oct 07, 2021 at 03:33:32PM -0500, Alex G. wrote: > > > 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 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 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 > > > > > > > > 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 > > > > > > > > > > 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 > > > > > > > > > > > > Suggested-by: Rasmus Villemoes > > > > > > > > > > > > # b4f73886 > > > > > > > > > > > > Reviewed-by: Alexandru Gagniuc > > > > > > > > > > > > > > > > > > > > > > The problem here is we don't include > > > > > > > > > > > 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 ***: 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
Re: [PATCH v5 02/29] kconfig: Add tools support to CONFIG_IS_ENABLED()
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 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 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 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 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 Suggested-by: Rasmus Villemoes # b4f73886 Reviewed-by: Alexandru Gagniuc The problem here is we don't include 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 ***: 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
Re: [PATCH v5 02/29] kconfig: Add tools support to CONFIG_IS_ENABLED()
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 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 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 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 > > > > > > > > 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 > > > > > > > > > > Suggested-by: Rasmus Villemoes > > > > > > > > > > # b4f73886 > > > > > > > > > > Reviewed-by: Alexandru Gagniuc > > > > > > > > > > > > > > > > > > The problem here is we don't include > > > > > > > > > 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 ***: 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. -- Tom signature.asc Description: PGP signature
Re: [PATCH v5 02/29] kconfig: Add tools support to CONFIG_IS_ENABLED()
On 10/7/21 1:50 PM, Simon Glass wrote: Hi Tom, On Thu, 7 Oct 2021 at 12:30, Tom Rini 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 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 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 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 Suggested-by: Rasmus Villemoes # b4f73886 Reviewed-by: Alexandru Gagniuc The problem here is we don't include 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 ***: 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
Re: [PATCH v5 02/29] kconfig: Add tools support to CONFIG_IS_ENABLED()
Hi Tom, On Thu, 7 Oct 2021 at 12:30, Tom Rini 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 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 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 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 > > > > > > > > Suggested-by: Rasmus Villemoes # > > > > > > > > b4f73886 > > > > > > > > Reviewed-by: Alexandru Gagniuc > > > > > > > > > > > > > > The problem here is we don't include > > > > > > > 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 ***: 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? Regards, Simon
Re: [PATCH v5 02/29] kconfig: Add tools support to CONFIG_IS_ENABLED()
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 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 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 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 > > > > > > > Suggested-by: Rasmus Villemoes # > > > > > > > b4f73886 > > > > > > > Reviewed-by: Alexandru Gagniuc > > > > > > > > > > > > The problem here is we don't include 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 ***: 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. -- Tom signature.asc Description: PGP signature
Re: [PATCH v5 02/29] kconfig: Add tools support to CONFIG_IS_ENABLED()
Hi Tom, On Thu, 7 Oct 2021 at 07:42, Tom Rini 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 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 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 > > > > > > Suggested-by: Rasmus Villemoes # > > > > > > b4f73886 > > > > > > Reviewed-by: Alexandru Gagniuc > > > > > > > > > > The problem here is we don't include 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 ***: 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 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? Regards, Simon
Re: [PATCH v5 02/29] kconfig: Add tools support to CONFIG_IS_ENABLED()
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 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 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 > > > > > Suggested-by: Rasmus Villemoes # b4f73886 > > > > > Reviewed-by: Alexandru Gagniuc > > > > > > > > The problem here is we don't include 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 ***: 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. -- Tom signature.asc Description: PGP signature
Re: [PATCH v5 02/29] kconfig: Add tools support to CONFIG_IS_ENABLED()
Hi Tom, On Wed, 6 Oct 2021 at 20:52, Tom Rini 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 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 > > > > Suggested-by: Rasmus Villemoes # b4f73886 > > > > Reviewed-by: Alexandru Gagniuc > > > > > > The problem here is we don't include 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 ***: 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? Regards, Simon
Re: [PATCH v5 02/29] kconfig: Add tools support to CONFIG_IS_ENABLED()
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 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 > > > Suggested-by: Rasmus Villemoes # b4f73886 > > > Reviewed-by: Alexandru Gagniuc > > > > The problem here is we don't include 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 ***: 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? -- Tom signature.asc Description: PGP signature
Re: [PATCH v5 02/29] kconfig: Add tools support to CONFIG_IS_ENABLED()
Hi Tom, On Wed, 6 Oct 2021 at 18:26, Tom Rini 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 > > Suggested-by: Rasmus Villemoes # b4f73886 > > Reviewed-by: Alexandru Gagniuc > > The problem here is we don't include 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 ***: 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. Regards, Simon
Re: [PATCH v5 02/29] kconfig: Add tools support to CONFIG_IS_ENABLED()
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 > Suggested-by: Rasmus Villemoes # b4f73886 > Reviewed-by: Alexandru Gagniuc The problem here is we don't include 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 ***: terminated -- Tom signature.asc Description: PGP signature
Re: [PATCH v5 02/29] kconfig: Add tools support to CONFIG_IS_ENABLED()
On 9/25/21 8:43 PM, 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 Suggested-by: Rasmus Villemoes # b4f73886 Under protest, Reviewed-by: Alexandru Gagniuc --- Changes in v5: - Update commit message - Use TOOLS_ instead of HOST_ Changes in v2: - Correct comment about USE_HOSTCC being undefined in CONFIG_VAL() - Fix up comment to put an underscore after every CONFIG include/linux/kconfig.h | 13 ++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/include/linux/kconfig.h b/include/linux/kconfig.h index d109ed3119e..a1d1a298426 100644 --- a/include/linux/kconfig.h +++ b/include/linux/kconfig.h @@ -31,11 +31,14 @@ (config_enabled(option)) /* - * U-Boot add-on: Helper macros to reference to different macros - * (CONFIG_ or CONFIG_SPL_ prefixed), depending on the build context. + * U-Boot add-on: Helper macros to reference to different macros (prefixed by + * CONFIG_, CONFIG_SPL_, CONFIG_TPL_ or CONFIG_TOOLS_), depending on the build + * context. */ -#if defined(CONFIG_TPL_BUILD) +#ifdef USE_HOSTCC +#define _CONFIG_PREFIX TOOLS_ +#elif defined(CONFIG_TPL_BUILD) #define _CONFIG_PREFIX TPL_ #elif defined(CONFIG_SPL_BUILD) #define _CONFIG_PREFIX SPL_ @@ -49,6 +52,7 @@ /* * CONFIG_VAL(FOO) evaluates to the value of + * CONFIG_TOOLS_FOO if USE_HOSTCC is defined, * CONFIG_FOO if CONFIG_SPL_BUILD is undefined, * CONFIG_SPL_FOO if CONFIG_SPL_BUILD is defined. * CONFIG_TPL_FOO if CONFIG_TPL_BUILD is defined. @@ -76,18 +80,21 @@ /* * CONFIG_IS_ENABLED(FOO) expands to + * 1 if USE_HOSTCC is defined and CONFIG_TOOLS_FOO is set to 'y', * 1 if CONFIG_SPL_BUILD is undefined and CONFIG_FOO is set to 'y', * 1 if CONFIG_SPL_BUILD is defined and CONFIG_SPL_FOO is set to 'y', * 1 if CONFIG_TPL_BUILD is defined and CONFIG_TPL_FOO is set to 'y', * 0 otherwise. * * CONFIG_IS_ENABLED(FOO, (abc)) expands to + * abc if USE_HOSTCC is defined and CONFIG_TOOLS_FOO is set to 'y', * abc if CONFIG_SPL_BUILD is undefined and CONFIG_FOO is set to 'y', * abc if CONFIG_SPL_BUILD is defined and CONFIG_SPL_FOO is set to 'y', * abc if CONFIG_TPL_BUILD is defined and CONFIG_TPL_FOO is set to 'y', * nothing otherwise. * * CONFIG_IS_ENABLED(FOO, (abc), (def)) expands to + * abc if USE_HOSTCC is defined and CONFIG_TOOLS_FOO is set to 'y', * abc if CONFIG_SPL_BUILD is undefined and CONFIG_FOO is set to 'y', * abc if CONFIG_SPL_BUILD is defined and CONFIG_SPL_FOO is set to 'y', * abc if CONFIG_TPL_BUILD is defined and CONFIG_TPL_FOO is set to 'y',
Re: [PATCH v5 02/29] kconfig: Add tools support to CONFIG_IS_ENABLED()
Hi Masahiro, On Mon, 27 Sept 2021 at 10:51, Masahiro Yamada wrote: > > On Tue, Sep 28, 2021 at 1:11 AM Alex G. wrote: > > > > > > > > On 9/25/21 8:43 PM, 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 > > > Suggested-by: Rasmus Villemoes # b4f73886 > > > --- > > > > In my vision, the host tools are target-agostic. They always support the > > same feature set: all features. From that point of view, it doesn't make > > sense to have options which enable or disable tools features. > > > Agree. > Host tools should not depend on any CONFIG option. Agreed, although arguably that is not quite what is happening here, We need some way for shared code to work, so the options are: if (tools_build() || CONFIG_IS_ENABLED(FIT_SIGNATURE)) or: if (CONFIG_IS_ENABLED(FIT_SIGNATURE)) I believe the second is better. BTW we do have a series to drop IS_ENABLED() and CONFIG_IS_ENABLED() and move to just CONFIG() so it would be: if (CONFIG(FIT_SIGNATURE)) > > In Linux kernel, most host tools are target-agnostic. > (It is true there are some exceptions such as objtool, > modpost, etc. but in theory we can make all of them > target-agnostic) > > In fact, U-Boot made much more tools target-dependent. > They must be rebuilt every time you change the configuration. Can you give some examples? We have tried for the exact opposite of that and should fix any instances we find. > > For example, mkimage in U-Boot supports many boards > and depends on the configuration. > It makes less sense to support all sort of board-specific > firmware in a huge single host tool. Maybe, but I would much prefer that there is one mkimage that supports everything. > > > In contrast, Barebox splits it into small programs, > zynq_mkimage, socfpga_mkimage, etc., > each of which is target-agnostic. [1] > Makefile decides only whether each program should be built or not. > > [1] > https://github.com/saschahauer/barebox/blob/v2021.07.0/scripts/Makefile#L22 How does it share code between boards and tools which includes CONFIG options? Regards, Simon [..]
Re: [PATCH v5 02/29] kconfig: Add tools support to CONFIG_IS_ENABLED()
Hi Alex, On Mon, 27 Sept 2021 at 10:11, Alex G. wrote: > > > > On 9/25/21 8:43 PM, 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 > > Suggested-by: Rasmus Villemoes # b4f73886 > > --- > > In my vision, the host tools are target-agostic. They always support the > same feature set: all features. From that point of view, it doesn't make > sense to have options which enable or disable tools features. > > I understand the motivation is to use CONFIG_IS_ENABLED(), which has the > appearance of making the code cleaner. We have to continue using #if > CONFIG_IS_ENABLED() -- is that an improvement over old-school #ifdefs? nit: if() not #if Most of the changes in this series are to remove #if and #idef > > So I question whether this new direction makes sense for the long term, > as opposed to taking a deeper look at the underlying code. The polite > thing from me would be to propose alternatives, which I don't have the > bandwidth these days. I won't be adding opposition to this series other > than these final thoughts. We can fix the code later, and then remove > the HOST configs. The key files to look at are those related to images in common/ which are built by tools. It's actually a bit hard to pass judgement right now since there is so much #ifdefing going on. You'll notice that one patch creates image-board.c to try to avoid compiling code unnecessarily for tools. I am sure there is more oppty for that. But Rome wasn't built in a day. One fundamental question is driver model. Of course it isn't supported in tools and I think it makes no sense to add it, since it just complicates the code. But as more of the feature become more diverse (e.g. hashing, crypto, other accelerators) we have to deal with making the software version of these features available in tools and also in the board. Anyway I think this series takes things forward and there is a path to do another pass to drop more #ifdefs as well. As to your question about using CONFIG_IS_ENABLED() for tools, I see it as a convenience for now, but I would not bank on it being needed in the fullness of time. It's just hard to know until we get closer to cleaning this up. Regards, Simon
Re: [PATCH v5 02/29] kconfig: Add tools support to CONFIG_IS_ENABLED()
On Tue, Sep 28, 2021 at 1:11 AM Alex G. wrote: > > > > On 9/25/21 8:43 PM, 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 > > Suggested-by: Rasmus Villemoes # b4f73886 > > --- > > In my vision, the host tools are target-agostic. They always support the > same feature set: all features. From that point of view, it doesn't make > sense to have options which enable or disable tools features. Agree. Host tools should not depend on any CONFIG option. In Linux kernel, most host tools are target-agnostic. (It is true there are some exceptions such as objtool, modpost, etc. but in theory we can make all of them target-agnostic) In fact, U-Boot made much more tools target-dependent. They must be rebuilt every time you change the configuration. For example, mkimage in U-Boot supports many boards and depends on the configuration. It makes less sense to support all sort of board-specific firmware in a huge single host tool. In contrast, Barebox splits it into small programs, zynq_mkimage, socfpga_mkimage, etc., each of which is target-agnostic. [1] Makefile decides only whether each program should be built or not. [1] https://github.com/saschahauer/barebox/blob/v2021.07.0/scripts/Makefile#L22 > > I understand the motivation is to use CONFIG_IS_ENABLED(), which has the > appearance of making the code cleaner. We have to continue using #if > CONFIG_IS_ENABLED() -- is that an improvement over old-school #ifdefs? > > So I question whether this new direction makes sense for the long term, > as opposed to taking a deeper look at the underlying code. The polite > thing from me would be to propose alternatives, which I don't have the > bandwidth these days. I won't be adding opposition to this series other > than these final thoughts. We can fix the code later, and then remove > the HOST configs. > > Alex > > > > > Changes in v5: > > - Update commit message > > - Use TOOLS_ instead of HOST_ > > > > Changes in v2: > > - Correct comment about USE_HOSTCC being undefined in CONFIG_VAL() > > - Fix up comment to put an underscore after every CONFIG > > > > include/linux/kconfig.h | 13 ++--- > > 1 file changed, 10 insertions(+), 3 deletions(-) > > > > diff --git a/include/linux/kconfig.h b/include/linux/kconfig.h > > index d109ed3119e..a1d1a298426 100644 > > --- a/include/linux/kconfig.h > > +++ b/include/linux/kconfig.h > > @@ -31,11 +31,14 @@ > > (config_enabled(option)) > > > > /* > > - * U-Boot add-on: Helper macros to reference to different macros > > - * (CONFIG_ or CONFIG_SPL_ prefixed), depending on the build context. > > + * U-Boot add-on: Helper macros to reference to different macros (prefixed > > by > > + * CONFIG_, CONFIG_SPL_, CONFIG_TPL_ or CONFIG_TOOLS_), depending on the > > build > > + * context. > >*/ > > > > -#if defined(CONFIG_TPL_BUILD) > > +#ifdef USE_HOSTCC > > +#define _CONFIG_PREFIX TOOLS_ > > +#elif defined(CONFIG_TPL_BUILD) > > #define _CONFIG_PREFIX TPL_ > > #elif defined(CONFIG_SPL_BUILD) > > #define _CONFIG_PREFIX SPL_ > > @@ -49,6 +52,7 @@ > > > > /* > >* CONFIG_VAL(FOO) evaluates to the value of > > + * CONFIG_TOOLS_FOO if USE_HOSTCC is defined, > >* CONFIG_FOO if CONFIG_SPL_BUILD is undefined, > >* CONFIG_SPL_FOO if CONFIG_SPL_BUILD is defined. > >* CONFIG_TPL_FOO if CONFIG_TPL_BUILD is defined. > > @@ -76,18 +80,21 @@ > > > > /* > >* CONFIG_IS_ENABLED(FOO) expands to > > + * 1 if USE_HOSTCC is defined and CONFIG_TOOLS_FOO is set to 'y', > >* 1 if CONFIG_SPL_BUILD is undefined and CONFIG_FOO is set to 'y', > >* 1 if CONFIG_SPL_BUILD is defined and CONFIG_SPL_FOO is set to 'y', > >* 1 if CONFIG_TPL_BUILD is defined and CONFIG_TPL_FOO is set to 'y', > >* 0 otherwise. > >* > >* CONFIG_IS_ENABLED(FOO, (abc)) expands to > > + * abc if USE_HOSTCC is defined and CONFIG_TOOLS_FOO is set to 'y', > >* abc if CONFIG_SPL_BUILD is undefined and CONFIG_FOO is set to 'y', > >* abc if CONFIG_SPL_BUILD is defined and CONFIG_SPL_FOO is set to 'y', > >* abc if CONFIG_TPL_BUILD is defined and CONFIG_TPL_FOO is set to 'y', > >* nothing otherwise. > >* > >* CONFIG_IS_ENABLED(FOO, (abc), (def)) expands to > > + * abc if USE_HOSTCC is defined and CONFIG_TOOLS_FOO is set
Re: [PATCH v5 02/29] kconfig: Add tools support to CONFIG_IS_ENABLED()
On 9/25/21 8:43 PM, 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 Suggested-by: Rasmus Villemoes # b4f73886 --- In my vision, the host tools are target-agostic. They always support the same feature set: all features. From that point of view, it doesn't make sense to have options which enable or disable tools features. I understand the motivation is to use CONFIG_IS_ENABLED(), which has the appearance of making the code cleaner. We have to continue using #if CONFIG_IS_ENABLED() -- is that an improvement over old-school #ifdefs? So I question whether this new direction makes sense for the long term, as opposed to taking a deeper look at the underlying code. The polite thing from me would be to propose alternatives, which I don't have the bandwidth these days. I won't be adding opposition to this series other than these final thoughts. We can fix the code later, and then remove the HOST configs. Alex Changes in v5: - Update commit message - Use TOOLS_ instead of HOST_ Changes in v2: - Correct comment about USE_HOSTCC being undefined in CONFIG_VAL() - Fix up comment to put an underscore after every CONFIG include/linux/kconfig.h | 13 ++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/include/linux/kconfig.h b/include/linux/kconfig.h index d109ed3119e..a1d1a298426 100644 --- a/include/linux/kconfig.h +++ b/include/linux/kconfig.h @@ -31,11 +31,14 @@ (config_enabled(option)) /* - * U-Boot add-on: Helper macros to reference to different macros - * (CONFIG_ or CONFIG_SPL_ prefixed), depending on the build context. + * U-Boot add-on: Helper macros to reference to different macros (prefixed by + * CONFIG_, CONFIG_SPL_, CONFIG_TPL_ or CONFIG_TOOLS_), depending on the build + * context. */ -#if defined(CONFIG_TPL_BUILD) +#ifdef USE_HOSTCC +#define _CONFIG_PREFIX TOOLS_ +#elif defined(CONFIG_TPL_BUILD) #define _CONFIG_PREFIX TPL_ #elif defined(CONFIG_SPL_BUILD) #define _CONFIG_PREFIX SPL_ @@ -49,6 +52,7 @@ /* * CONFIG_VAL(FOO) evaluates to the value of + * CONFIG_TOOLS_FOO if USE_HOSTCC is defined, * CONFIG_FOO if CONFIG_SPL_BUILD is undefined, * CONFIG_SPL_FOO if CONFIG_SPL_BUILD is defined. * CONFIG_TPL_FOO if CONFIG_TPL_BUILD is defined. @@ -76,18 +80,21 @@ /* * CONFIG_IS_ENABLED(FOO) expands to + * 1 if USE_HOSTCC is defined and CONFIG_TOOLS_FOO is set to 'y', * 1 if CONFIG_SPL_BUILD is undefined and CONFIG_FOO is set to 'y', * 1 if CONFIG_SPL_BUILD is defined and CONFIG_SPL_FOO is set to 'y', * 1 if CONFIG_TPL_BUILD is defined and CONFIG_TPL_FOO is set to 'y', * 0 otherwise. * * CONFIG_IS_ENABLED(FOO, (abc)) expands to + * abc if USE_HOSTCC is defined and CONFIG_TOOLS_FOO is set to 'y', * abc if CONFIG_SPL_BUILD is undefined and CONFIG_FOO is set to 'y', * abc if CONFIG_SPL_BUILD is defined and CONFIG_SPL_FOO is set to 'y', * abc if CONFIG_TPL_BUILD is defined and CONFIG_TPL_FOO is set to 'y', * nothing otherwise. * * CONFIG_IS_ENABLED(FOO, (abc), (def)) expands to + * abc if USE_HOSTCC is defined and CONFIG_TOOLS_FOO is set to 'y', * abc if CONFIG_SPL_BUILD is undefined and CONFIG_FOO is set to 'y', * abc if CONFIG_SPL_BUILD is defined and CONFIG_SPL_FOO is set to 'y', * abc if CONFIG_TPL_BUILD is defined and CONFIG_TPL_FOO is set to 'y',
[PATCH v5 02/29] kconfig: Add tools support to CONFIG_IS_ENABLED()
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 Suggested-by: Rasmus Villemoes # b4f73886 --- Changes in v5: - Update commit message - Use TOOLS_ instead of HOST_ Changes in v2: - Correct comment about USE_HOSTCC being undefined in CONFIG_VAL() - Fix up comment to put an underscore after every CONFIG include/linux/kconfig.h | 13 ++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/include/linux/kconfig.h b/include/linux/kconfig.h index d109ed3119e..a1d1a298426 100644 --- a/include/linux/kconfig.h +++ b/include/linux/kconfig.h @@ -31,11 +31,14 @@ (config_enabled(option)) /* - * U-Boot add-on: Helper macros to reference to different macros - * (CONFIG_ or CONFIG_SPL_ prefixed), depending on the build context. + * U-Boot add-on: Helper macros to reference to different macros (prefixed by + * CONFIG_, CONFIG_SPL_, CONFIG_TPL_ or CONFIG_TOOLS_), depending on the build + * context. */ -#if defined(CONFIG_TPL_BUILD) +#ifdef USE_HOSTCC +#define _CONFIG_PREFIX TOOLS_ +#elif defined(CONFIG_TPL_BUILD) #define _CONFIG_PREFIX TPL_ #elif defined(CONFIG_SPL_BUILD) #define _CONFIG_PREFIX SPL_ @@ -49,6 +52,7 @@ /* * CONFIG_VAL(FOO) evaluates to the value of + * CONFIG_TOOLS_FOO if USE_HOSTCC is defined, * CONFIG_FOO if CONFIG_SPL_BUILD is undefined, * CONFIG_SPL_FOO if CONFIG_SPL_BUILD is defined. * CONFIG_TPL_FOO if CONFIG_TPL_BUILD is defined. @@ -76,18 +80,21 @@ /* * CONFIG_IS_ENABLED(FOO) expands to + * 1 if USE_HOSTCC is defined and CONFIG_TOOLS_FOO is set to 'y', * 1 if CONFIG_SPL_BUILD is undefined and CONFIG_FOO is set to 'y', * 1 if CONFIG_SPL_BUILD is defined and CONFIG_SPL_FOO is set to 'y', * 1 if CONFIG_TPL_BUILD is defined and CONFIG_TPL_FOO is set to 'y', * 0 otherwise. * * CONFIG_IS_ENABLED(FOO, (abc)) expands to + * abc if USE_HOSTCC is defined and CONFIG_TOOLS_FOO is set to 'y', * abc if CONFIG_SPL_BUILD is undefined and CONFIG_FOO is set to 'y', * abc if CONFIG_SPL_BUILD is defined and CONFIG_SPL_FOO is set to 'y', * abc if CONFIG_TPL_BUILD is defined and CONFIG_TPL_FOO is set to 'y', * nothing otherwise. * * CONFIG_IS_ENABLED(FOO, (abc), (def)) expands to + * abc if USE_HOSTCC is defined and CONFIG_TOOLS_FOO is set to 'y', * abc if CONFIG_SPL_BUILD is undefined and CONFIG_FOO is set to 'y', * abc if CONFIG_SPL_BUILD is defined and CONFIG_SPL_FOO is set to 'y', * abc if CONFIG_TPL_BUILD is defined and CONFIG_TPL_FOO is set to 'y', -- 2.33.0.685.g46640cef36-goog
