[PATCH] riscv: additional crash information

Sean Anderson seanga2 at gmail.com
Sat Aug 1 16:05:40 CEST 2020


On 8/1/20 9:59 AM, Heinrich Schuchardt wrote:
> On 8/1/20 3:53 PM, Sean Anderson wrote:
>> On 7/29/20 4:36 PM, Heinrich Schuchardt wrote:
>>> If an exception occurs, the relocated program counter and return address
>>> are required for an analysis.
>>
>> Thanks for putting together a patch for this. I realized after I added
>> the feature originally that I had left out RA from the default error
>> info, but I never got around to fixing it.
>>
>>>
>>> With this patch you get:
>>>
>>>     => exception undefined
>>>
>>>     Unhandled exception: Illegal instruction
>>>     EPC: 0000000080595908 RA: 000000008059c0c6 TVAL: 000000008030c01e
>>>     EPC: 0000000080007908 RA: 000000008000e0c6 reloc
>>>
>>> We can use the relocated addresses to find the involved functions in
>>> u.boot.map:
>>>
>>>     .text.do_undefined
>>>                 0x0000000080007908        0x8 cmd/built-in.o
>>>     .text.cmd_process
>>>                 0x000000008000dfcc      0x11a common/built-in.o
>>>                 0x000000008000dfcc                cmd_process
>>>
>>> If an exception occurs in an UEFI binary additionally the load addresses of
>>> the UEFI binaries are needed. With this patch:
>>>
>>>     => setenv efi_selftest exception
>>>     => bootefi selftest
>>>
>>>     Unhandled exception: Illegal instruction
>>>     EPC: 000000008042e18a RA: 000000008042e18a TVAL: 000000008030c01e
>>>     EPC: 000000007fea018a RA: 000000007fea018a reloc
>>>
>>>     UEFI image [0x0000000000000000:0xffffffffffffffff] '/\selftest'
>>>     UEFI image [0x000000008042e000:0x000000008042e43f] pc=0x18a '/bug.efi'
>>>
>>> The value pc=0x18a matches the position of the illegal instruction in
>>> efi_selftest_miniapp_exception.efi (loaded as /bug.efi);
>>>
>>>     asm volatile (".word 0xffffffff\n");
>>>
>>>     00000180   93 85 C5 11  1C 64 22 85  82 97 FF FF  FF FF 1C 64
>>>
>>> Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
>>> ---
>>>  arch/riscv/lib/interrupts.c | 57 +++++++++++++++++++++++--------------
>>>  1 file changed, 35 insertions(+), 22 deletions(-)
>>>
>>> diff --git a/arch/riscv/lib/interrupts.c b/arch/riscv/lib/interrupts.c
>>> index 074c70ee77..bcdfc5ea3a 100644
>>> --- a/arch/riscv/lib/interrupts.c
>>> +++ b/arch/riscv/lib/interrupts.c
>>> @@ -10,36 +10,43 @@
>>>   */
>>>
>>>  #include <common.h>
>>> +#include <efi_loader.h>
>>>  #include <hang.h>
>>>  #include <irq_func.h>
>>>  #include <asm/ptrace.h>
>>>  #include <asm/system.h>
>>>  #include <asm/encoding.h>
>>>
>>> +DECLARE_GLOBAL_DATA_PTR;
>>> +
>>> +static void show_efi_loaded_images(uintptr_t epc)
>>> +{
>>> +	efi_print_image_infos((void *)epc);
>>> +}
>>> +
>>>  static void show_regs(struct pt_regs *regs)
>>>  {
>>>  #ifdef CONFIG_SHOW_REGS
>>> -	printf("RA: " REG_FMT " SP:  " REG_FMT " GP:  " REG_FMT "\n",
>>> -	       regs->ra, regs->sp, regs->gp);
>>> -	printf("TP: " REG_FMT " T0:  " REG_FMT " T1:  " REG_FMT "\n",
>>> -	       regs->tp, regs->t0, regs->t1);
>>> -	printf("T2: " REG_FMT " S0:  " REG_FMT " S1:  " REG_FMT "\n",
>>> -	       regs->t2, regs->s0, regs->s1);
>>> -	printf("A0: " REG_FMT " A1:  " REG_FMT " A2:  " REG_FMT "\n",
>>> -	       regs->a0, regs->a1, regs->a2);
>>> -	printf("A3: " REG_FMT " A4:  " REG_FMT " A5:  " REG_FMT "\n",
>>> -	       regs->a3, regs->a4, regs->a5);
>>> -	printf("A6: " REG_FMT " A7:  " REG_FMT " S2:  " REG_FMT "\n",
>>> -	       regs->a6, regs->a7, regs->s2);
>>> -	printf("S3: " REG_FMT " S4:  " REG_FMT " S5:  " REG_FMT "\n",
>>> -	       regs->s3, regs->s4, regs->s5);
>>> -	printf("S6: " REG_FMT " S7:  " REG_FMT " S8:  " REG_FMT "\n",
>>> -	       regs->s6, regs->s7, regs->s8);
>>> -	printf("S9: " REG_FMT " S10: " REG_FMT " S11: " REG_FMT "\n",
>>> -	       regs->s9, regs->s10, regs->s11);
>>> -	printf("T3: " REG_FMT " T4:  " REG_FMT " T5:  " REG_FMT "\n",
>>> -	       regs->t3, regs->t4, regs->t5);
>>> -	printf("T6: " REG_FMT "\n", regs->t6);
>>> +	printf("SP:  " REG_FMT " GP:  " REG_FMT " TP:  " REG_FMT "\n",
>>> +	       regs->sp, regs->gp, regs->tp);
>>> +	printf("T0:  " REG_FMT " T1:  " REG_FMT " T2:  " REG_FMT "\n",
>>> +	       regs->t0, regs->t1, regs->t2);
>>> +	printf("S0:  " REG_FMT " S1:  " REG_FMT " A0:  " REG_FMT "\n",
>>> +	       regs->s0, regs->s1, regs->a0);
>>> +	printf("A1:  " REG_FMT " A2:  " REG_FMT " A3:  " REG_FMT "\n",
>>> +	       regs->a1, regs->a2, regs->a3);
>>> +	printf("A4:  " REG_FMT " A5:  " REG_FMT " A6:  " REG_FMT "\n",
>>> +	       regs->a4, regs->a5, regs->a6);
>>> +	printf("A7:  " REG_FMT " S2:  " REG_FMT " S3:  " REG_FMT "\n",
>>> +	       regs->a7, regs->s2, regs->s3);
>>> +	printf("S4:  " REG_FMT " S5:  " REG_FMT " S6:  " REG_FMT "\n",
>>> +	       regs->s4, regs->s5, regs->s6);
>>> +	printf("S7:  " REG_FMT " S8:  " REG_FMT " S9:  " REG_FMT "\n",
>>> +	       regs->s7, regs->s8, regs->s9);
>>> +	printf("S10: " REG_FMT " S11: " REG_FMT " T3:  " REG_FMT "\n",
>>> +	       regs->s10, regs->s11, regs->t3);
>>> +	printf("T4:  " REG_FMT " T5:  " REG_FMT " T6:  " REG_FMT "\n\n",
>>> +	       regs->t4, regs->t5, regs->t6);
>>>  #endif
>>>  }
>>>
>>> @@ -69,8 +76,14 @@ static void _exit_trap(ulong code, ulong epc, ulong tval, struct pt_regs *regs)
>>>  	else
>>>  		printf("Unhandled exception code: %ld\n", code);
>>>
>>> -	printf("EPC: " REG_FMT " TVAL: " REG_FMT "\n", epc, tval);
>>> +	printf("EPC: " REG_FMT " RA: " REG_FMT " TVAL: " REG_FMT "\n",
>>> +	       epc, regs->ra, tval);
>>> +	if (gd->flags & GD_FLG_RELOC)
>>> +		printf("EPC: " REG_FMT " RA: " REG_FMT " reloc\n\n",
>>
>> I'm not so sure about the wording of this last line. Perhaps we could
>> reword this to something like "EPC: ... RA: ... (adjusted for relocation
>> offset)"? That's a bit wordy for my taste, but I think it's less
>> ambiguous than what you have now.
> 
> ARM has (see s://u-boot.readthedocs.io/en/latest/develop/crash_dumps.html):
> 
> "Synchronous Abort" handler, esr 0x02000000
> elr: 000000000000eb64 lr : 000000000001f9d0 (reloc)
> elr: 000000007ff40b64 lr : 000000007ff519d0
> 
> Maybe "reloc adjusted"?

Yeah that sounds good to me. With that change,

Reviewed-by: Sean Anderson <seanga2 at gmail.com>
Tested-by: Sean Anderson <seanga2 at gmail.com>


More information about the U-Boot mailing list