[PATCH RFC] EFI: strip xen.efi when putting it on the EFI partition

[PATCH RFC] EFI: strip xen.efi when putting it on the EFI partition

[Jan Beulich]

With debug info retained, xen.efi can be quite large. Unlike for xen.gz
there's no intermediate step (mkelf32 there) involved which would strip
debug info kind of as a side effect. While the installing of xen.efi on
the EFI partition is an optional step (intended to be a courtesy to the
developer), adjust it also for the purpose of documenting what distros
would be expected to do during boot loader configuration (which is what
would normally put xen.efi into the EFI partition).

Model the control over stripping after Linux'es module installation,
except that the stripped executable is constructed in the build area
instead of in the destination location. This is to conserve on space
used there - EFI partitions tend to be only a few hundred Mb in size.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
RFC: GNU strip 2.38 appears to have issues when acting on a PE binary:
     - the new file positions of the sections do not respect the file
       alignment specified by the header (a resulting looks to work on
       one EFI implementation where I did actually try it, but I don't
       think we can rely on that),
     - file name symbols are also stripped; while there is a separate
       --keep-file-symbols option (which I would have thought to be on
       by default anyway), its use makes no difference.
     Older GNU strip (observed with 2.35.1) doesn't work at all ("Data
     Directory size (1c) exceeds space left in section (8)").

--- a/xen/Makefile
+++ b/xen/Makefile
@@ -461,6 +461,22 @@ 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
+# --strip-debug 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
+else
+efi-strip-opt := $(INSTALL_EFI_STRIP)
+endif
+
+endif
+
 .PHONY: _install
 _install: D=$(DESTDIR)
 _install: T=$(notdir $(TARGET))
@@ -485,6 +501,9 @@ _install: $(TARGET)$(CONFIG_XEN_INSTALL_
 		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; \

Re: [PATCH RFC] EFI: strip xen.efi when putting it on the EFI partition

[Bertrand Marquis]

Hi Jan,

> On 25 Apr 2022, at 11:46, Jan Beulich <jbeulich@suse.com> wrote:
> 
> With debug info retained, xen.efi can be quite large. Unlike for xen.gz
> there's no intermediate step (mkelf32 there) involved which would strip
> debug info kind of as a side effect. While the installing of xen.efi on
> the EFI partition is an optional step (intended to be a courtesy to the
> developer), adjust it also for the purpose of documenting what distros
> would be expected to do during boot loader configuration (which is what
> would normally put xen.efi into the EFI partition).
> 
> Model the control over stripping after Linux'es module installation,
> except that the stripped executable is constructed in the build area
> instead of in the destination location. This is to conserve on space
> used there - EFI partitions tend to be only a few hundred Mb in size.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> ---
> RFC: GNU strip 2.38 appears to have issues when acting on a PE binary:
>     - the new file positions of the sections do not respect the file
>       alignment specified by the header (a resulting looks to work on
>       one EFI implementation where I did actually try it, but I don't
>       think we can rely on that),
>     - file name symbols are also stripped; while there is a separate
>       --keep-file-symbols option (which I would have thought to be on
>       by default anyway), its use makes no difference.
>     Older GNU strip (observed with 2.35.1) doesn't work at all ("Data
>     Directory size (1c) exceeds space left in section (8)").
> 
> --- a/xen/Makefile
> +++ b/xen/Makefile
> @@ -461,6 +461,22 @@ 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
> +# --strip-debug 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
> +else
> +efi-strip-opt := $(INSTALL_EFI_STRIP)
> +endif
> +
> +endif

This does sound very complex and using combination of ifdef and ifeq on an external variable is not done anywhere else.

How about splitting into a variable to turn strip on or off and let the user override a local variable setting up the strip options if he wants to ?

Something like:

EFI_STRIP_OPTION ?= "—strip-debug"

And then just using INSTALL_EFI_STRIP to strip or not during the _install phase 

One wanting to use no specific option would have to pass INSTALL_EFI_STRIP=1 EFI_STRIP_OPTION=“” for example.

Cheers
Bertrand

> +
> .PHONY: _install
> _install: D=$(DESTDIR)
> _install: T=$(notdir $(TARGET))
> @@ -485,6 +501,9 @@ _install: $(TARGET)$(CONFIG_XEN_INSTALL_
> 		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; \
> 
> 

Re: [PATCH RFC] EFI: strip xen.efi when putting it on the EFI partition

[Jan Beulich]

On 26.04.2022 14:26, Bertrand Marquis wrote:
> Hi Jan,
> 
>> On 25 Apr 2022, at 11:46, Jan Beulich <jbeulich@suse.com> wrote:
>>
>> With debug info retained, xen.efi can be quite large. Unlike for xen.gz
>> there's no intermediate step (mkelf32 there) involved which would strip
>> debug info kind of as a side effect. While the installing of xen.efi on
>> the EFI partition is an optional step (intended to be a courtesy to the
>> developer), adjust it also for the purpose of documenting what distros
>> would be expected to do during boot loader configuration (which is what
>> would normally put xen.efi into the EFI partition).
>>
>> Model the control over stripping after Linux'es module installation,
>> except that the stripped executable is constructed in the build area
>> instead of in the destination location. This is to conserve on space
>> used there - EFI partitions tend to be only a few hundred Mb in size.
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> ---
>> RFC: GNU strip 2.38 appears to have issues when acting on a PE binary:
>>     - the new file positions of the sections do not respect the file
>>       alignment specified by the header (a resulting looks to work on
>>       one EFI implementation where I did actually try it, but I don't
>>       think we can rely on that),
>>     - file name symbols are also stripped; while there is a separate
>>       --keep-file-symbols option (which I would have thought to be on
>>       by default anyway), its use makes no difference.
>>     Older GNU strip (observed with 2.35.1) doesn't work at all ("Data
>>     Directory size (1c) exceeds space left in section (8)").
>>
>> --- a/xen/Makefile
>> +++ b/xen/Makefile
>> @@ -461,6 +461,22 @@ 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
>> +# --strip-debug 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
>> +else
>> +efi-strip-opt := $(INSTALL_EFI_STRIP)
>> +endif
>> +
>> +endif
> 
> This does sound very complex and using combination of ifdef and ifeq on an external variable is not done anywhere else.
> 
> How about splitting into a variable to turn strip on or off and let the user override a local variable setting up the strip options if he wants to ?
> 
> Something like:
> 
> EFI_STRIP_OPTION ?= "—strip-debug"
> 
> And then just using INSTALL_EFI_STRIP to strip or not during the _install phase 

This "just using INSTALL_EFI_STRIP" is what we have with the present
version as well, and I'm not really looking forward to have two
separate variable to act upon. It was for this particular reason that
I took Linux'es module installation process as a "template".

> One wanting to use no specific option would have to pass INSTALL_EFI_STRIP=1 EFI_STRIP_OPTION=“” for example.

This particular example wouldn't strip anything aiui, and hence would
needlessly touch the binary (and perhaps make changes to it just as a
side effect: For example I'm observing the string table growing, which
I have yet to investigate in binutils).

Jan

Re: [PATCH RFC] EFI: strip xen.efi when putting it on the EFI partition

[Bertrand Marquis]

Hi Jan,

> On 26 Apr 2022, at 15:09, Jan Beulich <jbeulich@suse.com> wrote:
> 
> On 26.04.2022 14:26, Bertrand Marquis wrote:
>> Hi Jan,
>> 
>>> On 25 Apr 2022, at 11:46, Jan Beulich <jbeulich@suse.com> wrote:
>>> 
>>> With debug info retained, xen.efi can be quite large. Unlike for xen.gz
>>> there's no intermediate step (mkelf32 there) involved which would strip
>>> debug info kind of as a side effect. While the installing of xen.efi on
>>> the EFI partition is an optional step (intended to be a courtesy to the
>>> developer), adjust it also for the purpose of documenting what distros
>>> would be expected to do during boot loader configuration (which is what
>>> would normally put xen.efi into the EFI partition).
>>> 
>>> Model the control over stripping after Linux'es module installation,
>>> except that the stripped executable is constructed in the build area
>>> instead of in the destination location. This is to conserve on space
>>> used there - EFI partitions tend to be only a few hundred Mb in size.
>>> 
>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>> ---
>>> RFC: GNU strip 2.38 appears to have issues when acting on a PE binary:
>>> - the new file positions of the sections do not respect the file
>>> alignment specified by the header (a resulting looks to work on
>>> one EFI implementation where I did actually try it, but I don't
>>> think we can rely on that),
>>> - file name symbols are also stripped; while there is a separate
>>> --keep-file-symbols option (which I would have thought to be on
>>> by default anyway), its use makes no difference.
>>> Older GNU strip (observed with 2.35.1) doesn't work at all ("Data
>>> Directory size (1c) exceeds space left in section (8)").
>>> 
>>> --- a/xen/Makefile
>>> +++ b/xen/Makefile
>>> @@ -461,6 +461,22 @@ 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
>>> +# --strip-debug 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
>>> +else
>>> +efi-strip-opt := $(INSTALL_EFI_STRIP)
>>> +endif
>>> +
>>> +endif
>> 
>> This does sound very complex and using combination of ifdef and ifeq on an external variable is not done anywhere else.
>> 
>> How about splitting into a variable to turn strip on or off and let the user override a local variable setting up the strip options if he wants to ?
>> 
>> Something like:
>> 
>> EFI_STRIP_OPTION ?= "—strip-debug"
>> 
>> And then just using INSTALL_EFI_STRIP to strip or not during the _install phase 
> 
> This "just using INSTALL_EFI_STRIP" is what we have with the present
> version as well, and I'm not really looking forward to have two
> separate variable to act upon. It was for this particular reason that
> I took Linux'es module installation process as a "template".

You need 2 variables only when you want to change the default option.
Anyway up to you but I think this is a bit unusual and using ifdef for non
internal variables is not something I would do.

> 
>> One wanting to use no specific option would have to pass INSTALL_EFI_STRIP=1 EFI_STRIP_OPTION=“” for example.
> 
> This particular example wouldn't strip anything aiui, and hence would
> needlessly touch the binary (and perhaps make changes to it just as a
> side effect: For example I'm observing the string table growing, which
> I have yet to investigate in binutils).

The example was not meant to be useful but just to show how an empty
option could be achieved as other use cases where quite obvious.

Bertrand

> 
> Jan

Re: [PATCH RFC] EFI: strip xen.efi when putting it on the EFI partition

[Jan Beulich]

On 25.04.2022 12:46, Jan Beulich wrote:
> With debug info retained, xen.efi can be quite large. Unlike for xen.gz
> there's no intermediate step (mkelf32 there) involved which would strip
> debug info kind of as a side effect. While the installing of xen.efi on
> the EFI partition is an optional step (intended to be a courtesy to the
> developer), adjust it also for the purpose of documenting what distros
> would be expected to do during boot loader configuration (which is what
> would normally put xen.efi into the EFI partition).
> 
> Model the control over stripping after Linux'es module installation,
> except that the stripped executable is constructed in the build area
> instead of in the destination location. This is to conserve on space
> used there - EFI partitions tend to be only a few hundred Mb in size.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> ---
> RFC: GNU strip 2.38 appears to have issues when acting on a PE binary:
>      - the new file positions of the sections do not respect the file
>        alignment specified by the header (a resulting looks to work on
>        one EFI implementation where I did actually try it, but I don't
>        think we can rely on that),
>      - file name symbols are also stripped; while there is a separate
>        --keep-file-symbols option (which I would have thought to be on
>        by default anyway), its use makes no difference.

Update to these items: The first one turned out to be an issue with a
not-yet-upstream patch that I've been carrying for a long time. I've
fixed that up, and will submit that patch (perhaps together with
further ones) in due course. Apart from that the list of remarks now
is

- file name symbols are also stripped; while there is a separate
  --keep-file-symbols option (which I would have thought to be on by
  default anyway), its use so far makes no difference,
- the string table grows in size, when one would expect it to shrink,
- linker version is changed in and timestamp zapped from the header.

Locally I have draft patches for all of these issues, but this means
stripping won't work overly well until at least 2.39.

Jan