[PATCH v2 00/14] selftests/nolibc: add minimal kernel config support - part1

[PATCH v2 00/14] selftests/nolibc: add minimal kernel config support - part1

[Zhangjin Wu]

Hi, Willy, Thomas

Here is the first part of v2 of our tinyconfig support for nolibc-test
[1], the patchset subject is reserved as before.

As discussed in v1 thread [1], to easier the review progress, the whole
tinyconfig support is divided into several parts, mainly by
architecture, here is the first part, include basic preparation and
powerpc example.

This patchset should be applied after the 32/64-bit powerpc support [2],
exactly these two are required by us:

    * selftests/nolibc: add extra config file customize support
    * selftests/nolibc: add XARCH and ARCH mapping support

In this patchset, we firstly add some misc preparations and at last add
the tinyconfig target and use powerpc as the first example. 

Tests:

    // powerpc run-user
    $ for arch in powerpc powerpc64 powerpc64le; do \
        rm -rf $PWD/kernel-$arch; \
        mkdir -p $PWD/kernel-$arch; \
        make run-user XARCH=$arch O=$PWD/kernel-$arch RUN_OUT=$PWD/run.$arch.out | grep "status: "; \
      done
    165 test(s): 157 passed,   8 skipped,   0 failed => status: warning
    165 test(s): 157 passed,   8 skipped,   0 failed => status: warning
    165 test(s): 157 passed,   8 skipped,   0 failed => status: warning

    // powerpc run
    $ for arch in powerpc powerpc64 powerpc64le; do \
        rm -rf $PWD/kernel-$arch; \
        mkdir -p $PWD/kernel-$arch; \
        make tinyconfig run XARCH=$arch O=$PWD/kernel-$arch RUN_OUT=$PWD/run.$arch.out; \
      done

    $ for arch in powerpc powerpc64 powerpc64le; do \
        make report XARCH=$arch O=$PWD/kernel-$arch RUN_OUT=$PWD/run.$arch.out | grep "status: "; \
      done
    165 test(s): 156 passed,   9 skipped,   0 failed => status: warning
    165 test(s): 156 passed,   9 skipped,   0 failed => status: warning
    165 test(s): 156 passed,   9 skipped,   0 failed => status: warning

    // the others, randomly choose some
    $ make run-user XARCH=arm O=$PWD/kernel-arm RUN_OUT=$PWD/run.arm.out CROSS_COMPILE=arm-linux-gnueabi- | grep status:
    165 test(s): 156 passed,   9 skipped,   0 failed => status: warning
    $ make run-user XARCH=x86_64 O=$PWD/kernel-arm RUN_OUT=$PWD/run.x86_64.out CROSS_COMPILE=x86_64-linux-gnu- | grep status:
    165 test(s): 157 passed,   8 skipped,   0 failed => status: warning
    $ make run-libc-test | grep status:
    165 test(s): 153 passed,  12 skipped,   0 failed => status: warning 

    // x86_64, require noapic kernel command line option for old qemu-system-x86_64 (v4.2.1)
    $ make run XARCH=x86_64 O=$PWD/kernel-x86_64 RUN_OUT=$PWD/run.x86_64.out CROSS_COMPILE=x86_64-linux-gnu- | grep status
    $ make rerun XARCH=x86_64 O=$PWD/kernel-x86_64 RUN_OUT=$PWD/run.x86_64.out CROSS_COMPILE=x86_64-linux-gnu- | grep status
    165 test(s): 159 passed,   6 skipped,   0 failed => status: warning

tinyconfig mainly targets as a time-saver, the misc preparations service
for the same goal, let's take a look:

* selftests/nolibc: allow report with existing test log

    Like rerun without rebuild, Add report (without rerun) to summarize
    the existing test log, this may work perfectly with the 'grep status'

* selftests/nolibc: add macros to enhance maintainability

    Several macros are added to dedup the shared code to shrink lines
    and easier the maintainability

    The macros are added just before the using area to avoid code change
    conflicts in the future.

* selftests/nolibc: print running log to screen

    Enable logging to let developers learn what is happening at the
    first glance, without the need to edit the Makefile and rerun it.

    These helps a lot when there is a long-time running, a failed
    poweroff or even a forever hang.

    For test summmary, the 'grep status' can be used together with the
    standalone report target.

* selftests/nolibc: fix up O= option support

    With objtree instead srctree for .config and IMAGE, now, O= works.

    Using O=$PWD/kernel-$arch avoid the mrproer for every build.

* selftests/nolibc: add menuconfig for development

    Allow manually tuning some options, mainly for a new architecture
    porting.

* selftests/nolibc: add mrproper for development
  selftests/nolibc: defconfig: remove mrproper target

    Split the mrproper target out of defconfig, when with O=, mrproper is not
    required by defconfig, but reserve it for the other use scenes.

* selftests/nolibc: string the core targets

    Allow simply 'make run' instead of 'make defconfig; make extconfig;
    make kernel; make run'.

* selftests/nolibc: allow quit qemu-system when poweroff fails

    When poweroff fails, allow exit while detects the power off string
    from output or the wait time is too long (specified by QEMU_TIMEOUT).

    This helps the boards who have no poweroff support or the kernel not
    enable the poweroff options (mainly for tinyconfig).

* selftests/nolibc: allow customize CROSS_COMPILE by architecture
* selftests/nolibc: customize CROSS_COMPILE for 32/64-bit powerpc

    This further saves a CROSS_COMPILE option for 'make run', it is very
    important when iterates all of the supported architectures and the
    compilers are not just prefixed with the XARCH variable.

    For example, binary of big endian powerpc64 can be compiled with
    powerpc64le-linux-gnu-, but the prefix is powerpc64le.

    Even if the pre-customized compiler not exist, we can configure
    CROSS_COMPILE_<ARCH> before the test loop to use the code.

* selftests/nolibc: add tinyconfig target
  selftests/nolibc: tinyconfig: add extra common options
  selftests/nolibc: tinyconfig: add support for 32/64-bit powerpc

    Here is the first architecture(and its variants) support tinyconfig.

    powerpc is actually a very good architecture, for it has 'various'
    variants for test.

Best regards,
Zhangjin
---
[1]: https://lore.kernel.org/lkml/cover.1687706332.git.falcon@tinylab.org/
[2]: https://lore.kernel.org/lkml/cover.1689713175.git.falcon@tinylab.org/


Zhangjin Wu (14):
  selftests/nolibc: allow report with existing test log
  selftests/nolibc: add macros to enhance maintainability
  selftests/nolibc: print running log to screen
  selftests/nolibc: fix up O= option support
  selftests/nolibc: add menuconfig for development
  selftests/nolibc: add mrproper for development
  selftests/nolibc: defconfig: remove mrproper target
  selftests/nolibc: string the core targets
  selftests/nolibc: allow quit qemu-system when poweroff fails
  selftests/nolibc: allow customize CROSS_COMPILE by architecture
  selftests/nolibc: customize CROSS_COMPILE for 32/64-bit powerpc
  selftests/nolibc: add tinyconfig target
  selftests/nolibc: tinyconfig: add extra common options
  selftests/nolibc: tinyconfig: add support for 32/64-bit powerpc

 tools/testing/selftests/nolibc/Makefile       | 102 ++++++++++++++----
 .../selftests/nolibc/configs/common.config    |   4 +
 .../selftests/nolibc/configs/powerpc.config   |   3 +
 .../selftests/nolibc/configs/powerpc64.config |   3 +
 .../nolibc/configs/powerpc64le.config         |   4 +
 5 files changed, 98 insertions(+), 18 deletions(-)
 create mode 100644 tools/testing/selftests/nolibc/configs/common.config
 create mode 100644 tools/testing/selftests/nolibc/configs/powerpc64.config
 create mode 100644 tools/testing/selftests/nolibc/configs/powerpc64le.config

-- 
2.25.1

[PATCH v2 01/14] selftests/nolibc: allow report with existing test log

[Zhangjin Wu]

After the tests finish, it is valuable to report and summarize with
existing test log.

This avoid rerun or run the tests again when not necessary.

Signed-off-by: Zhangjin Wu <falcon@tinylab.org>
---
 tools/testing/selftests/nolibc/Makefile | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tools/testing/selftests/nolibc/Makefile b/tools/testing/selftests/nolibc/Makefile
index ab17e0d8b7e2..0cd17de2062c 100644
--- a/tools/testing/selftests/nolibc/Makefile
+++ b/tools/testing/selftests/nolibc/Makefile
@@ -206,6 +206,10 @@ rerun:
 	$(Q)qemu-system-$(QEMU_ARCH) -display none -no-reboot -kernel "$(srctree)/$(IMAGE)" -serial stdio $(QEMU_ARGS) > "$(CURDIR)/run.out"
 	$(Q)$(REPORT) $(CURDIR)/run.out
 
+# report with existing test log
+report:
+	$(Q)$(REPORT_RUN_OUT)
+
 clean:
 	$(call QUIET_CLEAN, sysroot)
 	$(Q)rm -rf sysroot
-- 
2.25.1

[PATCH v2 02/14] selftests/nolibc: add macros to enhance maintainability

[Zhangjin Wu]

The kernel targets share the same kernel make operations, the same
.config file, the same kernel image, add MAKE_KERNEL, KERNEL_CONFIG and
KERNEL_IMAGE for them.

Many targets share the same logging related settings, let's add common
variables RUN_OUT, LOG_OUT and REPORT_RUN_OUT for them.

The qemu run/rerun targets share the same qemu system run command, add
QEMU_SYSTEM_RUN for them.

Signed-off-by: Zhangjin Wu <falcon@tinylab.org>
---
 tools/testing/selftests/nolibc/Makefile | 41 ++++++++++++++++---------
 1 file changed, 27 insertions(+), 14 deletions(-)

diff --git a/tools/testing/selftests/nolibc/Makefile b/tools/testing/selftests/nolibc/Makefile
index 0cd17de2062c..8c531518bb9f 100644
--- a/tools/testing/selftests/nolibc/Makefile
+++ b/tools/testing/selftests/nolibc/Makefile
@@ -166,45 +166,58 @@ endif
 libc-test: nolibc-test.c
 	$(QUIET_CC)$(CC) -o $@ $<
 
+# common macros for logging
+RUN_OUT = $(CURDIR)/run.out
+LOG_OUT = > "$(RUN_OUT)"
+REPORT_RUN_OUT = $(REPORT) "$(RUN_OUT)"
+
 # local libc-test
 run-libc-test: libc-test
-	$(Q)./libc-test > "$(CURDIR)/run.out" || :
-	$(Q)$(REPORT) $(CURDIR)/run.out
+	$(Q)./libc-test $(LOG_OUT) || :
+	$(Q)$(REPORT_RUN_OUT)
 
 # local nolibc-test
 run-nolibc-test: nolibc-test
-	$(Q)./nolibc-test > "$(CURDIR)/run.out" || :
-	$(Q)$(REPORT) $(CURDIR)/run.out
+	$(Q)./nolibc-test $(LOG_OUT) || :
+	$(Q)$(REPORT_RUN_OUT)
 
 # qemu user-land test
 run-user: nolibc-test
-	$(Q)qemu-$(QEMU_ARCH) ./nolibc-test > "$(CURDIR)/run.out" || :
-	$(Q)$(REPORT) $(CURDIR)/run.out
+	$(Q)qemu-$(QEMU_ARCH) ./nolibc-test $(LOG_OUT) || :
+	$(Q)$(REPORT_RUN_OUT)
 
 initramfs: nolibc-test
 	$(QUIET_MKDIR)mkdir -p initramfs
 	$(call QUIET_INSTALL, initramfs/init)
 	$(Q)cp nolibc-test initramfs/init
 
+# common macros for kernel targets
+MAKE_KERNEL   = $(MAKE) -C $(srctree) ARCH=$(ARCH) CC=$(CC) CROSS_COMPILE=$(CROSS_COMPILE)
+KERNEL_CONFIG = $(srctree)/.config
+KERNEL_IMAGE  = $(srctree)/$(IMAGE)
+
 defconfig:
-	$(Q)$(MAKE) -C $(srctree) ARCH=$(ARCH) CC=$(CC) CROSS_COMPILE=$(CROSS_COMPILE) mrproper $(DEFCONFIG) prepare
+	$(Q)$(MAKE_KERNEL) mrproper $(DEFCONFIG) prepare
 
 extconfig:
-	$(Q)$(srctree)/scripts/kconfig/merge_config.sh -O "$(srctree)" -m "$(srctree)/.config" $(foreach c,$(EXTCONFIG),$(wildcard $(CURDIR)/configs/$c))
-	$(Q)$(MAKE) -C $(srctree) ARCH=$(ARCH) CC=$(CC) CROSS_COMPILE=$(CROSS_COMPILE) KCONFIG_ALLCONFIG="$(srctree)/.config" allnoconfig
+	$(Q)$(srctree)/scripts/kconfig/merge_config.sh -O "$(srctree)" -m "$(KERNEL_CONFIG)" $(foreach c,$(EXTCONFIG),$(wildcard $(CURDIR)/configs/$c))
+	$(Q)$(MAKE_KERNEL) KCONFIG_ALLCONFIG="$(KERNEL_CONFIG)" allnoconfig
 
 kernel: initramfs
-	$(Q)$(MAKE) -C $(srctree) ARCH=$(ARCH) CC=$(CC) CROSS_COMPILE=$(CROSS_COMPILE) $(IMAGE_NAME) CONFIG_INITRAMFS_SOURCE=$(CURDIR)/initramfs
+	$(Q)$(MAKE_KERNEL) $(IMAGE_NAME) CONFIG_INITRAMFS_SOURCE=$(CURDIR)/initramfs
+
+# common macros for qemu run/rerun targets
+QEMU_SYSTEM_RUN = qemu-system-$(QEMU_ARCH) -display none -no-reboot -kernel "$(KERNEL_IMAGE)" -serial stdio $(QEMU_ARGS)
 
 # run the tests after building the kernel
 run: kernel
-	$(Q)qemu-system-$(QEMU_ARCH) -display none -no-reboot -kernel "$(srctree)/$(IMAGE)" -serial stdio $(QEMU_ARGS) > "$(CURDIR)/run.out"
-	$(Q)$(REPORT) $(CURDIR)/run.out
+	$(Q)$(QEMU_SYSTEM_RUN) $(LOG_OUT)
+	$(Q)$(REPORT_RUN_OUT)
 
 # re-run the tests from an existing kernel
 rerun:
-	$(Q)qemu-system-$(QEMU_ARCH) -display none -no-reboot -kernel "$(srctree)/$(IMAGE)" -serial stdio $(QEMU_ARGS) > "$(CURDIR)/run.out"
-	$(Q)$(REPORT) $(CURDIR)/run.out
+	$(Q)$(QEMU_SYSTEM_RUN) $(LOG_OUT)
+	$(Q)$(REPORT_RUN_OUT)
 
 # report with existing test log
 report:
-- 
2.25.1

[PATCH v2 03/14] selftests/nolibc: print running log to screen

[Zhangjin Wu]

When poweroff fails, qemu-system will hang there without any output.

It is very hard to debug in such case, let's print the running log to
the screen to allow users to learn what is happening at the first
glance, without editing the Makefile manually every time.

To get a clean output, the 'grep status' command can be used.

Signed-off-by: Zhangjin Wu <falcon@tinylab.org>
---
 tools/testing/selftests/nolibc/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/nolibc/Makefile b/tools/testing/selftests/nolibc/Makefile
index 8c531518bb9f..f1c8e4a0f1b2 100644
--- a/tools/testing/selftests/nolibc/Makefile
+++ b/tools/testing/selftests/nolibc/Makefile
@@ -168,7 +168,7 @@ libc-test: nolibc-test.c
 
 # common macros for logging
 RUN_OUT = $(CURDIR)/run.out
-LOG_OUT = > "$(RUN_OUT)"
+LOG_OUT = | tee "$(RUN_OUT)"
 REPORT_RUN_OUT = $(REPORT) "$(RUN_OUT)"
 
 # local libc-test
-- 
2.25.1

[PATCH v2 04/14] selftests/nolibc: fix up O= option support

[Zhangjin Wu]

To avoid pollute the source code tree and avoid mrproper for every
architecture switch, the O= argument must be supported.

Both IMAGE and .config are from the building directory, let's use
objtree instead of srctree for them.

If no O= option specified, means building kernel in source code tree,
objtree should be srctree in such case.

Suggested-by: Willy Tarreau <w@1wt.eu>
Link: https://lore.kernel.org/lkml/ZK0AB1OXH1s2xYsh@1wt.eu/
Signed-off-by: Zhangjin Wu <falcon@tinylab.org>
---
 tools/testing/selftests/nolibc/Makefile | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/nolibc/Makefile b/tools/testing/selftests/nolibc/Makefile
index f1c8e4a0f1b2..058e7be070ea 100644
--- a/tools/testing/selftests/nolibc/Makefile
+++ b/tools/testing/selftests/nolibc/Makefile
@@ -9,6 +9,9 @@ ifeq ($(srctree),)
 srctree := $(patsubst %/tools/testing/selftests/,%,$(dir $(CURDIR)))
 endif
 
+# add objtree for O= argument, required by IMAGE and .config
+objtree ?= $(srctree)
+
 ifeq ($(ARCH),)
 include $(srctree)/scripts/subarch.include
 ARCH = $(SUBARCH)
@@ -193,14 +196,14 @@ initramfs: nolibc-test
 
 # common macros for kernel targets
 MAKE_KERNEL   = $(MAKE) -C $(srctree) ARCH=$(ARCH) CC=$(CC) CROSS_COMPILE=$(CROSS_COMPILE)
-KERNEL_CONFIG = $(srctree)/.config
-KERNEL_IMAGE  = $(srctree)/$(IMAGE)
+KERNEL_CONFIG = $(objtree)/.config
+KERNEL_IMAGE  = $(objtree)/$(IMAGE)
 
 defconfig:
 	$(Q)$(MAKE_KERNEL) mrproper $(DEFCONFIG) prepare
 
 extconfig:
-	$(Q)$(srctree)/scripts/kconfig/merge_config.sh -O "$(srctree)" -m "$(KERNEL_CONFIG)" $(foreach c,$(EXTCONFIG),$(wildcard $(CURDIR)/configs/$c))
+	$(Q)$(srctree)/scripts/kconfig/merge_config.sh -O "$(objtree)" -m "$(KERNEL_CONFIG)" $(foreach c,$(EXTCONFIG),$(wildcard $(CURDIR)/configs/$c))
 	$(Q)$(MAKE_KERNEL) KCONFIG_ALLCONFIG="$(KERNEL_CONFIG)" allnoconfig
 
 kernel: initramfs
-- 
2.25.1

[PATCH v2 05/14] selftests/nolibc: add menuconfig for development

[Zhangjin Wu]

The default DEFCONFIG_<ARCH> may not always work for all architectures,
let's allow users to tune some kernel config options with 'menuconfig'.

This is important when porting nolibc to a new architecture, it also
allows to speed up nolibc 'run' target testing via manually disabling
tons of unnecessary kernel config options.

Signed-off-by: Zhangjin Wu <falcon@tinylab.org>
---
 tools/testing/selftests/nolibc/Makefile | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tools/testing/selftests/nolibc/Makefile b/tools/testing/selftests/nolibc/Makefile
index 058e7be070ea..06954b4b3885 100644
--- a/tools/testing/selftests/nolibc/Makefile
+++ b/tools/testing/selftests/nolibc/Makefile
@@ -202,6 +202,9 @@ KERNEL_IMAGE  = $(objtree)/$(IMAGE)
 defconfig:
 	$(Q)$(MAKE_KERNEL) mrproper $(DEFCONFIG) prepare
 
+menuconfig:
+	$(Q)$(MAKE_KERNEL) menuconfig
+
 extconfig:
 	$(Q)$(srctree)/scripts/kconfig/merge_config.sh -O "$(objtree)" -m "$(KERNEL_CONFIG)" $(foreach c,$(EXTCONFIG),$(wildcard $(CURDIR)/configs/$c))
 	$(Q)$(MAKE_KERNEL) KCONFIG_ALLCONFIG="$(KERNEL_CONFIG)" allnoconfig
-- 
2.25.1

[PATCH v2 06/14] selftests/nolibc: add mrproper for development

[Zhangjin Wu]

When want to start a new building with O= option, the old generated
files in the source code tree must be mrproper-ed, adding mrproper
target here to avoid switch to the source code tree manually.

Signed-off-by: Zhangjin Wu <falcon@tinylab.org>
---
 tools/testing/selftests/nolibc/Makefile | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tools/testing/selftests/nolibc/Makefile b/tools/testing/selftests/nolibc/Makefile
index 06954b4b3885..9d9902b54e5e 100644
--- a/tools/testing/selftests/nolibc/Makefile
+++ b/tools/testing/selftests/nolibc/Makefile
@@ -199,6 +199,9 @@ MAKE_KERNEL   = $(MAKE) -C $(srctree) ARCH=$(ARCH) CC=$(CC) CROSS_COMPILE=$(CROS
 KERNEL_CONFIG = $(objtree)/.config
 KERNEL_IMAGE  = $(objtree)/$(IMAGE)
 
+mrproper:
+	$(Q)$(MAKE_KERNEL) mrproper
+
 defconfig:
 	$(Q)$(MAKE_KERNEL) mrproper $(DEFCONFIG) prepare
 
-- 
2.25.1

[PATCH v2 07/14] selftests/nolibc: defconfig: remove mrproper target

[Zhangjin Wu]

The O=/path/to/kernel-<ARCH> option allows to build kernel for different
architectures in different output directories, in this scene, it doesn't
need the mrproper operation for defconfig anymore.

If really require to clean up the source code tree, let users run the
standalone mrproper target on demand.

Signed-off-by: Zhangjin Wu <falcon@tinylab.org>
---
 tools/testing/selftests/nolibc/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/nolibc/Makefile b/tools/testing/selftests/nolibc/Makefile
index 9d9902b54e5e..83cb4b017bef 100644
--- a/tools/testing/selftests/nolibc/Makefile
+++ b/tools/testing/selftests/nolibc/Makefile
@@ -203,7 +203,7 @@ mrproper:
 	$(Q)$(MAKE_KERNEL) mrproper
 
 defconfig:
-	$(Q)$(MAKE_KERNEL) mrproper $(DEFCONFIG) prepare
+	$(Q)$(MAKE_KERNEL) $(DEFCONFIG) prepare
 
 menuconfig:
 	$(Q)$(MAKE_KERNEL) menuconfig
-- 
2.25.1

[PATCH v2 08/14] selftests/nolibc: string the core targets

[Zhangjin Wu]

To avoid run targets one by one manually and boringly, let's string them
with IMAGE and .config, the MAKE command will trigger the dependencies
for us.

Note, to allow do menuconfig before extconfig manually, only trigger
defconfig while the .config is not there, it means only trigger
defconfig for the first run or after a mrproper.

Signed-off-by: Zhangjin Wu <falcon@tinylab.org>
---
 tools/testing/selftests/nolibc/Makefile | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/tools/testing/selftests/nolibc/Makefile b/tools/testing/selftests/nolibc/Makefile
index 83cb4b017bef..541f3565e584 100644
--- a/tools/testing/selftests/nolibc/Makefile
+++ b/tools/testing/selftests/nolibc/Makefile
@@ -150,6 +150,7 @@ all: run
 
 sysroot: sysroot/$(ARCH)/include
 
+PHONY = sysroot/$(ARCH)/include
 sysroot/$(ARCH)/include:
 	$(Q)rm -rf sysroot/$(ARCH) sysroot/sysroot
 	$(QUIET_MKDIR)mkdir -p sysroot
@@ -205,21 +206,28 @@ mrproper:
 defconfig:
 	$(Q)$(MAKE_KERNEL) $(DEFCONFIG) prepare
 
-menuconfig:
+PHONY += $(KERNEL_CONFIG)
+$(KERNEL_CONFIG):
+	$(Q)if [ ! -f "$(KERNEL_CONFIG)" ]; then $(MAKE) --no-print-directory defconfig; fi
+
+menuconfig: $(KERNEL_CONFIG)
 	$(Q)$(MAKE_KERNEL) menuconfig
 
-extconfig:
+extconfig: $(KERNEL_CONFIG)
 	$(Q)$(srctree)/scripts/kconfig/merge_config.sh -O "$(objtree)" -m "$(KERNEL_CONFIG)" $(foreach c,$(EXTCONFIG),$(wildcard $(CURDIR)/configs/$c))
 	$(Q)$(MAKE_KERNEL) KCONFIG_ALLCONFIG="$(KERNEL_CONFIG)" allnoconfig
 
-kernel: initramfs
+kernel: extconfig
+	$(Q)$(MAKE) --no-print-directory initramfs
 	$(Q)$(MAKE_KERNEL) $(IMAGE_NAME) CONFIG_INITRAMFS_SOURCE=$(CURDIR)/initramfs
 
 # common macros for qemu run/rerun targets
 QEMU_SYSTEM_RUN = qemu-system-$(QEMU_ARCH) -display none -no-reboot -kernel "$(KERNEL_IMAGE)" -serial stdio $(QEMU_ARGS)
 
 # run the tests after building the kernel
-run: kernel
+PHONY += $(KERNEL_IMAGE)
+$(KERNEL_IMAGE): kernel
+run: $(KERNEL_IMAGE)
 	$(Q)$(QEMU_SYSTEM_RUN) $(LOG_OUT)
 	$(Q)$(REPORT_RUN_OUT)
 
@@ -244,4 +252,4 @@ clean:
 	$(call QUIET_CLEAN, run.out)
 	$(Q)rm -rf run.out
 
-.PHONY: sysroot/$(ARCH)/include
+.PHONY: $(PHONY)
-- 
2.25.1

[PATCH v2 09/14] selftests/nolibc: allow quit qemu-system when poweroff fails

[Zhangjin Wu]

The kernel of some architectures can not poweroff qemu-system normally,
especially for tinyconfig.

Some architectures may have no kernel poweroff support, the others may
require more kernel config options and therefore slow down the
tinyconfig build and test. and also, it's very hard (and some even not
possible) to find out the exact poweroff related kernel config options
for every architecture.

Since the low-level poweroff support is heavily kernel & qemu dependent,
it is not that critical to both nolibc and nolibc-test, let's simply
ignore the poweroff required kernel config options for tinyconfig (and
even for defconfig) and quit qemu-system after a specified timeout or
with an expected system halt or poweroff string (these strings mean our
reboot() library routine is perfectly ok).

QEMU_TIMEOUT value can be configured for every architecture based on
their time cost requirement of boot+test+poweroff.

Signed-off-by: Zhangjin Wu <falcon@tinylab.org>
---
 tools/testing/selftests/nolibc/Makefile | 23 +++++++++++++++++++++--
 1 file changed, 21 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/nolibc/Makefile b/tools/testing/selftests/nolibc/Makefile
index 541f3565e584..a03fab020ebe 100644
--- a/tools/testing/selftests/nolibc/Makefile
+++ b/tools/testing/selftests/nolibc/Makefile
@@ -93,6 +93,9 @@ QEMU_ARGS_s390       = -M s390-ccw-virtio -m 1G -append "console=ttyS0 panic=-1
 QEMU_ARGS_loongarch  = -M virt -append "console=ttyS0,115200 panic=-1 $(TEST:%=NOLIBC_TEST=%)"
 QEMU_ARGS            = $(QEMU_ARGS_$(XARCH)) $(QEMU_ARGS_EXTRA)
 
+# QEMU_TIMEOUT: some architectures can not poweroff normally, especially for tinyconfig
+QEMU_TIMEOUT             = $(QEMU_TIMEOUT_$(XARCH))
+
 # OUTPUT is only set when run from the main makefile, otherwise
 # it defaults to this nolibc directory.
 OUTPUT ?= $(CURDIR)/
@@ -224,16 +227,32 @@ kernel: extconfig
 # common macros for qemu run/rerun targets
 QEMU_SYSTEM_RUN = qemu-system-$(QEMU_ARCH) -display none -no-reboot -kernel "$(KERNEL_IMAGE)" -serial stdio $(QEMU_ARGS)
 
+ifneq ($(QEMU_TIMEOUT),)
+TIMEOUT_CMD = t=$(QEMU_TIMEOUT); \
+	while [ $$t -gt 0 ]; do                                                       \
+	    sleep 5; t=$$(expr $$t - 5); echo "detecting power off ...";              \
+	    if grep -qE "reboot: System halted|reboot: Power down" "$(RUN_OUT)"; then \
+		pkill -9 qemu-system-$(QEMU_ARCH);                                    \
+		echo "powered off, test finish"; t=1; break;                          \
+	    fi;                                                                       \
+	done;                                                                         \
+	if [ $$t -le 0 ]; then pkill -9 qemu-system-$(QEMU_ARCH); echo "qemu-system-$(QEMU_ARCH) timeout"; fi
+
+TIMEOUT_QEMU_RUN = ($(QEMU_SYSTEM_RUN) $(LOG_OUT) &); $(TIMEOUT_CMD)
+else
+TIMEOUT_QEMU_RUN = $(QEMU_SYSTEM_RUN) $(LOG_OUT)
+endif
+
 # run the tests after building the kernel
 PHONY += $(KERNEL_IMAGE)
 $(KERNEL_IMAGE): kernel
 run: $(KERNEL_IMAGE)
-	$(Q)$(QEMU_SYSTEM_RUN) $(LOG_OUT)
+	$(Q)$(TIMEOUT_QEMU_RUN)
 	$(Q)$(REPORT_RUN_OUT)
 
 # re-run the tests from an existing kernel
 rerun:
-	$(Q)$(QEMU_SYSTEM_RUN) $(LOG_OUT)
+	$(Q)$(TIMEOUT_QEMU_RUN)
 	$(Q)$(REPORT_RUN_OUT)
 
 # report with existing test log
-- 
2.25.1

[PATCH v2 10/14] selftests/nolibc: allow customize CROSS_COMPILE by architecture

[Zhangjin Wu]

Some cross compilers may not just be prefixed with ARCH or XARCH,
customize them by architecture may easier the test a lot, especially,
when iterate with XARCH or ARCH.

After customizing this for every architecture, the minimal test argument
will be architecture itself, no CROSS_COMPILE will be passed.

If the installed cross compiler is not the same as the one customized,
we can also pass CROSS_COMPILE from command line as before, no
regression.

Signed-off-by: Zhangjin Wu <falcon@tinylab.org>
---
 tools/testing/selftests/nolibc/Makefile | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/tools/testing/selftests/nolibc/Makefile b/tools/testing/selftests/nolibc/Makefile
index a03fab020ebe..3c2be27747ea 100644
--- a/tools/testing/selftests/nolibc/Makefile
+++ b/tools/testing/selftests/nolibc/Makefile
@@ -42,6 +42,12 @@ IMAGE_loongarch  = arch/loongarch/boot/vmlinuz.efi
 IMAGE            = $(IMAGE_$(XARCH))
 IMAGE_NAME       = $(notdir $(IMAGE))
 
+# CROSS_COMPILE: cross toolchain prefix by architecture
+CROSS_COMPILE             ?= $(CROSS_COMPILE_$(XARCH))
+
+# make sure CC is prefixed with CROSS_COMPILE
+$(call allow-override,CC,$(CROSS_COMPILE)gcc)
+
 # default kernel configurations that appear to be usable
 DEFCONFIG_i386       = defconfig
 DEFCONFIG_x86_64     = defconfig
-- 
2.25.1

[PATCH v2 11/14] selftests/nolibc: customize CROSS_COMPILE for 32/64-bit powerpc

[Zhangjin Wu]

The little-endian powerpc64le compilers provided by Ubuntu and Fedora
are able to compile big endian kernel and big endian nolibc-test [1].

These default CROSS_COMPILE settings allow us to test target
architectures with:

    $ cd /path/to/tools/testing/selftests/nolibc/

    $ for arch in powerpc powerpc64 powerpc64le; do \
        make run-user XARCH=$arch | grep "status: "; \
      done

If want to use another cross compiler, please simply pass CROSS_COMPILE
or CC as before.

For example, it is able to build 64-bit nolibc-test with the big endian
powerpc64-linux-gcc crosstool from [2]:

    $ wget -c https://mirrors.edge.kernel.org/pub/tools/crosstool/files/bin/x86_64/13.1.0/x86_64-gcc-13.1.0-nolibc-powerpc64-linux.tar.xz
    $ tar xvf x86_64-gcc-13.1.0-nolibc-powerpc64-linux.tar.xz
    $ export PATH=$PWD/gcc-13.1.0-nolibc/powerpc64-linux/bin/:$PATH

    $ export CROSS_COMPILE_powerpc64=powerpc64-linux-
    $ export CROSS_COMPILE_powerpc64le=powerpc64-linux-
    $ for arch in powerpc64 powerpc64le; do \
        make run-user XARCH=$arch | grep "status: "; \
      done

Or specify CC directly with full path:

    $ export CC=$PWD/gcc-13.1.0-nolibc/powerpc64-linux/bin/powerpc64-linux-gcc
    $ for arch in powerpc64 powerpc64le; do \
        make run-user XARCH=$arch | grep "status: "; \
      done

[1]: https://github.com/open-power/skiboot
[2]: https://mirrors.edge.kernel.org/pub/tools/crosstool/

Signed-off-by: Zhangjin Wu <falcon@tinylab.org>
---
 tools/testing/selftests/nolibc/Makefile | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tools/testing/selftests/nolibc/Makefile b/tools/testing/selftests/nolibc/Makefile
index 3c2be27747ea..eec2935672ad 100644
--- a/tools/testing/selftests/nolibc/Makefile
+++ b/tools/testing/selftests/nolibc/Makefile
@@ -43,6 +43,9 @@ IMAGE            = $(IMAGE_$(XARCH))
 IMAGE_NAME       = $(notdir $(IMAGE))
 
 # CROSS_COMPILE: cross toolchain prefix by architecture
+CROSS_COMPILE_powerpc     ?= powerpc-linux-gnu-
+CROSS_COMPILE_powerpc64   ?= powerpc64le-linux-gnu-
+CROSS_COMPILE_powerpc64le ?= powerpc64le-linux-gnu-
 CROSS_COMPILE             ?= $(CROSS_COMPILE_$(XARCH))
 
 # make sure CC is prefixed with CROSS_COMPILE
-- 
2.25.1

[PATCH v2 12/14] selftests/nolibc: add tinyconfig target

[Zhangjin Wu]

The original tinyconfig target only enables minimal kernel config
options, it can speed up the kernel build and nolibc test a lot and also
brings us with smaller kernel image size.

But the default enabled options are not enough for qemu boot and console
print, some additional config options should be added for every
architecture individually.

Signed-off-by: Zhangjin Wu <falcon@tinylab.org>
---
 tools/testing/selftests/nolibc/Makefile | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tools/testing/selftests/nolibc/Makefile b/tools/testing/selftests/nolibc/Makefile
index eec2935672ad..f42782fa78a9 100644
--- a/tools/testing/selftests/nolibc/Makefile
+++ b/tools/testing/selftests/nolibc/Makefile
@@ -218,6 +218,9 @@ mrproper:
 defconfig:
 	$(Q)$(MAKE_KERNEL) $(DEFCONFIG) prepare
 
+tinyconfig:
+	$(Q)$(MAKE_KERNEL) tinyconfig prepare
+
 PHONY += $(KERNEL_CONFIG)
 $(KERNEL_CONFIG):
 	$(Q)if [ ! -f "$(KERNEL_CONFIG)" ]; then $(MAKE) --no-print-directory defconfig; fi
-- 
2.25.1

[PATCH v2 13/14] selftests/nolibc: tinyconfig: add extra common options

[Zhangjin Wu]

The original kernel tinyconfig target has already enabled some common
options, but they are not enough to enable boot and print.

    $ find kernel/ arch/*/ -name "tiny*.config"
    kernel/configs/tiny-base.config
    kernel/configs/tiny.config
    arch/x86/configs/tiny.config

To enable qemu boot and console print, additional kernel config options
are required, include the common parts and the architecture specific
parts.

Here adds minimal extra common parts for all architectures:

* for initrd: CONFIG_BLK_DEV_INITRD
* for init executable: CONFIG_BINFMT_ELF
* for test result print: CONFIG_PRINTK, CONFIG_TTY

Signed-off-by: Zhangjin Wu <falcon@tinylab.org>
---
 tools/testing/selftests/nolibc/configs/common.config | 4 ++++
 1 file changed, 4 insertions(+)
 create mode 100644 tools/testing/selftests/nolibc/configs/common.config

diff --git a/tools/testing/selftests/nolibc/configs/common.config b/tools/testing/selftests/nolibc/configs/common.config
new file mode 100644
index 000000000000..3957f812faac
--- /dev/null
+++ b/tools/testing/selftests/nolibc/configs/common.config
@@ -0,0 +1,4 @@
+CONFIG_BLK_DEV_INITRD=y
+CONFIG_BINFMT_ELF=y
+CONFIG_PRINTK=y
+CONFIG_TTY=y
-- 
2.25.1

[PATCH v2 14/14] selftests/nolibc: tinyconfig: add support for 32/64-bit powerpc

[Zhangjin Wu]

Firstly, add extra config files for powerpc, powerpc64le and powerpc64.

Second, QEMU_TIMEOUT is configured as 60 seconds for powerpc to allow
quit qemu-system-ppc even if poweroff fails. In normal host machine, ~20
seconds may be enough for boot+test+poweroff, but 60 seconds is used
here to gurantee it at least finish even in a very slow host machine or
the host machine is too busy. Both powerpc64le and powerpc64 can
poweroff normally, no need to configure QEMU_TIMEOUT for them.

It is able to use tinyconfig as the minimal config target to speed up
the run target of powerpc:

    $ for arch in powerpc powerpc64 powerpc64le; do \
        rm -rf $PWD/kernel-$arch; \
        mkdir -p $PWD/kernel-$arch; \
        make tinyconfig run XARCH=$arch O=$PWD/kernel-$arch | grep status ; \
      done

rerun with architecture specific run.out (for later report):

    $ for arch in powerpc powerpc64 powerpc64le; do \
        mkdir -p $PWD/kernel-$arch; \
        make rerun XARCH=$arch O=$PWD/kernel-$arch RUN_OUT=$PWD/run.$arch.out | grep status ; \
      done

report:

    $ for arch in powerpc powerpc64 powerpc64le; do \
        make report RUN_OUT=$PWD/run.$arch.out | grep status ; \
      done

Signed-off-by: Zhangjin Wu <falcon@tinylab.org>
---
 tools/testing/selftests/nolibc/Makefile                   | 1 +
 tools/testing/selftests/nolibc/configs/powerpc.config     | 3 +++
 tools/testing/selftests/nolibc/configs/powerpc64.config   | 3 +++
 tools/testing/selftests/nolibc/configs/powerpc64le.config | 4 ++++
 4 files changed, 11 insertions(+)
 create mode 100644 tools/testing/selftests/nolibc/configs/powerpc64.config
 create mode 100644 tools/testing/selftests/nolibc/configs/powerpc64le.config

diff --git a/tools/testing/selftests/nolibc/Makefile b/tools/testing/selftests/nolibc/Makefile
index f42782fa78a9..b01346323e35 100644
--- a/tools/testing/selftests/nolibc/Makefile
+++ b/tools/testing/selftests/nolibc/Makefile
@@ -103,6 +103,7 @@ QEMU_ARGS_loongarch  = -M virt -append "console=ttyS0,115200 panic=-1 $(TEST:%=N
 QEMU_ARGS            = $(QEMU_ARGS_$(XARCH)) $(QEMU_ARGS_EXTRA)
 
 # QEMU_TIMEOUT: some architectures can not poweroff normally, especially for tinyconfig
+QEMU_TIMEOUT_powerpc     = 60
 QEMU_TIMEOUT             = $(QEMU_TIMEOUT_$(XARCH))
 
 # OUTPUT is only set when run from the main makefile, otherwise
diff --git a/tools/testing/selftests/nolibc/configs/powerpc.config b/tools/testing/selftests/nolibc/configs/powerpc.config
index b1975f8253f7..29123cee14c4 100644
--- a/tools/testing/selftests/nolibc/configs/powerpc.config
+++ b/tools/testing/selftests/nolibc/configs/powerpc.config
@@ -1,3 +1,6 @@
+CONFIG_COMPAT_32BIT_TIME=y
+CONFIG_PPC_PMAC=y
+CONFIG_PPC_OF_BOOT_TRAMPOLINE=y
 CONFIG_SERIAL_PMACZILOG=y
 CONFIG_SERIAL_PMACZILOG_TTYS=y
 CONFIG_SERIAL_PMACZILOG_CONSOLE=y
diff --git a/tools/testing/selftests/nolibc/configs/powerpc64.config b/tools/testing/selftests/nolibc/configs/powerpc64.config
new file mode 100644
index 000000000000..4e17f0cdb99f
--- /dev/null
+++ b/tools/testing/selftests/nolibc/configs/powerpc64.config
@@ -0,0 +1,3 @@
+CONFIG_PPC64=y
+CONFIG_PPC_POWERNV=y
+CONFIG_HVC_OPAL=y
diff --git a/tools/testing/selftests/nolibc/configs/powerpc64le.config b/tools/testing/selftests/nolibc/configs/powerpc64le.config
new file mode 100644
index 000000000000..713b227f506f
--- /dev/null
+++ b/tools/testing/selftests/nolibc/configs/powerpc64le.config
@@ -0,0 +1,4 @@
+CONFIG_PPC64=y
+CONFIG_PPC_POWERNV=y
+CONFIG_HVC_OPAL=y
+CONFIG_CPU_LITTLE_ENDIAN=y
-- 
2.25.1

Re: [PATCH v2 02/14] selftests/nolibc: add macros to enhance maintainability

[Willy Tarreau]

On Wed, Jul 19, 2023 at 09:19:10PM +0800, Zhangjin Wu wrote:
> The kernel targets share the same kernel make operations, the same
> .config file, the same kernel image, add MAKE_KERNEL, KERNEL_CONFIG and
> KERNEL_IMAGE for them.
> 
> Many targets share the same logging related settings, let's add common
> variables RUN_OUT, LOG_OUT and REPORT_RUN_OUT for them.
> 
> The qemu run/rerun targets share the same qemu system run command, add
> QEMU_SYSTEM_RUN for them.
> 
> Signed-off-by: Zhangjin Wu <falcon@tinylab.org>
> ---
>  tools/testing/selftests/nolibc/Makefile | 41 ++++++++++++++++---------
>  1 file changed, 27 insertions(+), 14 deletions(-)
> 
> diff --git a/tools/testing/selftests/nolibc/Makefile b/tools/testing/selftests/nolibc/Makefile
> index 0cd17de2062c..8c531518bb9f 100644
> --- a/tools/testing/selftests/nolibc/Makefile
> +++ b/tools/testing/selftests/nolibc/Makefile
> @@ -166,45 +166,58 @@ endif
>  libc-test: nolibc-test.c
>  	$(QUIET_CC)$(CC) -o $@ $<
>  
> +# common macros for logging
> +RUN_OUT = $(CURDIR)/run.out
> +LOG_OUT = > "$(RUN_OUT)"
> +REPORT_RUN_OUT = $(REPORT) "$(RUN_OUT)"
> +
>  # local libc-test
>  run-libc-test: libc-test
> -	$(Q)./libc-test > "$(CURDIR)/run.out" || :
> -	$(Q)$(REPORT) $(CURDIR)/run.out
> +	$(Q)./libc-test $(LOG_OUT) || :
> +	$(Q)$(REPORT_RUN_OUT)

Sorry but I don't find that this improves maintainability, quite the
opposite in fact. One reason is that you never visually expect that
some shell indirection delimiters are hidden in a macro that seems
to only convey a name. Sure there are valid use cases for this, but
I think that here it's just adding too much abstraction and it makes
it quite hard to unfold all of this mentally.

Please try to keep the number of macros to the minimum that needs to
be replaced or forced by the user. Here I'm not seeing a compelling
reason for a user to want to force LOG_OUT to something else. Also
makefile macros are generally a pain to debug, which is another
reason for not putting too many of them.

I noticed that your next patch changes LOG_OUT to tee, it could have
done it everywhere and wouldn't affect readability as much.

Thanks,
Willy

Re: [PATCH v2 03/14] selftests/nolibc: print running log to screen

[Willy Tarreau]

On Wed, Jul 19, 2023 at 09:20:17PM +0800, Zhangjin Wu wrote:
> When poweroff fails, qemu-system will hang there without any output.
> 
> It is very hard to debug in such case, let's print the running log to
> the screen to allow users to learn what is happening at the first
> glance, without editing the Makefile manually every time.
> 
> To get a clean output, the 'grep status' command can be used.

The problem with doing this is that it rolls back to the initial
version that breaks with qemu. When its stdout is sent to a pipe, we've
found that the output got randomly mangled and/or missing contents.
It's only when sent to a file that it's OK. I suspect it has something
to do with non-blocking writes being used to avoid blocking the
emulation but I could be totally wrong. That's the reason why we had
to switch to a file.

And I'd rather avoid starting it in the background as well. Maybe
you'd want to run the qemu command under the "timeout" one ? That
could be better than nothing.

Willy

Re: [PATCH v2 05/14] selftests/nolibc: add menuconfig for development

[Willy Tarreau]

On Wed, Jul 19, 2023 at 09:22:37PM +0800, Zhangjin Wu wrote:
> The default DEFCONFIG_<ARCH> may not always work for all architectures,
> let's allow users to tune some kernel config options with 'menuconfig'.
> 
> This is important when porting nolibc to a new architecture, it also
> allows to speed up nolibc 'run' target testing via manually disabling
> tons of unnecessary kernel config options.
> 
> Signed-off-by: Zhangjin Wu <falcon@tinylab.org>
> ---
>  tools/testing/selftests/nolibc/Makefile | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/tools/testing/selftests/nolibc/Makefile b/tools/testing/selftests/nolibc/Makefile
> index 058e7be070ea..06954b4b3885 100644
> --- a/tools/testing/selftests/nolibc/Makefile
> +++ b/tools/testing/selftests/nolibc/Makefile
> @@ -202,6 +202,9 @@ KERNEL_IMAGE  = $(objtree)/$(IMAGE)
>  defconfig:
>  	$(Q)$(MAKE_KERNEL) mrproper $(DEFCONFIG) prepare
>  
> +menuconfig:
> +	$(Q)$(MAKE_KERNEL) menuconfig

What is the real benefit of this compared to just running the
standard "make menuconfig" ? We should avoid adding makefile targets
that do not bring value on top of that the top-level makefile does,
because it will make the useful ones much harder to spot, and will
tend to encourage users to use only that makefile during the tests,
which is a bad practice since many targets will always be missing
or work differently. It's important in my opinion that we strictly
stick to what is useful to operate the tests themselves and this
one doesn't make me feel like it qualifies as such.

Willy

Re: [PATCH v2 06/14] selftests/nolibc: add mrproper for development

[Willy Tarreau]

On Wed, Jul 19, 2023 at 09:23:48PM +0800, Zhangjin Wu wrote:
> +mrproper:
> +	$(Q)$(MAKE_KERNEL) mrproper
> +

Same as previous one, I'm not convinced about the benefit at all.
As a user I would expect that it cleans everything in the build
directory while in fact it seems it will only apply to the kernel.

Willy

Re: [PATCH v2 07/14] selftests/nolibc: defconfig: remove mrproper target

[Willy Tarreau]

On Wed, Jul 19, 2023 at 09:24:54PM +0800, Zhangjin Wu wrote:
> The O=/path/to/kernel-<ARCH> option allows to build kernel for different
> architectures in different output directories, in this scene, it doesn't
> need the mrproper operation for defconfig anymore.
> 
> If really require to clean up the source code tree, let users run the
> standalone mrproper target on demand.

But that's precisely what's going to make it more and more annoying
to run simple tests. The mrproper was there precisely because one
config at a time was being used, so without it we'll restart to see
plenty of failures as it used to be before mrproper was added. I
understand what you're trying to do with the O=, but then if you're
already passing "O=", why not also pass "defconfig" ?

I mean, let's make sure this makefile is only used to manipulate
the tests. It must not become a huge wrapper for the kernel makefile
otherwise it will become extremely complicated to use to run just a
simple test. And with this one and the last few ones, I'm starting
to sense that I'll have to read a README to figure how to reliably
run a test.

In my opinion, there are mainly two use cases :
  - user, manually: commands should be short, forgiving to user
    mistakes, and easy to remember. I.e. they're compatible with
    upper-arrow, then enter.

  - scripts: these are the ones already running in loops with tons
    of variables, setting object directories with O=$arch/... and
    taking care of their own cleanups. These ones will already be
    user-specific and can very well accomodate one or two extra
    lines for a make mrproper or make defconfig if needed.

The second ones deserve thinking and control anyway. The first ones
should mostly not fail for a user mistake and in the worst case waste
a bit of their time by rebuilding something that could have been
avoided. But I do want to prioritize the user here. And that's also
why I want the makefile to be easy to read with as few macros as
possible, because once it works for you, it's easy to figure what is
being done, and how to exploit it from your scripts. The opposite is
not true. Nobody reads a makefile full of macros to try to figure how
to run their first test or why a test that worked once now fails.

Thanks,
Willy

Re: [PATCH v2 08/14] selftests/nolibc: string the core targets

[Willy Tarreau]

On Wed, Jul 19, 2023 at 09:26:01PM +0800, Zhangjin Wu wrote:
> To avoid run targets one by one manually and boringly, let's string them
> with IMAGE and .config, the MAKE command will trigger the dependencies
> for us.
> 
> Note, to allow do menuconfig before extconfig manually, only trigger
> defconfig while the .config is not there, it means only trigger
> defconfig for the first run or after a mrproper.
> 
> Signed-off-by: Zhangjin Wu <falcon@tinylab.org>
> ---
>  tools/testing/selftests/nolibc/Makefile | 18 +++++++++++++-----
>  1 file changed, 13 insertions(+), 5 deletions(-)
> 
> diff --git a/tools/testing/selftests/nolibc/Makefile b/tools/testing/selftests/nolibc/Makefile
> index 83cb4b017bef..541f3565e584 100644
> --- a/tools/testing/selftests/nolibc/Makefile
> +++ b/tools/testing/selftests/nolibc/Makefile
(...)
> -extconfig:
> +extconfig: $(KERNEL_CONFIG)
>  	$(Q)$(srctree)/scripts/kconfig/merge_config.sh -O "$(objtree)" -m "$(KERNEL_CONFIG)" $(foreach c,$(EXTCONFIG),$(wildcard $(CURDIR)/configs/$c))
>  	$(Q)$(MAKE_KERNEL) KCONFIG_ALLCONFIG="$(KERNEL_CONFIG)" allnoconfig
>  
> -kernel: initramfs
> +kernel: extconfig
> +	$(Q)$(MAKE) --no-print-directory initramfs

There seems to be something wrong here. From what I'm seeing, now if I
run "make kernel" it will run extconfig and possibly change the config
I just edited.

Or am I missing something ? I must confess all of this is becoming more
and more obscure :-(

Willy

Re: [PATCH v2 09/14] selftests/nolibc: allow quit qemu-system when poweroff fails

[Willy Tarreau]

On Wed, Jul 19, 2023 at 09:27:08PM +0800, Zhangjin Wu wrote:
> The kernel of some architectures can not poweroff qemu-system normally,
> especially for tinyconfig.
> 
> Some architectures may have no kernel poweroff support, the others may
> require more kernel config options and therefore slow down the
> tinyconfig build and test. and also, it's very hard (and some even not
> possible) to find out the exact poweroff related kernel config options
> for every architecture.
> 
> Since the low-level poweroff support is heavily kernel & qemu dependent,
> it is not that critical to both nolibc and nolibc-test, let's simply
> ignore the poweroff required kernel config options for tinyconfig (and
> even for defconfig) and quit qemu-system after a specified timeout or
> with an expected system halt or poweroff string (these strings mean our
> reboot() library routine is perfectly ok).
> 
> QEMU_TIMEOUT value can be configured for every architecture based on
> their time cost requirement of boot+test+poweroff.
> 
> Signed-off-by: Zhangjin Wu <falcon@tinylab.org>
> ---
>  tools/testing/selftests/nolibc/Makefile | 23 +++++++++++++++++++++--
>  1 file changed, 21 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/testing/selftests/nolibc/Makefile b/tools/testing/selftests/nolibc/Makefile
> index 541f3565e584..a03fab020ebe 100644
> --- a/tools/testing/selftests/nolibc/Makefile
> +++ b/tools/testing/selftests/nolibc/Makefile
> @@ -93,6 +93,9 @@ QEMU_ARGS_s390       = -M s390-ccw-virtio -m 1G -append "console=ttyS0 panic=-1
>  QEMU_ARGS_loongarch  = -M virt -append "console=ttyS0,115200 panic=-1 $(TEST:%=NOLIBC_TEST=%)"
>  QEMU_ARGS            = $(QEMU_ARGS_$(XARCH)) $(QEMU_ARGS_EXTRA)
>  
> +# QEMU_TIMEOUT: some architectures can not poweroff normally, especially for tinyconfig
> +QEMU_TIMEOUT             = $(QEMU_TIMEOUT_$(XARCH))
> +
>  # OUTPUT is only set when run from the main makefile, otherwise
>  # it defaults to this nolibc directory.
>  OUTPUT ?= $(CURDIR)/
> @@ -224,16 +227,32 @@ kernel: extconfig
>  # common macros for qemu run/rerun targets
>  QEMU_SYSTEM_RUN = qemu-system-$(QEMU_ARCH) -display none -no-reboot -kernel "$(KERNEL_IMAGE)" -serial stdio $(QEMU_ARGS)
>  
> +ifneq ($(QEMU_TIMEOUT),)
> +TIMEOUT_CMD = t=$(QEMU_TIMEOUT); \
> +	while [ $$t -gt 0 ]; do                                                       \
> +	    sleep 5; t=$$(expr $$t - 5); echo "detecting power off ...";              \
> +	    if grep -qE "reboot: System halted|reboot: Power down" "$(RUN_OUT)"; then \
> +		pkill -9 qemu-system-$(QEMU_ARCH);                                    \
> +		echo "powered off, test finish"; t=1; break;                          \
> +	    fi;                                                                       \
> +	done;                                                                         \
> +	if [ $$t -le 0 ]; then pkill -9 qemu-system-$(QEMU_ARCH); echo "qemu-system-$(QEMU_ARCH) timeout"; fi

Please have a look at the "timeout" command whichi makes all this much
simpler. Also, please get used to never ever use kill -9 first. This
is exactly the way to leave temporary files and IPCs wandering around
while many programs that care about cleanups at least try to do that
upon a regular TERM or INT signal.

Willy

Re: [PATCH v2 12/14] selftests/nolibc: add tinyconfig target

[Willy Tarreau]

On Wed, Jul 19, 2023 at 09:30:30PM +0800, Zhangjin Wu wrote:
> The original tinyconfig target only enables minimal kernel config
> options, it can speed up the kernel build and nolibc test a lot and also
> brings us with smaller kernel image size.
> 
> But the default enabled options are not enough for qemu boot and console
> print, some additional config options should be added for every
> architecture individually.
> 
> Signed-off-by: Zhangjin Wu <falcon@tinylab.org>
> ---
>  tools/testing/selftests/nolibc/Makefile | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/tools/testing/selftests/nolibc/Makefile b/tools/testing/selftests/nolibc/Makefile
> index eec2935672ad..f42782fa78a9 100644
> --- a/tools/testing/selftests/nolibc/Makefile
> +++ b/tools/testing/selftests/nolibc/Makefile
> @@ -218,6 +218,9 @@ mrproper:
>  defconfig:
>  	$(Q)$(MAKE_KERNEL) $(DEFCONFIG) prepare
>  
> +tinyconfig:
> +	$(Q)$(MAKE_KERNEL) tinyconfig prepare

So for the same reasons as defconfig above, I'd actually keep mrproper
here. And if we figure that tinyconfig is never called by the user
directly but as a dependency from the makefile itself or scripts,
then we likely don't even need to create a visible entry for it.

Willy

Re: [PATCH v2 14/14] selftests/nolibc: tinyconfig: add support for 32/64-bit powerpc

[Willy Tarreau]

On Wed, Jul 19, 2023 at 09:32:46PM +0800, Zhangjin Wu wrote:
> Firstly, add extra config files for powerpc, powerpc64le and powerpc64.
> 
> Second, QEMU_TIMEOUT is configured as 60 seconds for powerpc to allow
> quit qemu-system-ppc even if poweroff fails. In normal host machine, ~20
> seconds may be enough for boot+test+poweroff, but 60 seconds is used
> here to gurantee it at least finish even in a very slow host machine or
> the host machine is too busy. Both powerpc64le and powerpc64 can
> poweroff normally, no need to configure QEMU_TIMEOUT for them.

Hmmm call me annoying, but this started with tinyconfig "in order to
save build time" and now it's enforcing a 1-minute timeout on a single
test. When I run the tests, they hardly last more than a few seconds
and sometimes even just about one second. If some tests last too long
doing nothing, we should adjust their config (e.g. useless probe of a
driver). If they can't power off due to a config option we need to fix
that option. If it can't power off due to the architecture, we can also
try the reboot (qemu is started with --no-reboot to stop instead of
rebooting), and as a last resort we should rely on the timeout in case
everything else fails. But then this timeout should be quite short
because we'll then have guaranteed from the choice of config options
that it boots and executes fast by default.

Finally, if we need to implement a timeout enforcement for at least
one arch because we do not control every failure case, then there's no
reason for considering that certain archs are safe against this and
others not. This means that we can (should?) implement the timeout by
default for every arch, and make sure that the timeout is never hit by
default, unless there's really absolutely no way to fix the arch that
cannot power down nor reboot, in which case the timeout should remain
short enough.

What's your opinion ?

Willy

Re: [PATCH v2 02/14] selftests/nolibc: add macros to enhance maintainability

[Zhangjin Wu]

Hi, Willy

> On Wed, Jul 19, 2023 at 09:19:10PM +0800, Zhangjin Wu wrote:
> > The kernel targets share the same kernel make operations, the same
> > .config file, the same kernel image, add MAKE_KERNEL, KERNEL_CONFIG and
> > KERNEL_IMAGE for them.
> > 
> > Many targets share the same logging related settings, let's add common
> > variables RUN_OUT, LOG_OUT and REPORT_RUN_OUT for them.
> > 
> > The qemu run/rerun targets share the same qemu system run command, add
> > QEMU_SYSTEM_RUN for them.
> > 
> > Signed-off-by: Zhangjin Wu <falcon@tinylab.org>
> > ---
> >  tools/testing/selftests/nolibc/Makefile | 41 ++++++++++++++++---------
> >  1 file changed, 27 insertions(+), 14 deletions(-)
> > 
> > diff --git a/tools/testing/selftests/nolibc/Makefile b/tools/testing/selftests/nolibc/Makefile
> > index 0cd17de2062c..8c531518bb9f 100644
> > --- a/tools/testing/selftests/nolibc/Makefile
> > +++ b/tools/testing/selftests/nolibc/Makefile
> > @@ -166,45 +166,58 @@ endif
> >  libc-test: nolibc-test.c
> >  	$(QUIET_CC)$(CC) -o $@ $<
> >  
> > +# common macros for logging
> > +RUN_OUT = $(CURDIR)/run.out
> > +LOG_OUT = > "$(RUN_OUT)"
> > +REPORT_RUN_OUT = $(REPORT) "$(RUN_OUT)"
> > +
> >  # local libc-test
> >  run-libc-test: libc-test
> > -	$(Q)./libc-test > "$(CURDIR)/run.out" || :
> > -	$(Q)$(REPORT) $(CURDIR)/run.out
> > +	$(Q)./libc-test $(LOG_OUT) || :
> > +	$(Q)$(REPORT_RUN_OUT)
> 
> Sorry but I don't find that this improves maintainability, quite the
> opposite in fact. One reason is that you never visually expect that
> some shell indirection delimiters are hidden in a macro that seems
> to only convey a name. Sure there are valid use cases for this, but
> I think that here it's just adding too much abstraction and it makes
> it quite hard to unfold all of this mentally.
>

Ok, will reserve less ones as possible as we can.

- RUN_OUT is required for architecture specific output
- REPORT_RUN_OUT is not necessary, will remove it

> Please try to keep the number of macros to the minimum that needs to
> be replaced or forced by the user. Here I'm not seeing a compelling
> reason for a user to want to force LOG_OUT to something else. Also
> makefile macros are generally a pain to debug, which is another
> reason for not putting too many of them.
>
> I noticed that your next patch changes LOG_OUT to tee, it could have
> done it everywhere and wouldn't affect readability as much.
>

I have forgetten to pick an old patch to silence the running log like
this:

    ifeq ($(QUIET_RUN),1)
    LOG_OUT = > "$(RUN_OUT)"
    else
    LOG_OUT = | tee "$(RUN_OUT)"
    endif

Without QUIET_RUN, we can silence the running log with:

    $ make run LOG_OUT="> /dev/null"

It is not meaningful like QUIET_RUN, seems the QUIET_RUN is not
necessary for we have 'grep status' now, so, let's remove this RUN_OUT
too.

Thanks,
Zhangjin

> Thanks,
> Willy

Re: [PATCH v2 03/14] selftests/nolibc: print running log to screen

[Zhangjin Wu]

Hi, Willy

> On Wed, Jul 19, 2023 at 09:20:17PM +0800, Zhangjin Wu wrote:
> > When poweroff fails, qemu-system will hang there without any output.
> > 
> > It is very hard to debug in such case, let's print the running log to
> > the screen to allow users to learn what is happening at the first
> > glance, without editing the Makefile manually every time.
> > 
> > To get a clean output, the 'grep status' command can be used.
> 
> The problem with doing this is that it rolls back to the initial
> version that breaks with qemu. When its stdout is sent to a pipe, we've
> found that the output got randomly mangled and/or missing contents.
> It's only when sent to a file that it's OK. I suspect it has something
> to do with non-blocking writes being used to avoid blocking the
> emulation but I could be totally wrong. That's the reason why we had
> to switch to a file.
>

ok, thanks for sharing the history, it is important.

> And I'd rather avoid starting it in the background as well. Maybe
> you'd want to run the qemu command under the "timeout" one ? That
> could be better than nothing.

Yeah, with the timeout logic, this may be not really required, so, let's
drop this patch and the LOG_OUT from the previous one.

with our timeout logic, we are able to quit qemu and then print the
running log to screen to tell users what happens background, let's
discuss timeout logic in its own patch.

Thanks,
Zhangjin

> 
> Willy

Re: [PATCH v2 05/14] selftests/nolibc: add menuconfig for development

[Zhangjin Wu]

> On Wed, Jul 19, 2023 at 09:22:37PM +0800, Zhangjin Wu wrote:
> > The default DEFCONFIG_<ARCH> may not always work for all architectures,
> > let's allow users to tune some kernel config options with 'menuconfig'.
> > 
> > This is important when porting nolibc to a new architecture, it also
> > allows to speed up nolibc 'run' target testing via manually disabling
> > tons of unnecessary kernel config options.
> > 
> > Signed-off-by: Zhangjin Wu <falcon@tinylab.org>
> > ---
> >  tools/testing/selftests/nolibc/Makefile | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/tools/testing/selftests/nolibc/Makefile b/tools/testing/selftests/nolibc/Makefile
> > index 058e7be070ea..06954b4b3885 100644
> > --- a/tools/testing/selftests/nolibc/Makefile
> > +++ b/tools/testing/selftests/nolibc/Makefile
> > @@ -202,6 +202,9 @@ KERNEL_IMAGE  = $(objtree)/$(IMAGE)
> >  defconfig:
> >  	$(Q)$(MAKE_KERNEL) mrproper $(DEFCONFIG) prepare
> >  
> > +menuconfig:
> > +	$(Q)$(MAKE_KERNEL) menuconfig
> 
> What is the real benefit of this compared to just running the
> standard "make menuconfig" ? We should avoid adding makefile targets
> that do not bring value on top of that the top-level makefile does,
> because it will make the useful ones much harder to spot, and will
> tend to encourage users to use only that makefile during the tests,
> which is a bad practice since many targets will always be missing
> or work differently. It's important in my opinion that we strictly
> stick to what is useful to operate the tests themselves and this
> one doesn't make me feel like it qualifies as such.

Ok, get it.

I did like develop nolibc in tools/testing/selftests/nolibc/ without
changing directories frequently or specifying the "-C" option
unnecessary ;-) 

Since "make menuconfig" is only required during the first porting of a new
architecture, so, it is ok to drop this patch.

Thanks,
Zhangjin

> 
> Willy

Re: [PATCH v2 07/14] selftests/nolibc: defconfig: remove mrproper target

[Zhangjin Wu]

> On Wed, Jul 19, 2023 at 09:24:54PM +0800, Zhangjin Wu wrote:
> > The O=/path/to/kernel-<ARCH> option allows to build kernel for different
> > architectures in different output directories, in this scene, it doesn't
> > need the mrproper operation for defconfig anymore.
> > 
> > If really require to clean up the source code tree, let users run the
> > standalone mrproper target on demand.
> 
> But that's precisely what's going to make it more and more annoying
> to run simple tests. The mrproper was there precisely because one
> config at a time was being used, so without it we'll restart to see
> plenty of failures as it used to be before mrproper was added. I
> understand what you're trying to do with the O=, but then if you're
> already passing "O=", why not also pass "defconfig" ?
> 
> I mean, let's make sure this makefile is only used to manipulate
> the tests. It must not become a huge wrapper for the kernel makefile
> otherwise it will become extremely complicated to use to run just a
> simple test. And with this one and the last few ones, I'm starting
> to sense that I'll have to read a README to figure how to reliably
> run a test.
> 
> In my opinion, there are mainly two use cases :
>   - user, manually: commands should be short, forgiving to user
>     mistakes, and easy to remember. I.e. they're compatible with
>     upper-arrow, then enter.
> 
>   - scripts: these are the ones already running in loops with tons
>     of variables, setting object directories with O=$arch/... and
>     taking care of their own cleanups. These ones will already be
>     user-specific and can very well accomodate one or two extra
>     lines for a make mrproper or make defconfig if needed.
> 
> The second ones deserve thinking and control anyway. The first ones
> should mostly not fail for a user mistake and in the worst case waste
> a bit of their time by rebuilding something that could have been
> avoided. But I do want to prioritize the user here. And that's also
> why I want the makefile to be easy to read with as few macros as
> possible, because once it works for you, it's easy to figure what is
> being done, and how to exploit it from your scripts. The opposite is
> not true. Nobody reads a makefile full of macros to try to figure how
> to run their first test or why a test that worked once now fails.
>

Ok, let's reserve mrproper for defconfig and also for tinyconfig if we
add standalone target for it.

let's drop patch and the previous one.

Thanks,
Zhangjin

> Thanks,
> Willy

Re: [PATCH v2 08/14] selftests/nolibc: string the core targets

[Zhangjin Wu]

> On Wed, Jul 19, 2023 at 09:26:01PM +0800, Zhangjin Wu wrote:
> > To avoid run targets one by one manually and boringly, let's string them
> > with IMAGE and .config, the MAKE command will trigger the dependencies
> > for us.
> > 
> > Note, to allow do menuconfig before extconfig manually, only trigger
> > defconfig while the .config is not there, it means only trigger
> > defconfig for the first run or after a mrproper.
> > 
> > Signed-off-by: Zhangjin Wu <falcon@tinylab.org>
> > ---
> >  tools/testing/selftests/nolibc/Makefile | 18 +++++++++++++-----
> >  1 file changed, 13 insertions(+), 5 deletions(-)
> > 
> > diff --git a/tools/testing/selftests/nolibc/Makefile b/tools/testing/selftests/nolibc/Makefile
> > index 83cb4b017bef..541f3565e584 100644
> > --- a/tools/testing/selftests/nolibc/Makefile
> > +++ b/tools/testing/selftests/nolibc/Makefile
> (...)
> > -extconfig:
> > +extconfig: $(KERNEL_CONFIG)
> >  	$(Q)$(srctree)/scripts/kconfig/merge_config.sh -O "$(objtree)" -m "$(KERNEL_CONFIG)" $(foreach c,$(EXTCONFIG),$(wildcard $(CURDIR)/configs/$c))
> >  	$(Q)$(MAKE_KERNEL) KCONFIG_ALLCONFIG="$(KERNEL_CONFIG)" allnoconfig
> >  
> > -kernel: initramfs
> > +kernel: extconfig
> > +	$(Q)$(MAKE) --no-print-directory initramfs
> 
> There seems to be something wrong here. From what I'm seeing, now if I
> run "make kernel" it will run extconfig and possibly change the config
> I just edited.
>

Yeah, extconfig will run for every 'make kernel', it is ok for us to
let kernel depends on $(KERNEL_CONFIG), but this requires users to run
extconfig explictly, the solution may be:

- move extconfig operations to defconfig target and future tinyconfig target (it looks cleaner ...)

    - but it is not convenient to trigger additional changes introduced by
      extconfig, not necessary, but really helpful, something like 'menuconfig'

- let users run extconfig manually after a defconfig or tinyconfig (it is a little complex)

    - this make users hard to learn what to do, should give up this method

- run extconfig for every 'make kernel' as it currently do

    - this may change something after a menuconfig, but it only trigger minimal
      required options, the 'hurt' should be minimal, but of course, it may confuse sometimes ;-(

As a summary, let's remove 'extconfig' and move its operations to the defconfig
and the future tinyconfig targets? 'extconfig' should be a 'default' config
action, let users apply additional ones manually from the top-level 'make
menuconfig', what's your idea?

> Or am I missing something ? I must confess all of this is becoming more
> and more obscure :-(

Yeah ... let's find a better solution ;-)

Best regards,
Zhangjin

> 
> Willy

Re: [PATCH v2 09/14] selftests/nolibc: allow quit qemu-system when poweroff fails

[Zhangjin Wu]

Hi, Willy

> On Wed, Jul 19, 2023 at 09:27:08PM +0800, Zhangjin Wu wrote:
> > The kernel of some architectures can not poweroff qemu-system normally,
> > especially for tinyconfig.
> > 
> > Some architectures may have no kernel poweroff support, the others may
> > require more kernel config options and therefore slow down the
> > tinyconfig build and test. and also, it's very hard (and some even not
> > possible) to find out the exact poweroff related kernel config options
> > for every architecture.
> > 
> > Since the low-level poweroff support is heavily kernel & qemu dependent,
> > it is not that critical to both nolibc and nolibc-test, let's simply
> > ignore the poweroff required kernel config options for tinyconfig (and
> > even for defconfig) and quit qemu-system after a specified timeout or
> > with an expected system halt or poweroff string (these strings mean our
> > reboot() library routine is perfectly ok).
> > 
> > QEMU_TIMEOUT value can be configured for every architecture based on
> > their time cost requirement of boot+test+poweroff.
> > 
> > Signed-off-by: Zhangjin Wu <falcon@tinylab.org>
> > ---
> >  tools/testing/selftests/nolibc/Makefile | 23 +++++++++++++++++++++--
> >  1 file changed, 21 insertions(+), 2 deletions(-)
> > 
> > diff --git a/tools/testing/selftests/nolibc/Makefile b/tools/testing/selftests/nolibc/Makefile
> > index 541f3565e584..a03fab020ebe 100644
> > --- a/tools/testing/selftests/nolibc/Makefile
> > +++ b/tools/testing/selftests/nolibc/Makefile
> > @@ -93,6 +93,9 @@ QEMU_ARGS_s390       = -M s390-ccw-virtio -m 1G -append "console=ttyS0 panic=-1
> >  QEMU_ARGS_loongarch  = -M virt -append "console=ttyS0,115200 panic=-1 $(TEST:%=NOLIBC_TEST=%)"
> >  QEMU_ARGS            = $(QEMU_ARGS_$(XARCH)) $(QEMU_ARGS_EXTRA)
> >  
> > +# QEMU_TIMEOUT: some architectures can not poweroff normally, especially for tinyconfig
> > +QEMU_TIMEOUT             = $(QEMU_TIMEOUT_$(XARCH))
> > +
> >  # OUTPUT is only set when run from the main makefile, otherwise
> >  # it defaults to this nolibc directory.
> >  OUTPUT ?= $(CURDIR)/
> > @@ -224,16 +227,32 @@ kernel: extconfig
> >  # common macros for qemu run/rerun targets
> >  QEMU_SYSTEM_RUN = qemu-system-$(QEMU_ARCH) -display none -no-reboot -kernel "$(KERNEL_IMAGE)" -serial stdio $(QEMU_ARGS)
> >  
> > +ifneq ($(QEMU_TIMEOUT),)
> > +TIMEOUT_CMD = t=$(QEMU_TIMEOUT); \
> > +	while [ $$t -gt 0 ]; do                                                       \
> > +	    sleep 5; t=$$(expr $$t - 5); echo "detecting power off ...";              \
> > +	    if grep -qE "reboot: System halted|reboot: Power down" "$(RUN_OUT)"; then \
> > +		pkill -9 qemu-system-$(QEMU_ARCH);                                    \
> > +		echo "powered off, test finish"; t=1; break;                          \
> > +	    fi;                                                                       \
> > +	done;                                                                         \
> > +	if [ $$t -le 0 ]; then pkill -9 qemu-system-$(QEMU_ARCH); echo "qemu-system-$(QEMU_ARCH) timeout"; fi
> 
> Please have a look at the "timeout" command whichi makes all this much
> simpler.

Yeah, I used 'timeout --forgeground' before, but it is still a dead wait
and it is very hard for us to configure a just right wait time.

If the configured wait time is short, the qemu will be killed during the
test or before running the test, If the configured wait time is long, we
will need to dead wait there even if the test is finished, although
during interactive running, we can press 'CTRL + A X', but for
background running, it is just time waste.

To fix up this issue, the above method used at last allow to detect the
output string, when the test finish and print something lines like:

    reboot: System halted
    reboot: Power down.

We will use pkill to send signal to tell qemu quit, so, it is ok for us
to configure a even'big' timeout, if the kernel can normally poweroff or
if nolibc can successfully send the poweroff syscall, the above message
will be detected and qemu will quit as expected, it will completely
avoid dead wait, the configured timeout will never happen, this is
comfortable.

The worst case is only when qemu or kernel reject to boot, for example,
a qemu bios missing or mismatch issue or a kernel regression or a wrong
kernel config option, for these cases, the real timeout logic will work
for us.

As a summary, our timeout logic here include two parts: one is
'poweroff' related string detection, another is the real timeout logic. 

Based on current implementation, I even plan to add the test finish
string in the expected strings:

    Leaving init with final status

And even further, when a hang detected (no normal poweroff or test
finish string detected), print the whole or last part of running log to
tell users what happens.

> Also, please get used to never ever use kill -9 first. This
> is exactly the way to leave temporary files and IPCs wandering around
> while many programs that care about cleanups at least try to do that
> upon a regular TERM or INT signal.
>

Ok, thanks, will use TERM or INT signal instead.

> Willy

Re: [PATCH v2 12/14] selftests/nolibc: add tinyconfig target

[Zhangjin Wu]

> On Wed, Jul 19, 2023 at 09:30:30PM +0800, Zhangjin Wu wrote:
> > The original tinyconfig target only enables minimal kernel config
> > options, it can speed up the kernel build and nolibc test a lot and also
> > brings us with smaller kernel image size.
> > 
> > But the default enabled options are not enough for qemu boot and console
> > print, some additional config options should be added for every
> > architecture individually.
> > 
> > Signed-off-by: Zhangjin Wu <falcon@tinylab.org>
> > ---
> >  tools/testing/selftests/nolibc/Makefile | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/tools/testing/selftests/nolibc/Makefile b/tools/testing/selftests/nolibc/Makefile
> > index eec2935672ad..f42782fa78a9 100644
> > --- a/tools/testing/selftests/nolibc/Makefile
> > +++ b/tools/testing/selftests/nolibc/Makefile
> > @@ -218,6 +218,9 @@ mrproper:
> >  defconfig:
> >  	$(Q)$(MAKE_KERNEL) $(DEFCONFIG) prepare
> >  
> > +tinyconfig:
> > +	$(Q)$(MAKE_KERNEL) tinyconfig prepare
> 
> So for the same reasons as defconfig above, I'd actually keep mrproper
> here.

Ok, let's add mrproper back here, since tinyconfig is fast enough, so, a
mrproper is not that time cost and a cleanup is really good prepare.

> And if we figure that tinyconfig is never called by the user
> directly but as a dependency from the makefile itself or scripts,
> then we likely don't even need to create a visible entry for it.
>

Great idea,

At first, tinyconfig can be triggered by something like:

    $ make run defconfig DEFCONFIG=tinyconfig

Perhaps we can let $(KERNEL_CONFIG) depends on the top-level
'tinyconfig' and trigger it by default:

    $(KERNEL_CONFIG):
	$(Q)if [ ! -f "$(KERNEL_CONFIG)" ]; then $(MAKE_KERNEL) --no-print-directory mrproper tinyconfig prepare; fi

Of course, we should triger the extra config above.

But this change must delay after we add tinyconfig support for all of
the nolibc supported architectures. before that, we should use
'defconfig' as we do currently.

So, it may be ok for us to drop this patch, but we also need to update
some commit messages who uses tinyconfig target directly.

Thanks,
Zhangjin

> Willy

Re: [PATCH v2 14/14] selftests/nolibc: tinyconfig: add support for 32/64-bit powerpc

[Zhangjin Wu]

Hi, Willy

> On Wed, Jul 19, 2023 at 09:32:46PM +0800, Zhangjin Wu wrote:
> > Firstly, add extra config files for powerpc, powerpc64le and powerpc64.
> > 
> > Second, QEMU_TIMEOUT is configured as 60 seconds for powerpc to allow
> > quit qemu-system-ppc even if poweroff fails. In normal host machine, ~20
> > seconds may be enough for boot+test+poweroff, but 60 seconds is used
> > here to gurantee it at least finish even in a very slow host machine or
> > the host machine is too busy. Both powerpc64le and powerpc64 can
> > poweroff normally, no need to configure QEMU_TIMEOUT for them.
> 
> Hmmm call me annoying, but this started with tinyconfig "in order to
> save build time" and now it's enforcing a 1-minute timeout on a single
> test. When I run the tests, they hardly last more than a few seconds
> and sometimes even just about one second. If some tests last too long
> doing nothing, we should adjust their config (e.g. useless probe of a
> driver). If they can't power off due to a config option we need to fix
> that option. If it can't power off due to the architecture, we can also
> try the reboot (qemu is started with --no-reboot to stop instead of
> rebooting), and as a last resort we should rely on the timeout in case
> everything else fails. But then this timeout should be quite short
> because we'll then have guaranteed from the choice of config options
> that it boots and executes fast by default.
>

As I just explained in this reply [1], our current timeout logic will
detect the 'power off' string at first, so, the 1-minute is the worst
case when the qemu even not print a 'power off' string, that should be a
bug, normally, after the 'power off' string detected, qemu will quit as
expected. the 1-minute is just configured here as a last watchdog to
detect a real hang (may be bios related or may be kernel realted) ;-)

So, the 60 seconds will never be reached, even there is a failed
poweroff, but a smaller one may be ok, what about 30 seconds?

[1]: https://lore.kernel.org/lkml/20230725145955.37685-1-falcon@tinylab.org/

> Finally, if we need to implement a timeout enforcement for at least
> one arch because we do not control every failure case, then there's no
> reason for considering that certain archs are safe against this and
> others not. This means that we can (should?) implement the timeout by
> default for every arch,

Agree, so, what your suggestion about the default timeout? ;-)

10 or 15 seconds may be not enough especially when running on a very
slow host machine, for example, my host will be very slow when the
battery is not in charging status ;-(

And also, the architectures like PowerPC using a very slow SLOF will
boot very slowly, sometimes 20 seconds may be not enough and it may cost
30+ seconds on a very slow machine.

> and make sure that the timeout is never hit by
> default

Yeah, it is the current behavior.

> , unless there's really absolutely no way to fix the arch that
> cannot power down nor reboot,

Even when the kernel not support poweroff, the 'power off' string will
be printed after our 'reboot' syscall, our current timeout logic will
detect this and let qemu quit. We even plan to detect the 'Leaving
init with final status' line.

so, it is not necessary to spend too much time to find out and enable
the kernel power off support for every architecture. and some
architectures may simply not support power-off, and also, some
architectures require too many 'heavy' options to let power-off work,
which may increase build time for tinyconfig a lot, for example, the
ACPI+PCI support are required for power-off for x86.

> in which case the timeout should remain
> short enough.
> 
> What's your opinion ?
>

As a summary, with current timeout logic, a big timeout is only hit when
a real hang happen. Even when the kernel not support power-off, the
power-off string will be detected by us and qemu will quit by pkill.

So, a not that big timeout for every architecture by default, but still
allow the architecture to configure a bigger one?

    QEMU_TIMEOUT_powerpc     = 35
    QEMU_TIMEOUT             = $(or $(QEMU_TIMEOUT_$(XARCH),30)

I will retest them carefully, I'm still worried about that a too small timeout
may kill qemu during test or even before running test, but it would run tests
and power-off normally if we not kill them.

And even further, I'm thinking about the detecting of the boot hang as
earier as we can, for example, these lines are good for us:

    // first line to detect bios hang, may be 5 seconds?
    Linux version 6.4.0+ ...

    // second line to detect kernel boot hang, may be 10 or 15 seconds?
    Run /init as init process

    // third line to detect test hang, ...
    Leaving init with final status

    // forth line to detect power-off
    reboot: Power down

So, even we configure a big timeout, but we can use a smaller default hang
detect setting for bios hang, kernel hang and test hang, it will kill qemu as
earier as we can, even hang happens, no need to wait for the timeout value we
configured.

Best regards,
Zhangjin

> Willy

Re: [PATCH v2 05/14] selftests/nolibc: add menuconfig for development

[Zhangjin Wu]

Hi, Willy

> > On Wed, Jul 19, 2023 at 09:22:37PM +0800, Zhangjin Wu wrote:
> > > The default DEFCONFIG_<ARCH> may not always work for all architectures,
> > > let's allow users to tune some kernel config options with 'menuconfig'.
> > > 
> > > This is important when porting nolibc to a new architecture, it also
> > > allows to speed up nolibc 'run' target testing via manually disabling
> > > tons of unnecessary kernel config options.
> > > 
> > > Signed-off-by: Zhangjin Wu <falcon@tinylab.org>
> > > ---
> > >  tools/testing/selftests/nolibc/Makefile | 3 +++
> > >  1 file changed, 3 insertions(+)
> > > 
> > > diff --git a/tools/testing/selftests/nolibc/Makefile b/tools/testing/selftests/nolibc/Makefile
> > > index 058e7be070ea..06954b4b3885 100644
> > > --- a/tools/testing/selftests/nolibc/Makefile
> > > +++ b/tools/testing/selftests/nolibc/Makefile
> > > @@ -202,6 +202,9 @@ KERNEL_IMAGE  = $(objtree)/$(IMAGE)
> > >  defconfig:
> > >  	$(Q)$(MAKE_KERNEL) mrproper $(DEFCONFIG) prepare
> > >  
> > > +menuconfig:
> > > +	$(Q)$(MAKE_KERNEL) menuconfig
> > 
> > What is the real benefit of this compared to just running the
> > standard "make menuconfig" ? We should avoid adding makefile targets
> > that do not bring value on top of that the top-level makefile does,
> > because it will make the useful ones much harder to spot, and will
> > tend to encourage users to use only that makefile during the tests,
> > which is a bad practice since many targets will always be missing
> > or work differently. It's important in my opinion that we strictly
> > stick to what is useful to operate the tests themselves and this
> > one doesn't make me feel like it qualifies as such.
> 
> Ok, get it.
> 
> I did like develop nolibc in tools/testing/selftests/nolibc/ without
> changing directories frequently or specifying the "-C" option
> unnecessary ;-) 
>
> Since "make menuconfig" is only required during the first porting of a new
> architecture, so, it is ok to drop this patch.
>

Oh, Willy, I did find this is very important again.

I just want to check and test if we need to disable vsx for 32-bit
powerpc too, so, I plan to re-configure kernel via menuconfig to disable
VSX in kernel side, and forcely enable vsx in application side, but it
was very 'painful' when I was running something like this:

    $ make defconfig ARCH=ppc CROSS_COMPILE=powerpc-linux-gnu-

To do a further menuconfig, I must switch to something else:

    $ make menuconfig ARCH=powerpc CROSS_COMPILE=powerpc-linux-gnu- -C ../../../../

Especially, our test is able to accept ARCH=ppc, but the top-level
kernel still only accept powerpc, it makes the development very
comfortable, of course, the typing of the relative or absolute path
every time is either not a good idea.

so, this doesn't simply duplicate with the one from top-level, it can
get the right ARCH, srctree for us, and this is heavily used by our
tinyconfig development to tune the config options very frequently, so,
let's still add this one in the new revision?

But I plan to merge the mrproper targets here to save another patch,
what about your idea?

    menuconfig mrproper:
	$(Q)$(MAKE_KERNEL) $@

Thanks,
Zhangjin

> Thanks,
> Zhangjin
> 
> > 
> > Willy

Re: [PATCH v2 08/14] selftests/nolibc: string the core targets

[Willy Tarreau]

On Tue, Jul 25, 2023 at 10:20:17PM +0800, Zhangjin Wu wrote:
> > On Wed, Jul 19, 2023 at 09:26:01PM +0800, Zhangjin Wu wrote:
> > > To avoid run targets one by one manually and boringly, let's string them
> > > with IMAGE and .config, the MAKE command will trigger the dependencies
> > > for us.
> > > 
> > > Note, to allow do menuconfig before extconfig manually, only trigger
> > > defconfig while the .config is not there, it means only trigger
> > > defconfig for the first run or after a mrproper.
> > > 
> > > Signed-off-by: Zhangjin Wu <falcon@tinylab.org>
> > > ---
> > >  tools/testing/selftests/nolibc/Makefile | 18 +++++++++++++-----
> > >  1 file changed, 13 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/tools/testing/selftests/nolibc/Makefile b/tools/testing/selftests/nolibc/Makefile
> > > index 83cb4b017bef..541f3565e584 100644
> > > --- a/tools/testing/selftests/nolibc/Makefile
> > > +++ b/tools/testing/selftests/nolibc/Makefile
> > (...)
> > > -extconfig:
> > > +extconfig: $(KERNEL_CONFIG)
> > >  	$(Q)$(srctree)/scripts/kconfig/merge_config.sh -O "$(objtree)" -m "$(KERNEL_CONFIG)" $(foreach c,$(EXTCONFIG),$(wildcard $(CURDIR)/configs/$c))
> > >  	$(Q)$(MAKE_KERNEL) KCONFIG_ALLCONFIG="$(KERNEL_CONFIG)" allnoconfig
> > >  
> > > -kernel: initramfs
> > > +kernel: extconfig
> > > +	$(Q)$(MAKE) --no-print-directory initramfs
> > 
> > There seems to be something wrong here. From what I'm seeing, now if I
> > run "make kernel" it will run extconfig and possibly change the config
> > I just edited.
> >
> 
> Yeah, extconfig will run for every 'make kernel', it is ok for us to
> let kernel depends on $(KERNEL_CONFIG), but this requires users to run
> extconfig explictly, the solution may be:
> 
> - move extconfig operations to defconfig target and future tinyconfig target (it looks cleaner ...)
> 
>     - but it is not convenient to trigger additional changes introduced by
>       extconfig, not necessary, but really helpful, something like 'menuconfig'
> 
> - let users run extconfig manually after a defconfig or tinyconfig (it is a little complex)
> 
>     - this make users hard to learn what to do, should give up this method
> 
> - run extconfig for every 'make kernel' as it currently do
> 
>     - this may change something after a menuconfig, but it only trigger minimal
>       required options, the 'hurt' should be minimal, but of course, it may confuse sometimes ;-(
> 
> As a summary, let's remove 'extconfig' and move its operations to the defconfig
> and the future tinyconfig targets? 'extconfig' should be a 'default' config
> action, let users apply additional ones manually from the top-level 'make
> menuconfig', what's your idea?

Well, it's important to apply the principle of least surprise for the
user. You're a developer who spent time working on your config, you
believe it's OK and you just remind that you've heard about that nolibc
test thing recently and you think "why not give it a try in case it spots
something I forgot in my config". Then you just run the test there and
once done your config was secretly modified. This is exactly an example
of what *not* to do. We should never modify user's config nor files in
general without an explicit request from the user. If the user types
"make defconfig", they're implicitly requesting to replace the config,
so we can do what we want with it. If they type "make kernel", they
expect to make a kernel based on this config, not to mollest this
precious config file and then make a kernel out of it.

So I'm fine with the idea of adding config snippets on top of defconfig
and tinyconfig to allow the user to generate a working config for a
given architecture, but not for modifying their config without their
consent.

Willy

Re: [PATCH v2 09/14] selftests/nolibc: allow quit qemu-system when poweroff fails

[Willy Tarreau]

On Tue, Jul 25, 2023 at 10:59:55PM +0800, Zhangjin Wu wrote:
> > On Wed, Jul 19, 2023 at 09:27:08PM +0800, Zhangjin Wu wrote:
> > > The kernel of some architectures can not poweroff qemu-system normally,
> > > especially for tinyconfig.
> > > 
> > > Some architectures may have no kernel poweroff support, the others may
> > > require more kernel config options and therefore slow down the
> > > tinyconfig build and test. and also, it's very hard (and some even not
> > > possible) to find out the exact poweroff related kernel config options
> > > for every architecture.
> > > 
> > > Since the low-level poweroff support is heavily kernel & qemu dependent,
> > > it is not that critical to both nolibc and nolibc-test, let's simply
> > > ignore the poweroff required kernel config options for tinyconfig (and
> > > even for defconfig) and quit qemu-system after a specified timeout or
> > > with an expected system halt or poweroff string (these strings mean our
> > > reboot() library routine is perfectly ok).
> > > 
> > > QEMU_TIMEOUT value can be configured for every architecture based on
> > > their time cost requirement of boot+test+poweroff.
> > > 
> > > Signed-off-by: Zhangjin Wu <falcon@tinylab.org>
> > > ---
> > >  tools/testing/selftests/nolibc/Makefile | 23 +++++++++++++++++++++--
> > >  1 file changed, 21 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/tools/testing/selftests/nolibc/Makefile b/tools/testing/selftests/nolibc/Makefile
> > > index 541f3565e584..a03fab020ebe 100644
> > > --- a/tools/testing/selftests/nolibc/Makefile
> > > +++ b/tools/testing/selftests/nolibc/Makefile
> > > @@ -93,6 +93,9 @@ QEMU_ARGS_s390       = -M s390-ccw-virtio -m 1G -append "console=ttyS0 panic=-1
> > >  QEMU_ARGS_loongarch  = -M virt -append "console=ttyS0,115200 panic=-1 $(TEST:%=NOLIBC_TEST=%)"
> > >  QEMU_ARGS            = $(QEMU_ARGS_$(XARCH)) $(QEMU_ARGS_EXTRA)
> > >  
> > > +# QEMU_TIMEOUT: some architectures can not poweroff normally, especially for tinyconfig
> > > +QEMU_TIMEOUT             = $(QEMU_TIMEOUT_$(XARCH))
> > > +
> > >  # OUTPUT is only set when run from the main makefile, otherwise
> > >  # it defaults to this nolibc directory.
> > >  OUTPUT ?= $(CURDIR)/
> > > @@ -224,16 +227,32 @@ kernel: extconfig
> > >  # common macros for qemu run/rerun targets
> > >  QEMU_SYSTEM_RUN = qemu-system-$(QEMU_ARCH) -display none -no-reboot -kernel "$(KERNEL_IMAGE)" -serial stdio $(QEMU_ARGS)
> > >  
> > > +ifneq ($(QEMU_TIMEOUT),)
> > > +TIMEOUT_CMD = t=$(QEMU_TIMEOUT); \
> > > +	while [ $$t -gt 0 ]; do                                                       \
> > > +	    sleep 5; t=$$(expr $$t - 5); echo "detecting power off ...";              \
> > > +	    if grep -qE "reboot: System halted|reboot: Power down" "$(RUN_OUT)"; then \
> > > +		pkill -9 qemu-system-$(QEMU_ARCH);                                    \
> > > +		echo "powered off, test finish"; t=1; break;                          \
> > > +	    fi;                                                                       \
> > > +	done;                                                                         \
> > > +	if [ $$t -le 0 ]; then pkill -9 qemu-system-$(QEMU_ARCH); echo "qemu-system-$(QEMU_ARCH) timeout"; fi
> > 
> > Please have a look at the "timeout" command whichi makes all this much
> > simpler.
> 
> Yeah, I used 'timeout --forgeground' before, but it is still a dead wait
> and it is very hard for us to configure a just right wait time.
> 
> If the configured wait time is short, the qemu will be killed during the
> test or before running the test, If the configured wait time is long, we
> will need to dead wait there even if the test is finished, although
> during interactive running, we can press 'CTRL + A X', but for
> background running, it is just time waste.
> 
> To fix up this issue, the above method used at last allow to detect the
> output string, when the test finish and print something lines like:
> 
>     reboot: System halted
>     reboot: Power down.
> 
> We will use pkill to send signal to tell qemu quit, so, it is ok for us
> to configure a even'big' timeout, if the kernel can normally poweroff or
> if nolibc can successfully send the poweroff syscall, the above message
> will be detected and qemu will quit as expected, it will completely
> avoid dead wait, the configured timeout will never happen, this is
> comfortable.
> 
> The worst case is only when qemu or kernel reject to boot, for example,
> a qemu bios missing or mismatch issue or a kernel regression or a wrong
> kernel config option, for these cases, the real timeout logic will work
> for us.
> 
> As a summary, our timeout logic here include two parts: one is
> 'poweroff' related string detection, another is the real timeout logic. 
> 
> Based on current implementation, I even plan to add the test finish
> string in the expected strings:
> 
>     Leaving init with final status
> 
> And even further, when a hang detected (no normal poweroff or test
> finish string detected), print the whole or last part of running log to
> tell users what happens.

Again, all of this seems ridiculously complicated for what seems to be
a very unlikely issue. You mentioned earlier:

  > > > The kernel of some architectures can not poweroff qemu-system
  > > > normally, especially for tinyconfig.

Till now all those I tested do power off correctly. So if the problem
is related to one specific arch, first it should be mentioned in the
commit message, and maybe we need to look closer why it's doing that
instead of papering over it, and if it's caused by tinyconfig, it just
means we need to produce a more reasonable config. But I still don't
like the idea of causing a problem on one side, and making the other
side totally ugly just because of the problem. The timeout solution
should be a last resort one which clearly explains why we can't do
differently.

Also, defconfig and tinyconfig are for people who work on nolibc. These
ones should support working in batch mode, being called from a script
iterating over multiple archs. For other config, we should not interfer
with what the user does. Maybe some will consider that it's fine to run
a test slowly on a simulated platform for example.

Willy

Re: [PATCH v2 05/14] selftests/nolibc: add menuconfig for development

[Willy Tarreau]

Hi Zhangjin,

On Thu, Jul 27, 2023 at 09:24:18PM +0800, Zhangjin Wu wrote:
> Hi, Willy
> 
> > > On Wed, Jul 19, 2023 at 09:22:37PM +0800, Zhangjin Wu wrote:
> > > > The default DEFCONFIG_<ARCH> may not always work for all architectures,
> > > > let's allow users to tune some kernel config options with 'menuconfig'.
> > > > 
> > > > This is important when porting nolibc to a new architecture, it also
> > > > allows to speed up nolibc 'run' target testing via manually disabling
> > > > tons of unnecessary kernel config options.
> > > > 
> > > > Signed-off-by: Zhangjin Wu <falcon@tinylab.org>
> > > > ---
> > > >  tools/testing/selftests/nolibc/Makefile | 3 +++
> > > >  1 file changed, 3 insertions(+)
> > > > 
> > > > diff --git a/tools/testing/selftests/nolibc/Makefile b/tools/testing/selftests/nolibc/Makefile
> > > > index 058e7be070ea..06954b4b3885 100644
> > > > --- a/tools/testing/selftests/nolibc/Makefile
> > > > +++ b/tools/testing/selftests/nolibc/Makefile
> > > > @@ -202,6 +202,9 @@ KERNEL_IMAGE  = $(objtree)/$(IMAGE)
> > > >  defconfig:
> > > >  	$(Q)$(MAKE_KERNEL) mrproper $(DEFCONFIG) prepare
> > > >  
> > > > +menuconfig:
> > > > +	$(Q)$(MAKE_KERNEL) menuconfig
> > > 
> > > What is the real benefit of this compared to just running the
> > > standard "make menuconfig" ? We should avoid adding makefile targets
> > > that do not bring value on top of that the top-level makefile does,
> > > because it will make the useful ones much harder to spot, and will
> > > tend to encourage users to use only that makefile during the tests,
> > > which is a bad practice since many targets will always be missing
> > > or work differently. It's important in my opinion that we strictly
> > > stick to what is useful to operate the tests themselves and this
> > > one doesn't make me feel like it qualifies as such.
> > 
> > Ok, get it.
> > 
> > I did like develop nolibc in tools/testing/selftests/nolibc/ without
> > changing directories frequently or specifying the "-C" option
> > unnecessary ;-) 
> >
> > Since "make menuconfig" is only required during the first porting of a new
> > architecture, so, it is ok to drop this patch.
> >
> 
> Oh, Willy, I did find this is very important again.
> 
> I just want to check and test if we need to disable vsx for 32-bit
> powerpc too, so, I plan to re-configure kernel via menuconfig to disable
> VSX in kernel side, and forcely enable vsx in application side, but it
> was very 'painful' when I was running something like this:
> 
>     $ make defconfig ARCH=ppc CROSS_COMPILE=powerpc-linux-gnu-
> 
> To do a further menuconfig, I must switch to something else:
> 
>     $ make menuconfig ARCH=powerpc CROSS_COMPILE=powerpc-linux-gnu- -C ../../../../
> 
> Especially, our test is able to accept ARCH=ppc, but the top-level
> kernel still only accept powerpc, it makes the development very
> comfortable, of course, the typing of the relative or absolute path
> every time is either not a good idea.

It is "able" but that's not what ought to be used, since it's going to
be remapped to "powerpc". You're supposed to use XARCH for this one
since it's a variant (we could find other names if needed). I suspect
it just shows that the "override ARCH :=" is excessive and should not
be there. Usually when you put override in a makefile, it's going to
pop up as a bug elsewhere. Thus actually you could very well have :

   make ARCH=powerpc XARCH=ppc CROSS_COMPILE=powerpc-linux-gnu-

as the prefix for all your commands. Some will prefer to work directly
from the kernel dir:

 $ make ARCH=powerpc XARCH=ppc CROSS_COMPILE=powerpc-linux-gnu- -C tools/testing/selftests/nolibc-test defconfig
 $ make ARCH=powerpc XARCH=ppc CROSS_COMPILE=powerpc-linux-gnu- menuconfig

Others would do it from the nolibc-test dir as you did above.

> so, this doesn't simply duplicate with the one from top-level, it can
> get the right ARCH, srctree for us, and this is heavily used by our
> tinyconfig development to tune the config options very frequently, so,
> let's still add this one in the new revision?

I'm not *fundamentally* opposed to menuconfig, I just think that it's
appearing at the wrong place. Why not oldconfig, allmodconfig, randconfig,
allnoconfig etc then ? There is nothing in the test directory that is
supposed to be used interactively, either it's run in a batch for cross-
arch testing, or you use it to validate your kernel when you're working
on it. It feels strange that one would want to configure their kernel
from this sub-dir.

> But I plan to merge the mrproper targets here to save another patch,
> what about your idea?
> 
>     menuconfig mrproper:
> 	$(Q)$(MAKE_KERNEL) $@

Please no. Someone doing that in hope to clean only the result of
the tests would in fact ruin their config:

  $ make -C tools/testing/selftests/nolibc-test mrproper

This is another reason for not encouraging users to develop from
within that dir.

Willy

Re: [PATCH v2 08/14] selftests/nolibc: string the core targets

[Zhangjin Wu]

> On Tue, Jul 25, 2023 at 10:20:17PM +0800, Zhangjin Wu wrote:
> > > On Wed, Jul 19, 2023 at 09:26:01PM +0800, Zhangjin Wu wrote:
> > > > To avoid run targets one by one manually and boringly, let's string them
> > > > with IMAGE and .config, the MAKE command will trigger the dependencies
> > > > for us.
> > > > 
> > > > Note, to allow do menuconfig before extconfig manually, only trigger
> > > > defconfig while the .config is not there, it means only trigger
> > > > defconfig for the first run or after a mrproper.
> > > > 
> > > > Signed-off-by: Zhangjin Wu <falcon@tinylab.org>
> > > > ---
> > > >  tools/testing/selftests/nolibc/Makefile | 18 +++++++++++++-----
> > > >  1 file changed, 13 insertions(+), 5 deletions(-)
> > > > 
> > > > diff --git a/tools/testing/selftests/nolibc/Makefile b/tools/testing/selftests/nolibc/Makefile
> > > > index 83cb4b017bef..541f3565e584 100644
> > > > --- a/tools/testing/selftests/nolibc/Makefile
> > > > +++ b/tools/testing/selftests/nolibc/Makefile
> > > (...)
> > > > -extconfig:
> > > > +extconfig: $(KERNEL_CONFIG)
> > > >  	$(Q)$(srctree)/scripts/kconfig/merge_config.sh -O "$(objtree)" -m "$(KERNEL_CONFIG)" $(foreach c,$(EXTCONFIG),$(wildcard $(CURDIR)/configs/$c))
> > > >  	$(Q)$(MAKE_KERNEL) KCONFIG_ALLCONFIG="$(KERNEL_CONFIG)" allnoconfig
> > > >  
> > > > -kernel: initramfs
> > > > +kernel: extconfig
> > > > +	$(Q)$(MAKE) --no-print-directory initramfs
> > > 
> > > There seems to be something wrong here. From what I'm seeing, now if I
> > > run "make kernel" it will run extconfig and possibly change the config
> > > I just edited.
> > >
> > 
> > Yeah, extconfig will run for every 'make kernel', it is ok for us to
> > let kernel depends on $(KERNEL_CONFIG), but this requires users to run
> > extconfig explictly, the solution may be:
> > 
> > - move extconfig operations to defconfig target and future tinyconfig target (it looks cleaner ...)
> > 
> >     - but it is not convenient to trigger additional changes introduced by
> >       extconfig, not necessary, but really helpful, something like 'menuconfig'
> > 
> > - let users run extconfig manually after a defconfig or tinyconfig (it is a little complex)
> > 
> >     - this make users hard to learn what to do, should give up this method
> > 
> > - run extconfig for every 'make kernel' as it currently do
> > 
> >     - this may change something after a menuconfig, but it only trigger minimal
> >       required options, the 'hurt' should be minimal, but of course, it may confuse sometimes ;-(
> > 
> > As a summary, let's remove 'extconfig' and move its operations to the defconfig
> > and the future tinyconfig targets? 'extconfig' should be a 'default' config
> > action, let users apply additional ones manually from the top-level 'make
> > menuconfig', what's your idea?
> 
> Well, it's important to apply the principle of least surprise for the
> user. You're a developer who spent time working on your config, you
> believe it's OK and you just remind that you've heard about that nolibc
> test thing recently and you think "why not give it a try in case it spots
> something I forgot in my config". Then you just run the test there and
> once done your config was secretly modified. This is exactly an example
> of what *not* to do. We should never modify user's config nor files in
> general without an explicit request from the user. If the user types
> "make defconfig", they're implicitly requesting to replace the config,
> so we can do what we want with it. If they type "make kernel", they
> expect to make a kernel based on this config, not to mollest this
> precious config file and then make a kernel out of it.
> 
> So I'm fine with the idea of adding config snippets on top of defconfig
> and tinyconfig to allow the user to generate a working config for a
> given architecture, but not for modifying their config without their
> consent.
>

Agree, seems our additional config snippets are minimal and 'necessary'
for boot and print, so, I ignored the override issue before.

What about the version in v3 here:
    https://lore.kernel.org/lkml/9b52e26748eda1ac108d569207bf428bf37b3bbc.1690489039.git.falcon@tinylab.org/

The 'defconfig' will only be triggered while there is no .config there,
I do think it is important, at the first time of using nolibc, I
directly run kernel but it fails for it has a manual defconfig
requirement every time, so, I do think a default defconfig for kernel
for the first run or after a mrproper is helpful, it doesn't modify any
.config for there is no one there.

    +PHONY += $(KERNEL_CONFIG)
    +$(KERNEL_CONFIG):
    +	$(Q)if [ ! -f "$(KERNEL_CONFIG)" ]; then $(MAKE) --no-print-directory defconfig; fi
    +
    +kernel: $(KERNEL_CONFIG)
    +	$(Q)$(MAKE) --no-print-directory initramfs
     	$(Q)$(MAKE_KERNEL) $(IMAGE_NAME) CONFIG_INITRAMFS_SOURCE=$(CURDIR)/initramfs

Thanks,
Zhangjin

> Willy

Re: [PATCH v2 05/14] selftests/nolibc: add menuconfig for development

[Zhangjin Wu]

Hi, Willy

[...]
> > > > 
> > > > What is the real benefit of this compared to just running the
> > > > standard "make menuconfig" ? We should avoid adding makefile targets
> > > > that do not bring value on top of that the top-level makefile does,
> > > > because it will make the useful ones much harder to spot, and will
> > > > tend to encourage users to use only that makefile during the tests,
> > > > which is a bad practice since many targets will always be missing
> > > > or work differently. It's important in my opinion that we strictly
> > > > stick to what is useful to operate the tests themselves and this
> > > > one doesn't make me feel like it qualifies as such.
> > > 
> > > Ok, get it.
> > > 
> > > I did like develop nolibc in tools/testing/selftests/nolibc/ without
> > > changing directories frequently or specifying the "-C" option
> > > unnecessary ;-) 
> > >
> > > Since "make menuconfig" is only required during the first porting of a new
> > > architecture, so, it is ok to drop this patch.
> > >
> > 
> > Oh, Willy, I did find this is very important again.
> > 
> > I just want to check and test if we need to disable vsx for 32-bit
> > powerpc too, so, I plan to re-configure kernel via menuconfig to disable
> > VSX in kernel side, and forcely enable vsx in application side, but it
> > was very 'painful' when I was running something like this:
> > 
> >     $ make defconfig ARCH=ppc CROSS_COMPILE=powerpc-linux-gnu-
> > 
> > To do a further menuconfig, I must switch to something else:
> > 
> >     $ make menuconfig ARCH=powerpc CROSS_COMPILE=powerpc-linux-gnu- -C ../../../../
> > 
> > Especially, our test is able to accept ARCH=ppc, but the top-level
> > kernel still only accept powerpc, it makes the development very
> > comfortable, of course, the typing of the relative or absolute path
> > every time is either not a good idea.
> 
> It is "able" but that's not what ought to be used, since it's going to
> be remapped to "powerpc". You're supposed to use XARCH for this one
> since it's a variant (we could find other names if needed). I suspect
> it just shows that the "override ARCH :=" is excessive and should not
> be there. Usually when you put override in a makefile, it's going to
> pop up as a bug elsewhere. Thus actually you could very well have :
> 
>    make ARCH=powerpc XARCH=ppc CROSS_COMPILE=powerpc-linux-gnu-
>

Oh, Seems I have misunderstood your suggestions in another reply, you
recommended to add ARCH variable to the help target, at last, I thought
it was better to only reserve ARCH as the consistent variable to users,
so, the 'override' keyword is added to allow passing anyone of powerpc,
ppc, ppc64 and ppc64le to ARCH, since XARCH only accept ppc, ppc64 and
ppc64le.

Further passing 'ARCH=powerpc XARCH=ppc' is also ok, but the combination
of ARCH and XARCH may require more time to teach a new user, is ok for
me to remove the override and carefully document this usage in Makefile.

Seems 'override' is really required, without it, when users wrongly try
ARCH=ppc, ARCH=ppc64, ARCH=ppc64le, the ARCH passed to kernel will be
completely wrong, it may mislead users ...

With this 'override', it is ok for users to use ARCH only, or XARCH or even
together with both of them, no failure and happy, to myself, I'm ok to remove
the 'override' keyword, ;-)

> as the prefix for all your commands. Some will prefer to work directly
> from the kernel dir:
> 
>  $ make ARCH=powerpc XARCH=ppc CROSS_COMPILE=powerpc-linux-gnu- -C tools/testing/selftests/nolibc-test defconfig
>  $ make ARCH=powerpc XARCH=ppc CROSS_COMPILE=powerpc-linux-gnu- menuconfig
> 
> Others would do it from the nolibc-test dir as you did above.
>

Yeah, we still need one more -C, but it is of course ok to do this in a
standalone script as you mentioned before.

> > so, this doesn't simply duplicate with the one from top-level, it can
> > get the right ARCH, srctree for us, and this is heavily used by our
> > tinyconfig development to tune the config options very frequently, so,
> > let's still add this one in the new revision?
> 
> I'm not *fundamentally* opposed to menuconfig, I just think that it's
> appearing at the wrong place. Why not oldconfig, allmodconfig, randconfig,
> allnoconfig etc then ? There is nothing in the test directory that is
> supposed to be used interactively, either it's run in a batch for cross-
> arch testing, or you use it to validate your kernel when you're working
> on it. It feels strange that one would want to configure their kernel
> from this sub-dir.

Ok, it is reasonable ;-)

> 
> > But I plan to merge the mrproper targets here to save another patch,
> > what about your idea?
> > 
> >     menuconfig mrproper:
> > 	$(Q)$(MAKE_KERNEL) $@
> 
> Please no. Someone doing that in hope to clean only the result of
> the tests would in fact ruin their config:
> 
>   $ make -C tools/testing/selftests/nolibc-test mrproper
>

Yeah, really a good reason to not add mrproper there.

> This is another reason for not encouraging users to develop from
> within that dir.
>

Thanks,
Zhangjin

> Willy

Re: [PATCH v2 08/14] selftests/nolibc: string the core targets

[Willy Tarreau]

On Sat, Jul 29, 2023 at 05:54:47PM +0800, Zhangjin Wu wrote:
> The 'defconfig' will only be triggered while there is no .config there,
> I do think it is important, at the first time of using nolibc, I
> directly run kernel but it fails for it has a manual defconfig
> requirement every time, so, I do think a default defconfig for kernel
> for the first run or after a mrproper is helpful, it doesn't modify any
> .config for there is no one there.

On the opposite, that's yet another example of automatic stuff that for
me adds zero value and just doubts in the user's head: "is it safe to
call this with my own config or should I keep a safe copy of it?",
"what will it use for the config?", "will the arch be correct if my
current config references 32BIT and the generated default one switches
it to 64?" etc.

Please let's not add unneeded dependencies and chaining. It does not
help and makes it harder to restart at one specific step, thus lowers
the overall value.

Willy

Re: [PATCH v2 08/14] selftests/nolibc: string the core targets

[Zhangjin Wu]

> On Sat, Jul 29, 2023 at 05:54:47PM +0800, Zhangjin Wu wrote:
> > The 'defconfig' will only be triggered while there is no .config there,
> > I do think it is important, at the first time of using nolibc, I
> > directly run kernel but it fails for it has a manual defconfig
> > requirement every time, so, I do think a default defconfig for kernel
> > for the first run or after a mrproper is helpful, it doesn't modify any
> > .config for there is no one there.
> 
> On the opposite, that's yet another example of automatic stuff that for
> me adds zero value and just doubts in the user's head: "is it safe to
> call this with my own config or should I keep a safe copy of it?",
> "what will it use for the config?", "will the arch be correct if my
> current config references 32BIT and the generated default one switches
> it to 64?" etc.
> 
> Please let's not add unneeded dependencies and chaining. It does not
> help and makes it harder to restart at one specific step, thus lowers
> the overall value.
>

Ok, let's do subtraction, drop this one and the timeout two.

Thanks,
Zhangjin

> Willy