[PATCH 00/31] Allow building sandbox with MSYS2

[PATCH 00/31] Allow building sandbox with MSYS2

[Simon Glass]

This expands the existing work to allow sandbox to build and run on
Windows using MSYS2.

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.

[1] https://gist.github.com/thales17/fb2e4cff60890a51d9dddd4c6e832ad2


Simon Glass (31):
  patman: Declare the future Series memory
  u_boot_pylib: Correct name of readme
  u_boot_pylib: Make pty optional
  binman: Avoid using a symlink
  binman: Don't require a HOME directory
  pylibfdt: Allow building on Windows
  Fix Makefile warning about parallel targets
  mkimage: Correct checking of configuration node
  Kbuild: Detect including an MSYS2 path
  sandbox: Provide a linker script for MSYS2
  sandbox: Provide an EFI link script for PE
  sandbox: Drop dead code from Makefile
  sandbox: Disable raw Ethernet on MSYS2
  sandbox: Drop signal handling for MSYS2
  sandbox: Correct SDL.h header inclusion
  sandbox: Include errno.h in the test header
  sandbox: Allow weak symbols to be dropped
  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
  sandbox: Fix up setting of monitor_len on MSYS2
  ctype: Avoid using a symlink
  build: Disable weak symbols for MSYS2
  Makefile: Disable unsupported compiler options with PE
  Makefile: Correct the ans1_compiler rule for MSYS2
  Makefile: Disable LTO when building with MSYS2
  doc: Update the MSYS2 packages and versions
  doc: Show how to build sandbox for MSYS2
  Makefile: Drop unwind tables
  CI: Enable sandbox build for Windows

 .azure-pipelines.yml                  |  27 ++
 Kconfig                               |  15 +
 Makefile                              |  27 +-
 arch/sandbox/Makefile                 |   7 -
 arch/sandbox/config.mk                |   8 +-
 arch/sandbox/cpu/Makefile             |   2 +
 arch/sandbox/cpu/os.c                 |   3 +-
 arch/sandbox/cpu/sdl.c                |   2 +-
 arch/sandbox/cpu/u-boot-pe.lds        | 447 ++++++++++++++++++++++++++
 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                     |  35 ++
 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                   |   4 +-
 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/dtc/pylibfdt/Makefile         |  16 +-
 scripts/make_pip.sh                   |   9 +-
 tools/Makefile                        |  14 +-
 tools/binman/binman                   |  11 +-
 tools/binman/cmdline.py               |   2 +-
 tools/image-host.c                    |   4 +-
 tools/patman/commit.py                |   3 +
 tools/u_boot_pylib/README.rst         |   2 +-
 tools/u_boot_pylib/cros_subprocess.py |  28 +-
 tools/u_boot_pylib/pyproject.toml     |   2 +-
 46 files changed, 803 insertions(+), 62 deletions(-)
 create mode 100644 arch/sandbox/cpu/u-boot-pe.lds
 create mode 100644 arch/x86/lib/pe_x86_64_efi.lds
 mode change 120000 => 100644 include/ctype.h
 mode change 120000 => 100755 tools/binman/binman

-- 
2.40.0.634.g4ca3ef3211-goog

[PATCH 01/31] patman: Declare the future Series memory

[Simon Glass]

This member is used in series.MakeCcFile() so should be declared in the
Series class.

Add a declaration to silence the warning.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 tools/patman/commit.py | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tools/patman/commit.py b/tools/patman/commit.py
index 9537de43d391..684225c0e60d 100644
--- a/tools/patman/commit.py
+++ b/tools/patman/commit.py
@@ -29,6 +29,8 @@ class Commit:
             value: Set of people who gave that rtag, each a name/email string
         warn: List of warnings for this commit, each a str
         patch (str): Filename of the patch file for this commit
+        future (concurrent.futures.Future): Future object for processing this
+            commit, or None
     """
     def __init__(self, hash):
         self.hash = hash
@@ -42,6 +44,7 @@ class Commit:
         self.rtags = collections.defaultdict(set)
         self.warn = []
         self.patch = ''
+        self.future = None
 
     def __str__(self):
         return self.subject
-- 
2.40.0.634.g4ca3ef3211-goog

[PATCH 02/31] u_boot_pylib: Correct name of readme

[Simon Glass]

This has an rst prefix rather than md. Fix it.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 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",
-- 
2.40.0.634.g4ca3ef3211-goog

[PATCH 03/31] u_boot_pylib: Make pty optional

[Simon Glass]

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>
---

 scripts/make_pip.sh                   |  9 +++++++--
 tools/u_boot_pylib/cros_subprocess.py | 28 ++++++++++++++++++++++-----
 2 files changed, 30 insertions(+), 7 deletions(-)

diff --git a/scripts/make_pip.sh b/scripts/make_pip.sh
index 4602dcf61c88..bcff65240ba1 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}
@@ -108,7 +113,7 @@ echo "Completed build of ${tool}"
 # Use --skip-existing to work even if the version is already present
 if [ -n "${upload}" ]; then
 	echo "Uploading from ${dir}"
-	python3 -m twine upload ${repo} -u __token__ dist/*
+	python3 -m twine upload ${repo} --verbose -u __token__ dist/*
 	echo "Completed upload of ${tool}"
 fi
 
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 = []
-- 
2.40.0.634.g4ca3ef3211-goog

[PATCH 04/31] binman: Avoid using a symlink

[Simon Glass]

Use a small Python script instead of a symlink to avoid problems with
Windows.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 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()
-- 
2.40.0.634.g4ca3ef3211-goog

[PATCH 05/31] binman: Don't require a HOME directory

[Simon Glass]

This doesn't exist on Windows, so use os.expanduser() instead.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 tools/binman/cmdline.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/binman/cmdline.py b/tools/binman/cmdline.py
index 4b875a9dcda3..fafccab06012 100644
--- a/tools/binman/cmdline.py
+++ b/tools/binman/cmdline.py
@@ -95,7 +95,7 @@ controlled by a description in the board device tree.'''
     parser.add_argument('-H', '--full-help', action='store_true',
         default=False, help='Display the README file')
     parser.add_argument('--tooldir', type=str,
-        default=os.path.join(os.getenv('HOME'), '.binman-tools'),
+        default=os.path.expanduser('~/binman-tools'),
         help='Set the directory to store tools')
     parser.add_argument('--toolpath', type=str, action='append',
         help='Add a path to the list of directories containing tools')
-- 
2.40.0.634.g4ca3ef3211-goog

[PATCH 06/31] pylibfdt: Allow building on Windows

[Simon Glass]

Handle the different shared-object extension with MSYS2 by creating a new
SOEXT variable.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 Makefile                      | 11 +++++++++++
 scripts/dtc/pylibfdt/Makefile | 16 +++++++++-------
 2 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/Makefile b/Makefile
index eaaf7d267d31..dd3fcd1782e5 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 SOEXT := so
+else
+export SOEXT := 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..feca0bb9abb1 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.$(SOEXT)
+
 # 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*.$(SOEXT)
 	$(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*.$(SOEXT) $(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
-- 
2.40.0.634.g4ca3ef3211-goog

[PATCH 07/31] Fix Makefile warning about parallel targets

[Simon Glass]

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>
---

 tools/Makefile | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/tools/Makefile b/tools/Makefile
index 38699b069d63..2c5df76638f6 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -263,7 +263,19 @@ 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 $(obj)/common/%.c $(obj)/env/%.c $(obj)/lib/%.c:
+# 	$(call cmd,wrap)
+
+$(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
-- 
2.40.0.634.g4ca3ef3211-goog

[PATCH 08/31] mkimage: Correct checking of configuration node

[Simon Glass]

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>
---

 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));
-- 
2.40.0.634.g4ca3ef3211-goog

[PATCH 09/31] Kbuild: Detect including an MSYS2 path

[Simon Glass]

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>
---

 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)))
-- 
2.40.0.634.g4ca3ef3211-goog

[PATCH 10/31] sandbox: Provide a linker script for MSYS2

[Simon Glass]

Add a script to allow the U-Boot sandbox executable to be built for
Windows. Add a note as to why this seems to be necessary for now.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 Makefile                       |  11 +-
 arch/sandbox/config.mk         |   4 +
 arch/sandbox/cpu/u-boot-pe.lds | 447 +++++++++++++++++++++++++++++++++
 3 files changed, 460 insertions(+), 2 deletions(-)
 create mode 100644 arch/sandbox/cpu/u-boot-pe.lds

diff --git a/Makefile b/Makefile
index dd3fcd1782e5..0aa97a2c3b48 100644
--- a/Makefile
+++ b/Makefile
@@ -1730,6 +1730,13 @@ 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
+$(warning msys)
+endif
+
 # Rule to link u-boot
 # May be overridden by arch/$(ARCH)/config.mk
 ifeq ($(LTO_ENABLE),y)
@@ -1738,7 +1745,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)					\
@@ -1749,7 +1756,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						\
diff --git a/arch/sandbox/config.mk b/arch/sandbox/config.mk
index 2d184c5f652a..e05daf57ef8e 100644
--- a/arch/sandbox/config.mk
+++ b/arch/sandbox/config.mk
@@ -71,3 +71,7 @@ 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 = $(srctree)/arch/sandbox/cpu/u-boot-pe.lds
+endif
diff --git a/arch/sandbox/cpu/u-boot-pe.lds b/arch/sandbox/cpu/u-boot-pe.lds
new file mode 100644
index 000000000000..031e70fafd03
--- /dev/null
+++ b/arch/sandbox/cpu/u-boot-pe.lds
@@ -0,0 +1,447 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * U-Boot note: This was obtained by using the -verbose linker option. The
+ * U-Boot additions are marked below.
+ *
+ * Ideally we would add sections to the executable, as is done with the Linux
+ * build. But PE executables do not appear to work correctly if unexpected
+ * sections are present:
+ *
+ *   $ /tmp/b/sandbox/u-boot.exe
+ *   -bash: /tmp/b/sandbox/u-boot.exe: cannot execute binary file: Exec format error
+ *
+ * So we take a approach of rewriting the whole file, for now. This will likely
+ * break in the future when a toolchain change is made.
+ */
+
+/* Default linker script, for normal executables */
+/* Copyright (C) 2014-2023 Free Software Foundation, Inc.
+   Copying and distribution of this script, with or without modification,
+   are permitted in any medium without royalty provided the copyright
+   notice and this notice are preserved.  */
+
+OUTPUT_FORMAT(pei-x86-64)
+SEARCH_DIR("/usr/x86_64-pc-msys/lib"); SEARCH_DIR("/usr/lib"); SEARCH_DIR("/usr/lib/w32api");
+SECTIONS
+{
+  /* Make the virtual address and file offset synced if the alignment is
+     lower than the target page size. */
+  . = SIZEOF_HEADERS;
+  . = ALIGN(__section_alignment__);
+  .text  __image_base__ + ( __section_alignment__ < 0x1000 ? . : __section_alignment__ ) :
+  {
+    KEEP (*(SORT_NONE(.init)))
+    *(.text)
+    *(SORT(.text$*))
+     *(.text.*)
+     *(.gnu.linkonce.t.*)
+    *(.glue_7t)
+    *(.glue_7)
+    . = ALIGN(8);
+       /* Note: we always define __CTOR_LIST__ and ___CTOR_LIST__ here,
+          we do not PROVIDE them.  This is because the ctors.o startup
+	  code in libgcc defines them as common symbols, with the
+          expectation that they will be overridden by the definitions
+	  here.  If we PROVIDE the symbols then they will not be
+	  overridden and global constructors will not be run.
+	  See PR 22762 for more details.
+
+	  This does mean that it is not possible for a user to define
+	  their own __CTOR_LIST__ and __DTOR_LIST__ symbols; if they do,
+	  the content from those variables are included but the symbols
+	  defined here silently take precedence.  If they truly need to
+	  be redefined, a custom linker script will have to be used.
+	  (The custom script can just be a copy of this script with the
+	  PROVIDE() qualifiers added).
+	  In particular this means that ld -Ur does not work, because
+	  the proper __CTOR_LIST__ set by ld -Ur is overridden by a
+	  bogus __CTOR_LIST__ set by the final link.  See PR 46.  */
+       ___CTOR_LIST__ = .;
+       __CTOR_LIST__ = .;
+       LONG (-1); LONG (-1);
+       KEEP (*(.ctors));
+       KEEP (*(.ctor));
+       KEEP (*(SORT_BY_NAME(.ctors.*)));
+       LONG (0); LONG (0);
+       /* See comment about __CTOR_LIST__ above.  The same reasoning
+    	  applies here too.  */
+       ___DTOR_LIST__ = .;
+       __DTOR_LIST__ = .;
+       LONG (-1); LONG (-1);
+       KEEP (*(.dtors));
+       KEEP (*(.dtor));
+       KEEP (*(SORT_BY_NAME(.dtors.*)));
+       LONG (0); LONG (0);
+    KEEP (*(SORT_NONE(.fini)))
+    /* ??? Why is .gcc_exc here?  */
+     *(.gcc_exc)
+    PROVIDE (etext = .);
+     KEEP (*(.gcc_except_table))
+  }
+  /* The Cygwin32 library uses a section to avoid copying certain data
+     on fork.  This used to be named ".data".  The linker used
+     to include this between __data_start__ and __data_end__, but that
+     breaks building the cygwin32 dll.  Instead, we name the section
+     ".data_cygwin_nocopy" and explicitly include it after __data_end__. */
+  .data BLOCK(__section_alignment__) :
+  {
+    __data_start__ = . ;
+    *(.data)
+    *(.data2)
+    *(SORT(.data$*))
+    KEEP(*(.jcr))
+    __data_end__ = . ;
+    *(.data_cygwin_nocopy)
+  }
+  .rdata BLOCK(__section_alignment__) :
+  {
+    *(.rdata)
+	     *(SORT(.rdata$*))
+    . = ALIGN(4);
+    __rt_psrelocs_start = .;
+    KEEP(*(.rdata_runtime_pseudo_reloc))
+    __rt_psrelocs_end = .;
+
+	/* 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 */
+   }
+  __rt_psrelocs_size = __rt_psrelocs_end - __rt_psrelocs_start;
+  ___RUNTIME_PSEUDO_RELOC_LIST_END__ = .;
+  __RUNTIME_PSEUDO_RELOC_LIST_END__ = .;
+  ___RUNTIME_PSEUDO_RELOC_LIST__ = . - __rt_psrelocs_size;
+  __RUNTIME_PSEUDO_RELOC_LIST__ = . - __rt_psrelocs_size;
+  .eh_frame BLOCK(__section_alignment__) :
+  {
+    KEEP (*(.eh_frame*))
+  }
+  .pdata BLOCK(__section_alignment__) :
+  {
+    KEEP(*(.pdata*))
+  }
+  .xdata BLOCK(__section_alignment__) :
+  {
+    KEEP(*(.xdata*))
+  }
+  .bss BLOCK(__section_alignment__) :
+  {
+    __bss_start__ = . ;
+    *(.bss)
+    *(COMMON)
+    __bss_end__ = . ;
+  }
+  .edata BLOCK(__section_alignment__) :
+  {
+    *(.edata)
+  }
+  /DISCARD/ :
+  {
+    *(.debug$S)
+    *(.debug$T)
+    *(.debug$F)
+     *(.drectve)
+     *(.note.GNU-stack)
+     *(.gnu.lto_*)
+  }
+  .idata BLOCK(__section_alignment__) :
+  {
+    /* This cannot currently be handled with grouped sections.
+	See pep.em:sort_sections.  */
+    KEEP (SORT(*)(.idata$2))
+    KEEP (SORT(*)(.idata$3))
+    /* These zeroes mark the end of the import list.  */
+    LONG (0); LONG (0); LONG (0); LONG (0); LONG (0);
+    . = ALIGN(8);
+    KEEP (SORT(*)(.idata$4))
+    __IAT_start__ = .;
+    SORT(*)(.idata$5)
+    __IAT_end__ = .;
+    KEEP (SORT(*)(.idata$6))
+    KEEP (SORT(*)(.idata$7))
+  }
+  .CRT BLOCK(__section_alignment__) :
+  {
+    ___crt_xc_start__ = . ;
+    KEEP (*(SORT(.CRT$XC*)))  /* C initialization */
+    ___crt_xc_end__ = . ;
+    ___crt_xi_start__ = . ;
+    KEEP (*(SORT(.CRT$XI*)))  /* C++ initialization */
+    ___crt_xi_end__ = . ;
+    ___crt_xl_start__ = . ;
+    KEEP (*(SORT(.CRT$XL*)))  /* TLS callbacks */
+    /* ___crt_xl_end__ is defined in the TLS Directory support code */
+    ___crt_xp_start__ = . ;
+    KEEP (*(SORT(.CRT$XP*)))  /* Pre-termination */
+    ___crt_xp_end__ = . ;
+    ___crt_xt_start__ = . ;
+    KEEP (*(SORT(.CRT$XT*)))  /* Termination */
+    ___crt_xt_end__ = . ;
+  }
+  /* Windows TLS expects .tls$AAA to be at the start and .tls$ZZZ to be
+     at the end of the .tls section.  This is important because _tls_start MUST
+     be at the beginning of the section to enable SECREL32 relocations with TLS
+     data.  */
+  .tls BLOCK(__section_alignment__) :
+  {
+    ___tls_start__ = . ;
+    KEEP (*(.tls$AAA))
+    KEEP (*(.tls))
+    KEEP (*(.tls$))
+    KEEP (*(SORT(.tls$*)))
+    KEEP (*(.tls$ZZZ))
+    ___tls_end__ = . ;
+  }
+  .endjunk BLOCK(__section_alignment__) :
+  {
+    /* end is deprecated, don't use it */
+    PROVIDE (end = .);
+    PROVIDE ( _end = .);
+     __end__ = .;
+  }
+  .rsrc BLOCK(__section_alignment__) : SUBALIGN(4)
+  {
+    KEEP (*(.rsrc))
+    KEEP (*(.rsrc$*))
+  }
+  .reloc BLOCK(__section_alignment__) :
+  {
+    *(.reloc)
+  }
+  .stab BLOCK(__section_alignment__) (NOLOAD) :
+  {
+    *(.stab)
+  }
+  .stabstr BLOCK(__section_alignment__) (NOLOAD) :
+  {
+    *(.stabstr)
+  }
+  /* DWARF debug sections.
+     Symbols in the DWARF debugging sections are relative to the beginning
+     of the section.  Unlike other targets that fake this by putting the
+     section VMA at 0, the PE format will not allow it.  */
+  /* DWARF 1.1 and DWARF 2.  */
+  .debug_aranges BLOCK(__section_alignment__) (NOLOAD) :
+  {
+    *(.debug_aranges)
+  }
+  .zdebug_aranges BLOCK(__section_alignment__) (NOLOAD) :
+  {
+    *(.zdebug_aranges)
+  }
+  .debug_pubnames BLOCK(__section_alignment__) (NOLOAD) :
+  {
+    *(.debug_pubnames)
+  }
+  .zdebug_pubnames BLOCK(__section_alignment__) (NOLOAD) :
+  {
+    *(.zdebug_pubnames)
+  }
+  /* DWARF 2.  */
+  .debug_info BLOCK(__section_alignment__) (NOLOAD) :
+  {
+    *(.debug_info .gnu.linkonce.wi.*)
+  }
+  .zdebug_info BLOCK(__section_alignment__) (NOLOAD) :
+  {
+    *(.zdebug_info .zdebug.gnu.linkonce.wi.*)
+  }
+  .debug_abbrev BLOCK(__section_alignment__) (NOLOAD) :
+  {
+    *(.debug_abbrev)
+  }
+  .zdebug_abbrev BLOCK(__section_alignment__) (NOLOAD) :
+  {
+    *(.zdebug_abbrev)
+  }
+  .debug_line BLOCK(__section_alignment__) (NOLOAD) :
+  {
+    *(.debug_line)
+  }
+  .zdebug_line BLOCK(__section_alignment__) (NOLOAD) :
+  {
+    *(.zdebug_line)
+  }
+  .debug_frame BLOCK(__section_alignment__) (NOLOAD) :
+  {
+    *(.debug_frame*)
+  }
+  .zdebug_frame BLOCK(__section_alignment__) (NOLOAD) :
+  {
+    *(.zdebug_frame*)
+  }
+  .debug_str BLOCK(__section_alignment__) (NOLOAD) :
+  {
+    *(.debug_str)
+  }
+  .zdebug_str BLOCK(__section_alignment__) (NOLOAD) :
+  {
+    *(.zdebug_str)
+  }
+  .debug_loc BLOCK(__section_alignment__) (NOLOAD) :
+  {
+    *(.debug_loc)
+  }
+  .zdebug_loc BLOCK(__section_alignment__) (NOLOAD) :
+  {
+    *(.zdebug_loc)
+  }
+  .debug_macinfo BLOCK(__section_alignment__) (NOLOAD) :
+  {
+    *(.debug_macinfo)
+  }
+  .zdebug_macinfo BLOCK(__section_alignment__) (NOLOAD) :
+  {
+    *(.zdebug_macinfo)
+  }
+  /* SGI/MIPS DWARF 2 extensions.  */
+  .debug_weaknames BLOCK(__section_alignment__) (NOLOAD) :
+  {
+    *(.debug_weaknames)
+  }
+  .zdebug_weaknames BLOCK(__section_alignment__) (NOLOAD) :
+  {
+    *(.zdebug_weaknames)
+  }
+  .debug_funcnames BLOCK(__section_alignment__) (NOLOAD) :
+  {
+    *(.debug_funcnames)
+  }
+  .zdebug_funcnames BLOCK(__section_alignment__) (NOLOAD) :
+  {
+    *(.zdebug_funcnames)
+  }
+  .debug_typenames BLOCK(__section_alignment__) (NOLOAD) :
+  {
+    *(.debug_typenames)
+  }
+  .zdebug_typenames BLOCK(__section_alignment__) (NOLOAD) :
+  {
+    *(.zdebug_typenames)
+  }
+  .debug_varnames BLOCK(__section_alignment__) (NOLOAD) :
+  {
+    *(.debug_varnames)
+  }
+  .zdebug_varnames BLOCK(__section_alignment__) (NOLOAD) :
+  {
+    *(.zdebug_varnames)
+  }
+  /* DWARF 3.  */
+  .debug_pubtypes BLOCK(__section_alignment__) (NOLOAD) :
+  {
+    *(.debug_pubtypes)
+  }
+  .zdebug_pubtypes BLOCK(__section_alignment__) (NOLOAD) :
+  {
+    *(.zdebug_pubtypes)
+  }
+  .debug_ranges BLOCK(__section_alignment__) (NOLOAD) :
+  {
+    *(.debug_ranges)
+  }
+  .zdebug_ranges BLOCK(__section_alignment__) (NOLOAD) :
+  {
+    *(.zdebug_ranges)
+  }
+  /* DWARF 4.  */
+  .debug_types BLOCK(__section_alignment__) (NOLOAD) :
+  {
+    *(.debug_types .gnu.linkonce.wt.*)
+  }
+  .zdebug_types BLOCK(__section_alignment__) (NOLOAD) :
+  {
+    *(.zdebug_types .gnu.linkonce.wt.*)
+  }
+  /* DWARF 5.  */
+  .debug_addr BLOCK(__section_alignment__) (NOLOAD) :
+  {
+    *(.debug_addr)
+  }
+  .zdebug_addr BLOCK(__section_alignment__) (NOLOAD) :
+  {
+    *(.zdebug_addr)
+  }
+  .debug_line_str BLOCK(__section_alignment__) (NOLOAD) :
+  {
+    *(.debug_line_str)
+  }
+  .zdebug_line_str BLOCK(__section_alignment__) (NOLOAD) :
+  {
+    *(.zdebug_line_str)
+  }
+  .debug_loclists BLOCK(__section_alignment__) (NOLOAD) :
+  {
+    *(.debug_loclists)
+  }
+  .zdebug_loclists BLOCK(__section_alignment__) (NOLOAD) :
+  {
+    *(.zdebug_loclists)
+  }
+  .debug_macro BLOCK(__section_alignment__) (NOLOAD) :
+  {
+    *(.debug_macro)
+  }
+  .zdebug_macro BLOCK(__section_alignment__) (NOLOAD) :
+  {
+    *(.zdebug_macro)
+  }
+  .debug_names BLOCK(__section_alignment__) (NOLOAD) :
+  {
+    *(.debug_names)
+  }
+  .zdebug_names BLOCK(__section_alignment__) (NOLOAD) :
+  {
+    *(.zdebug_names)
+  }
+  .debug_rnglists BLOCK(__section_alignment__) (NOLOAD) :
+  {
+    *(.debug_rnglists)
+  }
+  .zdebug_rnglists BLOCK(__section_alignment__) (NOLOAD) :
+  {
+    *(.zdebug_rnglists)
+  }
+  .debug_str_offsets BLOCK(__section_alignment__) (NOLOAD) :
+  {
+    *(.debug_str_offsets)
+  }
+  .zdebug_str_offsets BLOCK(__section_alignment__) (NOLOAD) :
+  {
+    *(.zdebug_str_offsets)
+  }
+  .debug_sup BLOCK(__section_alignment__) (NOLOAD) :
+  {
+    *(.debug_sup)
+  }
+  /* For Go and Rust.  */
+  .debug_gdb_scripts BLOCK(__section_alignment__) (NOLOAD) :
+  {
+    *(.debug_gdb_scripts)
+  }
+  .zdebug_gdb_scripts BLOCK(__section_alignment__) (NOLOAD) :
+  {
+    *(.zdebug_gdb_scripts)
+  }
+}
-- 
2.40.0.634.g4ca3ef3211-goog

[PATCH 11/31] sandbox: Provide an EFI link script for PE

[Simon Glass]

Add another case for sandbox, when it is built on Windows.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 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 e05daf57ef8e..3269e6f3d2ce 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) }
+}
-- 
2.40.0.634.g4ca3ef3211-goog

[PATCH 12/31] sandbox: Drop dead code from Makefile

[Simon Glass]

This is not used, since the same code appears in cpu/Makefile

Drop it.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 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)
-- 
2.40.0.634.g4ca3ef3211-goog

[PATCH 13/31] sandbox: Disable raw Ethernet on MSYS2

[Simon Glass]

This relies on Linux features so cannot be built for Windows. Drop it.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 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
-- 
2.40.0.634.g4ca3ef3211-goog

[PATCH 14/31] sandbox: Drop signal handling for MSYS2

[Simon Glass]

The Linux register format is not used in Windows, so disable this feature.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 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 5e66304e2b9d..c8b83f923ddb 100644
--- a/arch/sandbox/cpu/os.c
+++ b/arch/sandbox/cpu/os.c
@@ -285,7 +285,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(__linux)
 	pc = context->uc_mcontext.gregs[REG_RIP];
 #elif defined(__aarch64__)
 	pc = context->uc_mcontext.pc;
-- 
2.40.0.634.g4ca3ef3211-goog

[PATCH 15/31] sandbox: Correct SDL.h header inclusion

[Simon Glass]

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>
---

 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>
 
 /**
-- 
2.40.0.634.g4ca3ef3211-goog

[PATCH 16/31] sandbox: Include errno.h in the test header

[Simon Glass]

Add this so that the header file can used without first including the
errno.h header.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 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>
 
-- 
2.40.0.634.g4ca3ef3211-goog

[PATCH 17/31] sandbox: Allow weak symbols to be dropped

[Simon Glass]

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>
---

 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
 
 /* ------------------------------------------------------------------------- */
 
-- 
2.40.0.634.g4ca3ef3211-goog

[PATCH 18/31] sections: Drop use of linux/types.h

[Simon Glass]

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>
---

 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
 
-- 
2.40.0.634.g4ca3ef3211-goog

[PATCH 19/31] sandbox: Drop incorrect inclusion of linux/types.h

[Simon Glass]

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>
---

 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 c8b83f923ddb..38689bff1695 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 0415f0f0e7a1..43e79fa9330e 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;
 
-- 
2.40.0.634.g4ca3ef3211-goog

[PATCH 20/31] sandbox: Drop inclusion of os.h in sscanf.c

[Simon Glass]

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>
---

 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>
-- 
2.40.0.634.g4ca3ef3211-goog

[PATCH 21/31] test: Avoid strange symbols in the assembler file

[Simon Glass]

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
'z' at present.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 include/test/test.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/test/test.h b/include/test/test.h
index 838e3ce8a8f3..3e3d14878a7b 100644
--- a/include/test/test.h
+++ b/include/test/test.h
@@ -127,8 +127,8 @@ struct unit_test {
 	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_~)
+#define UNIT_TEST_ALL_START()	ll_entry_start(struct unit_test, ut_$)
+#define UNIT_TEST_ALL_END()	ll_entry_start(struct unit_test, ut_z)
 #define UNIT_TEST_ALL_COUNT()	(UNIT_TEST_ALL_END() - UNIT_TEST_ALL_START())
 
 /* Sizes for devres tests */
-- 
2.40.0.634.g4ca3ef3211-goog

[PATCH 22/31] sandbox: Fix up setting of monitor_len on MSYS2

[Simon Glass]

The required linker symbols are not present. For now just return 0 in
this case.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 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 */
-- 
2.40.0.634.g4ca3ef3211-goog

[PATCH 23/31] ctype: Avoid using a symlink

[Simon Glass]

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>
---

 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>
-- 
2.40.0.634.g4ca3ef3211-goog

[PATCH 24/31] build: Disable weak symbols for MSYS2

[Simon Glass]

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>
---

 Kconfig                             | 15 +++++++++++++++
 include/linux/compiler_attributes.h |  4 ++++
 2 files changed, 19 insertions(+)

diff --git a/Kconfig b/Kconfig
index f24e4f0a331e..ca1402d09d10 100644
--- a/Kconfig
+++ b/Kconfig
@@ -72,6 +72,21 @@ config CLANG_VERSION
 	int
 	default $(shell,$(srctree)/scripts/clang-version.sh $(CC))
 
+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-weak-function-attribute
  *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Variable-Attributes.html#index-weak-variable-attribute
  */
+#ifdef CONFIG_WEAK_SYMBOLS
 #define __weak                          __attribute__((__weak__))
+#else
+#define __weak
+#endif
 
 #endif /* __LINUX_COMPILER_ATTRIBUTES_H */
-- 
2.40.0.634.g4ca3ef3211-goog

[PATCH 25/31] Makefile: Disable unsupported compiler options with PE

[Simon Glass]

The MSYS2 compiler does not support some of these options. Drop them to
avoid build errors.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 Makefile             | 2 ++
 scripts/Makefile.lib | 5 ++++-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index 0aa97a2c3b48..773260b81d5d 100644
--- a/Makefile
+++ b/Makefile
@@ -818,7 +818,9 @@ KBUILD_CPPFLAGS += $(KCPPFLAGS)
 KBUILD_AFLAGS += $(KAFLAGS)
 KBUILD_CFLAGS += $(KCFLAGS)
 
+ifeq ($(MSYS_VERSION),0)
 KBUILD_LDFLAGS  += -z noexecstack
+endif
 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..d309e26aaec8 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
+ifeq ($(MSYS_VERSION),0)
+KBUILD_EFILDFLAGS += -zexecstack -znocombreloc -znorelro
+endif
 KBUILD_EFILDFLAGS += $(call ld-option,--no-warn-rwx-segments)
 quiet_cmd_efi_ld = LD      $@
 cmd_efi_ld = $(LD) $(KBUILD_EFILDFLAGS) -T $(EFI_LDS_PATH) \
-- 
2.40.0.634.g4ca3ef3211-goog

[PATCH 26/31] Makefile: Correct the ans1_compiler rule for MSYS2

[Simon Glass]

Add the required extension to the Makefile rule.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 Makefile               | 1 +
 scripts/Makefile.build | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index 773260b81d5d..08eab87d0a11 100644
--- a/Makefile
+++ b/Makefile
@@ -48,6 +48,7 @@ ifeq ($(MSYS_VERSION),0)
 export SOEXT := so
 else
 export SOEXT := dll
+export ELFEXT := .exe
 endif
 
 # Avoid funny character set dependencies
diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 97dd4a64f6ef..547dc039ceb4 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$(ELFEXT)
 	$(call cmd,asn1_compiler)
 
 # Build the compiled-in targets
-- 
2.40.0.634.g4ca3ef3211-goog

[PATCH 27/31] Makefile: Disable LTO when building with MSYS2

[Simon Glass]

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)

Drop LTO for now.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 Makefile | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Makefile b/Makefile
index 08eab87d0a11..3a35d14a4497 100644
--- a/Makefile
+++ b/Makefile
@@ -658,9 +658,11 @@ 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 ($(MSYS_VERSION),0)
 ifeq ($(NO_LTO),)
 LTO_ENABLE=$(if $(CONFIG_LTO),y)
 endif
+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
-- 
2.40.0.634.g4ca3ef3211-goog

[PATCH 28/31] doc: Update the MSYS2 packages and versions

[Simon Glass]

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>
---

 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-x86_64-20230318.exe
 
 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
-- 
2.40.0.634.g4ca3ef3211-goog

[PATCH 29/31] doc: Show how to build sandbox for MSYS2

[Simon Glass]

Add instructions for building u-boot.exe to run on Windows.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 doc/build/gcc.rst | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/doc/build/gcc.rst b/doc/build/gcc.rst
index a0650a51db4b..40d0bde52413 100644
--- a/doc/build/gcc.rst
+++ b/doc/build/gcc.rst
@@ -184,8 +184,43 @@ Important ones are
 * clean - remove most generated files but keep the configuration
 * mrproper - remove all generated files + config + various backup files
 
+Building for Windows
+--------------------
+
+Limited support is available for Windows, including building sandbox.
+
+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`_. 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-for-development
-- 
2.40.0.634.g4ca3ef3211-goog

[PATCH 30/31] Makefile: Drop unwind tables

[Simon Glass]

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>
---

 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 d309e26aaec8..1c3781a751fc 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
 # ---------------------------------------------------------------------------
-- 
2.40.0.634.g4ca3ef3211-goog

[PATCH 31/31] CI: Enable sandbox build for Windows

[Simon Glass]

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>
---

 .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
+          %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:
-- 
2.40.0.634.g4ca3ef3211-goog

Re: [PATCH 14/31] sandbox: Drop signal handling for MSYS2

[Heinrich Schuchardt]

Am 25. April 2023 01:08:19 MESZ schrieb Simon Glass <sjg@chromium.org>:
>The Linux register format is not used in Windows, so disable this feature.
>
>Signed-off-by: Simon Glass <sjg@chromium.org>
>---
>
> 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 5e66304e2b9d..c8b83f923ddb 100644
>--- a/arch/sandbox/cpu/os.c
>+++ b/arch/sandbox/cpu/os.c
>@@ -285,7 +285,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(__linux)

In the commit message you only mention Linux and Windows. How about BSD and other POSIX?

> 	pc = context->uc_mcontext.gregs[REG_RIP];
> #elif defined(__aarch64__)

__aarch64__ does not imply Linux. There are Windows and other POSIX (BSD, macOS) systems running on ARM.

__riscv, armhf may also occur on different POSIX systems.

Best regards

Heinrich

> 	pc = context->uc_mcontext.pc;

Re: [PATCH 29/31] doc: Show how to build sandbox for MSYS2

[Heinrich Schuchardt]

Am 25. April 2023 01:08:34 MESZ schrieb Simon Glass <sjg@chromium.org>:
>Add instructions for building u-boot.exe to run on Windows.
>
>Signed-off-by: Simon Glass <sjg@chromium.org>
>---
>
> doc/build/gcc.rst | 35 +++++++++++++++++++++++++++++++++++
> 1 file changed, 35 insertions(+)
>
>diff --git a/doc/build/gcc.rst b/doc/build/gcc.rst
>index a0650a51db4b..40d0bde52413 100644
>--- a/doc/build/gcc.rst
>+++ b/doc/build/gcc.rst
>@@ -184,8 +184,43 @@ Important ones are
> * clean - remove most generated files but keep the configuration
> * mrproper - remove all generated files + config + various backup files
> 
>+Building for Windows
>+--------------------
>+
>+Limited support is available for Windows, including building sandbox.
>+
>+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`_. Then run the MSYS2

Please, describe why you suggest to enable developer mode.

According to https://learn.microsoft.com/en-us/windows/security/threat-protection/security-policy-settings/create-symbolic-links creation of symbolic links depends on policies and user authorization.

Please, mention the security implications.

Best regards

Heinrich

>+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-for-development

Re: [PATCH 21/31] test: Avoid strange symbols in the assembler file

[Heinrich Schuchardt]

Am 25. April 2023 01:08:26 MESZ schrieb Simon Glass <sjg@chromium.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
>'z' at present.
>
>Signed-off-by: Simon Glass <sjg@chromium.org>
>---
>
> include/test/test.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
>diff --git a/include/test/test.h b/include/test/test.h
>index 838e3ce8a8f3..3e3d14878a7b 100644
>--- a/include/test/test.h
>+++ b/include/test/test.h
>@@ -127,8 +127,8 @@ struct unit_test {
> 	ll_entry_count(struct unit_test, ut_ ## _suite)
> 
> /* Use ! and ~ so that all tests will be sorted between these two values */

This comment needs to be updated.

>-#define UNIT_TEST_ALL_START()	ll_entry_start(struct unit_test, ut_!)
>-#define UNIT_TEST_ALL_END()	ll_entry_start(struct unit_test, ut_~)
>+#define UNIT_TEST_ALL_START()	ll_entry_start(struct unit_test, ut_$)
>+#define UNIT_TEST_ALL_END()	ll_entry_start(struct unit_test, ut_z)

Can we use ut_zz here?

Best regards

Heinrich

> #define UNIT_TEST_ALL_COUNT()	(UNIT_TEST_ALL_END() - UNIT_TEST_ALL_START())
> 
> /* Sizes for devres tests */

Re: [PATCH 17/31] sandbox: Allow weak symbols to be dropped

[Heinrich Schuchardt]

Am 25. April 2023 01:08:22 MESZ schrieb Simon Glass <sjg@chromium.org>:
>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.

This comment does not explain the necessity of the change. Why is it really needed? Has the compiler version in MSYS a problem?

Best regards

Heinrich

>
>Signed-off-by: Simon Glass <sjg@chromium.org>
>---
>
> 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
> 
> /* ------------------------------------------------------------------------- */
> 

Re: [PATCH 17/31] sandbox: Allow weak symbols to be dropped

[Bin Meng]

Hi Simon,

On Tue, Apr 25, 2023 at 7:09 AM Simon Glass <sjg@chromium.org> wrote:
>
> 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>
> ---
>
>  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(-)
>

You probably need to use:

__declspec(selectany)

to replace __weak in the ELF for the *nix world.

Note this Microsoft bizarre does not provide the "strong override
weak" effect but I suspect what you only need is to get the build pass
on Windows so it should be okay.

Regards,
Bin

Re: [PATCH 29/31] doc: Show how to build sandbox for MSYS2

[Heinrich Schuchardt]

Am 25. April 2023 01:08:34 MESZ schrieb Simon Glass <sjg@chromium.org>:
>Add instructions for building u-boot.exe to run on Windows.
>
>Signed-off-by: Simon Glass <sjg@chromium.org>
>---
>
> doc/build/gcc.rst | 35 +++++++++++++++++++++++++++++++++++
> 1 file changed, 35 insertions(+)
>
>diff --git a/doc/build/gcc.rst b/doc/build/gcc.rst
>index a0650a51db4b..40d0bde52413 100644
>--- a/doc/build/gcc.rst
>+++ b/doc/build/gcc.rst
>@@ -184,8 +184,43 @@ Important ones are
> * clean - remove most generated files but keep the configuration
> * mrproper - remove all generated files + config + various backup files
> 
>+Building for Windows
>+--------------------

Do you mean 'on Windows'?

>+
>+Limited support is available for Windows, including building sandbox.
>+
>+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`.

This chapter sems not to be about building on Windows generally. Are you possibly using MinGW or MinGW-w64?  How about cygwin?

Please, describe the build environment accurately.

Wouldn't it be a much saner approach to use WSL? Then we don't need any of the quirks.

Best regards

Heinrich

>+
>+First enable Windows developer mode with `Developer Mode`_. 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-for-development

Re: [PATCH 00/31] Allow building sandbox with MSYS2

[Heinrich Schuchardt]

Am 25. April 2023 01:08:05 MESZ schrieb Simon Glass <sjg@chromium.org>:
>This expands the existing work to allow sandbox to build and run on
>Windows using MSYS2.

Why do we need this?
Wouldn't a developer on Windows be much better served using WSL (https://learn.microsoft.com/en-us/windows/wsl/install)?

Best regards

Heinrich

>
>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.
>
>[1] https://gist.github.com/thales17/fb2e4cff60890a51d9dddd4c6e832ad2
>
>
>Simon Glass (31):
>  patman: Declare the future Series memory
>  u_boot_pylib: Correct name of readme
>  u_boot_pylib: Make pty optional
>  binman: Avoid using a symlink
>  binman: Don't require a HOME directory
>  pylibfdt: Allow building on Windows
>  Fix Makefile warning about parallel targets
>  mkimage: Correct checking of configuration node
>  Kbuild: Detect including an MSYS2 path
>  sandbox: Provide a linker script for MSYS2
>  sandbox: Provide an EFI link script for PE
>  sandbox: Drop dead code from Makefile
>  sandbox: Disable raw Ethernet on MSYS2
>  sandbox: Drop signal handling for MSYS2
>  sandbox: Correct SDL.h header inclusion
>  sandbox: Include errno.h in the test header
>  sandbox: Allow weak symbols to be dropped
>  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
>  sandbox: Fix up setting of monitor_len on MSYS2
>  ctype: Avoid using a symlink
>  build: Disable weak symbols for MSYS2
>  Makefile: Disable unsupported compiler options with PE
>  Makefile: Correct the ans1_compiler rule for MSYS2
>  Makefile: Disable LTO when building with MSYS2
>  doc: Update the MSYS2 packages and versions
>  doc: Show how to build sandbox for MSYS2
>  Makefile: Drop unwind tables
>  CI: Enable sandbox build for Windows
>
> .azure-pipelines.yml                  |  27 ++
> Kconfig                               |  15 +
> Makefile                              |  27 +-
> arch/sandbox/Makefile                 |   7 -
> arch/sandbox/config.mk                |   8 +-
> arch/sandbox/cpu/Makefile             |   2 +
> arch/sandbox/cpu/os.c                 |   3 +-
> arch/sandbox/cpu/sdl.c                |   2 +-
> arch/sandbox/cpu/u-boot-pe.lds        | 447 ++++++++++++++++++++++++++
> 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                     |  35 ++
> 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                   |   4 +-
> 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/dtc/pylibfdt/Makefile         |  16 +-
> scripts/make_pip.sh                   |   9 +-
> tools/Makefile                        |  14 +-
> tools/binman/binman                   |  11 +-
> tools/binman/cmdline.py               |   2 +-
> tools/image-host.c                    |   4 +-
> tools/patman/commit.py                |   3 +
> tools/u_boot_pylib/README.rst         |   2 +-
> tools/u_boot_pylib/cros_subprocess.py |  28 +-
> tools/u_boot_pylib/pyproject.toml     |   2 +-
> 46 files changed, 803 insertions(+), 62 deletions(-)
> create mode 100644 arch/sandbox/cpu/u-boot-pe.lds
> create mode 100644 arch/x86/lib/pe_x86_64_efi.lds
> mode change 120000 => 100644 include/ctype.h
> mode change 120000 => 100755 tools/binman/binman
>

Re: [PATCH 29/31] doc: Show how to build sandbox for MSYS2

[Bin Meng]

On Tue, Apr 25, 2023 at 10:54 AM Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
>
>
> Am 25. April 2023 01:08:34 MESZ schrieb Simon Glass <sjg@chromium.org>:
> >Add instructions for building u-boot.exe to run on Windows.
> >
> >Signed-off-by: Simon Glass <sjg@chromium.org>
> >---
> >
> > doc/build/gcc.rst | 35 +++++++++++++++++++++++++++++++++++
> > 1 file changed, 35 insertions(+)
> >
> >diff --git a/doc/build/gcc.rst b/doc/build/gcc.rst
> >index a0650a51db4b..40d0bde52413 100644
> >--- a/doc/build/gcc.rst
> >+++ b/doc/build/gcc.rst
> >@@ -184,8 +184,43 @@ Important ones are
> > * clean - remove most generated files but keep the configuration
> > * mrproper - remove all generated files + config + various backup files
> >
> >+Building for Windows
> >+--------------------
>
> Do you mean 'on Windows'?
>
> >+
> >+Limited support is available for Windows, including building sandbox.
> >+
> >+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`.
>
> This chapter sems not to be about building on Windows generally. Are you possibly using MinGW or MinGW-w64?  How about cygwin?

The toolchain in the MSYS2 env should be MinGW-w64. cygwin is not for
native Windows application build.

>
> Please, describe the build environment accurately.
>
> Wouldn't it be a much saner approach to use WSL? Then we don't need any of the quirks.
>

WSL is essentially Linux and that's not what Simon wanted to do in
this series. Building sandbox under WSL is no different from what it
is on a Linux machine.

Regards,
Bin

Re: [PATCH 29/31] doc: Show how to build sandbox for MSYS2

[Heinrich Schuchardt]

Am 25. April 2023 05:01:30 MESZ schrieb Bin Meng <bmeng.cn@gmail.com>:
>On Tue, Apr 25, 2023 at 10:54 AM Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>>
>>
>>
>> Am 25. April 2023 01:08:34 MESZ schrieb Simon Glass <sjg@chromium.org>:
>> >Add instructions for building u-boot.exe to run on Windows.
>> >
>> >Signed-off-by: Simon Glass <sjg@chromium.org>
>> >---
>> >
>> > doc/build/gcc.rst | 35 +++++++++++++++++++++++++++++++++++
>> > 1 file changed, 35 insertions(+)
>> >
>> >diff --git a/doc/build/gcc.rst b/doc/build/gcc.rst
>> >index a0650a51db4b..40d0bde52413 100644
>> >--- a/doc/build/gcc.rst
>> >+++ b/doc/build/gcc.rst
>> >@@ -184,8 +184,43 @@ Important ones are
>> > * clean - remove most generated files but keep the configuration
>> > * mrproper - remove all generated files + config + various backup files
>> >
>> >+Building for Windows
>> >+--------------------
>>
>> Do you mean 'on Windows'?
>>
>> >+
>> >+Limited support is available for Windows, including building sandbox.
>> >+
>> >+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`.
>>
>> This chapter sems not to be about building on Windows generally. Are you possibly using MinGW or MinGW-w64?  How about cygwin?
>
>The toolchain in the MSYS2 env should be MinGW-w64. cygwin is not for
>native Windows application build.
>

This patch is incomplete.

>>
>> Please, describe the build environment accurately.
>>
>> Wouldn't it be a much saner approach to use WSL? Then we don't need any of the quirks.
>>
>
>WSL is essentially Linux and that's not what Simon wanted to do in
>this series. Building sandbox under WSL is no different from what it
>is on a Linux machine.

That is why the motivation for this series remains unclear. 

Best regards

Heinrich

>
>Regards,
>Bin

Re: [PATCH 03/31] u_boot_pylib: Make pty optional

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 1386 bytes --]

On Mon, Apr 24, 2023 at 05:08:08PM -0600, Simon Glass wrote:
> 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>
> ---
> 
>  scripts/make_pip.sh                   |  9 +++++++--
>  tools/u_boot_pylib/cros_subprocess.py | 28 ++++++++++++++++++++++-----
>  2 files changed, 30 insertions(+), 7 deletions(-)
> 
> diff --git a/scripts/make_pip.sh b/scripts/make_pip.sh
> index 4602dcf61c88..bcff65240ba1 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}
> @@ -108,7 +113,7 @@ echo "Completed build of ${tool}"
>  # Use --skip-existing to work even if the version is already present
>  if [ -n "${upload}" ]; then
>  	echo "Uploading from ${dir}"
> -	python3 -m twine upload ${repo} -u __token__ dist/*
> +	python3 -m twine upload ${repo} --verbose -u __token__ dist/*
>  	echo "Completed upload of ${tool}"
>  fi

The verbose here is I assume a debugging left-over.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 25/31] Makefile: Disable unsupported compiler options with PE

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 1550 bytes --]

On Mon, Apr 24, 2023 at 05:08:30PM -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>
> ---
> 
>  Makefile             | 2 ++
>  scripts/Makefile.lib | 5 ++++-
>  2 files changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/Makefile b/Makefile
> index 0aa97a2c3b48..773260b81d5d 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -818,7 +818,9 @@ KBUILD_CPPFLAGS += $(KCPPFLAGS)
>  KBUILD_AFLAGS += $(KAFLAGS)
>  KBUILD_CFLAGS += $(KCFLAGS)
>  
> +ifeq ($(MSYS_VERSION),0)
>  KBUILD_LDFLAGS  += -z noexecstack
> +endif
>  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..d309e26aaec8 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
> +ifeq ($(MSYS_VERSION),0)
> +KBUILD_EFILDFLAGS += -zexecstack -znocombreloc -znorelro
> +endif
>  KBUILD_EFILDFLAGS += $(call ld-option,--no-warn-rwx-segments)
>  quiet_cmd_efi_ld = LD      $@
>  cmd_efi_ld = $(LD) $(KBUILD_EFILDFLAGS) -T $(EFI_LDS_PATH) \

We should use $(call ld-option,...) here instead.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 00/31] Allow building sandbox with MSYS2

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 990 bytes --]

On Tue, Apr 25, 2023 at 04:59:53AM +0200, Heinrich Schuchardt wrote:
> 
> 
> Am 25. April 2023 01:08:05 MESZ schrieb Simon Glass <sjg@chromium.org>:
> >This expands the existing work to allow sandbox to build and run on
> >Windows using MSYS2.
> 
> Why do we need this?
> Wouldn't a developer on Windows be much better served using WSL (https://learn.microsoft.com/en-us/windows/wsl/install)?

Depending on corporate rules that may or may not be allowed. But I too
am interested in the use case driving these changes. Sandbox is
essentially a test platform. We test that we can do host tools on
Windows via MSYS2 for semi-reasonable reasons, since those tools can be
used in a number of places. We do the same on macOS in Azure as a check
on building regular targets there too (which should work and I believe
people do, both from macOS and from *BSD which this is a stand-in for,
as we can't get free VMs for them today). What's the end goal of this
series?

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 06/31] pylibfdt: Allow building on Windows

[Pali Rohár]

On Monday 24 April 2023 17:08:11 Simon Glass wrote:
> Handle the different shared-object extension with MSYS2 by creating a new
> SOEXT variable.

LIBEXT would be a better name as "SO" name refers to ELF.
I saw that variable LIBEXT is used in more projects for this purpose.

> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> 
>  Makefile                      | 11 +++++++++++
>  scripts/dtc/pylibfdt/Makefile | 16 +++++++++-------
>  2 files changed, 20 insertions(+), 7 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index eaaf7d267d31..dd3fcd1782e5 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 SOEXT := so
> +else
> +export SOEXT := 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..feca0bb9abb1 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.$(SOEXT)
> +
>  # 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*.$(SOEXT)
>  	$(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*.$(SOEXT) $(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
> -- 
> 2.40.0.634.g4ca3ef3211-goog
> 

Re: [PATCH 24/31] build: Disable weak symbols for MSYS2

[Pali Rohár]

On Monday 24 April 2023 17:08:29 Simon Glass wrote:
> Weak symbols are not well supported by the PE format, so disable them.

They are supported by PE format. This is likely issue of (older)
toolchain. What about rather requiring better toolchain version and
fix special cases of weak functions do not work correctly?

> 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>
> ---
> 
>  Kconfig                             | 15 +++++++++++++++
>  include/linux/compiler_attributes.h |  4 ++++
>  2 files changed, 19 insertions(+)
> 
> diff --git a/Kconfig b/Kconfig
> index f24e4f0a331e..ca1402d09d10 100644
> --- a/Kconfig
> +++ b/Kconfig
> @@ -72,6 +72,21 @@ config CLANG_VERSION
>  	int
>  	default $(shell,$(srctree)/scripts/clang-version.sh $(CC))
>  
> +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-weak-function-attribute
>   *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Variable-Attributes.html#index-weak-variable-attribute
>   */
> +#ifdef CONFIG_WEAK_SYMBOLS
>  #define __weak                          __attribute__((__weak__))
> +#else
> +#define __weak
> +#endif
>  
>  #endif /* __LINUX_COMPILER_ATTRIBUTES_H */
> -- 
> 2.40.0.634.g4ca3ef3211-goog
> 

Re: [PATCH 10/31] sandbox: Provide a linker script for MSYS2

[Pali Rohár]

On Monday 24 April 2023 17:08:15 Simon Glass wrote:
> Add a script to allow the U-Boot sandbox executable to be built for
> Windows. Add a note as to why this seems to be necessary for now.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> 
>  Makefile                       |  11 +-
>  arch/sandbox/config.mk         |   4 +
>  arch/sandbox/cpu/u-boot-pe.lds | 447 +++++++++++++++++++++++++++++++++
>  3 files changed, 460 insertions(+), 2 deletions(-)
>  create mode 100644 arch/sandbox/cpu/u-boot-pe.lds
> 
> diff --git a/Makefile b/Makefile
> index dd3fcd1782e5..0aa97a2c3b48 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1730,6 +1730,13 @@ 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
> +$(warning msys)
> +endif
> +
>  # Rule to link u-boot
>  # May be overridden by arch/$(ARCH)/config.mk
>  ifeq ($(LTO_ENABLE),y)
> @@ -1738,7 +1745,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)					\
> @@ -1749,7 +1756,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						\
> diff --git a/arch/sandbox/config.mk b/arch/sandbox/config.mk
> index 2d184c5f652a..e05daf57ef8e 100644
> --- a/arch/sandbox/config.mk
> +++ b/arch/sandbox/config.mk
> @@ -71,3 +71,7 @@ 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 = $(srctree)/arch/sandbox/cpu/u-boot-pe.lds
> +endif
> diff --git a/arch/sandbox/cpu/u-boot-pe.lds b/arch/sandbox/cpu/u-boot-pe.lds
> new file mode 100644
> index 000000000000..031e70fafd03
> --- /dev/null
> +++ b/arch/sandbox/cpu/u-boot-pe.lds
> @@ -0,0 +1,447 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * U-Boot note: This was obtained by using the -verbose linker option. The
> + * U-Boot additions are marked below.
> + *
> + * Ideally we would add sections to the executable, as is done with the Linux
> + * build. But PE executables do not appear to work correctly if unexpected
> + * sections are present:
> + *
> + *   $ /tmp/b/sandbox/u-boot.exe
> + *   -bash: /tmp/b/sandbox/u-boot.exe: cannot execute binary file: Exec format error
> + *
> + * So we take a approach of rewriting the whole file, for now. This will likely
> + * break in the future when a toolchain change is made.

Why not rather provide "layer" linker script which does this "rewriting"
on top of the default linker script? With this way it is not needed to
update linker script when a toolchain change it.

> + */
> +
> +/* Default linker script, for normal executables */
> +/* Copyright (C) 2014-2023 Free Software Foundation, Inc.
> +   Copying and distribution of this script, with or without modification,
> +   are permitted in any medium without royalty provided the copyright
> +   notice and this notice are preserved.  */
> +
> +OUTPUT_FORMAT(pei-x86-64)
> +SEARCH_DIR("/usr/x86_64-pc-msys/lib"); SEARCH_DIR("/usr/lib"); SEARCH_DIR("/usr/lib/w32api");
> +SECTIONS
> +{
> +  /* Make the virtual address and file offset synced if the alignment is
> +     lower than the target page size. */
> +  . = SIZEOF_HEADERS;
> +  . = ALIGN(__section_alignment__);
> +  .text  __image_base__ + ( __section_alignment__ < 0x1000 ? . : __section_alignment__ ) :
> +  {
> +    KEEP (*(SORT_NONE(.init)))
> +    *(.text)
> +    *(SORT(.text$*))
> +     *(.text.*)
> +     *(.gnu.linkonce.t.*)
> +    *(.glue_7t)
> +    *(.glue_7)
> +    . = ALIGN(8);
> +       /* Note: we always define __CTOR_LIST__ and ___CTOR_LIST__ here,
> +          we do not PROVIDE them.  This is because the ctors.o startup
> +	  code in libgcc defines them as common symbols, with the
> +          expectation that they will be overridden by the definitions
> +	  here.  If we PROVIDE the symbols then they will not be
> +	  overridden and global constructors will not be run.
> +	  See PR 22762 for more details.
> +
> +	  This does mean that it is not possible for a user to define
> +	  their own __CTOR_LIST__ and __DTOR_LIST__ symbols; if they do,
> +	  the content from those variables are included but the symbols
> +	  defined here silently take precedence.  If they truly need to
> +	  be redefined, a custom linker script will have to be used.
> +	  (The custom script can just be a copy of this script with the
> +	  PROVIDE() qualifiers added).
> +	  In particular this means that ld -Ur does not work, because
> +	  the proper __CTOR_LIST__ set by ld -Ur is overridden by a
> +	  bogus __CTOR_LIST__ set by the final link.  See PR 46.  */
> +       ___CTOR_LIST__ = .;
> +       __CTOR_LIST__ = .;
> +       LONG (-1); LONG (-1);
> +       KEEP (*(.ctors));
> +       KEEP (*(.ctor));
> +       KEEP (*(SORT_BY_NAME(.ctors.*)));
> +       LONG (0); LONG (0);
> +       /* See comment about __CTOR_LIST__ above.  The same reasoning
> +    	  applies here too.  */
> +       ___DTOR_LIST__ = .;
> +       __DTOR_LIST__ = .;
> +       LONG (-1); LONG (-1);
> +       KEEP (*(.dtors));
> +       KEEP (*(.dtor));
> +       KEEP (*(SORT_BY_NAME(.dtors.*)));
> +       LONG (0); LONG (0);
> +    KEEP (*(SORT_NONE(.fini)))
> +    /* ??? Why is .gcc_exc here?  */
> +     *(.gcc_exc)
> +    PROVIDE (etext = .);
> +     KEEP (*(.gcc_except_table))
> +  }
> +  /* The Cygwin32 library uses a section to avoid copying certain data
> +     on fork.  This used to be named ".data".  The linker used
> +     to include this between __data_start__ and __data_end__, but that
> +     breaks building the cygwin32 dll.  Instead, we name the section
> +     ".data_cygwin_nocopy" and explicitly include it after __data_end__. */
> +  .data BLOCK(__section_alignment__) :
> +  {
> +    __data_start__ = . ;
> +    *(.data)
> +    *(.data2)
> +    *(SORT(.data$*))
> +    KEEP(*(.jcr))
> +    __data_end__ = . ;
> +    *(.data_cygwin_nocopy)
> +  }
> +  .rdata BLOCK(__section_alignment__) :
> +  {
> +    *(.rdata)
> +	     *(SORT(.rdata$*))
> +    . = ALIGN(4);
> +    __rt_psrelocs_start = .;
> +    KEEP(*(.rdata_runtime_pseudo_reloc))
> +    __rt_psrelocs_end = .;
> +
> +	/* 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 */
> +   }
> +  __rt_psrelocs_size = __rt_psrelocs_end - __rt_psrelocs_start;
> +  ___RUNTIME_PSEUDO_RELOC_LIST_END__ = .;
> +  __RUNTIME_PSEUDO_RELOC_LIST_END__ = .;
> +  ___RUNTIME_PSEUDO_RELOC_LIST__ = . - __rt_psrelocs_size;
> +  __RUNTIME_PSEUDO_RELOC_LIST__ = . - __rt_psrelocs_size;
> +  .eh_frame BLOCK(__section_alignment__) :
> +  {
> +    KEEP (*(.eh_frame*))
> +  }
> +  .pdata BLOCK(__section_alignment__) :
> +  {
> +    KEEP(*(.pdata*))
> +  }
> +  .xdata BLOCK(__section_alignment__) :
> +  {
> +    KEEP(*(.xdata*))
> +  }
> +  .bss BLOCK(__section_alignment__) :
> +  {
> +    __bss_start__ = . ;
> +    *(.bss)
> +    *(COMMON)
> +    __bss_end__ = . ;
> +  }
> +  .edata BLOCK(__section_alignment__) :
> +  {
> +    *(.edata)
> +  }
> +  /DISCARD/ :
> +  {
> +    *(.debug$S)
> +    *(.debug$T)
> +    *(.debug$F)
> +     *(.drectve)
> +     *(.note.GNU-stack)
> +     *(.gnu.lto_*)
> +  }
> +  .idata BLOCK(__section_alignment__) :
> +  {
> +    /* This cannot currently be handled with grouped sections.
> +	See pep.em:sort_sections.  */
> +    KEEP (SORT(*)(.idata$2))
> +    KEEP (SORT(*)(.idata$3))
> +    /* These zeroes mark the end of the import list.  */
> +    LONG (0); LONG (0); LONG (0); LONG (0); LONG (0);
> +    . = ALIGN(8);
> +    KEEP (SORT(*)(.idata$4))
> +    __IAT_start__ = .;
> +    SORT(*)(.idata$5)
> +    __IAT_end__ = .;
> +    KEEP (SORT(*)(.idata$6))
> +    KEEP (SORT(*)(.idata$7))
> +  }
> +  .CRT BLOCK(__section_alignment__) :
> +  {
> +    ___crt_xc_start__ = . ;
> +    KEEP (*(SORT(.CRT$XC*)))  /* C initialization */
> +    ___crt_xc_end__ = . ;
> +    ___crt_xi_start__ = . ;
> +    KEEP (*(SORT(.CRT$XI*)))  /* C++ initialization */
> +    ___crt_xi_end__ = . ;
> +    ___crt_xl_start__ = . ;
> +    KEEP (*(SORT(.CRT$XL*)))  /* TLS callbacks */
> +    /* ___crt_xl_end__ is defined in the TLS Directory support code */
> +    ___crt_xp_start__ = . ;
> +    KEEP (*(SORT(.CRT$XP*)))  /* Pre-termination */
> +    ___crt_xp_end__ = . ;
> +    ___crt_xt_start__ = . ;
> +    KEEP (*(SORT(.CRT$XT*)))  /* Termination */
> +    ___crt_xt_end__ = . ;
> +  }
> +  /* Windows TLS expects .tls$AAA to be at the start and .tls$ZZZ to be
> +     at the end of the .tls section.  This is important because _tls_start MUST
> +     be at the beginning of the section to enable SECREL32 relocations with TLS
> +     data.  */
> +  .tls BLOCK(__section_alignment__) :
> +  {
> +    ___tls_start__ = . ;
> +    KEEP (*(.tls$AAA))
> +    KEEP (*(.tls))
> +    KEEP (*(.tls$))
> +    KEEP (*(SORT(.tls$*)))
> +    KEEP (*(.tls$ZZZ))
> +    ___tls_end__ = . ;
> +  }
> +  .endjunk BLOCK(__section_alignment__) :
> +  {
> +    /* end is deprecated, don't use it */
> +    PROVIDE (end = .);
> +    PROVIDE ( _end = .);
> +     __end__ = .;
> +  }
> +  .rsrc BLOCK(__section_alignment__) : SUBALIGN(4)
> +  {
> +    KEEP (*(.rsrc))
> +    KEEP (*(.rsrc$*))
> +  }
> +  .reloc BLOCK(__section_alignment__) :
> +  {
> +    *(.reloc)
> +  }
> +  .stab BLOCK(__section_alignment__) (NOLOAD) :
> +  {
> +    *(.stab)
> +  }
> +  .stabstr BLOCK(__section_alignment__) (NOLOAD) :
> +  {
> +    *(.stabstr)
> +  }
> +  /* DWARF debug sections.
> +     Symbols in the DWARF debugging sections are relative to the beginning
> +     of the section.  Unlike other targets that fake this by putting the
> +     section VMA at 0, the PE format will not allow it.  */
> +  /* DWARF 1.1 and DWARF 2.  */
> +  .debug_aranges BLOCK(__section_alignment__) (NOLOAD) :
> +  {
> +    *(.debug_aranges)
> +  }
> +  .zdebug_aranges BLOCK(__section_alignment__) (NOLOAD) :
> +  {
> +    *(.zdebug_aranges)
> +  }
> +  .debug_pubnames BLOCK(__section_alignment__) (NOLOAD) :
> +  {
> +    *(.debug_pubnames)
> +  }
> +  .zdebug_pubnames BLOCK(__section_alignment__) (NOLOAD) :
> +  {
> +    *(.zdebug_pubnames)
> +  }
> +  /* DWARF 2.  */
> +  .debug_info BLOCK(__section_alignment__) (NOLOAD) :
> +  {
> +    *(.debug_info .gnu.linkonce.wi.*)
> +  }
> +  .zdebug_info BLOCK(__section_alignment__) (NOLOAD) :
> +  {
> +    *(.zdebug_info .zdebug.gnu.linkonce.wi.*)
> +  }
> +  .debug_abbrev BLOCK(__section_alignment__) (NOLOAD) :
> +  {
> +    *(.debug_abbrev)
> +  }
> +  .zdebug_abbrev BLOCK(__section_alignment__) (NOLOAD) :
> +  {
> +    *(.zdebug_abbrev)
> +  }
> +  .debug_line BLOCK(__section_alignment__) (NOLOAD) :
> +  {
> +    *(.debug_line)
> +  }
> +  .zdebug_line BLOCK(__section_alignment__) (NOLOAD) :
> +  {
> +    *(.zdebug_line)
> +  }
> +  .debug_frame BLOCK(__section_alignment__) (NOLOAD) :
> +  {
> +    *(.debug_frame*)
> +  }
> +  .zdebug_frame BLOCK(__section_alignment__) (NOLOAD) :
> +  {
> +    *(.zdebug_frame*)
> +  }
> +  .debug_str BLOCK(__section_alignment__) (NOLOAD) :
> +  {
> +    *(.debug_str)
> +  }
> +  .zdebug_str BLOCK(__section_alignment__) (NOLOAD) :
> +  {
> +    *(.zdebug_str)
> +  }
> +  .debug_loc BLOCK(__section_alignment__) (NOLOAD) :
> +  {
> +    *(.debug_loc)
> +  }
> +  .zdebug_loc BLOCK(__section_alignment__) (NOLOAD) :
> +  {
> +    *(.zdebug_loc)
> +  }
> +  .debug_macinfo BLOCK(__section_alignment__) (NOLOAD) :
> +  {
> +    *(.debug_macinfo)
> +  }
> +  .zdebug_macinfo BLOCK(__section_alignment__) (NOLOAD) :
> +  {
> +    *(.zdebug_macinfo)
> +  }
> +  /* SGI/MIPS DWARF 2 extensions.  */
> +  .debug_weaknames BLOCK(__section_alignment__) (NOLOAD) :
> +  {
> +    *(.debug_weaknames)
> +  }
> +  .zdebug_weaknames BLOCK(__section_alignment__) (NOLOAD) :
> +  {
> +    *(.zdebug_weaknames)
> +  }
> +  .debug_funcnames BLOCK(__section_alignment__) (NOLOAD) :
> +  {
> +    *(.debug_funcnames)
> +  }
> +  .zdebug_funcnames BLOCK(__section_alignment__) (NOLOAD) :
> +  {
> +    *(.zdebug_funcnames)
> +  }
> +  .debug_typenames BLOCK(__section_alignment__) (NOLOAD) :
> +  {
> +    *(.debug_typenames)
> +  }
> +  .zdebug_typenames BLOCK(__section_alignment__) (NOLOAD) :
> +  {
> +    *(.zdebug_typenames)
> +  }
> +  .debug_varnames BLOCK(__section_alignment__) (NOLOAD) :
> +  {
> +    *(.debug_varnames)
> +  }
> +  .zdebug_varnames BLOCK(__section_alignment__) (NOLOAD) :
> +  {
> +    *(.zdebug_varnames)
> +  }
> +  /* DWARF 3.  */
> +  .debug_pubtypes BLOCK(__section_alignment__) (NOLOAD) :
> +  {
> +    *(.debug_pubtypes)
> +  }
> +  .zdebug_pubtypes BLOCK(__section_alignment__) (NOLOAD) :
> +  {
> +    *(.zdebug_pubtypes)
> +  }
> +  .debug_ranges BLOCK(__section_alignment__) (NOLOAD) :
> +  {
> +    *(.debug_ranges)
> +  }
> +  .zdebug_ranges BLOCK(__section_alignment__) (NOLOAD) :
> +  {
> +    *(.zdebug_ranges)
> +  }
> +  /* DWARF 4.  */
> +  .debug_types BLOCK(__section_alignment__) (NOLOAD) :
> +  {
> +    *(.debug_types .gnu.linkonce.wt.*)
> +  }
> +  .zdebug_types BLOCK(__section_alignment__) (NOLOAD) :
> +  {
> +    *(.zdebug_types .gnu.linkonce.wt.*)
> +  }
> +  /* DWARF 5.  */
> +  .debug_addr BLOCK(__section_alignment__) (NOLOAD) :
> +  {
> +    *(.debug_addr)
> +  }
> +  .zdebug_addr BLOCK(__section_alignment__) (NOLOAD) :
> +  {
> +    *(.zdebug_addr)
> +  }
> +  .debug_line_str BLOCK(__section_alignment__) (NOLOAD) :
> +  {
> +    *(.debug_line_str)
> +  }
> +  .zdebug_line_str BLOCK(__section_alignment__) (NOLOAD) :
> +  {
> +    *(.zdebug_line_str)
> +  }
> +  .debug_loclists BLOCK(__section_alignment__) (NOLOAD) :
> +  {
> +    *(.debug_loclists)
> +  }
> +  .zdebug_loclists BLOCK(__section_alignment__) (NOLOAD) :
> +  {
> +    *(.zdebug_loclists)
> +  }
> +  .debug_macro BLOCK(__section_alignment__) (NOLOAD) :
> +  {
> +    *(.debug_macro)
> +  }
> +  .zdebug_macro BLOCK(__section_alignment__) (NOLOAD) :
> +  {
> +    *(.zdebug_macro)
> +  }
> +  .debug_names BLOCK(__section_alignment__) (NOLOAD) :
> +  {
> +    *(.debug_names)
> +  }
> +  .zdebug_names BLOCK(__section_alignment__) (NOLOAD) :
> +  {
> +    *(.zdebug_names)
> +  }
> +  .debug_rnglists BLOCK(__section_alignment__) (NOLOAD) :
> +  {
> +    *(.debug_rnglists)
> +  }
> +  .zdebug_rnglists BLOCK(__section_alignment__) (NOLOAD) :
> +  {
> +    *(.zdebug_rnglists)
> +  }
> +  .debug_str_offsets BLOCK(__section_alignment__) (NOLOAD) :
> +  {
> +    *(.debug_str_offsets)
> +  }
> +  .zdebug_str_offsets BLOCK(__section_alignment__) (NOLOAD) :
> +  {
> +    *(.zdebug_str_offsets)
> +  }
> +  .debug_sup BLOCK(__section_alignment__) (NOLOAD) :
> +  {
> +    *(.debug_sup)
> +  }
> +  /* For Go and Rust.  */
> +  .debug_gdb_scripts BLOCK(__section_alignment__) (NOLOAD) :
> +  {
> +    *(.debug_gdb_scripts)
> +  }
> +  .zdebug_gdb_scripts BLOCK(__section_alignment__) (NOLOAD) :
> +  {
> +    *(.zdebug_gdb_scripts)
> +  }
> +}
> -- 
> 2.40.0.634.g4ca3ef3211-goog
> 

Re: [PATCH 25/31] Makefile: Disable unsupported compiler options with PE

[Pali Rohár]

On Monday 24 April 2023 17:08:30 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>
> ---
> 
>  Makefile             | 2 ++
>  scripts/Makefile.lib | 5 ++++-
>  2 files changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/Makefile b/Makefile
> index 0aa97a2c3b48..773260b81d5d 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -818,7 +818,9 @@ KBUILD_CPPFLAGS += $(KCPPFLAGS)
>  KBUILD_AFLAGS += $(KAFLAGS)
>  KBUILD_CFLAGS += $(KCFLAGS)
>  
> +ifeq ($(MSYS_VERSION),0)
>  KBUILD_LDFLAGS  += -z noexecstack

What about rather using $(call ...) instead of ifeq?

> +endif
>  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..d309e26aaec8 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
> +ifeq ($(MSYS_VERSION),0)
> +KBUILD_EFILDFLAGS += -zexecstack -znocombreloc -znorelro
> +endif
>  KBUILD_EFILDFLAGS += $(call ld-option,--no-warn-rwx-segments)
>  quiet_cmd_efi_ld = LD      $@
>  cmd_efi_ld = $(LD) $(KBUILD_EFILDFLAGS) -T $(EFI_LDS_PATH) \
> -- 
> 2.40.0.634.g4ca3ef3211-goog
> 

Re: [PATCH 26/31] Makefile: Correct the ans1_compiler rule for MSYS2

[Pali Rohár]

On Monday 24 April 2023 17:08:31 Simon Glass wrote:
> Add the required extension to the Makefile rule.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> 
>  Makefile               | 1 +
>  scripts/Makefile.build | 2 +-
>  2 files changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/Makefile b/Makefile
> index 773260b81d5d..08eab87d0a11 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -48,6 +48,7 @@ ifeq ($(MSYS_VERSION),0)
>  export SOEXT := so
>  else
>  export SOEXT := dll
> +export ELFEXT := .exe

.EXE is not extension for ELF format :-)

Autotools and also other tools use EXEEXT Makefile variable for this.
What about following this de-factor standard variable name?

>  endif
>  
>  # Avoid funny character set dependencies
> diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> index 97dd4a64f6ef..547dc039ceb4 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$(ELFEXT)
>  	$(call cmd,asn1_compiler)
>  
>  # Build the compiled-in targets
> -- 
> 2.40.0.634.g4ca3ef3211-goog
> 

Re: [PATCH 27/31] Makefile: Disable LTO when building with MSYS2

[Pali Rohár]

On Monday 24 April 2023 17:08:32 Simon Glass wrote:
> 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)

This issue should be rather fixed...

> Drop LTO for now.

... and until it happens is not CONFIG_LTO for disabling enough?

LTO does not work for more other boards / platforms and it is just _not_
enabled via CONFIG_LTO in those cases...

> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> 
>  Makefile | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Makefile b/Makefile
> index 08eab87d0a11..3a35d14a4497 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -658,9 +658,11 @@ 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 ($(MSYS_VERSION),0)
>  ifeq ($(NO_LTO),)
>  LTO_ENABLE=$(if $(CONFIG_LTO),y)
>  endif
> +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
> -- 
> 2.40.0.634.g4ca3ef3211-goog
> 

Re: [PATCH 17/31] sandbox: Allow weak symbols to be dropped

[Pali Rohár]

On Tuesday 25 April 2023 10:44:17 Bin Meng wrote:
> Hi Simon,
> 
> On Tue, Apr 25, 2023 at 7:09 AM Simon Glass <sjg@chromium.org> wrote:
> >
> > 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>
> > ---
> >
> >  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(-)
> >
> 
> You probably need to use:
> 
> __declspec(selectany)
> 
> to replace __weak in the ELF for the *nix world.
> 
> Note this Microsoft bizarre does not provide the "strong override
> weak" effect but I suspect what you only need is to get the build pass
> on Windows so it should be okay.
> 
> Regards,
> Bin

You have selectany and external weaks support in PE format. For the
majority of weak function cases, this is enough. Cannot we use them?

Re: [PATCH 00/31] Allow building sandbox with MSYS2

[Simon Glass]

Hi Tom,

On Tue, 25 Apr 2023 at 10:04, Tom Rini <trini@konsulko.com> wrote:
>
> On Tue, Apr 25, 2023 at 04:59:53AM +0200, Heinrich Schuchardt wrote:
> >
> >
> > Am 25. April 2023 01:08:05 MESZ schrieb Simon Glass <sjg@chromium.org>:
> > >This expands the existing work to allow sandbox to build and run on
> > >Windows using MSYS2.
> >
> > Why do we need this?
> > Wouldn't a developer on Windows be much better served using WSL (https://learn.microsoft.com/en-us/windows/wsl/install)?
>
> Depending on corporate rules that may or may not be allowed. But I too
> am interested in the use case driving these changes. Sandbox is
> essentially a test platform. We test that we can do host tools on
> Windows via MSYS2 for semi-reasonable reasons, since those tools can be
> used in a number of places. We do the same on macOS in Azure as a check
> on building regular targets there too (which should work and I believe
> people do, both from macOS and from *BSD which this is a stand-in for,
> as we can't get free VMs for them today). What's the end goal of this
> series?

It is really just an extension of the existing tools-only build. I am
not sure it is strongly motivated, but the end goal is to be able to
do small features / development on U-Boot on Windows, since some users
seem to only use a 'pure' Windows environment and don't use WSL, which
after all is for cross-compiling. For example, it is possible to build
a FIT and check it using the tools, but it is not possible (without
this series) to see whether U-Boot will load it and behave correctly.

The binman changes are to make that work on Windows (pip install
binary-manager) which is important for that tool.

The 'weak' issue is something that I am hoping can be resolved (in
fact Bin made a promising comment on that patch).

This has all come up as we try to establish a standard 'payload'
format which can be used across the industry. For better or worse,
some people do use Windows for development.

Regards,
Simon

Re: [PATCH 10/31] sandbox: Provide a linker script for MSYS2

[Simon Glass]

Hi Pali,

On Tue, 25 Apr 2023 at 10:21, Pali Rohár <pali@kernel.org> wrote:
>
> On Monday 24 April 2023 17:08:15 Simon Glass wrote:
> > Add a script to allow the U-Boot sandbox executable to be built for
> > Windows. Add a note as to why this seems to be necessary for now.
> >
> > Signed-off-by: Simon Glass <sjg@chromium.org>
> > ---
> >
> >  Makefile                       |  11 +-
> >  arch/sandbox/config.mk         |   4 +
> >  arch/sandbox/cpu/u-boot-pe.lds | 447 +++++++++++++++++++++++++++++++++
> >  3 files changed, 460 insertions(+), 2 deletions(-)
> >  create mode 100644 arch/sandbox/cpu/u-boot-pe.lds
> >
> > diff --git a/Makefile b/Makefile
> > index dd3fcd1782e5..0aa97a2c3b48 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -1730,6 +1730,13 @@ 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
> > +$(warning msys)
> > +endif
> > +
> >  # Rule to link u-boot
> >  # May be overridden by arch/$(ARCH)/config.mk
> >  ifeq ($(LTO_ENABLE),y)
> > @@ -1738,7 +1745,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)                                 \
> > @@ -1749,7 +1756,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                                              \
> > diff --git a/arch/sandbox/config.mk b/arch/sandbox/config.mk
> > index 2d184c5f652a..e05daf57ef8e 100644
> > --- a/arch/sandbox/config.mk
> > +++ b/arch/sandbox/config.mk
> > @@ -71,3 +71,7 @@ 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 = $(srctree)/arch/sandbox/cpu/u-boot-pe.lds
> > +endif
> > diff --git a/arch/sandbox/cpu/u-boot-pe.lds b/arch/sandbox/cpu/u-boot-pe.lds
> > new file mode 100644
> > index 000000000000..031e70fafd03
> > --- /dev/null
> > +++ b/arch/sandbox/cpu/u-boot-pe.lds
> > @@ -0,0 +1,447 @@
> > +/* SPDX-License-Identifier: GPL-2.0+ */
> > +/*
> > + * U-Boot note: This was obtained by using the -verbose linker option. The
> > + * U-Boot additions are marked below.
> > + *
> > + * Ideally we would add sections to the executable, as is done with the Linux
> > + * build. But PE executables do not appear to work correctly if unexpected
> > + * sections are present:
> > + *
> > + *   $ /tmp/b/sandbox/u-boot.exe
> > + *   -bash: /tmp/b/sandbox/u-boot.exe: cannot execute binary file: Exec format error
> > + *
> > + * So we take a approach of rewriting the whole file, for now. This will likely
> > + * break in the future when a toolchain change is made.
>
> Why not rather provide "layer" linker script which does this "rewriting"
> on top of the default linker script? With this way it is not needed to
> update linker script when a toolchain change it.
>

How can we reliably do that, though? We don't really know the format
of the script and where to insert stuff.

Regards,
Simon

Re: [PATCH 10/31] sandbox: Provide a linker script for MSYS2

[Pali Rohár]

On Tuesday 25 April 2023 12:01:04 Simon Glass wrote:
> Hi Pali,
> 
> On Tue, 25 Apr 2023 at 10:21, Pali Rohár <pali@kernel.org> wrote:
> >
> > On Monday 24 April 2023 17:08:15 Simon Glass wrote:
> > > Add a script to allow the U-Boot sandbox executable to be built for
> > > Windows. Add a note as to why this seems to be necessary for now.
> > >
> > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > > ---
> > >
> > >  Makefile                       |  11 +-
> > >  arch/sandbox/config.mk         |   4 +
> > >  arch/sandbox/cpu/u-boot-pe.lds | 447 +++++++++++++++++++++++++++++++++
> > >  3 files changed, 460 insertions(+), 2 deletions(-)
> > >  create mode 100644 arch/sandbox/cpu/u-boot-pe.lds
> > >
> > > diff --git a/Makefile b/Makefile
> > > index dd3fcd1782e5..0aa97a2c3b48 100644
> > > --- a/Makefile
> > > +++ b/Makefile
> > > @@ -1730,6 +1730,13 @@ 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
> > > +$(warning msys)
> > > +endif
> > > +
> > >  # Rule to link u-boot
> > >  # May be overridden by arch/$(ARCH)/config.mk
> > >  ifeq ($(LTO_ENABLE),y)
> > > @@ -1738,7 +1745,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)                                 \
> > > @@ -1749,7 +1756,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                                              \
> > > diff --git a/arch/sandbox/config.mk b/arch/sandbox/config.mk
> > > index 2d184c5f652a..e05daf57ef8e 100644
> > > --- a/arch/sandbox/config.mk
> > > +++ b/arch/sandbox/config.mk
> > > @@ -71,3 +71,7 @@ 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 = $(srctree)/arch/sandbox/cpu/u-boot-pe.lds
> > > +endif
> > > diff --git a/arch/sandbox/cpu/u-boot-pe.lds b/arch/sandbox/cpu/u-boot-pe.lds
> > > new file mode 100644
> > > index 000000000000..031e70fafd03
> > > --- /dev/null
> > > +++ b/arch/sandbox/cpu/u-boot-pe.lds
> > > @@ -0,0 +1,447 @@
> > > +/* SPDX-License-Identifier: GPL-2.0+ */
> > > +/*
> > > + * U-Boot note: This was obtained by using the -verbose linker option. The
> > > + * U-Boot additions are marked below.
> > > + *
> > > + * Ideally we would add sections to the executable, as is done with the Linux
> > > + * build. But PE executables do not appear to work correctly if unexpected
> > > + * sections are present:
> > > + *
> > > + *   $ /tmp/b/sandbox/u-boot.exe
> > > + *   -bash: /tmp/b/sandbox/u-boot.exe: cannot execute binary file: Exec format error
> > > + *
> > > + * So we take a approach of rewriting the whole file, for now. This will likely
> > > + * break in the future when a toolchain change is made.
> >
> > Why not rather provide "layer" linker script which does this "rewriting"
> > on top of the default linker script? With this way it is not needed to
> > update linker script when a toolchain change it.
> >
> 
> How can we reliably do that, though? We don't really know the format
> of the script and where to insert stuff.
> 
> Regards,
> Simon

Well, I do not know what is the current issue. The proposed script in
your patch looks like it is copied from some binutils version and then
modified. Also I do not know from which binutils you have copied and
what modification you have done in it. So I cannot react or comment
anymore more here.

My idea is that to write those modifications into new layer script.

Re: [PATCH 10/31] sandbox: Provide a linker script for MSYS2

[Simon Glass]

Hi Pali,

On Tue, 25 Apr 2023 at 12:11, Pali Rohár <pali@kernel.org> wrote:
>
> On Tuesday 25 April 2023 12:01:04 Simon Glass wrote:
> > Hi Pali,
> >
> > On Tue, 25 Apr 2023 at 10:21, Pali Rohár <pali@kernel.org> wrote:
> > >
> > > On Monday 24 April 2023 17:08:15 Simon Glass wrote:
> > > > Add a script to allow the U-Boot sandbox executable to be built for
> > > > Windows. Add a note as to why this seems to be necessary for now.
> > > >
> > > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > > > ---
> > > >
> > > >  Makefile                       |  11 +-
> > > >  arch/sandbox/config.mk         |   4 +
> > > >  arch/sandbox/cpu/u-boot-pe.lds | 447 +++++++++++++++++++++++++++++++++
> > > >  3 files changed, 460 insertions(+), 2 deletions(-)
> > > >  create mode 100644 arch/sandbox/cpu/u-boot-pe.lds
> > > >
> > > > diff --git a/Makefile b/Makefile
> > > > index dd3fcd1782e5..0aa97a2c3b48 100644
> > > > --- a/Makefile
> > > > +++ b/Makefile
> > > > @@ -1730,6 +1730,13 @@ 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
> > > > +$(warning msys)
> > > > +endif
> > > > +
> > > >  # Rule to link u-boot
> > > >  # May be overridden by arch/$(ARCH)/config.mk
> > > >  ifeq ($(LTO_ENABLE),y)
> > > > @@ -1738,7 +1745,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)                                 \
> > > > @@ -1749,7 +1756,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                                              \
> > > > diff --git a/arch/sandbox/config.mk b/arch/sandbox/config.mk
> > > > index 2d184c5f652a..e05daf57ef8e 100644
> > > > --- a/arch/sandbox/config.mk
> > > > +++ b/arch/sandbox/config.mk
> > > > @@ -71,3 +71,7 @@ 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 = $(srctree)/arch/sandbox/cpu/u-boot-pe.lds
> > > > +endif
> > > > diff --git a/arch/sandbox/cpu/u-boot-pe.lds b/arch/sandbox/cpu/u-boot-pe.lds
> > > > new file mode 100644
> > > > index 000000000000..031e70fafd03
> > > > --- /dev/null
> > > > +++ b/arch/sandbox/cpu/u-boot-pe.lds
> > > > @@ -0,0 +1,447 @@
> > > > +/* SPDX-License-Identifier: GPL-2.0+ */
> > > > +/*
> > > > + * U-Boot note: This was obtained by using the -verbose linker option. The
> > > > + * U-Boot additions are marked below.
> > > > + *
> > > > + * Ideally we would add sections to the executable, as is done with the Linux
> > > > + * build. But PE executables do not appear to work correctly if unexpected
> > > > + * sections are present:
> > > > + *
> > > > + *   $ /tmp/b/sandbox/u-boot.exe
> > > > + *   -bash: /tmp/b/sandbox/u-boot.exe: cannot execute binary file: Exec format error
> > > > + *
> > > > + * So we take a approach of rewriting the whole file, for now. This will likely
> > > > + * break in the future when a toolchain change is made.
> > >
> > > Why not rather provide "layer" linker script which does this "rewriting"
> > > on top of the default linker script? With this way it is not needed to
> > > update linker script when a toolchain change it.
> > >
> >
> > How can we reliably do that, though? We don't really know the format
> > of the script and where to insert stuff.
> >
> > Regards,
> > Simon
>
> Well, I do not know what is the current issue. The proposed script in

See the comment at the top of the script:

+ * Ideally we would add sections to the executable, as is done with the Linux
+ * build. But PE executables do not appear to work correctly if unexpected
+ * sections are present:
+ *
+ *   $ /tmp/b/sandbox/u-boot.exe
+ *   -bash: /tmp/b/sandbox/u-boot.exe: cannot execute binary file:
Exec format error
+ *
+ * So we take a approach of rewriting the whole file, for now. This will likely
+ * break in the future when a toolchain change is made.

> your patch looks like it is copied from some binutils version and then
> modified.

Yes, exactly.

> Also I do not know from which binutils you have copied and
> what modification you have done in it.

But I have marked this clearly in the script. See  /* U-Boot additions
from here on */

> So I cannot react or comment
> anymore more here.
>
> My idea is that to write those modifications into new layer script.

So I think you are suggesting that I get binutils to emit the default
script, then have a script which detects the end of the .rdata section
and emits its own stuff in there? That may be more robust, since it is
unlikely that the .rdata section will be removed.

But I was rather hoping that someone could help with the core problem,
i.e. why I cannot just insert another section into the image, like we
do with ELF?

Regards,
Simon

Re: [PATCH 10/31] sandbox: Provide a linker script for MSYS2

[Pali Rohár]

On Tuesday 25 April 2023 13:23:04 Simon Glass wrote:
> Hi Pali,
> 
> On Tue, 25 Apr 2023 at 12:11, Pali Rohár <pali@kernel.org> wrote:
> >
> > On Tuesday 25 April 2023 12:01:04 Simon Glass wrote:
> > > Hi Pali,
> > >
> > > On Tue, 25 Apr 2023 at 10:21, Pali Rohár <pali@kernel.org> wrote:
> > > >
> > > > On Monday 24 April 2023 17:08:15 Simon Glass wrote:
> > > > > Add a script to allow the U-Boot sandbox executable to be built for
> > > > > Windows. Add a note as to why this seems to be necessary for now.
> > > > >
> > > > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > > > > ---
> > > > >
> > > > >  Makefile                       |  11 +-
> > > > >  arch/sandbox/config.mk         |   4 +
> > > > >  arch/sandbox/cpu/u-boot-pe.lds | 447 +++++++++++++++++++++++++++++++++
> > > > >  3 files changed, 460 insertions(+), 2 deletions(-)
> > > > >  create mode 100644 arch/sandbox/cpu/u-boot-pe.lds
> > > > >
> > > > > diff --git a/Makefile b/Makefile
> > > > > index dd3fcd1782e5..0aa97a2c3b48 100644
> > > > > --- a/Makefile
> > > > > +++ b/Makefile
> > > > > @@ -1730,6 +1730,13 @@ 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
> > > > > +$(warning msys)
> > > > > +endif
> > > > > +
> > > > >  # Rule to link u-boot
> > > > >  # May be overridden by arch/$(ARCH)/config.mk
> > > > >  ifeq ($(LTO_ENABLE),y)
> > > > > @@ -1738,7 +1745,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)                                 \
> > > > > @@ -1749,7 +1756,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                                              \
> > > > > diff --git a/arch/sandbox/config.mk b/arch/sandbox/config.mk
> > > > > index 2d184c5f652a..e05daf57ef8e 100644
> > > > > --- a/arch/sandbox/config.mk
> > > > > +++ b/arch/sandbox/config.mk
> > > > > @@ -71,3 +71,7 @@ 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 = $(srctree)/arch/sandbox/cpu/u-boot-pe.lds
> > > > > +endif
> > > > > diff --git a/arch/sandbox/cpu/u-boot-pe.lds b/arch/sandbox/cpu/u-boot-pe.lds
> > > > > new file mode 100644
> > > > > index 000000000000..031e70fafd03
> > > > > --- /dev/null
> > > > > +++ b/arch/sandbox/cpu/u-boot-pe.lds
> > > > > @@ -0,0 +1,447 @@
> > > > > +/* SPDX-License-Identifier: GPL-2.0+ */
> > > > > +/*
> > > > > + * U-Boot note: This was obtained by using the -verbose linker option. The
> > > > > + * U-Boot additions are marked below.
> > > > > + *
> > > > > + * Ideally we would add sections to the executable, as is done with the Linux
> > > > > + * build. But PE executables do not appear to work correctly if unexpected
> > > > > + * sections are present:
> > > > > + *
> > > > > + *   $ /tmp/b/sandbox/u-boot.exe
> > > > > + *   -bash: /tmp/b/sandbox/u-boot.exe: cannot execute binary file: Exec format error
> > > > > + *
> > > > > + * So we take a approach of rewriting the whole file, for now. This will likely
> > > > > + * break in the future when a toolchain change is made.
> > > >
> > > > Why not rather provide "layer" linker script which does this "rewriting"
> > > > on top of the default linker script? With this way it is not needed to
> > > > update linker script when a toolchain change it.
> > > >
> > >
> > > How can we reliably do that, though? We don't really know the format
> > > of the script and where to insert stuff.
> > >
> > > Regards,
> > > Simon
> >
> > Well, I do not know what is the current issue. The proposed script in
> 
> See the comment at the top of the script:
> 
> + * Ideally we would add sections to the executable, as is done with the Linux
> + * build. But PE executables do not appear to work correctly if unexpected
> + * sections are present:
> + *
> + *   $ /tmp/b/sandbox/u-boot.exe
> + *   -bash: /tmp/b/sandbox/u-boot.exe: cannot execute binary file:
> Exec format error
> + *
> + * So we take a approach of rewriting the whole file, for now. This will likely
> + * break in the future when a toolchain change is made.
> 
> > your patch looks like it is copied from some binutils version and then
> > modified.
> 
> Yes, exactly.
> 
> > Also I do not know from which binutils you have copied and
> > what modification you have done in it.
> 
> But I have marked this clearly in the script. See  /* U-Boot additions
> from here on */

I read this, but I have not found from which binutils you took it...
There are many released binutils versions and also each binutils
version has more PE linker scripts. So it is really hard to track from
which it comes. -verbose argument show you the final linker script but
we need to know also from which files it was composed...

> > So I cannot react or comment
> > anymore more here.
> >
> > My idea is that to write those modifications into new layer script.
> 
> So I think you are suggesting that I get binutils to emit the default
> script, then have a script which detects the end of the .rdata section
> and emits its own stuff in there? That may be more robust, since it is
> unlikely that the .rdata section will be removed.

Yes, exactly. And you do not need to take and copy default script from
binutils to u-boot sources. If you write "layer" linker script (I hope
that this is how it is called in LD documentation), then LD
automatically uses its own default script and apply your "layer" script
on it. So with this way you can completely avoid copy+paste files from
binutils to u-boot repository.

> But I was rather hoping that someone could help with the core problem,
> i.e. why I cannot just insert another section into the image, like we
> do with ELF?

That is interesting. Theoretically it should work but I have not seen it
widely used outside of NT kernel modules. So maybe there is some bug in
Windows loader for userspace binaries, or another bug in GNU LD, or just
a bug in u-boot / linker script.

Could you show linker script which is causing generation of that buggy
non-working binary? And ideally can you provide also that buggy EXE
binary? I could try to inspect it, but I do not have time to setup MSYS2
environment to compile my own buggy EXE binary.

> 
> Regards,
> Simon

Re: [PATCH 10/31] sandbox: Provide a linker script for MSYS2

[Simon Glass]

Hi Pali,

On Tue, 25 Apr 2023 at 13:33, Pali Rohár <pali@kernel.org> wrote:
>
> On Tuesday 25 April 2023 13:23:04 Simon Glass wrote:
> > Hi Pali,
> >
> > On Tue, 25 Apr 2023 at 12:11, Pali Rohár <pali@kernel.org> wrote:
> > >
> > > On Tuesday 25 April 2023 12:01:04 Simon Glass wrote:
> > > > Hi Pali,
> > > >
> > > > On Tue, 25 Apr 2023 at 10:21, Pali Rohár <pali@kernel.org> wrote:
> > > > >
> > > > > On Monday 24 April 2023 17:08:15 Simon Glass wrote:
> > > > > > Add a script to allow the U-Boot sandbox executable to be built for
> > > > > > Windows. Add a note as to why this seems to be necessary for now.
> > > > > >
> > > > > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > > > > > ---
> > > > > >
> > > > > >  Makefile                       |  11 +-
> > > > > >  arch/sandbox/config.mk         |   4 +
> > > > > >  arch/sandbox/cpu/u-boot-pe.lds | 447 +++++++++++++++++++++++++++++++++
> > > > > >  3 files changed, 460 insertions(+), 2 deletions(-)
> > > > > >  create mode 100644 arch/sandbox/cpu/u-boot-pe.lds
> > > > > >
> > > > > > diff --git a/Makefile b/Makefile
> > > > > > index dd3fcd1782e5..0aa97a2c3b48 100644
> > > > > > --- a/Makefile
> > > > > > +++ b/Makefile
> > > > > > @@ -1730,6 +1730,13 @@ 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
> > > > > > +$(warning msys)
> > > > > > +endif
> > > > > > +
> > > > > >  # Rule to link u-boot
> > > > > >  # May be overridden by arch/$(ARCH)/config.mk
> > > > > >  ifeq ($(LTO_ENABLE),y)
> > > > > > @@ -1738,7 +1745,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)                                 \
> > > > > > @@ -1749,7 +1756,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                                              \
> > > > > > diff --git a/arch/sandbox/config.mk b/arch/sandbox/config.mk
> > > > > > index 2d184c5f652a..e05daf57ef8e 100644
> > > > > > --- a/arch/sandbox/config.mk
> > > > > > +++ b/arch/sandbox/config.mk
> > > > > > @@ -71,3 +71,7 @@ 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 = $(srctree)/arch/sandbox/cpu/u-boot-pe.lds
> > > > > > +endif
> > > > > > diff --git a/arch/sandbox/cpu/u-boot-pe.lds b/arch/sandbox/cpu/u-boot-pe.lds
> > > > > > new file mode 100644
> > > > > > index 000000000000..031e70fafd03
> > > > > > --- /dev/null
> > > > > > +++ b/arch/sandbox/cpu/u-boot-pe.lds
> > > > > > @@ -0,0 +1,447 @@
> > > > > > +/* SPDX-License-Identifier: GPL-2.0+ */
> > > > > > +/*
> > > > > > + * U-Boot note: This was obtained by using the -verbose linker option. The
> > > > > > + * U-Boot additions are marked below.
> > > > > > + *
> > > > > > + * Ideally we would add sections to the executable, as is done with the Linux
> > > > > > + * build. But PE executables do not appear to work correctly if unexpected
> > > > > > + * sections are present:
> > > > > > + *
> > > > > > + *   $ /tmp/b/sandbox/u-boot.exe
> > > > > > + *   -bash: /tmp/b/sandbox/u-boot.exe: cannot execute binary file: Exec format error
> > > > > > + *
> > > > > > + * So we take a approach of rewriting the whole file, for now. This will likely
> > > > > > + * break in the future when a toolchain change is made.
> > > > >
> > > > > Why not rather provide "layer" linker script which does this "rewriting"
> > > > > on top of the default linker script? With this way it is not needed to
> > > > > update linker script when a toolchain change it.
> > > > >
> > > >
> > > > How can we reliably do that, though? We don't really know the format
> > > > of the script and where to insert stuff.
> > > >
> > > > Regards,
> > > > Simon
> > >
> > > Well, I do not know what is the current issue. The proposed script in
> >
> > See the comment at the top of the script:
> >
> > + * Ideally we would add sections to the executable, as is done with the Linux
> > + * build. But PE executables do not appear to work correctly if unexpected
> > + * sections are present:
> > + *
> > + *   $ /tmp/b/sandbox/u-boot.exe
> > + *   -bash: /tmp/b/sandbox/u-boot.exe: cannot execute binary file:
> > Exec format error
> > + *
> > + * So we take a approach of rewriting the whole file, for now. This will likely
> > + * break in the future when a toolchain change is made.
> >
> > > your patch looks like it is copied from some binutils version and then
> > > modified.
> >
> > Yes, exactly.
> >
> > > Also I do not know from which binutils you have copied and
> > > what modification you have done in it.
> >
> > But I have marked this clearly in the script. See  /* U-Boot additions
> > from here on */
>
> I read this, but I have not found from which binutils you took it...
> There are many released binutils versions and also each binutils
> version has more PE linker scripts. So it is really hard to track from
> which it comes. -verbose argument show you the final linker script but
> we need to know also from which files it was composed...

$ ld -V
GNU ld (GNU Binutils) 2.40
  Supported emulations:
   i386pep
   i386pe

sglass@DESKTOP-OHNGJ4K MINGW64 ~/u-boot
$ cc -v
Using built-in specs.
COLLECT_GCC=cc
COLLECT_LTO_WRAPPER=/usr/lib/gcc/x86_64-pc-msys/11.3.0/lto-wrapper.exe
Target: x86_64-pc-msys
Configured with: /c/S/gcc/src/gcc-11.3.0/configure
--build=x86_64-pc-msys --prefix=/usr --libexecdir=/usr/lib
--enable-bootstrap --enable-shared --enable-shared-libgcc
--enable-static --enable-version-specific-runtime-libs
--with-arch=x86-64 --with-tune=generic --disable-multilib
--enable-__cxa_atexit --with-dwarf2
--enable-languages=c,c++,fortran,lto --enable-graphite
--enable-threads=posix --enable-libatomic --enable-libgomp
--disable-libitm --enable-libquadmath --enable-libquadmath-support
--disable-libssp --disable-win32-registry --disable-symvers
--with-gnu-ld --with-gnu-as --disable-isl-version-check
--enable-checking=release --without-libiconv-prefix
--without-libintl-prefix --with-system-zlib --enable-linker-build-id
--with-default-libstdcxx-abi=gcc4-compatible
--enable-libstdcxx-filesystem-ts
Thread model: posix
Supported LTO compression algorithms: zlib


>
> > > So I cannot react or comment
> > > anymore more here.
> > >
> > > My idea is that to write those modifications into new layer script.
> >
> > So I think you are suggesting that I get binutils to emit the default
> > script, then have a script which detects the end of the .rdata section
> > and emits its own stuff in there? That may be more robust, since it is
> > unlikely that the .rdata section will be removed.
>
> Yes, exactly. And you do not need to take and copy default script from
> binutils to u-boot sources. If you write "layer" linker script (I hope
> that this is how it is called in LD documentation), then LD
> automatically uses its own default script and apply your "layer" script
> on it. So with this way you can completely avoid copy+paste files from
> binutils to u-boot repository.

That is considerably more complicated, though.

>
> > But I was rather hoping that someone could help with the core problem,
> > i.e. why I cannot just insert another section into the image, like we
> > do with ELF?
>
> That is interesting. Theoretically it should work but I have not seen it
> widely used outside of NT kernel modules. So maybe there is some bug in
> Windows loader for userspace binaries, or another bug in GNU LD, or just
> a bug in u-boot / linker script.
>
> Could you show linker script which is causing generation of that buggy
> non-working binary? And ideally can you provide also that buggy EXE
> binary? I could try to inspect it, but I do not have time to setup MSYS2
> environment to compile my own buggy EXE binary.

Please see [1] for the file.

sglass@DESKTOP-OHNGJ4K MINGW64 ~/u-boot
$ objdump.exe -h /tmp/b/sandbox/u-boot.exe

/tmp/b/sandbox/u-boot.exe:     file format pei-x86-64

Sections:
Idx Name          Size      VMA               LMA               File off  Algn
  0 .text         00208b98  0000000100401000  0000000100401000  00000600  2**4
                  CONTENTS, ALLOC, LOAD, READONLY, CODE, DATA
  1 __u_boot_list 00014960  0000000100609ba0  0000000100609ba0  00209200  2**2
                  CONTENTS, ALLOC, LOAD, DATA
  2 _u_boot_sandbox_getopt 000000c0  000000010061e500
000000010061e500  0021dc00  2**2
                  CONTENTS, ALLOC, LOAD, DATA
  3 .data         00019a20  000000010061f000  000000010061f000  0021de00  2**4
                  CONTENTS, ALLOC, LOAD, DATA
  4 .rodata.ttf.init 00036e10  0000000100639000  0000000100639000
00237a00  2**2
                  CONTENTS, ALLOC, LOAD, DATA
  5 .rodata.splash.init 00001b20  0000000100670000  0000000100670000
0026ea00  2**2
                  CONTENTS, ALLOC, LOAD, DATA
  6 .rodata.helloworld.init 00003030  0000000100672000
0000000100672000  00270600  2**2
                  CONTENTS, ALLOC, LOAD, DATA
  7 .data.efi_runtime 00000170  0000000100676000  0000000100676000
00273800  2**4
                  CONTENTS, ALLOC, LOAD, DATA
  8 .dtb.init.rodata 000007c0  0000000100677000  0000000100677000
00273a00  2**2
                  CONTENTS, ALLOC, LOAD, DATA
  9 .rdata        000a3fe0  0000000100678000  0000000100678000  00274200  2**4
                  CONTENTS, ALLOC, LOAD, READONLY, DATA
 10 .buildid      00000035  000000010071c000  000000010071c000  00318200  2**2
                  CONTENTS, ALLOC, LOAD, READONLY, DATA
 11 .rodata.efi_runtime 00000400  000000010071d000  000000010071d000
00318400  2**2
                  CONTENTS, ALLOC, LOAD, READONLY, DATA
 12 .pdata        0001785c  000000010071e000  000000010071e000  00318800  2**2
                  CONTENTS, ALLOC, LOAD, READONLY, DATA
 13 .xdata        00016f58  0000000100736000  0000000100736000  00330200  2**2
                  CONTENTS, ALLOC, LOAD, READONLY, DATA
 14 .bss          0001ecc0  000000010074d000  000000010074d000  00000000  2**4
                  ALLOC
 15 .idata        00000894  000000010076c000  000000010076c000  00347200  2**2
                  CONTENTS, ALLOC, LOAD, DATA
 16 .rsrc         000004e8  000000010076d000  000000010076d000  00347c00  2**2
                  CONTENTS, ALLOC, LOAD, DATA
 17 .reloc        00005284  000000010076e000  000000010076e000  00348200  2**2
                  CONTENTS, ALLOC, LOAD, READONLY, DATA
 18 .debug_aranges 0000b720  0000000100774000  0000000100774000  0034d600  2**0
                  CONTENTS, READONLY, DEBUGGING
 19 .debug_info   00a6c954  0000000100780000  0000000100780000  00358e00  2**0
                  CONTENTS, READONLY, DEBUGGING
 20 .debug_abbrev 000cb6b2  00000001011ed000  00000001011ed000  00dc5800  2**0
                  CONTENTS, READONLY, DEBUGGING
 21 .debug_line   001f646a  00000001012b9000  00000001012b9000  00e91000  2**0
                  CONTENTS, READONLY, DEBUGGING
 22 .debug_frame  000a9bd0  00000001014b0000  00000001014b0000  01087600  2**0
                  CONTENTS, READONLY, DEBUGGING
 23 .debug_str    0001d6c1  000000010155a000  000000010155a000  01131200  2**0
                  CONTENTS, READONLY, DEBUGGING
 24 .debug_loc    004673a1  0000000101578000  0000000101578000  0114ea00  2**0
                  CONTENTS, READONLY, DEBUGGING
 25 .debug_ranges 00060260  00000001019e0000  00000001019e0000  015b5e00  2**0
                  CONTENTS, READONLY, DEBUGGING
 26 .debug_line_str 00000264  0000000101a41000  0000000101a41000  01616200  2**0
                  CONTENTS, READONLY, DEBUGGING

sglass@DESKTOP-OHNGJ4K MINGW64 ~/u-boot
$ !$
/tmp/b/sandbox/u-boot.exe
-bash: /tmp/b/sandbox/u-boot.exe: cannot execute binary file: Exec format error


Regards,
Simon

[1] https://drive.google.com/file/d/1vtJlvCd1BxkhRtQtUe4YqiRizcUP_zB2/view?usp=share_link

Re: [PATCH 27/31] Makefile: Disable LTO when building with MSYS2

[Simon Glass]

Hi Pali,

On Tue, 25 Apr 2023 at 10:28, Pali Rohár <pali@kernel.org> wrote:
>
> On Monday 24 April 2023 17:08:32 Simon Glass wrote:
> > 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)
>
> This issue should be rather fixed...
>
> > Drop LTO for now.
>
> ... and until it happens is not CONFIG_LTO for disabling enough?
>
> LTO does not work for more other boards / platforms and it is just _not_
> enabled via CONFIG_LTO in those cases...

The thing is, LTO is enabled for sandbox normally (clang and gcc). It
is just the MSYS2 platform where there are problems.

I am not sure how to fix it, since I don't know what discarded section
it is referring to.

Regards,
Simon

Re: [PATCH 27/31] Makefile: Disable LTO when building with MSYS2

[Pali Rohár]

On Tuesday 25 April 2023 19:04:10 Simon Glass wrote:
> Hi Pali,
> 
> On Tue, 25 Apr 2023 at 10:28, Pali Rohár <pali@kernel.org> wrote:
> >
> > On Monday 24 April 2023 17:08:32 Simon Glass wrote:
> > > 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)
> >
> > This issue should be rather fixed...
> >
> > > Drop LTO for now.
> >
> > ... and until it happens is not CONFIG_LTO for disabling enough?
> >
> > LTO does not work for more other boards / platforms and it is just _not_
> > enabled via CONFIG_LTO in those cases...
> 
> The thing is, LTO is enabled for sandbox normally (clang and gcc). It
> is just the MSYS2 platform where there are problems.

So what about having CONFIG_LTO by default 'n' for CONFIG_MSYS2?

> I am not sure how to fix it, since I don't know what discarded section
> it is referring to.
> 
> Regards,
> Simon

Re: [PATCH 10/31] sandbox: Provide a linker script for MSYS2

[Pali Rohár]

On Tuesday 25 April 2023 18:50:59 Simon Glass wrote:
> Hi Pali,
> 
> On Tue, 25 Apr 2023 at 13:33, Pali Rohár <pali@kernel.org> wrote:
> >
> > On Tuesday 25 April 2023 13:23:04 Simon Glass wrote:
> > > Hi Pali,
> > >
> > > On Tue, 25 Apr 2023 at 12:11, Pali Rohár <pali@kernel.org> wrote:
> > > >
> > > > On Tuesday 25 April 2023 12:01:04 Simon Glass wrote:
> > > > > Hi Pali,
> > > > >
> > > > > On Tue, 25 Apr 2023 at 10:21, Pali Rohár <pali@kernel.org> wrote:
> > > > > >
> > > > > > On Monday 24 April 2023 17:08:15 Simon Glass wrote:
> > > > > > > Add a script to allow the U-Boot sandbox executable to be built for
> > > > > > > Windows. Add a note as to why this seems to be necessary for now.
> > > > > > >
> > > > > > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > > > > > > ---
> > > > > > >
> > > > > > >  Makefile                       |  11 +-
> > > > > > >  arch/sandbox/config.mk         |   4 +
> > > > > > >  arch/sandbox/cpu/u-boot-pe.lds | 447 +++++++++++++++++++++++++++++++++
> > > > > > >  3 files changed, 460 insertions(+), 2 deletions(-)
> > > > > > >  create mode 100644 arch/sandbox/cpu/u-boot-pe.lds
> > > > > > >
> > > > > > > diff --git a/Makefile b/Makefile
> > > > > > > index dd3fcd1782e5..0aa97a2c3b48 100644
> > > > > > > --- a/Makefile
> > > > > > > +++ b/Makefile
> > > > > > > @@ -1730,6 +1730,13 @@ 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
> > > > > > > +$(warning msys)
> > > > > > > +endif
> > > > > > > +
> > > > > > >  # Rule to link u-boot
> > > > > > >  # May be overridden by arch/$(ARCH)/config.mk
> > > > > > >  ifeq ($(LTO_ENABLE),y)
> > > > > > > @@ -1738,7 +1745,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)                                 \
> > > > > > > @@ -1749,7 +1756,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                                              \
> > > > > > > diff --git a/arch/sandbox/config.mk b/arch/sandbox/config.mk
> > > > > > > index 2d184c5f652a..e05daf57ef8e 100644
> > > > > > > --- a/arch/sandbox/config.mk
> > > > > > > +++ b/arch/sandbox/config.mk
> > > > > > > @@ -71,3 +71,7 @@ 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 = $(srctree)/arch/sandbox/cpu/u-boot-pe.lds
> > > > > > > +endif
> > > > > > > diff --git a/arch/sandbox/cpu/u-boot-pe.lds b/arch/sandbox/cpu/u-boot-pe.lds
> > > > > > > new file mode 100644
> > > > > > > index 000000000000..031e70fafd03
> > > > > > > --- /dev/null
> > > > > > > +++ b/arch/sandbox/cpu/u-boot-pe.lds
> > > > > > > @@ -0,0 +1,447 @@
> > > > > > > +/* SPDX-License-Identifier: GPL-2.0+ */
> > > > > > > +/*
> > > > > > > + * U-Boot note: This was obtained by using the -verbose linker option. The
> > > > > > > + * U-Boot additions are marked below.
> > > > > > > + *
> > > > > > > + * Ideally we would add sections to the executable, as is done with the Linux
> > > > > > > + * build. But PE executables do not appear to work correctly if unexpected
> > > > > > > + * sections are present:
> > > > > > > + *
> > > > > > > + *   $ /tmp/b/sandbox/u-boot.exe
> > > > > > > + *   -bash: /tmp/b/sandbox/u-boot.exe: cannot execute binary file: Exec format error
> > > > > > > + *
> > > > > > > + * So we take a approach of rewriting the whole file, for now. This will likely
> > > > > > > + * break in the future when a toolchain change is made.
> > > > > >
> > > > > > Why not rather provide "layer" linker script which does this "rewriting"
> > > > > > on top of the default linker script? With this way it is not needed to
> > > > > > update linker script when a toolchain change it.
> > > > > >
> > > > >
> > > > > How can we reliably do that, though? We don't really know the format
> > > > > of the script and where to insert stuff.
> > > > >
> > > > > Regards,
> > > > > Simon
> > > >
> > > > Well, I do not know what is the current issue. The proposed script in
> > >
> > > See the comment at the top of the script:
> > >
> > > + * Ideally we would add sections to the executable, as is done with the Linux
> > > + * build. But PE executables do not appear to work correctly if unexpected
> > > + * sections are present:
> > > + *
> > > + *   $ /tmp/b/sandbox/u-boot.exe
> > > + *   -bash: /tmp/b/sandbox/u-boot.exe: cannot execute binary file:
> > > Exec format error
> > > + *
> > > + * So we take a approach of rewriting the whole file, for now. This will likely
> > > + * break in the future when a toolchain change is made.
> > >
> > > > your patch looks like it is copied from some binutils version and then
> > > > modified.
> > >
> > > Yes, exactly.
> > >
> > > > Also I do not know from which binutils you have copied and
> > > > what modification you have done in it.
> > >
> > > But I have marked this clearly in the script. See  /* U-Boot additions
> > > from here on */
> >
> > I read this, but I have not found from which binutils you took it...
> > There are many released binutils versions and also each binutils
> > version has more PE linker scripts. So it is really hard to track from
> > which it comes. -verbose argument show you the final linker script but
> > we need to know also from which files it was composed...
> 
> $ ld -V
> GNU ld (GNU Binutils) 2.40
>   Supported emulations:
>    i386pep
>    i386pe
> 
> sglass@DESKTOP-OHNGJ4K MINGW64 ~/u-boot
> $ cc -v
> Using built-in specs.
> COLLECT_GCC=cc
> COLLECT_LTO_WRAPPER=/usr/lib/gcc/x86_64-pc-msys/11.3.0/lto-wrapper.exe
> Target: x86_64-pc-msys
> Configured with: /c/S/gcc/src/gcc-11.3.0/configure
> --build=x86_64-pc-msys --prefix=/usr --libexecdir=/usr/lib
> --enable-bootstrap --enable-shared --enable-shared-libgcc
> --enable-static --enable-version-specific-runtime-libs
> --with-arch=x86-64 --with-tune=generic --disable-multilib
> --enable-__cxa_atexit --with-dwarf2
> --enable-languages=c,c++,fortran,lto --enable-graphite
> --enable-threads=posix --enable-libatomic --enable-libgomp
> --disable-libitm --enable-libquadmath --enable-libquadmath-support
> --disable-libssp --disable-win32-registry --disable-symvers
> --with-gnu-ld --with-gnu-as --disable-isl-version-check
> --enable-checking=release --without-libiconv-prefix
> --without-libintl-prefix --with-system-zlib --enable-linker-build-id
> --with-default-libstdcxx-abi=gcc4-compatible
> --enable-libstdcxx-filesystem-ts
> Thread model: posix
> Supported LTO compression algorithms: zlib
> 
> 
> >
> > > > So I cannot react or comment
> > > > anymore more here.
> > > >
> > > > My idea is that to write those modifications into new layer script.
> > >
> > > So I think you are suggesting that I get binutils to emit the default
> > > script, then have a script which detects the end of the .rdata section
> > > and emits its own stuff in there? That may be more robust, since it is
> > > unlikely that the .rdata section will be removed.
> >
> > Yes, exactly. And you do not need to take and copy default script from
> > binutils to u-boot sources. If you write "layer" linker script (I hope
> > that this is how it is called in LD documentation), then LD
> > automatically uses its own default script and apply your "layer" script
> > on it. So with this way you can completely avoid copy+paste files from
> > binutils to u-boot repository.
> 
> That is considerably more complicated, though.
> 
> >
> > > But I was rather hoping that someone could help with the core problem,
> > > i.e. why I cannot just insert another section into the image, like we
> > > do with ELF?
> >
> > That is interesting. Theoretically it should work but I have not seen it
> > widely used outside of NT kernel modules. So maybe there is some bug in
> > Windows loader for userspace binaries, or another bug in GNU LD, or just
> > a bug in u-boot / linker script.
> >
> > Could you show linker script which is causing generation of that buggy
> > non-working binary? And ideally can you provide also that buggy EXE
> > binary? I could try to inspect it, but I do not have time to setup MSYS2
> > environment to compile my own buggy EXE binary.
> 
> Please see [1] for the file.
> 
> sglass@DESKTOP-OHNGJ4K MINGW64 ~/u-boot
> $ objdump.exe -h /tmp/b/sandbox/u-boot.exe
> 
> /tmp/b/sandbox/u-boot.exe:     file format pei-x86-64
> 
> Sections:
> Idx Name          Size      VMA               LMA               File off  Algn
>   0 .text         00208b98  0000000100401000  0000000100401000  00000600  2**4
>                   CONTENTS, ALLOC, LOAD, READONLY, CODE, DATA
>   1 __u_boot_list 00014960  0000000100609ba0  0000000100609ba0  00209200  2**2
>                   CONTENTS, ALLOC, LOAD, DATA
>   2 _u_boot_sandbox_getopt 000000c0  000000010061e500
> 000000010061e500  0021dc00  2**2
>                   CONTENTS, ALLOC, LOAD, DATA
>   3 .data         00019a20  000000010061f000  000000010061f000  0021de00  2**4
>                   CONTENTS, ALLOC, LOAD, DATA
>   4 .rodata.ttf.init 00036e10  0000000100639000  0000000100639000
> 00237a00  2**2
>                   CONTENTS, ALLOC, LOAD, DATA
>   5 .rodata.splash.init 00001b20  0000000100670000  0000000100670000
> 0026ea00  2**2
>                   CONTENTS, ALLOC, LOAD, DATA
>   6 .rodata.helloworld.init 00003030  0000000100672000
> 0000000100672000  00270600  2**2
>                   CONTENTS, ALLOC, LOAD, DATA
>   7 .data.efi_runtime 00000170  0000000100676000  0000000100676000
> 00273800  2**4
>                   CONTENTS, ALLOC, LOAD, DATA
>   8 .dtb.init.rodata 000007c0  0000000100677000  0000000100677000
> 00273a00  2**2
>                   CONTENTS, ALLOC, LOAD, DATA
>   9 .rdata        000a3fe0  0000000100678000  0000000100678000  00274200  2**4
>                   CONTENTS, ALLOC, LOAD, READONLY, DATA
>  10 .buildid      00000035  000000010071c000  000000010071c000  00318200  2**2
>                   CONTENTS, ALLOC, LOAD, READONLY, DATA
>  11 .rodata.efi_runtime 00000400  000000010071d000  000000010071d000
> 00318400  2**2
>                   CONTENTS, ALLOC, LOAD, READONLY, DATA
>  12 .pdata        0001785c  000000010071e000  000000010071e000  00318800  2**2
>                   CONTENTS, ALLOC, LOAD, READONLY, DATA
>  13 .xdata        00016f58  0000000100736000  0000000100736000  00330200  2**2
>                   CONTENTS, ALLOC, LOAD, READONLY, DATA
>  14 .bss          0001ecc0  000000010074d000  000000010074d000  00000000  2**4
>                   ALLOC
>  15 .idata        00000894  000000010076c000  000000010076c000  00347200  2**2
>                   CONTENTS, ALLOC, LOAD, DATA
>  16 .rsrc         000004e8  000000010076d000  000000010076d000  00347c00  2**2
>                   CONTENTS, ALLOC, LOAD, DATA
>  17 .reloc        00005284  000000010076e000  000000010076e000  00348200  2**2
>                   CONTENTS, ALLOC, LOAD, READONLY, DATA
>  18 .debug_aranges 0000b720  0000000100774000  0000000100774000  0034d600  2**0
>                   CONTENTS, READONLY, DEBUGGING
>  19 .debug_info   00a6c954  0000000100780000  0000000100780000  00358e00  2**0
>                   CONTENTS, READONLY, DEBUGGING
>  20 .debug_abbrev 000cb6b2  00000001011ed000  00000001011ed000  00dc5800  2**0
>                   CONTENTS, READONLY, DEBUGGING
>  21 .debug_line   001f646a  00000001012b9000  00000001012b9000  00e91000  2**0
>                   CONTENTS, READONLY, DEBUGGING
>  22 .debug_frame  000a9bd0  00000001014b0000  00000001014b0000  01087600  2**0
>                   CONTENTS, READONLY, DEBUGGING
>  23 .debug_str    0001d6c1  000000010155a000  000000010155a000  01131200  2**0
>                   CONTENTS, READONLY, DEBUGGING
>  24 .debug_loc    004673a1  0000000101578000  0000000101578000  0114ea00  2**0
>                   CONTENTS, READONLY, DEBUGGING
>  25 .debug_ranges 00060260  00000001019e0000  00000001019e0000  015b5e00  2**0
>                   CONTENTS, READONLY, DEBUGGING
>  26 .debug_line_str 00000264  0000000101a41000  0000000101a41000  01616200  2**0
>                   CONTENTS, READONLY, DEBUGGING
> 
> sglass@DESKTOP-OHNGJ4K MINGW64 ~/u-boot
> $ !$
> /tmp/b/sandbox/u-boot.exe
> -bash: /tmp/b/sandbox/u-boot.exe: cannot execute binary file: Exec format error

I have run readpe parser on this binary and parser crashed! Have not
seen such thing it. So I suspect that this binary is broken. I will look
at it deeply later. Another thing which I see suspicious in above output
are section names. They are too long and cannot fit into PE format which
has (small) limit on section names. Not sure if this can be an issue...

> 
> Regards,
> Simon
> 
> [1] https://drive.google.com/file/d/1vtJlvCd1BxkhRtQtUe4YqiRizcUP_zB2/view?usp=share_link

Re: [PATCH 10/31] sandbox: Provide a linker script for MSYS2

[Pali Rohár]

On Wednesday 26 April 2023 09:13:55 Pali Rohár wrote:
> On Tuesday 25 April 2023 18:50:59 Simon Glass wrote:
> > Hi Pali,
> > 
> > On Tue, 25 Apr 2023 at 13:33, Pali Rohár <pali@kernel.org> wrote:
> > >
> > > On Tuesday 25 April 2023 13:23:04 Simon Glass wrote:
> > > > Hi Pali,
> > > >
> > > > On Tue, 25 Apr 2023 at 12:11, Pali Rohár <pali@kernel.org> wrote:
> > > > >
> > > > > On Tuesday 25 April 2023 12:01:04 Simon Glass wrote:
> > > > > > Hi Pali,
> > > > > >
> > > > > > On Tue, 25 Apr 2023 at 10:21, Pali Rohár <pali@kernel.org> wrote:
> > > > > > >
> > > > > > > On Monday 24 April 2023 17:08:15 Simon Glass wrote:
> > > > > > > > Add a script to allow the U-Boot sandbox executable to be built for
> > > > > > > > Windows. Add a note as to why this seems to be necessary for now.
> > > > > > > >
> > > > > > > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > > > > > > > ---
> > > > > > > >
> > > > > > > >  Makefile                       |  11 +-
> > > > > > > >  arch/sandbox/config.mk         |   4 +
> > > > > > > >  arch/sandbox/cpu/u-boot-pe.lds | 447 +++++++++++++++++++++++++++++++++
> > > > > > > >  3 files changed, 460 insertions(+), 2 deletions(-)
> > > > > > > >  create mode 100644 arch/sandbox/cpu/u-boot-pe.lds
> > > > > > > >
> > > > > > > > diff --git a/Makefile b/Makefile
> > > > > > > > index dd3fcd1782e5..0aa97a2c3b48 100644
> > > > > > > > --- a/Makefile
> > > > > > > > +++ b/Makefile
> > > > > > > > @@ -1730,6 +1730,13 @@ 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
> > > > > > > > +$(warning msys)
> > > > > > > > +endif
> > > > > > > > +
> > > > > > > >  # Rule to link u-boot
> > > > > > > >  # May be overridden by arch/$(ARCH)/config.mk
> > > > > > > >  ifeq ($(LTO_ENABLE),y)
> > > > > > > > @@ -1738,7 +1745,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)                                 \
> > > > > > > > @@ -1749,7 +1756,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                                              \
> > > > > > > > diff --git a/arch/sandbox/config.mk b/arch/sandbox/config.mk
> > > > > > > > index 2d184c5f652a..e05daf57ef8e 100644
> > > > > > > > --- a/arch/sandbox/config.mk
> > > > > > > > +++ b/arch/sandbox/config.mk
> > > > > > > > @@ -71,3 +71,7 @@ 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 = $(srctree)/arch/sandbox/cpu/u-boot-pe.lds
> > > > > > > > +endif
> > > > > > > > diff --git a/arch/sandbox/cpu/u-boot-pe.lds b/arch/sandbox/cpu/u-boot-pe.lds
> > > > > > > > new file mode 100644
> > > > > > > > index 000000000000..031e70fafd03
> > > > > > > > --- /dev/null
> > > > > > > > +++ b/arch/sandbox/cpu/u-boot-pe.lds
> > > > > > > > @@ -0,0 +1,447 @@
> > > > > > > > +/* SPDX-License-Identifier: GPL-2.0+ */
> > > > > > > > +/*
> > > > > > > > + * U-Boot note: This was obtained by using the -verbose linker option. The
> > > > > > > > + * U-Boot additions are marked below.
> > > > > > > > + *
> > > > > > > > + * Ideally we would add sections to the executable, as is done with the Linux
> > > > > > > > + * build. But PE executables do not appear to work correctly if unexpected
> > > > > > > > + * sections are present:
> > > > > > > > + *
> > > > > > > > + *   $ /tmp/b/sandbox/u-boot.exe
> > > > > > > > + *   -bash: /tmp/b/sandbox/u-boot.exe: cannot execute binary file: Exec format error
> > > > > > > > + *
> > > > > > > > + * So we take a approach of rewriting the whole file, for now. This will likely
> > > > > > > > + * break in the future when a toolchain change is made.
> > > > > > >
> > > > > > > Why not rather provide "layer" linker script which does this "rewriting"
> > > > > > > on top of the default linker script? With this way it is not needed to
> > > > > > > update linker script when a toolchain change it.
> > > > > > >
> > > > > >
> > > > > > How can we reliably do that, though? We don't really know the format
> > > > > > of the script and where to insert stuff.
> > > > > >
> > > > > > Regards,
> > > > > > Simon
> > > > >
> > > > > Well, I do not know what is the current issue. The proposed script in
> > > >
> > > > See the comment at the top of the script:
> > > >
> > > > + * Ideally we would add sections to the executable, as is done with the Linux
> > > > + * build. But PE executables do not appear to work correctly if unexpected
> > > > + * sections are present:
> > > > + *
> > > > + *   $ /tmp/b/sandbox/u-boot.exe
> > > > + *   -bash: /tmp/b/sandbox/u-boot.exe: cannot execute binary file:
> > > > Exec format error
> > > > + *
> > > > + * So we take a approach of rewriting the whole file, for now. This will likely
> > > > + * break in the future when a toolchain change is made.
> > > >
> > > > > your patch looks like it is copied from some binutils version and then
> > > > > modified.
> > > >
> > > > Yes, exactly.
> > > >
> > > > > Also I do not know from which binutils you have copied and
> > > > > what modification you have done in it.
> > > >
> > > > But I have marked this clearly in the script. See  /* U-Boot additions
> > > > from here on */
> > >
> > > I read this, but I have not found from which binutils you took it...
> > > There are many released binutils versions and also each binutils
> > > version has more PE linker scripts. So it is really hard to track from
> > > which it comes. -verbose argument show you the final linker script but
> > > we need to know also from which files it was composed...
> > 
> > $ ld -V
> > GNU ld (GNU Binutils) 2.40
> >   Supported emulations:
> >    i386pep
> >    i386pe
> > 
> > sglass@DESKTOP-OHNGJ4K MINGW64 ~/u-boot
> > $ cc -v
> > Using built-in specs.
> > COLLECT_GCC=cc
> > COLLECT_LTO_WRAPPER=/usr/lib/gcc/x86_64-pc-msys/11.3.0/lto-wrapper.exe
> > Target: x86_64-pc-msys
> > Configured with: /c/S/gcc/src/gcc-11.3.0/configure
> > --build=x86_64-pc-msys --prefix=/usr --libexecdir=/usr/lib
> > --enable-bootstrap --enable-shared --enable-shared-libgcc
> > --enable-static --enable-version-specific-runtime-libs
> > --with-arch=x86-64 --with-tune=generic --disable-multilib
> > --enable-__cxa_atexit --with-dwarf2
> > --enable-languages=c,c++,fortran,lto --enable-graphite
> > --enable-threads=posix --enable-libatomic --enable-libgomp
> > --disable-libitm --enable-libquadmath --enable-libquadmath-support
> > --disable-libssp --disable-win32-registry --disable-symvers
> > --with-gnu-ld --with-gnu-as --disable-isl-version-check
> > --enable-checking=release --without-libiconv-prefix
> > --without-libintl-prefix --with-system-zlib --enable-linker-build-id
> > --with-default-libstdcxx-abi=gcc4-compatible
> > --enable-libstdcxx-filesystem-ts
> > Thread model: posix
> > Supported LTO compression algorithms: zlib
> > 
> > 
> > >
> > > > > So I cannot react or comment
> > > > > anymore more here.
> > > > >
> > > > > My idea is that to write those modifications into new layer script.
> > > >
> > > > So I think you are suggesting that I get binutils to emit the default
> > > > script, then have a script which detects the end of the .rdata section
> > > > and emits its own stuff in there? That may be more robust, since it is
> > > > unlikely that the .rdata section will be removed.
> > >
> > > Yes, exactly. And you do not need to take and copy default script from
> > > binutils to u-boot sources. If you write "layer" linker script (I hope
> > > that this is how it is called in LD documentation), then LD
> > > automatically uses its own default script and apply your "layer" script
> > > on it. So with this way you can completely avoid copy+paste files from
> > > binutils to u-boot repository.
> > 
> > That is considerably more complicated, though.
> > 
> > >
> > > > But I was rather hoping that someone could help with the core problem,
> > > > i.e. why I cannot just insert another section into the image, like we
> > > > do with ELF?
> > >
> > > That is interesting. Theoretically it should work but I have not seen it
> > > widely used outside of NT kernel modules. So maybe there is some bug in
> > > Windows loader for userspace binaries, or another bug in GNU LD, or just
> > > a bug in u-boot / linker script.
> > >
> > > Could you show linker script which is causing generation of that buggy
> > > non-working binary? And ideally can you provide also that buggy EXE
> > > binary? I could try to inspect it, but I do not have time to setup MSYS2
> > > environment to compile my own buggy EXE binary.
> > 
> > Please see [1] for the file.
> > 
> > sglass@DESKTOP-OHNGJ4K MINGW64 ~/u-boot
> > $ objdump.exe -h /tmp/b/sandbox/u-boot.exe
> > 
> > /tmp/b/sandbox/u-boot.exe:     file format pei-x86-64
> > 
> > Sections:
> > Idx Name          Size      VMA               LMA               File off  Algn
> >   0 .text         00208b98  0000000100401000  0000000100401000  00000600  2**4
> >                   CONTENTS, ALLOC, LOAD, READONLY, CODE, DATA
> >   1 __u_boot_list 00014960  0000000100609ba0  0000000100609ba0  00209200  2**2
> >                   CONTENTS, ALLOC, LOAD, DATA
> >   2 _u_boot_sandbox_getopt 000000c0  000000010061e500
> > 000000010061e500  0021dc00  2**2
> >                   CONTENTS, ALLOC, LOAD, DATA
> >   3 .data         00019a20  000000010061f000  000000010061f000  0021de00  2**4
> >                   CONTENTS, ALLOC, LOAD, DATA
> >   4 .rodata.ttf.init 00036e10  0000000100639000  0000000100639000
> > 00237a00  2**2
> >                   CONTENTS, ALLOC, LOAD, DATA
> >   5 .rodata.splash.init 00001b20  0000000100670000  0000000100670000
> > 0026ea00  2**2
> >                   CONTENTS, ALLOC, LOAD, DATA
> >   6 .rodata.helloworld.init 00003030  0000000100672000
> > 0000000100672000  00270600  2**2
> >                   CONTENTS, ALLOC, LOAD, DATA
> >   7 .data.efi_runtime 00000170  0000000100676000  0000000100676000
> > 00273800  2**4
> >                   CONTENTS, ALLOC, LOAD, DATA
> >   8 .dtb.init.rodata 000007c0  0000000100677000  0000000100677000
> > 00273a00  2**2
> >                   CONTENTS, ALLOC, LOAD, DATA
> >   9 .rdata        000a3fe0  0000000100678000  0000000100678000  00274200  2**4
> >                   CONTENTS, ALLOC, LOAD, READONLY, DATA
> >  10 .buildid      00000035  000000010071c000  000000010071c000  00318200  2**2
> >                   CONTENTS, ALLOC, LOAD, READONLY, DATA
> >  11 .rodata.efi_runtime 00000400  000000010071d000  000000010071d000
> > 00318400  2**2
> >                   CONTENTS, ALLOC, LOAD, READONLY, DATA
> >  12 .pdata        0001785c  000000010071e000  000000010071e000  00318800  2**2
> >                   CONTENTS, ALLOC, LOAD, READONLY, DATA
> >  13 .xdata        00016f58  0000000100736000  0000000100736000  00330200  2**2
> >                   CONTENTS, ALLOC, LOAD, READONLY, DATA
> >  14 .bss          0001ecc0  000000010074d000  000000010074d000  00000000  2**4
> >                   ALLOC
> >  15 .idata        00000894  000000010076c000  000000010076c000  00347200  2**2
> >                   CONTENTS, ALLOC, LOAD, DATA
> >  16 .rsrc         000004e8  000000010076d000  000000010076d000  00347c00  2**2
> >                   CONTENTS, ALLOC, LOAD, DATA
> >  17 .reloc        00005284  000000010076e000  000000010076e000  00348200  2**2
> >                   CONTENTS, ALLOC, LOAD, READONLY, DATA
> >  18 .debug_aranges 0000b720  0000000100774000  0000000100774000  0034d600  2**0
> >                   CONTENTS, READONLY, DEBUGGING
> >  19 .debug_info   00a6c954  0000000100780000  0000000100780000  00358e00  2**0
> >                   CONTENTS, READONLY, DEBUGGING
> >  20 .debug_abbrev 000cb6b2  00000001011ed000  00000001011ed000  00dc5800  2**0
> >                   CONTENTS, READONLY, DEBUGGING
> >  21 .debug_line   001f646a  00000001012b9000  00000001012b9000  00e91000  2**0
> >                   CONTENTS, READONLY, DEBUGGING
> >  22 .debug_frame  000a9bd0  00000001014b0000  00000001014b0000  01087600  2**0
> >                   CONTENTS, READONLY, DEBUGGING
> >  23 .debug_str    0001d6c1  000000010155a000  000000010155a000  01131200  2**0
> >                   CONTENTS, READONLY, DEBUGGING
> >  24 .debug_loc    004673a1  0000000101578000  0000000101578000  0114ea00  2**0
> >                   CONTENTS, READONLY, DEBUGGING
> >  25 .debug_ranges 00060260  00000001019e0000  00000001019e0000  015b5e00  2**0
> >                   CONTENTS, READONLY, DEBUGGING
> >  26 .debug_line_str 00000264  0000000101a41000  0000000101a41000  01616200  2**0
> >                   CONTENTS, READONLY, DEBUGGING
> > 
> > sglass@DESKTOP-OHNGJ4K MINGW64 ~/u-boot
> > $ !$
> > /tmp/b/sandbox/u-boot.exe
> > -bash: /tmp/b/sandbox/u-boot.exe: cannot execute binary file: Exec format error
> 
> I have run readpe parser on this binary and parser crashed! Have not
> seen such thing it. So I suspect that this binary is broken. I will look
> at it deeply later. Another thing which I see suspicious in above output
> are section names. They are too long and cannot fit into PE format which
> has (small) limit on section names. Not sure if this can be an issue...

Ok, this binary is really broken / corrupted. objdump -p also show lot
of errors "xdata section corrupt". Maybe this is just an another bug in
LD? I have found them a lot and some of them I reported to their
bugzilla. If you are trying to produce PE binaries it would be better to
switch to MS tools they always had better quality for production
software. GNU tools are here good just for playing or research.

> > 
> > Regards,
> > Simon
> > 
> > [1] https://drive.google.com/file/d/1vtJlvCd1BxkhRtQtUe4YqiRizcUP_zB2/view?usp=share_link

Re: [PATCH 27/31] Makefile: Disable LTO when building with MSYS2

[Simon Glass]

Hi Pali,

On Wed, 26 Apr 2023 at 01:07, Pali Rohár <pali@kernel.org> wrote:
>
> On Tuesday 25 April 2023 19:04:10 Simon Glass wrote:
> > Hi Pali,
> >
> > On Tue, 25 Apr 2023 at 10:28, Pali Rohár <pali@kernel.org> wrote:
> > >
> > > On Monday 24 April 2023 17:08:32 Simon Glass wrote:
> > > > 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)
> > >
> > > This issue should be rather fixed...
> > >
> > > > Drop LTO for now.
> > >
> > > ... and until it happens is not CONFIG_LTO for disabling enough?
> > >
> > > LTO does not work for more other boards / platforms and it is just _not_
> > > enabled via CONFIG_LTO in those cases...
> >
> > The thing is, LTO is enabled for sandbox normally (clang and gcc). It
> > is just the MSYS2 platform where there are problems.
>
> So what about having CONFIG_LTO by default 'n' for CONFIG_MSYS2?

But that would require creating a new board. I am trying to use the
same board, just building it in a different environment.


>
> > I am not sure how to fix it, since I don't know what discarded section
> > it is referring to.

Regards,
Simon

Re: [PATCH 27/31] Makefile: Disable LTO when building with MSYS2

[Pali Rohár]

On Thursday 27 April 2023 10:25:13 Simon Glass wrote:
> Hi Pali,
> 
> On Wed, 26 Apr 2023 at 01:07, Pali Rohár <pali@kernel.org> wrote:
> >
> > On Tuesday 25 April 2023 19:04:10 Simon Glass wrote:
> > > Hi Pali,
> > >
> > > On Tue, 25 Apr 2023 at 10:28, Pali Rohár <pali@kernel.org> wrote:
> > > >
> > > > On Monday 24 April 2023 17:08:32 Simon Glass wrote:
> > > > > 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)
> > > >
> > > > This issue should be rather fixed...
> > > >
> > > > > Drop LTO for now.
> > > >
> > > > ... and until it happens is not CONFIG_LTO for disabling enough?
> > > >
> > > > LTO does not work for more other boards / platforms and it is just _not_
> > > > enabled via CONFIG_LTO in those cases...
> > >
> > > The thing is, LTO is enabled for sandbox normally (clang and gcc). It
> > > is just the MSYS2 platform where there are problems.
> >
> > So what about having CONFIG_LTO by default 'n' for CONFIG_MSYS2?
> 
> But that would require creating a new board. I am trying to use the
> same board, just building it in a different environment.

Sorry, but I do not understand why you need a new board. Just introduce
a new Kconfig symbol CONFIG_MSYS2 which value is by default assigned
from the detected compiler and then set default value for CONFIG_LTO
symbol to "n" when CONFIG_MSYS2 is "y". This does not require a new
board and neither menuconfig user changes of (default) symbols.

> 
> >
> > > I am not sure how to fix it, since I don't know what discarded section
> > > it is referring to.
> 
> Regards,
> Simon

Re: [PATCH 27/31] Makefile: Disable LTO when building with MSYS2

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 1426 bytes --]

On Thu, Apr 27, 2023 at 10:25:13AM -0600, Simon Glass wrote:
> Hi Pali,
> 
> On Wed, 26 Apr 2023 at 01:07, Pali Rohár <pali@kernel.org> wrote:
> >
> > On Tuesday 25 April 2023 19:04:10 Simon Glass wrote:
> > > Hi Pali,
> > >
> > > On Tue, 25 Apr 2023 at 10:28, Pali Rohár <pali@kernel.org> wrote:
> > > >
> > > > On Monday 24 April 2023 17:08:32 Simon Glass wrote:
> > > > > 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)
> > > >
> > > > This issue should be rather fixed...
> > > >
> > > > > Drop LTO for now.
> > > >
> > > > ... and until it happens is not CONFIG_LTO for disabling enough?
> > > >
> > > > LTO does not work for more other boards / platforms and it is just _not_
> > > > enabled via CONFIG_LTO in those cases...
> > >
> > > The thing is, LTO is enabled for sandbox normally (clang and gcc). It
> > > is just the MSYS2 platform where there are problems.
> >
> > So what about having CONFIG_LTO by default 'n' for CONFIG_MSYS2?
> 
> But that would require creating a new board. I am trying to use the
> same board, just building it in a different environment.

I think we need to make CONFIG_LTO depend on CC_IS_GCC for now as it
also doesn't work (but could be addressed) for CC_IS_CLANG.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 27/31] Makefile: Disable LTO when building with MSYS2

[Marek Behún]

On Thu, 27 Apr 2023 13:34:53 -0400
Tom Rini <trini@konsulko.com> wrote:

> On Thu, Apr 27, 2023 at 10:25:13AM -0600, Simon Glass wrote:
> > Hi Pali,
> > 
> > On Wed, 26 Apr 2023 at 01:07, Pali Rohár <pali@kernel.org> wrote:  
> > >
> > > On Tuesday 25 April 2023 19:04:10 Simon Glass wrote:  
> > > > Hi Pali,
> > > >
> > > > On Tue, 25 Apr 2023 at 10:28, Pali Rohár <pali@kernel.org> wrote:  
> > > > >
> > > > > On Monday 24 April 2023 17:08:32 Simon Glass wrote:  
> > > > > > 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)  
> > > > >
> > > > > This issue should be rather fixed...
> > > > >  
> > > > > > Drop LTO for now.  
> > > > >
> > > > > ... and until it happens is not CONFIG_LTO for disabling enough?
> > > > >
> > > > > LTO does not work for more other boards / platforms and it is just _not_
> > > > > enabled via CONFIG_LTO in those cases...  
> > > >
> > > > The thing is, LTO is enabled for sandbox normally (clang and gcc). It
> > > > is just the MSYS2 platform where there are problems.  
> > >
> > > So what about having CONFIG_LTO by default 'n' for CONFIG_MSYS2?  
> > 
> > But that would require creating a new board. I am trying to use the
> > same board, just building it in a different environment.  
> 
> I think we need to make CONFIG_LTO depend on CC_IS_GCC for now as it
> also doesn't work (but could be addressed) for CC_IS_CLANG.
> 

It got broken? It should work at least for sandbox...

Marek

Re: [PATCH 27/31] Makefile: Disable LTO when building with MSYS2

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 2128 bytes --]

On Fri, Apr 28, 2023 at 09:37:57AM +0200, Marek Behún wrote:
> On Thu, 27 Apr 2023 13:34:53 -0400
> Tom Rini <trini@konsulko.com> wrote:
> 
> > On Thu, Apr 27, 2023 at 10:25:13AM -0600, Simon Glass wrote:
> > > Hi Pali,
> > > 
> > > On Wed, 26 Apr 2023 at 01:07, Pali Rohár <pali@kernel.org> wrote:  
> > > >
> > > > On Tuesday 25 April 2023 19:04:10 Simon Glass wrote:  
> > > > > Hi Pali,
> > > > >
> > > > > On Tue, 25 Apr 2023 at 10:28, Pali Rohár <pali@kernel.org> wrote:  
> > > > > >
> > > > > > On Monday 24 April 2023 17:08:32 Simon Glass wrote:  
> > > > > > > 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)  
> > > > > >
> > > > > > This issue should be rather fixed...
> > > > > >  
> > > > > > > Drop LTO for now.  
> > > > > >
> > > > > > ... and until it happens is not CONFIG_LTO for disabling enough?
> > > > > >
> > > > > > LTO does not work for more other boards / platforms and it is just _not_
> > > > > > enabled via CONFIG_LTO in those cases...  
> > > > >
> > > > > The thing is, LTO is enabled for sandbox normally (clang and gcc). It
> > > > > is just the MSYS2 platform where there are problems.  
> > > >
> > > > So what about having CONFIG_LTO by default 'n' for CONFIG_MSYS2?  
> > > 
> > > But that would require creating a new board. I am trying to use the
> > > same board, just building it in a different environment.  
> > 
> > I think we need to make CONFIG_LTO depend on CC_IS_GCC for now as it
> > also doesn't work (but could be addressed) for CC_IS_CLANG.
> 
> It got broken? It should work at least for sandbox...

Ah right, we do clang + sandbox (so + LTO). In my clang for arm
experiments, I could only get LTO to almost-link if I also used lld
instead of GNU ld (on the platform I was checking there's some _other_
problem that causes clang's SPL build to be something like 40kB larger
than gcc, so even with LTO it was still overflowing).

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 17/31] sandbox: Allow weak symbols to be dropped

[Simon Glass]

Hi Bin,

On Mon, 24 Apr 2023 at 20:44, Bin Meng <bmeng.cn@gmail.com> wrote:
>
> Hi Simon,
>
> On Tue, Apr 25, 2023 at 7:09 AM Simon Glass <sjg@chromium.org> wrote:
> >
> > 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>
> > ---
> >
> >  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(-)
> >
>
> You probably need to use:
>
> __declspec(selectany)
>
> to replace __weak in the ELF for the *nix world.
>
> Note this Microsoft bizarre does not provide the "strong override
> weak" effect but I suspect what you only need is to get the build pass
> on Windows so it should be okay.

With that I get:

lib/sha1.c:226:1: error: ‘selectany’ attribute applies only to
initialized variables with external linkage

and it dies.

The line is:

__weak void sha1_process(sha1_context *ctx, const unsigned char *data,
unsigned int blocks)

I tried adding a prototype to the header but got the same error.

Regards,
Simon

Re: [PATCH 17/31] sandbox: Allow weak symbols to be dropped

[Simon Glass]

Hi Heinrich,

On Mon, 24 Apr 2023 at 20:47, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
>
>
> Am 25. April 2023 01:08:22 MESZ schrieb Simon Glass <sjg@chromium.org>:
> >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.
>
> This comment does not explain the necessity of the change. Why is it really needed? Has the compiler version in MSYS a problem?

Yes, from what I understand, PE has never really supported weak
symbols properly. I'm not sure if that has changed, but it doesn't
look like it.

I'm hoping there is another workaround.

That said, I really don't like weak symbols and I think we should use them less.

[..]

Regards,
Simon

Re: [PATCH 17/31] sandbox: Allow weak symbols to be dropped

[Simon Glass]

Hi Pali,

On Tue, 25 Apr 2023 at 10:31, Pali Rohár <pali@kernel.org> wrote:
>
> On Tuesday 25 April 2023 10:44:17 Bin Meng wrote:
> > Hi Simon,
> >
> > On Tue, Apr 25, 2023 at 7:09 AM Simon Glass <sjg@chromium.org> wrote:
> > >
> > > 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>
> > > ---
> > >
> > >  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(-)
> > >
> >
> > You probably need to use:
> >
> > __declspec(selectany)
> >
> > to replace __weak in the ELF for the *nix world.
> >
> > Note this Microsoft bizarre does not provide the "strong override
> > weak" effect but I suspect what you only need is to get the build pass
> > on Windows so it should be okay.
> >
> > Regards,
> > Bin
>
> You have selectany and external weaks support in PE format. For the
> majority of weak function cases, this is enough. Cannot we use them?

Not that I can tell.

Can you provide a patch to linux/compiler_attributes.h which handles this?

Regards,
Simon

Re: [PATCH 24/31] build: Disable weak symbols for MSYS2

[Simon Glass]

Hi Pali,

On Tue, 25 Apr 2023 at 10:21, Pali Rohár <pali@kernel.org> wrote:
>
> On Monday 24 April 2023 17:08:29 Simon Glass wrote:
> > Weak symbols are not well supported by the PE format, so disable them.
>
> They are supported by PE format. This is likely issue of (older)
> toolchain. What about rather requiring better toolchain version and
> fix special cases of weak functions do not work correctly?

What compiler are you using? How do weak symbols work in that compiler?

I am using:

$ gcc -v

Using built-in specs.

COLLECT_GCC=gcc

COLLECT_LTO_WRAPPER=/usr/lib/gcc/x86_64-pc-msys/11.3.0/lto-wrapper.exe

Target: x86_64-pc-msys

Configured with: /c/S/gcc/src/gcc-11.3.0/configure
--build=x86_64-pc-msys --prefix=/usr --libexecdir=/usr/lib
--enable-bootstrap --enable-shared --enable-shared-libgcc
--enable-static --enable-version-specific-runtime-libs
--with-arch=x86-64 --with-tune=generic --disable-multilib
--enable-__cxa_atexit --with-dwarf2
--enable-languages=c,c++,fortran,lto --enable-graphite
--enable-threads=posix --enable-libatomic --enable-libgomp
--disable-libitm --enable-libquadmath --enable-libquadmath-support
--disable-libssp --disable-win32-registry --disable-symvers
--with-gnu-ld --with-gnu-as --disable-isl-version-check
--enable-checking=release --without-libiconv-prefix
--without-libintl-prefix --with-system-zlib --enable-linker-build-id
--with-default-libstdcxx-abi=gcc4-compatible
--enable-libstdcxx-filesystem-ts

Thread model: posix

Supported LTO compression algorithms: zlib

gcc version 11.3.0 (GCC)


sglass@DESKTOP-OHNGJ4K MINGW64 ~/u-boot

$ ld -v

GNU ld (GNU Binutils) 2.40

Regards,
Simon


>
> > 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>
> > ---
> >
> >  Kconfig                             | 15 +++++++++++++++
> >  include/linux/compiler_attributes.h |  4 ++++
> >  2 files changed, 19 insertions(+)
> >
> > diff --git a/Kconfig b/Kconfig
> > index f24e4f0a331e..ca1402d09d10 100644
> > --- a/Kconfig
> > +++ b/Kconfig
> > @@ -72,6 +72,21 @@ config CLANG_VERSION
> >       int
> >       default $(shell,$(srctree)/scripts/clang-version.sh $(CC))
> >
> > +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-weak-function-attribute
> >   *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Variable-Attributes.html#index-weak-variable-attribute
> >   */
> > +#ifdef CONFIG_WEAK_SYMBOLS
> >  #define __weak                          __attribute__((__weak__))
> > +#else
> > +#define __weak
> > +#endif
> >
> >  #endif /* __LINUX_COMPILER_ATTRIBUTES_H */
> > --
> > 2.40.0.634.g4ca3ef3211-goog
> >

Re: [PATCH 17/31] sandbox: Allow weak symbols to be dropped

[Pali Rohár]

On Saturday 29 April 2023 12:41:31 Simon Glass wrote:
> Hi Pali,
> 
> On Tue, 25 Apr 2023 at 10:31, Pali Rohár <pali@kernel.org> wrote:
> >
> > On Tuesday 25 April 2023 10:44:17 Bin Meng wrote:
> > > Hi Simon,
> > >
> > > On Tue, Apr 25, 2023 at 7:09 AM Simon Glass <sjg@chromium.org> wrote:
> > > >
> > > > 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>
> > > > ---
> > > >
> > > >  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(-)
> > > >
> > >
> > > You probably need to use:
> > >
> > > __declspec(selectany)
> > >
> > > to replace __weak in the ELF for the *nix world.
> > >
> > > Note this Microsoft bizarre does not provide the "strong override
> > > weak" effect but I suspect what you only need is to get the build pass
> > > on Windows so it should be okay.
> > >
> > > Regards,
> > > Bin
> >
> > You have selectany and external weaks support in PE format. For the
> > majority of weak function cases, this is enough. Cannot we use them?
> 
> Not that I can tell.
> 
> Can you provide a patch to linux/compiler_attributes.h which handles this?

I can after all my patches on the list are reviewed and merged.

No, I'm not going to _again_ spend time on another patch and then wait
years after somebody take it.

> Regards,
> Simon

Re: [PATCH 17/31] sandbox: Allow weak symbols to be dropped

[Bin Meng]

Hi Simon,

On Sun, Apr 30, 2023 at 2:41 AM Simon Glass <sjg@chromium.org> wrote:
>
> Hi Bin,
>
> On Mon, 24 Apr 2023 at 20:44, Bin Meng <bmeng.cn@gmail.com> wrote:
> >
> > Hi Simon,
> >
> > On Tue, Apr 25, 2023 at 7:09 AM Simon Glass <sjg@chromium.org> wrote:
> > >
> > > 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>
> > > ---
> > >
> > >  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(-)
> > >
> >
> > You probably need to use:
> >
> > __declspec(selectany)
> >
> > to replace __weak in the ELF for the *nix world.
> >
> > Note this Microsoft bizarre does not provide the "strong override
> > weak" effect but I suspect what you only need is to get the build pass
> > on Windows so it should be okay.
>
> With that I get:
>
> lib/sha1.c:226:1: error: ‘selectany’ attribute applies only to
> initialized variables with external linkage
>
> and it dies.
>
> The line is:
>
> __weak void sha1_process(sha1_context *ctx, const unsigned char *data,
> unsigned int blocks)
>
> I tried adding a prototype to the header but got the same error.
>

I will see if I can find some time to check this.

Regards,
Bin