git: 70c712a86da5 - main - sdt: Support fetching the probe sixth argument with MI machinery

From: Mark Johnston <markj_at_FreeBSD.org>
Date: Thu, 20 Jun 2024 16:45:43 UTC
The branch main has been updated by markj:

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

commit 70c712a86da5d9760074b84dee15e229190bbd11
Author:     Mark Johnston <markj@FreeBSD.org>
AuthorDate: 2024-06-20 16:40:25 +0000
Commit:     Mark Johnston <markj@FreeBSD.org>
CommitDate: 2024-06-20 16:40:25 +0000

    sdt: Support fetching the probe sixth argument with MI machinery
    
    SDT calls dtrace_probe() directly, and this can be used to pass up to
    five probe arguments directly.  To pass the sixth argument (SDT
    currently doesn't support more than this), we use a hack: just add
    additional parameters to the call and cast dtrace_probe accordingly.
    This happens to work on amd64, but doesn't work in general.
    
    Modify SDT to call dtrace_probe() after storing arguments beyond the
    first five in thread-local storage.  Implement sdt_getargval() to fetch
    extra argument values this way.  An alternative would be to use invop
    handlers instead and make sdt_probe_func point to a breakpoint
    instruction, so that one can extract arguments using the breakpoint
    exception trapframe, but this makes the providers more expensive when
    enabled and doesn't seem justified.  This approach works well unless we
    want to add more than one or two more parameters to SDT probes, which
    seems unlikely at present.
    
    In particular, this fixes fetching the last argument of most ip and tcp
    probes on arm64.
    
    Reported by:    rwatson
    Reviewed by:    Domagoj Stolfa
    MFC after:      1 month
    Sponsored by:   Innovate UK
    Differential Revision:  https://reviews.freebsd.org/D45648
---
 sys/cddl/dev/dtrace/dtrace_cddl.h |  3 ++-
 sys/cddl/dev/sdt/sdt.c            | 35 +++++++++++++++++++++++++++++++----
 sys/kern/kern_sdt.c               |  8 ++++----
 sys/sys/sdt.h                     |  5 +----
 4 files changed, 38 insertions(+), 13 deletions(-)

diff --git a/sys/cddl/dev/dtrace/dtrace_cddl.h b/sys/cddl/dev/dtrace/dtrace_cddl.h
index 42a4721fbb90..c151c8cf9660 100644
--- a/sys/cddl/dev/dtrace/dtrace_cddl.h
+++ b/sys/cddl/dev/dtrace/dtrace_cddl.h
@@ -79,7 +79,7 @@ typedef struct kdtrace_thread {
 #ifdef __amd64__
 	uintptr_t	td_dtrace_regv;
 #endif
-	uint64_t	td_hrtime;	/* Last time on cpu. */
+	uintptr_t	td_dtrace_sdt_arg[1];	/* Space for extra SDT args */
 	void		*td_dtrace_sscr; /* Saved scratch space location. */
 	void		*td_systrace_args; /* syscall probe arguments. */
 	uint64_t	td_fasttrap_tp_gen; /* Tracepoint hash table gen. */
@@ -110,6 +110,7 @@ typedef struct kdtrace_thread {
 #define	t_dtrace_scrpc	td_dtrace->td_dtrace_scrpc
 #define	t_dtrace_astpc	td_dtrace->td_dtrace_astpc
 #define	t_dtrace_regv	td_dtrace->td_dtrace_regv
+#define	t_dtrace_sdt_arg	td_dtrace->td_dtrace_sdt_arg
 #define	t_dtrace_sscr	td_dtrace->td_dtrace_sscr
 #define	t_dtrace_systrace_args	td_dtrace->td_systrace_args
 #define	t_fasttrap_tp_gen	td_dtrace->td_fasttrap_tp_gen
diff --git a/sys/cddl/dev/sdt/sdt.c b/sys/cddl/dev/sdt/sdt.c
index 51fa0432437c..461f454186a3 100644
--- a/sys/cddl/dev/sdt/sdt.c
+++ b/sys/cddl/dev/sdt/sdt.c
@@ -58,8 +58,11 @@
 #include <sys/dtrace.h>
 #include <sys/dtrace_bsd.h>
 
+#include <cddl/dev/dtrace/dtrace_cddl.h>
+
 /* DTrace methods. */
 static void	sdt_getargdesc(void *, dtrace_id_t, void *, dtrace_argdesc_t *);
+static uint64_t	sdt_getargval(void *, dtrace_id_t, void *, int, int);
 static void	sdt_provide_probes(void *, dtrace_probedesc_t *);
 static void	sdt_destroy(void *, dtrace_id_t, void *);
 static void	sdt_enable(void *, dtrace_id_t, void *);
@@ -93,7 +96,7 @@ static dtrace_pops_t sdt_pops = {
 	.dtps_suspend =		NULL,
 	.dtps_resume =		NULL,
 	.dtps_getargdesc =	sdt_getargdesc,
-	.dtps_getargval =	NULL,
+	.dtps_getargval =	sdt_getargval,
 	.dtps_usermode =	NULL,
 	.dtps_destroy =		sdt_destroy,
 };
@@ -321,6 +324,23 @@ sdt_getargdesc(void *arg, dtrace_id_t id, void *parg, dtrace_argdesc_t *desc)
 	}
 }
 
+/*
+ * Fetch arguments beyond the first five passed directly to dtrace_probe().
+ * FreeBSD's SDT implement currently only supports up to 6 arguments, so we just
+ * need to handle arg5 here.
+ */
+static uint64_t
+sdt_getargval(void *arg __unused, dtrace_id_t id __unused,
+    void *parg __unused, int argno, int aframes __unused)
+{
+	if (argno != 5) {
+		DTRACE_CPUFLAG_SET(CPU_DTRACE_ILLOP);
+		return (0);
+	} else {
+		return (curthread->t_dtrace_sdt_arg[argno - 5]);
+	}
+}
+
 static void
 sdt_destroy(void *arg, dtrace_id_t id, void *parg)
 {
@@ -449,14 +469,21 @@ sdt_load_probes_cb(linker_file_t lf, void *arg __unused)
 	return (0);
 }
 
+static void
+sdt_dtrace_probe(dtrace_id_t id, uintptr_t arg0, uintptr_t arg1,
+    uintptr_t arg2, uintptr_t arg3, uintptr_t arg4, uintptr_t arg5)
+{
+	curthread->t_dtrace_sdt_arg[0] = arg5;
+	dtrace_probe(id, arg0, arg1, arg2, arg3, arg4);
+}
+
 static void
 sdt_load(void)
 {
 
 	TAILQ_INIT(&sdt_prov_list);
 
-	sdt_probe_func = dtrace_probe;
-	sdt_probe6_func = (sdt_probe6_func_t)dtrace_probe;
+	sdt_probe_func = sdt_dtrace_probe;
 
 	sdt_kld_load_tag = EVENTHANDLER_REGISTER(kld_load, sdt_kld_load, NULL,
 	    EVENTHANDLER_PRI_ANY);
@@ -482,7 +509,6 @@ sdt_unload(void)
 	EVENTHANDLER_DEREGISTER(kld_unload_try, sdt_kld_unload_try_tag);
 
 	sdt_probe_func = sdt_probe_stub;
-	sdt_probe6_func = (sdt_probe6_func_t)sdt_probe_stub;
 
 	TAILQ_FOREACH_SAFE(prov, &sdt_prov_list, prov_entry, tmp) {
 		ret = dtrace_unregister(prov->id);
@@ -515,3 +541,4 @@ SYSUNINIT(sdt_unload, SI_SUB_DTRACE_PROVIDER, SI_ORDER_ANY, sdt_unload, NULL);
 DEV_MODULE(sdt, sdt_modevent, NULL);
 MODULE_VERSION(sdt, 1);
 MODULE_DEPEND(sdt, dtrace, 1, 1, 1);
+MODULE_DEPEND(sdt, opensolaris, 1, 1, 1);
diff --git a/sys/kern/kern_sdt.c b/sys/kern/kern_sdt.c
index b7213d2051fc..9704326299e6 100644
--- a/sys/kern/kern_sdt.c
+++ b/sys/kern/kern_sdt.c
@@ -28,6 +28,7 @@
 #include <sys/param.h>
 #include <sys/systm.h>
 #include <sys/kdb.h>
+#include <sys/proc.h>
 #include <sys/sdt.h>
 
 SDT_PROVIDER_DEFINE(sdt);
@@ -37,7 +38,6 @@ SDT_PROVIDER_DEFINE(sdt);
  * dtrace_probe() when it loads.
  */
 sdt_probe_func_t sdt_probe_func = sdt_probe_stub;
-sdt_probe6_func_t sdt_probe6_func = (sdt_probe6_func_t)sdt_probe_stub;
 volatile bool __read_frequently sdt_probes_enabled;
 
 /*
@@ -48,7 +48,7 @@ volatile bool __read_frequently sdt_probes_enabled;
 void
 sdt_probe_stub(uint32_t id __unused, uintptr_t arg0 __unused,
     uintptr_t arg1 __unused, uintptr_t arg2 __unused, uintptr_t arg3 __unused,
-    uintptr_t arg4 __unused)
+    uintptr_t arg4 __unused, uintptr_t arg5 __unused)
 {
 	printf("sdt_probe_stub: unexpectedly called\n");
 	kdb_backtrace();
@@ -58,12 +58,12 @@ void
 sdt_probe(uint32_t id, uintptr_t arg0, uintptr_t arg1,
     uintptr_t arg2, uintptr_t arg3, uintptr_t arg4)
 {
-	sdt_probe_func(id, arg0, arg1, arg2, arg3, arg4);
+	sdt_probe_func(id, arg0, arg1, arg2, arg3, arg4, 0);
 }
 
 void
 sdt_probe6(uint32_t id, uintptr_t arg0, uintptr_t arg1,
     uintptr_t arg2, uintptr_t arg3, uintptr_t arg4, uintptr_t arg5)
 {
-	sdt_probe6_func(id, arg0, arg1, arg2, arg3, arg4, arg5);
+	sdt_probe_func(id, arg0, arg1, arg2, arg3, arg4, arg5);
 }
diff --git a/sys/sys/sdt.h b/sys/sys/sdt.h
index 147d58c53ef4..0987f1cc19c3 100644
--- a/sys/sys/sdt.h
+++ b/sys/sys/sdt.h
@@ -419,15 +419,12 @@ __sdt_probe##uniq:;							\
  * way to avoid having to rely on CDDL code.
  */
 typedef	void (*sdt_probe_func_t)(uint32_t, uintptr_t arg0, uintptr_t arg1,
-    uintptr_t arg2, uintptr_t arg3, uintptr_t arg4);
-typedef	void (*sdt_probe6_func_t)(uint32_t, uintptr_t arg0, uintptr_t arg1,
     uintptr_t arg2, uintptr_t arg3, uintptr_t arg4, uintptr_t arg5);
 
 /*
  * The 'sdt' provider will set it to dtrace_probe when it loads.
  */
 extern sdt_probe_func_t		sdt_probe_func;
-extern sdt_probe6_func_t	sdt_probe6_func;
 
 struct sdt_probe;
 struct sdt_provider;
@@ -466,7 +463,7 @@ struct sdt_provider {
 };
 
 void sdt_probe_stub(uint32_t, uintptr_t, uintptr_t, uintptr_t, uintptr_t,
-    uintptr_t);
+    uintptr_t, uintptr_t);
 
 SDT_PROVIDER_DECLARE(sdt);