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

["Marek Marczykowski-Górecki" <marmarek@invisiblethingslab.com>]

[-- Attachment #1: Type: text/plain, Size: 4583 bytes --]

On Thu, Nov 20, 2025 at 01:59:24PM +0000, Frediano Ziglio wrote:
> On Sat, 15 Nov 2025 at 06:23, Frediano Ziglio <freddy77@gmail.com> wrote:
> >
> > On Fri, 14 Nov 2025 at 19:18, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
> > >
> > > On 14/11/2025 3:40 pm, Oleksii Kurochko wrote:
> > > >
> > > >
> > > > On 11/13/25 4:43 PM, Frediano Ziglio wrote:
> > > >> From: Frediano Ziglio <frediano.ziglio@cloud.com>
> > > >>
> > > >> For xen.gz file we strip all symbols and have an additional
> > > >> xen-syms.efi 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>
> > > > Release-Acked-By: Oleksii Kurochko <oleksii.kurochko@gmail.com>
> > > >
> > > > Thanks.
> > >
> > > Thanks.  Unfortunately CI says no.
> > >
> > > Ubuntu's 20.04, 18.04 and 16.04 all fail:
> > > https://gitlab.com/xen-project/hardware/xen-staging/-/pipelines/2159622869
> > >
> > > From 16.04:
> > >
> > > 2025-11-14T18:01:51.192964Z 01O strip xen-syms.efi -o xen.efi
> > > 2025-11-14T18:01:51.198151Z 01O strip:xen-syms.efi[.init]: relocation count is negative: File truncated
> > > 2025-11-14T18:01:51.198166Z 01O strip: xen.efi: Failed to read debug data section
> > > 2025-11-14T18:01:51.198169Z 01O strip:xen.efi: error copying private BFD data: File truncated
> > > 2025-11-14T18:01:51.198932Z 01O arch/x86/Makefile:207: recipe for target 'xen.efi' failed
> > > 2025-11-14T18:01:51.198937Z 01O make[3]: *** [xen.efi] Error 1
> > > 2025-11-14T18:01:51.199616Z 01O build.mk:90: recipe for target 'xen' failed
> > > 2025-11-14T18:01:51.199619Z 01O make[2]: *** [xen] Error 2
> > > 2025-11-14T18:01:51.200402Z 01O Makefile:600: recipe for target 'xen' failed
> > > 2025-11-14T18:01:51.200409Z 01O make[1]: *** [xen] Error 2
> > >
> > >
> > > I find it hard to believe that the relocation count is really negative,
> > > and given that newer binuitls works, I expect this is a binutils bug.
> > >
> >
> > Unless the message is just misleading I find it hard to have a
> > negative number of items in a container.
> >
> > > Nevertheless, we need some workaround.  Given that the previous
> > > behaviour was not to strip, I think we can reuse that for broken toolchains?
> > >
> >
> > Something like that ?
> >
> > diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
> > index a154ffe6b2..c465eb12e2 100644
> > --- a/xen/arch/x86/Makefile
> > +++ b/xen/arch/x86/Makefile
> > @@ -236,7 +236,9 @@ ifeq ($(CONFIG_DEBUG_INFO),y)
> >         $(if $(filter --strip-debug,$(EFI_LDFLAGS)),:$(space))$(OBJCOPY) \
> >                 -O elf64-x86-64 $(TARGET)-syms.efi $@.elf
> >  endif
> > -       $(STRIP) $(TARGET)-syms.efi -o $@
> > +       $(STRIP) $(TARGET)-syms.efi -o $@ || { \
> > +               LANG=C strip $(TARGET)-syms.efi -o $@ 2>&1 | grep -q \
> > +               "relocation count is negative" && mv -f $(TARGET)-syms.efi $@; }
> >  ifneq ($(CONFIG_DEBUG_INFO),y)
> >         rm -f $(TARGET)-syms.efi
> >  endif
> >
> > It will fall back to not stripping in case that bug is detected. I
> > don't know how to test it.
> > (the LANG=C is to always force the English message).
> >
> 
> It looks like this change works better and CI is happy.
> It duplicates the linking with -s option if the strip fails.
> Yes, it's a hack and almost duplicates the one command above.
> What about it?

Is it guaranteed to match xen-syms.efi?

There is an alternative option: based on observation that Ubuntu 16.04
runs out of support in April 2026 - which is before Xen 4.22 release,
maybe we can drop that test from CI already?

> diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
> index a154ffe6b2..5f5162841e 100644
> --- a/xen/arch/x86/Makefile
> +++ b/xen/arch/x86/Makefile
> @@ -236,7 +236,10 @@ ifeq ($(CONFIG_DEBUG_INFO),y)
>         $(if $(filter --strip-debug,$(EFI_LDFLAGS)),:$(space))$(OBJCOPY) \
>                 -O elf64-x86-64 $(TARGET)-syms.efi $@.elf
>  endif
> -       $(STRIP) $(TARGET)-syms.efi -o $@
> +       $(STRIP) $(TARGET)-syms.efi -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) -s -o $@
>  ifneq ($(CONFIG_DEBUG_INFO),y)
>         rm -f $(TARGET)-syms.efi
>  endif
> 
> Frediano

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]