git: 5a800e965345 - main - Revert "arm64: write PID in CONTEXTIDR_EL1 on ctx switch"
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
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");