From 7fb1d1e0382c2841906fbc1bad7d902b21fe67de Mon Sep 17 00:00:00 2001 From: Masahiro Yamada Date: Mon, 23 Sep 2024 16:56:02 +0900 Subject: [PATCH 1/8] kbuild: move non-boot built-in DTBs to .rodata section Commit aab94339cd85 ("of: Add support for linking device tree blobs into vmlinux") introduced a mechanism to embed DTBs into vmlinux. Initially, it was used for wrapping boot DTBs in arch/*/boot/dts/, but it is now reused for more generic purposes, such as testing. Built-in DTBs are discarded because KERNEL_DTB() is part of INIT_DATA, as defined in include/asm-generic/vmlinux.lds.h. This has not been an issue so far because OF unittests are triggered during boot, as defined by late_initcall(of_unittest). However, the recent clk KUnit test additions have caused problems because KUnit can execute test suites after boot. For example: # echo > /sys/kernel/debug/kunit/clk_register_clk_parent_data_device/run This command triggers a stack trace because built-in DTBs have already been freed. While it is possible to move such test suites from kunit_test_suites to kunit_test_init_section_suites, it would be preferable to avoid usage limitations. This commit moves non-boot built-in DTBs to the .rodata section. Since these generic DTBs are looked up by name, they do not need to be placed in the special .dtb.init.rodata section. Boot DTBs should remain in .dtb.init.rodata because the arch boot code generally does not know the DT name, thus it uses the __dtb_start symbol to locate it. This separation also ensures that the __dtb_start symbol references the boot DTB. Currently, the .dtb.init.rodata is a mixture of both boot and non-boot DTBs. The __dtb_start symbol must be followed by the boot DTB, but we currently rely on the link order (i.e., the order in Makefiles), which is very fragile. The implementation is kind of cheesy; the section is .dtb.init.rodata when $(obj) starts with arch/$(SRCARCH)/boot/dts, and .rodata section otherwise. This will be refactored later. Fixes: 5c9dd72d8385 ("of: Add a KUnit test for overlays and test managed APIs") Fixes: 5776526beb95 ("clk: Add KUnit tests for clk fixed rate basic type") Fixes: 274aff8711b2 ("clk: Add KUnit tests for clks registered with struct clk_parent_data") Signed-off-by: Masahiro Yamada Acked-by: Rob Herring (Arm) --- scripts/Makefile.dtbs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/scripts/Makefile.dtbs b/scripts/Makefile.dtbs index 46009d5f1486..8d56c0815f33 100644 --- a/scripts/Makefile.dtbs +++ b/scripts/Makefile.dtbs @@ -34,12 +34,14 @@ $(obj)/dtbs-list: $(dtb-y) FORCE # Assembly file to wrap dtb(o) # --------------------------------------------------------------------------- +builtin-dtb-section = $(if $(filter arch/$(SRCARCH)/boot/dts%, $(obj)),.dtb.init.rodata,.rodata) + # Generate an assembly file to wrap the output of the device tree compiler quiet_cmd_wrap_S_dtb = WRAP $@ cmd_wrap_S_dtb = { \ symbase=__$(patsubst .%,%,$(suffix $<))_$(subst -,_,$(notdir $*)); \ echo '\#include '; \ - echo '.section .dtb.init.rodata,"a"'; \ + echo '.section $(builtin-dtb-section),"a"'; \ echo '.balign STRUCT_ALIGNMENT'; \ echo ".global $${symbase}_begin"; \ echo "$${symbase}_begin:"; \ From 4d46b5b623e0adee1153b1d80689211e5094ae44 Mon Sep 17 00:00:00 2001 From: Masahiro Yamada Date: Wed, 25 Sep 2024 20:25:31 +0900 Subject: [PATCH 2/8] kconfig: fix infinite loop in sym_calc_choice() Since commit f79dc03fe68c ("kconfig: refactor choice value calculation"), Kconfig for ARCH=powerpc may result in an infinite loop. This occurs because there are two entries for POWERPC64_CPU in a choice block. If the same symbol appears twice in a choice block, the ->choice_link node is added twice to ->choice_members, resulting a corrupted linked list. A simple test case is: choice prompt "choice" config A bool "A" config B bool "B 1" config B bool "B 2" endchoice Running 'make defconfig' results in an infinite loop. One solution is to replace the current two entries: config POWERPC64_CPU bool "Generic (POWER5 and PowerPC 970 and above)" depends on PPC_BOOK3S_64 && !CPU_LITTLE_ENDIAN select PPC_64S_HASH_MMU config POWERPC64_CPU bool "Generic (POWER8 and above)" depends on PPC_BOOK3S_64 && CPU_LITTLE_ENDIAN select ARCH_HAS_FAST_MULTIPLIER select PPC_64S_HASH_MMU select PPC_HAS_LBARX_LHARX with the following single entry: config POWERPC64_CPU bool "Generic 64 bit powerpc" depends on PPC_BOOK3S_64 select ARCH_HAS_FAST_MULTIPLIER if CPU_LITTLE_ENDIAN select PPC_64S_HASH_MMU select PPC_HAS_LBARX_LHARX if CPU_LITTLE_ENDIAN In my opinion, the latter looks cleaner, but PowerPC maintainers may prefer to display different prompts depending on CPU_LITTLE_ENDIAN. For now, this commit fixes the issue in Kconfig, restoring the original behavior. I will reconsider whether such a use case is worth supporting. Fixes: f79dc03fe68c ("kconfig: refactor choice value calculation") Reported-by: Marco Bonelli Closes: https://lore.kernel.org/all/1763151587.3581913.1727224126288@privateemail.com/ Signed-off-by: Masahiro Yamada --- scripts/kconfig/parser.y | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/scripts/kconfig/parser.y b/scripts/kconfig/parser.y index 1ad60f9e164e..bc43fb67c7c4 100644 --- a/scripts/kconfig/parser.y +++ b/scripts/kconfig/parser.y @@ -159,8 +159,14 @@ config_stmt: config_entry_start config_option_list yynerrs++; } - list_add_tail(¤t_entry->sym->choice_link, - ¤t_choice->choice_members); + /* + * If the same symbol appears twice in a choice block, the list + * node would be added twice, leading to a broken linked list. + * list_empty() ensures that this symbol has not yet added. + */ + if (list_empty(¤t_entry->sym->choice_link)) + list_add_tail(¤t_entry->sym->choice_link, + ¤t_choice->choice_members); } printd(DEBUG_PARSE, "%s:%d:endconfig\n", cur_filename, cur_lineno); From 8d095547debdd26583171a6b589acbc9fd76aa9f Mon Sep 17 00:00:00 2001 From: Masahiro Yamada Date: Tue, 1 Oct 2024 02:02:22 +0900 Subject: [PATCH 3/8] kconfig: clear expr::val_is_valid when allocated Since commit 95573cac25c6 ("kconfig: cache expression values"), xconfig emits a lot of false-positive "unmet direct dependencies" warnings. While conf_read() clears val_is_valid flags, 'make xconfig' calculates symbol values even before the conf_read() call. This is another issue that should be addressed separately, but it has revealed that the val_is_valid field is not initialized. Fixes: 95573cac25c6 ("kconfig: cache expression values") Signed-off-by: Masahiro Yamada --- scripts/kconfig/expr.c | 1 + 1 file changed, 1 insertion(+) diff --git a/scripts/kconfig/expr.c b/scripts/kconfig/expr.c index 78738ef412de..16f92c4a775a 100644 --- a/scripts/kconfig/expr.c +++ b/scripts/kconfig/expr.c @@ -47,6 +47,7 @@ static struct expr *expr_lookup(enum expr_type type, void *l, void *r) e->type = type; e->left._initdata = l; e->right._initdata = r; + e->val_is_valid = false; hash_add(expr_hashtable, &e->node, hash); From da724c33b685463720b1c625ac440e894dc57ec0 Mon Sep 17 00:00:00 2001 From: Masahiro Yamada Date: Tue, 1 Oct 2024 02:02:23 +0900 Subject: [PATCH 4/8] kconfig: qconf: move conf_read() before drawing tree pain The constructor of ConfigMainWindow() calls show*View(), which needs to calculate symbol values. conf_read() must be called before that. Fixes: 060e05c3b422 ("kconfig: qconf: remove initial call to conf_changed()") Signed-off-by: Masahiro Yamada --- scripts/kconfig/qconf.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/kconfig/qconf.cc b/scripts/kconfig/qconf.cc index 97fce13e551e..7dba8014ead4 100644 --- a/scripts/kconfig/qconf.cc +++ b/scripts/kconfig/qconf.cc @@ -1505,6 +1505,8 @@ ConfigMainWindow::ConfigMainWindow(void) connect(helpText, &ConfigInfoView::menuSelected, this, &ConfigMainWindow::setMenuLink); + conf_read(NULL); + QString listMode = configSettings->value("/listMode", "symbol").toString(); if (listMode == "single") showSingleView(); @@ -1906,8 +1908,6 @@ int main(int ac, char** av) configApp->connect(configApp, SIGNAL(lastWindowClosed()), SLOT(quit())); configApp->connect(configApp, SIGNAL(aboutToQuit()), v, SLOT(saveSettings())); - conf_read(NULL); - v->show(); configApp->exec(); From 984ed20ece1c6c20789ece040cbff3eb1a388fa9 Mon Sep 17 00:00:00 2001 From: Masahiro Yamada Date: Tue, 1 Oct 2024 18:02:22 +0900 Subject: [PATCH 5/8] kconfig: qconf: fix buffer overflow in debug links If you enable "Option -> Show Debug Info" and click a link, the program terminates with the following error: *** buffer overflow detected ***: terminated The buffer overflow is caused by the following line: strcat(data, "$"); The buffer needs one more byte to accommodate the additional character. Fixes: c4f7398bee9c ("kconfig: qconf: make debug links work again") Signed-off-by: Masahiro Yamada --- scripts/kconfig/qconf.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/kconfig/qconf.cc b/scripts/kconfig/qconf.cc index 7dba8014ead4..e260cab1c2af 100644 --- a/scripts/kconfig/qconf.cc +++ b/scripts/kconfig/qconf.cc @@ -1166,7 +1166,7 @@ void ConfigInfoView::clicked(const QUrl &url) { QByteArray str = url.toEncoded(); const std::size_t count = str.size(); - char *data = new char[count + 1]; + char *data = new char[count + 2]; // '$' + '\0' struct symbol **result; struct menu *m = NULL; From c14a30468230c608731f36569bfd9785bb486131 Mon Sep 17 00:00:00 2001 From: Sami Tolvanen Date: Mon, 23 Sep 2024 18:18:47 +0000 Subject: [PATCH 6/8] scripts: import more list macros Import list_is_first, list_is_last, list_replace, and list_replace_init. Signed-off-by: Sami Tolvanen Signed-off-by: Masahiro Yamada --- scripts/include/list.h | 50 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/scripts/include/list.h b/scripts/include/list.h index fea1e2b79063..8bdcaadca709 100644 --- a/scripts/include/list.h +++ b/scripts/include/list.h @@ -127,6 +127,36 @@ static inline void list_del(struct list_head *entry) entry->prev = LIST_POISON2; } +/** + * list_replace - replace old entry by new one + * @old : the element to be replaced + * @new : the new element to insert + * + * If @old was empty, it will be overwritten. + */ +static inline void list_replace(struct list_head *old, + struct list_head *new) +{ + new->next = old->next; + new->next->prev = new; + new->prev = old->prev; + new->prev->next = new; +} + +/** + * list_replace_init - replace old entry by new one and initialize the old one + * @old : the element to be replaced + * @new : the new element to insert + * + * If @old was empty, it will be overwritten. + */ +static inline void list_replace_init(struct list_head *old, + struct list_head *new) +{ + list_replace(old, new); + INIT_LIST_HEAD(old); +} + /** * list_move - delete from one list and add as another's head * @list: the entry to move @@ -150,6 +180,26 @@ static inline void list_move_tail(struct list_head *list, list_add_tail(list, head); } +/** + * list_is_first -- tests whether @list is the first entry in list @head + * @list: the entry to test + * @head: the head of the list + */ +static inline int list_is_first(const struct list_head *list, const struct list_head *head) +{ + return list->prev == head; +} + +/** + * list_is_last - tests whether @list is the last entry in list @head + * @list: the entry to test + * @head: the head of the list + */ +static inline int list_is_last(const struct list_head *list, const struct list_head *head) +{ + return list->next == head; +} + /** * list_is_head - tests whether @list is the list @head * @list: the entry to test From d939881a15b13c028257471d8853d12d83686bcc Mon Sep 17 00:00:00 2001 From: Xu Yang Date: Wed, 25 Sep 2024 13:32:30 +0800 Subject: [PATCH 7/8] kbuild: fix a typo dt_binding_schema -> dt_binding_schemas If we follow "make help" to "make dt_binding_schema", we will see below error: $ make dt_binding_schema make[1]: *** No rule to make target 'dt_binding_schema'. Stop. make: *** [Makefile:224: __sub-make] Error 2 It should be a typo. So this will fix it. Fixes: 604a57ba9781 ("dt-bindings: kbuild: Add separate target/dependency for processed-schema.json") Signed-off-by: Xu Yang Reviewed-by: Nicolas Schier Signed-off-by: Masahiro Yamada --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 187a4ce2728e..7bb521fae64a 100644 --- a/Makefile +++ b/Makefile @@ -1645,7 +1645,7 @@ help: echo '* dtbs - Build device tree blobs for enabled boards'; \ echo ' dtbs_install - Install dtbs to $(INSTALL_DTBS_PATH)'; \ echo ' dt_binding_check - Validate device tree binding documents and examples'; \ - echo ' dt_binding_schema - Build processed device tree binding schemas'; \ + echo ' dt_binding_schemas - Build processed device tree binding schemas'; \ echo ' dtbs_check - Validate device tree source files';\ echo '') From 82cb44308951ad4ce7a8500b9e025d27d7fb3526 Mon Sep 17 00:00:00 2001 From: Aaron Thompson Date: Fri, 4 Oct 2024 07:52:45 +0000 Subject: [PATCH 8/8] kbuild: deb-pkg: Remove blank first line from maint scripts The blank line causes execve() to fail: # strace ./postinst execve("./postinst", ...) = -1 ENOEXEC (Exec format error) strace: exec: Exec format error +++ exited with 1 +++ However running the scripts via shell does work (at least with bash) because the shell attempts to execute the file as a shell script when execve() fails. Fixes: b611daae5efc ("kbuild: deb-pkg: split image and debug objects staging out into functions") Signed-off-by: Aaron Thompson Reviewed-by: Nathan Chancellor Reviewed-by: Nicolas Schier Signed-off-by: Masahiro Yamada --- scripts/package/builddeb | 1 - 1 file changed, 1 deletion(-) diff --git a/scripts/package/builddeb b/scripts/package/builddeb index c1757db6aa8a..404587fc71fe 100755 --- a/scripts/package/builddeb +++ b/scripts/package/builddeb @@ -74,7 +74,6 @@ install_linux_image () { mkdir -p "${pdir}/DEBIAN" cat <<-EOF > "${pdir}/DEBIAN/${script}" - #!/bin/sh set -e