From 5e3ab1894e1ef0520925038f8d4e4a451e841345 Mon Sep 17 00:00:00 2001 From: Konstantin Belousov Date: Tue, 24 Dec 2024 18:39:38 +0200 Subject: [PATCH 1/3] amd64: extract code to print fault details from trap_fatal() into a new helper Suggested by: markj Reviewed by: kevans, markj Sponsored by: The FreeBSD Foundation MFC after: 1 week Differential revision: https://reviews.freebsd.org/D48186 --- sys/amd64/amd64/trap.c | 29 ++++++++++++++++++++++------- 1 file changed, 22 insertions(+), 7 deletions(-) diff --git a/sys/amd64/amd64/trap.c b/sys/amd64/amd64/trap.c index 4590be501d64..3b23a34662d5 100644 --- a/sys/amd64/amd64/trap.c +++ b/sys/amd64/amd64/trap.c @@ -107,6 +107,7 @@ void trap_check(struct trapframe *frame); void dblfault_handler(struct trapframe *frame); static int trap_pfault(struct trapframe *, bool, int *, int *); +static void trap_diag(struct trapframe *, vm_offset_t); static void trap_fatal(struct trapframe *, vm_offset_t); #ifdef KDTRACE_HOOKS static bool trap_user_dtrace(struct trapframe *, @@ -150,6 +151,13 @@ static const char *const trap_msg[] = { [T_DTRACE_RET] = "DTrace pid return trap", }; +static const char * +traptype_to_msg(u_int type) +{ + return (type < nitems(trap_msg) ? trap_msg[type] : + "unknown/reserved trap"); +} + static int uprintf_signal; SYSCTL_INT(_machdep, OID_AUTO, uprintf_signal, CTLFLAG_RWTUN, &uprintf_signal, 0, @@ -857,15 +865,12 @@ after_vmfault: } static void -trap_fatal(struct trapframe *frame, vm_offset_t eva) +trap_diag(struct trapframe *frame, vm_offset_t eva) { int code, ss; u_int type; struct soft_segment_descriptor softseg; struct user_segment_descriptor *gdt; -#ifdef KDB - bool handled; -#endif code = frame->tf_err; type = frame->tf_trapno; @@ -925,8 +930,20 @@ trap_fatal(struct trapframe *frame, vm_offset_t eva) printf("r13: %016lx r14: %016lx r15: %016lx\n", frame->tf_r13, frame->tf_r14, frame->tf_r15); + printf("trap number = %d\n", type); +} + +static void +trap_fatal(struct trapframe *frame, vm_offset_t eva) +{ + u_int type; + + type = frame->tf_trapno; + trap_diag(frame, eva); #ifdef KDB if (debugger_on_trap) { + bool handled; + kdb_why = KDB_WHY_TRAP; handled = kdb_trap(type, 0, frame); kdb_why = KDB_WHY_UNSET; @@ -934,9 +951,7 @@ trap_fatal(struct trapframe *frame, vm_offset_t eva) return; } #endif - printf("trap number = %d\n", type); - panic("%s", type < nitems(trap_msg) ? trap_msg[type] : - "unknown/reserved trap"); + panic("%s", traptype_to_msg(type)); } #ifdef KDTRACE_HOOKS From dd2b5443644505af51c95503898ab363e7d7c29d Mon Sep 17 00:00:00 2001 From: Konstantin Belousov Date: Tue, 24 Dec 2024 04:35:16 +0200 Subject: [PATCH 2/3] amd64: on any fault during call to EFI RT, restore execution and print fault details The fault info should be useful to see what specifically BIOS tried to do and why it faulted. E.g. it might allow to see which EFI memory segment needs to be mapped in addition to normal runtime segments, to work around the fault. Reviewed by: kevans, markj Sponsored by: The FreeBSD Foundation MFC after: 1 week Differential revision: https://reviews.freebsd.org/D48186 --- sys/amd64/amd64/trap.c | 30 ++++++++++++++++++------------ 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/sys/amd64/amd64/trap.c b/sys/amd64/amd64/trap.c index 3b23a34662d5..8e9b115ef224 100644 --- a/sys/amd64/amd64/trap.c +++ b/sys/amd64/amd64/trap.c @@ -423,6 +423,20 @@ trap(struct trapframe *frame) KASSERT(cold || td->td_ucred != NULL, ("kernel trap doesn't have ucred")); + + /* + * Most likely, EFI RT faulted. This check prevents + * kdb from handling breakpoints set on the BIOS text, + * if such option is ever needed. + */ + if ((td->td_pflags & TDP_EFIRT) != 0 && + curpcb->pcb_onfault != NULL && type != T_PAGEFLT) { + trap_diag(frame, 0); + printf("EFI RT fault %s\n", traptype_to_msg(type)); + frame->tf_rip = (long)curpcb->pcb_onfault; + return; + } + switch (type) { case T_PAGEFLT: /* page fault */ (void)trap_pfault(frame, false, NULL, NULL); @@ -586,18 +600,6 @@ trap(struct trapframe *frame) * FALLTHROUGH (TRCTRAP kernel mode, kernel address) */ case T_BPTFLT: - /* - * Most likely, EFI RT hitting INT3. This - * check prevents kdb from handling - * breakpoints set on the BIOS text, if such - * option is ever needed. - */ - if ((td->td_pflags & TDP_EFIRT) != 0 && - curpcb->pcb_onfault != NULL) { - frame->tf_rip = (long)curpcb->pcb_onfault; - return; - } - /* * If KDB is enabled, let it handle the debugger trap. * Otherwise, debugger traps "can't happen". @@ -857,6 +859,10 @@ trap_pfault(struct trapframe *frame, bool usermode, int *signo, int *ucode) after_vmfault: if (td->td_intr_nesting_level == 0 && curpcb->pcb_onfault != NULL) { + if ((td->td_pflags & TDP_EFIRT) != 0) { + trap_diag(frame, eva); + printf("EFI RT page fault\n"); + } frame->tf_rip = (long)curpcb->pcb_onfault; return (0); } From 3e8f4a30594fad6784504d019613ad815b6c9dc5 Mon Sep 17 00:00:00 2001 From: Ahmad Khalifa Date: Sat, 21 Dec 2024 23:09:54 +0200 Subject: [PATCH 3/3] efirt: use correct ABI for runtime EFI functions When calling EFI RT methods through no fault path. MFC after: 1 week --- sys/dev/efidev/efirt.c | 31 ++++++++++++++++--------------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/sys/dev/efidev/efirt.c b/sys/dev/efidev/efirt.c index 8cada0d5061b..fcbed48af395 100644 --- a/sys/dev/efidev/efirt.c +++ b/sys/dev/efidev/efirt.c @@ -490,31 +490,32 @@ efi_rt_arch_call_nofault(struct efirt_callinfo *ec) switch (ec->ec_argcnt) { case 0: - ec->ec_efi_status = ((register_t (*)(void))ec->ec_fptr)(); + ec->ec_efi_status = ((register_t EFIABI_ATTR (*)(void)) + ec->ec_fptr)(); break; case 1: - ec->ec_efi_status = ((register_t (*)(register_t))ec->ec_fptr) - (ec->ec_arg1); + ec->ec_efi_status = ((register_t EFIABI_ATTR (*)(register_t)) + ec->ec_fptr)(ec->ec_arg1); break; case 2: - ec->ec_efi_status = ((register_t (*)(register_t, register_t)) - ec->ec_fptr)(ec->ec_arg1, ec->ec_arg2); + ec->ec_efi_status = ((register_t EFIABI_ATTR (*)(register_t, + register_t))ec->ec_fptr)(ec->ec_arg1, ec->ec_arg2); break; case 3: - ec->ec_efi_status = ((register_t (*)(register_t, register_t, - register_t))ec->ec_fptr)(ec->ec_arg1, ec->ec_arg2, - ec->ec_arg3); + ec->ec_efi_status = ((register_t EFIABI_ATTR (*)(register_t, + register_t, register_t))ec->ec_fptr)(ec->ec_arg1, + ec->ec_arg2, ec->ec_arg3); break; case 4: - ec->ec_efi_status = ((register_t (*)(register_t, register_t, - register_t, register_t))ec->ec_fptr)(ec->ec_arg1, - ec->ec_arg2, ec->ec_arg3, ec->ec_arg4); + ec->ec_efi_status = ((register_t EFIABI_ATTR (*)(register_t, + register_t, register_t, register_t))ec->ec_fptr)( + ec->ec_arg1, ec->ec_arg2, ec->ec_arg3, ec->ec_arg4); break; case 5: - ec->ec_efi_status = ((register_t (*)(register_t, register_t, - register_t, register_t, register_t))ec->ec_fptr)( - ec->ec_arg1, ec->ec_arg2, ec->ec_arg3, ec->ec_arg4, - ec->ec_arg5); + ec->ec_efi_status = ((register_t EFIABI_ATTR (*)(register_t, + register_t, register_t, register_t, register_t)) + ec->ec_fptr)(ec->ec_arg1, ec->ec_arg2, ec->ec_arg3, + ec->ec_arg4, ec->ec_arg5); break; default: panic("efi_rt_arch_call: %d args", (int)ec->ec_argcnt);