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 <freddy77@gmail.com>
---
 docs/misc/efi.pandoc  |  8 +-------
 xen/Kconfig.debug     |  9 ++-------
 xen/Makefile          | 19 -------------------
 xen/arch/x86/Makefile |  1 +
 4 files changed, 4 insertions(+), 33 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..725bcef340 100644
--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -238,6 +238,7 @@ endif
 		> $@.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))$(STRIP) $@
 endif
 	rm -f $(dot-target).[0-9]* $(@D)/..$(@F).[0-9]*
 ifeq ($(CONFIG_XEN_IBT),y)
-- 
2.43.0

On 10.06.2025 12:12, Frediano Ziglio wrote:
> --- a/xen/arch/x86/Makefile
> +++ b/xen/arch/x86/Makefile
> @@ -238,6 +238,7 @@ endif
>  		> $@.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))$(STRIP) $@
>  endif
>  	rm -f $(dot-target).[0-9]* $(@D)/..$(@F).[0-9]*
>  ifeq ($(CONFIG_XEN_IBT),y)

Oh, one more thing: It is bad practice to modify the output of a rule multiple
times. An interrupted or otherwise failed make (at the "right" position) may
then leave something in place which a subsequent incremental build would not
know needs updating (again).

Jan

On 10.06.2025 12:12, 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.

And the other difference (compressed vs not) still remains.

> xen.efi.elf can be used for debugging.

Hmm, that's the result of ...

> --- a/xen/arch/x86/Makefile
> +++ b/xen/arch/x86/Makefile
> @@ -238,6 +238,7 @@ endif
>  		> $@.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))$(STRIP) $@
>  endif
>  	rm -f $(dot-target).[0-9]* $(@D)/..$(@F).[0-9]*
>  ifeq ($(CONFIG_XEN_IBT),y)

... objcopy. Having looked at the involved code in that utility, I mistrust its
output from such a conversion to really be an exact representation of the input.
IOW I'd much rather use the original file. As a possible compromise, could we
perhaps merely strip debug info, but retain the symbol table, matching the
prior default for $(efi-strip-opt)?

Jan

On 6/11/25 05:35, Jan Beulich wrote:
> On 10.06.2025 12:12, 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.
> 
> And the other difference (compressed vs not) still remains.
> 
>> xen.efi.elf can be used for debugging.
> 
> Hmm, that's the result of ...
> 
>> --- a/xen/arch/x86/Makefile
>> +++ b/xen/arch/x86/Makefile
>> @@ -238,6 +238,7 @@ endif
>>  		> $@.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))$(STRIP) $@
>>  endif
>>  	rm -f $(dot-target).[0-9]* $(@D)/..$(@F).[0-9]*
>>  ifeq ($(CONFIG_XEN_IBT),y)
> 
> ... objcopy. Having looked at the involved code in that utility, I mistrust its
> output from such a conversion to really be an exact representation of the input.
> IOW I'd much rather use the original file. As a possible compromise, could we
> perhaps merely strip debug info, but retain the symbol table, matching the
> prior default for $(efi-strip-opt)?

The string table and all debug symbols need to be stripped.  This means no
sections longer than 8 bytes and no debug information at all.  This is a
requirement of the Portable Executable specification, and I really do not
want this to be messed with given UEFI Secure Boot requirements.
-- 
Sincerely,
Demi Marie Obenour (she/her/hers)

On Wed, Jun 11, 2025 at 10:35 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 10.06.2025 12:12, 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.
>
> And the other difference (compressed vs not) still remains.
>
> > xen.efi.elf can be used for debugging.
>
> Hmm, that's the result of ...
>
> > --- a/xen/arch/x86/Makefile
> > +++ b/xen/arch/x86/Makefile
> > @@ -238,6 +238,7 @@ endif
> >               > $@.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))$(STRIP) $@
> >  endif
> >       rm -f $(dot-target).[0-9]* $(@D)/..$(@F).[0-9]*
> >  ifeq ($(CONFIG_XEN_IBT),y)
>
> ... objcopy. Having looked at the involved code in that utility, I mistrust its
> output from such a conversion to really be an exact representation of the input.

From https://xenbits.xen.org/gitweb/?p=xen.git;a=commit;f=xen/arch/x86/Makefile;h=f514bab30ef8d4ade77a27c926e283c9bbbf9ffd,

    Today it is not possible to analyse crash dumps of a system in
    hypervisor mode when it had been booted via EFI, as the crash utility
    doesn't understand the file format of xen.efi.

    This can easily be solved by creating an ELF file from xen.efi via
    objcopy. Using that file as name list for crash enables the user to
    analyse the dump in hypervisor mode. Note that crash isn't happy with
    a file containing no text and data, so using --only-keep-debug is not
    an option.

Isn't that true anymore?

> IOW I'd much rather use the original file. As a possible compromise, could we
> perhaps merely strip debug info, but retain the symbol table, matching the
> prior default for $(efi-strip-opt)?

Not clear the compromise you are intending here, the debug file has
all information available, why do you need an intermediate one? We
strip everything for ELF, why not be coherent?

>
> Jan
>

Frediano

On 11.06.2025 12:49, Frediano Ziglio wrote:
> On Wed, Jun 11, 2025 at 10:35 AM Jan Beulich <jbeulich@suse.com> wrote:
>>
>> On 10.06.2025 12:12, 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.
>>
>> And the other difference (compressed vs not) still remains.
>>
>>> xen.efi.elf can be used for debugging.
>>
>> Hmm, that's the result of ...
>>
>>> --- a/xen/arch/x86/Makefile
>>> +++ b/xen/arch/x86/Makefile
>>> @@ -238,6 +238,7 @@ endif
>>>               > $@.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))$(STRIP) $@
>>>  endif
>>>       rm -f $(dot-target).[0-9]* $(@D)/..$(@F).[0-9]*
>>>  ifeq ($(CONFIG_XEN_IBT),y)
>>
>> ... objcopy. Having looked at the involved code in that utility, I mistrust its
>> output from such a conversion to really be an exact representation of the input.
> 
> From https://xenbits.xen.org/gitweb/?p=xen.git;a=commit;f=xen/arch/x86/Makefile;h=f514bab30ef8d4ade77a27c926e283c9bbbf9ffd,
> 
>     Today it is not possible to analyse crash dumps of a system in
>     hypervisor mode when it had been booted via EFI, as the crash utility
>     doesn't understand the file format of xen.efi.
> 
>     This can easily be solved by creating an ELF file from xen.efi via
>     objcopy. Using that file as name list for crash enables the user to
>     analyse the dump in hypervisor mode. Note that crash isn't happy with
>     a file containing no text and data, so using --only-keep-debug is not
>     an option.
> 
> Isn't that true anymore?

That's likely as true as before - heavily depending on objcopy not screwing up.
I don't think any thorough analysis was done back when that commit was put
together.

>> IOW I'd much rather use the original file. As a possible compromise, could we
>> perhaps merely strip debug info, but retain the symbol table, matching the
>> prior default for $(efi-strip-opt)?
> 
> Not clear the compromise you are intending here, the debug file has
> all information available, why do you need an intermediate one? We
> strip everything for ELF, why not be coherent?

If I want to e.g. see the disassembly of xen.efi, I find it heavily misleading
that I then would have to disassemble xen.efi.elf instead. One can get used to
it, sure. But for xen.gz / xen-syms it just was that way from the very
beginning (plus it's kind of obvious that a *.gz file cannot directly be
disassembled anyway). And for disassembly, the loss of debug info is far less
impactful than the loss of the symbol table. (Hence also why only debug info
was being stripped during installation so far.)

Jan

On 10/06/2025 11:12 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.

Agreed.  What Xen previous had violates the principle of lease surprise.

> Signed-off-by: Frediano Ziglio <freddy77@gmail.com>

Did you intend to send this patch personally?

~Andrew

On 10.06.2025 12:17, Andrew Cooper wrote:
> On 10/06/2025 11:12 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.
> 
> Agreed.  What Xen previous had violates the principle of lease surprise.

Except there was a reason for this: While you can point e.g. debuggers at
the file with symbol table and debug info for the ELF image, I have been
unaware of similar abilities when it comes to PE/COFF binaries in an
otherwise ELF world.

Jan

On Tue, Jun 10, 2025 at 11:17 AM Andrew Cooper
<andrew.cooper3@citrix.com> wrote:
>
> On 10/06/2025 11:12 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.
>
> Agreed.  What Xen previous had violates the principle of lease surprise.
>
> > Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
>
> Did you intend to send this patch personally?
>

I noted. No, it was developed as part of CSG work. I think some
configuration left from the time we had issues with the email.
Still me, so the signed off should be okay. Should I update and send it again?

> ~Andrew
>
Frediano