xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3] xen: Strip xen.efi by default
@ 2025-11-05 15:38 Frediano Ziglio
  2025-11-05 20:31 ` Demi Marie Obenour
  2025-11-06 10:32 ` Jan Beulich
  0 siblings, 2 replies; 14+ messages in thread
From: Frediano Ziglio @ 2025-11-05 15:38 UTC (permalink / raw)
  To: xen-devel
  Cc: Frediano Ziglio, Andrew Cooper, Anthony PERARD, Michal Orzel,
	Jan Beulich, Julien Grall, Roger Pau Monné,
	Stefano Stabellini, Frediano Ziglio

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

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@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.
---
 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.
 
 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..c118ab7b7d 100644
--- 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
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH v3] xen: Strip xen.efi by default
  2025-11-05 15:38 [PATCH v3] xen: Strip xen.efi by default Frediano Ziglio
@ 2025-11-05 20:31 ` Demi Marie Obenour
  2025-11-06  2:00   ` Frediano Ziglio
  2025-11-06 10:32 ` Jan Beulich
  1 sibling, 1 reply; 14+ messages in thread
From: Demi Marie Obenour @ 2025-11-05 20:31 UTC (permalink / raw)
  To: Frediano Ziglio, xen-devel
  Cc: Frediano Ziglio, Andrew Cooper, Anthony PERARD, Michal Orzel,
	Jan Beulich, Julien Grall, Roger Pau Monné,
	Stefano Stabellini, Frediano Ziglio


[-- Attachment #1.1.1: Type: text/plain, Size: 6208 bytes --]

On 11/5/25 10:38, Frediano Ziglio wrote:
> From: Frediano Ziglio <frediano.ziglio@cloud.com>
> 
> 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@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.
> ---
>  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.
>  
>  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..c118ab7b7d 100644
> --- 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

Does this also strip the string table from xen.efi?  I'm concerned that
signing xen.efi for secure boot won't work if there is a string table.
In particular, it appears that EDK2 will miscalculate the file hash if
the string table is before the signature.  Moving the string table after
the signature invalidates the pointer to it.  The only exception is if
the string table is itself in a section, but I don't know if that is the
case.
-- 
Sincerely,
Demi Marie Obenour (she/her/hers)

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 7253 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3] xen: Strip xen.efi by default
  2025-11-05 20:31 ` Demi Marie Obenour
@ 2025-11-06  2:00   ` Frediano Ziglio
  2025-11-06  3:52     ` Demi Marie Obenour
  0 siblings, 1 reply; 14+ messages in thread
From: Frediano Ziglio @ 2025-11-06  2:00 UTC (permalink / raw)
  To: Demi Marie Obenour
  Cc: Frediano Ziglio, xen-devel, Frediano Ziglio, Andrew Cooper,
	Anthony PERARD, Michal Orzel, Jan Beulich, Julien Grall,
	Roger Pau Monné, Stefano Stabellini,
	Marek Marczykowski-Górecki

On Wed, 5 Nov 2025 at 20:31, Demi Marie Obenour <demiobenour@gmail.com> wrote:
>
> On 11/5/25 10:38, Frediano Ziglio wrote:
> > From: Frediano Ziglio <frediano.ziglio@cloud.com>
> >
> > 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@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.
> > ---
> >  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.
> >
> >  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..c118ab7b7d 100644
> > --- 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
>
> Does this also strip the string table from xen.efi?  I'm concerned that
> signing xen.efi for secure boot won't work if there is a string table.
> In particular, it appears that EDK2 will miscalculate the file hash if
> the string table is before the signature.  Moving the string table after
> the signature invalidates the pointer to it.  The only exception is if
> the string table is itself in a section, but I don't know if that is the
> case.

I don't know if the string table is stripped but I can surely confirm
that signing xen.efi is working with secure boot.

Frediano


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3] xen: Strip xen.efi by default
  2025-11-06  2:00   ` Frediano Ziglio
@ 2025-11-06  3:52     ` Demi Marie Obenour
  2025-11-06  9:58       ` Frediano Ziglio
  0 siblings, 1 reply; 14+ messages in thread
From: Demi Marie Obenour @ 2025-11-06  3:52 UTC (permalink / raw)
  To: Frediano Ziglio
  Cc: Frediano Ziglio, xen-devel, Frediano Ziglio, Andrew Cooper,
	Anthony PERARD, Michal Orzel, Jan Beulich, Julien Grall,
	Roger Pau Monné, Stefano Stabellini,
	Marek Marczykowski-Górecki


[-- Attachment #1.1.1: Type: text/plain, Size: 7063 bytes --]

On 11/5/25 21:00, Frediano Ziglio wrote:
> On Wed, 5 Nov 2025 at 20:31, Demi Marie Obenour <demiobenour@gmail.com> wrote:
>>
>> On 11/5/25 10:38, Frediano Ziglio wrote:
>>> From: Frediano Ziglio <frediano.ziglio@cloud.com>
>>>
>>> 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@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.
>>> ---
>>>  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.
>>>
>>>  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..c118ab7b7d 100644
>>> --- 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
>>
>> Does this also strip the string table from xen.efi?  I'm concerned that
>> signing xen.efi for secure boot won't work if there is a string table.
>> In particular, it appears that EDK2 will miscalculate the file hash if
>> the string table is before the signature.  Moving the string table after
>> the signature invalidates the pointer to it.  The only exception is if
>> the string table is itself in a section, but I don't know if that is the
>> case.
> 
> I don't know if the string table is stripped but I can surely confirm
> that signing xen.efi is working with secure boot.
> 
> Frediano

Does objdump on the signed file return correct section names?
-- 
Sincerely,
Demi Marie Obenour (she/her/hers)

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 7253 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3] xen: Strip xen.efi by default
  2025-11-06  3:52     ` Demi Marie Obenour
@ 2025-11-06  9:58       ` Frediano Ziglio
  2025-11-06 10:28         ` Jan Beulich
  0 siblings, 1 reply; 14+ messages in thread
From: Frediano Ziglio @ 2025-11-06  9:58 UTC (permalink / raw)
  To: Demi Marie Obenour
  Cc: Frediano Ziglio, xen-devel, Frediano Ziglio, Andrew Cooper,
	Anthony PERARD, Michal Orzel, Jan Beulich, Julien Grall,
	Roger Pau Monné, Stefano Stabellini,
	Marek Marczykowski-Górecki

On Thu, 6 Nov 2025 at 03:52, Demi Marie Obenour <demiobenour@gmail.com> wrote:
>
> On 11/5/25 21:00, Frediano Ziglio wrote:
> > On Wed, 5 Nov 2025 at 20:31, Demi Marie Obenour <demiobenour@gmail.com> wrote:
> >>
> >> On 11/5/25 10:38, Frediano Ziglio wrote:
> >>> From: Frediano Ziglio <frediano.ziglio@cloud.com>
> >>>
> >>> 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@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.
> >>> ---
> >>>  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.
> >>>
> >>>  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..c118ab7b7d 100644
> >>> --- 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
> >>
> >> Does this also strip the string table from xen.efi?  I'm concerned that
> >> signing xen.efi for secure boot won't work if there is a string table.
> >> In particular, it appears that EDK2 will miscalculate the file hash if
> >> the string table is before the signature.  Moving the string table after
> >> the signature invalidates the pointer to it.  The only exception is if
> >> the string table is itself in a section, but I don't know if that is the
> >> case.
> >
> > I don't know if the string table is stripped but I can surely confirm
> > that signing xen.efi is working with secure boot.
> >
> > Frediano
>
> Does objdump on the signed file return correct section names?

From objdump -x

Sections:
Idx Name          Size      VMA               LMA               File off  Algn
  0 .text         0016c9ae  ffff82d040200000  ffff82d040200000  00000320  2**4
                  CONTENTS, ALLOC, LOAD, READONLY, CODE
  1 .rodata       0006b9e8  ffff82d040400000  ffff82d040400000  0016cce0  2**2
                  CONTENTS, ALLOC, LOAD, DATA
  2 .buildid      00000035  ffff82d04046c000  ffff82d04046c000  001d86e0  2**2
                  CONTENTS, ALLOC, LOAD, READONLY, DATA
  3 .init.text    0004d123  ffff82d040600000  ffff82d040600000  001d8720  2**2
                  CONTENTS, ALLOC, LOAD, READONLY, CODE
  4 .init.data    0006c9b0  ffff82d040800000  ffff82d040800000  00225860  2**2
                  CONTENTS, ALLOC, LOAD, DATA
  5 .data.read_mostly 00028da8  ffff82d040a00000  ffff82d040a00000
00292220  2**4
                  CONTENTS, ALLOC, LOAD, DATA
  6 .data         0000feec  ffff82d040a29000  ffff82d040a29000  002bafe0  2**4
                  CONTENTS, ALLOC, LOAD, DATA
  7 .bss          00223108  ffff82d040a39000  ffff82d040a39000  00000000  2**4
                  ALLOC
  8 .reloc        000016b8  ffff82d040c5d000  ffff82d040c5d000  002caee0  2**2
                  CONTENTS, ALLOC, LOAD, READONLY, DATA
  9 .sbat         000000a6  ffff82d040c5f000  ffff82d040c5f000  002cc5a0  2**2
                  CONTENTS, READONLY

Which looks correct.

From hexdump -C I can see close to the end

...
002cc580  30 ae 38 ae 60 ae 00 00  00 80 a3 00 10 00 00 00  |0.8.`...........|
002cc590  a0 ae c0 ae e0 ae 00 00  00 00 00 00 00 00 00 00  |................|
002cc5a0  73 62 61 74 2c 31 2c 53  42 41 54 20 56 65 72 73  |sbat,1,SBAT Vers|
002cc5b0  69 6f 6e 2c 73 62 61 74  2c 31 2c 68 74 74 70 73  |ion,sbat,1,https|
002cc5c0  3a 2f 2f 67 69 74 68 75  62 2e 63 6f 6d 2f 72 68  |://github.com/rh|
002cc5d0  62 6f 6f 74 2f 73 68 69  6d 2f 62 6c 6f 62 2f 6d  |boot/shim/blob/m|
002cc5e0  61 69 6e 2f 53 42 41 54  2e 6d 64 0a 78 65 6e 2e  |ain/SBAT.md.xen.|
002cc5f0  78 73 2c 31 2c 43 6c 6f  75 64 20 53 6f 66 74 77  |xs,1,Cloud Softw|
002cc600  61 72 65 20 47 72 6f 75  70 2c 78 65 6e 2c 34 2e  |are Group,xen,4.|
002cc610  32 30 2e 31 2d 37 2e 32  32 2e 67 33 65 30 36 37  |20.1-7.22.g3e067|
002cc620  32 36 62 2e 78 73 39 2c  6d 61 69 6c 74 6f 3a 73  |26b.xs9,mailto:s|
002cc630  65 63 75 72 69 74 79 40  78 65 6e 73 65 72 76 65  |ecurity@xenserve|
002cc640  72 2e 63 6f 6d 0a 00 00  00 00 00 00 00 00 00 00  |r.com...........|
002cc650  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
002cc660  2c 00 00 00 2e 69 6e 69  74 2e 74 65 78 74 00 2e  |,....init.text..|
002cc670  69 6e 69 74 2e 64 61 74  61 00 2e 64 61 74 61 2e  |init.data..data.|
002cc680  72 65 61 64 5f 6d 6f 73  74 6c 79 00 00 00 00 00  |read_mostly.....|
002cc690  9e 05 00 00 00 02 02 00  30 82 05 92 06 09 2a 86  |........0.....*.|
002cc6a0  48 86 f7 0d 01 07 02 a0  82 05 83 30 82 05 7f 02  |H..........0....|
002cc6b0  01 01 31 0f 30 0d 06 09  60 86 48 01 65 03 04 02  |..1.0...`.H.e...|
002cc6c0  01 05 00 30 5c 06 0a 2b  06 01 04 01 82 37 02 01  |...0\..+.....7..|
002cc6d0  04 a0 4e 30 4c 30 17 06  0a 2b 06 01 04 01 82 37  |..N0L0...+.....7|
002cc6e0  02 01 0f 30 09 03 01 00  a0 04 a2 02 80 00 30 31  |...0..........01|
002cc6f0  30 0d 06 09 60 86 48 01  65 03 04 02 01 05 00 04  |0...`.H.e.......|
002cc700  20 e2 47 64 f8 e8 7b 62  eb 17 e0 13 0a 0d 93 02  | .Gd..{b........|
002cc710  7a d8 3b f0 20 a8 ee 3d  49 98 3f de c1 47 de 15  |z.;. ..=I.?..G..|
002cc720  43 a0 82 03 2c 30 82 03  28 30 82 02 10 a0 03 02  |C...,0..(0......|
002cc730  01 02 02 11 00 8f fc 11  bf 41 54 40 74 89 2c 53  |.........AT@t.,S|
002cc740  a5 78 c1 e8 32 30 0d 06  09 2a 86 48 86 f7 0d 01  |.x..20...*.H....|
002cc750  01 0b 05 00 30 1c 31 1a  30 18 06 03 55 04 03 13  |....0.1.0...U...|
002cc760  11 58 65 6e 53 65 72 76  65 72 20 58 65 6e 20 64  |.XenServer Xen d|
002cc770  65 76 30 1e 17 0d 32 35  30 33 32 30 31 36 35 35  |ev0...2503201655|
002cc780  30 37 5a 17 0d 33 37 30  31 31 39 30 33 31 34 30  |07Z..37011903140|
002cc790  37 5a 30 1c 31 1a 30 18  06 03 55 04 03 13 11 58  |7Z0.1.0...U....X|
002cc7a0  65 6e 53 65 72 76 65 72  20 58 65 6e 20 64 65 76  |enServer Xen dev|
...

So, this confirms that the string table is there to support larger
section names and the signature is there and it's working.

--
Frediano


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3] xen: Strip xen.efi by default
  2025-11-06  9:58       ` Frediano Ziglio
@ 2025-11-06 10:28         ` Jan Beulich
  2025-11-06 10:40           ` Demi Marie Obenour
  2025-11-06 16:32           ` Frediano Ziglio
  0 siblings, 2 replies; 14+ messages in thread
From: Jan Beulich @ 2025-11-06 10:28 UTC (permalink / raw)
  To: Frediano Ziglio
  Cc: xen-devel, Frediano Ziglio, Andrew Cooper, Anthony PERARD,
	Michal Orzel, Julien Grall, Roger Pau Monné,
	Stefano Stabellini, Marek Marczykowski-Górecki,
	Demi Marie Obenour

On 06.11.2025 10:58, Frediano Ziglio wrote:
> On Thu, 6 Nov 2025 at 03:52, Demi Marie Obenour <demiobenour@gmail.com> wrote:
>> Does objdump on the signed file return correct section names?
> 
> From objdump -x
> 
> Sections:
> Idx Name          Size      VMA               LMA               File off  Algn
>   0 .text         0016c9ae  ffff82d040200000  ffff82d040200000  00000320  2**4
>                   CONTENTS, ALLOC, LOAD, READONLY, CODE
>   1 .rodata       0006b9e8  ffff82d040400000  ffff82d040400000  0016cce0  2**2
>                   CONTENTS, ALLOC, LOAD, DATA
>   2 .buildid      00000035  ffff82d04046c000  ffff82d04046c000  001d86e0  2**2
>                   CONTENTS, ALLOC, LOAD, READONLY, DATA
>   3 .init.text    0004d123  ffff82d040600000  ffff82d040600000  001d8720  2**2
>                   CONTENTS, ALLOC, LOAD, READONLY, CODE
>   4 .init.data    0006c9b0  ffff82d040800000  ffff82d040800000  00225860  2**2
>                   CONTENTS, ALLOC, LOAD, DATA
>   5 .data.read_mostly 00028da8  ffff82d040a00000  ffff82d040a00000
> 00292220  2**4
>                   CONTENTS, ALLOC, LOAD, DATA
>   6 .data         0000feec  ffff82d040a29000  ffff82d040a29000  002bafe0  2**4
>                   CONTENTS, ALLOC, LOAD, DATA
>   7 .bss          00223108  ffff82d040a39000  ffff82d040a39000  00000000  2**4
>                   ALLOC
>   8 .reloc        000016b8  ffff82d040c5d000  ffff82d040c5d000  002caee0  2**2
>                   CONTENTS, ALLOC, LOAD, READONLY, DATA
>   9 .sbat         000000a6  ffff82d040c5f000  ffff82d040c5f000  002cc5a0  2**2
>                   CONTENTS, READONLY
> 
> Which looks correct.
> 
> From hexdump -C I can see close to the end
> 
> ...
> 002cc580  30 ae 38 ae 60 ae 00 00  00 80 a3 00 10 00 00 00  |0.8.`...........|
> 002cc590  a0 ae c0 ae e0 ae 00 00  00 00 00 00 00 00 00 00  |................|
> 002cc5a0  73 62 61 74 2c 31 2c 53  42 41 54 20 56 65 72 73  |sbat,1,SBAT Vers|
> 002cc5b0  69 6f 6e 2c 73 62 61 74  2c 31 2c 68 74 74 70 73  |ion,sbat,1,https|
> 002cc5c0  3a 2f 2f 67 69 74 68 75  62 2e 63 6f 6d 2f 72 68  |://github.com/rh|
> 002cc5d0  62 6f 6f 74 2f 73 68 69  6d 2f 62 6c 6f 62 2f 6d  |boot/shim/blob/m|
> 002cc5e0  61 69 6e 2f 53 42 41 54  2e 6d 64 0a 78 65 6e 2e  |ain/SBAT.md.xen.|
> 002cc5f0  78 73 2c 31 2c 43 6c 6f  75 64 20 53 6f 66 74 77  |xs,1,Cloud Softw|
> 002cc600  61 72 65 20 47 72 6f 75  70 2c 78 65 6e 2c 34 2e  |are Group,xen,4.|
> 002cc610  32 30 2e 31 2d 37 2e 32  32 2e 67 33 65 30 36 37  |20.1-7.22.g3e067|
> 002cc620  32 36 62 2e 78 73 39 2c  6d 61 69 6c 74 6f 3a 73  |26b.xs9,mailto:s|
> 002cc630  65 63 75 72 69 74 79 40  78 65 6e 73 65 72 76 65  |ecurity@xenserve|
> 002cc640  72 2e 63 6f 6d 0a 00 00  00 00 00 00 00 00 00 00  |r.com...........|
> 002cc650  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
> 002cc660  2c 00 00 00 2e 69 6e 69  74 2e 74 65 78 74 00 2e  |,....init.text..|
> 002cc670  69 6e 69 74 2e 64 61 74  61 00 2e 64 61 74 61 2e  |init.data..data.|
> 002cc680  72 65 61 64 5f 6d 6f 73  74 6c 79 00 00 00 00 00  |read_mostly.....|
> 002cc690  9e 05 00 00 00 02 02 00  30 82 05 92 06 09 2a 86  |........0.....*.|
> 002cc6a0  48 86 f7 0d 01 07 02 a0  82 05 83 30 82 05 7f 02  |H..........0....|
> 002cc6b0  01 01 31 0f 30 0d 06 09  60 86 48 01 65 03 04 02  |..1.0...`.H.e...|
> 002cc6c0  01 05 00 30 5c 06 0a 2b  06 01 04 01 82 37 02 01  |...0\..+.....7..|
> 002cc6d0  04 a0 4e 30 4c 30 17 06  0a 2b 06 01 04 01 82 37  |..N0L0...+.....7|
> 002cc6e0  02 01 0f 30 09 03 01 00  a0 04 a2 02 80 00 30 31  |...0..........01|
> 002cc6f0  30 0d 06 09 60 86 48 01  65 03 04 02 01 05 00 04  |0...`.H.e.......|
> 002cc700  20 e2 47 64 f8 e8 7b 62  eb 17 e0 13 0a 0d 93 02  | .Gd..{b........|
> 002cc710  7a d8 3b f0 20 a8 ee 3d  49 98 3f de c1 47 de 15  |z.;. ..=I.?..G..|
> 002cc720  43 a0 82 03 2c 30 82 03  28 30 82 02 10 a0 03 02  |C...,0..(0......|
> 002cc730  01 02 02 11 00 8f fc 11  bf 41 54 40 74 89 2c 53  |.........AT@t.,S|
> 002cc740  a5 78 c1 e8 32 30 0d 06  09 2a 86 48 86 f7 0d 01  |.x..20...*.H....|
> 002cc750  01 0b 05 00 30 1c 31 1a  30 18 06 03 55 04 03 13  |....0.1.0...U...|
> 002cc760  11 58 65 6e 53 65 72 76  65 72 20 58 65 6e 20 64  |.XenServer Xen d|
> 002cc770  65 76 30 1e 17 0d 32 35  30 33 32 30 31 36 35 35  |ev0...2503201655|
> 002cc780  30 37 5a 17 0d 33 37 30  31 31 39 30 33 31 34 30  |07Z..37011903140|
> 002cc790  37 5a 30 1c 31 1a 30 18  06 03 55 04 03 13 11 58  |7Z0.1.0...U....X|
> 002cc7a0  65 6e 53 65 72 76 65 72  20 58 65 6e 20 64 65 76  |enServer Xen dev|
> ...
> 
> So, this confirms that the string table is there to support larger
> section names and the signature is there and it's working.

But is it going to work on all EFI implementations, or merely the one you tried?
Of course it would help if Demi could give more concrete pointers to (possible)
implementations where there might be (known? suspected?) issues.

Jan


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3] xen: Strip xen.efi by default
  2025-11-05 15:38 [PATCH v3] xen: Strip xen.efi by default Frediano Ziglio
  2025-11-05 20:31 ` Demi Marie Obenour
@ 2025-11-06 10:32 ` Jan Beulich
  2025-11-06 16:37   ` Frediano Ziglio
  1 sibling, 1 reply; 14+ messages in thread
From: Jan Beulich @ 2025-11-06 10:32 UTC (permalink / raw)
  To: Frediano Ziglio
  Cc: Andrew Cooper, Anthony PERARD, Michal Orzel, Julien Grall,
	Roger Pau Monné, Stefano Stabellini, xen-devel,
	Frediano Ziglio

On 05.11.2025 16:38, Frediano Ziglio wrote:
> From: Frediano Ziglio <frediano.ziglio@cloud.com>
> 
> 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@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.
> ---
>  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?

> --- 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 also think the mv should sit ahead of the cleaning-up rm.

Jan

Jan


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3] xen: Strip xen.efi by default
  2025-11-06 10:28         ` Jan Beulich
@ 2025-11-06 10:40           ` Demi Marie Obenour
  2025-11-06 16:32           ` Frediano Ziglio
  1 sibling, 0 replies; 14+ messages in thread
From: Demi Marie Obenour @ 2025-11-06 10:40 UTC (permalink / raw)
  To: Jan Beulich, Frediano Ziglio
  Cc: xen-devel, Frediano Ziglio, Andrew Cooper, Anthony PERARD,
	Michal Orzel, Julien Grall, Roger Pau Monné,
	Stefano Stabellini, Marek Marczykowski-Górecki


[-- Attachment #1.1.1: Type: text/plain, Size: 5463 bytes --]

On 11/6/25 05:28, Jan Beulich wrote:
> On 06.11.2025 10:58, Frediano Ziglio wrote:
>> On Thu, 6 Nov 2025 at 03:52, Demi Marie Obenour <demiobenour@gmail.com> wrote:
>>> Does objdump on the signed file return correct section names?
>>
>> From objdump -x
>>
>> Sections:
>> Idx Name          Size      VMA               LMA               File off  Algn
>>   0 .text         0016c9ae  ffff82d040200000  ffff82d040200000  00000320  2**4
>>                   CONTENTS, ALLOC, LOAD, READONLY, CODE
>>   1 .rodata       0006b9e8  ffff82d040400000  ffff82d040400000  0016cce0  2**2
>>                   CONTENTS, ALLOC, LOAD, DATA
>>   2 .buildid      00000035  ffff82d04046c000  ffff82d04046c000  001d86e0  2**2
>>                   CONTENTS, ALLOC, LOAD, READONLY, DATA
>>   3 .init.text    0004d123  ffff82d040600000  ffff82d040600000  001d8720  2**2
>>                   CONTENTS, ALLOC, LOAD, READONLY, CODE
>>   4 .init.data    0006c9b0  ffff82d040800000  ffff82d040800000  00225860  2**2
>>                   CONTENTS, ALLOC, LOAD, DATA
>>   5 .data.read_mostly 00028da8  ffff82d040a00000  ffff82d040a00000
>> 00292220  2**4
>>                   CONTENTS, ALLOC, LOAD, DATA
>>   6 .data         0000feec  ffff82d040a29000  ffff82d040a29000  002bafe0  2**4
>>                   CONTENTS, ALLOC, LOAD, DATA
>>   7 .bss          00223108  ffff82d040a39000  ffff82d040a39000  00000000  2**4
>>                   ALLOC
>>   8 .reloc        000016b8  ffff82d040c5d000  ffff82d040c5d000  002caee0  2**2
>>                   CONTENTS, ALLOC, LOAD, READONLY, DATA
>>   9 .sbat         000000a6  ffff82d040c5f000  ffff82d040c5f000  002cc5a0  2**2
>>                   CONTENTS, READONLY
>>
>> Which looks correct.
>>
>> From hexdump -C I can see close to the end
>>
>> ...
>> 002cc580  30 ae 38 ae 60 ae 00 00  00 80 a3 00 10 00 00 00  |0.8.`...........|
>> 002cc590  a0 ae c0 ae e0 ae 00 00  00 00 00 00 00 00 00 00  |................|
>> 002cc5a0  73 62 61 74 2c 31 2c 53  42 41 54 20 56 65 72 73  |sbat,1,SBAT Vers|
>> 002cc5b0  69 6f 6e 2c 73 62 61 74  2c 31 2c 68 74 74 70 73  |ion,sbat,1,https|
>> 002cc5c0  3a 2f 2f 67 69 74 68 75  62 2e 63 6f 6d 2f 72 68  |://github.com/rh|
>> 002cc5d0  62 6f 6f 74 2f 73 68 69  6d 2f 62 6c 6f 62 2f 6d  |boot/shim/blob/m|
>> 002cc5e0  61 69 6e 2f 53 42 41 54  2e 6d 64 0a 78 65 6e 2e  |ain/SBAT.md.xen.|
>> 002cc5f0  78 73 2c 31 2c 43 6c 6f  75 64 20 53 6f 66 74 77  |xs,1,Cloud Softw|
>> 002cc600  61 72 65 20 47 72 6f 75  70 2c 78 65 6e 2c 34 2e  |are Group,xen,4.|
>> 002cc610  32 30 2e 31 2d 37 2e 32  32 2e 67 33 65 30 36 37  |20.1-7.22.g3e067|
>> 002cc620  32 36 62 2e 78 73 39 2c  6d 61 69 6c 74 6f 3a 73  |26b.xs9,mailto:s|
>> 002cc630  65 63 75 72 69 74 79 40  78 65 6e 73 65 72 76 65  |ecurity@xenserve|
>> 002cc640  72 2e 63 6f 6d 0a 00 00  00 00 00 00 00 00 00 00  |r.com...........|
>> 002cc650  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
>> 002cc660  2c 00 00 00 2e 69 6e 69  74 2e 74 65 78 74 00 2e  |,....init.text..|
>> 002cc670  69 6e 69 74 2e 64 61 74  61 00 2e 64 61 74 61 2e  |init.data..data.|
>> 002cc680  72 65 61 64 5f 6d 6f 73  74 6c 79 00 00 00 00 00  |read_mostly.....|
>> 002cc690  9e 05 00 00 00 02 02 00  30 82 05 92 06 09 2a 86  |........0.....*.|
>> 002cc6a0  48 86 f7 0d 01 07 02 a0  82 05 83 30 82 05 7f 02  |H..........0....|
>> 002cc6b0  01 01 31 0f 30 0d 06 09  60 86 48 01 65 03 04 02  |..1.0...`.H.e...|
>> 002cc6c0  01 05 00 30 5c 06 0a 2b  06 01 04 01 82 37 02 01  |...0\..+.....7..|
>> 002cc6d0  04 a0 4e 30 4c 30 17 06  0a 2b 06 01 04 01 82 37  |..N0L0...+.....7|
>> 002cc6e0  02 01 0f 30 09 03 01 00  a0 04 a2 02 80 00 30 31  |...0..........01|
>> 002cc6f0  30 0d 06 09 60 86 48 01  65 03 04 02 01 05 00 04  |0...`.H.e.......|
>> 002cc700  20 e2 47 64 f8 e8 7b 62  eb 17 e0 13 0a 0d 93 02  | .Gd..{b........|
>> 002cc710  7a d8 3b f0 20 a8 ee 3d  49 98 3f de c1 47 de 15  |z.;. ..=I.?..G..|
>> 002cc720  43 a0 82 03 2c 30 82 03  28 30 82 02 10 a0 03 02  |C...,0..(0......|
>> 002cc730  01 02 02 11 00 8f fc 11  bf 41 54 40 74 89 2c 53  |.........AT@t.,S|
>> 002cc740  a5 78 c1 e8 32 30 0d 06  09 2a 86 48 86 f7 0d 01  |.x..20...*.H....|
>> 002cc750  01 0b 05 00 30 1c 31 1a  30 18 06 03 55 04 03 13  |....0.1.0...U...|
>> 002cc760  11 58 65 6e 53 65 72 76  65 72 20 58 65 6e 20 64  |.XenServer Xen d|
>> 002cc770  65 76 30 1e 17 0d 32 35  30 33 32 30 31 36 35 35  |ev0...2503201655|
>> 002cc780  30 37 5a 17 0d 33 37 30  31 31 39 30 33 31 34 30  |07Z..37011903140|
>> 002cc790  37 5a 30 1c 31 1a 30 18  06 03 55 04 03 13 11 58  |7Z0.1.0...U....X|
>> 002cc7a0  65 6e 53 65 72 76 65 72  20 58 65 6e 20 64 65 76  |enServer Xen dev|
>> ...
>>
>> So, this confirms that the string table is there to support larger
>> section names and the signature is there and it's working.
> 
> But is it going to work on all EFI implementations, or merely the one you tried?
> Of course it would help if Demi could give more concrete pointers to (possible)
> implementations where there might be (known? suspected?) issues.
> 
> Jan

I misread the PE hashing code in EDK2.  I assumed it mishandled the case where
there is data *between* the sections and the signature, but it actually mishandles
the case where there is data *after* the signature.  I'll file an EDK2 PR to
reject such images on the grounds that they could never have worked.
-- 
Sincerely,
Demi Marie Obenour (she/her/hers)

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 7253 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3] xen: Strip xen.efi by default
  2025-11-06 10:28         ` Jan Beulich
  2025-11-06 10:40           ` Demi Marie Obenour
@ 2025-11-06 16:32           ` Frediano Ziglio
  2025-11-07  7:04             ` Jan Beulich
  1 sibling, 1 reply; 14+ messages in thread
From: Frediano Ziglio @ 2025-11-06 16:32 UTC (permalink / raw)
  To: Jan Beulich
  Cc: xen-devel, Frediano Ziglio, Andrew Cooper, Anthony PERARD,
	Michal Orzel, Julien Grall, Roger Pau Monné,
	Stefano Stabellini, Marek Marczykowski-Górecki,
	Demi Marie Obenour

On Thu, 6 Nov 2025 at 10:27, Jan Beulich <jbeulich@suse.com> wrote:
>
> On 06.11.2025 10:58, Frediano Ziglio wrote:
> > On Thu, 6 Nov 2025 at 03:52, Demi Marie Obenour <demiobenour@gmail.com> wrote:
> >> Does objdump on the signed file return correct section names?
> >
> > From objdump -x
> >
> > Sections:
> > Idx Name          Size      VMA               LMA               File off  Algn
> >   0 .text         0016c9ae  ffff82d040200000  ffff82d040200000  00000320  2**4
> >                   CONTENTS, ALLOC, LOAD, READONLY, CODE
> >   1 .rodata       0006b9e8  ffff82d040400000  ffff82d040400000  0016cce0  2**2
> >                   CONTENTS, ALLOC, LOAD, DATA
> >   2 .buildid      00000035  ffff82d04046c000  ffff82d04046c000  001d86e0  2**2
> >                   CONTENTS, ALLOC, LOAD, READONLY, DATA
> >   3 .init.text    0004d123  ffff82d040600000  ffff82d040600000  001d8720  2**2
> >                   CONTENTS, ALLOC, LOAD, READONLY, CODE
> >   4 .init.data    0006c9b0  ffff82d040800000  ffff82d040800000  00225860  2**2
> >                   CONTENTS, ALLOC, LOAD, DATA
> >   5 .data.read_mostly 00028da8  ffff82d040a00000  ffff82d040a00000
> > 00292220  2**4
> >                   CONTENTS, ALLOC, LOAD, DATA
> >   6 .data         0000feec  ffff82d040a29000  ffff82d040a29000  002bafe0  2**4
> >                   CONTENTS, ALLOC, LOAD, DATA
> >   7 .bss          00223108  ffff82d040a39000  ffff82d040a39000  00000000  2**4
> >                   ALLOC
> >   8 .reloc        000016b8  ffff82d040c5d000  ffff82d040c5d000  002caee0  2**2
> >                   CONTENTS, ALLOC, LOAD, READONLY, DATA
> >   9 .sbat         000000a6  ffff82d040c5f000  ffff82d040c5f000  002cc5a0  2**2
> >                   CONTENTS, READONLY
> >
> > Which looks correct.
> >
> > From hexdump -C I can see close to the end
> >
> > ...
> > 002cc580  30 ae 38 ae 60 ae 00 00  00 80 a3 00 10 00 00 00  |0.8.`...........|
> > 002cc590  a0 ae c0 ae e0 ae 00 00  00 00 00 00 00 00 00 00  |................|
> > 002cc5a0  73 62 61 74 2c 31 2c 53  42 41 54 20 56 65 72 73  |sbat,1,SBAT Vers|
> > 002cc5b0  69 6f 6e 2c 73 62 61 74  2c 31 2c 68 74 74 70 73  |ion,sbat,1,https|
> > 002cc5c0  3a 2f 2f 67 69 74 68 75  62 2e 63 6f 6d 2f 72 68  |://github.com/rh|
> > 002cc5d0  62 6f 6f 74 2f 73 68 69  6d 2f 62 6c 6f 62 2f 6d  |boot/shim/blob/m|
> > 002cc5e0  61 69 6e 2f 53 42 41 54  2e 6d 64 0a 78 65 6e 2e  |ain/SBAT.md.xen.|
> > 002cc5f0  78 73 2c 31 2c 43 6c 6f  75 64 20 53 6f 66 74 77  |xs,1,Cloud Softw|
> > 002cc600  61 72 65 20 47 72 6f 75  70 2c 78 65 6e 2c 34 2e  |are Group,xen,4.|
> > 002cc610  32 30 2e 31 2d 37 2e 32  32 2e 67 33 65 30 36 37  |20.1-7.22.g3e067|
> > 002cc620  32 36 62 2e 78 73 39 2c  6d 61 69 6c 74 6f 3a 73  |26b.xs9,mailto:s|
> > 002cc630  65 63 75 72 69 74 79 40  78 65 6e 73 65 72 76 65  |ecurity@xenserve|
> > 002cc640  72 2e 63 6f 6d 0a 00 00  00 00 00 00 00 00 00 00  |r.com...........|
> > 002cc650  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
> > 002cc660  2c 00 00 00 2e 69 6e 69  74 2e 74 65 78 74 00 2e  |,....init.text..|
> > 002cc670  69 6e 69 74 2e 64 61 74  61 00 2e 64 61 74 61 2e  |init.data..data.|
> > 002cc680  72 65 61 64 5f 6d 6f 73  74 6c 79 00 00 00 00 00  |read_mostly.....|
> > 002cc690  9e 05 00 00 00 02 02 00  30 82 05 92 06 09 2a 86  |........0.....*.|
> > 002cc6a0  48 86 f7 0d 01 07 02 a0  82 05 83 30 82 05 7f 02  |H..........0....|
> > 002cc6b0  01 01 31 0f 30 0d 06 09  60 86 48 01 65 03 04 02  |..1.0...`.H.e...|
> > 002cc6c0  01 05 00 30 5c 06 0a 2b  06 01 04 01 82 37 02 01  |...0\..+.....7..|
> > 002cc6d0  04 a0 4e 30 4c 30 17 06  0a 2b 06 01 04 01 82 37  |..N0L0...+.....7|
> > 002cc6e0  02 01 0f 30 09 03 01 00  a0 04 a2 02 80 00 30 31  |...0..........01|
> > 002cc6f0  30 0d 06 09 60 86 48 01  65 03 04 02 01 05 00 04  |0...`.H.e.......|
> > 002cc700  20 e2 47 64 f8 e8 7b 62  eb 17 e0 13 0a 0d 93 02  | .Gd..{b........|
> > 002cc710  7a d8 3b f0 20 a8 ee 3d  49 98 3f de c1 47 de 15  |z.;. ..=I.?..G..|
> > 002cc720  43 a0 82 03 2c 30 82 03  28 30 82 02 10 a0 03 02  |C...,0..(0......|
> > 002cc730  01 02 02 11 00 8f fc 11  bf 41 54 40 74 89 2c 53  |.........AT@t.,S|
> > 002cc740  a5 78 c1 e8 32 30 0d 06  09 2a 86 48 86 f7 0d 01  |.x..20...*.H....|
> > 002cc750  01 0b 05 00 30 1c 31 1a  30 18 06 03 55 04 03 13  |....0.1.0...U...|
> > 002cc760  11 58 65 6e 53 65 72 76  65 72 20 58 65 6e 20 64  |.XenServer Xen d|
> > 002cc770  65 76 30 1e 17 0d 32 35  30 33 32 30 31 36 35 35  |ev0...2503201655|
> > 002cc780  30 37 5a 17 0d 33 37 30  31 31 39 30 33 31 34 30  |07Z..37011903140|
> > 002cc790  37 5a 30 1c 31 1a 30 18  06 03 55 04 03 13 11 58  |7Z0.1.0...U....X|
> > 002cc7a0  65 6e 53 65 72 76 65 72  20 58 65 6e 20 64 65 76  |enServer Xen dev|
> > ...
> >
> > So, this confirms that the string table is there to support larger
> > section names and the signature is there and it's working.
>
> But is it going to work on all EFI implementations, or merely the one you tried?

Can you be more specific ?
The file was tested using dozens of different hardware and under Qemu.
Only x64 if it's what you mean.

> Of course it would help if Demi could give more concrete pointers to (possible)
> implementations where there might be (known? suspected?) issues.
>
> Jan

Frediano


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3] xen: Strip xen.efi by default
  2025-11-06 10:32 ` Jan Beulich
@ 2025-11-06 16:37   ` Frediano Ziglio
  2025-11-07  7:08     ` Jan Beulich
  0 siblings, 1 reply; 14+ messages in thread
From: Frediano Ziglio @ 2025-11-06 16:37 UTC (permalink / raw)
  To: Jan Beulich
  Cc: Frediano Ziglio, Andrew Cooper, Anthony PERARD, Michal Orzel,
	Julien Grall, Roger Pau Monné, Stefano Stabellini, xen-devel

On Thu, 6 Nov 2025 at 10:32, Jan Beulich <jbeulich@suse.com> wrote:
>
> On 05.11.2025 16:38, Frediano Ziglio wrote:
> > From: Frediano Ziglio <frediano.ziglio@cloud.com>
> >
> > 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@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.
> > ---
> >  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


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3] xen: Strip xen.efi by default
  2025-11-06 16:32           ` Frediano Ziglio
@ 2025-11-07  7:04             ` Jan Beulich
  2025-11-10 12:37               ` Frediano Ziglio
  0 siblings, 1 reply; 14+ messages in thread
From: Jan Beulich @ 2025-11-07  7:04 UTC (permalink / raw)
  To: Frediano Ziglio
  Cc: xen-devel, Frediano Ziglio, Andrew Cooper, Anthony PERARD,
	Michal Orzel, Julien Grall, Roger Pau Monné,
	Stefano Stabellini, Marek Marczykowski-Górecki,
	Demi Marie Obenour

On 06.11.2025 17:32, Frediano Ziglio wrote:
> On Thu, 6 Nov 2025 at 10:27, Jan Beulich <jbeulich@suse.com> wrote:
>>
>> On 06.11.2025 10:58, Frediano Ziglio wrote:
>>> On Thu, 6 Nov 2025 at 03:52, Demi Marie Obenour <demiobenour@gmail.com> wrote:
>>>> Does objdump on the signed file return correct section names?
>>>
>>> From objdump -x
>>>
>>> Sections:
>>> Idx Name          Size      VMA               LMA               File off  Algn
>>>   0 .text         0016c9ae  ffff82d040200000  ffff82d040200000  00000320  2**4
>>>                   CONTENTS, ALLOC, LOAD, READONLY, CODE
>>>   1 .rodata       0006b9e8  ffff82d040400000  ffff82d040400000  0016cce0  2**2
>>>                   CONTENTS, ALLOC, LOAD, DATA
>>>   2 .buildid      00000035  ffff82d04046c000  ffff82d04046c000  001d86e0  2**2
>>>                   CONTENTS, ALLOC, LOAD, READONLY, DATA
>>>   3 .init.text    0004d123  ffff82d040600000  ffff82d040600000  001d8720  2**2
>>>                   CONTENTS, ALLOC, LOAD, READONLY, CODE
>>>   4 .init.data    0006c9b0  ffff82d040800000  ffff82d040800000  00225860  2**2
>>>                   CONTENTS, ALLOC, LOAD, DATA
>>>   5 .data.read_mostly 00028da8  ffff82d040a00000  ffff82d040a00000
>>> 00292220  2**4
>>>                   CONTENTS, ALLOC, LOAD, DATA
>>>   6 .data         0000feec  ffff82d040a29000  ffff82d040a29000  002bafe0  2**4
>>>                   CONTENTS, ALLOC, LOAD, DATA
>>>   7 .bss          00223108  ffff82d040a39000  ffff82d040a39000  00000000  2**4
>>>                   ALLOC
>>>   8 .reloc        000016b8  ffff82d040c5d000  ffff82d040c5d000  002caee0  2**2
>>>                   CONTENTS, ALLOC, LOAD, READONLY, DATA
>>>   9 .sbat         000000a6  ffff82d040c5f000  ffff82d040c5f000  002cc5a0  2**2
>>>                   CONTENTS, READONLY
>>>
>>> Which looks correct.
>>>
>>> From hexdump -C I can see close to the end
>>>
>>> ...
>>> 002cc580  30 ae 38 ae 60 ae 00 00  00 80 a3 00 10 00 00 00  |0.8.`...........|
>>> 002cc590  a0 ae c0 ae e0 ae 00 00  00 00 00 00 00 00 00 00  |................|
>>> 002cc5a0  73 62 61 74 2c 31 2c 53  42 41 54 20 56 65 72 73  |sbat,1,SBAT Vers|
>>> 002cc5b0  69 6f 6e 2c 73 62 61 74  2c 31 2c 68 74 74 70 73  |ion,sbat,1,https|
>>> 002cc5c0  3a 2f 2f 67 69 74 68 75  62 2e 63 6f 6d 2f 72 68  |://github.com/rh|
>>> 002cc5d0  62 6f 6f 74 2f 73 68 69  6d 2f 62 6c 6f 62 2f 6d  |boot/shim/blob/m|
>>> 002cc5e0  61 69 6e 2f 53 42 41 54  2e 6d 64 0a 78 65 6e 2e  |ain/SBAT.md.xen.|
>>> 002cc5f0  78 73 2c 31 2c 43 6c 6f  75 64 20 53 6f 66 74 77  |xs,1,Cloud Softw|
>>> 002cc600  61 72 65 20 47 72 6f 75  70 2c 78 65 6e 2c 34 2e  |are Group,xen,4.|
>>> 002cc610  32 30 2e 31 2d 37 2e 32  32 2e 67 33 65 30 36 37  |20.1-7.22.g3e067|
>>> 002cc620  32 36 62 2e 78 73 39 2c  6d 61 69 6c 74 6f 3a 73  |26b.xs9,mailto:s|
>>> 002cc630  65 63 75 72 69 74 79 40  78 65 6e 73 65 72 76 65  |ecurity@xenserve|
>>> 002cc640  72 2e 63 6f 6d 0a 00 00  00 00 00 00 00 00 00 00  |r.com...........|
>>> 002cc650  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
>>> 002cc660  2c 00 00 00 2e 69 6e 69  74 2e 74 65 78 74 00 2e  |,....init.text..|
>>> 002cc670  69 6e 69 74 2e 64 61 74  61 00 2e 64 61 74 61 2e  |init.data..data.|
>>> 002cc680  72 65 61 64 5f 6d 6f 73  74 6c 79 00 00 00 00 00  |read_mostly.....|
>>> 002cc690  9e 05 00 00 00 02 02 00  30 82 05 92 06 09 2a 86  |........0.....*.|
>>> 002cc6a0  48 86 f7 0d 01 07 02 a0  82 05 83 30 82 05 7f 02  |H..........0....|
>>> 002cc6b0  01 01 31 0f 30 0d 06 09  60 86 48 01 65 03 04 02  |..1.0...`.H.e...|
>>> 002cc6c0  01 05 00 30 5c 06 0a 2b  06 01 04 01 82 37 02 01  |...0\..+.....7..|
>>> 002cc6d0  04 a0 4e 30 4c 30 17 06  0a 2b 06 01 04 01 82 37  |..N0L0...+.....7|
>>> 002cc6e0  02 01 0f 30 09 03 01 00  a0 04 a2 02 80 00 30 31  |...0..........01|
>>> 002cc6f0  30 0d 06 09 60 86 48 01  65 03 04 02 01 05 00 04  |0...`.H.e.......|
>>> 002cc700  20 e2 47 64 f8 e8 7b 62  eb 17 e0 13 0a 0d 93 02  | .Gd..{b........|
>>> 002cc710  7a d8 3b f0 20 a8 ee 3d  49 98 3f de c1 47 de 15  |z.;. ..=I.?..G..|
>>> 002cc720  43 a0 82 03 2c 30 82 03  28 30 82 02 10 a0 03 02  |C...,0..(0......|
>>> 002cc730  01 02 02 11 00 8f fc 11  bf 41 54 40 74 89 2c 53  |.........AT@t.,S|
>>> 002cc740  a5 78 c1 e8 32 30 0d 06  09 2a 86 48 86 f7 0d 01  |.x..20...*.H....|
>>> 002cc750  01 0b 05 00 30 1c 31 1a  30 18 06 03 55 04 03 13  |....0.1.0...U...|
>>> 002cc760  11 58 65 6e 53 65 72 76  65 72 20 58 65 6e 20 64  |.XenServer Xen d|
>>> 002cc770  65 76 30 1e 17 0d 32 35  30 33 32 30 31 36 35 35  |ev0...2503201655|
>>> 002cc780  30 37 5a 17 0d 33 37 30  31 31 39 30 33 31 34 30  |07Z..37011903140|
>>> 002cc790  37 5a 30 1c 31 1a 30 18  06 03 55 04 03 13 11 58  |7Z0.1.0...U....X|
>>> 002cc7a0  65 6e 53 65 72 76 65 72  20 58 65 6e 20 64 65 76  |enServer Xen dev|
>>> ...
>>>
>>> So, this confirms that the string table is there to support larger
>>> section names and the signature is there and it's working.
>>
>> But is it going to work on all EFI implementations, or merely the one you tried?
> 
> Can you be more specific ?
> The file was tested using dozens of different hardware and under Qemu.
> Only x64 if it's what you mean.

No, I was referring to the fact that there are distinct EFI implementations,
which may differ in their loader behavior. But see also Demi's clarification
on her original remark.

Jan


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3] xen: Strip xen.efi by default
  2025-11-06 16:37   ` Frediano Ziglio
@ 2025-11-07  7:08     ` Jan Beulich
  2025-11-10 12:33       ` Frediano Ziglio
  0 siblings, 1 reply; 14+ messages in thread
From: Jan Beulich @ 2025-11-07  7:08 UTC (permalink / raw)
  To: Frediano Ziglio
  Cc: Frediano Ziglio, Andrew Cooper, Anthony PERARD, Michal Orzel,
	Julien Grall, Roger Pau Monné, Stefano Stabellini, xen-devel

On 06.11.2025 17:37, Frediano Ziglio wrote:
> On Thu, 6 Nov 2025 at 10:32, Jan Beulich <jbeulich@suse.com> wrote:
>> On 05.11.2025 16:38, Frediano Ziglio wrote:
>>> --- 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 ?

The former. We want to have the binary available that the linker produced
directly. Anything else are extra's for what people think they may need.

>>> --- 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.

Sorry, typo - "stale" was meant.

> 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.

How does this matter in this context? The description talks of a xen-syms.efi
being generated, yet I'm simply unable to spot where that would be happening.

>> 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.

Except that the rm is tidying up rather than creating anything.

Jan


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3] xen: Strip xen.efi by default
  2025-11-07  7:08     ` Jan Beulich
@ 2025-11-10 12:33       ` Frediano Ziglio
  0 siblings, 0 replies; 14+ messages in thread
From: Frediano Ziglio @ 2025-11-10 12:33 UTC (permalink / raw)
  To: Jan Beulich
  Cc: Frediano Ziglio, Andrew Cooper, Anthony PERARD, Michal Orzel,
	Julien Grall, Roger Pau Monné, Stefano Stabellini, xen-devel

On Fri, 7 Nov 2025 at 07:08, Jan Beulich <jbeulich@suse.com> wrote:
>
> On 06.11.2025 17:37, Frediano Ziglio wrote:
> > On Thu, 6 Nov 2025 at 10:32, Jan Beulich <jbeulich@suse.com> wrote:
> >> On 05.11.2025 16:38, Frediano Ziglio wrote:
> >>> --- 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 ?
>
> The former. We want to have the binary available that the linker produced
> directly. Anything else are extra's for what people think they may need.
>

Done in v4.

> >>> --- 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.
>
> Sorry, typo - "stale" was meant.
>
> > 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.
>
> How does this matter in this context? The description talks of a xen-syms.efi
> being generated, yet I'm simply unable to spot where that would be happening.
>

It was "generated" with a "cp" command, now I use "mv" + "strip -o"
(in v4) to make it more clear.

> >> 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.
>
> Except that the rm is tidying up rather than creating anything.
>

Updated this moving endbr check before the "mv" and the cleanup after.

> Jan

Frediano


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3] xen: Strip xen.efi by default
  2025-11-07  7:04             ` Jan Beulich
@ 2025-11-10 12:37               ` Frediano Ziglio
  0 siblings, 0 replies; 14+ messages in thread
From: Frediano Ziglio @ 2025-11-10 12:37 UTC (permalink / raw)
  To: Jan Beulich
  Cc: xen-devel, Frediano Ziglio, Andrew Cooper, Anthony PERARD,
	Michal Orzel, Julien Grall, Roger Pau Monné,
	Stefano Stabellini, Marek Marczykowski-Górecki,
	Demi Marie Obenour

On Fri, 7 Nov 2025 at 07:04, Jan Beulich <jbeulich@suse.com> wrote:
>
> On 06.11.2025 17:32, Frediano Ziglio wrote:
> > On Thu, 6 Nov 2025 at 10:27, Jan Beulich <jbeulich@suse.com> wrote:
> >>
> >> On 06.11.2025 10:58, Frediano Ziglio wrote:
> >>> On Thu, 6 Nov 2025 at 03:52, Demi Marie Obenour <demiobenour@gmail.com> wrote:
> >>>> Does objdump on the signed file return correct section names?
> >>>
> >>> From objdump -x
> >>>
> >>> Sections:
> >>> Idx Name          Size      VMA               LMA               File off  Algn
> >>>   0 .text         0016c9ae  ffff82d040200000  ffff82d040200000  00000320  2**4
> >>>                   CONTENTS, ALLOC, LOAD, READONLY, CODE
> >>>   1 .rodata       0006b9e8  ffff82d040400000  ffff82d040400000  0016cce0  2**2
> >>>                   CONTENTS, ALLOC, LOAD, DATA
> >>>   2 .buildid      00000035  ffff82d04046c000  ffff82d04046c000  001d86e0  2**2
> >>>                   CONTENTS, ALLOC, LOAD, READONLY, DATA
> >>>   3 .init.text    0004d123  ffff82d040600000  ffff82d040600000  001d8720  2**2
> >>>                   CONTENTS, ALLOC, LOAD, READONLY, CODE
> >>>   4 .init.data    0006c9b0  ffff82d040800000  ffff82d040800000  00225860  2**2
> >>>                   CONTENTS, ALLOC, LOAD, DATA
> >>>   5 .data.read_mostly 00028da8  ffff82d040a00000  ffff82d040a00000
> >>> 00292220  2**4
> >>>                   CONTENTS, ALLOC, LOAD, DATA
> >>>   6 .data         0000feec  ffff82d040a29000  ffff82d040a29000  002bafe0  2**4
> >>>                   CONTENTS, ALLOC, LOAD, DATA
> >>>   7 .bss          00223108  ffff82d040a39000  ffff82d040a39000  00000000  2**4
> >>>                   ALLOC
> >>>   8 .reloc        000016b8  ffff82d040c5d000  ffff82d040c5d000  002caee0  2**2
> >>>                   CONTENTS, ALLOC, LOAD, READONLY, DATA
> >>>   9 .sbat         000000a6  ffff82d040c5f000  ffff82d040c5f000  002cc5a0  2**2
> >>>                   CONTENTS, READONLY
> >>>
> >>> Which looks correct.
> >>>
> >>> From hexdump -C I can see close to the end
> >>>
> >>> ...
> >>> 002cc580  30 ae 38 ae 60 ae 00 00  00 80 a3 00 10 00 00 00  |0.8.`...........|
> >>> 002cc590  a0 ae c0 ae e0 ae 00 00  00 00 00 00 00 00 00 00  |................|
> >>> 002cc5a0  73 62 61 74 2c 31 2c 53  42 41 54 20 56 65 72 73  |sbat,1,SBAT Vers|
> >>> 002cc5b0  69 6f 6e 2c 73 62 61 74  2c 31 2c 68 74 74 70 73  |ion,sbat,1,https|
> >>> 002cc5c0  3a 2f 2f 67 69 74 68 75  62 2e 63 6f 6d 2f 72 68  |://github.com/rh|
> >>> 002cc5d0  62 6f 6f 74 2f 73 68 69  6d 2f 62 6c 6f 62 2f 6d  |boot/shim/blob/m|
> >>> 002cc5e0  61 69 6e 2f 53 42 41 54  2e 6d 64 0a 78 65 6e 2e  |ain/SBAT.md.xen.|
> >>> 002cc5f0  78 73 2c 31 2c 43 6c 6f  75 64 20 53 6f 66 74 77  |xs,1,Cloud Softw|
> >>> 002cc600  61 72 65 20 47 72 6f 75  70 2c 78 65 6e 2c 34 2e  |are Group,xen,4.|
> >>> 002cc610  32 30 2e 31 2d 37 2e 32  32 2e 67 33 65 30 36 37  |20.1-7.22.g3e067|
> >>> 002cc620  32 36 62 2e 78 73 39 2c  6d 61 69 6c 74 6f 3a 73  |26b.xs9,mailto:s|
> >>> 002cc630  65 63 75 72 69 74 79 40  78 65 6e 73 65 72 76 65  |ecurity@xenserve|
> >>> 002cc640  72 2e 63 6f 6d 0a 00 00  00 00 00 00 00 00 00 00  |r.com...........|
> >>> 002cc650  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
> >>> 002cc660  2c 00 00 00 2e 69 6e 69  74 2e 74 65 78 74 00 2e  |,....init.text..|
> >>> 002cc670  69 6e 69 74 2e 64 61 74  61 00 2e 64 61 74 61 2e  |init.data..data.|
> >>> 002cc680  72 65 61 64 5f 6d 6f 73  74 6c 79 00 00 00 00 00  |read_mostly.....|
> >>> 002cc690  9e 05 00 00 00 02 02 00  30 82 05 92 06 09 2a 86  |........0.....*.|
> >>> 002cc6a0  48 86 f7 0d 01 07 02 a0  82 05 83 30 82 05 7f 02  |H..........0....|
> >>> 002cc6b0  01 01 31 0f 30 0d 06 09  60 86 48 01 65 03 04 02  |..1.0...`.H.e...|
> >>> 002cc6c0  01 05 00 30 5c 06 0a 2b  06 01 04 01 82 37 02 01  |...0\..+.....7..|
> >>> 002cc6d0  04 a0 4e 30 4c 30 17 06  0a 2b 06 01 04 01 82 37  |..N0L0...+.....7|
> >>> 002cc6e0  02 01 0f 30 09 03 01 00  a0 04 a2 02 80 00 30 31  |...0..........01|
> >>> 002cc6f0  30 0d 06 09 60 86 48 01  65 03 04 02 01 05 00 04  |0...`.H.e.......|
> >>> 002cc700  20 e2 47 64 f8 e8 7b 62  eb 17 e0 13 0a 0d 93 02  | .Gd..{b........|
> >>> 002cc710  7a d8 3b f0 20 a8 ee 3d  49 98 3f de c1 47 de 15  |z.;. ..=I.?..G..|
> >>> 002cc720  43 a0 82 03 2c 30 82 03  28 30 82 02 10 a0 03 02  |C...,0..(0......|
> >>> 002cc730  01 02 02 11 00 8f fc 11  bf 41 54 40 74 89 2c 53  |.........AT@t.,S|
> >>> 002cc740  a5 78 c1 e8 32 30 0d 06  09 2a 86 48 86 f7 0d 01  |.x..20...*.H....|
> >>> 002cc750  01 0b 05 00 30 1c 31 1a  30 18 06 03 55 04 03 13  |....0.1.0...U...|
> >>> 002cc760  11 58 65 6e 53 65 72 76  65 72 20 58 65 6e 20 64  |.XenServer Xen d|
> >>> 002cc770  65 76 30 1e 17 0d 32 35  30 33 32 30 31 36 35 35  |ev0...2503201655|
> >>> 002cc780  30 37 5a 17 0d 33 37 30  31 31 39 30 33 31 34 30  |07Z..37011903140|
> >>> 002cc790  37 5a 30 1c 31 1a 30 18  06 03 55 04 03 13 11 58  |7Z0.1.0...U....X|
> >>> 002cc7a0  65 6e 53 65 72 76 65 72  20 58 65 6e 20 64 65 76  |enServer Xen dev|
> >>> ...
> >>>
> >>> So, this confirms that the string table is there to support larger
> >>> section names and the signature is there and it's working.
> >>
> >> But is it going to work on all EFI implementations, or merely the one you tried?
> >
> > Can you be more specific ?
> > The file was tested using dozens of different hardware and under Qemu.
> > Only x64 if it's what you mean.
>
> No, I was referring to the fact that there are distinct EFI implementations,
> which may differ in their loader behavior. But see also Demi's clarification
> on her original remark.
>
> Jan

Demi posted an update on that remark that it was a mistake.

The signature should be applied as the last step resulting in it to be
at the end of the file. This as any following update to the file
(beside updating the checksum or the signature itself) would
invalidate the signature and correctly the firmware would refuse to
load the file if secure boot is enabled.

Frediano


^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2025-11-10 12:37 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-05 15:38 [PATCH v3] xen: Strip xen.efi by default Frediano Ziglio
2025-11-05 20:31 ` Demi Marie Obenour
2025-11-06  2:00   ` Frediano Ziglio
2025-11-06  3:52     ` Demi Marie Obenour
2025-11-06  9:58       ` Frediano Ziglio
2025-11-06 10:28         ` Jan Beulich
2025-11-06 10:40           ` Demi Marie Obenour
2025-11-06 16:32           ` Frediano Ziglio
2025-11-07  7:04             ` Jan Beulich
2025-11-10 12:37               ` Frediano Ziglio
2025-11-06 10:32 ` Jan Beulich
2025-11-06 16:37   ` Frediano Ziglio
2025-11-07  7:08     ` Jan Beulich
2025-11-10 12:33       ` Frediano Ziglio

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).