From eb7c901995d407fa177d99270a2684f844db3921 Mon Sep 17 00:00:00 2001
From: Matt Macy <mmacy@FreeBSD.org>
Date: Fri, 8 Jun 2018 04:58:03 +0000
Subject: [PATCH] hwpmc: simplify calling convention for hwpmc interrupt
 handling

pmc_process_interrupt takes 5 arguments when only 3 are needed.
cpu is always available in curcpu and inuserspace can always be
derived from the passed trapframe.

While facially a reasonable cleanup this change was motivated
by the need to workaround a compiler bug.

core2_intr(cpu, tf) ->
  pmc_process_interrupt(cpu, ring, pmc, tf, inuserspace) ->
    pmc_add_sample(cpu, ring, pm, tf, inuserspace)

In the process of optimizing the tail call the tf pointer was getting
clobbered:

(kgdb) up
    at /storage/mmacy/devel/freebsd/sys/dev/hwpmc/hwpmc_mod.c:4709
4709                                pmc_save_kernel_callchain(ps->ps_pc,
(kgdb) up
1205                    error = pmc_process_interrupt(cpu, PMC_HR, pm, tf,

resulting in a crash in pmc_save_kernel_callchain.
---
 sys/amd64/amd64/trap.c          |  2 +-
 sys/arm/arm/pmu.c               |  2 +-
 sys/dev/hwpmc/hwpmc_amd.c       |  8 ++++----
 sys/dev/hwpmc/hwpmc_arm64.c     |  8 ++++----
 sys/dev/hwpmc/hwpmc_armv7.c     |  8 ++++----
 sys/dev/hwpmc/hwpmc_core.c      | 21 +++++++++------------
 sys/dev/hwpmc/hwpmc_mips.c      |  3 +--
 sys/dev/hwpmc/hwpmc_mod.c       | 22 ++++++++++------------
 sys/dev/hwpmc/hwpmc_mpc7xxx.c   |  3 +--
 sys/dev/hwpmc/hwpmc_ppc970.c    |  3 +--
 sys/dev/hwpmc/hwpmc_soft.c      |  3 +--
 sys/i386/i386/trap.c            |  2 +-
 sys/kern/kern_pmc.c             |  2 +-
 sys/mips/atheros/apb.c          |  2 +-
 sys/mips/cavium/octeon_pmc.c    |  2 +-
 sys/powerpc/powerpc/interrupt.c |  2 +-
 sys/sys/pmc.h                   |  7 +++----
 sys/sys/pmckern.h               |  2 +-
 18 files changed, 46 insertions(+), 56 deletions(-)

diff --git a/sys/amd64/amd64/trap.c b/sys/amd64/amd64/trap.c
index dfa38fb1376f..362f349a1d90 100644
--- a/sys/amd64/amd64/trap.c
+++ b/sys/amd64/amd64/trap.c
@@ -214,7 +214,7 @@ trap(struct trapframe *frame)
 		 * the NMI was consumed by it and we can return immediately.
 		 */
 		if (pmc_intr != NULL &&
-		    (*pmc_intr)(PCPU_GET(cpuid), frame) != 0)
+		    (*pmc_intr)(frame) != 0)
 			return;
 #endif
 
diff --git a/sys/arm/arm/pmu.c b/sys/arm/arm/pmu.c
index a085e50e9572..2428ef068195 100644
--- a/sys/arm/arm/pmu.c
+++ b/sys/arm/arm/pmu.c
@@ -123,7 +123,7 @@ pmu_intr(void *arg)
 	/* Only call into the HWPMC framework if we know there is work. */
 	if (r != 0 && pmc_intr) {
 		tf = arg;
-		(*pmc_intr)(PCPU_GET(cpuid), tf);
+		(*pmc_intr)(tf);
 	}
 #endif
 
diff --git a/sys/dev/hwpmc/hwpmc_amd.c b/sys/dev/hwpmc/hwpmc_amd.c
index 5bb20d4b83c6..9b5f2a5519e8 100644
--- a/sys/dev/hwpmc/hwpmc_amd.c
+++ b/sys/dev/hwpmc/hwpmc_amd.c
@@ -627,14 +627,15 @@ amd_stop_pmc(int cpu, int ri)
  */
 
 static int
-amd_intr(int cpu, struct trapframe *tf)
+amd_intr(struct trapframe *tf)
 {
-	int i, error, retval;
+	int i, error, retval, cpu;
 	uint32_t config, evsel, perfctr;
 	struct pmc *pm;
 	struct amd_cpu *pac;
 	pmc_value_t v;
 
+	cpu = curcpu;
 	KASSERT(cpu >= 0 && cpu < pmc_cpu_max(),
 	    ("[amd,%d] out of range CPU %d", __LINE__, cpu));
 
@@ -688,8 +689,7 @@ amd_intr(int cpu, struct trapframe *tf)
 		wrmsr(perfctr, AMD_RELOAD_COUNT_TO_PERFCTR_VALUE(v));
 
 		/* Restart the counter if logging succeeded. */
-		error = pmc_process_interrupt(cpu, PMC_HR, pm, tf,
-		    TRAPF_USERMODE(tf));
+		error = pmc_process_interrupt(PMC_HR, pm, tf);
 		if (error == 0)
 			wrmsr(evsel, config);
 	}
diff --git a/sys/dev/hwpmc/hwpmc_arm64.c b/sys/dev/hwpmc/hwpmc_arm64.c
index 2e54e3870ba2..6fa319f99e82 100644
--- a/sys/dev/hwpmc/hwpmc_arm64.c
+++ b/sys/dev/hwpmc/hwpmc_arm64.c
@@ -323,18 +323,19 @@ arm64_release_pmc(int cpu, int ri, struct pmc *pmc)
 }
 
 static int
-arm64_intr(int cpu, struct trapframe *tf)
+arm64_intr(struct trapframe *tf)
 {
 	struct arm64_cpu *pc;
 	int retval, ri;
 	struct pmc *pm;
 	int error;
-	int reg;
+	int reg, cpu;
 
 	KASSERT(cpu >= 0 && cpu < pmc_cpu_max(),
 	    ("[arm64,%d] CPU %d out of range", __LINE__, cpu));
 
 	retval = 0;
+	cpu = curcpu;
 	pc = arm64_pcpu[cpu];
 
 	for (ri = 0; ri < arm64_npmcs; ri++) {
@@ -357,8 +358,7 @@ arm64_intr(int cpu, struct trapframe *tf)
 		if (pm->pm_state != PMC_STATE_RUNNING)
 			continue;
 
-		error = pmc_process_interrupt(cpu, PMC_HR, pm, tf,
-		    TRAPF_USERMODE(tf));
+		error = pmc_process_interrupt(PMC_HR, pm, tf);
 		if (error)
 			arm64_stop_pmc(cpu, ri);
 
diff --git a/sys/dev/hwpmc/hwpmc_armv7.c b/sys/dev/hwpmc/hwpmc_armv7.c
index 2b0f28f27691..0a601145f8de 100644
--- a/sys/dev/hwpmc/hwpmc_armv7.c
+++ b/sys/dev/hwpmc/hwpmc_armv7.c
@@ -307,18 +307,19 @@ armv7_release_pmc(int cpu, int ri, struct pmc *pmc)
 }
 
 static int
-armv7_intr(int cpu, struct trapframe *tf)
+armv7_intr(struct trapframe *tf)
 {
 	struct armv7_cpu *pc;
 	int retval, ri;
 	struct pmc *pm;
 	int error;
-	int reg;
+	int reg, cpu;
 
 	KASSERT(cpu >= 0 && cpu < pmc_cpu_max(),
 	    ("[armv7,%d] CPU %d out of range", __LINE__, cpu));
 
 	retval = 0;
+	cpu = curcpu;
 	pc = armv7_pcpu[cpu];
 
 	for (ri = 0; ri < armv7_npmcs; ri++) {
@@ -348,8 +349,7 @@ armv7_intr(int cpu, struct trapframe *tf)
 		if (pm->pm_state != PMC_STATE_RUNNING)
 			continue;
 
-		error = pmc_process_interrupt(cpu, PMC_HR, pm, tf,
-		    TRAPF_USERMODE(tf));
+		error = pmc_process_interrupt(PMC_HR, pm, tf);
 		if (error)
 			armv7_stop_pmc(cpu, ri);
 
diff --git a/sys/dev/hwpmc/hwpmc_core.c b/sys/dev/hwpmc/hwpmc_core.c
index b308f51ef181..ecdd73b786d8 100644
--- a/sys/dev/hwpmc/hwpmc_core.c
+++ b/sys/dev/hwpmc/hwpmc_core.c
@@ -1056,7 +1056,7 @@ iap_initialize(struct pmc_mdep *md, int maxcpu, int npmc, int pmcwidth,
 }
 
 static int
-core_intr(int cpu, struct trapframe *tf)
+core_intr(struct trapframe *tf)
 {
 	pmc_value_t v;
 	struct pmc *pm;
@@ -1064,11 +1064,11 @@ core_intr(int cpu, struct trapframe *tf)
 	int error, found_interrupt, ri;
 	uint64_t msr;
 
-	PMCDBG3(MDP,INT, 1, "cpu=%d tf=0x%p um=%d", cpu, (void *) tf,
+	PMCDBG3(MDP,INT, 1, "cpu=%d tf=0x%p um=%d", curcpu, (void *) tf,
 	    TRAPF_USERMODE(tf));
 
 	found_interrupt = 0;
-	cc = core_pcpu[cpu];
+	cc = core_pcpu[curcpu];
 
 	for (ri = 0; ri < core_iap_npmc; ri++) {
 
@@ -1084,8 +1084,7 @@ core_intr(int cpu, struct trapframe *tf)
 		if (pm->pm_state != PMC_STATE_RUNNING)
 			continue;
 
-		error = pmc_process_interrupt(cpu, PMC_HR, pm, tf,
-		    TRAPF_USERMODE(tf));
+		error = pmc_process_interrupt(PMC_HR, pm, tf);
 
 		v = pm->pm_sc.pm_reloadcount;
 		v = iap_reload_count_to_perfctr_value(v);
@@ -1117,7 +1116,7 @@ core_intr(int cpu, struct trapframe *tf)
 }
 
 static int
-core2_intr(int cpu, struct trapframe *tf)
+core2_intr(struct trapframe *tf)
 {
 	int error, found_interrupt, n;
 	uint64_t flag, intrstatus, intrenable, msr;
@@ -1141,7 +1140,7 @@ core2_intr(int cpu, struct trapframe *tf)
 	    (uintmax_t) intrstatus);
 
 	found_interrupt = 0;
-	cc = core_pcpu[cpu];
+	cc = core_pcpu[curcpu];
 
 	KASSERT(cc != NULL, ("[core,%d] null pcpu", __LINE__));
 
@@ -1173,8 +1172,7 @@ core2_intr(int cpu, struct trapframe *tf)
 		    !PMC_IS_SAMPLING_MODE(PMC_TO_MODE(pm)))
 			continue;
 
-		error = pmc_process_interrupt(cpu, PMC_HR, pm, tf,
-		    TRAPF_USERMODE(tf));
+		error = pmc_process_interrupt(PMC_HR, pm, tf);
 
 		if (error)
 			intrenable &= ~flag;
@@ -1184,7 +1182,7 @@ core2_intr(int cpu, struct trapframe *tf)
 		/* Reload sampling count. */
 		wrmsr(IAF_CTR0 + n, v);
 
-		PMCDBG4(MDP,INT, 1, "iaf-intr cpu=%d error=%d v=%jx(%jx)", cpu,
+		PMCDBG4(MDP,INT, 1, "iaf-intr cpu=%d error=%d v=%jx(%jx)", curcpu,
 		    error, (uintmax_t) v, (uintmax_t) rdpmc(IAF_RI_TO_MSR(n)));
 	}
 
@@ -1202,8 +1200,7 @@ core2_intr(int cpu, struct trapframe *tf)
 		    !PMC_IS_SAMPLING_MODE(PMC_TO_MODE(pm)))
 			continue;
 
-		error = pmc_process_interrupt(cpu, PMC_HR, pm, tf,
-		    TRAPF_USERMODE(tf));
+		error = pmc_process_interrupt(PMC_HR, pm, tf);
 		if (error)
 			intrenable &= ~flag;
 
diff --git a/sys/dev/hwpmc/hwpmc_mips.c b/sys/dev/hwpmc/hwpmc_mips.c
index 7a27777d7578..99b8defff981 100644
--- a/sys/dev/hwpmc/hwpmc_mips.c
+++ b/sys/dev/hwpmc/hwpmc_mips.c
@@ -289,8 +289,7 @@ mips_pmc_intr(int cpu, struct trapframe *tf)
 		retval = 1;
 		if (pm->pm_state != PMC_STATE_RUNNING)
 			continue;
-		error = pmc_process_interrupt(cpu, PMC_HR, pm, tf,
-		    TRAPF_USERMODE(tf));
+		error = pmc_process_interrupt(PMC_HR, pm, tf);
 		if (error) {
 			/* Clear/disable the relevant counter */
 			if (ri == 0)
diff --git a/sys/dev/hwpmc/hwpmc_mod.c b/sys/dev/hwpmc/hwpmc_mod.c
index 1d5e28ab30ad..65770110677b 100644
--- a/sys/dev/hwpmc/hwpmc_mod.c
+++ b/sys/dev/hwpmc/hwpmc_mod.c
@@ -207,8 +207,7 @@ static int	pmc_debugflags_parse(char *newstr, char *fence);
 #endif
 
 static int	load(struct module *module, int cmd, void *arg);
-static int	pmc_add_sample(int cpu, int ring, struct pmc *pm,
-    struct trapframe *tf, int inuserspace);
+static int	pmc_add_sample(int ring, struct pmc *pm, struct trapframe *tf);
 static void	pmc_add_thread_descriptors_from_proc(struct proc *p,
     struct pmc_process *pp);
 static int	pmc_attach_process(struct proc *p, struct pmc *pm);
@@ -4640,10 +4639,9 @@ pmc_post_callchain_callback(void)
  */
 
 static int
-pmc_add_sample(int cpu, int ring, struct pmc *pm, struct trapframe *tf,
-    int inuserspace)
+pmc_add_sample(int ring, struct pmc *pm, struct trapframe *tf)
 {
-	int error, callchaindepth;
+	int error, cpu, callchaindepth, inuserspace;
 	struct thread *td;
 	struct pmc_sample *ps;
 	struct pmc_samplebuffer *psb;
@@ -4653,8 +4651,9 @@ pmc_add_sample(int cpu, int ring, struct pmc *pm, struct trapframe *tf,
 	/*
 	 * Allocate space for a sample buffer.
 	 */
+	cpu = curcpu;
 	psb = pmc_pcpu[cpu]->pc_sb[ring];
-
+	inuserspace = TRAPF_USERMODE(tf);
 	ps = psb->ps_write;
 	if (ps->ps_nsamples == PMC_SAMPLE_INUSE) {
 		counter_u64_add(ps->ps_pmc->pm_runcount, -1);
@@ -4743,19 +4742,18 @@ pmc_add_sample(int cpu, int ring, struct pmc *pm, struct trapframe *tf,
  */
 
 int
-pmc_process_interrupt(int cpu, int ring, struct pmc *pm, struct trapframe *tf,
-    int inuserspace)
+pmc_process_interrupt(int ring, struct pmc *pm, struct trapframe *tf)
 {
 	struct thread *td;
 
 	td = curthread;
 	if ((pm->pm_flags & PMC_F_USERCALLCHAIN) &&
-           (td->td_proc->p_flag & P_KPROC) == 0 &&
-           !inuserspace) {
+        (td->td_proc->p_flag & P_KPROC) == 0 &&
+		!TRAPF_USERMODE(tf)) {
 		atomic_add_int(&curthread->td_pmcpend, 1);
-		return (pmc_add_sample(cpu, PMC_UR, pm, tf, 0));
+		return (pmc_add_sample(PMC_UR, pm, tf));
 	}
-	return (pmc_add_sample(cpu, ring, pm, tf, inuserspace));
+	return (pmc_add_sample(ring, pm, tf));
 }
 
 /*
diff --git a/sys/dev/hwpmc/hwpmc_mpc7xxx.c b/sys/dev/hwpmc/hwpmc_mpc7xxx.c
index a019d73884bc..457d55c343d1 100644
--- a/sys/dev/hwpmc/hwpmc_mpc7xxx.c
+++ b/sys/dev/hwpmc/hwpmc_mpc7xxx.c
@@ -702,8 +702,7 @@ mpc7xxx_intr(int cpu, struct trapframe *tf)
 			continue;
 
 		/* Stop the counter if logging fails. */
-		error = pmc_process_interrupt(cpu, PMC_HR, pm, tf,
-		    TRAPF_USERMODE(tf));
+		error = pmc_process_interrupt(PMC_HR, pm, tf);
 		if (error != 0)
 			mpc7xxx_stop_pmc(cpu, i);
 
diff --git a/sys/dev/hwpmc/hwpmc_ppc970.c b/sys/dev/hwpmc/hwpmc_ppc970.c
index c49f7f2ca7e1..3c7213f8b4b4 100644
--- a/sys/dev/hwpmc/hwpmc_ppc970.c
+++ b/sys/dev/hwpmc/hwpmc_ppc970.c
@@ -519,8 +519,7 @@ ppc970_intr(int cpu, struct trapframe *tf)
 		if (pm->pm_state != PMC_STATE_RUNNING)
 			continue;
 
-		error = pmc_process_interrupt(cpu, PMC_HR, pm, tf,
-		    TRAPF_USERMODE(tf));
+		error = pmc_process_interrupt(PMC_HR, pm, tf);
 		if (error != 0)
 			ppc970_stop_pmc(cpu, i);
 
diff --git a/sys/dev/hwpmc/hwpmc_soft.c b/sys/dev/hwpmc/hwpmc_soft.c
index 1924e1185f2b..77a8c7b3abbb 100644
--- a/sys/dev/hwpmc/hwpmc_soft.c
+++ b/sys/dev/hwpmc/hwpmc_soft.c
@@ -425,8 +425,7 @@ pmc_soft_intr(struct pmckern_soft *ks)
 			else
 				continue;
 			user_mode = TRAPF_USERMODE(ks->pm_tf);
-			error = pmc_process_interrupt(ks->pm_cpu, PMC_SR, pm,
-			    ks->pm_tf, user_mode);
+			error = pmc_process_interrupt(PMC_SR, pm, ks->pm_tf);
 			if (error) {
 				soft_stop_pmc(ks->pm_cpu, ri);
 				continue;
diff --git a/sys/i386/i386/trap.c b/sys/i386/i386/trap.c
index acabc6f404fb..6fa0f14efe92 100644
--- a/sys/i386/i386/trap.c
+++ b/sys/i386/i386/trap.c
@@ -248,7 +248,7 @@ trap(struct trapframe *frame)
 		 * return immediately.
 		 */
 		if (pmc_intr != NULL &&
-		    (*pmc_intr)(PCPU_GET(cpuid), frame) != 0)
+		    (*pmc_intr)(frame) != 0)
 			return;
 #endif
 
diff --git a/sys/kern/kern_pmc.c b/sys/kern/kern_pmc.c
index 0747f84a70f5..3ec091716776 100644
--- a/sys/kern/kern_pmc.c
+++ b/sys/kern/kern_pmc.c
@@ -71,7 +71,7 @@ const int pmc_kernel_version = PMC_KERNEL_VERSION;
 int __read_mostly (*pmc_hook)(struct thread *td, int function, void *arg) = NULL;
 
 /* Interrupt handler */
-int __read_mostly (*pmc_intr)(int cpu, struct trapframe *tf) = NULL;
+int __read_mostly (*pmc_intr)(struct trapframe *tf) = NULL;
 
 DPCPU_DEFINE(uint8_t, pmc_sampled);
 
diff --git a/sys/mips/atheros/apb.c b/sys/mips/atheros/apb.c
index ba323c0fa11e..b1cc83bc5d02 100644
--- a/sys/mips/atheros/apb.c
+++ b/sys/mips/atheros/apb.c
@@ -388,7 +388,7 @@ apb_filter(void *arg)
 					tf = td->td_intr_frame;
 
 					if (pmc_intr)
-						(*pmc_intr)(PCPU_GET(cpuid), tf);
+						(*pmc_intr)(PCPU_GET(tf);
 					continue;
 				}
 				/* Ignore timer interrupts */
diff --git a/sys/mips/cavium/octeon_pmc.c b/sys/mips/cavium/octeon_pmc.c
index 2a0885b74b9b..70fb69ca33ef 100644
--- a/sys/mips/cavium/octeon_pmc.c
+++ b/sys/mips/cavium/octeon_pmc.c
@@ -111,7 +111,7 @@ octeon_pmc_intr(void *arg)
 	struct trapframe *tf = PCPU_GET(curthread)->td_intr_frame;
 
 	if (pmc_intr)
-		(*pmc_intr)(PCPU_GET(cpuid), tf);
+		(*pmc_intr)(PCPU_GET(tf);
 
 	return (FILTER_HANDLED);
 }
diff --git a/sys/powerpc/powerpc/interrupt.c b/sys/powerpc/powerpc/interrupt.c
index 58075fb9a69c..2ad0c5167f66 100644
--- a/sys/powerpc/powerpc/interrupt.c
+++ b/sys/powerpc/powerpc/interrupt.c
@@ -112,7 +112,7 @@ powerpc_interrupt(struct trapframe *framep)
 	case EXC_PERF:
 		critical_enter();
 		KASSERT(pmc_intr != NULL, ("Performance exception, but no handler!"));
-		(*pmc_intr)(PCPU_GET(cpuid), framep);
+		(*pmc_intr)(framep);
 		if (pmc_hook && (PCPU_GET(curthread)->td_pflags & TDP_CALLCHAIN))
 			pmc_hook(PCPU_GET(curthread), PMC_FN_USER_CALLCHAIN, framep);
 		critical_exit();
diff --git a/sys/sys/pmc.h b/sys/sys/pmc.h
index 428d1f381990..0c7a3a331abd 100644
--- a/sys/sys/pmc.h
+++ b/sys/sys/pmc.h
@@ -61,7 +61,7 @@
  *
  * The patch version is incremented for every bug fix.
  */
-#define	PMC_VERSION_MAJOR	0x08
+#define	PMC_VERSION_MAJOR	0x09
 #define	PMC_VERSION_MINOR	0x03
 #define	PMC_VERSION_PATCH	0x0000
 
@@ -1053,7 +1053,7 @@ struct pmc_mdep  {
 	int (*pmd_switch_out)(struct pmc_cpu *_p, struct pmc_process *_pp);
 
 	/* handle a PMC interrupt */
-	int (*pmd_intr)(int _cpu, struct trapframe *_tf);
+	int (*pmd_intr)(struct trapframe *_tf);
 
 	/*
 	 * PMC class dependent information.
@@ -1209,8 +1209,7 @@ MALLOC_DECLARE(M_PMC);
 struct pmc_mdep *pmc_md_initialize(void);	/* MD init function */
 void	pmc_md_finalize(struct pmc_mdep *_md);	/* MD fini function */
 int	pmc_getrowdisp(int _ri);
-int	pmc_process_interrupt(int _cpu, int _ring, struct pmc *_pm,
-    struct trapframe *_tf, int _inuserspace);
+int	pmc_process_interrupt(int _ring, struct pmc *_pm, struct trapframe *_tf);
 int	pmc_save_kernel_callchain(uintptr_t *_cc, int _maxsamples,
     struct trapframe *_tf);
 int	pmc_save_user_callchain(uintptr_t *_cc, int _maxsamples,
diff --git a/sys/sys/pmckern.h b/sys/sys/pmckern.h
index aa4961ede132..69b72139d686 100644
--- a/sys/sys/pmckern.h
+++ b/sys/sys/pmckern.h
@@ -176,7 +176,7 @@ struct pmc_domain_buffer_header {
 
 /* hook */
 extern int (*pmc_hook)(struct thread *_td, int _function, void *_arg);
-extern int (*pmc_intr)(int _cpu, struct trapframe *_frame);
+extern int (*pmc_intr)(struct trapframe *_frame);
 
 /* SX lock protecting the hook */
 extern struct sx pmc_sx;