In my case, a csb_patcher.sh script [1] - among other things - delivers the example config files for the opensource AGESA boards. Although these configs haven't been updated for a while (i.e. a last update of AMD Lenovo G505S config [2] is 1 year ago), I got away with this: they are still working, just get refreshed when a user does "menuconfig" & "make" and result in a working coreboot build. However, with a commit 0eab62b [3] they are rejected [see 4].
Of course I'm going to manually "refresh" the configs to temporarily fix this problem, but I see this could increase the maintenance burden and reduce the usefulness of coreboot config files shared online... Are there any advantages of KCONFIG_STRICT / KCONFIG_WERROR that outweigh these potential issues?
[1] https://review.coreboot.org/c/coreboot/+/64873 [2] https://review.coreboot.org/c/coreboot/+/64829 [3] https://review.coreboot.org/c/coreboot/+/79259 [4]
./coreboot/$ make menuconfig src/soc/intel/meteorlake/Kconfig:457:warning: config symbol defined without type ./coreboot/.config:26:warning: unknown symbol: COMPRESS_RAMSTAGE ./coreboot/.config:230:warning: unknown symbol: ARCH_ALL_STAGES_X86 ./coreboot/.config:249:warning: unknown symbol: VBT_DATA_SIZE_KB ./coreboot/.config:256:warning: unknown symbol: UART_PCI_ADDR ./coreboot/.config:264:warning: unknown symbol: S3_DATA_POS ./coreboot/.config:265:warning: unknown symbol: S3_DATA_SIZE ./coreboot/.config:274:warning: unknown symbol: LOGICAL_CPUS ./coreboot/.config:459:warning: unknown symbol: INTEL_GMA_OPREGION_2_0 ./coreboot/.config:572:warning: unknown symbol: PAYLOAD_YABITS ./coreboot/.config:574:warning: unknown symbol: PAYLOAD_TIANOCORE
ERROR: 10 warnings encountered, and warnings are errors.
make: *** [build/util/kconfig/Makefile.real:47: menuconfig] Error 1
On 28.11.23 19:04, Mike Banon mikebdp2@gmail.com wrote:
Are there any advantages of KCONFIG_STRICT / KCONFIG_WERROR that outweigh these potential issues?
It ensures that we don't silently build with unknown symbols (typo in manual editing, changes to the config, ...), and wonder why CONFIG_UART_DEBUG=y doesn't actually enable serial output on select Intel chipsets (solution: it was renamed 5 years ago).
I put a subset of your question up for discussion[0] earlier today, including a patch that tweaks the behavior to make things pass the *config targets. That should resolve your issue while keeping the safety benefit, no?
Patrick
[0] https://mail.coreboot.org/hyperkitty/list/coreboot@coreboot.org/thread/M543F...
Hey Mike, I think you should be able to just append change the kconfig values when you run make to override the current settings.
something like `make menuconfig KCONFIG_WERROR=0 KCONFIG_WARN_UNKNOWN_SYMBOLS=0`
if we update where they're set in the makefile from := to ?=, you could even just have them set as environment variables so they don't need to be on the command line.
I wouldn't suggest that most people do that, but what you're trying to do is a special case.
Martin Nov 28, 2023, 11:05 by mikebdp2@gmail.com:
In my case, a csb_patcher.sh script [1] - among other things - delivers the example config files for the opensource AGESA boards. Although these configs haven't been updated for a while (i.e. a last update of AMD Lenovo G505S config [2] is 1 year ago), I got away with this: they are still working, just get refreshed when a user does "menuconfig" & "make" and result in a working coreboot build. However, with a commit 0eab62b [3] they are rejected [see 4].
Of course I'm going to manually "refresh" the configs to temporarily fix this problem, but I see this could increase the maintenance burden and reduce the usefulness of coreboot config files shared online... Are there any advantages of KCONFIG_STRICT / KCONFIG_WERROR that outweigh these potential issues?
[1] https://review.coreboot.org/c/coreboot/+/64873 [2] https://review.coreboot.org/c/coreboot/+/64829 [3] https://review.coreboot.org/c/coreboot/+/79259 [4]
./coreboot/$ make menuconfig src/soc/intel/meteorlake/Kconfig:457:warning: config symbol defined without type ./coreboot/.config:26:warning: unknown symbol: COMPRESS_RAMSTAGE ./coreboot/.config:230:warning: unknown symbol: ARCH_ALL_STAGES_X86 ./coreboot/.config:249:warning: unknown symbol: VBT_DATA_SIZE_KB ./coreboot/.config:256:warning: unknown symbol: UART_PCI_ADDR ./coreboot/.config:264:warning: unknown symbol: S3_DATA_POS ./coreboot/.config:265:warning: unknown symbol: S3_DATA_SIZE ./coreboot/.config:274:warning: unknown symbol: LOGICAL_CPUS ./coreboot/.config:459:warning: unknown symbol: INTEL_GMA_OPREGION_2_0 ./coreboot/.config:572:warning: unknown symbol: PAYLOAD_YABITS ./coreboot/.config:574:warning: unknown symbol: PAYLOAD_TIANOCORE
ERROR: 10 warnings encountered, and warnings are errors.
make: *** [build/util/kconfig/Makefile.real:47: menuconfig] Error 1
Best regards, Mike Banon Open Source Community Manager of 3mdeb - https://3mdeb.com/ _______________________________________________ coreboot mailing list -- coreboot@coreboot.org To unsubscribe send an email to coreboot-leave@coreboot.org
On 29.11.23 00:20, Martin Roth via coreboot coreboot@coreboot.org wrote:
Hey Mike, I think you should be able to just append change the kconfig values when you run make to override the current settings.
something like `make menuconfig KCONFIG_WERROR=0 KCONFIG_WARN_UNKNOWN_SYMBOLS=0`
if we update where they're set in the makefile from := to ?=, you could even just have them set as environment variables so they don't need to be on the command line.
Kconfig doesn't check for the value in the variables, just for their presence. Therefore we'd need to unexport the variables, and that's not possible per-target (despite the gnu make docs claiming otherwise) or on the make command line.
I started a discussion in another thread on that, maybe we could discuss this over at https://mail.coreboot.org/hyperkitty/list/coreboot@coreboot.org/thread/M543F...
Hi Patrick,
we two had an IRC conversation on monday about this topic but didn't finish this.
Your above mentioned patch helps (I'm a reviewer of this). When I tried I also noticed that this does not point out loudly if you have problems in your config. So the approach without the patch (in result more strict) is from my opinion better if we have a workaround beside edit config manually. In the parallel thread here Martin mentioned the same workaround as you mentioned on IRC on Monday. But I get not work. I added for test a line in a working .config with "NOT_KNOWN_SWITCH=1234" which produces the error and changed now the coreboot toplevel Makefile according: -KCONFIG_WERROR:= 1 -KCONFIG_WARN_UNKNOWN_SYMBOLS:= 1 +KCONFIG_WERROR ?= 1 +KCONFIG_WARN_UNKNOWN_SYMBOLS ?= 1
And did as Martin and also you mentioned make menuconfig KCONFIG_WERROR=0 KCONFIG_WARN_UNKNOWN_SYMBOLS=0
But the error still exist.
Can the cause be a dependency from make? We use v4.2.1 on Red Hat 8.8.
Regards Uwe
29. November 2023 12:14, "Poeche, Uwe via coreboot" coreboot@coreboot.org schrieb:
Your above mentioned patch helps (I'm a reviewer of this). When I tried I also noticed that this does not point out loudly if you have problems in your config. So the approach without the patch (in result more strict) is from my opinion better if we have a workaround beside edit config manually. In the parallel thread here Martin mentioned the same workaround as you mentioned on IRC on Monday. But I get not work.
Right, because the code checks for the variables' presence, not for their value. I was mistaken on that, as was Martin apparently.
I modified the changeset so that it still doesn't fail on warnings during *config, but still prints them. That doesn't matter a whole lot with menuconfig and nconfig because they proceed to clear the screen immediately after, but oldconfig and olddefconfig are somewhat more helpful now:
$ make oldconfig /home/pgeorgi/coreboot/.config:152:warning: unknown symbol: VBT_DATA_SIZE_KB * * Restart config... * ...
$ make olddefconfig /home/pgeorgi/coreboot/.config:152:warning: unknown symbol: VBT_DATA_SIZE_KB # # configuration written to /home/pgeorgi/coreboot/.config #
Does that help?
Patrick
At the moment none of the suggestions work (tried make oldconfig, make olddefconfig, make menuconfig KCONFIG_WERROR=0 KCONFIG_WARN_UNKNOWN_SYMBOLS=0 "). Any workaround alternative to git revert of this commit would be nice
On Wed, Nov 29, 2023 at 1:41 PM Patrick Georgi via coreboot coreboot@coreboot.org wrote:
On 29.11.23 00:20, Martin Roth via coreboot coreboot@coreboot.org wrote:
Hey Mike, I think you should be able to just append change the kconfig values when you run make to override the current settings.
something like `make menuconfig KCONFIG_WERROR=0 KCONFIG_WARN_UNKNOWN_SYMBOLS=0`
if we update where they're set in the makefile from := to ?=, you could even just have them set as environment variables so they don't need to be on the command line.
Kconfig doesn't check for the value in the variables, just for their presence. Therefore we'd need to unexport the variables, and that's not possible per-target (despite the gnu make docs claiming otherwise) or on the make command line.
I started a discussion in another thread on that, maybe we could discuss this over at https://mail.coreboot.org/hyperkitty/list/coreboot@coreboot.org/thread/M543F... _______________________________________________ coreboot mailing list -- coreboot@coreboot.org To unsubscribe send an email to coreboot-leave@coreboot.org
Thank you, the patch at https://review.coreboot.org/c/coreboot/+/79298 really helps, I +1 it now
On Wed, Nov 29, 2023 at 8:39 PM Mike Banon mikebdp2@gmail.com wrote:
At the moment none of the suggestions work (tried make oldconfig, make olddefconfig, make menuconfig KCONFIG_WERROR=0 KCONFIG_WARN_UNKNOWN_SYMBOLS=0 "). Any workaround alternative to git revert of this commit would be nice
On Wed, Nov 29, 2023 at 1:41 PM Patrick Georgi via coreboot coreboot@coreboot.org wrote:
On 29.11.23 00:20, Martin Roth via coreboot coreboot@coreboot.org wrote:
Hey Mike, I think you should be able to just append change the kconfig values when you run make to override the current settings.
something like `make menuconfig KCONFIG_WERROR=0 KCONFIG_WARN_UNKNOWN_SYMBOLS=0`
if we update where they're set in the makefile from := to ?=, you could even just have them set as environment variables so they don't need to be on the command line.
Kconfig doesn't check for the value in the variables, just for their presence. Therefore we'd need to unexport the variables, and that's not possible per-target (despite the gnu make docs claiming otherwise) or on the make command line.
I started a discussion in another thread on that, maybe we could discuss this over at https://mail.coreboot.org/hyperkitty/list/coreboot@coreboot.org/thread/M543F... _______________________________________________ coreboot mailing list -- coreboot@coreboot.org To unsubscribe send an email to coreboot-leave@coreboot.org
-- Best regards, Mike Banon Open Source Community Manager of 3mdeb - https://3mdeb.com/
-
Martin Roth -
Mike Banon -
Patrick Georgi -
patrick＠coreboot.org
-
Poeche, Uwe