git: 2c10be9e06d4 - main - arm64: Handle translation faults for thread structures

From: Mark Johnston <markj_at_FreeBSD.org>
Date: Wed, 02 Nov 2022 17:47:17 UTC
The branch main has been updated by markj:

URL: https://cgit.FreeBSD.org/src/commit/?id=2c10be9e06d4577ad8b60a7cb415ecbad60b5370

commit 2c10be9e06d4577ad8b60a7cb415ecbad60b5370
Author:     Mark Johnston <markj@FreeBSD.org>
AuthorDate: 2022-11-02 17:27:27 +0000
Commit:     Mark Johnston <markj@FreeBSD.org>
CommitDate: 2022-11-02 17:46:25 +0000

    arm64: Handle translation faults for thread structures
    
    The break-before-make requirement poses a problem when promoting or
    demoting mappings containing thread structures: a CPU may raise a
    translation fault while accessing curthread, and data_abort() accesses
    the thread again before pmap_fault() can translate the address and
    return.
    
    Normally this isn't a problem because we have a hack to ensure that
    slabs used by the thread zone are always accessed via the direct map,
    where promotions and demotions are rare.  However, this hack doesn't
    work properly with UMA_MD_SMALL_ALLOC disabled, as is the case with
    KASAN configured (since our KASAN implementation does not shadow the
    direct map and so tries to force the use of the kernel map wherever
    possible).
    
    Fix the problem by modifying data_abort() to handle translation faults
    in the kernel map without dereferencing "td", i.e., curthread, and
    without enabling interrupts.  pmap_klookup() has special handling for
    translation faults which makes it safe to call in this context.  Then,
    revert the aforementioned hack.
    
    Reviewed by:    kevans, alc, kib, andrew
    MFC after:      1 month
    Sponsored by:   Juniper Networks, Inc.
    Sponsored by:   Klara, Inc.
    Differential Revision:  https://reviews.freebsd.org/D37231
---
 sys/arm64/arm64/trap.c | 55 ++++++++++++++++++++++++++++++++------------------
 sys/kern/kern_thread.c | 14 +------------
 2 files changed, 36 insertions(+), 33 deletions(-)

diff --git a/sys/arm64/arm64/trap.c b/sys/arm64/arm64/trap.c
index 21843694f48c..f48e33db914e 100644
--- a/sys/arm64/arm64/trap.c
+++ b/sys/arm64/arm64/trap.c
@@ -246,7 +246,6 @@ data_abort(struct thread *td, struct trapframe *frame, uint64_t esr,
     uint64_t far, int lower)
 {
 	struct vm_map *map;
-	struct proc *p;
 	struct pcb *pcb;
 	vm_prot_t ftype;
 	int error, sig, ucode;
@@ -268,28 +267,44 @@ data_abort(struct thread *td, struct trapframe *frame, uint64_t esr,
 	}
 #endif
 
-	pcb = td->td_pcb;
-	p = td->td_proc;
-	if (lower)
-		map = &p->p_vmspace->vm_map;
-	else {
-		intr_enable();
-
+	if (lower) {
+		map = &td->td_proc->p_vmspace->vm_map;
+	} else if (!ADDR_IS_CANONICAL(far)) {
 		/* We received a TBI/PAC/etc. fault from the kernel */
-		if (!ADDR_IS_CANONICAL(far)) {
-			error = KERN_INVALID_ADDRESS;
-			goto bad_far;
+		error = KERN_INVALID_ADDRESS;
+		goto bad_far;
+	} else if (ADDR_IS_KERNEL(far)) {
+		/*
+		 * Handle a special case: the data abort was caused by accessing
+		 * a thread structure while its mapping was being promoted or
+		 * demoted, as a consequence of the break-before-make rule.  It
+		 * is not safe to enable interrupts or dereference "td" before
+		 * this case is handled.
+		 *
+		 * In principle, if pmap_klookup() fails, there is no need to
+		 * call pmap_fault() below, but avoiding that call is not worth
+		 * the effort.
+		 */
+		if (ESR_ELx_EXCEPTION(esr) == EXCP_DATA_ABORT) {
+			switch (esr & ISS_DATA_DFSC_MASK) {
+			case ISS_DATA_DFSC_TF_L0:
+			case ISS_DATA_DFSC_TF_L1:
+			case ISS_DATA_DFSC_TF_L2:
+			case ISS_DATA_DFSC_TF_L3:
+				if (pmap_klookup(far, NULL))
+					return;
+				break;
+			}
 		}
-
-		/* The top bit tells us which range to use */
-		if (ADDR_IS_KERNEL(far)) {
+		intr_enable();
+		map = kernel_map;
+	} else {
+		intr_enable();
+		map = &td->td_proc->p_vmspace->vm_map;
+		if (map == NULL)
 			map = kernel_map;
-		} else {
-			map = &p->p_vmspace->vm_map;
-			if (map == NULL)
-				map = kernel_map;
-		}
 	}
+	pcb = td->td_pcb;
 
 	/*
 	 * Try to handle translation, access flag, and permission faults.
@@ -334,11 +349,11 @@ data_abort(struct thread *td, struct trapframe *frame, uint64_t esr,
 	/* Fault in the page. */
 	error = vm_fault_trap(map, far, ftype, VM_FAULT_NORMAL, &sig, &ucode);
 	if (error != KERN_SUCCESS) {
-bad_far:
 		if (lower) {
 			call_trapsignal(td, sig, ucode, (void *)far,
 			    ESR_ELx_EXCEPTION(esr));
 		} else {
+bad_far:
 			if (td->td_intr_nesting_level == 0 &&
 			    pcb->pcb_onfault != 0) {
 				frame->tf_x[0] = error;
diff --git a/sys/kern/kern_thread.c b/sys/kern/kern_thread.c
index 39bda326dc0d..03fb20bd81d9 100644
--- a/sys/kern/kern_thread.c
+++ b/sys/kern/kern_thread.c
@@ -503,7 +503,6 @@ threadinit(void)
 {
 	u_long i;
 	lwpid_t tid0;
-	uint32_t flags;
 
 	/*
 	 * Place an upper limit on threads which can be allocated.
@@ -531,20 +530,9 @@ threadinit(void)
 	if (tid0 != THREAD0_TID)
 		panic("tid0 %d != %d\n", tid0, THREAD0_TID);
 
-	flags = UMA_ZONE_NOFREE;
-#ifdef __aarch64__
-	/*
-	 * Force thread structures to be allocated from the direct map.
-	 * Otherwise, superpage promotions and demotions may temporarily
-	 * invalidate thread structure mappings.  For most dynamically allocated
-	 * structures this is not a problem, but translation faults cannot be
-	 * handled without accessing curthread.
-	 */
-	flags |= UMA_ZONE_CONTIG;
-#endif
 	thread_zone = uma_zcreate("THREAD", sched_sizeof_thread(),
 	    thread_ctor, thread_dtor, thread_init, thread_fini,
-	    32 - 1, flags);
+	    32 - 1, UMA_ZONE_NOFREE);
 	tidhashtbl = hashinit(maxproc / 2, M_TIDHASH, &tidhash);
 	tidhashlock = (tidhash + 1) / 64;
 	if (tidhashlock > 0)