Re: [PATCH v2] xen: Strip xen.efi by default

["Marek Marczykowski-Górecki" <marmarek@invisiblethingslab.com>]

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

On Thu, Oct 02, 2025 at 02:05:56PM +0100, Andrew Cooper wrote:
> On 12/06/2025 11:07 am, Frediano Ziglio wrote:
> > For xen.gz file we strip all symbols and have an additional
> > xen-syms file version with all symbols.
> > Make xen.efi more coherent stripping all symbols too.
> > xen.efi.elf can be used for debugging.
> >
> > Signed-off-by: Frediano Ziglio <frediano.ziglio@cloud.com>

Generally,
Reviewed-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>

But this may want a line in CHANGELOG.md, just for a little more
visibility for people packaging Xen, as it may affect what should be
included in debuginfo sub-package.

> > ---
> > Changes since v1:
> > - avoid leaving target if some command fails
> 
> CC-ing the EFI maintainers, as this is an EFI change.

Thanks. I did noticed the patch independently, but only a few minutes
earlier due to missing CC...

> At the recent QubesOS hackathon, Michał Żygowski (3mdeb) found that
> stripping Xen was the difference between the system booting and not.
> 
> With debugging symbols, xen.efi was ~32M and is placed above the 4G
> boundary by the EFI loader, hitting Xen's sanity check that it's below 4G.
> 
> Xen does still have a requirement to live below the 4G boundary.  At a
> minimum, idle_pg_table needs to be addressable with a 32bit %cr3, but I
> bet that isn't the only restriction we have.
> 
> So, either we find a way of telling the EFI loader (using PE+ headers
> only) that we require to be below 4G (I have no idea if this is
> possible), or we strip xen.efi by default.
> 
> I don't think making Xen.efi safe to operate above the 4G boundary is a
> viable option at this point.
> 
> As Xen's defaults are broken on modern systems, this is also a bugfix
> candidate for 4.21, so CC Oleksii.

I agree with this wanting to be considered for 4.21.

> ~Andrew
> 
> (Retaining full patch for those CC'd into the thread)
> 
> > ---
> >  docs/misc/efi.pandoc  |  8 +-------
> >  xen/Kconfig.debug     |  9 ++-------
> >  xen/Makefile          | 19 -------------------
> >  xen/arch/x86/Makefile |  8 +++++---
> >  4 files changed, 8 insertions(+), 36 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 d14093017e..cafbb1236c 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 and xen.efi.elf
> > +	  binaries.
> >  
> >  endmenu
> > diff --git a/xen/Makefile b/xen/Makefile
> > index 8fc4e042ff..664c4ea7b8 100644
> > --- a/xen/Makefile
> > +++ b/xen/Makefile
> > @@ -488,22 +488,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))
> > @@ -530,9 +514,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 ce724a9daa..e0ebc8c73e 100644
> > --- a/xen/arch/x86/Makefile
> > +++ b/xen/arch/x86/Makefile
> > @@ -232,14 +232,16 @@ 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 $@.tmp
> > +	$(NM) -pa --format=sysv $@.tmp \
> >  		| $(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 $@.tmp $@.elf
> > +	$(if $(filter --strip-debug,$(EFI_LDFLAGS)),:$(space))$(STRIP) $@.tmp
> >  endif
> >  	rm -f $(dot-target).[0-9]* $(@D)/..$(@F).[0-9]*
> > +	mv -f $@.tmp $@
> >  ifeq ($(CONFIG_XEN_IBT),y)
> >  	$(SHELL) $(srctree)/tools/check-endbr.sh $@
> >  endif
> 

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab

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