[PATCH v2 00/30] Allow building sandbox with MSYS2

This expands the existing work to allow sandbox to build and run natively on Windows using MSYS2.
This produces a native application that can run on Windows. This is quite different from using WSL to cross-compile a Linux application, which is then run in a virtual environment.
It also fixes a few issues so that binman can be used.
There are various limitations and some features do not work fully yet. In particular, weak functions are not well supported on Windows so these are disabled. Various minor compiler-flag and filename adjustments are also needed.
This does not support SDL, so U-Boot has no display. This is potentially possible, but requires using minggw instead of the base toolchain [1] so needs to be dealt with separately.
Note: This series is split into two parts: generic fixes and MSYS2 stuff.
Patches up until this one should be ready to be applied:
Makefile: Disable LTO when not building with gcc
After that, some of the MSYS2 stuff may need more work, depending on review comments.
[1] https://gist.github.com/thales17/fb2e4cff60890a51d9dddd4c6e832ad2
Changes in v2: - Split out u_boot_pylib from the subprocess patch - move u_boot_pylib changes to a separate patch - Use LIBEXT instead of SOEXT - Update comment and use zz to make it less likely we have a problem - Make LTO depend on !MSYS2 rather than adding another check - Also disable LTO for clang, except with sandbox - Update commit message to mention other OSes - Check for __MSYS__ instead of __linux - Use cc-option and ld-option instead - Use EXEEXT instead of ELFEXT - Add an awk script to augment the built-in link script - Clearify the documentation to explain the environment better - Update the cover letter to better explain the motivation
Simon Glass (30): u_boot_pylib: Correct name of readme u_boot_pylib: Avoid deleting the test_util.py file u_boot_pylib: Make pty optional binman: Avoid using a symlink pylibfdt: Allow building on Windows Fix Makefile warning about parallel targets mkimage: Correct checking of configuration node sandbox: Drop dead code from Makefile sandbox: Correct SDL.h header inclusion sandbox: Include errno.h in the test header sections: Drop use of linux/types.h sandbox: Drop incorrect inclusion of linux/types.h sandbox: Drop inclusion of os.h in sscanf.c test: Avoid strange symbols in the assembler file ctype: Avoid using a symlink Makefile: Disable LTO when not building with gcc Kbuild: Detect including an MSYS2 path sandbox: Disable raw Ethernet on MSYS2 sandbox: Drop signal handling for MSYS2 sandbox: Fix up setting of monitor_len on MSYS2 Makefile: Disable unsupported compiler options with PE Makefile: Correct the ans1_compiler rule for MSYS2 sandbox: Augment the linker script for MSYS2 sandbox: Provide an EFI link script for PE sandbox: Allow weak symbols to be dropped build: Disable weak symbols for MSYS2 doc: Update the MSYS2 packages and versions doc: Show how to build sandbox for MSYS2 Makefile: Drop unwind tables for MSYS CI: Enable sandbox build for Windows
.azure-pipelines.yml | 27 +++++++++ Kconfig | 17 ++++++ Makefile | 31 ++++++++-- arch/sandbox/Makefile | 7 --- arch/sandbox/config.mk | 23 +++++++- arch/sandbox/cpu/Makefile | 2 + arch/sandbox/cpu/os.c | 3 +- arch/sandbox/cpu/sdl.c | 2 +- arch/sandbox/cpu/u-boot-pe.lds.in | 25 ++++++++ arch/sandbox/include/asm/test.h | 1 + arch/x86/lib/crt0_x86_64_efi.S | 2 + arch/x86/lib/pe_x86_64_efi.lds | 83 +++++++++++++++++++++++++++ cmd/bootefi.c | 3 +- cmd/bootz.c | 3 + common/board_f.c | 2 +- common/usb.c | 3 + doc/build/gcc.rst | 40 +++++++++++++ doc/build/tools.rst | 20 ++++--- drivers/core/root.c | 3 + drivers/net/Makefile | 2 + drivers/spi/sandbox_spi.c | 3 + env/env.c | 6 ++ include/asm-generic/sections.h | 16 +++--- include/ctype.h | 7 ++- include/linux/compiler_attributes.h | 4 ++ include/os.h | 2 - include/test/test.h | 6 +- lib/efi_loader/Makefile | 8 +++ lib/efi_loader/efi_image_loader.c | 3 + lib/efi_loader/efi_runtime.c | 4 ++ lib/lmb.c | 4 +- lib/sscanf.c | 1 - lib/time.c | 3 + scripts/Kbuild.include | 3 +- scripts/Makefile.build | 2 +- scripts/Makefile.lib | 9 ++- scripts/add_to_rdata.awk | 25 ++++++++ scripts/dtc/pylibfdt/Makefile | 16 +++--- scripts/make_pip.sh | 7 ++- tools/Makefile | 11 +++- tools/binman/binman | 11 +++- tools/image-host.c | 4 +- tools/u_boot_pylib/README.rst | 2 +- tools/u_boot_pylib/cros_subprocess.py | 28 +++++++-- tools/u_boot_pylib/pyproject.toml | 2 +- 45 files changed, 420 insertions(+), 66 deletions(-) create mode 100644 arch/sandbox/cpu/u-boot-pe.lds.in create mode 100644 arch/x86/lib/pe_x86_64_efi.lds mode change 120000 => 100644 include/ctype.h create mode 100644 scripts/add_to_rdata.awk mode change 120000 => 100755 tools/binman/binman

This has an rst prefix rather than md. Fix it.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
tools/u_boot_pylib/README.rst | 2 +- tools/u_boot_pylib/pyproject.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/u_boot_pylib/README.rst b/tools/u_boot_pylib/README.rst index 93858f5571da..b1bab4364896 100644 --- a/tools/u_boot_pylib/README.rst +++ b/tools/u_boot_pylib/README.rst @@ -1,6 +1,6 @@ .. SPDX-License-Identifier: GPL-2.0+
-# U-Boot Python Library +U-Boot Python Library =====================
This is a Python library used by various U-Boot tools, including patman, diff --git a/tools/u_boot_pylib/pyproject.toml b/tools/u_boot_pylib/pyproject.toml index 3f33caf6f8d2..31a4c0adae87 100644 --- a/tools/u_boot_pylib/pyproject.toml +++ b/tools/u_boot_pylib/pyproject.toml @@ -9,7 +9,7 @@ authors = [ { name="Simon Glass", email="sjg@chromium.org" }, ] description = "U-Boot python library" -readme = "README.md" +readme = "README.rst" requires-python = ">=3.7" classifiers = [ "Programming Language :: Python :: 3",

This is needed, so update the removal code to keep it.
Signed-off-by: Simon Glass sjg@chromium.org ---
Changes in v2: - Split out u_boot_pylib from the subprocess patch
scripts/make_pip.sh | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/scripts/make_pip.sh b/scripts/make_pip.sh index 4602dcf61c88..ef43e028e385 100755 --- a/scripts/make_pip.sh +++ b/scripts/make_pip.sh @@ -91,7 +91,12 @@ find ${dest} -name __pycache__ -type f -exec rm {} ; find ${dest} -depth -name __pycache__ -exec rmdir 112 ;
# Remove test files -rm -rf ${dest}/*test* +for path in ${dest}/*test*; do + echo ${path} + if ! [[ "${path}" =~ .*test_util.* ]]; then + rm -rf ${path} + fi +done
mkdir ${dir}/tests cd ${dir}

This library is not available on Windows. Detect this and work around it by using a normal pipe.
Signed-off-by: Simon Glass sjg@chromium.org ---
Changes in v2: - move u_boot_pylib changes to a separate patch
tools/u_boot_pylib/cros_subprocess.py | 28 ++++++++++++++++++++++----- 1 file changed, 23 insertions(+), 5 deletions(-)
diff --git a/tools/u_boot_pylib/cros_subprocess.py b/tools/u_boot_pylib/cros_subprocess.py index cd614f38a648..35cef7333f3a 100644 --- a/tools/u_boot_pylib/cros_subprocess.py +++ b/tools/u_boot_pylib/cros_subprocess.py @@ -16,12 +16,18 @@ progress information and filter output in real time.
import errno import os -import pty import select import subprocess import sys import unittest
+try: + import pty + HAVE_PTY = True +except: + # For Windows + HAVE_PTY = False +
# Import these here so the caller does not need to import subprocess also. PIPE = subprocess.PIPE @@ -74,11 +80,17 @@ class Popen(subprocess.Popen): stderr_pty = None
if stdout == PIPE_PTY: - stdout_pty = pty.openpty() - stdout = os.fdopen(stdout_pty[1]) + if HAVE_PTY: + stdout_pty = pty.openpty() + stdout = os.fdopen(stdout_pty[1]) + else: + stdout = PIPE if stderr == PIPE_PTY: - stderr_pty = pty.openpty() - stderr = os.fdopen(stderr_pty[1]) + if HAVE_PTY: + stderr_pty = pty.openpty() + stderr = os.fdopen(stderr_pty[1]) + else: + stderr = PIPE
super(Popen, self).__init__(args, stdin=stdin, stdout=stdout, stderr=stderr, shell=shell, cwd=cwd, env=env, @@ -156,6 +168,12 @@ class Popen(subprocess.Popen): Note also that if you set stderr to STDOUT, then stderr will be empty and the combined output will just be the same as stdout. """ + if not HAVE_PTY: + stdout, stderr = self.communicate(input_buf) + stdout = self.convert_data(stdout) + stderr = self.convert_data(stderr) + combined = self.convert_data(stdout + stderr) + return (stdout, stderr, combined)
read_set = [] write_set = []

Use a small Python script instead of a symlink to avoid problems with Windows.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
tools/binman/binman | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) mode change 120000 => 100755 tools/binman/binman
diff --git a/tools/binman/binman b/tools/binman/binman deleted file mode 120000 index 11a5d8e18ab7..000000000000 --- a/tools/binman/binman +++ /dev/null @@ -1 +0,0 @@ -main.py \ No newline at end of file diff --git a/tools/binman/binman b/tools/binman/binman new file mode 100755 index 000000000000..2b0633073abc --- /dev/null +++ b/tools/binman/binman @@ -0,0 +1,10 @@ +#!/usr/bin/env python3 +# SPDX-License-Identifier: GPL-2.0+ + +import os + +our_path = os.path.dirname(os.path.realpath(__file__)) + +import main + +main.start_binman()

Hi Simon,
On Sun, Apr 30, 2023 at 9:30 AM Simon Glass sjg@chromium.org wrote:
Use a small Python script instead of a symlink to avoid problems with Windows.
Signed-off-by: Simon Glass sjg@chromium.org
(no changes since v1)
tools/binman/binman | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) mode change 120000 => 100755 tools/binman/binman
This patch does not seem necessary if:
1. Enable Windows developer mode 2. export MSYS=winsymlinks:native
Regards, Bin

Handle the different shared-object extension with MSYS2 by creating a new SOEXT variable.
Signed-off-by: Simon Glass sjg@chromium.org ---
Changes in v2: - Use LIBEXT instead of SOEXT
Makefile | 11 +++++++++++ scripts/dtc/pylibfdt/Makefile | 16 +++++++++------- 2 files changed, 20 insertions(+), 7 deletions(-)
diff --git a/Makefile b/Makefile index 166acba27032..2453a80eca62 100644 --- a/Makefile +++ b/Makefile @@ -39,6 +39,17 @@ else ifeq ("riscv64", $(MK_ARCH)) endif undefine MK_ARCH
+# Building on Windows with MSYS2 +export MSYS_VERSION = $(if $(findstring Msys, $(shell uname -o)),$(word 1, $(subst ., ,$(shell uname -r))),0) +# $(info The version of MSYS you are running is $(MSYS_VERSION) (0 meaning not MSYS at all)) + +# Sets the extension to use for shared-object files +ifeq ($(MSYS_VERSION),0) +export LIBEXT := so +else +export LIBEXT := dll +endif + # Avoid funny character set dependencies unexport LC_ALL LC_COLLATE=C diff --git a/scripts/dtc/pylibfdt/Makefile b/scripts/dtc/pylibfdt/Makefile index e442d5c24201..314ef91e527b 100644 --- a/scripts/dtc/pylibfdt/Makefile +++ b/scripts/dtc/pylibfdt/Makefile @@ -7,6 +7,8 @@ LIBFDT_srcdir = $(abspath $(srctree)/$(src)/../libfdt)
include $(LIBFDT_srcdir)/Makefile.libfdt
+LIBFILE := _libfdt.$(LIBEXT) + # Unfortunately setup.py (or actually the Python distutil implementation) puts # files into the same directory as the .i file. We cannot touch the source # directory, so we "ship" .i file into the objtree. @@ -29,16 +31,16 @@ quiet_cmd_pymod = PYMOD $@ rebuild: $(src)/setup.py $(PYLIBFDT_srcs) @# Remove the library since otherwise Python doesn't seem to regenerate @# the libfdt.py file if it is missing. - @rm -f $(obj)/_libfdt*.so + @rm -f $(obj)/_libfdt*.$(LIBEXT) $(call if_changed,pymod) - @# Rename the file to _libfdt.so so this Makefile doesn't run every time - @if [ ! -e $(obj)/_libfdt.so ]; then \ - mv $(obj)/_libfdt*.so $(obj)/_libfdt.so; \ + @# Rename the file to $(LIBFILE) so this Makefile doesn't run every time + @if [ ! -e $(obj)/$(LIBFILE) ]; then \ + mv $(obj)/_libfdt*.$(LIBEXT) $(obj)/$(LIBFILE); \ fi
-$(obj)/_libfdt.so $(obj)/libfdt.py &: rebuild +$(obj)/$(LIBFILE) $(obj)/libfdt.py &: rebuild @:
-always += _libfdt.so libfdt.py +always += $(LIBFILE) libfdt.py
-clean-files += libfdt.i _libfdt.so libfdt.py libfdt_wrap.c +clean-files += libfdt.i $(LIBFILE) libfdt.py libfdt_wrap.c

Hi Simon,
On Sun, Apr 30, 2023 at 9:30 AM Simon Glass sjg@chromium.org wrote:
Handle the different shared-object extension with MSYS2 by creating a new SOEXT variable.
Signed-off-by: Simon Glass sjg@chromium.org
Changes in v2:
- Use LIBEXT instead of SOEXT
Makefile | 11 +++++++++++ scripts/dtc/pylibfdt/Makefile | 16 +++++++++------- 2 files changed, 20 insertions(+), 7 deletions(-)
diff --git a/Makefile b/Makefile index 166acba27032..2453a80eca62 100644 --- a/Makefile +++ b/Makefile @@ -39,6 +39,17 @@ else ifeq ("riscv64", $(MK_ARCH)) endif undefine MK_ARCH
+# Building on Windows with MSYS2 +export MSYS_VERSION = $(if $(findstring Msys, $(shell uname -o)),$(word 1, $(subst ., ,$(shell uname -r))),0)
It looks like the exact version of MSYS2 is not useful to U-Boot build? What we only need is to detect whether we are building for MSYS2.
So parsing `shell uname -o` should be enough. Then the variable name can be named as MSYS?
+# $(info The version of MSYS you are running is $(MSYS_VERSION) (0 meaning not MSYS at all))
Commented out?
+# Sets the extension to use for shared-object files +ifeq ($(MSYS_VERSION),0) +export LIBEXT := so +else +export LIBEXT := dll +endif
# Avoid funny character set dependencies unexport LC_ALL LC_COLLATE=C diff --git a/scripts/dtc/pylibfdt/Makefile b/scripts/dtc/pylibfdt/Makefile index e442d5c24201..314ef91e527b 100644 --- a/scripts/dtc/pylibfdt/Makefile +++ b/scripts/dtc/pylibfdt/Makefile @@ -7,6 +7,8 @@ LIBFDT_srcdir = $(abspath $(srctree)/$(src)/../libfdt)
include $(LIBFDT_srcdir)/Makefile.libfdt
+LIBFILE := _libfdt.$(LIBEXT)
# Unfortunately setup.py (or actually the Python distutil implementation) puts # files into the same directory as the .i file. We cannot touch the source # directory, so we "ship" .i file into the objtree. @@ -29,16 +31,16 @@ quiet_cmd_pymod = PYMOD $@ rebuild: $(src)/setup.py $(PYLIBFDT_srcs) @# Remove the library since otherwise Python doesn't seem to regenerate @# the libfdt.py file if it is missing.
@rm -f $(obj)/_libfdt*.so
@rm -f $(obj)/_libfdt*.$(LIBEXT) $(call if_changed,pymod)
@# Rename the file to _libfdt.so so this Makefile doesn't run every time
@if [ ! -e $(obj)/_libfdt.so ]; then \
mv $(obj)/_libfdt*.so $(obj)/_libfdt.so; \
@# Rename the file to $(LIBFILE) so this Makefile doesn't run every time
@if [ ! -e $(obj)/$(LIBFILE) ]; then \
mv $(obj)/_libfdt*.$(LIBEXT) $(obj)/$(LIBFILE); \ fi
-$(obj)/_libfdt.so $(obj)/libfdt.py &: rebuild +$(obj)/$(LIBFILE) $(obj)/libfdt.py &: rebuild @:
-always += _libfdt.so libfdt.py +always += $(LIBFILE) libfdt.py
-clean-files += libfdt.i _libfdt.so libfdt.py libfdt_wrap.c
+clean-files += libfdt.i $(LIBFILE) libfdt.py libfdt_wrap.c
Regards, Bin

These targets are not really parallel; they are handled one at a time when invoked by other rules. Some recent versions of make (e.g. on MSYS2) give a warning about this. Split out the rules to avoid the warning.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
tools/Makefile | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/tools/Makefile b/tools/Makefile index 38699b069d63..22ae5c30351f 100644 --- a/tools/Makefile +++ b/tools/Makefile @@ -263,7 +263,16 @@ HOSTCFLAGS_sha512.o := -pedantic -DCONFIG_SHA512 -DCONFIG_SHA384 quiet_cmd_wrap = WRAP $@ cmd_wrap = echo "#include <../$(patsubst $(obj)/%,%,$@)>" >$@
-$(obj)/boot/%.c $(obj)/common/%.c $(obj)/env/%.c $(obj)/lib/%.c: +$(obj)/boot/%.c: + $(call cmd,wrap) + +$(obj)/common/%.c: + $(call cmd,wrap) + +$(obj)/env/%.c: + $(call cmd,wrap) + +$(obj)/lib/%.c: $(call cmd,wrap)
clean-dirs := lib common

Hi Simon,
On Sun, Apr 30, 2023 at 9:30 AM Simon Glass sjg@chromium.org wrote:
These targets are not really parallel; they are handled one at a time when invoked by other rules. Some recent versions of make (e.g. on MSYS2) give a warning about this. Split out the rules to avoid the warning.
I did not see any warning without this patch.
Also it looks both MSYS2 and Ubuntu 22.04 has make v4.3 which doesn't emit any warning.
Signed-off-by: Simon Glass sjg@chromium.org
(no changes since v1)
tools/Makefile | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/tools/Makefile b/tools/Makefile index 38699b069d63..22ae5c30351f 100644 --- a/tools/Makefile +++ b/tools/Makefile @@ -263,7 +263,16 @@ HOSTCFLAGS_sha512.o := -pedantic -DCONFIG_SHA512 -DCONFIG_SHA384 quiet_cmd_wrap = WRAP $@ cmd_wrap = echo "#include <../$(patsubst $(obj)/%,%,$@)>" >$@
-$(obj)/boot/%.c $(obj)/common/%.c $(obj)/env/%.c $(obj)/lib/%.c: +$(obj)/boot/%.c:
$(call cmd,wrap)
+$(obj)/common/%.c:
$(call cmd,wrap)
+$(obj)/env/%.c:
$(call cmd,wrap)
+$(obj)/lib/%.c: $(call cmd,wrap)
clean-dirs := lib common
Regards, Bin

This returns a negative value on error. Update the logic to detect this and avoid a segfault when the node is not found.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
tools/image-host.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/tools/image-host.c b/tools/image-host.c index 4a24dee8153c..e4e44e33badd 100644 --- a/tools/image-host.c +++ b/tools/image-host.c @@ -1336,8 +1336,10 @@ int fit_check_sign(const void *fit, const void *key, int ret;
cfg_noffset = fit_conf_get_node(fit, fit_uname_config); - if (!cfg_noffset) + if (cfg_noffset < 0) { + fprintf(stderr, "Configuration node not found\n"); return -1; + }
printf("Verifying Hash Integrity for node '%s'... ", fdt_get_name(fit, cfg_noffset, NULL));

On Sun, Apr 30, 2023 at 9:30 AM Simon Glass sjg@chromium.org wrote:
This returns a negative value on error. Update the logic to detect this and avoid a segfault when the node is not found.
Signed-off-by: Simon Glass sjg@chromium.org
(no changes since v1)
tools/image-host.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
Reviewed-by: Bin Meng bmeng@tinylab.org

This is not used, since the same code appears in cpu/Makefile
Drop it.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
arch/sandbox/Makefile | 7 ------- 1 file changed, 7 deletions(-)
diff --git a/arch/sandbox/Makefile b/arch/sandbox/Makefile index a335f8acfde6..f6cf859f249c 100644 --- a/arch/sandbox/Makefile +++ b/arch/sandbox/Makefile @@ -4,10 +4,3 @@ head-y := arch/sandbox/cpu/start.o arch/sandbox/cpu/os.o head-$(CONFIG_SANDBOX_SDL) += arch/sandbox/cpu/sdl.o libs-y += arch/sandbox/cpu/ libs-y += arch/sandbox/lib/ - -# sdl.c fails to compile with -fshort-wchar using musl. -cmd_cc_sdl.o = $(CC) $(filter-out -nostdinc -fshort-wchar, \ - $(patsubst -I%,-idirafter%,$(c_flags))) -fno-lto -c -o $@ $< - -$(obj)/sdl.o: $(src)/sdl.c FORCE - $(call if_changed_dep,cc_sdl.o)

On Sun, Apr 30, 2023 at 9:30 AM Simon Glass sjg@chromium.org wrote:
This is not used, since the same code appears in cpu/Makefile
Drop it.
Signed-off-by: Simon Glass sjg@chromium.org
(no changes since v1)
arch/sandbox/Makefile | 7 ------- 1 file changed, 7 deletions(-)
Reviewed-by: Bin Meng bmeng@tinylab.org

We rely on sdl-config to provide the include directory for this header file, so should not add the include directory in the C file. Drop it so that this works as expected on Windows.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
arch/sandbox/cpu/sdl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/sandbox/cpu/sdl.c b/arch/sandbox/cpu/sdl.c index 2c570ed8d164..31d3e9c32dec 100644 --- a/arch/sandbox/cpu/sdl.c +++ b/arch/sandbox/cpu/sdl.c @@ -7,7 +7,7 @@ #include <unistd.h> #include <stdbool.h> #include <linux/input.h> -#include <SDL2/SDL.h> +#include <SDL.h> #include <asm/state.h>
/**

On Sun, Apr 30, 2023 at 9:30 AM Simon Glass sjg@chromium.org wrote:
We rely on sdl-config to provide the include directory for this header file, so should not add the include directory in the C file. Drop it so that this works as expected on Windows.
Signed-off-by: Simon Glass sjg@chromium.org
(no changes since v1)
arch/sandbox/cpu/sdl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Bin Meng bmeng@tinylab.org

Add this so that the header file can used without first including the errno.h header.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
arch/sandbox/include/asm/test.h | 1 + 1 file changed, 1 insertion(+)
diff --git a/arch/sandbox/include/asm/test.h b/arch/sandbox/include/asm/test.h index e482271fe975..86968ea6d345 100644 --- a/arch/sandbox/include/asm/test.h +++ b/arch/sandbox/include/asm/test.h @@ -8,6 +8,7 @@ #ifndef __ASM_TEST_H #define __ASM_TEST_H
+#include <errno.h> #include <video.h> #include <pci_ids.h>

On Sun, Apr 30, 2023 at 9:30 AM Simon Glass sjg@chromium.org wrote:
Add this so that the header file can used without first including the errno.h header.
Signed-off-by: Simon Glass sjg@chromium.org
(no changes since v1)
arch/sandbox/include/asm/test.h | 1 + 1 file changed, 1 insertion(+)
Reviewed-by: Bin Meng bmeng@tinylab.org

Use 'unsigned long' instead of 'ulong' so this file does not need to include the linux/types.h header file. This allows it to be built with MSYS2.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
include/asm-generic/sections.h | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-)
diff --git a/include/asm-generic/sections.h b/include/asm-generic/sections.h index 267f1db73f23..4ed9a8478cc5 100644 --- a/include/asm-generic/sections.h +++ b/include/asm-generic/sections.h @@ -8,8 +8,6 @@ #ifndef _ASM_GENERIC_SECTIONS_H_ #define _ASM_GENERIC_SECTIONS_H_
-#include <linux/types.h> - /* References to section boundaries */
extern char _text[], _stext[], _etext[]; @@ -71,7 +69,7 @@ extern char __image_copy_end[]; extern void _start(void);
/* - * ARM defines its symbols as char[]. Other arches define them as ulongs. + * ARM defines its symbols as char[]. Other archs define them as unsigned long */ #ifdef CONFIG_ARM
@@ -86,13 +84,13 @@ extern char __rel_dyn_end[]; #else /* don't use offsets: */
/* Exports from the Linker Script */ -extern ulong __data_end; -extern ulong __rel_dyn_start; -extern ulong __rel_dyn_end; -extern ulong __bss_end; -extern ulong _image_binary_end; +extern unsigned long __data_end; +extern unsigned long __rel_dyn_start; +extern unsigned long __rel_dyn_end; +extern unsigned long __bss_end; +extern unsigned long _image_binary_end;
-extern ulong _TEXT_BASE; /* code start */ +extern unsigned long _TEXT_BASE; /* code start */
#endif

On Sat, Apr 29, 2023 at 07:29:44PM -0600, Simon Glass wrote:
Use 'unsigned long' instead of 'ulong' so this file does not need to include the linux/types.h header file. This allows it to be built with MSYS2.
Signed-off-by: Simon Glass sjg@chromium.org
Reviewed-by: Tom Rini trini@konsulko.com

On Sun, Apr 30, 2023 at 9:30 AM Simon Glass sjg@chromium.org wrote:
Use 'unsigned long' instead of 'ulong' so this file does not need to include the linux/types.h header file. This allows it to be built with MSYS2.
Signed-off-by: Simon Glass sjg@chromium.org
(no changes since v1)
include/asm-generic/sections.h | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-)
Reviewed-by: Bin Meng bmeng@tinylab.org

This header file should only be included in U-Boot standalone files, not those built in the Linux environment. When not building for Linux, the header file does not exist, except in the U-Boot tree. In any case, it should not be used.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
arch/sandbox/cpu/os.c | 1 - include/os.h | 2 -- 2 files changed, 3 deletions(-)
diff --git a/arch/sandbox/cpu/os.c b/arch/sandbox/cpu/os.c index 9e93a0fa571f..e76568ebdd32 100644 --- a/arch/sandbox/cpu/os.c +++ b/arch/sandbox/cpu/os.c @@ -26,7 +26,6 @@ #include <sys/time.h> #include <sys/types.h> #include <linux/compiler_attributes.h> -#include <linux/types.h>
#include <asm/fuzzing_engine.h> #include <asm/getopt.h> diff --git a/include/os.h b/include/os.h index 968412b0a822..561ef1264b13 100644 --- a/include/os.h +++ b/include/os.h @@ -11,8 +11,6 @@ #ifndef __OS_H__ #define __OS_H__
-#include <linux/types.h> - struct rtc_time; struct sandbox_state;

On Sun, Apr 30, 2023 at 9:30 AM Simon Glass sjg@chromium.org wrote:
This header file should only be included in U-Boot standalone files, not those built in the Linux environment. When not building for Linux, the header file does not exist, except in the U-Boot tree. In any case, it should not be used.
Signed-off-by: Simon Glass sjg@chromium.org
(no changes since v1)
arch/sandbox/cpu/os.c | 1 - include/os.h | 2 -- 2 files changed, 3 deletions(-)
Reviewed-by: Bin Meng bmeng@tinylab.org

There is no need for this file and it should not be included unless sandbox is being used. Drop it.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
lib/sscanf.c | 1 - 1 file changed, 1 deletion(-)
diff --git a/lib/sscanf.c b/lib/sscanf.c index 4c35c035fe10..b79f6d79255c 100644 --- a/lib/sscanf.c +++ b/lib/sscanf.c @@ -17,7 +17,6 @@
#if !defined HAVE_LIBC
-#include <os.h> #include <linux/kernel.h> #include <linux/ctype.h> #include <vsprintf.h>

On Sun, Apr 30, 2023 at 9:30 AM Simon Glass sjg@chromium.org wrote:
There is no need for this file and it should not be included unless sandbox is being used. Drop it.
Signed-off-by: Simon Glass sjg@chromium.org
(no changes since v1)
lib/sscanf.c | 1 - 1 file changed, 1 deletion(-)
Reviewed-by: Bin Meng bmeng@tinylab.org

This works correctly on Linux with ELF but not on Windows with PE, since it creates assembly symbols with invalid names.
Use the lowest/highest valid characters instead. This should still work correctly, since we have no tests starting with $ and none starting with 'zz' at present.
Signed-off-by: Simon Glass sjg@chromium.org ---
Changes in v2: - Update comment and use zz to make it less likely we have a problem
include/test/test.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/include/test/test.h b/include/test/test.h index 838e3ce8a8f3..fd07238a7b0b 100644 --- a/include/test/test.h +++ b/include/test/test.h @@ -126,9 +126,9 @@ struct unit_test { #define UNIT_TEST_SUITE_COUNT(_suite) \ ll_entry_count(struct unit_test, ut_ ## _suite)
-/* Use ! and ~ so that all tests will be sorted between these two values */ -#define UNIT_TEST_ALL_START() ll_entry_start(struct unit_test, ut_!) -#define UNIT_TEST_ALL_END() ll_entry_start(struct unit_test, ut_~) +/* Use $ and zz so that all tests will be sorted between these two values */ +#define UNIT_TEST_ALL_START() ll_entry_start(struct unit_test, ut_$) +#define UNIT_TEST_ALL_END() ll_entry_start(struct unit_test, ut_zz) #define UNIT_TEST_ALL_COUNT() (UNIT_TEST_ALL_END() - UNIT_TEST_ALL_START())
/* Sizes for devres tests */

On Sun, Apr 30, 2023 at 9:30 AM Simon Glass sjg@chromium.org wrote:
This works correctly on Linux with ELF but not on Windows with PE, since it creates assembly symbols with invalid names.
Use the lowest/highest valid characters instead. This should still work correctly, since we have no tests starting with $ and none starting with 'zz' at present.
Signed-off-by: Simon Glass sjg@chromium.org
Changes in v2:
- Update comment and use zz to make it less likely we have a problem
include/test/test.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
Reviewed-by: Bin Meng bmeng@tinylab.org Tested-by: Bin Meng bmeng@tinylab.org

Windows doesn't really support symlinks so fails to build this file. Use a single-line #include instead.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
include/ctype.h | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) mode change 120000 => 100644 include/ctype.h
diff --git a/include/ctype.h b/include/ctype.h deleted file mode 120000 index 9e43f9c6c6c4..000000000000 --- a/include/ctype.h +++ /dev/null @@ -1 +0,0 @@ -linux/ctype.h \ No newline at end of file diff --git a/include/ctype.h b/include/ctype.h new file mode 100644 index 000000000000..155e3c63e59d --- /dev/null +++ b/include/ctype.h @@ -0,0 +1,6 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/* + * Copyright 2023 Google LLC + */ + +#include <linux/ctype.h>

Hi Simon,
On Sun, Apr 30, 2023 at 9:30 AM Simon Glass sjg@chromium.org wrote:
Windows doesn't really support symlinks so fails to build this file. Use a single-line #include instead.
Signed-off-by: Simon Glass sjg@chromium.org
(no changes since v1)
include/ctype.h | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) mode change 120000 => 100644 include/ctype.h
This patch does not seem necessary if:
1. Enable Windows developer mode 2. export MSYS=winsymlinks:native
Regards, Bin

For MSYS2 this creates a lot of errors of the form:
`__stack_chk_fail' referenced in section `.text' of ...ltrans.o: defined in discarded section `.text' of common/stackprot.o (symbol from plugin)
For clang it doesn't work, except with sandbox.
Update the dependency to (hopefully) deal with all of that.
Signed-off-by: Simon Glass sjg@chromium.org ---
Changes in v2: - Make LTO depend on !MSYS2 rather than adding another check - Also disable LTO for clang, except with sandbox
Kconfig | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/Kconfig b/Kconfig index 888b9984ac3b..9ac816abef1c 100644 --- a/Kconfig +++ b/Kconfig @@ -72,6 +72,9 @@ config CLANG_VERSION int default $(shell,$(srctree)/scripts/clang-version.sh $(CC))
+config CC_IS_MSYS + def_bool $(success,uname -o | grep -q Msys) + choice prompt "Optimization level" default CC_OPTIMIZE_FOR_SIZE @@ -121,6 +124,8 @@ config ARCH_SUPPORTS_LTO config LTO bool "Enable Link Time Optimizations" depends on ARCH_SUPPORTS_LTO + depends on CC_IS_GCC || (CC_IS_CLANG && SANDBOX) + depends on !CC_IS_MSYS help This option enables Link Time Optimization (LTO), a mechanism which allows the compiler to optimize between different compilation units.

On Sat, Apr 29, 2023 at 07:29:49PM -0600, Simon Glass wrote:
For MSYS2 this creates a lot of errors of the form:
`__stack_chk_fail' referenced in section `.text' of ...ltrans.o: defined in discarded section `.text' of common/stackprot.o (symbol from plugin)
For clang it doesn't work, except with sandbox.
Update the dependency to (hopefully) deal with all of that.
Signed-off-by: Simon Glass sjg@chromium.org
Reviewed-by: Tom Rini trini@konsulko.com

Hi Simon,
On Sun, Apr 30, 2023 at 9:30 AM Simon Glass sjg@chromium.org wrote:
For MSYS2 this creates a lot of errors of the form:
`__stack_chk_fail' referenced in section `.text' of ...ltrans.o: defined in discarded section `.text' of common/stackprot.o (symbol from plugin)
For clang it doesn't work, except with sandbox.
To clarify, you didn't use clang on Windows to build Sandbox?
Update the dependency to (hopefully) deal with all of that.
Signed-off-by: Simon Glass sjg@chromium.org
Changes in v2:
- Make LTO depend on !MSYS2 rather than adding another check
- Also disable LTO for clang, except with sandbox
Kconfig | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/Kconfig b/Kconfig index 888b9984ac3b..9ac816abef1c 100644 --- a/Kconfig +++ b/Kconfig @@ -72,6 +72,9 @@ config CLANG_VERSION int default $(shell,$(srctree)/scripts/clang-version.sh $(CC))
+config CC_IS_MSYS
def_bool $(success,uname -o | grep -q Msys)
choice prompt "Optimization level" default CC_OPTIMIZE_FOR_SIZE @@ -121,6 +124,8 @@ config ARCH_SUPPORTS_LTO config LTO bool "Enable Link Time Optimizations" depends on ARCH_SUPPORTS_LTO
depends on CC_IS_GCC || (CC_IS_CLANG && SANDBOX)
This looks like a separate patch needed for adding dependency check for LTO?
depends on !CC_IS_MSYS help This option enables Link Time Optimization (LTO), a mechanism which allows the compiler to optimize between different compilation units.
--
Regards, Bin

The source-tree directory is prepended to relative include paths, but this does not work on Windows, where a path may have a drive letter like C: at the start of it.
This breaks SDL which includes an absolute path to the header directory to the C flags, e.g. -IC:/msys64/mingw64/include/SDL2
Add this as a special case to leave these absolute paths alone on Windows.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
scripts/Kbuild.include | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include index 62e0207f91b4..411a768a7767 100644 --- a/scripts/Kbuild.include +++ b/scripts/Kbuild.include @@ -205,9 +205,10 @@ clean := -f $(srctree)/scripts/Makefile.clean obj hdr-inst := -f $(srctree)/scripts/Makefile.headersinst obj
# Prefix -I with $(srctree) if it is not an absolute path. +# Detect C: (C drive) with MSYS2 # skip if -I has no parameter addtree = $(if $(patsubst -I%,%,$(1)), \ -$(if $(filter-out -I/% -I./% -I../%,$(1)),$(patsubst -I%,-I$(srctree)/%,$(1)),$(1)),$(1)) +$(if $(filter-out -I/% -I./% -I../% -IC:%,$(1)),$(patsubst -I%,-I$(srctree)/%,$(1)),$(1)),$(1))
# Find all -I options and call addtree flags = $(foreach o,$($(1)),$(if $(filter -I%,$(o)),$(call addtree,$(o)),$(o)))

On Sat, Apr 29, 2023 at 07:29:50PM -0600, Simon Glass wrote:
The source-tree directory is prepended to relative include paths, but this does not work on Windows, where a path may have a drive letter like C: at the start of it.
This breaks SDL which includes an absolute path to the header directory to the C flags, e.g. -IC:/msys64/mingw64/include/SDL2
Add this as a special case to leave these absolute paths alone on Windows.
Signed-off-by: Simon Glass sjg@chromium.org
(no changes since v1)
scripts/Kbuild.include | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include index 62e0207f91b4..411a768a7767 100644 --- a/scripts/Kbuild.include +++ b/scripts/Kbuild.include @@ -205,9 +205,10 @@ clean := -f $(srctree)/scripts/Makefile.clean obj hdr-inst := -f $(srctree)/scripts/Makefile.headersinst obj
# Prefix -I with $(srctree) if it is not an absolute path. +# Detect C: (C drive) with MSYS2 # skip if -I has no parameter addtree = $(if $(patsubst -I%,%,$(1)), \ -$(if $(filter-out -I/% -I./% -I../%,$(1)),$(patsubst -I%,-I$(srctree)/%,$(1)),$(1)),$(1)) +$(if $(filter-out -I/% -I./% -I../% -IC:%,$(1)),$(patsubst -I%,-I$(srctree)/%,$(1)),$(1)),$(1))
# Find all -I options and call addtree flags = $(foreach o,$($(1)),$(if $(filter -I%,$(o)),$(call addtree,$(o)),$(o)))
This feels a bit fragile, stuff could be on some network drive instead and so another letter, can we make this a bit more generic?

Hi Simon,
On Sun, Apr 30, 2023 at 9:30 AM Simon Glass sjg@chromium.org wrote:
The source-tree directory is prepended to relative include paths, but this does not work on Windows, where a path may have a drive letter like C: at the start of it.
This breaks SDL which includes an absolute path to the header directory to the C flags, e.g. -IC:/msys64/mingw64/include/SDL2
Add this as a special case to leave these absolute paths alone on Windows.
Signed-off-by: Simon Glass sjg@chromium.org
(no changes since v1)
scripts/Kbuild.include | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include index 62e0207f91b4..411a768a7767 100644 --- a/scripts/Kbuild.include +++ b/scripts/Kbuild.include @@ -205,9 +205,10 @@ clean := -f $(srctree)/scripts/Makefile.clean obj hdr-inst := -f $(srctree)/scripts/Makefile.headersinst obj
# Prefix -I with $(srctree) if it is not an absolute path. +# Detect C: (C drive) with MSYS2 # skip if -I has no parameter addtree = $(if $(patsubst -I%,%,$(1)), \ -$(if $(filter-out -I/% -I./% -I../%,$(1)),$(patsubst -I%,-I$(srctree)/%,$(1)),$(1)),$(1)) +$(if $(filter-out -I/% -I./% -I../% -IC:%,$(1)),$(patsubst -I%,-I$(srctree)/%,$(1)),$(1)),$(1))
What happens if MSYS2 is not installed to the C drive?
# Find all -I options and call addtree flags = $(foreach o,$($(1)),$(if $(filter -I%,$(o)),$(call addtree,$(o)),$(o))) --
Regards, Bin

This relies on Linux features so cannot be built for Windows. Drop it.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
arch/sandbox/cpu/Makefile | 2 ++ drivers/net/Makefile | 2 ++ 2 files changed, 4 insertions(+)
diff --git a/arch/sandbox/cpu/Makefile b/arch/sandbox/cpu/Makefile index 7c5c52652f5c..be6b57692dd5 100644 --- a/arch/sandbox/cpu/Makefile +++ b/arch/sandbox/cpu/Makefile @@ -9,7 +9,9 @@ obj-y := cache.o cpu.o state.o extra-y := start.o os.o extra-$(CONFIG_SANDBOX_SDL) += sdl.o obj-$(CONFIG_SPL_BUILD) += spl.o +ifeq ($(MSYS_VERSION),0) obj-$(CONFIG_ETH_SANDBOX_RAW) += eth-raw-os.o +endif
# os.c is build in the system environment, so needs standard includes # CFLAGS_REMOVE_os.o cannot be used to drop header include path diff --git a/drivers/net/Makefile b/drivers/net/Makefile index 46a40e2ed9f8..6580f8d85510 100644 --- a/drivers/net/Makefile +++ b/drivers/net/Makefile @@ -30,8 +30,10 @@ obj-$(CONFIG_ETH_DESIGNWARE_MESON8B) += dwmac_meson8b.o obj-$(CONFIG_ETH_DESIGNWARE_S700) += dwmac_s700.o obj-$(CONFIG_ETH_DESIGNWARE_SOCFPGA) += dwmac_socfpga.o obj-$(CONFIG_ETH_SANDBOX) += sandbox.o +ifeq ($(MSYS_VERSION),0) obj-$(CONFIG_ETH_SANDBOX_RAW) += sandbox-raw-bus.o obj-$(CONFIG_ETH_SANDBOX_RAW) += sandbox-raw.o +endif obj-$(CONFIG_FEC_MXC) += fec_mxc.o obj-$(CONFIG_FMAN_ENET) += fm/ obj-$(CONFIG_FMAN_ENET) += fsl_mdio.o

Hi Simon,
On Sun, Apr 30, 2023 at 9:30 AM Simon Glass sjg@chromium.org wrote:
This relies on Linux features so cannot be built for Windows. Drop it.
Signed-off-by: Simon Glass sjg@chromium.org
(no changes since v1)
arch/sandbox/cpu/Makefile | 2 ++ drivers/net/Makefile | 2 ++ 2 files changed, 4 insertions(+)
diff --git a/arch/sandbox/cpu/Makefile b/arch/sandbox/cpu/Makefile index 7c5c52652f5c..be6b57692dd5 100644 --- a/arch/sandbox/cpu/Makefile +++ b/arch/sandbox/cpu/Makefile @@ -9,7 +9,9 @@ obj-y := cache.o cpu.o state.o extra-y := start.o os.o extra-$(CONFIG_SANDBOX_SDL) += sdl.o obj-$(CONFIG_SPL_BUILD) += spl.o +ifeq ($(MSYS_VERSION),0) obj-$(CONFIG_ETH_SANDBOX_RAW) += eth-raw-os.o +endif
# os.c is build in the system environment, so needs standard includes # CFLAGS_REMOVE_os.o cannot be used to drop header include path diff --git a/drivers/net/Makefile b/drivers/net/Makefile index 46a40e2ed9f8..6580f8d85510 100644 --- a/drivers/net/Makefile +++ b/drivers/net/Makefile @@ -30,8 +30,10 @@ obj-$(CONFIG_ETH_DESIGNWARE_MESON8B) += dwmac_meson8b.o obj-$(CONFIG_ETH_DESIGNWARE_S700) += dwmac_s700.o obj-$(CONFIG_ETH_DESIGNWARE_SOCFPGA) += dwmac_socfpga.o obj-$(CONFIG_ETH_SANDBOX) += sandbox.o +ifeq ($(MSYS_VERSION),0) obj-$(CONFIG_ETH_SANDBOX_RAW) += sandbox-raw-bus.o obj-$(CONFIG_ETH_SANDBOX_RAW) += sandbox-raw.o +endif obj-$(CONFIG_FEC_MXC) += fec_mxc.o obj-$(CONFIG_FMAN_ENET) += fm/ obj-$(CONFIG_FMAN_ENET) += fsl_mdio.o --
In patch #16, you introduced CC_IS_MSYS. I think you can just make CONFIG_ETH_SANDBNOX_RAW depend on !CC_IS_MSYS?
Regards, Bin

The Linux register format used on Linux (and perhaps other OSes) is not used on Windows, so disable this feature.
Signed-off-by: Simon Glass sjg@chromium.org ---
Changes in v2: - Update commit message to mention other OSes - Check for __MSYS__ instead of __linux
arch/sandbox/cpu/os.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/sandbox/cpu/os.c b/arch/sandbox/cpu/os.c index e76568ebdd32..522fe8a6f2b1 100644 --- a/arch/sandbox/cpu/os.c +++ b/arch/sandbox/cpu/os.c @@ -288,7 +288,7 @@ static void os_signal_handler(int sig, siginfo_t *info, void *con) ucontext_t __maybe_unused *context = con; unsigned long pc;
-#if defined(__x86_64__) +#if defined(__x86_64__) && !defined(__MSYS__) pc = context->uc_mcontext.gregs[REG_RIP]; #elif defined(__aarch64__) pc = context->uc_mcontext.pc;

Hi Simon,
On Sun, Apr 30, 2023 at 9:30 AM Simon Glass sjg@chromium.org wrote:
The Linux register format used on Linux (and perhaps other OSes) is not used on Windows, so disable this feature.
Signed-off-by: Simon Glass sjg@chromium.org
Changes in v2:
- Update commit message to mention other OSes
- Check for __MSYS__ instead of __linux
arch/sandbox/cpu/os.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/sandbox/cpu/os.c b/arch/sandbox/cpu/os.c index e76568ebdd32..522fe8a6f2b1 100644 --- a/arch/sandbox/cpu/os.c +++ b/arch/sandbox/cpu/os.c @@ -288,7 +288,7 @@ static void os_signal_handler(int sig, siginfo_t *info, void *con) ucontext_t __maybe_unused *context = con; unsigned long pc;
-#if defined(__x86_64__) +#if defined(__x86_64__) && !defined(__MSYS__)
I think we need to comment out all places in os.c that use signal handlers, e.g.: sigatcion() does not exist on Windows.
Or we consider rewriting signal related codes in a portable way, e.g.: Windows only has signal() API.
pc = context->uc_mcontext.gregs[REG_RIP];
#elif defined(__aarch64__) pc = context->uc_mcontext.pc; --
Regards, Bin

The required linker symbols are not present. For now just return 0 in this case.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
common/board_f.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/common/board_f.c b/common/board_f.c index 1688e27071fc..4f380d7c82c8 100644 --- a/common/board_f.c +++ b/common/board_f.c @@ -290,7 +290,7 @@ static int setup_mon_len(void) { #if defined(__ARM__) || defined(__MICROBLAZE__) gd->mon_len = (ulong)&__bss_end - (ulong)_start; -#elif defined(CONFIG_SANDBOX) && !defined(__riscv) +#elif defined(CONFIG_SANDBOX) && !defined(__riscv) && !defined(__MSYS__) gd->mon_len = (ulong)&_end - (ulong)_init; #elif defined(CONFIG_SANDBOX) /* gcc does not provide _init in crti.o on RISC-V */

Hi Simon,
On Sun, Apr 30, 2023 at 9:30 AM Simon Glass sjg@chromium.org wrote:
The required linker symbols are not present. For now just return 0 in this case.
Is it possible to add _end in the Sandbox linker script?
Signed-off-by: Simon Glass sjg@chromium.org
(no changes since v1)
common/board_f.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Regards, Bin

The MSYS2 compiler does not support some of these options. Drop them to avoid build errors.
Signed-off-by: Simon Glass sjg@chromium.org ---
Changes in v2: - Use cc-option and ld-option instead
Makefile | 2 +- scripts/Makefile.lib | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/Makefile b/Makefile index 2453a80eca62..240562dff00b 100644 --- a/Makefile +++ b/Makefile @@ -819,7 +819,7 @@ KBUILD_CPPFLAGS += $(KCPPFLAGS) KBUILD_AFLAGS += $(KAFLAGS) KBUILD_CFLAGS += $(KCFLAGS)
-KBUILD_LDFLAGS += -z noexecstack +KBUILD_LDFLAGS += $(call ld-option,-znoexecstack) KBUILD_LDFLAGS += $(call ld-option,--no-warn-rwx-segments)
KBUILD_HOSTCFLAGS += $(if $(CONFIG_TOOLS_DEBUG),-g) diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib index 7b27224b5d44..aaae37d50a43 100644 --- a/scripts/Makefile.lib +++ b/scripts/Makefile.lib @@ -425,7 +425,10 @@ cmd_efi_objcopy = $(OBJCOPY) -j .header -j .text -j .sdata -j .data -j \ $(obj)/%.efi: $(obj)/%_efi.so $(call cmd,efi_objcopy)
-KBUILD_EFILDFLAGS = -nostdlib -zexecstack -znocombreloc -znorelro +KBUILD_EFILDFLAGS := -nostdlib +KBUILD_EFILDFLAGS += $(call ld-option,-zexecstack) +KBUILD_EFILDFLAGS += $(call ld-option,-znocombreloc) +KBUILD_EFILDFLAGS += $(call ld-option,-znorelro) KBUILD_EFILDFLAGS += $(call ld-option,--no-warn-rwx-segments) quiet_cmd_efi_ld = LD $@ cmd_efi_ld = $(LD) $(KBUILD_EFILDFLAGS) -T $(EFI_LDS_PATH) \

On Sat, Apr 29, 2023 at 07:29:54PM -0600, Simon Glass wrote:
The MSYS2 compiler does not support some of these options. Drop them to avoid build errors.
Signed-off-by: Simon Glass sjg@chromium.org
Reviewed-by: Tom Rini trini@konsulko.com

Hi Simon,
On Sun, Apr 30, 2023 at 9:30 AM Simon Glass sjg@chromium.org wrote:
The MSYS2 compiler does not support some of these options. Drop them to avoid build errors.
I think the commit message is better reworded to something like:
Test some linker options as some toolchains like MSYS2 do not support them.
Signed-off-by: Simon Glass sjg@chromium.org
Changes in v2:
- Use cc-option and ld-option instead
Makefile | 2 +- scripts/Makefile.lib | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/Makefile b/Makefile index 2453a80eca62..240562dff00b 100644 --- a/Makefile +++ b/Makefile @@ -819,7 +819,7 @@ KBUILD_CPPFLAGS += $(KCPPFLAGS) KBUILD_AFLAGS += $(KAFLAGS) KBUILD_CFLAGS += $(KCFLAGS)
-KBUILD_LDFLAGS += -z noexecstack +KBUILD_LDFLAGS += $(call ld-option,-znoexecstack) KBUILD_LDFLAGS += $(call ld-option,--no-warn-rwx-segments)
KBUILD_HOSTCFLAGS += $(if $(CONFIG_TOOLS_DEBUG),-g) diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib index 7b27224b5d44..aaae37d50a43 100644 --- a/scripts/Makefile.lib +++ b/scripts/Makefile.lib @@ -425,7 +425,10 @@ cmd_efi_objcopy = $(OBJCOPY) -j .header -j .text -j .sdata -j .data -j \ $(obj)/%.efi: $(obj)/%_efi.so $(call cmd,efi_objcopy)
-KBUILD_EFILDFLAGS = -nostdlib -zexecstack -znocombreloc -znorelro +KBUILD_EFILDFLAGS := -nostdlib +KBUILD_EFILDFLAGS += $(call ld-option,-zexecstack) +KBUILD_EFILDFLAGS += $(call ld-option,-znocombreloc) +KBUILD_EFILDFLAGS += $(call ld-option,-znorelro) KBUILD_EFILDFLAGS += $(call ld-option,--no-warn-rwx-segments) quiet_cmd_efi_ld = LD $@ cmd_efi_ld = $(LD) $(KBUILD_EFILDFLAGS) -T $(EFI_LDS_PATH) \ --
Otherwise, Reviewed-by: Bin Meng bmeng@tinylab.org

Add the required extension to the Makefile rule.
Signed-off-by: Simon Glass sjg@chromium.org ---
Changes in v2: - Use EXEEXT instead of ELFEXT
Makefile | 1 + scripts/Makefile.build | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/Makefile b/Makefile index 240562dff00b..a328652f0f23 100644 --- a/Makefile +++ b/Makefile @@ -48,6 +48,7 @@ ifeq ($(MSYS_VERSION),0) export LIBEXT := so else export LIBEXT := dll +export EXEEXT := .exe endif
# Avoid funny character set dependencies diff --git a/scripts/Makefile.build b/scripts/Makefile.build index 97dd4a64f6ef..a494e2f105b8 100644 --- a/scripts/Makefile.build +++ b/scripts/Makefile.build @@ -309,7 +309,7 @@ quiet_cmd_asn1_compiler = ASN.1 $@ cmd_asn1_compiler = $(objtree)/tools/asn1_compiler $< \ $(subst .h,.c,$@) $(subst .c,.h,$@)
-$(obj)/%.asn1.c $(obj)/%.asn1.h: $(src)/%.asn1 $(objtree)/tools/asn1_compiler +$(obj)/%.asn1.c $(obj)/%.asn1.h: $(src)/%.asn1 $(objtree)/tools/asn1_compiler$(EXEEXT) $(call cmd,asn1_compiler)
# Build the compiled-in targets

On Sat, Apr 29, 2023 at 07:29:55PM -0600, Simon Glass wrote:
Add the required extension to the Makefile rule.
Signed-off-by: Simon Glass sjg@chromium.org
Reviewed-by: Tom Rini trini@konsulko.com

Hi Simon,
On Sun, Apr 30, 2023 at 9:30 AM Simon Glass sjg@chromium.org wrote:
Add the required extension to the Makefile rule.
Signed-off-by: Simon Glass sjg@chromium.org
Changes in v2:
- Use EXEEXT instead of ELFEXT
Makefile | 1 + scripts/Makefile.build | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/Makefile b/Makefile index 240562dff00b..a328652f0f23 100644 --- a/Makefile +++ b/Makefile @@ -48,6 +48,7 @@ ifeq ($(MSYS_VERSION),0) export LIBEXT := so else export LIBEXT := dll
To make such handling consistent, please make LIBEXT be .so and .dll (including the dot like exe)
+export EXEEXT := .exe endif
# Avoid funny character set dependencies diff --git a/scripts/Makefile.build b/scripts/Makefile.build index 97dd4a64f6ef..a494e2f105b8 100644 --- a/scripts/Makefile.build +++ b/scripts/Makefile.build @@ -309,7 +309,7 @@ quiet_cmd_asn1_compiler = ASN.1 $@ cmd_asn1_compiler = $(objtree)/tools/asn1_compiler $< \ $(subst .h,.c,$@) $(subst .c,.h,$@)
-$(obj)/%.asn1.c $(obj)/%.asn1.h: $(src)/%.asn1 $(objtree)/tools/asn1_compiler +$(obj)/%.asn1.c $(obj)/%.asn1.h: $(src)/%.asn1 $(objtree)/tools/asn1_compiler$(EXEEXT) $(call cmd,asn1_compiler)
# Build the compiled-in targets
Otherwise, Reviewed-by: Bin Meng bmeng@tinylab.org

On Wednesday 03 May 2023 10:31:06 Bin Meng wrote:
Hi Simon,
On Sun, Apr 30, 2023 at 9:30 AM Simon Glass sjg@chromium.org wrote:
Add the required extension to the Makefile rule.
Signed-off-by: Simon Glass sjg@chromium.org
Changes in v2:
- Use EXEEXT instead of ELFEXT
Makefile | 1 + scripts/Makefile.build | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/Makefile b/Makefile index 240562dff00b..a328652f0f23 100644 --- a/Makefile +++ b/Makefile @@ -48,6 +48,7 @@ ifeq ($(MSYS_VERSION),0) export LIBEXT := so else export LIBEXT := dll
To make such handling consistent, please make LIBEXT be .so and .dll (including the dot like exe)
IIRC de-facto standard Makefile variable LIBEXT does not contain dot. So this is how it is used...
+export EXEEXT := .exe endif
# Avoid funny character set dependencies diff --git a/scripts/Makefile.build b/scripts/Makefile.build index 97dd4a64f6ef..a494e2f105b8 100644 --- a/scripts/Makefile.build +++ b/scripts/Makefile.build @@ -309,7 +309,7 @@ quiet_cmd_asn1_compiler = ASN.1 $@ cmd_asn1_compiler = $(objtree)/tools/asn1_compiler $< \ $(subst .h,.c,$@) $(subst .c,.h,$@)
-$(obj)/%.asn1.c $(obj)/%.asn1.h: $(src)/%.asn1 $(objtree)/tools/asn1_compiler +$(obj)/%.asn1.c $(obj)/%.asn1.h: $(src)/%.asn1 $(objtree)/tools/asn1_compiler$(EXEEXT) $(call cmd,asn1_compiler)
# Build the compiled-in targets
Otherwise, Reviewed-by: Bin Meng bmeng@tinylab.org

We need to place the linker lists, etc. in the .rdata section but this is not possible with the default linker script. We can only add new sections, which causes Windows to give an "Exec format error" error.
Add a rule to create a new linker script, by obtaining the one from the linker and adding some things to the end of the .rdata section.
Signed-off-by: Simon Glass sjg@chromium.org ---
Changes in v2: - Add an awk script to augment the built-in link script
Makefile | 17 ++++++++++++----- arch/sandbox/config.mk | 19 ++++++++++++++++++- arch/sandbox/cpu/u-boot-pe.lds.in | 25 +++++++++++++++++++++++++ scripts/add_to_rdata.awk | 25 +++++++++++++++++++++++++ 4 files changed, 80 insertions(+), 6 deletions(-) create mode 100644 arch/sandbox/cpu/u-boot-pe.lds.in create mode 100644 scripts/add_to_rdata.awk
diff --git a/Makefile b/Makefile index a328652f0f23..760c143049aa 100644 --- a/Makefile +++ b/Makefile @@ -1734,6 +1734,12 @@ else u-boot-keep-syms-lto := endif
+ifeq ($(MSYS_VERSION),0) +add_ld_script := -T u-boot.lds +else +add_ld_script := u-boot.lds +endif + # Rule to link u-boot # May be overridden by arch/$(ARCH)/config.mk ifeq ($(LTO_ENABLE),y) @@ -1742,7 +1748,7 @@ quiet_cmd_u-boot__ ?= LTO $@ $(CC) -nostdlib -nostartfiles \ $(LTO_FINAL_LDFLAGS) $(c_flags) \ $(KBUILD_LDFLAGS:%=-Wl,%) $(LDFLAGS_u-boot:%=-Wl,%) -o $@ \ - -T u-boot.lds $(u-boot-init) \ + $(add_ld_script) $(u-boot-init) \ -Wl,--whole-archive \ $(u-boot-main) \ $(u-boot-keep-syms-lto) \ @@ -1753,7 +1759,7 @@ quiet_cmd_u-boot__ ?= LTO $@ else quiet_cmd_u-boot__ ?= LD $@ cmd_u-boot__ ?= $(LD) $(KBUILD_LDFLAGS) $(LDFLAGS_u-boot) -o $@ \ - -T u-boot.lds $(u-boot-init) \ + $(add_ld_script) $(u-boot-init) \ --whole-archive \ $(u-boot-main) \ --no-whole-archive \ @@ -1905,10 +1911,11 @@ endif # prepare2 creates a makefile if using a separate output directory prepare2: prepare3 outputmakefile cfg
+# Allow the linker script to be generated from LDSCRIPT_IN prepare1: prepare2 $(version_h) $(timestamp_h) $(dt_h) $(env_h) \ - include/config/auto.conf -ifeq ($(wildcard $(LDSCRIPT)),) - @echo >&2 " Could not find linker script." + include/config/auto.conf $(if $(LDSCRIPT_IN),$(LDSCRIPT)) +ifeq ($(wildcard $(LDSCRIPT))$(LDSCRIPT_IN),) + @echo >&2 " Could not find linker script $(LDSCRIPT)" @/bin/false endif
diff --git a/arch/sandbox/config.mk b/arch/sandbox/config.mk index 2d184c5f652a..c97c39d4301b 100644 --- a/arch/sandbox/config.mk +++ b/arch/sandbox/config.mk @@ -1,4 +1,4 @@ -# SPDX-License-Identifier: GPL-2.0+ + # SPDX-License-Identifier: GPL-2.0+ # Copyright (c) 2011 The Chromium OS Authors.
PLATFORM_CPPFLAGS += -D__SANDBOX__ -U_FORTIFY_SOURCE @@ -71,3 +71,20 @@ EFI_CRT0 := crt0_sandbox_efi.o EFI_RELOC := reloc_sandbox_efi.o AFLAGS_crt0_sandbox_efi.o += -DHOST_ARCH="$(HOST_ARCH)" CFLAGS_reloc_sandbox_efi.o += -DHOST_ARCH="$(HOST_ARCH)" + +ifneq ($(MSYS_VERSION),0) +LDSCRIPT := $(objtree)/u-boot-pe.lds + +AWK_RDATA := ${srctree}/scripts/add_to_rdata.awk +LDSCRIPT_IN := ${srctree}/arch/sandbox/cpu/u-boot-pe.lds.in + +quiet_cmd_gen_lds = GEN LDS $@ +cmd_gen_lds = echo "int main() { return 0; }" | $(CC) -x c - -Wl,-verbose | \ + awk -f $(AWK_RDATA) -v INFILE=$< >$@ + +# Write out the contents of INFILE immediately before the close of the .rdata +# block +$(LDSCRIPT): $(LDSCRIPT_IN) $(AWK_RDATA) FORCE + $(call if_changed,gen_lds) + +endif diff --git a/arch/sandbox/cpu/u-boot-pe.lds.in b/arch/sandbox/cpu/u-boot-pe.lds.in new file mode 100644 index 000000000000..0ec7ef3bb350 --- /dev/null +++ b/arch/sandbox/cpu/u-boot-pe.lds.in @@ -0,0 +1,25 @@ + /* U-Boot additions from here on */ + . = ALIGN(4); + KEEP(*(SORT(__u_boot_list*))); + + *(_u_boot_sandbox_getopt_start) + *(_u_boot_sandbox_getopt) + *(_u_boot_sandbox_getopt_end) + + *(___efi_runtime_start) + *(efi_runtime_text) + *(efi_runtime_data) + *(___efi_runtime_stop) + + *(___efi_runtime_rel_start) + *(.relefi_runtime_text) + *(.relefi_runtime_data) + *(___efi_runtime_rel_stop) + + . = ALIGN(4); + *(.rodata.ttf.init) + *(.rodata.splash.init) + *(.rodata.helloworld.init) + *(.dtb.init.rodata) + + /* U-Boot additions end */ diff --git a/scripts/add_to_rdata.awk b/scripts/add_to_rdata.awk new file mode 100644 index 000000000000..43fdfe8bb789 --- /dev/null +++ b/scripts/add_to_rdata.awk @@ -0,0 +1,25 @@ +# SPDX-License-Identifier: GPL-2.0+ +# +# Copyright 2023 Google, Inc +# +# Awk script to extract the default link script from the linker and write out +# the contents of INFILE immediately before the close of the .rdata section. + +# to a C string which can be compiled into U-Boot. + +# INS = 1 if we are inside the link script (delimited by ======== lines) +# INR = 1 if we are inside the .rdata section + +# When we see } while in the .rdata part of the link script, insert INFILE +/}/ { if (INS && INR) { while ((getline < INFILE) > 0) {print}; DONE=1; INR=0; $0="}"; }} + +# Find start and end of link script +/===================/ { if (INS) exit; INS=1; next; } + +# If inside the link script, print each line +{ if (INS) print; } + +# Detect the .rdata section and get ready to insert INFILE when we see the end } +/.rdata.*:/ {INR=1; } + +END { if (!DONE) { print "add_to_rdata.awk: Could not find link script in ld output" > "/dev/stderr"; exit 1;} }

Hi Simon,
On Sun, Apr 30, 2023 at 9:30 AM Simon Glass sjg@chromium.org wrote:
We need to place the linker lists, etc. in the .rdata section but this is not possible with the default linker script. We can only add new sections, which causes Windows to give an "Exec format error" error.
Add a rule to create a new linker script, by obtaining the one from the linker and adding some things to the end of the .rdata section.
I am not sure I understand this. So the arch/sandbox/cpu/u-boot.lds does not work for MSYS2?
Signed-off-by: Simon Glass sjg@chromium.org
Changes in v2:
- Add an awk script to augment the built-in link script
Makefile | 17 ++++++++++++----- arch/sandbox/config.mk | 19 ++++++++++++++++++- arch/sandbox/cpu/u-boot-pe.lds.in | 25 +++++++++++++++++++++++++ scripts/add_to_rdata.awk | 25 +++++++++++++++++++++++++ 4 files changed, 80 insertions(+), 6 deletions(-) create mode 100644 arch/sandbox/cpu/u-boot-pe.lds.in create mode 100644 scripts/add_to_rdata.awk
diff --git a/Makefile b/Makefile index a328652f0f23..760c143049aa 100644 --- a/Makefile +++ b/Makefile @@ -1734,6 +1734,12 @@ else u-boot-keep-syms-lto := endif
+ifeq ($(MSYS_VERSION),0) +add_ld_script := -T u-boot.lds +else +add_ld_script := u-boot.lds
MSYS2 does not need "-T"?
+endif
# Rule to link u-boot # May be overridden by arch/$(ARCH)/config.mk ifeq ($(LTO_ENABLE),y) @@ -1742,7 +1748,7 @@ quiet_cmd_u-boot__ ?= LTO $@ $(CC) -nostdlib -nostartfiles \ $(LTO_FINAL_LDFLAGS) $(c_flags) \ $(KBUILD_LDFLAGS:%=-Wl,%) $(LDFLAGS_u-boot:%=-Wl,%) -o $@ \
-T u-boot.lds $(u-boot-init) \
$(add_ld_script) $(u-boot-init) \ -Wl,--whole-archive \ $(u-boot-main) \ $(u-boot-keep-syms-lto) \
@@ -1753,7 +1759,7 @@ quiet_cmd_u-boot__ ?= LTO $@ else quiet_cmd_u-boot__ ?= LD $@ cmd_u-boot__ ?= $(LD) $(KBUILD_LDFLAGS) $(LDFLAGS_u-boot) -o $@ \
-T u-boot.lds $(u-boot-init) \
$(add_ld_script) $(u-boot-init) \ --whole-archive \ $(u-boot-main) \ --no-whole-archive \
@@ -1905,10 +1911,11 @@ endif # prepare2 creates a makefile if using a separate output directory prepare2: prepare3 outputmakefile cfg
+# Allow the linker script to be generated from LDSCRIPT_IN prepare1: prepare2 $(version_h) $(timestamp_h) $(dt_h) $(env_h) \
include/config/auto.conf
-ifeq ($(wildcard $(LDSCRIPT)),)
@echo >&2 " Could not find linker script."
include/config/auto.conf $(if $(LDSCRIPT_IN),$(LDSCRIPT))
+ifeq ($(wildcard $(LDSCRIPT))$(LDSCRIPT_IN),)
@echo >&2 " Could not find linker script $(LDSCRIPT)" @/bin/false
endif
diff --git a/arch/sandbox/config.mk b/arch/sandbox/config.mk index 2d184c5f652a..c97c39d4301b 100644 --- a/arch/sandbox/config.mk +++ b/arch/sandbox/config.mk @@ -1,4 +1,4 @@ -# SPDX-License-Identifier: GPL-2.0+
# SPDX-License-Identifier: GPL-2.0+
# Copyright (c) 2011 The Chromium OS Authors.
PLATFORM_CPPFLAGS += -D__SANDBOX__ -U_FORTIFY_SOURCE @@ -71,3 +71,20 @@ EFI_CRT0 := crt0_sandbox_efi.o EFI_RELOC := reloc_sandbox_efi.o AFLAGS_crt0_sandbox_efi.o += -DHOST_ARCH="$(HOST_ARCH)" CFLAGS_reloc_sandbox_efi.o += -DHOST_ARCH="$(HOST_ARCH)"
+ifneq ($(MSYS_VERSION),0) +LDSCRIPT := $(objtree)/u-boot-pe.lds
+AWK_RDATA := ${srctree}/scripts/add_to_rdata.awk +LDSCRIPT_IN := ${srctree}/arch/sandbox/cpu/u-boot-pe.lds.in
+quiet_cmd_gen_lds = GEN LDS $@ +cmd_gen_lds = echo "int main() { return 0; }" | $(CC) -x c - -Wl,-verbose | \
awk -f $(AWK_RDATA) -v INFILE=$< >$@
+# Write out the contents of INFILE immediately before the close of the .rdata +# block +$(LDSCRIPT): $(LDSCRIPT_IN) $(AWK_RDATA) FORCE
$(call if_changed,gen_lds)
+endif diff --git a/arch/sandbox/cpu/u-boot-pe.lds.in b/arch/sandbox/cpu/u-boot-pe.lds.in new file mode 100644 index 000000000000..0ec7ef3bb350 --- /dev/null +++ b/arch/sandbox/cpu/u-boot-pe.lds.in @@ -0,0 +1,25 @@
/* U-Boot additions from here on */
. = ALIGN(4);
KEEP(*(SORT(__u_boot_list*)));
*(_u_boot_sandbox_getopt_start)
*(_u_boot_sandbox_getopt)
*(_u_boot_sandbox_getopt_end)
*(___efi_runtime_start)
*(efi_runtime_text)
*(efi_runtime_data)
*(___efi_runtime_stop)
*(___efi_runtime_rel_start)
*(.relefi_runtime_text)
*(.relefi_runtime_data)
*(___efi_runtime_rel_stop)
. = ALIGN(4);
*(.rodata.ttf.init)
*(.rodata.splash.init)
*(.rodata.helloworld.init)
*(.dtb.init.rodata)
/* U-Boot additions end */
diff --git a/scripts/add_to_rdata.awk b/scripts/add_to_rdata.awk new file mode 100644 index 000000000000..43fdfe8bb789 --- /dev/null +++ b/scripts/add_to_rdata.awk @@ -0,0 +1,25 @@ +# SPDX-License-Identifier: GPL-2.0+ +# +# Copyright 2023 Google, Inc +# +# Awk script to extract the default link script from the linker and write out +# the contents of INFILE immediately before the close of the .rdata section.
+# to a C string which can be compiled into U-Boot.
+# INS = 1 if we are inside the link script (delimited by ======== lines) +# INR = 1 if we are inside the .rdata section
+# When we see } while in the .rdata part of the link script, insert INFILE +/}/ { if (INS && INR) { while ((getline < INFILE) > 0) {print}; DONE=1; INR=0; $0="}"; }}
+# Find start and end of link script +/===================/ { if (INS) exit; INS=1; next; }
+# If inside the link script, print each line +{ if (INS) print; }
+# Detect the .rdata section and get ready to insert INFILE when we see the end } +/.rdata.*:/ {INR=1; }
+END { if (!DONE) { print "add_to_rdata.awk: Could not find link script in ld output" > "/dev/stderr"; exit 1;} }
Regards, Bin

Add another case for sandbox, when it is built on Windows.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
arch/sandbox/config.mk | 4 +- arch/x86/lib/crt0_x86_64_efi.S | 2 + arch/x86/lib/pe_x86_64_efi.lds | 83 ++++++++++++++++++++++++++++++++++ 3 files changed, 88 insertions(+), 1 deletion(-) create mode 100644 arch/x86/lib/pe_x86_64_efi.lds
diff --git a/arch/sandbox/config.mk b/arch/sandbox/config.mk index c97c39d4301b..d397ae3fe29b 100644 --- a/arch/sandbox/config.mk +++ b/arch/sandbox/config.mk @@ -44,7 +44,9 @@ cmd_u-boot-spl = (cd $(obj) && $(CC) -o $(SPL_BIN) -Wl,-T u-boot-spl.lds \ -Wl,--no-whole-archive \ $(PLATFORM_LIBS) -Wl,-Map -Wl,u-boot-spl.map -Wl,--gc-sections)
-ifeq ($(HOST_ARCH),$(HOST_ARCH_X86_64)) +ifneq ($(MSYS_VERSION),0) +EFI_LDS := ${SRCDIR}/../../../arch/x86/lib/pe_x86_64_efi.lds +else ifeq ($(HOST_ARCH),$(HOST_ARCH_X86_64)) EFI_LDS := ${SRCDIR}/../../../arch/x86/lib/elf_x86_64_efi.lds EFI_TARGET := --target=efi-app-x86_64 else ifeq ($(HOST_ARCH),$(HOST_ARCH_X86)) diff --git a/arch/x86/lib/crt0_x86_64_efi.S b/arch/x86/lib/crt0_x86_64_efi.S index 47ed5af97228..cd61b4bdd82f 100644 --- a/arch/x86/lib/crt0_x86_64_efi.S +++ b/arch/x86/lib/crt0_x86_64_efi.S @@ -15,6 +15,7 @@ _start: subq $8, %rsp
+#ifndef __CYGWIN__ pushq %rcx pushq %rdx
@@ -28,6 +29,7 @@ _start:
testq %rax, %rax jnz .exit +#endif
call efi_main .exit: diff --git a/arch/x86/lib/pe_x86_64_efi.lds b/arch/x86/lib/pe_x86_64_efi.lds new file mode 100644 index 000000000000..1ee08f6e662e --- /dev/null +++ b/arch/x86/lib/pe_x86_64_efi.lds @@ -0,0 +1,83 @@ +/* SPDX-License-Identifier: BSD-2-Clause */ +/* + * U-Boot EFI linker script + * + * Modified from usr/lib32/elf_x86_64_efi.lds in gnu-efi + */ + +OUTPUT_FORMAT("pe-x86-64", "pe-x86-64", "pe-x86-64") +OUTPUT_ARCH(i386:x86-64) +ENTRY(_start) +SECTIONS +{ + image_base = .; + .hash : { *(.hash) } /* this MUST come first, EFI expects it */ + . = ALIGN(4096); + .eh_frame : { + *(.eh_frame) + } + + . = ALIGN(4096); + + .text : { + *(.text) + *(.text.*) + *(.gnu.linkonce.t.*) + } + + . = ALIGN(4096); + + .reloc : { + *(.reloc) + } + + . = ALIGN(4096); + + .data : { + *(.rodata*) + *(.got.plt) + *(.got) + *(.data*) + *(.sdata) + /* the EFI loader doesn't seem to like a .bss section, so we stick + * it all into .data: */ + *(.sbss) + *(.scommon) + *(.dynbss) + *(.bss*) + *(COMMON) + *(.rel.local) + + /* U-Boot lists and device tree */ + . = ALIGN(8); + *(SORT(__u_boot_list*)); + . = ALIGN(8); + *(.dtb*); + } + + . = ALIGN(4096); + .dynamic : { *(.dynamic) } + . = ALIGN(4096); + + .rela : { + *(.rela.data*) + *(.rela.got) + *(.rela.stab) + *(.rela__u_boot_list*) + } + + . = ALIGN(4096); + .dynsym : { *(.dynsym) } + . = ALIGN(4096); + .dynstr : { *(.dynstr) } + . = ALIGN(4096); + + /DISCARD/ : { *(.eh_frame) } + + .ignored.reloc : { + *(.rela.reloc) + *(.note.GNU-stack) + } + + .comment 0 : { *(.comment) } +}

Hi Simon,
On Sun, Apr 30, 2023 at 9:30 AM Simon Glass sjg@chromium.org wrote:
Add another case for sandbox, when it is built on Windows.
Signed-off-by: Simon Glass sjg@chromium.org
(no changes since v1)
arch/sandbox/config.mk | 4 +- arch/x86/lib/crt0_x86_64_efi.S | 2 + arch/x86/lib/pe_x86_64_efi.lds | 83 ++++++++++++++++++++++++++++++++++ 3 files changed, 88 insertions(+), 1 deletion(-) create mode 100644 arch/x86/lib/pe_x86_64_efi.lds
diff --git a/arch/sandbox/config.mk b/arch/sandbox/config.mk index c97c39d4301b..d397ae3fe29b 100644 --- a/arch/sandbox/config.mk +++ b/arch/sandbox/config.mk @@ -44,7 +44,9 @@ cmd_u-boot-spl = (cd $(obj) && $(CC) -o $(SPL_BIN) -Wl,-T u-boot-spl.lds \ -Wl,--no-whole-archive \ $(PLATFORM_LIBS) -Wl,-Map -Wl,u-boot-spl.map -Wl,--gc-sections)
-ifeq ($(HOST_ARCH),$(HOST_ARCH_X86_64)) +ifneq ($(MSYS_VERSION),0) +EFI_LDS := ${SRCDIR}/../../../arch/x86/lib/pe_x86_64_efi.lds +else ifeq ($(HOST_ARCH),$(HOST_ARCH_X86_64)) EFI_LDS := ${SRCDIR}/../../../arch/x86/lib/elf_x86_64_efi.lds EFI_TARGET := --target=efi-app-x86_64 else ifeq ($(HOST_ARCH),$(HOST_ARCH_X86)) diff --git a/arch/x86/lib/crt0_x86_64_efi.S b/arch/x86/lib/crt0_x86_64_efi.S index 47ed5af97228..cd61b4bdd82f 100644 --- a/arch/x86/lib/crt0_x86_64_efi.S +++ b/arch/x86/lib/crt0_x86_64_efi.S @@ -15,6 +15,7 @@ _start: subq $8, %rsp
+#ifndef __CYGWIN__
I think this should be __MSYS2__?
pushq %rcx pushq %rdx
@@ -28,6 +29,7 @@ _start:
testq %rax, %rax jnz .exit
+#endif
call efi_main
.exit: diff --git a/arch/x86/lib/pe_x86_64_efi.lds b/arch/x86/lib/pe_x86_64_efi.lds new file mode 100644 index 000000000000..1ee08f6e662e --- /dev/null +++ b/arch/x86/lib/pe_x86_64_efi.lds @@ -0,0 +1,83 @@ +/* SPDX-License-Identifier: BSD-2-Clause */ +/*
- U-Boot EFI linker script
- Modified from usr/lib32/elf_x86_64_efi.lds in gnu-efi
- */
+OUTPUT_FORMAT("pe-x86-64", "pe-x86-64", "pe-x86-64") +OUTPUT_ARCH(i386:x86-64) +ENTRY(_start) +SECTIONS +{
image_base = .;
.hash : { *(.hash) } /* this MUST come first, EFI expects it */
. = ALIGN(4096);
.eh_frame : {
*(.eh_frame)
}
. = ALIGN(4096);
.text : {
*(.text)
*(.text.*)
*(.gnu.linkonce.t.*)
}
. = ALIGN(4096);
.reloc : {
*(.reloc)
}
. = ALIGN(4096);
.data : {
*(.rodata*)
*(.got.plt)
*(.got)
*(.data*)
*(.sdata)
/* the EFI loader doesn't seem to like a .bss section, so we stick
* it all into .data: */
*(.sbss)
*(.scommon)
*(.dynbss)
*(.bss*)
*(COMMON)
*(.rel.local)
/* U-Boot lists and device tree */
. = ALIGN(8);
*(SORT(__u_boot_list*));
. = ALIGN(8);
*(.dtb*);
}
. = ALIGN(4096);
.dynamic : { *(.dynamic) }
. = ALIGN(4096);
.rela : {
*(.rela.data*)
*(.rela.got)
*(.rela.stab)
*(.rela__u_boot_list*)
}
. = ALIGN(4096);
.dynsym : { *(.dynsym) }
. = ALIGN(4096);
.dynstr : { *(.dynstr) }
. = ALIGN(4096);
/DISCARD/ : { *(.eh_frame) }
.ignored.reloc : {
*(.rela.reloc)
*(.note.GNU-stack)
}
.comment 0 : { *(.comment) }
+}
Regards, Bin

The sandbox build makes use of a small number of weak symbols. Allow these to be dropped when building for the PE format, since its support for weak symbols is poor.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
cmd/bootefi.c | 3 ++- cmd/bootz.c | 3 +++ common/usb.c | 3 +++ drivers/core/root.c | 3 +++ drivers/spi/sandbox_spi.c | 3 +++ env/env.c | 6 ++++++ lib/efi_loader/efi_image_loader.c | 3 +++ lib/efi_loader/efi_runtime.c | 4 ++++ lib/lmb.c | 4 +++- lib/time.c | 3 +++ 10 files changed, 33 insertions(+), 2 deletions(-)
diff --git a/cmd/bootefi.c b/cmd/bootefi.c index 8aa15a64c836..21e4a87da53a 100644 --- a/cmd/bootefi.c +++ b/cmd/bootefi.c @@ -358,7 +358,8 @@ static efi_status_t do_bootefi_exec(efi_handle_t handle, void *load_options) u16 *exit_data = NULL;
/* On ARM switch from EL3 or secure mode to EL2 or non-secure mode */ - switch_to_non_secure_mode(); + if (IS_ENABLED(CONFIG_WEAK_SYMBOLS)) + switch_to_non_secure_mode();
/* * The UEFI standard requires that the watchdog timer is set to five diff --git a/cmd/bootz.c b/cmd/bootz.c index f1423573d23d..1ffafbd6f2b9 100644 --- a/cmd/bootz.c +++ b/cmd/bootz.c @@ -13,6 +13,8 @@ #include <log.h> #include <linux/compiler.h>
+#ifdef CONFIG_WEAK_SYMBOLS + int __weak bootz_setup(ulong image, ulong *start, ulong *end) { /* Please define bootz_setup() for your platform */ @@ -20,6 +22,7 @@ int __weak bootz_setup(ulong image, ulong *start, ulong *end) puts("Your platform's zImage format isn't supported yet!\n"); return -1; } +#endif
/* * zImage booting support diff --git a/common/usb.c b/common/usb.c index ae9253dfc0ed..cff53254a379 100644 --- a/common/usb.c +++ b/common/usb.c @@ -1220,6 +1220,8 @@ int usb_new_device(struct usb_device *dev) } #endif
+#ifdef CONFIG_WEAK_SYMBOLS + __weak int board_usb_init(int index, enum usb_init_type init) { @@ -1231,6 +1233,7 @@ int board_usb_cleanup(int index, enum usb_init_type init) { return 0; } +#endif /* CONFIG_WEAK_SYMBOLS */
bool usb_device_has_child_on_port(struct usb_device *parent, int port) { diff --git a/drivers/core/root.c b/drivers/core/root.c index c4fb48548bb3..e311f93a08c9 100644 --- a/drivers/core/root.c +++ b/drivers/core/root.c @@ -347,10 +347,13 @@ int dm_extended_scan(bool pre_reloc_only) } #endif
+#ifdef CONFIG_WEAK_SYMBOLS + __weak int dm_scan_other(bool pre_reloc_only) { return 0; } +#endif
#if CONFIG_IS_ENABLED(OF_PLATDATA_INST) && CONFIG_IS_ENABLED(READ_ONLY) void *dm_priv_to_rw(void *priv) diff --git a/drivers/spi/sandbox_spi.c b/drivers/spi/sandbox_spi.c index f844597d04cf..f68c9f33bdb4 100644 --- a/drivers/spi/sandbox_spi.c +++ b/drivers/spi/sandbox_spi.c @@ -41,12 +41,15 @@ struct sandbox_spi_priv { uint mode; };
+#ifdef CONFIG_WEAK_SYMBOLS + __weak int sandbox_spi_get_emul(struct sandbox_state *state, struct udevice *bus, struct udevice *slave, struct udevice **emulp) { return -ENOENT; } +#endif
uint sandbox_spi_get_speed(struct udevice *dev) { diff --git a/env/env.c b/env/env.c index ad774f41175b..bcc66b4a6a12 100644 --- a/env/env.c +++ b/env/env.c @@ -53,6 +53,8 @@ static struct env_driver *_env_driver_lookup(enum env_location loc) return NULL; }
+#ifdef CONFIG_WEAK_SYMBOLS + static enum env_location env_locations[] = { #ifdef CONFIG_ENV_IS_IN_EEPROM ENVL_EEPROM, @@ -88,6 +90,7 @@ static enum env_location env_locations[] = { ENVL_NOWHERE, #endif }; +#endif /* CONFIG_WEAK_SYMBOLS */
static bool env_has_inited(enum env_location location) { @@ -106,6 +109,8 @@ static void env_set_inited(enum env_location location) gd->env_has_init |= BIT(location); }
+#ifdef CONFIG_WEAK_SYMBOLS + /** * arch_env_get_location() - Returns the best env location for an arch * @op: operations performed on the environment @@ -155,6 +160,7 @@ __weak enum env_location env_get_location(enum env_operation op, int prio) { return arch_env_get_location(op, prio); } +#endif /* CONFIG_WEAK_SYMBOLS */
/** * env_driver_lookup() - Finds the most suited environment location diff --git a/lib/efi_loader/efi_image_loader.c b/lib/efi_loader/efi_image_loader.c index 26df0da16c93..c473cd58cfb8 100644 --- a/lib/efi_loader/efi_image_loader.c +++ b/lib/efi_loader/efi_image_loader.c @@ -174,10 +174,13 @@ static efi_status_t efi_loader_relocate(const IMAGE_BASE_RELOCATION *rel, return EFI_SUCCESS; }
+#ifdef CONFIG_WEAK_SYMBOLS + void __weak invalidate_icache_all(void) { /* If the system doesn't support icache_all flush, cross our fingers */ } +#endif
/** * efi_set_code_and_data_type() - determine the memory types to be used for code diff --git a/lib/efi_loader/efi_runtime.c b/lib/efi_loader/efi_runtime.c index bf54d6ad871d..d7cf3b7e9dc8 100644 --- a/lib/efi_loader/efi_runtime.c +++ b/lib/efi_loader/efi_runtime.c @@ -375,6 +375,9 @@ out: return EFI_EXIT(EFI_UNSUPPORTED); #endif } + +#ifdef CONFIG_WEAK_SYMBOLS + /** * efi_reset_system() - reset system * @@ -399,6 +402,7 @@ void __weak __efi_runtime EFIAPI efi_reset_system( { return; } +#endif /* CONFIG_WEAK_SYMBOLS */
/** * efi_reset_system_init() - initialize the reset driver diff --git a/lib/lmb.c b/lib/lmb.c index b2c233edb64e..f46442aba48a 100644 --- a/lib/lmb.c +++ b/lib/lmb.c @@ -148,9 +148,11 @@ void arch_lmb_reserve_generic(struct lmb *lmb, ulong sp, ulong end, ulong align)
lmb_reserve(lmb, sp, bank_end - sp + 1);
+#ifdef CONFIG_WEAK_SYMBOLS + if (gd->flags & GD_FLG_SKIP_RELOC) lmb_reserve(lmb, (phys_addr_t)(uintptr_t)_start, gd->mon_len); - +#endif break; } } diff --git a/lib/time.c b/lib/time.c index 00f4a1ac8fb3..e8da2a3aa6a6 100644 --- a/lib/time.c +++ b/lib/time.c @@ -179,6 +179,8 @@ uint64_t usec_to_tick(unsigned long usec) return tick; }
+#ifdef CONFIG_WEAK_SYMBOLS + void __weak __udelay(unsigned long usec) { uint64_t tmp; @@ -188,6 +190,7 @@ void __weak __udelay(unsigned long usec) while (get_ticks() < tmp+1) /* loop till event */ /*NOP*/; } +#endif
/* ------------------------------------------------------------------------- */

Weak symbols are not well supported by the PE format, so disable them. We need to manually ensure that only one function is present in the source code.
Add a Kconfig option to control this and enable it when building for Windows.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
Kconfig | 12 ++++++++++++ include/linux/compiler_attributes.h | 4 ++++ 2 files changed, 16 insertions(+)
diff --git a/Kconfig b/Kconfig index 9ac816abef1c..985b09680934 100644 --- a/Kconfig +++ b/Kconfig @@ -75,6 +75,18 @@ config CLANG_VERSION config CC_IS_MSYS def_bool $(success,uname -o | grep -q Msys)
+config WEAK_SYMBOLS + bool "Enable use of weak symbols" + default y if !CC_IS_MSYS + help + The Portable Executable (PE) format used by Windows does not support + weak symbols very well. Even where it can be made to work, the __weak + function attribute cannot be made to work with PE. Supporting weak + symbols would involve changing the source code in undesirable ways. + + This option controls whether weak symbols are used, or not. When + disabled, the __weak function attribute does nothing. + choice prompt "Optimization level" default CC_OPTIMIZE_FOR_SIZE diff --git a/include/linux/compiler_attributes.h b/include/linux/compiler_attributes.h index 44c9a08d7346..c954109a065b 100644 --- a/include/linux/compiler_attributes.h +++ b/include/linux/compiler_attributes.h @@ -268,6 +268,10 @@ * gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-wea... * gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Variable-Attributes.html#index-wea... */ +#ifdef CONFIG_WEAK_SYMBOLS #define __weak __attribute__((__weak__)) +#else +#define __weak +#endif
#endif /* __LINUX_COMPILER_ATTRIBUTES_H */

Hi Simon,
On Sun, Apr 30, 2023 at 9:30 AM Simon Glass sjg@chromium.org wrote:
Weak symbols are not well supported by the PE format, so disable them. We need to manually ensure that only one function is present in the source code.
Add a Kconfig option to control this and enable it when building for Windows.
Signed-off-by: Simon Glass sjg@chromium.org
(no changes since v1)
Kconfig | 12 ++++++++++++ include/linux/compiler_attributes.h | 4 ++++ 2 files changed, 16 insertions(+)
diff --git a/Kconfig b/Kconfig index 9ac816abef1c..985b09680934 100644 --- a/Kconfig +++ b/Kconfig @@ -75,6 +75,18 @@ config CLANG_VERSION config CC_IS_MSYS def_bool $(success,uname -o | grep -q Msys)
+config WEAK_SYMBOLS
bool "Enable use of weak symbols"
default y if !CC_IS_MSYS
help
The Portable Executable (PE) format used by Windows does not support
weak symbols very well. Even where it can be made to work, the __weak
function attribute cannot be made to work with PE. Supporting weak
symbols would involve changing the source code in undesirable ways.
This option controls whether weak symbols are used, or not. When
disabled, the __weak function attribute does nothing.
choice prompt "Optimization level" default CC_OPTIMIZE_FOR_SIZE diff --git a/include/linux/compiler_attributes.h b/include/linux/compiler_attributes.h index 44c9a08d7346..c954109a065b 100644 --- a/include/linux/compiler_attributes.h +++ b/include/linux/compiler_attributes.h @@ -268,6 +268,10 @@
- gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-wea...
- gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Variable-Attributes.html#index-wea...
*/ +#ifdef CONFIG_WEAK_SYMBOLS #define __weak __attribute__((__weak__)) +#else +#define __weak +#endif
#endif /* __LINUX_COMPILER_ATTRIBUTES_H */
I am adding Fangrui who is a toolchain expert to this thread.
I chatted with him off-line, he thought we could probably go with the GCC+lld-link route. GNU ld's PE/COFF support is quite out of date.
Maybe switching to lld-link could also solve the linker script issue you are trying to resolve in patch#23 "sandbox: Augment the linker script for MSYS2" ?
Regards, Bin

On Tue, May 2, 2023 at 11:30 PM Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Sun, Apr 30, 2023 at 9:30 AM Simon Glass sjg@chromium.org wrote:
Weak symbols are not well supported by the PE format, so disable them. We need to manually ensure that only one function is present in the source code.
Add a Kconfig option to control this and enable it when building for Windows.
Signed-off-by: Simon Glass sjg@chromium.org
(no changes since v1)
Kconfig | 12 ++++++++++++ include/linux/compiler_attributes.h | 4 ++++ 2 files changed, 16 insertions(+)
diff --git a/Kconfig b/Kconfig index 9ac816abef1c..985b09680934 100644 --- a/Kconfig +++ b/Kconfig @@ -75,6 +75,18 @@ config CLANG_VERSION config CC_IS_MSYS def_bool $(success,uname -o | grep -q Msys)
+config WEAK_SYMBOLS
bool "Enable use of weak symbols"
default y if !CC_IS_MSYS
help
The Portable Executable (PE) format used by Windows does not support
weak symbols very well. Even where it can be made to work, the __weak
function attribute cannot be made to work with PE. Supporting weak
symbols would involve changing the source code in undesirable ways.
This option controls whether weak symbols are used, or not. When
disabled, the __weak function attribute does nothing.
choice prompt "Optimization level" default CC_OPTIMIZE_FOR_SIZE diff --git a/include/linux/compiler_attributes.h b/include/linux/compiler_attributes.h index 44c9a08d7346..c954109a065b 100644 --- a/include/linux/compiler_attributes.h +++ b/include/linux/compiler_attributes.h @@ -268,6 +268,10 @@
- gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-wea...
- gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Variable-Attributes.html#index-wea...
*/ +#ifdef CONFIG_WEAK_SYMBOLS #define __weak __attribute__((__weak__)) +#else +#define __weak +#endif
#endif /* __LINUX_COMPILER_ATTRIBUTES_H */
I am adding Fangrui who is a toolchain expert to this thread.
I chatted with him off-line, he thought we could probably go with the GCC+lld-link route. GNU ld's PE/COFF support is quite out of date.
Maybe switching to lld-link could also solve the linker script issue you are trying to resolve in patch#23 "sandbox: Augment the linker script for MSYS2" ?
Regards, Bin
https://maskray.me/blog/2021-04-25-weak-symbol#pecoff has some notes about weak symbol support for PE/COFF. The "undefined reference" issue is like the following:
printf 'void f(); int main() { f(); }' > a.c printf '__attribute__((weak)) void f() {} void unused() {}' > b.c x86_64-w64-mingw32-gcc -c a.c b.c
% x86_64-w64-mingw32-gcc a.o b.o /usr/bin/x86_64-w64-mingw32-ld: /tmp/ccyRH2ap.o:a.c:(.text+0xe): undefined reference to `f' collect2: error: ld returned 1 exit status
Making the __weak macro a no-op drops all the semantics about weak, which would likely lead to some pitfalls. I suggest that u-boot doesn't add support for a linker that doesn't the above case.
If u-boot is willing to refactor some code to avoid the above situation, I think it'd be fine as well.

These are now quite out-of-date. Update to 2023 versions and bring in those are are needed for the newer tool features.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
doc/build/tools.rst | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-)
diff --git a/doc/build/tools.rst b/doc/build/tools.rst index ec0172292585..6b7342a27606 100644 --- a/doc/build/tools.rst +++ b/doc/build/tools.rst @@ -24,15 +24,18 @@ you can use MSYS2, a software distro and building platform for Windows. Download the MSYS2 installer from https://www.msys2.org. Make sure you have installed all required packages below in order to build these host tools::
- * gcc (9.1.0) - * make (4.2.1) - * bison (3.4.2) - * diffutils (3.7) - * openssl-devel (1.1.1.d) + * gcc (11.3.0-3) + * make (4.4.1-1) + * bison (3.8.2-4) + * diffutils (3.9-1) + * flex (2.6.4-3) + * libgnutls-devel (3.8.0-1) + * libutil-linux-devel (2.35.2) + * openssl-devel (3.1.0-1)
Note the version numbers in these parentheses above are the package versions at the time being when writing this document. The MSYS2 installer tested is -http://repo.msys2.org/distrib/x86_64/msys2-x86_64-20190524.exe. +https://github.com/msys2/msys2-installer/releases/download/2023-03-18/msys2-...
There are 3 MSYS subsystems installed: MSYS2, MinGW32 and MinGW64. Each subsystem provides an environment to build Windows applications. The MSYS2 @@ -43,5 +46,6 @@ applications using a linux toolchain (gcc, bash, etc), targeting respectively
Launch the MSYS2 shell of the MSYS2 environment, and do the following::
- $ make tools-only_defconfig - $ make tools-only + $ pacman -S bison diffutils flex gcc libgnutls-devel libutil-linux-devel \ + make openssl-devel + $ make tools-only_defconfig tools-only

On Sat, Apr 29, 2023 at 07:30:00PM -0600, Simon Glass wrote:
These are now quite out-of-date. Update to 2023 versions and bring in those are are needed for the newer tool features.
Signed-off-by: Simon Glass sjg@chromium.org
(no changes since v1)
doc/build/tools.rst | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-)
diff --git a/doc/build/tools.rst b/doc/build/tools.rst index ec0172292585..6b7342a27606 100644 --- a/doc/build/tools.rst +++ b/doc/build/tools.rst @@ -24,15 +24,18 @@ you can use MSYS2, a software distro and building platform for Windows. Download the MSYS2 installer from https://www.msys2.org. Make sure you have installed all required packages below in order to build these host tools::
- gcc (9.1.0)
- make (4.2.1)
- bison (3.4.2)
- diffutils (3.7)
- openssl-devel (1.1.1.d)
- gcc (11.3.0-3)
- make (4.4.1-1)
- bison (3.8.2-4)
- diffutils (3.9-1)
- flex (2.6.4-3)
- libgnutls-devel (3.8.0-1)
- libutil-linux-devel (2.35.2)
- openssl-devel (3.1.0-1)
Note the version numbers in these parentheses above are the package versions at the time being when writing this document. The MSYS2 installer tested is -http://repo.msys2.org/distrib/x86_64/msys2-x86_64-20190524.exe. +https://github.com/msys2/msys2-installer/releases/download/2023-03-18/msys2-...
This should match the msys version we use in Azure, which is neither 2019 nor 2023. As part of splitting this series in to generic fixes that MSYS work has exposed and MSYS work itself, please update Azure to something newer, deal with any fall-out, and update this doc, thanks!

Add instructions for building u-boot.exe to run on Windows.
Signed-off-by: Simon Glass sjg@chromium.org ---
Changes in v2: - Clearify the documentation to explain the environment better
doc/build/gcc.rst | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+)
diff --git a/doc/build/gcc.rst b/doc/build/gcc.rst index a0650a51db4b..96861c2b3d90 100644 --- a/doc/build/gcc.rst +++ b/doc/build/gcc.rst @@ -184,8 +184,48 @@ Important ones are * clean - remove most generated files but keep the configuration * mrproper - remove all generated files + config + various backup files
+Building on/for Windows +----------------------- + +Limited support is available for Windows, including building sandbox in the +MSYS2 environment. This produces native applications, but they must have access +to the `msys-2.0.dll` file. + +Note that this is not the same as running Windows Subsystem for Linux (WSL), +which is designed to build Linux applications. + +It is best to use an out-of-tree build, so you can build multiple boards, +with the output in a temporary directory like `/tmp/b`. + +First enable Windows developer mode with `Developer Mode`_ so that symbolic +links can be used. Then run the MSYS2 shell and enable symbolic links:: + + cd + echo "export MSYS=winsymlinks:nativestrict" >>.bashrc + +Close all MSYS2 shells so that the setting takes effect. + +To build sandbox, first install some required packages:: + + pacman install bc bison diffutils flex gcc libgnutls-devel \ + libutil-linux-devel make openssl-devel python python-setuptools swig + +then:: + + make O=/tmp/b/sandbox -j$(nproc) sandbox_defconfig all + +Note that it currently only gets as far as running binman, since this doesn't +fully work on Windows. + +You can also build sandbox_spl:: + + make O=/tmp/b/sandbox_spl -j$(nproc) sandbox_spl_defconfig all + + Installation ------------
The process for installing U-Boot on the target device is device specific. Please, refer to the board specific documentation :doc:`../board/index`. + +.. _`Developer Mode`: https://msdn.microsoft.com/en-us/windows/uwp/get-started/enable-your-device-...

Hi Simon,
On Sun, Apr 30, 2023 at 9:30 AM Simon Glass sjg@chromium.org wrote:
Add instructions for building u-boot.exe to run on Windows.
Signed-off-by: Simon Glass sjg@chromium.org
Changes in v2:
- Clearify the documentation to explain the environment better
doc/build/gcc.rst | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+)
diff --git a/doc/build/gcc.rst b/doc/build/gcc.rst index a0650a51db4b..96861c2b3d90 100644 --- a/doc/build/gcc.rst +++ b/doc/build/gcc.rst @@ -184,8 +184,48 @@ Important ones are
- clean - remove most generated files but keep the configuration
- mrproper - remove all generated files + config + various backup files
+Building on/for Windows +-----------------------
+Limited support is available for Windows, including building sandbox in the +MSYS2 environment. This produces native applications, but they must have access +to the `msys-2.0.dll` file.
+Note that this is not the same as running Windows Subsystem for Linux (WSL), +which is designed to build Linux applications.
+It is best to use an out-of-tree build, so you can build multiple boards, +with the output in a temporary directory like `/tmp/b`.
+First enable Windows developer mode with `Developer Mode`_ so that symbolic +links can be used. Then run the MSYS2 shell and enable symbolic links::
- cd
- echo "export MSYS=winsymlinks:nativestrict" >>.bashrc
I think we should use winsymlinks:native as this is most similar to the behavior of ln -s on *nix.
+Close all MSYS2 shells so that the setting takes effect.
+To build sandbox, first install some required packages::
- pacman install bc bison diffutils flex gcc libgnutls-devel \
This should be 'pacman -S'
libutil-linux-devel make openssl-devel python python-setuptools swig
+then::
- make O=/tmp/b/sandbox -j$(nproc) sandbox_defconfig all
+Note that it currently only gets as far as running binman, since this doesn't +fully work on Windows.
+You can also build sandbox_spl::
- make O=/tmp/b/sandbox_spl -j$(nproc) sandbox_spl_defconfig all
Installation
The process for installing U-Boot on the target device is device specific. Please, refer to the board specific documentation :doc:`../board/index`.
+.. _`Developer Mode`: https://msdn.microsoft.com/en-us/windows/uwp/get-started/enable-your-device-...
Regards, Bin

These give lots of warnings for the form:
lib/efi_loader/helloworld.o:helloworld.c:(.pdata+0x0): relocation truncated to fit: IMAGE_REL_AMD64_ADDR32NB against `.text'
The tables are not needed except when debugging, so disable them for now.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
lib/efi_loader/Makefile | 8 ++++++++ scripts/Makefile.lib | 4 ++++ 2 files changed, 12 insertions(+)
diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile index 13a35eae6c06..c17d39eba91e 100644 --- a/lib/efi_loader/Makefile +++ b/lib/efi_loader/Makefile @@ -15,11 +15,19 @@ CFLAGS_efi_boottime.o += \ CFLAGS_boothart.o := $(CFLAGS_EFI) -Os -ffreestanding CFLAGS_REMOVE_boothart.o := $(CFLAGS_NON_EFI) CFLAGS_helloworld.o := $(CFLAGS_EFI) -Os -ffreestanding + +ifneq ($(MSYS_VERSION),0) +CFLAGS_helloworld.o += -fno-asynchronous-unwind-tables -fno-unwind-tables +endif + CFLAGS_REMOVE_helloworld.o := $(CFLAGS_NON_EFI) CFLAGS_dtbdump.o := $(CFLAGS_EFI) -Os -ffreestanding CFLAGS_REMOVE_dtbdump.o := $(CFLAGS_NON_EFI) CFLAGS_initrddump.o := $(CFLAGS_EFI) -Os -ffreestanding CFLAGS_REMOVE_initrddump.o := $(CFLAGS_NON_EFI) +ifneq ($(MSYS_VERSION),0) +CFLAGS_initrddump.o += -fno-asynchronous-unwind-tables -fno-unwind-tables +endif
ifdef CONFIG_RISCV always += boothart.efi diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib index aaae37d50a43..e751e452214e 100644 --- a/scripts/Makefile.lib +++ b/scripts/Makefile.lib @@ -450,6 +450,10 @@ targets += $(obj)/efi_crt0.o $(obj)/efi_reloc.o $(obj)/efi_freestanding.o
CFLAGS_REMOVE_efi_reloc.o := $(LTO_CFLAGS) CFLAGS_REMOVE_efi_freestanding.o := $(LTO_CFLAGS) +ifneq ($(MSYS_VERSION),0) +CFLAGS_efi_reloc.o += -fno-asynchronous-unwind-tables -fno-unwind-tables +CFLAGS_efi_freestanding.o += -fno-asynchronous-unwind-tables -fno-unwind-tables +endif
# ACPI # ---------------------------------------------------------------------------

Add a new rule to build sandbox for Windows. For now, no tests are run in this configuration.
Signed-off-by: Simon Glass sjg@chromium.org ---
Changes in v2: - Update the cover letter to better explain the motivation
.azure-pipelines.yml | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+)
diff --git a/.azure-pipelines.yml b/.azure-pipelines.yml index 76ffdeebd667..d15a86ff3650 100644 --- a/.azure-pipelines.yml +++ b/.azure-pipelines.yml @@ -39,6 +39,33 @@ stages: # Tell MSYS2 not to ‘cd’ our startup directory to HOME CHERE_INVOKING: yes
+ - job: sandbox_windows + displayName: 'Ensure sandbox build for Windows' + pool: + vmImage: $(windows_vm) + steps: + - powershell: | + (New-Object Net.WebClient).DownloadFile("https://github.com/msys2/msys2-installer/releases/download/2021-06-04/msys2-...", "sfx.exe") + displayName: 'Install MSYS2' + - script: | + sfx.exe -y -o%CD:~0,2%\ + %CD:~0,2%\msys64\usr\bin\bash -lc " " + %CD:~0,2%\msys64\usr\bin\bash -lc "pacman --noconfirm -Syuu" + %CD:~0,2%\msys64\usr\bin\bash -lc "pacman --noconfirm -Syuu" + displayName: 'Update MSYS2' + - script: | + %CD:~0,2%\msys64\usr\bin\bash -lc "pacman --noconfirm --needed -Sy bc bison diffutils flex gcc libgnutls-devel libutil-linux-devel make openssl-devel python python-setuptools swig" + displayName: 'Install Toolchain' + - script: | + echo make sandbox_defconfig all > build-tools.sh + %CD:~0,2%\msys64\usr\bin\bash -lc "bash build-tools.sh" + displayName: 'Build sandbox' + env: + # Tell MSYS2 we need a POSIX emulation layer + MSYSTEM: MSYS + # Tell MSYS2 not to ‘cd’ our startup directory to HOME + CHERE_INVOKING: yes + - job: tools_only_macOS displayName: 'Ensure host tools build for macOS X' pool:

Hi Simon,
On Sun, Apr 30, 2023 at 9:30 AM Simon Glass sjg@chromium.org wrote:
Add a new rule to build sandbox for Windows. For now, no tests are run in this configuration.
Signed-off-by: Simon Glass sjg@chromium.org
Changes in v2:
- Update the cover letter to better explain the motivation
.azure-pipelines.yml | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+)
diff --git a/.azure-pipelines.yml b/.azure-pipelines.yml index 76ffdeebd667..d15a86ff3650 100644 --- a/.azure-pipelines.yml +++ b/.azure-pipelines.yml @@ -39,6 +39,33 @@ stages: # Tell MSYS2 not to ‘cd’ our startup directory to HOME CHERE_INVOKING: yes
- job: sandbox_windows
- displayName: 'Ensure sandbox build for Windows'
- pool:
vmImage: $(windows_vm)
- steps:
- powershell: |
(New-Object Net.WebClient).DownloadFile("https://github.com/msys2/msys2-installer/releases/download/2021-06-04/msys2-base-x86_64-20210604.sfx.exe", "sfx.exe")
displayName: 'Install MSYS2'
- script: |
sfx.exe -y -o%CD:~0,2%\
%CD:~0,2%\msys64\usr\bin\bash -lc " "
%CD:~0,2%\msys64\usr\bin\bash -lc "pacman --noconfirm -Syuu"
%CD:~0,2%\msys64\usr\bin\bash -lc "pacman --noconfirm -Syuu"
displayName: 'Update MSYS2'
- script: |
%CD:~0,2%\msys64\usr\bin\bash -lc "pacman --noconfirm --needed -Sy bc bison diffutils flex gcc libgnutls-devel libutil-linux-devel make openssl-devel python python-setuptools swig"
displayName: 'Install Toolchain'
- script: |
echo make sandbox_defconfig all > build-tools.sh
nits: build-sandbox.sh
%CD:~0,2%\msys64\usr\bin\bash -lc "bash build-tools.sh"
displayName: 'Build sandbox'
env:
# Tell MSYS2 we need a POSIX emulation layer
MSYSTEM: MSYS
# Tell MSYS2 not to ‘cd’ our startup directory to HOME
CHERE_INVOKING: yes
Missed turning on symbolic link config for MSYS2
MSYS: winsymlinks:native
- job: tools_only_macOS displayName: 'Ensure host tools build for macOS X' pool:
Regards, Bin
participants (5)
-
Bin Meng
-
Fangrui Song
-
Pali Rohár
-
Simon Glass
-
Tom Rini