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

[Frediano Ziglio <freddy77@xxxxxxxxx>]

On Mon, 10 Nov 2025 at 13:40, Jan Beulich <jbeulich@xxxxxxxx> wrote:
>
> On 10.11.2025 13:58, Frediano Ziglio wrote:
> > --- a/xen/arch/x86/Makefile
> > +++ b/xen/arch/x86/Makefile
> > @@ -228,17 +228,21 @@ 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 $(TARGET)-syms.efi
> > +     $(NM) -pa --format=sysv $(TARGET)-syms.efi \
> >               | $(objtree)/tools/symbols --all-symbols --xensyms --sysv 
> > --sort \
> >               > $@.map
>
> This part if fine with me now.
>
> > -ifeq ($(CONFIG_DEBUG_INFO),y)
> > -     $(if $(filter --strip-debug,$(EFI_LDFLAGS)),:$(space))$(OBJCOPY) -O 
> > elf64-x86-64 $@ $@.elf
> > +ifeq ($(CONFIG_DEBUG_INFO)_$(filter --strip-debug,$(EFI_LDFLAGS)),y_)
>
> This extra change looks correct to me, yet I wonder if doing it this way isn't
> fragile. If EFI_LDFLAGS wasn't set globally, but only for xen.efi, aiui this
> wouldn't work anymore.
>

Maybe, but it's not a regression of this patch, the same test was
present before, just using another syntax

> > +     $(OBJCOPY) -O elf64-x86-64 $(TARGET)-syms.efi $@.elf
> > +     $(STRIP) $(TARGET)-syms.efi -o $@.tmp
> > +else
> > +     mv -f $(TARGET)-syms.efi -o $@.tmp
>
> This, while I think I understand why you do it, looks somewhat odd. Plus the
> reason you do it is, like ...
>

Can you explain why "it looks somewhat odd" ?

> >  endif
> > -     rm -f $(dot-target).[0-9]* $(@D)/..$(@F).[0-9]*
> >  ifeq ($(CONFIG_XEN_IBT),y)
> > -     $(SHELL) $(srctree)/tools/check-endbr.sh $@
> > +     $(SHELL) $(srctree)/tools/check-endbr.sh $@.tmp
> >  endif
> > +     mv -f $@.tmp $@
> > +     rm -f $(dot-target).[0-9]* $(@D)/..$(@F).[0-9]*
> >  else
> >  $(TARGET).efi: FORCE
> >       rm -f $@
>
> ... (still) most of this, unrelated (and, as before, not mentioned at all in
> the description).
>

I will remove it for now.

> Jan


Frediano