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

[Frediano Ziglio <freddy77@xxxxxxxxx>]

On Thu, 6 Nov 2025 at 10:32, Jan Beulich <jbeulich@xxxxxxxx> wrote:
>
> On 05.11.2025 16:38, Frediano Ziglio wrote:
> > From: Frediano Ziglio <frediano.ziglio@xxxxxxxxx>
> >
> > 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-syms.efi can be used for debugging.
> >
> > Signed-off-by: Frediano Ziglio <frediano.ziglio@xxxxxxxxx>
> > ---
> > 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.
> > ---
> >  docs/misc/efi.pandoc  |  8 +-------
> >  xen/Kconfig.debug     |  9 ++-------
> >  xen/Makefile          | 19 -------------------
> >  xen/arch/x86/Makefile |  9 ++++++---
> >  4 files changed, 9 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 d900d926c5..58ee10ee3e 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.
>
> Why xen.efi.elf and not xen-syms.efi?
>

I forgot to update this part.
Now that I see the comment, was the suggestion about having an
additional xen-syms.efi file or having xen-syms.efi instead of
xen.efi.elf ?

> > --- a/xen/arch/x86/Makefile
> > +++ b/xen/arch/x86/Makefile
> > @@ -228,14 +228,17 @@ 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))cp -f \
> > +        $@.tmp $(TARGET)-syms.efi
> > +     $(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
>
> I see $@.tmp here, but no sign of xen-syms. Did you submit a stake patch? Am
> I missing something?
>

I don't know what a "stake patch" is.
xen-syms.efi (that is $(TARGET)-syms.efi in the Makefile) is not a
target of this rule so if the rule fails it will be generated again.

> I also think the mv should sit ahead of the cleaning-up rm.
>

Are you sure?
Usually you want it as the last command so any failure won't create
the target. For instance here if check-endbr.sh is failing the target
is still created and next make command will succeed.

> Jan
>
> Jan

Frediano