Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 6 May 2023 17:49:46 GMT
From:      Mitchell Horne <mhorne@FreeBSD.org>
To:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org
Subject:   git: aba91805aa92 - main - hwpmc: use kstack_contains()
Message-ID:  <202305061749.346HnksB017974@gitrepo.freebsd.org>

next in thread | raw e-mail | index | archive | help
The branch main has been updated by mhorne:

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

commit aba91805aa92a47b2f3f01741a55ff9f07c42d04
Author:     Mitchell Horne <mhorne@FreeBSD.org>
AuthorDate: 2023-05-05 21:59:01 +0000
Commit:     Mitchell Horne <mhorne@FreeBSD.org>
CommitDate: 2023-05-06 17:49:19 +0000

    hwpmc: use kstack_contains()
    
    This existing helper function is preferable to the hand-rolled
    calculation of the kstack bounds.
    
    Make some small style improvements while here. Notably, rename every
    instance of "r", the return address, to "ra". Tidy the includes in the
    affected files.
    
    Reviewed by:    jkoshy
    MFC after:      2 weeks
    Sponsored by:   The FreeBSD Foundation
    Differential Revision:  https://reviews.freebsd.org/D39909
---
 sys/amd64/include/pmc_mdep.h   |  8 +++---
 sys/arm/include/pmc_mdep.h     |  6 ++---
 sys/arm64/include/pmc_mdep.h   |  9 +++----
 sys/dev/hwpmc/hwpmc_arm.c      | 35 +++++++++++---------------
 sys/dev/hwpmc/hwpmc_arm64_md.c | 23 ++++++-----------
 sys/dev/hwpmc/hwpmc_x86.c      | 56 ++++++++++++++++++------------------------
 sys/i386/include/pmc_mdep.h    |  6 ++---
 sys/riscv/include/pmc_mdep.h   | 11 ++++-----
 8 files changed, 61 insertions(+), 93 deletions(-)

diff --git a/sys/amd64/include/pmc_mdep.h b/sys/amd64/include/pmc_mdep.h
index 026a9ae2e669..5563f5b1bdbb 100644
--- a/sys/amd64/include/pmc_mdep.h
+++ b/sys/amd64/include/pmc_mdep.h
@@ -109,11 +109,9 @@ union pmc_md_pmc {
 	((PC) >= (uintptr_t) start_exceptions &&	\
 	 (PC) < (uintptr_t) end_exceptions)
 
-#define	PMC_IN_KERNEL_STACK(S,START,END)		\
-	((S) >= (START) && (S) < (END))
-#define	PMC_IN_KERNEL(va) INKERNEL(va)
-
-#define	PMC_IN_USERSPACE(va) ((va) <= VM_MAXUSER_ADDRESS)
+#define	PMC_IN_KERNEL_STACK(va)	kstack_contains(curthread, (va), sizeof(va))
+#define	PMC_IN_KERNEL(va)	INKERNEL(va)
+#define	PMC_IN_USERSPACE(va)	((va) <= VM_MAXUSER_ADDRESS)
 
 /* Build a fake kernel trapframe from current instruction pointer. */
 #define PMC_FAKE_TRAPFRAME(TF)						\
diff --git a/sys/arm/include/pmc_mdep.h b/sys/arm/include/pmc_mdep.h
index 69cb0c84deca..d7b80abb64b0 100644
--- a/sys/arm/include/pmc_mdep.h
+++ b/sys/arm/include/pmc_mdep.h
@@ -52,11 +52,9 @@ union pmc_md_pmc {
 	struct pmc_md_armv7_pmc		pm_armv7;
 };
 
-#define	PMC_IN_KERNEL_STACK(S,START,END)		\
-	((S) >= (START) && (S) < (END))
+#define	PMC_IN_KERNEL_STACK(va)	kstack_contains(curthread, (va), sizeof(va))
 #define	PMC_IN_KERNEL(va)	INKERNEL((va))
-
-#define	PMC_IN_USERSPACE(va) ((va) <= VM_MAXUSER_ADDRESS)
+#define	PMC_IN_USERSPACE(va)	((va) <= VM_MAXUSER_ADDRESS)
 
 #define	PMC_TRAPFRAME_TO_PC(TF)		((TF)->tf_pc)
 #define	PMC_TRAPFRAME_TO_FP(TF)		((TF)->tf_r11)
diff --git a/sys/arm64/include/pmc_mdep.h b/sys/arm64/include/pmc_mdep.h
index d27bdbd5afc1..049bb2b22751 100644
--- a/sys/arm64/include/pmc_mdep.h
+++ b/sys/arm64/include/pmc_mdep.h
@@ -66,12 +66,11 @@ union pmc_md_pmc {
 	struct pmc_md_dmc620_pmc	pm_dmc620;
 };
 
-#define	PMC_IN_KERNEL_STACK(S,START,END)		\
-	((S) >= (START) && (S) < (END))
+#define	PMC_IN_KERNEL_STACK(va)	kstack_contains(curthread, (va), sizeof(va))
 #define	PMC_IN_KERNEL(va)	INKERNEL((va))
-#define	PMC_IN_USERSPACE(va) ((va) <= VM_MAXUSER_ADDRESS)
-#define	PMC_TRAPFRAME_TO_PC(TF)		((TF)->tf_elr)
-#define	PMC_TRAPFRAME_TO_FP(TF)		((TF)->tf_x[29])
+#define	PMC_IN_USERSPACE(va)	((va) <= VM_MAXUSER_ADDRESS)
+#define	PMC_TRAPFRAME_TO_PC(TF)	((TF)->tf_elr)
+#define	PMC_TRAPFRAME_TO_FP(TF)	((TF)->tf_x[29])
 
 /*
  * Prototypes
diff --git a/sys/dev/hwpmc/hwpmc_arm.c b/sys/dev/hwpmc/hwpmc_arm.c
index c585a6cefd1e..a0e2e6823219 100644
--- a/sys/dev/hwpmc/hwpmc_arm.c
+++ b/sys/dev/hwpmc/hwpmc_arm.c
@@ -31,18 +31,17 @@
 __FBSDID("$FreeBSD$");
 
 #include <sys/param.h>
-#include <sys/pmc.h>
-#include <sys/proc.h>
 #include <sys/systm.h>
+#include <sys/pmc.h>
+
+#include <vm/vm.h>
+#include <vm/pmap.h>
 
 #include <machine/cpu.h>
 #include <machine/md_var.h>
 #include <machine/pmc_mdep.h>
 #include <machine/stack.h>
-
-#include <vm/vm.h>
-#include <vm/vm_param.h>
-#include <vm/pmap.h>
+#include <machine/vmparam.h>
 
 /* XXX: Userland code compiled with gcc will need an heuristic
  * to be correctly detected. 
@@ -78,45 +77,39 @@ int
 pmc_save_kernel_callchain(uintptr_t *cc, int maxsamples,
     struct trapframe *tf)
 {
-	uintptr_t pc, r, stackstart, stackend, fp;
-	struct thread *td;
+	uintptr_t pc, ra, fp;
 	int count;
 
 	KASSERT(TRAPF_USERMODE(tf) == 0,("[arm,%d] not a kernel backtrace",
 	    __LINE__));
 
-	td = curthread;
 	pc = PMC_TRAPFRAME_TO_PC(tf);
 	*cc++ = pc;
 
 	if (maxsamples <= 1)
 		return (1);
 
-	stackstart = (uintptr_t) td->td_kstack;
-	stackend = (uintptr_t) td->td_kstack + td->td_kstack_pages * PAGE_SIZE;
 	fp = PMC_TRAPFRAME_TO_FP(tf);
-
-	if (!PMC_IN_KERNEL(pc) ||
-	    !PMC_IN_KERNEL_STACK(fp, stackstart, stackend))
+	if (!PMC_IN_KERNEL(pc) || !PMC_IN_KERNEL_STACK(fp))
 		return (1);
 
 	for (count = 1; count < maxsamples; count++) {
 		/* Use saved lr as pc. */
-		r = fp + PC_OFF * sizeof(uintptr_t);
-		if (!PMC_IN_KERNEL_STACK(r, stackstart, stackend))
+		ra = fp + PC_OFF * sizeof(uintptr_t);
+		if (!PMC_IN_KERNEL_STACK(ra))
 			break;
-		pc = *(uintptr_t *)r;
+		pc = *(uintptr_t *)ra;
 		if (!PMC_IN_KERNEL(pc))
 			break;
 
 		*cc++ = pc;
 
 		/* Switch to next frame up */
-		r = fp + FP_OFF * sizeof(uintptr_t);
-		if (!PMC_IN_KERNEL_STACK(r, stackstart, stackend))
+		ra = fp + FP_OFF * sizeof(uintptr_t);
+		if (!PMC_IN_KERNEL_STACK(ra))
 			break;
-		fp = *(uintptr_t *)r;
-		if (!PMC_IN_KERNEL_STACK(fp, stackstart, stackend))
+		fp = *(uintptr_t *)ra;
+		if (!PMC_IN_KERNEL_STACK(fp))
 			break;
 	}
 
diff --git a/sys/dev/hwpmc/hwpmc_arm64_md.c b/sys/dev/hwpmc/hwpmc_arm64_md.c
index dd51f7bd18d5..4220badb30a6 100644
--- a/sys/dev/hwpmc/hwpmc_arm64_md.c
+++ b/sys/dev/hwpmc/hwpmc_arm64_md.c
@@ -31,18 +31,17 @@
 __FBSDID("$FreeBSD$");
 
 #include <sys/param.h>
-#include <sys/pmc.h>
-#include <sys/proc.h>
 #include <sys/systm.h>
+#include <sys/pmc.h>
+
+#include <vm/vm.h>
+#include <vm/pmap.h>
 
 #include <machine/cpu.h>
 #include <machine/md_var.h>
 #include <machine/pmc_mdep.h>
 #include <machine/stack.h>
-
-#include <vm/vm.h>
-#include <vm/vm_param.h>
-#include <vm/pmap.h>
+#include <machine/vmparam.h>
 
 struct pmc_mdep *
 pmc_md_initialize(void)
@@ -59,30 +58,22 @@ pmc_md_finalize(struct pmc_mdep *md)
 }
 
 int
-pmc_save_kernel_callchain(uintptr_t *cc, int maxsamples,
-    struct trapframe *tf)
+pmc_save_kernel_callchain(uintptr_t *cc, int maxsamples, struct trapframe *tf)
 {
 	struct unwind_state frame;
-	uintptr_t stackstart, stackend;
-	struct thread *td;
 	int count;
 
 	KASSERT(TRAPF_USERMODE(tf) == 0,("[arm64,%d] not a kernel backtrace",
 	    __LINE__));
 
-	td = curthread;
 	frame.pc = PMC_TRAPFRAME_TO_PC(tf);
 	*cc++ = frame.pc;
 
 	if (maxsamples <= 1)
 		return (1);
 
-	stackstart = (uintptr_t) td->td_kstack;
-	stackend = (uintptr_t) td->td_kstack + td->td_kstack_pages * PAGE_SIZE;
 	frame.fp = PMC_TRAPFRAME_TO_FP(tf);
-
-	if (!PMC_IN_KERNEL(frame.pc) ||
-	    !PMC_IN_KERNEL_STACK(frame.fp, stackstart, stackend))
+	if (!PMC_IN_KERNEL(frame.pc) || !PMC_IN_KERNEL_STACK(frame.fp))
 		return (1);
 
 	for (count = 1; count < maxsamples; count++) {
diff --git a/sys/dev/hwpmc/hwpmc_x86.c b/sys/dev/hwpmc/hwpmc_x86.c
index e427139244ad..913c8f756815 100644
--- a/sys/dev/hwpmc/hwpmc_x86.c
+++ b/sys/dev/hwpmc/hwpmc_x86.c
@@ -34,21 +34,22 @@
 __FBSDID("$FreeBSD$");
 
 #include <sys/param.h>
+#include <sys/systm.h>
 #include <sys/bus.h>
 #include <sys/pmc.h>
-#include <sys/proc.h>
-#include <sys/systm.h>
+
+#include <vm/vm.h>
+#include <vm/pmap.h>
 
 #include <machine/cpu.h>
 #include <machine/cputypes.h>
-#include <machine/intr_machdep.h>
-#include <x86/apicvar.h>
-#include <machine/pmc_mdep.h>
+#include <machine/intr_machdep.h>	/* For x86/apicvar.h */
 #include <machine/md_var.h>
+#include <machine/pmc_mdep.h>
+#include <machine/stack.h>
+#include <machine/vmparam.h>
 
-#include <vm/vm.h>
-#include <vm/vm_param.h>
-#include <vm/pmap.h>
+#include <x86/apicvar.h>
 
 #include "hwpmc_soft.h"
 
@@ -155,36 +156,29 @@ pmc_save_user_callchain(uintptr_t *cc, int nframes, struct trapframe *tf)
 int __nosanitizeaddress __nosanitizememory
 pmc_save_kernel_callchain(uintptr_t *cc, int nframes, struct trapframe *tf)
 {
-	int n;
+	uintptr_t fp, pc, ra, sp;
 	uint32_t instr;
-	uintptr_t fp, pc, r, sp, stackstart, stackend;
-	struct thread *td;
+	int n;
 
 	KASSERT(TRAPF_USERMODE(tf) == 0,("[x86,%d] not a kernel backtrace",
 	    __LINE__));
 
-	td = curthread;
 	pc = PMC_TRAPFRAME_TO_PC(tf);
 	fp = PMC_TRAPFRAME_TO_FP(tf);
 	sp = PMC_TRAPFRAME_TO_KERNEL_SP(tf);
 
 	*cc++ = pc;
-	r = fp + sizeof(uintptr_t); /* points to return address */
+	ra = fp + sizeof(uintptr_t); /* points to return address */
 
 	if (nframes <= 1)
 		return (1);
 
-	stackstart = (uintptr_t) td->td_kstack;
-	stackend = (uintptr_t) td->td_kstack + td->td_kstack_pages * PAGE_SIZE;
-
-	if (PMC_IN_TRAP_HANDLER(pc) ||
-	    !PMC_IN_KERNEL(pc) ||
-	    !PMC_IN_KERNEL_STACK(r, stackstart, stackend) ||
-	    !PMC_IN_KERNEL_STACK(sp, stackstart, stackend) ||
-	    !PMC_IN_KERNEL_STACK(fp, stackstart, stackend))
+	if (PMC_IN_TRAP_HANDLER(pc) || !PMC_IN_KERNEL(pc) ||
+	    !PMC_IN_KERNEL_STACK(ra) || !PMC_IN_KERNEL_STACK(sp) ||
+	    !PMC_IN_KERNEL_STACK(fp))
 		return (1);
 
-	instr = *(uint32_t *) pc;
+	instr = *(uint32_t *)pc;
 
 	/*
 	 * Determine whether the interrupted function was in the
@@ -205,15 +199,15 @@ pmc_save_kernel_callchain(uintptr_t *cc, int nframes, struct trapframe *tf)
 		 * and the caller's address is therefore at sp[1].
 		 */
 		sp += sizeof(uintptr_t);
-		if (!PMC_IN_KERNEL_STACK(sp, stackstart, stackend))
+		if (!PMC_IN_KERNEL_STACK(sp))
 			return (1);
-		pc = *(uintptr_t *) sp;
+		pc = *(uintptr_t *)sp;
 	} else {
 		/*
 		 * Not in the function prologue or epilogue.
 		 */
-		pc = *(uintptr_t *) r;
-		fp = *(uintptr_t *) fp;
+		pc = *(uintptr_t *)ra;
+		fp = *(uintptr_t *)fp;
 	}
 
 	for (n = 1; n < nframes; n++) {
@@ -222,12 +216,11 @@ pmc_save_kernel_callchain(uintptr_t *cc, int nframes, struct trapframe *tf)
 		if (PMC_IN_TRAP_HANDLER(pc))
 			break;
 
-		r = fp + sizeof(uintptr_t);
-		if (!PMC_IN_KERNEL_STACK(fp, stackstart, stackend) ||
-		    !PMC_IN_KERNEL_STACK(r, stackstart, stackend))
+		ra = fp + sizeof(uintptr_t);
+		if (!PMC_IN_KERNEL_STACK(fp) || !PMC_IN_KERNEL_STACK(ra))
 			break;
-		pc = *(uintptr_t *) r;
-		fp = *(uintptr_t *) fp;
+		pc = *(uintptr_t *)ra;
+		fp = *(uintptr_t *)fp;
 	}
 
 	return (n);
@@ -236,7 +229,6 @@ pmc_save_kernel_callchain(uintptr_t *cc, int nframes, struct trapframe *tf)
 /*
  * Machine dependent initialization for x86 class platforms.
  */
-
 struct pmc_mdep *
 pmc_md_initialize(void)
 {
diff --git a/sys/i386/include/pmc_mdep.h b/sys/i386/include/pmc_mdep.h
index 3d498e006a36..15bdd7a65dc8 100644
--- a/sys/i386/include/pmc_mdep.h
+++ b/sys/i386/include/pmc_mdep.h
@@ -121,11 +121,9 @@ struct pmc_mdep;
 #define	PMC_TRAPFRAME_TO_USER_SP(TF)	((TF)->tf_esp)
 #define	PMC_TRAPFRAME_TO_KERNEL_SP(TF)	((uintptr_t) &((TF)->tf_esp))
 
-#define	PMC_IN_KERNEL_STACK(S,START,END)		\
-	((S) >= (START) && (S) < (END))
+#define	PMC_IN_KERNEL_STACK(va)	kstack_contains(curthread, (va), sizeof(va))
 #define	PMC_IN_KERNEL(va)	INKERNEL(va)
-
-#define	PMC_IN_USERSPACE(va) ((va) <= VM_MAXUSER_ADDRESS)
+#define	PMC_IN_USERSPACE(va)	((va) <= VM_MAXUSER_ADDRESS)
 
 #define	PMC_IN_TRAP_HANDLER(PC) 			\
 	((PC) >= (uintptr_t)start_exceptions + setidt_disp &&	\
diff --git a/sys/riscv/include/pmc_mdep.h b/sys/riscv/include/pmc_mdep.h
index 10dc5c8b98ed..b192d75ec14b 100644
--- a/sys/riscv/include/pmc_mdep.h
+++ b/sys/riscv/include/pmc_mdep.h
@@ -48,12 +48,11 @@ union pmc_md_pmc {
 	struct pmc_md_riscv_pmc		pm_riscv;
 };
 
-#define	PMC_IN_KERNEL_STACK(S,START,END)		\
-	((S) >= (START) && (S) < (END))
-#define	PMC_IN_KERNEL(va)		INKERNEL((va))
-#define	PMC_IN_USERSPACE(va)		((va) <= VM_MAXUSER_ADDRESS)
-#define	PMC_TRAPFRAME_TO_PC(TF)		((TF)->tf_ra)
-#define	PMC_TRAPFRAME_TO_FP(TF)		(0)	/* stub */
+#define	PMC_IN_KERNEL_STACK(va)	kstack_contains(curthread, (va), sizeof(va))
+#define	PMC_IN_KERNEL(va)	INKERNEL((va))
+#define	PMC_IN_USERSPACE(va)	((va) <= VM_MAXUSER_ADDRESS)
+#define	PMC_TRAPFRAME_TO_PC(TF)	((TF)->tf_ra)
+#define	PMC_TRAPFRAME_TO_FP(TF)	(0)	/* stub */
 
 /*
  * Prototypes



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