From d88260710b212815ae1f461aec24ad926c668d36 Mon Sep 17 00:00:00 2001 From: "Matwey V. Kornilov" Date: Tue, 9 Aug 2022 18:54:07 +0300 Subject: [PATCH 01/10] distro_bootcmd: Introduce support for extension command Try to load required DTB overlays if the board supports extensions and CONFIG_CMD_EXTENSION is enabled. Signed-off-by: Matwey V. Kornilov --- include/config_distro_bootcmd.h | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/include/config_distro_bootcmd.h b/include/config_distro_bootcmd.h index c6e9c49741..2157f3533e 100644 --- a/include/config_distro_bootcmd.h +++ b/include/config_distro_bootcmd.h @@ -160,11 +160,13 @@ "scan_dev_for_efi=" \ "setenv efi_fdtfile ${fdtfile}; " \ BOOTENV_EFI_SET_FDTFILE_FALLBACK \ + BOOTENV_RUN_EXTENSION_INIT \ "for prefix in ${efi_dtb_prefixes}; do " \ "if test -e ${devtype} " \ "${devnum}:${distro_bootpart} " \ "${prefix}${efi_fdtfile}; then " \ "run load_efi_dtb; " \ + BOOTENV_RUN_EXTENSION_APPLY \ "fi;" \ "done;" \ "run boot_efi_bootmgr;" \ @@ -416,6 +418,34 @@ BOOT_TARGET_DEVICES_references_PXE_without_CONFIG_CMD_DHCP_or_PXE #endif +#if defined(CONFIG_CMD_EXTENSION) +#define BOOTENV_RUN_EXTENSION_INIT "run extension_init; " +#define BOOTENV_RUN_EXTENSION_APPLY "run extension_apply; " +#define BOOTENV_SET_EXTENSION_NEED_INIT \ + "extension_need_init=; " \ + "setenv extension_overlay_addr ${fdtoverlay_addr_r}; " +#define BOOTENV_SHARED_EXTENSION \ + "extension_init=" \ + "echo Extension init...; " \ + "if ${extension_need_init}; then " \ + "extension_need_init=false; " \ + "extension scan; " \ + "fi\0" \ + \ + "extension_overlay_cmd=" \ + "load ${devtype} ${devnum}:${distro_bootpart} " \ + "${extension_overlay_addr} ${prefix}${extension_overlay_name}\0" \ + "extension_apply=" \ + "if fdt addr -q ${fdt_addr_r}; then " \ + "extension apply all; " \ + "fi\0" +#else +#define BOOTENV_RUN_EXTENSION_INIT +#define BOOTENV_RUN_EXTENSION_APPLY +#define BOOTENV_SET_EXTENSION_NEED_INIT +#define BOOTENV_SHARED_EXTENSION +#endif + #define BOOTENV_DEV_NAME(devtypeu, devtypel, instance, ...) \ BOOTENV_DEV_NAME_##devtypeu(devtypeu, devtypel, instance, ## __VA_ARGS__) #define BOOTENV_BOOT_TARGETS \ @@ -435,6 +465,7 @@ BOOTENV_SHARED_UBIFS \ BOOTENV_SHARED_EFI \ BOOTENV_SHARED_VIRTIO \ + BOOTENV_SHARED_EXTENSION \ "boot_prefixes=/ /boot/\0" \ "boot_scripts=boot.scr.uimg boot.scr\0" \ "boot_script_dhcp=boot.scr.uimg\0" \ @@ -499,6 +530,7 @@ BOOTENV_SET_NVME_NEED_INIT \ BOOTENV_SET_IDE_NEED_INIT \ BOOTENV_SET_VIRTIO_NEED_INIT \ + BOOTENV_SET_EXTENSION_NEED_INIT \ "for target in ${boot_targets}; do " \ "run bootcmd_${target}; " \ "done\0" From dc0d17c26a3a03634994c59adb0b657807827183 Mon Sep 17 00:00:00 2001 From: Samuel Dionne-Riel Date: Thu, 18 Aug 2022 15:44:04 -0400 Subject: [PATCH 02/10] cmd: Add pause command This command is being introduced with the goal of allowing user-friendly "generic use case" U-Boot builds to pause until user input under some situations. The main use case would be when a boot failure happens, to pause until the user has had time to acknowledge the current state. Tested using: make && ./u-boot -v -T -c 'ut lib lib_test_hush_pause' Signed-off-by: Samuel Dionne-Riel Cc: Simon Glass --- cmd/Kconfig | 6 +++++ cmd/Makefile | 1 + cmd/pause.c | 32 ++++++++++++++++++++++ configs/sandbox64_defconfig | 1 + configs/sandbox_defconfig | 1 + doc/usage/cmd/pause.rst | 53 +++++++++++++++++++++++++++++++++++++ doc/usage/index.rst | 1 + test/cmd/Makefile | 3 +++ test/cmd/test_pause.c | 45 +++++++++++++++++++++++++++++++ 9 files changed, 143 insertions(+) create mode 100644 cmd/pause.c create mode 100644 doc/usage/cmd/pause.rst create mode 100644 test/cmd/test_pause.c diff --git a/cmd/Kconfig b/cmd/Kconfig index 211ebe9c87..67c7d2512f 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -1971,6 +1971,12 @@ config CMD_GETTIME milliseconds. See also the 'bootstage' command which provides more flexibility for boot timing. +config CMD_PAUSE + bool "pause command" + help + Delay execution waiting for any user input. + Useful to allow the user to read a failure log. + config CMD_RNG bool "rng command" depends on DM_RNG diff --git a/cmd/Makefile b/cmd/Makefile index 6e87522b62..7abe1d3630 100644 --- a/cmd/Makefile +++ b/cmd/Makefile @@ -102,6 +102,7 @@ obj-$(CONFIG_CMD_MFSL) += mfsl.o obj-$(CONFIG_CMD_MII) += mii.o obj-$(CONFIG_CMD_MISC) += misc.o obj-$(CONFIG_CMD_MDIO) += mdio.o +obj-$(CONFIG_CMD_PAUSE) += pause.o obj-$(CONFIG_CMD_SLEEP) += sleep.o obj-$(CONFIG_CMD_MMC) += mmc.o obj-$(CONFIG_CMD_OPTEE_RPMB) += optee_rpmb.o diff --git a/cmd/pause.c b/cmd/pause.c new file mode 100644 index 0000000000..c97833c0d7 --- /dev/null +++ b/cmd/pause.c @@ -0,0 +1,32 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * (C) Copyright 2021 + * Samuel Dionne-Riel + */ + +#include +#include + +static int do_pause(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) +{ + char *message = "Press any key to continue..."; + + if (argc == 2) + message = argv[1]; + + /* No newline, so it sticks to the bottom of the screen */ + printf("%s", message); + + /* Wait on "any" key... */ + (void) getchar(); + + /* Since there was no newline, we need it now */ + printf("\n"); + + return CMD_RET_SUCCESS; +} + +U_BOOT_CMD(pause, 2, 1, do_pause, + "delay until user input", + "[prompt] - Wait until users presses any key. [prompt] can be used to customize the message.\n" +); diff --git a/configs/sandbox64_defconfig b/configs/sandbox64_defconfig index 4599db73ed..9a1b6e4bbf 100644 --- a/configs/sandbox64_defconfig +++ b/configs/sandbox64_defconfig @@ -67,6 +67,7 @@ CONFIG_CMD_BMP=y CONFIG_CMD_EFIDEBUG=y CONFIG_CMD_RTC=y CONFIG_CMD_TIME=y +CONFIG_CMD_PAUSE=y CONFIG_CMD_TIMER=y CONFIG_CMD_SOUND=y CONFIG_CMD_QFW=y diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig index 4a0c51c7f5..e6a1718571 100644 --- a/configs/sandbox_defconfig +++ b/configs/sandbox_defconfig @@ -96,6 +96,7 @@ CONFIG_CMD_BOOTCOUNT=y CONFIG_CMD_EFIDEBUG=y CONFIG_CMD_RTC=y CONFIG_CMD_TIME=y +CONFIG_CMD_PAUSE=y CONFIG_CMD_TIMER=y CONFIG_CMD_SOUND=y CONFIG_CMD_QFW=y diff --git a/doc/usage/cmd/pause.rst b/doc/usage/cmd/pause.rst new file mode 100644 index 0000000000..c79e399c02 --- /dev/null +++ b/doc/usage/cmd/pause.rst @@ -0,0 +1,53 @@ +.. SPDX-License-Identifier: GPL-2.0-or-later: + +pause command +============= + +Synopsis +-------- + +:: + + pause [prompt] + + +Description +----------- + +The pause command delays execution waiting for any user input. + +It can accept a single parameter to change the prompt message. + +Examples +-------- + +Using with the default prompt: + +:: + + => pause + Press any key to continue... + + +Using with a custom prompt: + +:: + + => pause 'Prompt for pause...' + Prompt for pause... + +Note that complex prompts require proper quoting: + +:: + + => pause Prompt for pause... + pause - delay until user input + + Usage: + pause [prompt] - Wait until users presses any key. [prompt] can be used to customize the message. + +Return value +------------ + +The return value $? is always set to 0 (true), unless invoked in an invalid +manner. diff --git a/doc/usage/index.rst b/doc/usage/index.rst index 28f9683a3e..5170fa8b4a 100644 --- a/doc/usage/index.rst +++ b/doc/usage/index.rst @@ -52,6 +52,7 @@ Shell commands cmd/mbr cmd/md cmd/mmc + cmd/pause cmd/pinmux cmd/printenv cmd/pstore diff --git a/test/cmd/Makefile b/test/cmd/Makefile index c331757425..1bb02d93a2 100644 --- a/test/cmd/Makefile +++ b/test/cmd/Makefile @@ -5,6 +5,9 @@ ifdef CONFIG_HUSH_PARSER obj-$(CONFIG_CONSOLE_RECORD) += test_echo.o endif +ifdef CONFIG_CONSOLE_RECORD +obj-$(CONFIG_CMD_PAUSE) += test_pause.o +endif obj-y += mem.o obj-$(CONFIG_CMD_ADDRMAP) += addrmap.o obj-$(CONFIG_CMD_FDT) += fdt.o diff --git a/test/cmd/test_pause.c b/test/cmd/test_pause.c new file mode 100644 index 0000000000..2b85cce327 --- /dev/null +++ b/test/cmd/test_pause.c @@ -0,0 +1,45 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Tests for pause command + * + * Copyright 2022, Samuel Dionne-Riel + */ + +#include +#include +#include +#include + +DECLARE_GLOBAL_DATA_PTR; + +static int lib_test_hush_pause(struct unit_test_state *uts) +{ + /* Test default message */ + console_record_reset_enable(); + /* Cook a newline when the command is expected to pause */ + console_in_puts("\n"); + ut_assertok(run_command("pause", 0)); + console_record_readline(uts->actual_str, sizeof(uts->actual_str)); + ut_asserteq_str("Press any key to continue...", uts->actual_str); + ut_assertok(ut_check_console_end(uts)); + + /* Test provided message */ + console_record_reset_enable(); + /* Cook a newline when the command is expected to pause */ + console_in_puts("\n"); + ut_assertok(run_command("pause 'Prompt for pause...'", 0)); + console_record_readline(uts->actual_str, sizeof(uts->actual_str)); + ut_asserteq_str("Prompt for pause...", uts->actual_str); + ut_assertok(ut_check_console_end(uts)); + + /* Test providing more than one params */ + console_record_reset_enable(); + /* No newline cooked here since the command is expected to fail */ + ut_asserteq(1, run_command("pause a b", 0)); + console_record_readline(uts->actual_str, sizeof(uts->actual_str)); + ut_asserteq_str("pause - delay until user input", uts->actual_str); + ut_asserteq(1, ut_check_console_end(uts)); + + return 0; +} +LIB_TEST(lib_test_hush_pause, 0); From 583f1b2f10be36f86a2da3686d406bfd90cfbc70 Mon Sep 17 00:00:00 2001 From: Sergei Antonov Date: Sun, 21 Aug 2022 16:34:20 +0300 Subject: [PATCH 03/10] arm: ARMv4 assembly compatibility There is currently a problem that U-Boot can not work on ARMv4 because assembly imlementations of memcpy() and some other functions use "bx lr" instruction that is not available on ARMv4 ("mov pc, lr" should be used instead). A working preprocessor-based solution to this problem is found in arch/arm/lib/relocate.S. Move it to the "ret" macro in arch/arm/include/asm/assembler.h and change all "bx lr" code to "ret lr" in functions that may run on ARMv4. Linux source code deals with this problem in the same manner. v1 -> v2: Comment update. Pointed out by Andre Przywara. Signed-off-by: Sergei Antonov CC: Samuel Holland CC: Ye Li CC: Simon Glass CC: Andre Przywara CC: Marek Vasut CC: Sean Anderson CC: Tom Rini --- arch/arm/include/asm/assembler.h | 10 ++++++++-- arch/arm/lib/lib1funcs.S | 8 ++++---- arch/arm/lib/memcpy.S | 6 +++--- arch/arm/lib/relocate.S | 10 ++-------- arch/arm/lib/setjmp.S | 4 ++-- 5 files changed, 19 insertions(+), 19 deletions(-) diff --git a/arch/arm/include/asm/assembler.h b/arch/arm/include/asm/assembler.h index b146918586..8d42ef4823 100644 --- a/arch/arm/include/asm/assembler.h +++ b/arch/arm/include/asm/assembler.h @@ -58,16 +58,22 @@ #endif /* - * We only support cores that support at least Thumb-1 and thus we use - * 'bx lr' + * Use 'bx lr' everywhere except ARMv4 (without 'T') where only 'mov pc, lr' + * works */ .irp c,,eq,ne,cs,cc,mi,pl,vs,vc,hi,ls,ge,lt,gt,le,hs,lo .macro ret\c, reg + + /* ARMv4- don't know bx lr but the assembler fails to see that */ +#ifdef __ARM_ARCH_4__ + mov\c pc, \reg +#else .ifeqs "\reg", "lr" bx\c \reg .else mov\c pc, \reg .endif +#endif .endm .endr diff --git a/arch/arm/lib/lib1funcs.S b/arch/arm/lib/lib1funcs.S index 700eee5fbb..7ff4446dd6 100644 --- a/arch/arm/lib/lib1funcs.S +++ b/arch/arm/lib/lib1funcs.S @@ -377,7 +377,7 @@ ENTRY(__gnu_thumb1_case_sqi) lsls r1, r1, #1 add lr, lr, r1 pop {r1} - bx lr + ret lr ENDPROC(__gnu_thumb1_case_sqi) .popsection @@ -391,7 +391,7 @@ ENTRY(__gnu_thumb1_case_uqi) lsls r1, r1, #1 add lr, lr, r1 pop {r1} - bx lr + ret lr ENDPROC(__gnu_thumb1_case_uqi) .popsection @@ -406,7 +406,7 @@ ENTRY(__gnu_thumb1_case_shi) lsls r1, r1, #1 add lr, lr, r1 pop {r0, r1} - bx lr + ret lr ENDPROC(__gnu_thumb1_case_shi) .popsection @@ -421,7 +421,7 @@ ENTRY(__gnu_thumb1_case_uhi) lsls r1, r1, #1 add lr, lr, r1 pop {r0, r1} - bx lr + ret lr ENDPROC(__gnu_thumb1_case_uhi) .popsection #endif diff --git a/arch/arm/lib/memcpy.S b/arch/arm/lib/memcpy.S index eee7a219ce..a1c996f94e 100644 --- a/arch/arm/lib/memcpy.S +++ b/arch/arm/lib/memcpy.S @@ -59,7 +59,7 @@ #endif ENTRY(memcpy) cmp r0, r1 - bxeq lr + reteq lr enter r4, lr @@ -148,7 +148,7 @@ ENTRY(memcpy) str1b r0, ip, cs, abort=21f exit r4, lr - bx lr + ret lr 9: rsb ip, ip, #4 cmp ip, #2 @@ -258,7 +258,7 @@ ENTRY(memcpy) .macro copy_abort_end ldmfd sp!, {r4, lr} - bx lr + ret lr .endm ENDPROC(memcpy) diff --git a/arch/arm/lib/relocate.S b/arch/arm/lib/relocate.S index 5102bfabde..dd6f2e3bd5 100644 --- a/arch/arm/lib/relocate.S +++ b/arch/arm/lib/relocate.S @@ -61,7 +61,7 @@ ENTRY(relocate_vectors) stmia r1!, {r2-r8,r10} #endif #endif - bx lr + ret lr ENDPROC(relocate_vectors) @@ -127,13 +127,7 @@ relocate_done: mcr p15, 0, r0, c7, c10, 4 /* drain write buffer */ #endif - /* ARMv4- don't know bx lr but the assembler fails to see that */ - -#ifdef __ARM_ARCH_4__ - mov pc, lr -#else - bx lr -#endif + ret lr ENDPROC(relocate_code) diff --git a/arch/arm/lib/setjmp.S b/arch/arm/lib/setjmp.S index 176a1d5315..2f041aeef0 100644 --- a/arch/arm/lib/setjmp.S +++ b/arch/arm/lib/setjmp.S @@ -17,7 +17,7 @@ ENTRY(setjmp) mov ip, sp stm a1, {v1-v8, ip, lr} mov a1, #0 - bx lr + ret lr ENDPROC(setjmp) .popsection @@ -31,6 +31,6 @@ ENTRY(longjmp) bne 1f mov a1, #1 1: - bx lr + ret lr ENDPROC(longjmp) .popsection From a55014d09b29d7bc2cbaeb8e41a155c9a257a183 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Wed, 3 Aug 2022 12:13:08 -0600 Subject: [PATCH 04/10] Makefile: Allow LTO to be disabled for a build LTO (Link-Time Optimisation) is an very useful feature which can significantly reduce the size of U-Boot binaries. So far it has been made available for selected ARM boards and sandbox. However, incremental builds are much slower when LTO is used. For example, an incremental build of sandbox takes 2.1 seconds on my machine, but 6.7 seconds with LTO enabled. Add a NO_LTO parameter to the build, similar to NO_SDL, so it can be disabled during development if needed, for faster builds. Add some documentation about LTO while we are here. Signed-off-by: Simon Glass --- Makefile | 17 ++++++++++++----- arch/arm/config.mk | 4 ++-- arch/arm/include/asm/global_data.h | 2 +- doc/build/gcc.rst | 17 +++++++++++++++++ scripts/Makefile.spl | 2 +- 5 files changed, 33 insertions(+), 9 deletions(-) diff --git a/Makefile b/Makefile index 541e942ed5..27375f033f 100644 --- a/Makefile +++ b/Makefile @@ -643,6 +643,13 @@ export CFLAGS_EFI # Compiler flags to add when building EFI app export CFLAGS_NON_EFI # Compiler flags to remove when building EFI app export EFI_TARGET # binutils target if EFI is natively supported +export LTO_ENABLE + +# This is y if LTO is enabled for this build. See NO_LTO=1 to disable LTO +ifeq ($(NO_LTO),) +LTO_ENABLE=$(if $(CONFIG_LTO),y) +endif + # If board code explicitly specified LDSCRIPT or CONFIG_SYS_LDSCRIPT, use # that (or fail if absent). Otherwise, search for a linker script in a # standard location. @@ -708,16 +715,16 @@ endif LTO_CFLAGS := LTO_FINAL_LDFLAGS := export LTO_CFLAGS LTO_FINAL_LDFLAGS -ifdef CONFIG_LTO +ifeq ($(LTO_ENABLE),y) ifeq ($(cc-name),clang) - LTO_CFLAGS += -flto + LTO_CFLAGS += -DLTO_ENABLE -flto LTO_FINAL_LDFLAGS += -flto AR = $(shell $(CC) -print-prog-name=llvm-ar) NM = $(shell $(CC) -print-prog-name=llvm-nm) else NPROC := $(shell nproc 2>/dev/null || echo 1) - LTO_CFLAGS += -flto=$(NPROC) + LTO_CFLAGS += -DLTO_ENABLE -flto=$(NPROC) LTO_FINAL_LDFLAGS += -fuse-linker-plugin -flto=$(NPROC) # use plugin aware tools @@ -1765,7 +1772,7 @@ ARCH_POSTLINK := $(wildcard $(srctree)/arch/$(ARCH)/Makefile.postlink) # Generate linker list symbols references to force compiler to not optimize # them away when compiling with LTO -ifdef CONFIG_LTO +ifeq ($(LTO_ENABLE),y) u-boot-keep-syms-lto := keep-syms-lto.o u-boot-keep-syms-lto_c := $(patsubst %.o,%.c,$(u-boot-keep-syms-lto)) @@ -1787,7 +1794,7 @@ endif # Rule to link u-boot # May be overridden by arch/$(ARCH)/config.mk -ifdef CONFIG_LTO +ifeq ($(LTO_ENABLE),y) quiet_cmd_u-boot__ ?= LTO $@ cmd_u-boot__ ?= \ $(CC) -nostdlib -nostartfiles \ diff --git a/arch/arm/config.mk b/arch/arm/config.mk index b3548ce243..2065438d05 100644 --- a/arch/arm/config.mk +++ b/arch/arm/config.mk @@ -15,11 +15,11 @@ CFLAGS_NON_EFI := -fno-pic -ffixed-r9 -ffunction-sections -fdata-sections \ -fstack-protector-strong CFLAGS_EFI := -fpic -fshort-wchar -ifneq ($(CONFIG_LTO)$(CONFIG_USE_PRIVATE_LIBGCC),yy) +ifneq ($(LTO_ENABLE)$(CONFIG_USE_PRIVATE_LIBGCC),yy) LDFLAGS_FINAL += --gc-sections endif -ifndef CONFIG_LTO +ifneq ($(LTO_ENABLE),y) PLATFORM_RELFLAGS += -ffunction-sections -fdata-sections endif diff --git a/arch/arm/include/asm/global_data.h b/arch/arm/include/asm/global_data.h index 6ee2a76761..cd6112dfcd 100644 --- a/arch/arm/include/asm/global_data.h +++ b/arch/arm/include/asm/global_data.h @@ -101,7 +101,7 @@ struct arch_global_data { #include -#if defined(__clang__) || defined(CONFIG_LTO) +#if defined(__clang__) || defined(LTO_ENABLE) #define DECLARE_GLOBAL_DATA_PTR #define gd get_gd() diff --git a/doc/build/gcc.rst b/doc/build/gcc.rst index ee544ad87e..a71f860a48 100644 --- a/doc/build/gcc.rst +++ b/doc/build/gcc.rst @@ -152,6 +152,23 @@ of dtc is new enough. It also makes sure that pylibfdt is present, if needed Note that the :doc:`tools` are always built with the included version of libfdt so it is not possible to build U-Boot tools with a system libfdt, at present. +Link-time optimisation (LTO) +~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +U-Boot supports link-time optimisation which can reduce the size of the final +U-Boot binaries, particularly with SPL. + +At present this can be enabled by ARM boards by adding `CONFIG_LTO=y` into the +defconfig file. Other architectures are not supported. LTO is enabled by default +for sandbox. + +This does incur a link-time penalty of several seconds. For faster incremental +builds during development, you can disable it by setting `NO_LTO` to `1`. + +.. code-block:: bash + + NO_LTO=1 make + Other build targets ~~~~~~~~~~~~~~~~~~~ diff --git a/scripts/Makefile.spl b/scripts/Makefile.spl index 3bafeb4fe9..0b3a51da13 100644 --- a/scripts/Makefile.spl +++ b/scripts/Makefile.spl @@ -489,7 +489,7 @@ endif # Rule to link u-boot-spl # May be overridden by arch/$(ARCH)/config.mk -ifdef CONFIG_LTO +ifeq ($(LTO_ENABLE),y) quiet_cmd_u-boot-spl ?= LTO $@ cmd_u-boot-spl ?= \ ( \ From 1aa168ca8fa472620a0aa72ff10b6eb4d0414e03 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Wed, 3 Aug 2022 12:13:09 -0600 Subject: [PATCH 05/10] ci: Add a test for a non-LTO build Check that sandbox builds and runs tests OK with LTO disabled. Signed-off-by: Simon Glass --- .azure-pipelines.yml | 4 ++++ .gitlab-ci.yml | 7 +++++++ 2 files changed, 11 insertions(+) diff --git a/.azure-pipelines.yml b/.azure-pipelines.yml index 0fa92479b4..d78a170d0c 100644 --- a/.azure-pipelines.yml +++ b/.azure-pipelines.yml @@ -243,6 +243,9 @@ stages: sandbox_clang: TEST_PY_BD: "sandbox" OVERRIDE: "-O clang-13" + sandbox_nolto: + TEST_PY_BD: "sandbox" + BUILD_ENV: "NO_LTO=1" sandbox_spl: TEST_PY_BD: "sandbox_spl" TEST_PY_TEST_SPEC: "test_ofplatdata or test_handoff or test_spl" @@ -354,6 +357,7 @@ stages: export TEST_PY_ID="${TEST_PY_ID}" export TEST_PY_TEST_SPEC="${TEST_PY_TEST_SPEC}" export OVERRIDE="${OVERRIDE}" + export BUILD_ENV="${BUILD_ENV}" EOF cat << "EOF" >> test.sh # the below corresponds to .gitlab-ci.yml "before_script" diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 5592862f74..8e94bf8d4e 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -33,6 +33,7 @@ stages: script: # If we've been asked to use clang only do one configuration. - export UBOOT_TRAVIS_BUILD_DIR=/tmp/${TEST_PY_BD} + - echo BUILD_ENV ${BUILD_ENV} - tools/buildman/buildman -o ${UBOOT_TRAVIS_BUILD_DIR} -w -E -W -e --board ${TEST_PY_BD} ${OVERRIDE} - cp ~/grub_x86.efi $UBOOT_TRAVIS_BUILD_DIR/ @@ -254,6 +255,12 @@ sandbox with clang test.py: OVERRIDE: "-O clang-13" <<: *buildman_and_testpy_dfn +sandbox without LTO test.py: + variables: + TEST_PY_BD: "sandbox" + BUILD_ENV: "NO_LTO=1" + <<: *buildman_and_testpy_dfn + sandbox_spl test.py: variables: TEST_PY_BD: "sandbox_spl" From e033c180d0327e8fa791660ae26cdebd5e400153 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Mon, 1 Aug 2022 07:58:44 -0600 Subject: [PATCH 06/10] dm: rtc: Make use of ut_assertnonnull() Use this (newish) macro since it is designed for the purpose of making sure things are non-NULL. Signed-off-by: Simon Glass --- test/dm/rtc.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/test/dm/rtc.c b/test/dm/rtc.c index c7f9f8f0ce..a8349756c1 100644 --- a/test/dm/rtc.c +++ b/test/dm/rtc.c @@ -66,7 +66,7 @@ static int dm_test_rtc_set_get(struct unit_test_state *uts) ut_assertok(dm_rtc_get(dev, &now)); ut_assertok(i2c_emul_find(dev, &emul)); - ut_assert(emul != NULL); + ut_assertnonnull(emul); /* Tell the RTC to go into manual mode */ old_offset = sandbox_i2c_rtc_set_offset(emul, false, 0); @@ -161,7 +161,7 @@ static int dm_test_rtc_read_write(struct unit_test_state *uts) ut_asserteq(memcmp(buf, "at", 3), 0); ut_assertok(i2c_emul_find(dev, &emul)); - ut_assert(emul != NULL); + ut_assertnonnull(emul); old_offset = sandbox_i2c_rtc_set_offset(emul, false, 0); ut_assertok(dm_rtc_get(dev, &time)); @@ -245,7 +245,7 @@ static int dm_test_rtc_reset(struct unit_test_state *uts) ut_assertok(dm_rtc_get(dev, &now)); ut_assertok(i2c_emul_find(dev, &emul)); - ut_assert(emul != NULL); + ut_assertnonnull(emul); old_base_time = sandbox_i2c_rtc_get_set_base_time(emul, 0); @@ -274,9 +274,9 @@ static int dm_test_rtc_dual(struct unit_test_state *uts) ut_assertok(dm_rtc_get(dev2, &now2)); ut_assertok(i2c_emul_find(dev1, &emul1)); - ut_assert(emul1 != NULL); + ut_assertnonnull(emul1); ut_assertok(i2c_emul_find(dev2, &emul2)); - ut_assert(emul2 != NULL); + ut_assertnonnull(emul2); offset = sandbox_i2c_rtc_set_offset(emul1, false, -1); sandbox_i2c_rtc_set_offset(emul2, false, offset + 1); From ea94d053e19ee3341301cf999a8dd78e37b83cca Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Mon, 1 Aug 2022 07:58:45 -0600 Subject: [PATCH 07/10] test: Allow running tests multiple times Some tests can have race conditions which are hard to detect on a single one. Add a way to run tests more than once, to help with this. Each individual test is run the requested number of times before moving to the next test. If any runs failed, a message is shown. This is most useful when running a single test, since running all tests multiple times can take a while. Signed-off-by: Simon Glass --- arch/sandbox/cpu/spl.c | 2 +- doc/develop/tests_sandbox.rst | 24 ++++++++++++++++++++++++ include/test/test.h | 2 ++ include/test/ut.h | 3 ++- test/cmd_ut.c | 12 ++++++++++-- test/dm/test-dm.c | 13 ++++++++++--- test/test-main.c | 14 +++++++++++--- 7 files changed, 60 insertions(+), 10 deletions(-) diff --git a/arch/sandbox/cpu/spl.c b/arch/sandbox/cpu/spl.c index fe5d44d36e..1d49a9bd10 100644 --- a/arch/sandbox/cpu/spl.c +++ b/arch/sandbox/cpu/spl.c @@ -89,7 +89,7 @@ void spl_board_init(void) int ret; ret = ut_run_list("spl", NULL, tests, count, - state->select_unittests); + state->select_unittests, 1); /* continue execution into U-Boot */ } } diff --git a/doc/develop/tests_sandbox.rst b/doc/develop/tests_sandbox.rst index 40cf8ecdd7..8e42a32afb 100644 --- a/doc/develop/tests_sandbox.rst +++ b/doc/develop/tests_sandbox.rst @@ -119,6 +119,30 @@ You can easily use gdb on these tests, without needing --gdbserver:: You can then single-step and look at variables as needed. +Running tests multiple times +---------------------------- + +Some tests can have race conditions which are hard to detect on a single +one. It is possible to run each individual test multiple times, before moving +to the next test, with the '-r' flag. + +This is most useful when running a single test, since running all tests +multiple times can take a while. + +For example:: + + => ut dm -r1000 dm_test_rtc_set_get + ... + Test: dm_test_rtc_set_get: rtc.c (flat tree) + Test: dm_test_rtc_set_get: rtc.c + test/dm/rtc.c:257, dm_test_rtc_reset(): old_base_time == base_time: Expected 0x62e7453c (1659323708), got 0x62e7453d (1659323709) + Test: dm_test_rtc_set_get: rtc.c (flat tree) + Test: dm_test_rtc_set_get: rtc.c + Test: dm_test_rtc_set_get: rtc.c (flat tree) + ... + Test dm_test_rtc_reset failed 3 times + + Running sandbox_spl tests directly ---------------------------------- diff --git a/include/test/test.h b/include/test/test.h index c888d68b1e..2b68331b54 100644 --- a/include/test/test.h +++ b/include/test/test.h @@ -20,6 +20,7 @@ * @testdev: Test device * @force_fail_alloc: Force all memory allocs to fail * @skip_post_probe: Skip uclass post-probe processing + * @runs_per_test: Number of times to run each test (typically 1) * @expect_str: Temporary string used to hold expected string value * @actual_str: Temporary string used to hold actual string value */ @@ -32,6 +33,7 @@ struct unit_test_state { struct udevice *testdev; int force_fail_alloc; int skip_post_probe; + int runs_per_test; char expect_str[512]; char actual_str[512]; }; diff --git a/include/test/ut.h b/include/test/ut.h index 18740f5807..f7d1d18f7c 100644 --- a/include/test/ut.h +++ b/include/test/ut.h @@ -403,9 +403,10 @@ void test_set_state(struct unit_test_state *uts); * @count: Number of tests to run * @select_name: Name of a single test to run (from the list provided). If NULL * then all tests are run + * @runs_per_test: Number of times to run each test (typically 1) * Return: 0 if all tests passed, -1 if any failed */ int ut_run_list(const char *name, const char *prefix, struct unit_test *tests, - int count, const char *select_name); + int count, const char *select_name, int runs_per_test); #endif diff --git a/test/cmd_ut.c b/test/cmd_ut.c index 3789c6b784..11c219b48a 100644 --- a/test/cmd_ut.c +++ b/test/cmd_ut.c @@ -18,10 +18,17 @@ int cmd_ut_category(const char *name, const char *prefix, struct unit_test *tests, int n_ents, int argc, char *const argv[]) { + int runs_per_text = 1; int ret; + if (argc > 1 && !strncmp("-r", argv[1], 2)) { + runs_per_text = dectoul(argv[1] + 2, NULL); + argv++; + argc++; + } + ret = ut_run_list(name, prefix, tests, n_ents, - argc > 1 ? argv[1] : NULL); + argc > 1 ? argv[1] : NULL, runs_per_text); return ret ? CMD_RET_FAILURE : 0; } @@ -168,7 +175,8 @@ static char ut_help_text[] = #ifdef CONFIG_CMD_LOADM "ut loadm [test-name]- test of parameters and load memory blob\n" #endif - ; + "All commands accept an optional [-r] flag before [test-name], to\n" + "run each test multiple times ( is in decimal)"; #endif /* CONFIG_SYS_LONGHELP */ U_BOOT_CMD( diff --git a/test/dm/test-dm.c b/test/dm/test-dm.c index f5cda81bbf..eb3581333b 100644 --- a/test/dm/test-dm.c +++ b/test/dm/test-dm.c @@ -29,13 +29,14 @@ DECLARE_GLOBAL_DATA_PTR; * "fdt_pre_reloc"), or NULL to run all * Return: 0 if all tests passed, 1 if not */ -static int dm_test_run(const char *test_name) +static int dm_test_run(const char *test_name, int runs_per_text) { struct unit_test *tests = UNIT_TEST_SUITE_START(dm_test); const int n_ents = UNIT_TEST_SUITE_COUNT(dm_test); int ret; - ret = ut_run_list("driver model", "dm_test_", tests, n_ents, test_name); + ret = ut_run_list("driver model", "dm_test_", tests, n_ents, test_name, + runs_per_text); return ret ? CMD_RET_FAILURE : 0; } @@ -43,9 +44,15 @@ static int dm_test_run(const char *test_name) int do_ut_dm(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) { const char *test_name = NULL; + int runs_per_text = 1; + if (argc > 1 && !strncmp("-r", argv[1], 2)) { + runs_per_text = dectoul(argv[1] + 2, NULL); + argv++; + argc++; + } if (argc > 1) test_name = argv[1]; - return dm_test_run(test_name); + return dm_test_run(test_name, runs_per_text); } diff --git a/test/test-main.c b/test/test-main.c index 31837e57a8..4f1c54b0f9 100644 --- a/test/test-main.c +++ b/test/test-main.c @@ -390,11 +390,17 @@ static int ut_run_tests(struct unit_test_state *uts, const char *prefix, for (test = tests; test < tests + count; test++) { const char *test_name = test->name; - int ret; + int ret, i, old_fail_count; if (!test_matches(prefix, test_name, select_name)) continue; - ret = ut_run_test_live_flat(uts, test, select_name); + old_fail_count = uts->fail_count; + for (i = 0; i < uts->runs_per_test; i++) + ret = ut_run_test_live_flat(uts, test, select_name); + if (uts->fail_count != old_fail_count) { + printf("Test %s failed %d times\n", select_name, + uts->fail_count - old_fail_count); + } found++; if (ret == -EAGAIN) continue; @@ -408,7 +414,8 @@ static int ut_run_tests(struct unit_test_state *uts, const char *prefix, } int ut_run_list(const char *category, const char *prefix, - struct unit_test *tests, int count, const char *select_name) + struct unit_test *tests, int count, const char *select_name, + int runs_per_test) { struct unit_test_state uts = { .fail_count = 0 }; bool has_dm_tests = false; @@ -432,6 +439,7 @@ int ut_run_list(const char *category, const char *prefix, printf("Running %d %s tests\n", count, category); uts.of_root = gd_of_root(); + uts.runs_per_test = runs_per_test; ret = ut_run_tests(&uts, prefix, tests, count, select_name); if (ret == -ENOENT) From c4d7247a389ae2770ddcdada5143401068a696c0 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Mon, 1 Aug 2022 07:58:46 -0600 Subject: [PATCH 08/10] dm: rtc: Avoid a race in the rtc_reset test Since resetting the RTC on sandbox causes it to read the base time from the system, we cannot rely on this being unchanged since it was last read. Allow for a one-second delay. Reviewed-by: Heinrich Schuchardt Fixes: https://source.denx.de/u-boot/u-boot/-/issues/4 Reported-by: Bin Meng Reported-by: Tom Rini Suggested-by: Rasmus Villemoes Signed-off-by: Simon Glass --- test/dm/rtc.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/test/dm/rtc.c b/test/dm/rtc.c index a8349756c1..e23905b3e2 100644 --- a/test/dm/rtc.c +++ b/test/dm/rtc.c @@ -251,10 +251,15 @@ static int dm_test_rtc_reset(struct unit_test_state *uts) ut_asserteq(0, sandbox_i2c_rtc_get_set_base_time(emul, -1)); - /* Resetting the RTC should put he base time back to normal */ + /* + * Resetting the RTC should put the base time back to normal. Allow for + * a one-second adjustment in case the time flips over while this + * test process is pre-empted, since reset_time() in i2c_rtc_emul.c + * reads the time from the OS. + */ ut_assertok(dm_rtc_reset(dev)); base_time = sandbox_i2c_rtc_get_set_base_time(emul, -1); - ut_asserteq(old_base_time, base_time); + ut_assert(base_time - old_base_time <= 1); return 0; } From fc7ceae0d5b7a017c3fed88a8bfe2eb3d24058a9 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Mon, 1 Aug 2022 07:58:47 -0600 Subject: [PATCH 09/10] dm: rtc: Try to avoid a race in rtc_set_get test It seems that the time can change in between getting it and reading the offset. Check for this and try again if this happens. Signed-off-by: Simon Glass --- test/dm/rtc.c | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/test/dm/rtc.c b/test/dm/rtc.c index e23905b3e2..50086ffcf3 100644 --- a/test/dm/rtc.c +++ b/test/dm/rtc.c @@ -60,16 +60,27 @@ static int dm_test_rtc_set_get(struct unit_test_state *uts) { struct rtc_time now, time, cmp; struct udevice *dev, *emul; - long offset, old_offset, old_base_time; + long offset, check_offset, old_offset, old_base_time; + int i; ut_assertok(uclass_get_device(UCLASS_RTC, 0, &dev)); - ut_assertok(dm_rtc_get(dev, &now)); ut_assertok(i2c_emul_find(dev, &emul)); ut_assertnonnull(emul); - /* Tell the RTC to go into manual mode */ - old_offset = sandbox_i2c_rtc_set_offset(emul, false, 0); + /* Get the offset, putting the RTC into manual mode */ + i = 0; + do { + check_offset = sandbox_i2c_rtc_set_offset(emul, false, 0); + ut_assertok(dm_rtc_get(dev, &now)); + + /* Tell the RTC to go into manual mode */ + old_offset = sandbox_i2c_rtc_set_offset(emul, false, 0); + + /* If the times changed in that period, read it again */ + } while (++i < 2 && check_offset != old_offset); + ut_asserteq(check_offset, old_offset); + old_base_time = sandbox_i2c_rtc_get_set_base_time(emul, -1); memset(&time, '\0', sizeof(time)); @@ -127,7 +138,8 @@ static int dm_test_rtc_set_get(struct unit_test_state *uts) ut_asserteq(now.tm_sec + 1, cmp.tm_sec); } - old_offset = sandbox_i2c_rtc_set_offset(emul, true, 0); + /* return RTC to normal mode */ + sandbox_i2c_rtc_set_offset(emul, true, 0); return 0; } From 21ddac140e3040b2693c1a5558a84c19a879c04f Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Mon, 1 Aug 2022 07:58:48 -0600 Subject: [PATCH 10/10] dm: rtc: Try to handle the localtime() race At present the sandbox timer uses localtime() which can jump around twice a year when daylight-saving time changes. It would be tricky to make use of gmtime() since we still need to present the time in local time, as seems to be required by U-Boot's RTC interface. The problem can only happen once, so use a loop to detect it and try again. This should be sufficient to detect either a change in the 'second' value, or a daylight-saving change. We can assume that the latter also incorporates a 'second' change, so there is no need to loop more than twice. Signed-off-by: Simon Glass --- test/dm/rtc.c | 28 +++++++++++++++++----------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/test/dm/rtc.c b/test/dm/rtc.c index 50086ffcf3..bf97dbbd2f 100644 --- a/test/dm/rtc.c +++ b/test/dm/rtc.c @@ -252,6 +252,7 @@ static int dm_test_rtc_reset(struct unit_test_state *uts) struct rtc_time now; struct udevice *dev, *emul; long old_base_time, base_time; + int i; ut_assertok(uclass_get_device(UCLASS_RTC, 0, &dev)); ut_assertok(dm_rtc_get(dev, &now)); @@ -259,19 +260,24 @@ static int dm_test_rtc_reset(struct unit_test_state *uts) ut_assertok(i2c_emul_find(dev, &emul)); ut_assertnonnull(emul); - old_base_time = sandbox_i2c_rtc_get_set_base_time(emul, 0); + i = 0; + do { + old_base_time = sandbox_i2c_rtc_get_set_base_time(emul, 0); - ut_asserteq(0, sandbox_i2c_rtc_get_set_base_time(emul, -1)); + ut_asserteq(0, sandbox_i2c_rtc_get_set_base_time(emul, -1)); - /* - * Resetting the RTC should put the base time back to normal. Allow for - * a one-second adjustment in case the time flips over while this - * test process is pre-empted, since reset_time() in i2c_rtc_emul.c - * reads the time from the OS. - */ - ut_assertok(dm_rtc_reset(dev)); - base_time = sandbox_i2c_rtc_get_set_base_time(emul, -1); - ut_assert(base_time - old_base_time <= 1); + ut_assertok(dm_rtc_reset(dev)); + base_time = sandbox_i2c_rtc_get_set_base_time(emul, -1); + + /* + * Resetting the RTC should put the base time back to normal. + * Allow for a one-timeadjustment in case the time flips over + * while this test process is pre-empted (either by a second + * or a daylight-saving change), since reset_time() in + * i2c_rtc_emul.c reads the time from the OS. + */ + } while (++i < 2 && base_time != old_base_time); + ut_asserteq(old_base_time, base_time); return 0; }