| Message ID | 79ebbde2-4be8-d393-476d-25326a2aa327@suse.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | EFI: strip xen.efi when putting it on the EFI partition | expand |
On 09.06.2022 17:52, 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> > --- > GNU strip 2.38 appears to have issues when acting on a PE binary: > - 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 retain its > size (or shrink), > - linker version is changed in and timestamp zapped from the header. > Older GNU strip (observed with 2.35.1) doesn't work at all ("Data > Directory size (1c) exceeds space left in section (8)"). > > Future GNU strip is going to honor --keep-file-symbols (and will also > have the other issues fixed). Question is whether we should use that > option (for the symbol table as a whole to make sense), or whether > instead we should (by default) strip the symbol table as well. Without any feedback / ack I guess I'll consider this of no interest (despite having heard otherwise, triggering me to put together the patch in the first place), and put on the pile of effectively rejected patches. Jan
Hi Jan, On 05/07/2022 17:03, Jan Beulich wrote: > On 09.06.2022 17:52, 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> >> --- >> GNU strip 2.38 appears to have issues when acting on a PE binary: >> - 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 retain its >> size (or shrink), >> - linker version is changed in and timestamp zapped from the header. >> Older GNU strip (observed with 2.35.1) doesn't work at all ("Data >> Directory size (1c) exceeds space left in section (8)"). >> >> Future GNU strip is going to honor --keep-file-symbols (and will also >> have the other issues fixed). Question is whether we should use that >> option (for the symbol table as a whole to make sense), or whether >> instead we should (by default) strip the symbol table as well. > > Without any feedback / ack I guess I'll consider this of no interest > (despite having heard otherwise, triggering me to put together the > patch in the first place), and put on the pile of effectively > rejected patches. Sorry, I haven't looked at this patch because I am not very familiar with the Makefile code. Reading through the commit message, I think having this patch would be quite beneficial. Regarding the review, Anthony has been working on the build system recently. So maybe he could give a review? (I have CCed him). Cheers,
Hi Jan, > -----Original Message----- > Subject: Re: [PATCH] EFI: strip xen.efi when putting it on the EFI partition > > On 09.06.2022 17:52, 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> > > --- > > GNU strip 2.38 appears to have issues when acting on a PE binary: > > - 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 retain its > > size (or shrink), > > - linker version is changed in and timestamp zapped from the header. > > Older GNU strip (observed with 2.35.1) doesn't work at all ("Data > > Directory size (1c) exceeds space left in section (8)"). > > > > Future GNU strip is going to honor --keep-file-symbols (and will also > > have the other issues fixed). Question is whether we should use that > > option (for the symbol table as a whole to make sense), or whether > > instead we should (by default) strip the symbol table as well. > > Without any feedback / ack I guess I'll consider this of no interest > (despite having heard otherwise, triggering me to put together the > patch in the first place), and put on the pile of effectively > rejected patches. I did a test for this patch on my x86 machine and I think this patch is doing the correct thing, so: Tested-by: Henry Wang <Henry.Wang@arm.com> I also noticed that Julien is suggesting maybe we can have Anthony as the reviewer for this patch, so I also add him in the CC of this email. Kind regards, Henry > > Jan
On Thu, Jun 09, 2022 at 05:52:45PM +0200, 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> > --- > GNU strip 2.38 appears to have issues when acting on a PE binary: > - 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 retain its > size (or shrink), > - linker version is changed in and timestamp zapped from the header. > Older GNU strip (observed with 2.35.1) doesn't work at all ("Data > Directory size (1c) exceeds space left in section (8)"). > > Future GNU strip is going to honor --keep-file-symbols (and will also > have the other issues fixed). Question is whether we should use that > option (for the symbol table as a whole to make sense), or whether > instead we should (by default) strip the symbol table as well. I guess trying to keep those symbol might be useful, if it's not too big, to help debugging system in production. > --- a/xen/Makefile > +++ b/xen/Makefile > @@ -465,6 +465,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. It would be useful to also document INSTALL_EFI_STRIP in ./INSTALL or in ./docs/misc/efi.pandoc (efi.pandoc is where EFI_VENDOR is documented for example, so probably a better place for the doc of the new option). > +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)) > @@ -489,6 +505,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 ||) \ This optional part of the script that ends with "||" means that if `strip` or `install` fails, we would install the non stripped binary. Is this something that's acceptable? (Plus of doing that is avoiding changing the next line, and avoiding a longer $(if ,). > $(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; \ An other thing, the "*.efi.stripped" file is I think going to be left over and not removed during `make clean`. Cheers,
On 06.07.2022 17:17, Anthony PERARD wrote: > On Thu, Jun 09, 2022 at 05:52:45PM +0200, Jan Beulich wrote: >> --- a/xen/Makefile >> +++ b/xen/Makefile >> @@ -465,6 +465,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. > > It would be useful to also document INSTALL_EFI_STRIP in ./INSTALL or in > ./docs/misc/efi.pandoc (efi.pandoc is where EFI_VENDOR is documented for > example, so probably a better place for the doc of the new option). Well, imo it's far preferable to install _something_ that works (even if it consumes more space) than not installing anything. I'll look into addressing the other two comments you gave. Jan
Hi Jan, On 2022/7/6 13:44, Henry Wang wrote: > Hi Jan, > >> -----Original Message----- >> Subject: Re: [PATCH] EFI: strip xen.efi when putting it on the EFI partition >> >> On 09.06.2022 17:52, 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> >>> --- >>> GNU strip 2.38 appears to have issues when acting on a PE binary: >>> - 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 retain its >>> size (or shrink), >>> - linker version is changed in and timestamp zapped from the header. >>> Older GNU strip (observed with 2.35.1) doesn't work at all ("Data >>> Directory size (1c) exceeds space left in section (8)"). >>> >>> Future GNU strip is going to honor --keep-file-symbols (and will also >>> have the other issues fixed). Question is whether we should use that >>> option (for the symbol table as a whole to make sense), or whether >>> instead we should (by default) strip the symbol table as well. >> >> Without any feedback / ack I guess I'll consider this of no interest >> (despite having heard otherwise, triggering me to put together the >> patch in the first place), and put on the pile of effectively >> rejected patches. > > I did a test for this patch on my x86 machine and I think this patch is > doing the correct thing, so: > > Tested-by: Henry Wang <Henry.Wang@arm.com> > Because there was no Arm EFI environment in hand at the time, Henry only tested the x86 part.I have setup an Arm platform with UEFI v2.70 (EDK II, 0x00010000) today, and this patch works well when boot Xen as an EFI application from UEFI shell. But the binaries sizes are the same with/without this patch. Is it expected? I have enabled: CONFIG_DEBUG=y CONFIG_DEBUG_INFO=y Is there anything I should be aware to test this patch? -rwxrwxr-x 1 weic weic 1081504 Jul 7 18:43 xen -rwxrwxr-x 1 weic weic 1081504 Jul 7 19:43 xen Tested-by (Arm only): Wei Chen <Wei.Chen@arm.com> Thanks, Wei Chen > I also noticed that Julien is suggesting maybe we can have Anthony as > the reviewer for this patch, so I also add him in the CC of this email. > > Kind regards, > Henry > >> >> Jan >
On 07.07.2022 13:58, Wei Chen wrote: > Hi Jan, > > On 2022/7/6 13:44, Henry Wang wrote: >> Hi Jan, >> >>> -----Original Message----- >>> Subject: Re: [PATCH] EFI: strip xen.efi when putting it on the EFI partition >>> >>> On 09.06.2022 17:52, 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> >>>> --- >>>> GNU strip 2.38 appears to have issues when acting on a PE binary: >>>> - 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 retain its >>>> size (or shrink), >>>> - linker version is changed in and timestamp zapped from the header. >>>> Older GNU strip (observed with 2.35.1) doesn't work at all ("Data >>>> Directory size (1c) exceeds space left in section (8)"). >>>> >>>> Future GNU strip is going to honor --keep-file-symbols (and will also >>>> have the other issues fixed). Question is whether we should use that >>>> option (for the symbol table as a whole to make sense), or whether >>>> instead we should (by default) strip the symbol table as well. >>> >>> Without any feedback / ack I guess I'll consider this of no interest >>> (despite having heard otherwise, triggering me to put together the >>> patch in the first place), and put on the pile of effectively >>> rejected patches. >> >> I did a test for this patch on my x86 machine and I think this patch is >> doing the correct thing, so: >> >> Tested-by: Henry Wang <Henry.Wang@arm.com> >> > > Because there was no Arm EFI environment in hand at the time, Henry only > tested the x86 part.I have setup an Arm platform with UEFI v2.70 (EDK > II, 0x00010000) today, and this patch works well when boot Xen as an EFI > application from UEFI shell. > > But the binaries sizes are the same with/without this patch. Is it expected? > I have enabled: > CONFIG_DEBUG=y > CONFIG_DEBUG_INFO=y Well, the way "xen" is built (and "xen.efi" only being an alias thereof), debug info is stripped in the course. That's quite different from x86, where - with a new enough linker - debug info is retained while linking (and it is truly linking by which xen.efi is built), and hence can make sense to strip while installing. > Is there anything I should be aware to test this patch? > > -rwxrwxr-x 1 weic weic 1081504 Jul 7 18:43 xen > -rwxrwxr-x 1 weic weic 1081504 Jul 7 19:43 xen > > Tested-by (Arm only): Wei Chen <Wei.Chen@arm.com> Thanks. Btw the proper form of the tag, as of a couple of months ago, is Tested-by: Wei Chen <Wei.Chen@arm.com> # arm Jan
--- a/xen/Makefile +++ b/xen/Makefile @@ -465,6 +465,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)) @@ -489,6 +505,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; \
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> --- GNU strip 2.38 appears to have issues when acting on a PE binary: - 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 retain its size (or shrink), - linker version is changed in and timestamp zapped from the header. Older GNU strip (observed with 2.35.1) doesn't work at all ("Data Directory size (1c) exceeds space left in section (8)"). Future GNU strip is going to honor --keep-file-symbols (and will also have the other issues fixed). Question is whether we should use that option (for the symbol table as a whole to make sense), or whether instead we should (by default) strip the symbol table as well.