[PATCH v10 0/2] xen: Strip xen.efi by default

[PATCH v10 0/2] xen: Strip xen.efi by default

[Frediano Ziglio]

From: Frediano Ziglio <frediano.ziglio@cloud.com>

The main purpose of this series is to strip xen.efi file.

First patch disable the build of the file on systems having broken toolstack.
Second patch strip the wanted file.

See changes on specific patches.

Frediano Ziglio (2):
  Do not attempt to workaround older binutils
  xen: Strip xen.efi by default

 .gitignore            |  1 +
 CHANGELOG.md          |  4 ++++
 docs/misc/efi.pandoc  |  8 +-------
 xen/Kconfig.debug     |  9 ++-------
 xen/Makefile          | 25 +++----------------------
 xen/arch/x86/Makefile | 12 +++++++++---
 xen/arch/x86/arch.mk  |  7 -------
 7 files changed, 20 insertions(+), 46 deletions(-)

-- 
2.43.0

[PATCH v10 1/2] Do not attempt to workaround older binutils

[Frediano Ziglio]

From: Frediano Ziglio <frediano.ziglio@cloud.com>

Older binutils versions do not handle correctly PE files.
It looks like they could work if they don't produce debug information
but they mess the PE file in other way like putting invalid
flags in sections.
For instance they set IMAGE_SCN_LNK_NRELOC_OVFL flag which should be
set only if the number of relocations are more than 64K and not on
executable (while xen.efi is an executable).
Although some UEFI implementation do not check for these minor flags
we should not allow building not working artifacts.
Also different tools will complain about the format (like
objdump and strip).

Signed-off-by: Frediano Ziglio <frediano.ziglio@cloud.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
Changes since v9:
- explain one of the broken flags.
---
 xen/arch/x86/arch.mk | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/xen/arch/x86/arch.mk b/xen/arch/x86/arch.mk
index 16368a498b..10eb8e4292 100644
--- a/xen/arch/x86/arch.mk
+++ b/xen/arch/x86/arch.mk
@@ -88,13 +88,6 @@ EFI_LDFLAGS := $(patsubst -m%,-mi386pep,$(LDFLAGS)) --subsystem=10 --enable-long
 LD_PE_check_cmd = $(call ld-option,$(EFI_LDFLAGS) --image-base=0x100000000 -o $(efi-check).efi $(efi-check).o)
 XEN_BUILD_PE := $(LD_PE_check_cmd)
 
-# If the above failed, it may be merely because of the linker not dealing well
-# with debug info. Try again with stripping it.
-ifeq ($(CONFIG_DEBUG_INFO)-$(XEN_BUILD_PE),y-n)
-EFI_LDFLAGS += --strip-debug
-XEN_BUILD_PE := $(LD_PE_check_cmd)
-endif
-
 ifeq ($(XEN_BUILD_PE),y)
 
 # Check if the linker produces fixups in PE by default
-- 
2.43.0

[PATCH v10 2/2] xen: Strip xen.efi by default

[Frediano Ziglio]

From: Frediano Ziglio <frediano.ziglio@cloud.com>

For xen.gz file we strip all symbols and have an additional
xen-syms.efi file version with all symbols.
Make xen.efi more coherent stripping all symbols too.
xen-syms.efi can be used for debugging.

Signed-off-by: Frediano Ziglio <frediano.ziglio@cloud.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
---
Changes since v1:
- avoid leaving target if some command fails.

Changes since v2:
- do not convert type but retain PE format;
- use xen-syms.efi for new file name, more consistent with ELF.

Changes since v3:
- update documentation;
- do not remove xen.efi.elf;
- check endbr instruction before generating final target.

Changes since v4:
- simplify condition check;
- avoid reuse of $@.tmp file.

Changes since v5:
- avoid creation of temporary file.

Changes since v6:
- install xen-syms.efi;
- always strip xen.efi;
- restore EFI_LDFLAGS check during rule execution;
- update CHANGELOG.md;
- added xen-syms.efi to .gitignore.

Changes since v7:
- move and improve CHANGELOG.md changes.

Changes since v8:
- rebase on master;
- clean xen-syms.efi file.

Changes since v9:
- Move changelog change to 4.22.
---
 .gitignore            |  1 +
 CHANGELOG.md          |  4 ++++
 docs/misc/efi.pandoc  |  8 +-------
 xen/Kconfig.debug     |  9 ++-------
 xen/Makefile          | 25 +++----------------------
 xen/arch/x86/Makefile | 12 +++++++++---
 6 files changed, 20 insertions(+), 39 deletions(-)

diff --git a/.gitignore b/.gitignore
index 57d54f676f..f282192b3e 100644
--- a/.gitignore
+++ b/.gitignore
@@ -223,6 +223,7 @@ tools/flask/policy/xenpolicy-*
 xen/xen
 xen/suppression-list.txt
 xen/xen-syms
+xen/xen-syms.efi
 xen/xen-syms.map
 xen/xen.*
 
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 3aaf598623..d15988fe1f 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -7,6 +7,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 ## [4.22.0 UNRELEASED](https://xenbits.xenproject.org/gitweb/?p=xen.git;a=shortlog;h=staging) - TBD
 
 ### Changed
+ - On x86:
+   - The install-time environment variable INSTALL_EFI_STRIP no longer exists.
+     xen.efi is always stripped, while the symbols remain available in
+     xen-syms.efi.
 
 ### Added
 
diff --git a/docs/misc/efi.pandoc b/docs/misc/efi.pandoc
index 11c1ac3346..c66b18a66b 100644
--- a/docs/misc/efi.pandoc
+++ b/docs/misc/efi.pandoc
@@ -20,13 +20,7 @@ Xen to load the configuration file even if multiboot modules are found.
 Once built, `make install-xen` will place the resulting binary directly into
 the EFI boot partition, provided `EFI_VENDOR` is set in the environment (and
 `EFI_MOUNTPOINT` is overridden as needed, should the default of `/boot/efi` not
-match your system). When built with debug info, the binary can be quite large.
-Setting `INSTALL_EFI_STRIP=1` in the environment will cause it to be stripped
-of debug info in the process of installing. `INSTALL_EFI_STRIP` can also be set
-to any combination of options suitable to pass to `strip`, in case the default
-ones don't do. The xen.efi binary will also be installed in `/usr/lib64/efi/`,
-unless `EFI_DIR` is set in the environment to override this default. This
-binary will not be stripped in the process.
+match your system).
 
 The binary itself will require a configuration file (names with the `.efi`
 extension of the binary's name replaced by `.cfg`, and - until an existing
diff --git a/xen/Kconfig.debug b/xen/Kconfig.debug
index d900d926c5..1a8e0c6ec3 100644
--- a/xen/Kconfig.debug
+++ b/xen/Kconfig.debug
@@ -147,12 +147,7 @@ config DEBUG_INFO
 	  Say Y here if you want to build Xen with debug information. This
 	  information is needed e.g. for doing crash dump analysis of the
 	  hypervisor via the "crash" tool.
-	  Saying Y will increase the size of the xen-syms and xen.efi
-	  binaries. In case the space on the EFI boot partition is rather
-	  limited, you may want to install a stripped variant of xen.efi in
-	  the EFI boot partition (look for "INSTALL_EFI_STRIP" in
-	  docs/misc/efi.pandoc for more information - when not using
-	  "make install-xen" for installing xen.efi, stripping needs to be
-	  done outside the Xen build environment).
+	  Saying Y will increase the size of the xen-syms, xen-syms.efi and
+	  xen.efi.elf binaries.
 
 endmenu
diff --git a/xen/Makefile b/xen/Makefile
index e6cf287425..c2f1f84aae 100644
--- a/xen/Makefile
+++ b/xen/Makefile
@@ -491,22 +491,6 @@ endif
 .PHONY: _build
 _build: $(TARGET)$(CONFIG_XEN_INSTALL_SUFFIX)
 
-# Strip
-#
-# INSTALL_EFI_STRIP, if defined, will cause xen.efi to be stripped before it
-# is installed. If INSTALL_EFI_STRIP is '1', then the default option(s) below
-# will be used. Otherwise, INSTALL_EFI_STRIP value will be used as the
-# option(s) to the strip command.
-ifdef INSTALL_EFI_STRIP
-
-ifeq ($(INSTALL_EFI_STRIP),1)
-efi-strip-opt := --strip-debug --keep-file-symbols
-else
-efi-strip-opt := $(INSTALL_EFI_STRIP)
-endif
-
-endif
-
 .PHONY: _install
 _install: D=$(DESTDIR)
 _install: T=$(notdir $(TARGET))
@@ -524,18 +508,15 @@ _install: $(TARGET)$(CONFIG_XEN_INSTALL_SUFFIX)
 	if [ -r $(TARGET).efi -a -n '$(EFI_DIR)' ]; then \
 		[ -d $(D)$(EFI_DIR) ] || $(INSTALL_DIR) $(D)$(EFI_DIR); \
 		$(INSTALL_DATA) $(TARGET).efi $(D)$(EFI_DIR)/$(T)-$(XEN_FULLVERSION).efi; \
-		for x in map elf; do \
-			if [ -e $(TARGET).efi.$$x ]; then \
-				$(INSTALL_DATA) $(TARGET).efi.$$x $(D)$(DEBUG_DIR)/$(T)-$(XEN_FULLVERSION).efi.$$x; \
+		for x in .efi.map .efi.elf -syms.efi; do \
+			if [ -e $(TARGET)$$x ]; then \
+				$(INSTALL_DATA) $(TARGET)$$x $(D)$(DEBUG_DIR)/$(T)-$(XEN_FULLVERSION)$$x; \
 			fi; \
 		done; \
 		ln -sf $(T)-$(XEN_FULLVERSION).efi $(D)$(EFI_DIR)/$(T)-$(XEN_VERSION).$(XEN_SUBVERSION).efi; \
 		ln -sf $(T)-$(XEN_FULLVERSION).efi $(D)$(EFI_DIR)/$(T)-$(XEN_VERSION).efi; \
 		ln -sf $(T)-$(XEN_FULLVERSION).efi $(D)$(EFI_DIR)/$(T).efi; \
 		if [ -n '$(EFI_MOUNTPOINT)' -a -n '$(EFI_VENDOR)' ]; then \
-			$(if $(efi-strip-opt), \
-			     $(STRIP) $(efi-strip-opt) -p -o $(TARGET).efi.stripped $(TARGET).efi && \
-			     $(INSTALL_DATA) $(TARGET).efi.stripped $(D)$(EFI_MOUNTPOINT)/efi/$(EFI_VENDOR)/$(T)-$(XEN_FULLVERSION).efi ||) \
 			$(INSTALL_DATA) $(TARGET).efi $(D)$(EFI_MOUNTPOINT)/efi/$(EFI_VENDOR)/$(T)-$(XEN_FULLVERSION).efi; \
 		elif [ "$(D)" = "$(patsubst $(shell cd $(XEN_ROOT) && pwd)/%,%,$(D))" ]; then \
 			echo 'EFI installation only partially done (EFI_VENDOR not set)' >&2; \
diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
index 300cc67407..ee787068f8 100644
--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -232,12 +232,17 @@ endif
 	$(MAKE) $(build)=$(@D) .$(@F).2r.o .$(@F).2s.o
 	$(LD) $(call EFI_LDFLAGS,$(VIRT_BASE)) -T $(obj)/efi.lds $< \
 	      $(dot-target).2r.o $(dot-target).2s.o $(orphan-handling-y) \
-	      $(note_file_option) -o $@
-	$(NM) -pa --format=sysv $@ \
+	      $(note_file_option) -o $(TARGET)-syms.efi
+	$(NM) -pa --format=sysv $(TARGET)-syms.efi \
 		| $(objtree)/tools/symbols --all-symbols --xensyms --sysv --sort \
 		> $@.map
 ifeq ($(CONFIG_DEBUG_INFO),y)
-	$(if $(filter --strip-debug,$(EFI_LDFLAGS)),:$(space))$(OBJCOPY) -O elf64-x86-64 $@ $@.elf
+	$(if $(filter --strip-debug,$(EFI_LDFLAGS)),:$(space))$(OBJCOPY) \
+		-O elf64-x86-64 $(TARGET)-syms.efi $@.elf
+endif
+	$(STRIP) $(TARGET)-syms.efi -o $@
+ifneq ($(CONFIG_DEBUG_INFO),y)
+	rm -f $(TARGET)-syms.efi
 endif
 	rm -f $(dot-target).[0-9]* $(@D)/..$(@F).[0-9]*
 ifeq ($(CONFIG_XEN_IBT),y)
@@ -281,6 +286,7 @@ $(obj)/xen.lds $(obj)/efi.lds: $(src)/xen.lds.S FORCE
 clean-files := \
     include/asm/asm-macros.* \
     $(objtree)/.xen-syms.[0-9]* \
+    $(objtree)/xen-syms.efi \
     $(objtree)/.xen.elf32 \
     $(objtree)/.xen.efi.[0-9]* \
     efi/*.efi
-- 
2.43.0

Re: [PATCH v10 1/2] Do not attempt to workaround older binutils

[Jan Beulich]

On 08.12.2025 14:39, Frediano Ziglio wrote:
> From: Frediano Ziglio <frediano.ziglio@cloud.com>
> 
> Older binutils versions do not handle correctly PE files.
> It looks like they could work if they don't produce debug information
> but they mess the PE file in other way like putting invalid
> flags in sections.
> For instance they set IMAGE_SCN_LNK_NRELOC_OVFL flag which should be
> set only if the number of relocations are more than 64K and not on
> executable (while xen.efi is an executable).
> Although some UEFI implementation do not check for these minor flags
> we should not allow building not working artifacts.

The sentence is self-contradictory imo: When "some UEFI implementation
do not check", what "not working artifacts" are you talking about?

> Also different tools will complain about the format (like
> objdump and strip).
> 
> Signed-off-by: Frediano Ziglio <frediano.ziglio@cloud.com>
> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>

As said in reply to v9 - I don't think we should be taking this step
unless we can prove the generated binaries to be entirely unusable.
Which, again as said, contradicts my personal experience.

Jan

Re: [PATCH v10 1/2] Do not attempt to workaround older binutils

[Frediano Ziglio]

On Mon, 8 Dec 2025 at 13:56, Jan Beulich <jbeulich@suse.com> wrote:
>
> On 08.12.2025 14:39, Frediano Ziglio wrote:
> > From: Frediano Ziglio <frediano.ziglio@cloud.com>
> >
> > Older binutils versions do not handle correctly PE files.
> > It looks like they could work if they don't produce debug information
> > but they mess the PE file in other way like putting invalid
> > flags in sections.
> > For instance they set IMAGE_SCN_LNK_NRELOC_OVFL flag which should be
> > set only if the number of relocations are more than 64K and not on
> > executable (while xen.efi is an executable).
> > Although some UEFI implementation do not check for these minor flags
> > we should not allow building not working artifacts.
>
> The sentence is self-contradictory imo: When "some UEFI implementation
> do not check", what "not working artifacts" are you talking about?
>

I cannot see the contradiction, some work, some not.

> > Also different tools will complain about the format (like
> > objdump and strip).
> >
> > Signed-off-by: Frediano Ziglio <frediano.ziglio@cloud.com>
> > Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
>
> As said in reply to v9 - I don't think we should be taking this step
> unless we can prove the generated binaries to be entirely unusable.

I prefer if the build produces 100% working instead of a russian roulette.

> Which, again as said, contradicts my personal experience.
>

I never said they never work. But implementations keep progressing and
there will be more of them improving their checks. For instance
currently implementations with enhanced memory protection will cause
early crashes.

> Jan

Frediano

Re: [PATCH v10 2/2] xen: Strip xen.efi by default

[Jason Andryuk]

On 2025-12-08 08:39, Frediano Ziglio wrote:
> From: Frediano Ziglio <frediano.ziglio@cloud.com>
> 
> For xen.gz file we strip all symbols and have an additional
> xen-syms.efi file version with all symbols.
> Make xen.efi more coherent stripping all symbols too.
> xen-syms.efi can be used for debugging.
> 
> Signed-off-by: Frediano Ziglio <frediano.ziglio@cloud.com>
> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Acked-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>

Reviewed-by: Jason Andryuk <jason.andryuk@amd.com>

While there is some discussion about patch 1, it seems like this patch 
can do in independently of it?

Regards,
Jason

Re: [PATCH v10 2/2] xen: Strip xen.efi by default

[Andrew Cooper]

On 05/05/2026 10:10 pm, Jason Andryuk wrote:
> On 2025-12-08 08:39, Frediano Ziglio wrote:
>> From: Frediano Ziglio <frediano.ziglio@cloud.com>
>>
>> For xen.gz file we strip all symbols and have an additional
>> xen-syms.efi file version with all symbols.
>> Make xen.efi more coherent stripping all symbols too.
>> xen-syms.efi can be used for debugging.
>>
>> Signed-off-by: Frediano Ziglio <frediano.ziglio@cloud.com>
>> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
>> Acked-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
>
> Reviewed-by: Jason Andryuk <jason.andryuk@amd.com>
>
> While there is some discussion about patch 1, it seems like this patch
> can do in independently of it?

No.  One of the containers in CI has a buggy binutils.

~Andrew