Re: [PATCH] xen: add support for crash dump analysis with xen.efi

[Jan Beulich <jbeulich@xxxxxxxx>]

On 21.04.2023 15:59, Juergen Gross wrote:
> Today it is not possible to analyse crash dumps of a system in
> hypervisor mode when it had been booted via EFI, as the crash utility
> doesn't understand the file format of xen.efi.
> 
> This can easily be solved by creating an ELF file from xen.efi via
> objcopy. Using that file as name list for crash enables the user to
> analyse the dump in hypervisor mode.
> 
> Signed-off-by: Juergen Gross <jgross@xxxxxxxx>
> ---
>  xen/Kconfig.debug     | 5 ++++-
>  xen/Makefile          | 4 ++++
>  xen/arch/x86/Makefile | 3 +++
>  3 files changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/Kconfig.debug b/xen/Kconfig.debug
> index 94e818ee09..4aec0fd3aa 100644
> --- a/xen/Kconfig.debug
> +++ b/xen/Kconfig.debug
> @@ -138,6 +138,9 @@ config DEBUG_INFO
>         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).
> +       done outside the Xen build environment). Note that stripping
> +       xen.efi using "INSTALL_EFI_STRIP" will disable the building of
> +       xen.efi.elf, which is needed for "crash" dump analysis of systems
> +       booted via EFI.

I'm afraid I don't understand this: INSTALL_EFI_STRIP only affects what
may (optionally) be placed on the EFI partition. The normally installed
xen.efi should be unaffected by this; an intermediate xen.efi.stripped
is created instead.

> --- a/xen/Makefile
> +++ b/xen/Makefile
> @@ -505,6 +505,9 @@ _install: $(TARGET)$(CONFIG_XEN_INSTALL_SUFFIX)
>               if [ -e $(TARGET).efi.map ]; then \
>                       $(INSTALL_DATA) $(TARGET).efi.map 
> $(D)$(DEBUG_DIR)/$(T)-$(XEN_FULLVERSION).efi.map; \
>               fi; \
> +             if [ -e $(TARGET).efi.elf ]; then \
> +                     $(INSTALL_DATA) $(TARGET).efi.elf 
> $(D)$(DEBUG_DIR)/$(T)-$(XEN_FULLVERSION).efi.elf; \
> +             fi; \

To avoid the risk of the two going out of sync (as also to limit
redundancy), could you wrap the earlier "if" in a "for x in map elf;
do ...; done" loop?

> --- a/xen/arch/x86/Makefile
> +++ b/xen/arch/x86/Makefile
> @@ -224,6 +224,9 @@ endif
>       $(MAKE) $(build)=$(@D) .$(@F).1r.o .$(@F).1s.o
>       $(LD) $(call EFI_LDFLAGS,$(VIRT_BASE)) -T $(obj)/efi.lds -N $< \
>             $(@D)/.$(@F).1r.o $(@D)/.$(@F).1s.o $(orphan-handling-y) 
> $(note_file_option) -o $@
> +ifeq ($(CONFIG_DEBUG_INFO),y)
> +     $(if $(filter --strip-debug,$(EFI_LDFLAGS)),,$(OBJCOPY) -O elf64-x86-64 
> $@ $@.elf)
> +endif
>       $(NM) -pa --format=sysv $(@D)/$(@F) \
>               | $(objtree)/tools/symbols --all-symbols --xensyms --sysv 
> --sort >$(@D)/$(@F).map
>       rm -f $(@D)/.$(@F).[0-9]* $(@D)/..$(@F).[0-9]*

Personally I think that - in case of build problems - having the map file
created is more important (and less likely to fail) than the ELF one, so
I'd prefer if the two steps could be ordered the other way around.

Further I vaguely recall possible issues with entirely blank make rule
lines. Plus having some trace of the command in a verbose log perhaps
also wouldn't hurt. IOW maybe better add another use of the colon command
here (we already have some, at least through $(MKRELOC)):

ifeq ($(CONFIG_DEBUG_INFO),y)
        $(if $(filter --strip-debug,$(EFI_LDFLAGS)),:)$(OBJCOPY) -O 
elf64-x86-64 $@ $@.elf
endif

?

Finally - do you really need to copy all the non-debug sections as well?
Might --only-keep-debug be helpful here (provided it works for a PE/COFF
-> ELF copy operation; it looks to do so in my simple experiments)?

Jan