diff mbox series

[U-Boot,1/1] arm: print information about loaded UEFI images

Message ID 20190404202347.17041-1-xypron.glpk@gmx.de
State Accepted, archived
Delegated to: Heinrich Schuchardt
Headers show
Series [U-Boot,1/1] arm: print information about loaded UEFI images | expand

Commit Message

Heinrich Schuchardt April 4, 2019, 8:23 p.m. UTC 
If an exception occurs in a UEFI loaded image we need the start address of
the image to determine the relocation offset.

This patch adds the necessary lines after the registers in the crash dump
for armv8. A possible output would be:

UEFI image [0x00000000bffe6000:0x00000000bffe631f] pc=0x138 '/\snp.efi'

With the offset 0x138 we can now find the relevant instruction in the
disassembled 'snp.efi' binary.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 arch/arm/lib/interrupts_64.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

--
2.20.1

Comments

AKASHI Takahiro April 9, 2019, 1:31 a.m. UTC | #1 
On Thu, Apr 04, 2019 at 10:23:47PM +0200, Heinrich Schuchardt wrote:
> If an exception occurs in a UEFI loaded image we need the start address of
> the image to determine the relocation offset.
> 
> This patch adds the necessary lines after the registers in the crash dump
> for armv8. A possible output would be:
> 
> UEFI image [0x00000000bffe6000:0x00000000bffe631f] pc=0x138 '/\snp.efi'
> 
> With the offset 0x138 we can now find the relevant instruction in the

'pc' is misleading, it is just an offset from the start address of
loaded image (reloc_base, in this case, 0xbffe6000). Why do we need this
field?

-Takahiro Akashi

> disassembled 'snp.efi' binary.
> 
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
>  arch/arm/lib/interrupts_64.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/arch/arm/lib/interrupts_64.c b/arch/arm/lib/interrupts_64.c
> index 458319ab48..0bfdb8d93d 100644
> --- a/arch/arm/lib/interrupts_64.c
> +++ b/arch/arm/lib/interrupts_64.c
> @@ -25,6 +25,11 @@ int disable_interrupts(void)
>  	return 0;
>  }
> 
> +static void show_efi_loaded_images(struct pt_regs *regs)
> +{
> +	efi_print_image_infos((void *)regs->elr);
> +}
> +
>  void show_regs(struct pt_regs *regs)
>  {
>  	int i;
> @@ -49,6 +54,7 @@ void do_bad_sync(struct pt_regs *pt_regs, unsigned int esr)
>  	efi_restore_gd();
>  	printf("Bad mode in \"Synchronous Abort\" handler, esr 0x%08x\n", esr);
>  	show_regs(pt_regs);
> +	show_efi_loaded_images(pt_regs);
>  	panic("Resetting CPU ...\n");
>  }
> 
> @@ -60,6 +66,7 @@ void do_bad_irq(struct pt_regs *pt_regs, unsigned int esr)
>  	efi_restore_gd();
>  	printf("Bad mode in \"Irq\" handler, esr 0x%08x\n", esr);
>  	show_regs(pt_regs);
> +	show_efi_loaded_images(pt_regs);
>  	panic("Resetting CPU ...\n");
>  }
> 
> @@ -71,6 +78,7 @@ void do_bad_fiq(struct pt_regs *pt_regs, unsigned int esr)
>  	efi_restore_gd();
>  	printf("Bad mode in \"Fiq\" handler, esr 0x%08x\n", esr);
>  	show_regs(pt_regs);
> +	show_efi_loaded_images(pt_regs);
>  	panic("Resetting CPU ...\n");
>  }
> 
> @@ -82,6 +90,7 @@ void do_bad_error(struct pt_regs *pt_regs, unsigned int esr)
>  	efi_restore_gd();
>  	printf("Bad mode in \"Error\" handler, esr 0x%08x\n", esr);
>  	show_regs(pt_regs);
> +	show_efi_loaded_images(pt_regs);
>  	panic("Resetting CPU ...\n");
>  }
> 
> @@ -93,6 +102,7 @@ void do_sync(struct pt_regs *pt_regs, unsigned int esr)
>  	efi_restore_gd();
>  	printf("\"Synchronous Abort\" handler, esr 0x%08x\n", esr);
>  	show_regs(pt_regs);
> +	show_efi_loaded_images(pt_regs);
>  	panic("Resetting CPU ...\n");
>  }
> 
> @@ -104,6 +114,7 @@ void do_irq(struct pt_regs *pt_regs, unsigned int esr)
>  	efi_restore_gd();
>  	printf("\"Irq\" handler, esr 0x%08x\n", esr);
>  	show_regs(pt_regs);
> +	show_efi_loaded_images(pt_regs);
>  	panic("Resetting CPU ...\n");
>  }
> 
> @@ -115,6 +126,7 @@ void do_fiq(struct pt_regs *pt_regs, unsigned int esr)
>  	efi_restore_gd();
>  	printf("\"Fiq\" handler, esr 0x%08x\n", esr);
>  	show_regs(pt_regs);
> +	show_efi_loaded_images(pt_regs);
>  	panic("Resetting CPU ...\n");
>  }
> 
> @@ -129,5 +141,6 @@ void __weak do_error(struct pt_regs *pt_regs, unsigned int esr)
>  	efi_restore_gd();
>  	printf("\"Error\" handler, esr 0x%08x\n", esr);
>  	show_regs(pt_regs);
> +	show_efi_loaded_images(pt_regs);
>  	panic("Resetting CPU ...\n");
>  }
> --
> 2.20.1
>
Heinrich Schuchardt April 9, 2019, 4:02 a.m. UTC | #2 
On 4/9/19 3:31 AM, AKASHI Takahiro wrote:
> On Thu, Apr 04, 2019 at 10:23:47PM +0200, Heinrich Schuchardt wrote:
>> If an exception occurs in a UEFI loaded image we need the start address of
>> the image to determine the relocation offset.
>>
>> This patch adds the necessary lines after the registers in the crash dump
>> for armv8. A possible output would be:
>>
>> UEFI image [0x00000000bffe6000:0x00000000bffe631f] pc=0x138 '/\snp.efi'
>>
>> With the offset 0x138 we can now find the relevant instruction in the
>
> 'pc' is misleading, it is just an offset from the start address of
> loaded image (reloc_base, in this case, 0xbffe6000). Why do we need this
> field?

Yes, this is the position of the program counter (pc) where the failure
occurred relative to the start of the image. As described above you need
this offset to find the violating code in the objdump output of your EFI
image.

You could of cause calculate the offset by hand from the other available
information. As we already show addresses relative to gc->reloc_off it
seems to be consistent to show the program counter relative to the
loaded image address here.

If you have a better idea for a label than "pc" I am open to your
suggestion but this would be a patch for efi_print_image_infos() and
does not directly relate to the current patch.

Best regards

Heinrich

>
> -Takahiro Akashi
>
>> disassembled 'snp.efi' binary.
>>
>> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
>> ---
>>  arch/arm/lib/interrupts_64.c | 13 +++++++++++++
>>  1 file changed, 13 insertions(+)
>>
>> diff --git a/arch/arm/lib/interrupts_64.c b/arch/arm/lib/interrupts_64.c
>> index 458319ab48..0bfdb8d93d 100644
>> --- a/arch/arm/lib/interrupts_64.c
>> +++ b/arch/arm/lib/interrupts_64.c
>> @@ -25,6 +25,11 @@ int disable_interrupts(void)
>>  	return 0;
>>  }
>>
>> +static void show_efi_loaded_images(struct pt_regs *regs)
>> +{
>> +	efi_print_image_infos((void *)regs->elr);
>> +}
>> +
>>  void show_regs(struct pt_regs *regs)
>>  {
>>  	int i;
>> @@ -49,6 +54,7 @@ void do_bad_sync(struct pt_regs *pt_regs, unsigned int esr)
>>  	efi_restore_gd();
>>  	printf("Bad mode in \"Synchronous Abort\" handler, esr 0x%08x\n", esr);
>>  	show_regs(pt_regs);
>> +	show_efi_loaded_images(pt_regs);
>>  	panic("Resetting CPU ...\n");
>>  }
>>
>> @@ -60,6 +66,7 @@ void do_bad_irq(struct pt_regs *pt_regs, unsigned int esr)
>>  	efi_restore_gd();
>>  	printf("Bad mode in \"Irq\" handler, esr 0x%08x\n", esr);
>>  	show_regs(pt_regs);
>> +	show_efi_loaded_images(pt_regs);
>>  	panic("Resetting CPU ...\n");
>>  }
>>
>> @@ -71,6 +78,7 @@ void do_bad_fiq(struct pt_regs *pt_regs, unsigned int esr)
>>  	efi_restore_gd();
>>  	printf("Bad mode in \"Fiq\" handler, esr 0x%08x\n", esr);
>>  	show_regs(pt_regs);
>> +	show_efi_loaded_images(pt_regs);
>>  	panic("Resetting CPU ...\n");
>>  }
>>
>> @@ -82,6 +90,7 @@ void do_bad_error(struct pt_regs *pt_regs, unsigned int esr)
>>  	efi_restore_gd();
>>  	printf("Bad mode in \"Error\" handler, esr 0x%08x\n", esr);
>>  	show_regs(pt_regs);
>> +	show_efi_loaded_images(pt_regs);
>>  	panic("Resetting CPU ...\n");
>>  }
>>
>> @@ -93,6 +102,7 @@ void do_sync(struct pt_regs *pt_regs, unsigned int esr)
>>  	efi_restore_gd();
>>  	printf("\"Synchronous Abort\" handler, esr 0x%08x\n", esr);
>>  	show_regs(pt_regs);
>> +	show_efi_loaded_images(pt_regs);
>>  	panic("Resetting CPU ...\n");
>>  }
>>
>> @@ -104,6 +114,7 @@ void do_irq(struct pt_regs *pt_regs, unsigned int esr)
>>  	efi_restore_gd();
>>  	printf("\"Irq\" handler, esr 0x%08x\n", esr);
>>  	show_regs(pt_regs);
>> +	show_efi_loaded_images(pt_regs);
>>  	panic("Resetting CPU ...\n");
>>  }
>>
>> @@ -115,6 +126,7 @@ void do_fiq(struct pt_regs *pt_regs, unsigned int esr)
>>  	efi_restore_gd();
>>  	printf("\"Fiq\" handler, esr 0x%08x\n", esr);
>>  	show_regs(pt_regs);
>> +	show_efi_loaded_images(pt_regs);
>>  	panic("Resetting CPU ...\n");
>>  }
>>
>> @@ -129,5 +141,6 @@ void __weak do_error(struct pt_regs *pt_regs, unsigned int esr)
>>  	efi_restore_gd();
>>  	printf("\"Error\" handler, esr 0x%08x\n", esr);
>>  	show_regs(pt_regs);
>> +	show_efi_loaded_images(pt_regs);
>>  	panic("Resetting CPU ...\n");
>>  }
>> --
>> 2.20.1
>>
>
AKASHI Takahiro April 11, 2019, 5:07 a.m. UTC | #3 
On Tue, Apr 09, 2019 at 06:02:05AM +0200, Heinrich Schuchardt wrote:
> On 4/9/19 3:31 AM, AKASHI Takahiro wrote:
> > On Thu, Apr 04, 2019 at 10:23:47PM +0200, Heinrich Schuchardt wrote:
> >> If an exception occurs in a UEFI loaded image we need the start address of
> >> the image to determine the relocation offset.
> >>
> >> This patch adds the necessary lines after the registers in the crash dump
> >> for armv8. A possible output would be:
> >>
> >> UEFI image [0x00000000bffe6000:0x00000000bffe631f] pc=0x138 '/\snp.efi'
> >>
> >> With the offset 0x138 we can now find the relevant instruction in the
> >
> > 'pc' is misleading, it is just an offset from the start address of
> > loaded image (reloc_base, in this case, 0xbffe6000). Why do we need this
> > field?
> 
> Yes, this is the position of the program counter (pc) where the failure
> occurred relative to the start of the image. As described above you need
> this offset to find the violating code in the objdump output of your EFI
> image.
> 
> You could of cause calculate the offset by hand from the other available
> information. As we already show addresses relative to gc->reloc_off it
> seems to be consistent to show the program counter relative to the
> loaded image address here.
> 
> If you have a better idea for a label than "pc" I am open to your
> suggestion but this would be a patch for efi_print_image_infos() and
> does not directly relate to the current patch.


crash at 0x138 in [0x00000000bffe6000:0x00000000bffe631f] '/\snp.efi'

or

crash at 0x00000000bffe6000+0x138 in '/\snp.efi'

Adding function name would be better, back trace of stack would be much better.

I know no way.

-Takahiro Akashi

> Best regards
> 
> Heinrich
> 
> >
> > -Takahiro Akashi
> >
> >> disassembled 'snp.efi' binary.
> >>
> >> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> >> ---
> >>  arch/arm/lib/interrupts_64.c | 13 +++++++++++++
> >>  1 file changed, 13 insertions(+)
> >>
> >> diff --git a/arch/arm/lib/interrupts_64.c b/arch/arm/lib/interrupts_64.c
> >> index 458319ab48..0bfdb8d93d 100644
> >> --- a/arch/arm/lib/interrupts_64.c
> >> +++ b/arch/arm/lib/interrupts_64.c
> >> @@ -25,6 +25,11 @@ int disable_interrupts(void)
> >>  	return 0;
> >>  }
> >>
> >> +static void show_efi_loaded_images(struct pt_regs *regs)
> >> +{
> >> +	efi_print_image_infos((void *)regs->elr);
> >> +}
> >> +
> >>  void show_regs(struct pt_regs *regs)
> >>  {
> >>  	int i;
> >> @@ -49,6 +54,7 @@ void do_bad_sync(struct pt_regs *pt_regs, unsigned int esr)
> >>  	efi_restore_gd();
> >>  	printf("Bad mode in \"Synchronous Abort\" handler, esr 0x%08x\n", esr);
> >>  	show_regs(pt_regs);
> >> +	show_efi_loaded_images(pt_regs);
> >>  	panic("Resetting CPU ...\n");
> >>  }
> >>
> >> @@ -60,6 +66,7 @@ void do_bad_irq(struct pt_regs *pt_regs, unsigned int esr)
> >>  	efi_restore_gd();
> >>  	printf("Bad mode in \"Irq\" handler, esr 0x%08x\n", esr);
> >>  	show_regs(pt_regs);
> >> +	show_efi_loaded_images(pt_regs);
> >>  	panic("Resetting CPU ...\n");
> >>  }
> >>
> >> @@ -71,6 +78,7 @@ void do_bad_fiq(struct pt_regs *pt_regs, unsigned int esr)
> >>  	efi_restore_gd();
> >>  	printf("Bad mode in \"Fiq\" handler, esr 0x%08x\n", esr);
> >>  	show_regs(pt_regs);
> >> +	show_efi_loaded_images(pt_regs);
> >>  	panic("Resetting CPU ...\n");
> >>  }
> >>
> >> @@ -82,6 +90,7 @@ void do_bad_error(struct pt_regs *pt_regs, unsigned int esr)
> >>  	efi_restore_gd();
> >>  	printf("Bad mode in \"Error\" handler, esr 0x%08x\n", esr);
> >>  	show_regs(pt_regs);
> >> +	show_efi_loaded_images(pt_regs);
> >>  	panic("Resetting CPU ...\n");
> >>  }
> >>
> >> @@ -93,6 +102,7 @@ void do_sync(struct pt_regs *pt_regs, unsigned int esr)
> >>  	efi_restore_gd();
> >>  	printf("\"Synchronous Abort\" handler, esr 0x%08x\n", esr);
> >>  	show_regs(pt_regs);
> >> +	show_efi_loaded_images(pt_regs);
> >>  	panic("Resetting CPU ...\n");
> >>  }
> >>
> >> @@ -104,6 +114,7 @@ void do_irq(struct pt_regs *pt_regs, unsigned int esr)
> >>  	efi_restore_gd();
> >>  	printf("\"Irq\" handler, esr 0x%08x\n", esr);
> >>  	show_regs(pt_regs);
> >> +	show_efi_loaded_images(pt_regs);
> >>  	panic("Resetting CPU ...\n");
> >>  }
> >>
> >> @@ -115,6 +126,7 @@ void do_fiq(struct pt_regs *pt_regs, unsigned int esr)
> >>  	efi_restore_gd();
> >>  	printf("\"Fiq\" handler, esr 0x%08x\n", esr);
> >>  	show_regs(pt_regs);
> >> +	show_efi_loaded_images(pt_regs);
> >>  	panic("Resetting CPU ...\n");
> >>  }
> >>
> >> @@ -129,5 +141,6 @@ void __weak do_error(struct pt_regs *pt_regs, unsigned int esr)
> >>  	efi_restore_gd();
> >>  	printf("\"Error\" handler, esr 0x%08x\n", esr);
> >>  	show_regs(pt_regs);
> >> +	show_efi_loaded_images(pt_regs);
> >>  	panic("Resetting CPU ...\n");
> >>  }
> >> --
> >> 2.20.1
> >>
> >
>
diff mbox series

Patch

diff --git a/arch/arm/lib/interrupts_64.c b/arch/arm/lib/interrupts_64.c
index 458319ab48..0bfdb8d93d 100644
--- a/arch/arm/lib/interrupts_64.c
+++ b/arch/arm/lib/interrupts_64.c
@@ -25,6 +25,11 @@  int disable_interrupts(void)
 	return 0;
 }

+static void show_efi_loaded_images(struct pt_regs *regs)
+{
+	efi_print_image_infos((void *)regs->elr);
+}
+
 void show_regs(struct pt_regs *regs)
 {
 	int i;
@@ -49,6 +54,7 @@  void do_bad_sync(struct pt_regs *pt_regs, unsigned int esr)
 	efi_restore_gd();
 	printf("Bad mode in \"Synchronous Abort\" handler, esr 0x%08x\n", esr);
 	show_regs(pt_regs);
+	show_efi_loaded_images(pt_regs);
 	panic("Resetting CPU ...\n");
 }

@@ -60,6 +66,7 @@  void do_bad_irq(struct pt_regs *pt_regs, unsigned int esr)
 	efi_restore_gd();
 	printf("Bad mode in \"Irq\" handler, esr 0x%08x\n", esr);
 	show_regs(pt_regs);
+	show_efi_loaded_images(pt_regs);
 	panic("Resetting CPU ...\n");
 }

@@ -71,6 +78,7 @@  void do_bad_fiq(struct pt_regs *pt_regs, unsigned int esr)
 	efi_restore_gd();
 	printf("Bad mode in \"Fiq\" handler, esr 0x%08x\n", esr);
 	show_regs(pt_regs);
+	show_efi_loaded_images(pt_regs);
 	panic("Resetting CPU ...\n");
 }

@@ -82,6 +90,7 @@  void do_bad_error(struct pt_regs *pt_regs, unsigned int esr)
 	efi_restore_gd();
 	printf("Bad mode in \"Error\" handler, esr 0x%08x\n", esr);
 	show_regs(pt_regs);
+	show_efi_loaded_images(pt_regs);
 	panic("Resetting CPU ...\n");
 }

@@ -93,6 +102,7 @@  void do_sync(struct pt_regs *pt_regs, unsigned int esr)
 	efi_restore_gd();
 	printf("\"Synchronous Abort\" handler, esr 0x%08x\n", esr);
 	show_regs(pt_regs);
+	show_efi_loaded_images(pt_regs);
 	panic("Resetting CPU ...\n");
 }

@@ -104,6 +114,7 @@  void do_irq(struct pt_regs *pt_regs, unsigned int esr)
 	efi_restore_gd();
 	printf("\"Irq\" handler, esr 0x%08x\n", esr);
 	show_regs(pt_regs);
+	show_efi_loaded_images(pt_regs);
 	panic("Resetting CPU ...\n");
 }

@@ -115,6 +126,7 @@  void do_fiq(struct pt_regs *pt_regs, unsigned int esr)
 	efi_restore_gd();
 	printf("\"Fiq\" handler, esr 0x%08x\n", esr);
 	show_regs(pt_regs);
+	show_efi_loaded_images(pt_regs);
 	panic("Resetting CPU ...\n");
 }

@@ -129,5 +141,6 @@  void __weak do_error(struct pt_regs *pt_regs, unsigned int esr)
 	efi_restore_gd();
 	printf("\"Error\" handler, esr 0x%08x\n", esr);
 	show_regs(pt_regs);
+	show_efi_loaded_images(pt_regs);
 	panic("Resetting CPU ...\n");
 }