git: 5a800e965345 - main - Revert "arm64: write PID in CONTEXTIDR_EL1 on ctx switch"

From: Jessica Clarke <jrtc27_at_FreeBSD.org>
Date: Tue, 10 Sep 2024 18:20:14 UTC
The branch main has been updated by jrtc27:

URL: https://cgit.FreeBSD.org/src/commit/?id=5a800e9653456e47aefec43c2c8aee4fa7ab42a6

commit 5a800e9653456e47aefec43c2c8aee4fa7ab42a6
Author:     Jessica Clarke <jrtc27@FreeBSD.org>
AuthorDate: 2024-09-10 18:14:39 +0000
Commit:     Jessica Clarke <jrtc27@FreeBSD.org>
CommitDate: 2024-09-10 18:14:39 +0000

    Revert "arm64: write PID in CONTEXTIDR_EL1 on ctx switch"
    
    arm64_pid_in_contextidr is a bool yet is loaded as a 64-bit word, so
    reads out of bounds and will get unknown junk in the upper bits, which
    may cause false (the default) to be interpreted as true by the following
    cbz. This also doesn't even link depending on the variable's alignment,
    currently breaking the clang-12 and clang-13 jobs in GitHub Actions.
    
    Whilst fixing that to use ldrb instead would be trivial, there's a
    similar issue reading p_pid below, which is an int32_t yet read as a
    64-bit word again. Fixing this means using ldrw or ldrsw, but it's not
    clear whether the expectation is for the PID to be zero-extended or
    sign-extended.
    
    Revert the commit to fix the build and, if it builds, the run-time
    behaviour until it can be corrected and re-landed.
    
    This reverts commit f05795e3f65f305cb770ae91d8e9c8f05d267e0d.
---
 sys/arm64/arm64/genassym.c    |  2 --
 sys/arm64/arm64/swtch.S       | 17 -----------------
 sys/arm64/arm64/sys_machdep.c |  6 ------
 3 files changed, 25 deletions(-)

diff --git a/sys/arm64/arm64/genassym.c b/sys/arm64/arm64/genassym.c
index 5a20169d51c3..a4db825e976c 100644
--- a/sys/arm64/arm64/genassym.c
+++ b/sys/arm64/arm64/genassym.c
@@ -58,8 +58,6 @@ ASSYM(PCB_TPIDRRO, offsetof(struct pcb, pcb_tpidrro_el0));
 ASSYM(PCB_ONFAULT, offsetof(struct pcb, pcb_onfault));
 ASSYM(PCB_FLAGS, offsetof(struct pcb, pcb_flags));
 
-ASSYM(PR_PID, offsetof(struct proc, p_pid));
-
 ASSYM(SF_UC, offsetof(struct sigframe, sf_uc));
 
 ASSYM(TD_PROC, offsetof(struct thread, td_proc));
diff --git a/sys/arm64/arm64/swtch.S b/sys/arm64/arm64/swtch.S
index 3a2bf2cb5a7f..6af70ca839a0 100644
--- a/sys/arm64/arm64/swtch.S
+++ b/sys/arm64/arm64/swtch.S
@@ -55,15 +55,6 @@
 999:
 .endm
 
-.macro pid_in_context_idr label
-	adrp	x9, arm64_pid_in_contextidr
-	ldr	x10, [x9, :lo12:arm64_pid_in_contextidr]
-	cbz	x10, \label
-	ldr	x9, [x1, #TD_PROC]
-	ldr	x10, [x9, #PR_PID]
-	msr	contextidr_el1, x10
-.endm
-
 /*
  * void cpu_throw(struct thread *old, struct thread *new)
  */
@@ -75,12 +66,8 @@ ENTRY(cpu_throw)
 	ldr	x4, [x0, #TD_PCB]
 	ldr	w5, [x4, #PCB_FLAGS]
 	clear_step_flag w5, x6
-
 1:
-	/* debug/trace: set CONTEXTIDR_EL1 to current PID, if enabled */
-	pid_in_context_idr 2f
 
-2:
 #ifdef VFP
 	/* Backup the new thread pointer around a call to C code */
 	mov	x19, x1
@@ -160,10 +147,6 @@ ENTRY(cpu_switch)
 	mov	x20, x1
 	mov	x21, x2
 
-	/* debug/trace: set CONTEXTIDR_EL1 to current PID, if enabled */
-	pid_in_context_idr 0f
-
-0:
 #ifdef VFP
 	bl	vfp_save_state_switch
 	mov	x0, x20
diff --git a/sys/arm64/arm64/sys_machdep.c b/sys/arm64/arm64/sys_machdep.c
index eedee1d27015..eedc57f7c572 100644
--- a/sys/arm64/arm64/sys_machdep.c
+++ b/sys/arm64/arm64/sys_machdep.c
@@ -30,7 +30,6 @@
 #include <sys/param.h>
 #include <sys/systm.h>
 #include <sys/proc.h>
-#include <sys/sysctl.h>
 #include <sys/sysproto.h>
 
 #include <vm/vm.h>
@@ -81,8 +80,3 @@ sysarch(struct thread *td, struct sysarch_args *uap)
 
 	return (error);
 }
-
-bool arm64_pid_in_contextidr = false;
-SYSCTL_BOOL(_machdep, OID_AUTO, pid_in_contextidr, CTLFLAG_RW,
-    &arm64_pid_in_contextidr, false,
-    "Save PID into CONTEXTIDR_EL1 register on context switch");