2023-10-10 12:34:55

by Thomas Weißschuh

[permalink] [raw]
Subject: [PATCH 0/5] selftests/nolibc: various build improvements

With the out-of-tree builds it's possible do incremental tests fairly fast:

$ time ./run-tests.sh
i386: 160 test(s): 160 passed, 0 skipped, 0 failed => status: success
x86_64: 160 test(s): 160 passed, 0 skipped, 0 failed => status: success
arm64: 160 test(s): 160 passed, 0 skipped, 0 failed => status: success
arm: 160 test(s): 160 passed, 0 skipped, 0 failed => status: success
mips: 160 test(s): 159 passed, 1 skipped, 0 failed => status: warning
ppc: 160 test(s): 160 passed, 0 skipped, 0 failed => status: success
ppc64: 160 test(s): 160 passed, 0 skipped, 0 failed => status: success
ppc64le: 160 test(s): 160 passed, 0 skipped, 0 failed => status: success
riscv: 160 test(s): 160 passed, 0 skipped, 0 failed => status: success
s390: 160 test(s): 159 passed, 1 skipped, 0 failed => status: warning
loongarch: 160 test(s): 159 passed, 1 skipped, 0 failed => status: warning

real 3m11.735s
user 4m20.354s
sys 1m11.880s

This is with an incremental kernel rebuild and testrun inside qemu.

Note:

"selftests/nolibc: use qemu-system-ppc64 also for ppc64le" was already
submitted standalone but I included it here again for easier testing and
review.

Signed-off-by: Thomas Weißschuh <[email protected]>
---
Thomas Weißschuh (5):
selftests/nolibc: use qemu-system-ppc64 also for ppc64le
selftests/nolibc: use EFI -bios for LoongArch qemu
selftests/nolibc: anchor paths in $(srcdir) if possible
selftests/nolibc: support out-of-tree builds
selftests/nolibc: generate config automatically

tools/testing/selftests/nolibc/Makefile | 36 +++++++++++++++++++++------------
1 file changed, 23 insertions(+), 13 deletions(-)
---
base-commit: d423dcd4ac21041618ab83455c09440d76dbc099
change-id: 20231010-nolibc-out-of-tree-b6684c6cf0e3

Best regards,
--
Thomas Weißschuh <[email protected]>


2023-10-10 12:34:57

by Thomas Weißschuh

[permalink] [raw]
Subject: [PATCH 1/5] selftests/nolibc: use qemu-system-ppc64 also for ppc64le

qemu-system-ppc64 can handle both big and little endian kernels.

While some setups, like Debian, provide a symlink to execute
qemu-system-ppc64 as qemu-system-ppc64le, others, like ArchLinux, do not.

So always use qemu-system-ppc64 directly.

Signed-off-by: Thomas Weißschuh <[email protected]>
---
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 891aa396163d..af60e07d3c12 100644
--- a/tools/testing/selftests/nolibc/Makefile
+++ b/tools/testing/selftests/nolibc/Makefile
@@ -82,7 +82,7 @@ QEMU_ARCH_arm = arm
QEMU_ARCH_mips = mipsel # works with malta_defconfig
QEMU_ARCH_ppc = ppc
QEMU_ARCH_ppc64 = ppc64
-QEMU_ARCH_ppc64le = ppc64le
+QEMU_ARCH_ppc64le = ppc64
QEMU_ARCH_riscv = riscv64
QEMU_ARCH_s390 = s390x
QEMU_ARCH_loongarch = loongarch64

--
2.42.0

2023-10-10 12:34:57

by Thomas Weißschuh

[permalink] [raw]
Subject: [PATCH 2/5] selftests/nolibc: use EFI -bios for LoongArch qemu

qemu for LoongArch does not work properly with direct kernel boot.
The kernel will panic during initialization and hang without any output.

When booting in EFI mode everything work correctly.

While users most likely don't have the LoongArch EFI binary installed at
least an explicit error about 'file not found' is better than a hanging
test without output that can never succeed.

Link: https://lore.kernel.org/loongarch/[email protected]/
Signed-off-by: Thomas Weißschuh <[email protected]>

---
Note: I'm wondering how this worked for anybody else.
---
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 af60e07d3c12..258293639572 100644
--- a/tools/testing/selftests/nolibc/Makefile
+++ b/tools/testing/selftests/nolibc/Makefile
@@ -100,7 +100,7 @@ QEMU_ARGS_ppc64 = -M powernv -append "console=hvc0 panic=-1 $(TEST:%=NOLIBC
QEMU_ARGS_ppc64le = -M powernv -append "console=hvc0 panic=-1 $(TEST:%=NOLIBC_TEST=%)"
QEMU_ARGS_riscv = -M virt -append "console=ttyS0 panic=-1 $(TEST:%=NOLIBC_TEST=%)"
QEMU_ARGS_s390 = -M s390-ccw-virtio -m 1G -append "console=ttyS0 panic=-1 $(TEST:%=NOLIBC_TEST=%)"
-QEMU_ARGS_loongarch = -M virt -append "console=ttyS0,115200 panic=-1 $(TEST:%=NOLIBC_TEST=%)"
+QEMU_ARGS_loongarch = -M virt -append "console=ttyS0,115200 panic=-1 $(TEST:%=NOLIBC_TEST=%)" -bios /usr/share/edk2/loongarch64/OVMF_CODE.fd
QEMU_ARGS = $(QEMU_ARGS_$(XARCH)) $(QEMU_ARGS_EXTRA)

# OUTPUT is only set when run from the main makefile, otherwise

--
2.42.0

2023-10-10 12:35:08

by Thomas Weißschuh

[permalink] [raw]
Subject: [PATCH 3/5] selftests/nolibc: anchor paths in $(srcdir) if possible

It is easier to recognize paths from their well-known location in the
source tree than having to resolve the relative path in ones head.

Signed-off-by: Thomas Weißschuh <[email protected]>
---
tools/testing/selftests/nolibc/Makefile | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/nolibc/Makefile b/tools/testing/selftests/nolibc/Makefile
index 258293639572..598d53c5cb7b 100644
--- a/tools/testing/selftests/nolibc/Makefile
+++ b/tools/testing/selftests/nolibc/Makefile
@@ -167,7 +167,7 @@ sysroot: sysroot/$(ARCH)/include
sysroot/$(ARCH)/include:
$(Q)rm -rf sysroot/$(ARCH) sysroot/sysroot
$(QUIET_MKDIR)mkdir -p sysroot
- $(Q)$(MAKE) -C ../../../include/nolibc ARCH=$(ARCH) OUTPUT=$(CURDIR)/sysroot/ headers_standalone
+ $(Q)$(MAKE) -C $(srctree)/tools/include/nolibc ARCH=$(ARCH) OUTPUT=$(CURDIR)/sysroot/ headers_standalone
$(Q)mv sysroot/sysroot sysroot/$(ARCH)

ifneq ($(NOLIBC_SYSROOT),0)
@@ -177,7 +177,7 @@ nolibc-test: nolibc-test.c sysroot/$(ARCH)/include
else
nolibc-test: nolibc-test.c
$(QUIET_CC)$(CC) $(CFLAGS) $(LDFLAGS) -o $@ \
- -nostdlib -static -include ../../../include/nolibc/nolibc.h $< -lgcc
+ -nostdlib -static -include $(srctree)/tools/include/nolibc/nolibc.h $< -lgcc
endif

libc-test: nolibc-test.c

--
2.42.0

2023-10-10 12:35:11

by Thomas Weißschuh

[permalink] [raw]
Subject: [PATCH 4/5] selftests/nolibc: support out-of-tree builds

Out of tree builds are much more convenient when building for multiple
architectures or configurations in parallel.

Only absolute O= parameters are supported as Makefile.include will
always resolve relative paths in relation to $(srctree) instead of the
current directory.

Add a call to "make outputmakefile" to verify that the sourcetree is
clean.

This is based on Zhangjins out-of-tree patch.
It extends that work for get_init_cpio support and also drops relative
O= specifications explicitly.

Link: https://lore.kernel.org/lkml/06d96bd81fe812a9718098a383678ad3beba98b1.1691215074.git.falcon@tinylab.org/
Signed-off-by: Thomas Weißschuh <[email protected]>
---
tools/testing/selftests/nolibc/Makefile | 18 ++++++++++++++----
1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/tools/testing/selftests/nolibc/Makefile b/tools/testing/selftests/nolibc/Makefile
index 598d53c5cb7b..21e3f7da2ecf 100644
--- a/tools/testing/selftests/nolibc/Makefile
+++ b/tools/testing/selftests/nolibc/Makefile
@@ -1,9 +1,16 @@
# SPDX-License-Identifier: GPL-2.0
# Makefile for nolibc tests
include ../../../scripts/Makefile.include
+include ../../../scripts/utilities.mak
# We need this for the "cc-option" macro.
include ../../../build/Build.include

+ifneq ($(O),)
+ifneq ($(call is-absolute,$(O)),y)
+$(error Only absolute O= parameters are supported)
+endif
+endif
+
# we're in ".../tools/testing/selftests/nolibc"
ifeq ($(srctree),)
srctree := $(patsubst %/tools/testing/selftests/,%,$(dir $(CURDIR)))
@@ -14,6 +21,8 @@ include $(srctree)/scripts/subarch.include
ARCH = $(SUBARCH)
endif

+objtree ?= $(srctree)
+
# XARCH extends the kernel's ARCH with a few variants of the same
# architecture that only differ by the configuration, the toolchain
# and the Qemu program used. It is copied as-is into ARCH except for
@@ -52,7 +61,7 @@ IMAGE_ppc64le = arch/powerpc/boot/zImage
IMAGE_riscv = arch/riscv/boot/Image
IMAGE_s390 = arch/s390/boot/bzImage
IMAGE_loongarch = arch/loongarch/boot/vmlinuz.efi
-IMAGE = $(IMAGE_$(XARCH))
+IMAGE = $(objtree)/$(IMAGE_$(XARCH))
IMAGE_NAME = $(notdir $(IMAGE))

# default kernel configurations that appear to be usable
@@ -167,6 +176,7 @@ sysroot: sysroot/$(ARCH)/include
sysroot/$(ARCH)/include:
$(Q)rm -rf sysroot/$(ARCH) sysroot/sysroot
$(QUIET_MKDIR)mkdir -p sysroot
+ $(Q)$(MAKE) -C $(srctree) outputmakefile
$(Q)$(MAKE) -C $(srctree)/tools/include/nolibc ARCH=$(ARCH) OUTPUT=$(CURDIR)/sysroot/ headers_standalone
$(Q)mv sysroot/sysroot sysroot/$(ARCH)

@@ -199,7 +209,7 @@ run-user: nolibc-test
$(Q)$(REPORT) $(CURDIR)/run.out

initramfs.cpio: kernel nolibc-test
- $(QUIET_GEN)echo 'file /init nolibc-test 755 0 0' | $(srctree)/usr/gen_init_cpio - > initramfs.cpio
+ $(QUIET_GEN)echo 'file /init nolibc-test 755 0 0' | $(objtree)/usr/gen_init_cpio - > initramfs.cpio

initramfs: nolibc-test
$(QUIET_MKDIR)mkdir -p initramfs
@@ -217,12 +227,12 @@ kernel-standalone: initramfs

# run the tests after building the kernel
run: kernel initramfs.cpio
- $(Q)qemu-system-$(QEMU_ARCH) -display none -no-reboot -kernel "$(srctree)/$(IMAGE)" -initrd initramfs.cpio -serial stdio $(QEMU_ARGS) > "$(CURDIR)/run.out"
+ $(Q)qemu-system-$(QEMU_ARCH) -display none -no-reboot -kernel "$(IMAGE)" -initrd initramfs.cpio -serial stdio $(QEMU_ARGS) > "$(CURDIR)/run.out"
$(Q)$(REPORT) $(CURDIR)/run.out

# re-run the tests from an existing kernel
rerun:
- $(Q)qemu-system-$(QEMU_ARCH) -display none -no-reboot -kernel "$(srctree)/$(IMAGE)" -initrd initramfs.cpio -serial stdio $(QEMU_ARGS) > "$(CURDIR)/run.out"
+ $(Q)qemu-system-$(QEMU_ARCH) -display none -no-reboot -kernel "$(IMAGE)" -initrd initramfs.cpio -serial stdio $(QEMU_ARGS) > "$(CURDIR)/run.out"
$(Q)$(REPORT) $(CURDIR)/run.out

# report with existing test log

--
2.42.0

2023-10-10 12:35:25

by Thomas Weißschuh

[permalink] [raw]
Subject: [PATCH 5/5] selftests/nolibc: generate config automatically

This new target generates a .config if none exists yet.

Also drop the defconfig target with its hidden call to 'mrproper' which
is fairly invasive.
If users want to overwrite their kernel existing kernel configuration
they can do so easily from the toplevel directory.

Signed-off-by: Thomas Weißschuh <[email protected]>
---
tools/testing/selftests/nolibc/Makefile | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/tools/testing/selftests/nolibc/Makefile b/tools/testing/selftests/nolibc/Makefile
index 21e3f7da2ecf..5a3623680f1a 100644
--- a/tools/testing/selftests/nolibc/Makefile
+++ b/tools/testing/selftests/nolibc/Makefile
@@ -173,7 +173,7 @@ all: run

sysroot: sysroot/$(ARCH)/include

-sysroot/$(ARCH)/include:
+sysroot/$(ARCH)/include: $(objtree)/.config
$(Q)rm -rf sysroot/$(ARCH) sysroot/sysroot
$(QUIET_MKDIR)mkdir -p sysroot
$(Q)$(MAKE) -C $(srctree) outputmakefile
@@ -216,13 +216,13 @@ initramfs: nolibc-test
$(call QUIET_INSTALL, initramfs/init)
$(Q)cp nolibc-test initramfs/init

-defconfig:
- $(Q)$(MAKE) -C $(srctree) ARCH=$(ARCH) CC=$(CC) CROSS_COMPILE=$(CROSS_COMPILE) mrproper $(DEFCONFIG) prepare
+$(objtree)/.config:
+ $(Q)$(MAKE) -C $(srctree) ARCH=$(ARCH) CC=$(CC) CROSS_COMPILE=$(CROSS_COMPILE) $(DEFCONFIG)

-kernel:
+kernel: $(objtree)/.config
$(Q)$(MAKE) -C $(srctree) ARCH=$(ARCH) CC=$(CC) CROSS_COMPILE=$(CROSS_COMPILE) $(IMAGE_NAME)

-kernel-standalone: initramfs
+kernel-standalone: $(objtree)/.config initramfs
$(Q)$(MAKE) -C $(srctree) ARCH=$(ARCH) CC=$(CC) CROSS_COMPILE=$(CROSS_COMPILE) $(IMAGE_NAME) CONFIG_INITRAMFS_SOURCE=$(CURDIR)/initramfs

# run the tests after building the kernel

--
2.42.0

2023-10-22 09:22:33

by Willy Tarreau

[permalink] [raw]
Subject: Re: [PATCH 2/5] selftests/nolibc: use EFI -bios for LoongArch qemu

Hi Thomas,

On Tue, Oct 10, 2023 at 02:33:57PM +0200, Thomas Wei?schuh wrote:
> qemu for LoongArch does not work properly with direct kernel boot.
> The kernel will panic during initialization and hang without any output.
>
> When booting in EFI mode everything work correctly.
>
> While users most likely don't have the LoongArch EFI binary installed at
> least an explicit error about 'file not found' is better than a hanging
> test without output that can never succeed.

Agreed. Let's hope at least users will be able to figure what's
missing depending on the message. There's one thing, though, you
hard-coded the path to the file system, and it's unlikely to be
located at the same place for everyone:

-bios /usr/share/edk2/loongarch64/OVMF_CODE.fd

Sure, it's also possible to force QEMU_ARGS but it's becoming complicated
due to the numerous arguments. Maybe use a QEMU_BIOS_loongarch variable
for this ? This way if this starts to generalize to other archs, we can
later simplify it and automatically append -bios when needed.

> Link: https://lore.kernel.org/loongarch/[email protected]/
> Signed-off-by: Thomas Wei?schuh <[email protected]>
>
> ---
> Note: I'm wondering how this worked for anybody else.

Not much surprised. As I mentioned, my qemu currently doesn't support
loongarch so I didn't boot that one. Maybe Zhangjin had this one as
part of his other patches.

Overall, on the principle, Acked-by: Willy Tarreau <[email protected]>

Thanks!
Willy

2023-10-22 09:23:01

by Willy Tarreau

[permalink] [raw]
Subject: Re: [PATCH 3/5] selftests/nolibc: anchor paths in $(srcdir) if possible

On Tue, Oct 10, 2023 at 02:33:58PM +0200, Thomas Wei?schuh wrote:
> It is easier to recognize paths from their well-known location in the
> source tree than having to resolve the relative path in ones head.
>
> Signed-off-by: Thomas Wei?schuh <[email protected]>
> ---
> tools/testing/selftests/nolibc/Makefile | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tools/testing/selftests/nolibc/Makefile b/tools/testing/selftests/nolibc/Makefile
> index 258293639572..598d53c5cb7b 100644
> --- a/tools/testing/selftests/nolibc/Makefile
> +++ b/tools/testing/selftests/nolibc/Makefile
> @@ -167,7 +167,7 @@ sysroot: sysroot/$(ARCH)/include
> sysroot/$(ARCH)/include:
> $(Q)rm -rf sysroot/$(ARCH) sysroot/sysroot
> $(QUIET_MKDIR)mkdir -p sysroot
> - $(Q)$(MAKE) -C ../../../include/nolibc ARCH=$(ARCH) OUTPUT=$(CURDIR)/sysroot/ headers_standalone
> + $(Q)$(MAKE) -C $(srctree)/tools/include/nolibc ARCH=$(ARCH) OUTPUT=$(CURDIR)/sysroot/ headers_standalone
> $(Q)mv sysroot/sysroot sysroot/$(ARCH)
>
> ifneq ($(NOLIBC_SYSROOT),0)
> @@ -177,7 +177,7 @@ nolibc-test: nolibc-test.c sysroot/$(ARCH)/include
> else
> nolibc-test: nolibc-test.c
> $(QUIET_CC)$(CC) $(CFLAGS) $(LDFLAGS) -o $@ \
> - -nostdlib -static -include ../../../include/nolibc/nolibc.h $< -lgcc
> + -nostdlib -static -include $(srctree)/tools/include/nolibc/nolibc.h $< -lgcc
> endif

Agreed, this comes from the early days where we didn't have srctree,
that's better this way.

Acked-by: Willy Tarreau <[email protected]>

Willy

2023-10-22 09:32:28

by Willy Tarreau

[permalink] [raw]
Subject: Re: [PATCH 4/5] selftests/nolibc: support out-of-tree builds

On Tue, Oct 10, 2023 at 02:33:59PM +0200, Thomas Wei?schuh wrote:
> Out of tree builds are much more convenient when building for multiple
> architectures or configurations in parallel.
>
> Only absolute O= parameters are supported as Makefile.include will
> always resolve relative paths in relation to $(srctree) instead of the
> current directory.
>
> Add a call to "make outputmakefile" to verify that the sourcetree is
> clean.

At first this worried me, I thought you meant clean in the git meaning,
but it's clean as in "make clean" from what I'm seeing. Yeah that sounds
reasonable.

> This is based on Zhangjins out-of-tree patch.
> It extends that work for get_init_cpio support and also drops relative
> O= specifications explicitly.

Yeah I remember these discussions about these shortcomings, that's
pretty reasonable.

> Link: https://lore.kernel.org/lkml/06d96bd81fe812a9718098a383678ad3beba98b1.1691215074.git.falcon@tinylab.org/
> Signed-off-by: Thomas Wei?schuh <[email protected]>
(...)

I think you should add a Suggested-by at least since Zhangjin attempted
that work quite a few times already and allowed to make progress in that
direction (maybe even co-developed, I'm not sure).

Acked-by: Willy Tarreau <[email protected]>
Willy

2023-10-22 09:37:34

by Willy Tarreau

[permalink] [raw]
Subject: Re: [PATCH 5/5] selftests/nolibc: generate config automatically

On Tue, Oct 10, 2023 at 02:34:00PM +0200, Thomas Wei?schuh wrote:
> This new target generates a .config if none exists yet.
>
> Also drop the defconfig target with its hidden call to 'mrproper' which
> is fairly invasive.
> If users want to overwrite their kernel existing kernel configuration
> they can do so easily from the toplevel directory.

Hmmm I'm not sure about that one, I pretty much remember seeing failing
arm64 builds when mrproper and prepare were missing.

I would argue that someone starting "make defconfig" does expect the
config to be dropped, hence mrproper to be called as well.

What specific issue did you face with it ? Maybe we can restrict it to
only a few cases ?

Willy

2023-10-24 16:06:39

by Thomas Weißschuh

[permalink] [raw]
Subject: Re: [PATCH 2/5] selftests/nolibc: use EFI -bios for LoongArch qemu

Oct 22, 2023 11:21:16 Willy Tarreau <[email protected]>:

> On Tue, Oct 10, 2023 at 02:33:57PM +0200, Thomas Weißschuh wrote:
>> qemu for LoongArch does not work properly with direct kernel boot.
>> The kernel will panic during initialization and hang without any output.
>>
>> When booting in EFI mode everything work correctly.
>>
>> While users most likely don't have the LoongArch EFI binary installed at
>> least an explicit error about 'file not found' is better than a hanging
>> test without output that can never succeed.
>
> Agreed. Let's hope at least users will be able to figure what's
> missing depending on the message. There's one thing, though, you
> hard-coded the path to the file system, and it's unlikely to be
> located at the same place for everyone:
>
>    -bios /usr/share/edk2/loongarch64/OVMF_CODE.fd
>
> Sure, it's also possible to force QEMU_ARGS but it's becoming complicated
> due to the numerous arguments. Maybe use a QEMU_BIOS_loongarch variable
> for this ? This way if this starts to generalize to other archs, we can
> later simplify it and automatically append -bios when needed.

My hope was for it to be a purely temporary bandaid.
But you are right, let's do it properly from the beginning.

>> Link: https://lore.kernel.org/loongarch/[email protected]/
>> Signed-off-by: Thomas Weißschuh <[email protected]>
>>
>> ---
>> Note: I'm wondering how this worked for anybody else.
>
> Not much surprised. As I mentioned, my qemu currently doesn't support
> loongarch so I didn't boot that one. Maybe Zhangjin had this one as
> part of his other patches.
>
> Overall, on the principle, Acked-by: Willy Tarreau <[email protected]>

Thanks!

2023-10-24 16:11:02

by Thomas Weißschuh

[permalink] [raw]
Subject: Re: [PATCH 4/5] selftests/nolibc: support out-of-tree builds

Oct 22, 2023 11:31:32 Willy Tarreau <[email protected]>:

> On Tue, Oct 10, 2023 at 02:33:59PM +0200, Thomas Weißschuh wrote:
>> Out of tree builds are much more convenient when building for multiple
>> architectures or configurations in parallel.
>>
>> Only absolute O= parameters are supported as Makefile.include will
>> always resolve relative paths in relation to $(srctree) instead of the
>> current directory.
>>
>> Add a call to "make outputmakefile" to verify that the sourcetree is
>> clean.
>
> At first this worried me, I thought you meant clean in the git meaning,
> but it's clean as in "make clean" from what I'm seeing. Yeah that sounds
> reasonable.
>
>> This is based on Zhangjins out-of-tree patch.
>> It extends that work for get_init_cpio support and also drops relative
>> O= specifications explicitly.
>
> Yeah I remember these discussions about these shortcomings, that's
> pretty reasonable.
>
>> Link: https://lore.kernel.org/lkml/06d96bd81fe812a9718098a383678ad3beba98b1.1691215074.git.falcon@tinylab.org/
>> Signed-off-by: Thomas Weißschuh <[email protected]>
> (...)
>
> I think you should add a Suggested-by at least since Zhangjin attempted
> that work quite a few times already and allowed to make progress in that
> direction (maybe even co-developed, I'm not sure).

Indeed. For a proper Co-developed-by I also need
a Signed-off-by by Zhangjin.

Zhangjin, are you fine with giving me one for this
patch?

> Acked-by: Willy Tarreau <[email protected]>

Thanks!

2023-10-24 16:14:25

by Willy Tarreau

[permalink] [raw]
Subject: Re: [PATCH 2/5] selftests/nolibc: use EFI -bios for LoongArch qemu

On Tue, Oct 24, 2023 at 06:06:11PM +0200, Thomas Wei?schuh wrote:
> Oct 22, 2023 11:21:16 Willy Tarreau <[email protected]>:
>
> > On Tue, Oct 10, 2023 at 02:33:57PM +0200, Thomas Wei?schuh wrote:
> >> qemu for LoongArch does not work properly with direct kernel boot.
> >> The kernel will panic during initialization and hang without any output.
> >>
> >> When booting in EFI mode everything work correctly.
> >>
> >> While users most likely don't have the LoongArch EFI binary installed at
> >> least an explicit error about 'file not found' is better than a hanging
> >> test without output that can never succeed.
> >
> > Agreed. Let's hope at least users will be able to figure what's
> > missing depending on the message. There's one thing, though, you
> > hard-coded the path to the file system, and it's unlikely to be
> > located at the same place for everyone:
> >
> > ?? -bios /usr/share/edk2/loongarch64/OVMF_CODE.fd
> >
> > Sure, it's also possible to force QEMU_ARGS but it's becoming complicated
> > due to the numerous arguments. Maybe use a QEMU_BIOS_loongarch variable
> > for this ? This way if this starts to generalize to other archs, we can
> > later simplify it and automatically append -bios when needed.
>
> My hope was for it to be a purely temporary bandaid.
> But you are right, let's do it properly from the beginning.

The right way to think about temporary code is that if it's supposed to
be quick to address, you don't want to introduce a temporary way of
proceeding that will change later as the change will annoy some users.
And if the reason for the temporary step is a temporary difficulty, you
can be certain nobody will ever try to address it and that temporary
will be definitive. So thinking "temporary" should generally ring a
bell "am I going to annoy users for no reason or am I putting myself in
a wrong corner". That's why I really try to avoid anything "temporary".
(But there's no problem with making the wrong choice and regretting
later, of course ;-)).

Cheers,
Willy

2023-10-24 16:17:40

by Thomas Weißschuh

[permalink] [raw]
Subject: Re: [PATCH 5/5] selftests/nolibc: generate config automatically

Oct 22, 2023 11:37:05 Willy Tarreau <[email protected]>:

> On Tue, Oct 10, 2023 at 02:34:00PM +0200, Thomas Weißschuh wrote:
>> This new target generates a .config if none exists yet.
>>
>> Also drop the defconfig target with its hidden call to 'mrproper' which
>> is fairly invasive.
>> If users want to overwrite their kernel existing kernel configuration
>> they can do so easily from the toplevel directory.
>
> Hmmm I'm not sure about that one, I pretty much remember seeing failing
> arm64 builds when mrproper and prepare were missing.
>
> I would argue that someone starting "make defconfig" does expect the
> config to be dropped, hence mrproper to be called as well.
>
> What specific issue did you face with it ? Maybe we can restrict it to
> only a few cases ?

It was mostly a quality improvement for my testscript, where the config was generated
automatically.

And then I dropped the combined defconfig target
because it seemed unnecessary.

Let's drop this patch.

Thomas

2023-10-26 17:52:00

by Zhangjin Wu

[permalink] [raw]
Subject: Re: [PATCH 4/5] selftests/nolibc: support out-of-tree builds

Hi, Thomas

> Oct 22, 2023 11:31:32 Willy Tarreau <[email protected]>:
>
> > (...)
> >
> > I think you should add a Suggested-by at least since Zhangjin attempted
> > that work quite a few times already and allowed to make progress in that
> > direction (maybe even co-developed, I'm not sure).
>
> Indeed. For a proper Co-developed-by I also need
> a Signed-off-by by Zhangjin.
>
> Zhangjin, are you fine with giving me one for this
> patch?
>

It is ok for me, thanks very much to make it really works!

BR,
Zhangjin Wu

> > Acked-by: Willy Tarreau <[email protected]>
>
> Thanks!