1. Patchew
  2. Xen
  3. View series

[PATCH v5] xen: Strip xen.efi by default

Frediano Ziglio posted 1 patch 1 month ago
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/20251110125859.78124-1-frediano.ziglio@citrix.com
There is a newer version of this series
docs/misc/efi.pandoc  |  8 +-------
xen/Kconfig.debug     |  9 ++-------
xen/Makefile          | 19 -------------------
xen/arch/x86/Makefile | 16 ++++++++++------
4 files changed, 13 insertions(+), 39 deletions(-)
[PATCH v5] xen: Strip xen.efi by default
Posted by Frediano Ziglio 1 month ago 
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>
---
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.
---
 docs/misc/efi.pandoc  |  8 +-------
 xen/Kconfig.debug     |  9 ++-------
 xen/Makefile          | 19 -------------------
 xen/arch/x86/Makefile | 16 ++++++++++------
 4 files changed, 13 insertions(+), 39 deletions(-)

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 ddcee8835c..605a26c181 100644
--- a/xen/Makefile
+++ b/xen/Makefile
@@ -493,22 +493,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))
@@ -535,9 +519,6 @@ _install: $(TARGET)$(CONFIG_XEN_INSTALL_SUFFIX)
 		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 407571c510..51d8084693 100644
--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -228,17 +228,21 @@ endif
 	$(MAKE) $(build)=$(@D) .$(@F).1r.o .$(@F).1s.o
 	$(LD) $(call EFI_LDFLAGS,$(VIRT_BASE)) -T $(obj)/efi.lds $< \
 	      $(dot-target).1r.o $(dot-target).1s.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
+ifeq ($(CONFIG_DEBUG_INFO)_$(filter --strip-debug,$(EFI_LDFLAGS)),y_)
+	$(OBJCOPY) -O elf64-x86-64 $(TARGET)-syms.efi $@.elf
+	$(STRIP) $(TARGET)-syms.efi -o $@.tmp
+else
+	mv -f $(TARGET)-syms.efi -o $@.tmp
 endif
-	rm -f $(dot-target).[0-9]* $(@D)/..$(@F).[0-9]*
 ifeq ($(CONFIG_XEN_IBT),y)
-	$(SHELL) $(srctree)/tools/check-endbr.sh $@
+	$(SHELL) $(srctree)/tools/check-endbr.sh $@.tmp
 endif
+	mv -f $@.tmp $@
+	rm -f $(dot-target).[0-9]* $(@D)/..$(@F).[0-9]*
 else
 $(TARGET).efi: FORCE
 	rm -f $@
-- 
2.43.0
Re: [PATCH v5] xen: Strip xen.efi by default
Posted by Jan Beulich 1 month ago 
On 10.11.2025 13:58, Frediano Ziglio wrote:
> --- a/xen/arch/x86/Makefile
> +++ b/xen/arch/x86/Makefile
> @@ -228,17 +228,21 @@ endif
>  	$(MAKE) $(build)=$(@D) .$(@F).1r.o .$(@F).1s.o
>  	$(LD) $(call EFI_LDFLAGS,$(VIRT_BASE)) -T $(obj)/efi.lds $< \
>  	      $(dot-target).1r.o $(dot-target).1s.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

This part if fine with me now.

> -ifeq ($(CONFIG_DEBUG_INFO),y)
> -	$(if $(filter --strip-debug,$(EFI_LDFLAGS)),:$(space))$(OBJCOPY) -O elf64-x86-64 $@ $@.elf
> +ifeq ($(CONFIG_DEBUG_INFO)_$(filter --strip-debug,$(EFI_LDFLAGS)),y_)

This extra change looks correct to me, yet I wonder if doing it this way isn't
fragile. If EFI_LDFLAGS wasn't set globally, but only for xen.efi, aiui this
wouldn't work anymore.

> +	$(OBJCOPY) -O elf64-x86-64 $(TARGET)-syms.efi $@.elf
> +	$(STRIP) $(TARGET)-syms.efi -o $@.tmp
> +else
> +	mv -f $(TARGET)-syms.efi -o $@.tmp

This, while I think I understand why you do it, looks somewhat odd. Plus the
reason you do it is, like ...

>  endif
> -	rm -f $(dot-target).[0-9]* $(@D)/..$(@F).[0-9]*
>  ifeq ($(CONFIG_XEN_IBT),y)
> -	$(SHELL) $(srctree)/tools/check-endbr.sh $@
> +	$(SHELL) $(srctree)/tools/check-endbr.sh $@.tmp
>  endif
> +	mv -f $@.tmp $@
> +	rm -f $(dot-target).[0-9]* $(@D)/..$(@F).[0-9]*
>  else
>  $(TARGET).efi: FORCE
>  	rm -f $@

... (still) most of this, unrelated (and, as before, not mentioned at all in
the description).

Jan
Re: [PATCH v5] xen: Strip xen.efi by default
Posted by Frediano Ziglio 1 month ago 
On Mon, 10 Nov 2025 at 13:40, Jan Beulich <jbeulich@suse.com> wrote:
>
> On 10.11.2025 13:58, Frediano Ziglio wrote:
> > --- a/xen/arch/x86/Makefile
> > +++ b/xen/arch/x86/Makefile
> > @@ -228,17 +228,21 @@ endif
> >       $(MAKE) $(build)=$(@D) .$(@F).1r.o .$(@F).1s.o
> >       $(LD) $(call EFI_LDFLAGS,$(VIRT_BASE)) -T $(obj)/efi.lds $< \
> >             $(dot-target).1r.o $(dot-target).1s.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
>
> This part if fine with me now.
>
> > -ifeq ($(CONFIG_DEBUG_INFO),y)
> > -     $(if $(filter --strip-debug,$(EFI_LDFLAGS)),:$(space))$(OBJCOPY) -O elf64-x86-64 $@ $@.elf
> > +ifeq ($(CONFIG_DEBUG_INFO)_$(filter --strip-debug,$(EFI_LDFLAGS)),y_)
>
> This extra change looks correct to me, yet I wonder if doing it this way isn't
> fragile. If EFI_LDFLAGS wasn't set globally, but only for xen.efi, aiui this
> wouldn't work anymore.
>

Maybe, but it's not a regression of this patch, the same test was
present before, just using another syntax

> > +     $(OBJCOPY) -O elf64-x86-64 $(TARGET)-syms.efi $@.elf
> > +     $(STRIP) $(TARGET)-syms.efi -o $@.tmp
> > +else
> > +     mv -f $(TARGET)-syms.efi -o $@.tmp
>
> This, while I think I understand why you do it, looks somewhat odd. Plus the
> reason you do it is, like ...
>

Can you explain why "it looks somewhat odd" ?

> >  endif
> > -     rm -f $(dot-target).[0-9]* $(@D)/..$(@F).[0-9]*
> >  ifeq ($(CONFIG_XEN_IBT),y)
> > -     $(SHELL) $(srctree)/tools/check-endbr.sh $@
> > +     $(SHELL) $(srctree)/tools/check-endbr.sh $@.tmp
> >  endif
> > +     mv -f $@.tmp $@
> > +     rm -f $(dot-target).[0-9]* $(@D)/..$(@F).[0-9]*
> >  else
> >  $(TARGET).efi: FORCE
> >       rm -f $@
>
> ... (still) most of this, unrelated (and, as before, not mentioned at all in
> the description).
>

I will remove it for now.

> Jan


Frediano
Re: [PATCH v5] xen: Strip xen.efi by default
Posted by Jan Beulich 1 month ago 
On 10.11.2025 16:36, Frediano Ziglio wrote:
> On Mon, 10 Nov 2025 at 13:40, Jan Beulich <jbeulich@suse.com> wrote:
>> On 10.11.2025 13:58, Frediano Ziglio wrote:
>>> --- a/xen/arch/x86/Makefile
>>> +++ b/xen/arch/x86/Makefile
>>> @@ -228,17 +228,21 @@ endif
>>>       $(MAKE) $(build)=$(@D) .$(@F).1r.o .$(@F).1s.o
>>>       $(LD) $(call EFI_LDFLAGS,$(VIRT_BASE)) -T $(obj)/efi.lds $< \
>>>             $(dot-target).1r.o $(dot-target).1s.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
>>
>> This part if fine with me now.
>>
>>> -ifeq ($(CONFIG_DEBUG_INFO),y)
>>> -     $(if $(filter --strip-debug,$(EFI_LDFLAGS)),:$(space))$(OBJCOPY) -O elf64-x86-64 $@ $@.elf
>>> +ifeq ($(CONFIG_DEBUG_INFO)_$(filter --strip-debug,$(EFI_LDFLAGS)),y_)
>>
>> This extra change looks correct to me, yet I wonder if doing it this way isn't
>> fragile. If EFI_LDFLAGS wasn't set globally, but only for xen.efi, aiui this
>> wouldn't work anymore.
> 
> Maybe, but it's not a regression of this patch, the same test was
> present before, just using another syntax

How is it not a (latent) regression? It is my understanding that the ifeq()
is processed when the Makefile is parsed, whereas the original $(if ...) was
processed as the rule is executed.

>>> +     $(OBJCOPY) -O elf64-x86-64 $(TARGET)-syms.efi $@.elf
>>> +     $(STRIP) $(TARGET)-syms.efi -o $@.tmp
>>> +else
>>> +     mv -f $(TARGET)-syms.efi -o $@.tmp
>>
>> This, while I think I understand why you do it, looks somewhat odd. Plus the
>> reason you do it is, like ...
> 
> Can you explain why "it looks somewhat odd" ?

You're moving a properly named file to a temporary one. (Just to later move
it again.)

Jan

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.