Skip site navigation (1)Skip section navigation (2) 
Date:      Sat, 14 Mar 2026 13:46:00 +0200
From:      Konstantin Belousov <kostikbel@gmail.com>
To:        A FreeBSD User <freebsd@walstatt-de.de>
Cc:        "Herbert J. Skuhra" <herbert@gojira.at>, Konstantin Belousov <kib@freebsd.org>, src-committers@freebsd.org, dev-commits-src-all@freebsd.org, dev-commits-src-main@freebsd.org
Subject:   Re: git: 914a53570750 - main - amd64: move efirt trap checks into the helper
Message-ID:  <abVKeC53t2ch7WAn@kib.kiev.ua>
In-Reply-To: <20260314120921.314e1677@thor.sb211.local>
References:  <69b494da.1d7bf.7cef39b3@gitrepo.freebsd.org> <87a4wasris.wl-herbert@gojira.at> <20260314120921.314e1677@thor.sb211.local>
index | next in thread | previous in thread | raw e-mail 

On Sat, Mar 14, 2026 at 12:15:33PM +0100, A FreeBSD User wrote:
> Am Tage des Herren Sat, 14 Mar 2026 10:13:15 +0100
> "Herbert J. Skuhra" <herbert@gojira.at> schrieb:
> 
> > On Fri, 13 Mar 2026 23:51:06 +0100, Konstantin Belousov wrote:
> > > 
> > > The branch main has been updated by kib:
> > > 
> > > URL: https://cgit.FreeBSD.org/src/commit/?id=914a53570750ce5a104a5870403d7669656fddc3
> > > 
> > > commit 914a53570750ce5a104a5870403d7669656fddc3
> > > Author:     Konstantin Belousov <kib@FreeBSD.org>
> > > AuthorDate: 2026-03-11 11:53:52 +0000
> > > Commit:     Konstantin Belousov <kib@FreeBSD.org>
> > > CommitDate: 2026-03-13 22:47:13 +0000
> > > 
> > >     amd64: move efirt trap checks into the helper
> > >     
> > >     Reviewed by:    imp, jhb
> > >     Sponsored by:   The FreeBSD Foundation
> > >     MFC after:      1 week
> > >     Differential revision:  https://reviews.freebsd.org/D55808
> > > ---
> > >  sys/amd64/amd64/trap.c | 55 ++++++++++++++++++++++++--------------------------
> > >  1 file changed, 26 insertions(+), 29 deletions(-)  
> > 
> > This is causing a kernel panic here.
> > 
> 
> me too.

My polite answer is that the messages do not provide useful information.
I got a useful trace from Peter Holm, and I think I know what is going
on there. My current patch is below, I will commit it after Peter'
confirmation.

If you have a different issue, you should report it in a way that allows
to diagnose the problem.

>From 7097dd1ec28472594a6fbb2f5bd8b6f88459f0e9 Mon Sep 17 00:00:00 2001
From: Konstantin Belousov <kib@FreeBSD.org>
Date: Sat, 14 Mar 2026 13:40:07 +0200
Subject: [PATCH] amd64: do reset %rip after page fault if pcb_onfault is set

for any kernel page fault, and not only for EFIRT case.

Reported by:	pho
Fixes:	914a53570750ce5a104a5870403d7669656fddc3
Sponsored by:	The FreeBSD Foundation
MFC after:	1 week
---
 sys/amd64/amd64/trap.c | 33 ++++++++++++++++++++-------------
 1 file changed, 20 insertions(+), 13 deletions(-)

diff --git a/sys/amd64/amd64/trap.c b/sys/amd64/amd64/trap.c
index 4bf56226d076..3a9323936d2d 100644
--- a/sys/amd64/amd64/trap.c
+++ b/sys/amd64/amd64/trap.c
@@ -219,15 +219,19 @@ trap_uprintf_signal(struct thread *td, struct trapframe *frame, register_t addr,
 }
 
 static bool
-trap_check_efirt(struct thread *td, struct trapframe *frame)
+trap_check_pcb_onfault(struct thread *td, struct trapframe *frame)
 {
-	/*
-	 * 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) {
+	bool res = false;
+
+	if (curpcb->pcb_onfault == NULL)
+		return (res);
+
+	if (__predict_false((td->td_pflags & TDP_EFIRT) != 0)) {
+		/*
+		 * Most likely, EFI RT faulted.  This check prevents
+		 * kdb from handling breakpoints set on the BIOS text,
+		 * if such option is ever needed.
+		 */
 		u_long cnt = atomic_fetchadd_long(&cnt_efirt_faults, 1);
 
 		if ((print_efirt_faults == 1 && cnt == 0) ||
@@ -236,10 +240,13 @@ trap_check_efirt(struct thread *td, struct trapframe *frame)
 			    traptype_to_msg(frame->tf_trapno));
 			trap_diag(frame, 0);
 		}
-		frame->tf_rip = (long)curpcb->pcb_onfault;
-		return (true);
+		res = true;
+	} else if (frame->tf_trapno == T_PAGEFLT) {
+		res = true;
 	}
-	return (false);
+	if (res)
+		frame->tf_rip = (register_t)curpcb->pcb_onfault;
+	return (res);
 }
 
 static void
@@ -494,7 +501,7 @@ trap(struct trapframe *frame)
 		KASSERT(cold || td->td_ucred != NULL,
 		    ("kernel trap doesn't have ucred"));
 
-		if (type != T_PAGEFLT && trap_check_efirt(td, frame))
+		if (type != T_PAGEFLT && trap_check_pcb_onfault(td, frame))
 			return;
 
 		switch (type) {
@@ -904,7 +911,7 @@ trap_pfault(struct trapframe *frame, bool usermode, int *signo, int *ucode)
 		return (1);
 after_vmfault:
 	if (td->td_intr_nesting_level == 0 &&
-	    trap_check_efirt(td, frame))
+	    trap_check_pcb_onfault(td, frame))
 		return (0);
 	trap_fatal(frame, eva);
 	return (-1);
-- 
2.53.0
home | help

Want to link to this message? Use this
URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?abVKeC53t2ch7WAn>