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

Juergen Gross posted 1 patch 1 year ago
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/20230421135933.23353-1-jgross@suse.com
There is a newer version of this series
xen/Kconfig.debug     | 5 ++++-
xen/Makefile          | 4 ++++
xen/arch/x86/Makefile | 3 +++
3 files changed, 11 insertions(+), 1 deletion(-)
[PATCH] xen: add support for crash dump analysis with xen.efi
Posted by Juergen Gross 1 year ago
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@suse.com>
---
 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.
 
 endmenu
diff --git a/xen/Makefile b/xen/Makefile
index 2710d7327e..db50dcb502 100644
--- 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; \
 		ln -sf $(T)-$(XEN_FULLVERSION).efi $(D)$(EFI_DIR)/$(T)-$(XEN_VERSION).$(XEN_SUBVERSION).efi; \
 		ln -sf $(T)-$(XEN_FULLVERSION).efi $(D)$(EFI_DIR)/$(T)-$(XEN_VERSION).efi; \
 		ln -sf $(T)-$(XEN_FULLVERSION).efi $(D)$(EFI_DIR)/$(T).efi; \
@@ -539,6 +542,7 @@ _uninstall:
 	rm -f $(D)$(DEBUG_DIR)/$(T)-syms-$(XEN_FULLVERSION).map
 	rm -f $(D)$(EFI_DIR)/$(T)-$(XEN_FULLVERSION).efi
 	rm -f $(D)$(EFI_DIR)/$(T)-$(XEN_VERSION).$(XEN_SUBVERSION).efi
+	rm -f $(D)$(DEBUG_DIR)/$(T)-$(XEN_FULLVERSION).efi.elf
 	rm -f $(D)$(DEBUG_DIR)/$(T)-$(XEN_FULLVERSION).efi.map
 	rm -f $(D)$(EFI_DIR)/$(T)-$(XEN_VERSION).efi
 	rm -f $(D)$(EFI_DIR)/$(T).efi
diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
index fc9487aa40..87ce00addf 100644
--- 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]*
-- 
2.35.3
Re: [PATCH] xen: add support for crash dump analysis with xen.efi
Posted by Jan Beulich 1 year ago
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@suse.com>
> ---
>  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
Re: [PATCH] xen: add support for crash dump analysis with xen.efi
Posted by Juergen Gross 1 year ago
On 24.04.23 14:01, Jan Beulich wrote:
> 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@suse.com>
>> ---
>>   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.

Oh, obviously I misinterpreted something. I'll drop this hunk.

> 
>> --- 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?

Sure, will do.

> 
>> --- 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.

Fine with me.

> 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
> 
> ?

Okay.

> 
> 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)?

No, doesn't work (objcopy does, but not crash with that file):

   crash: xen.efi.elf: no text and data contents


Juergen
Re: [PATCH] xen: add support for crash dump analysis with xen.efi
Posted by Jan Beulich 1 year ago
On 24.04.2023 15:44, Juergen Gross wrote:
> On 24.04.23 14:01, Jan Beulich wrote:
>> 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)?
> 
> No, doesn't work (objcopy does, but not crash with that file):
> 
>    crash: xen.efi.elf: no text and data contents

Oh, wow. What use do they have for the .text / .data contents? I very much
hope they use the in-memory Xen image for analysis, not what they may read
out of the image.

In any event, please add half a sentence to the description to mention
this aspect.

Jan
Re: [PATCH] xen: add support for crash dump analysis with xen.efi
Posted by Juergen Gross 1 year ago
On 24.04.23 16:09, Jan Beulich wrote:
> On 24.04.2023 15:44, Juergen Gross wrote:
>> On 24.04.23 14:01, Jan Beulich wrote:
>>> 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)?
>>
>> No, doesn't work (objcopy does, but not crash with that file):
>>
>>     crash: xen.efi.elf: no text and data contents
> 
> Oh, wow. What use do they have for the .text / .data contents? I very much
> hope they use the in-memory Xen image for analysis, not what they may read
> out of the image.

I think crash is fine in this regard. At least __start_xen() seems to be all
zeroes in my test dump, which is fine for an .init function after the system
is up and running. :-)

> In any event, please add half a sentence to the description to mention
> this aspect.

Okay.


Juergen