* [PATCH 1/1] firmware: smccc: Add tracepoints when SMCCC calls are made
@ 2021-09-22 22:37 ` Elliot Berman
0 siblings, 0 replies; 6+ messages in thread
From: Elliot Berman @ 2021-09-22 22:37 UTC (permalink / raw)
To: Mark Rutland, Lorenzo Pieralisi, Sudeep Holla, Steven Rostedt,
Ingo Molnar
Cc: Elliot Berman, Prasad Sodagudi, Guru Das Srinagesh, linux-kernel,
linux-arm-kernel, linux-arm-msm
Add trace events for SMCCC calls. These traces allow for convenient
mechanism for kernel to log SMC/HVC instructions without requiring
extracting traces from firmware. SMCCC spec currently [1] allows for 7
argument registers and 4 result registers. The first argument should
always be a function identifier, so use that to match the exit trace.
[1]: ARM DEN 0028D (https://developer.arm.com/documentation/den0028/d/)
Signed-off-by: Elliot Berman <quic_eberman@quicinc.com>
---
drivers/firmware/smccc/Kconfig | 8 ++
drivers/firmware/smccc/Makefile | 1 +
drivers/firmware/smccc/arm-smccc-traces.c | 14 ++++
include/linux/arm-smccc.h | 38 +++++++--
include/trace/events/arm_smccc.h | 99 +++++++++++++++++++++++
5 files changed, 154 insertions(+), 6 deletions(-)
create mode 100644 drivers/firmware/smccc/arm-smccc-traces.c
create mode 100644 include/trace/events/arm_smccc.h
diff --git a/drivers/firmware/smccc/Kconfig b/drivers/firmware/smccc/Kconfig
index 15e7466179a6..fb7d2da1558e 100644
--- a/drivers/firmware/smccc/Kconfig
+++ b/drivers/firmware/smccc/Kconfig
@@ -23,3 +23,11 @@ config ARM_SMCCC_SOC_ID
help
Include support for the SoC bus on the ARM SMCCC firmware based
platforms providing some sysfs information about the SoC variant.
+
+config TRACE_ARM_SMCCC
+ bool "Trace ARM SMCCC Calls"
+ depends on HAVE_ARM_SMCCC
+ depends on TRACEPOINTS
+ default y
+ help
+ Enable trace events to log SMC and HVC service calls.
diff --git a/drivers/firmware/smccc/Makefile b/drivers/firmware/smccc/Makefile
index 40d19144a860..be50ae665040 100644
--- a/drivers/firmware/smccc/Makefile
+++ b/drivers/firmware/smccc/Makefile
@@ -2,3 +2,4 @@
#
obj-$(CONFIG_HAVE_ARM_SMCCC_DISCOVERY) += smccc.o kvm_guest.o
obj-$(CONFIG_ARM_SMCCC_SOC_ID) += soc_id.o
+obj-$(CONFIG_TRACE_ARM_SMCCC) += arm-smccc-traces.o
diff --git a/drivers/firmware/smccc/arm-smccc-traces.c b/drivers/firmware/smccc/arm-smccc-traces.c
new file mode 100644
index 000000000000..9c6282644c02
--- /dev/null
+++ b/drivers/firmware/smccc/arm-smccc-traces.c
@@ -0,0 +1,14 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * ARM SMCCC Trace points
+ *
+ * Copyright (C) 2021, The Linux Foundation. All rights reserved.
+ */
+
+#include <linux/export.h>
+
+#define CREATE_TRACE_POINTS
+#include <trace/events/arm_smccc.h>
+
+EXPORT_TRACEPOINT_SYMBOL_GPL(arm_smccc_start);
+EXPORT_TRACEPOINT_SYMBOL_GPL(arm_smccc_end);
diff --git a/include/linux/arm-smccc.h b/include/linux/arm-smccc.h
index 7d1cabe15262..c214cebb1603 100644
--- a/include/linux/arm-smccc.h
+++ b/include/linux/arm-smccc.h
@@ -190,6 +190,7 @@
#include <linux/linkage.h>
#include <linux/types.h>
+#include <trace/events/arm_smccc.h>
enum arm_smccc_conduit {
SMCCC_CONDUIT_NONE,
@@ -343,13 +344,25 @@ asmlinkage void __arm_smccc_hvc(unsigned long a0, unsigned long a1,
unsigned long a5, unsigned long a6, unsigned long a7,
struct arm_smccc_res *res, struct arm_smccc_quirk *quirk);
-#define arm_smccc_smc(...) __arm_smccc_smc(__VA_ARGS__, NULL)
+#define __arm_smccc(call, arg0, arg1, arg2, arg3, arg4, arg5, arg6, arg7, res, quirk) \
+ do { \
+ trace_arm_smccc_start(tracepoint_string(#call), \
+ (arg0), (arg1), (arg2), (arg3), (arg4), \
+ (arg5), (arg6), (arg7)); \
+ __arm_smccc_##call((arg0), (arg1), (arg2), (arg3), (arg4), (arg5), (arg6), (arg7),\
+ (res), (quirk)); \
+ trace_arm_smccc_end(tracepoint_string(#call), (arg0), \
+ (res)->a0, (res)->a1, (res)->a2, (res)->a3); \
+ } while (0)
+
-#define arm_smccc_smc_quirk(...) __arm_smccc_smc(__VA_ARGS__)
+#define arm_smccc_smc(...) __arm_smccc(smc, __VA_ARGS__, NULL)
-#define arm_smccc_hvc(...) __arm_smccc_hvc(__VA_ARGS__, NULL)
+#define arm_smccc_smc_quirk(...) __arm_smccc(smc, __VA_ARGS__)
-#define arm_smccc_hvc_quirk(...) __arm_smccc_hvc(__VA_ARGS__)
+#define arm_smccc_hvc(...) __arm_smccc(hvc, __VA_ARGS__, NULL)
+
+#define arm_smccc_hvc_quirk(...) __arm_smccc(hvc, __VA_ARGS__)
/* SMCCC v1.1 implementation madness follows */
#ifdef CONFIG_ARM64
@@ -450,22 +463,35 @@ asmlinkage void __arm_smccc_hvc(unsigned long a0, unsigned long a1,
: smccc_sve_clobbers "memory"
#define __constraints(count) ___constraints(count)
+#define __trace_arg_0 ((unsigned long)arg0)
+#define __trace_arg_1 __trace_arg_0, ((unsigned long)arg1)
+#define __trace_arg_2 __trace_arg_1, ((unsigned long)arg2)
+#define __trace_arg_3 __trace_arg_2, ((unsigned long)arg3)
+#define __trace_arg_4 __trace_arg_3, ((unsigned long)arg4)
+#define __trace_arg_5 __trace_arg_4, ((unsigned long)arg5)
+#define __trace_arg_6 __trace_arg_5, ((unsigned long)arg6)
+#define __trace_arg_7 __trace_arg_6, ((unsigned long)arg7)
+#define ___trace_args(count) __trace_arg_##count
+#define __trace_args(count) ___trace_args(count)
+
/*
* We have an output list that is not necessarily used, and GCC feels
* entitled to optimise the whole sequence away. "volatile" is what
* makes it stick.
*/
-#define __arm_smccc_1_1(inst, ...) \
+#define __arm_smccc_1_1(inst, a0, ...) \
do { \
register unsigned long r0 asm("r0"); \
register unsigned long r1 asm("r1"); \
register unsigned long r2 asm("r2"); \
register unsigned long r3 asm("r3"); \
__declare_args(__count_args(__VA_ARGS__), __VA_ARGS__); \
+ trace_arm_smccc_start_vargs(tracepoint_string(inst), __trace_args(__count_args(a0, __VA_ARGS__))); \
asm volatile(SMCCC_SVE_CHECK \
inst "\n" : \
"=r" (r0), "=r" (r1), "=r" (r2), "=r" (r3) \
- __constraints(__count_args(__VA_ARGS__))); \
+ __constraints(__count_args(a0, __VA_ARGS__))); \
+ trace_arm_smccc_end(tracepoint_string(inst), a0, r0, r1, r2, r3); \
if (___res) \
*___res = (typeof(*___res)){r0, r1, r2, r3}; \
} while (0)
diff --git a/include/trace/events/arm_smccc.h b/include/trace/events/arm_smccc.h
new file mode 100644
index 000000000000..533377d2d066
--- /dev/null
+++ b/include/trace/events/arm_smccc.h
@@ -0,0 +1,99 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright (c) 2021, Qualcomm Innovation Center, Inc. All rights reserved.
+ */
+
+#if IS_REACHABLE(CONFIG_TRACE_ARM_SMCCC) && !defined(__KVM_NVHE_HYPERVISOR__)
+
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM arm_smccc
+
+#if !defined(_TRACE_ARM_SMCCC_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_ARM_SMCCC_H
+
+
+#include <linux/tracepoint.h>
+
+TRACE_EVENT(arm_smccc_start,
+ TP_PROTO(const char *call, unsigned long a0, unsigned long a1, unsigned long a2,
+ unsigned long a3, unsigned long a4, unsigned long a5, unsigned long a6,
+ unsigned long a7),
+
+ TP_ARGS(call, a0, a1, a2, a3, a4, a5, a6, a7),
+
+ TP_STRUCT__entry(
+ __field(unsigned long, a0)
+ __field(unsigned long, a1)
+ __field(unsigned long, a2)
+ __field(unsigned long, a3)
+ __field(unsigned long, a4)
+ __field(unsigned long, a5)
+ __field(unsigned long, a6)
+ __field(unsigned long, a7)
+ __string(call, call)
+ ),
+
+ TP_fast_assign(
+ __entry->a0 = a0;
+ __entry->a1 = a1;
+ __entry->a2 = a2;
+ __entry->a3 = a3;
+ __entry->a4 = a4;
+ __entry->a5 = a5;
+ __entry->a6 = a6;
+ __entry->a7 = a7;
+
+ __assign_str(call, call);
+ ),
+
+ TP_printk("call=%s fn=%04x args={%0lx %0lx %0lx %0lx %0lx %0lx %0lx}",
+ __get_str(call), __entry->a0, __entry->a1, __entry->a2,
+ __entry->a3, __entry->a4, __entry->a5, __entry->a6, __entry->a7)
+);
+
+TRACE_EVENT(arm_smccc_end,
+ TP_PROTO(const char *call, unsigned long a0, unsigned long r0, unsigned long r1,
+ unsigned long r2, unsigned long r3),
+
+ TP_ARGS(call, a0, r0, r1, r2, r3),
+
+ TP_STRUCT__entry(
+ __field(unsigned long, a0)
+ __field(unsigned long, r0)
+ __field(unsigned long, r1)
+ __field(unsigned long, r2)
+ __field(unsigned long, r3)
+ __string(call, call)
+ ),
+
+ TP_fast_assign(
+ __entry->a0 = a0;
+ __entry->r0 = r0;
+ __entry->r1 = r1;
+ __entry->r2 = r2;
+ __entry->r3 = r3;
+
+ __assign_str(call, call);
+ ),
+
+ TP_printk("call=%s fn=%04x res={%0lx %0lx %0lx %0lx}",
+ __get_str(call), __entry->a0, __entry->r0, __entry->r1,
+ __entry->r2, __entry->r3)
+);
+
+#define _trace_arm_smccc_start(call, a0, a1, a2, a3, a4, a5, a6, a7, ...) \
+ trace_arm_smccc_start(call, a0, a1, a2, a3, a4, a5, a6, a7)
+
+#define trace_arm_smccc_start_vargs(call, ...) \
+ _trace_arm_smccc_start(call, __VA_ARGS__, 0, 0, 0, 0, 0, 0, 0, 0)
+
+#endif
+
+/* This part must be outside protection */
+#include <trace/define_trace.h>
+
+#else
+#define trace_arm_smccc_start(...)
+#define trace_arm_smccc_end(...)
+
+#define trace_arm_smccc_start_vargs(...)
+#endif
--
2.33.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 1/1] firmware: smccc: Add tracepoints when SMCCC calls are made
@ 2021-09-22 22:37 ` Elliot Berman
0 siblings, 0 replies; 6+ messages in thread
From: Elliot Berman @ 2021-09-22 22:37 UTC (permalink / raw)
To: Mark Rutland, Lorenzo Pieralisi, Sudeep Holla, Steven Rostedt,
Ingo Molnar
Cc: Elliot Berman, Prasad Sodagudi, Guru Das Srinagesh, linux-kernel,
linux-arm-kernel, linux-arm-msm
Add trace events for SMCCC calls. These traces allow for convenient
mechanism for kernel to log SMC/HVC instructions without requiring
extracting traces from firmware. SMCCC spec currently [1] allows for 7
argument registers and 4 result registers. The first argument should
always be a function identifier, so use that to match the exit trace.
[1]: ARM DEN 0028D (https://developer.arm.com/documentation/den0028/d/)
Signed-off-by: Elliot Berman <quic_eberman@quicinc.com>
---
drivers/firmware/smccc/Kconfig | 8 ++
drivers/firmware/smccc/Makefile | 1 +
drivers/firmware/smccc/arm-smccc-traces.c | 14 ++++
include/linux/arm-smccc.h | 38 +++++++--
include/trace/events/arm_smccc.h | 99 +++++++++++++++++++++++
5 files changed, 154 insertions(+), 6 deletions(-)
create mode 100644 drivers/firmware/smccc/arm-smccc-traces.c
create mode 100644 include/trace/events/arm_smccc.h
diff --git a/drivers/firmware/smccc/Kconfig b/drivers/firmware/smccc/Kconfig
index 15e7466179a6..fb7d2da1558e 100644
--- a/drivers/firmware/smccc/Kconfig
+++ b/drivers/firmware/smccc/Kconfig
@@ -23,3 +23,11 @@ config ARM_SMCCC_SOC_ID
help
Include support for the SoC bus on the ARM SMCCC firmware based
platforms providing some sysfs information about the SoC variant.
+
+config TRACE_ARM_SMCCC
+ bool "Trace ARM SMCCC Calls"
+ depends on HAVE_ARM_SMCCC
+ depends on TRACEPOINTS
+ default y
+ help
+ Enable trace events to log SMC and HVC service calls.
diff --git a/drivers/firmware/smccc/Makefile b/drivers/firmware/smccc/Makefile
index 40d19144a860..be50ae665040 100644
--- a/drivers/firmware/smccc/Makefile
+++ b/drivers/firmware/smccc/Makefile
@@ -2,3 +2,4 @@
#
obj-$(CONFIG_HAVE_ARM_SMCCC_DISCOVERY) += smccc.o kvm_guest.o
obj-$(CONFIG_ARM_SMCCC_SOC_ID) += soc_id.o
+obj-$(CONFIG_TRACE_ARM_SMCCC) += arm-smccc-traces.o
diff --git a/drivers/firmware/smccc/arm-smccc-traces.c b/drivers/firmware/smccc/arm-smccc-traces.c
new file mode 100644
index 000000000000..9c6282644c02
--- /dev/null
+++ b/drivers/firmware/smccc/arm-smccc-traces.c
@@ -0,0 +1,14 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * ARM SMCCC Trace points
+ *
+ * Copyright (C) 2021, The Linux Foundation. All rights reserved.
+ */
+
+#include <linux/export.h>
+
+#define CREATE_TRACE_POINTS
+#include <trace/events/arm_smccc.h>
+
+EXPORT_TRACEPOINT_SYMBOL_GPL(arm_smccc_start);
+EXPORT_TRACEPOINT_SYMBOL_GPL(arm_smccc_end);
diff --git a/include/linux/arm-smccc.h b/include/linux/arm-smccc.h
index 7d1cabe15262..c214cebb1603 100644
--- a/include/linux/arm-smccc.h
+++ b/include/linux/arm-smccc.h
@@ -190,6 +190,7 @@
#include <linux/linkage.h>
#include <linux/types.h>
+#include <trace/events/arm_smccc.h>
enum arm_smccc_conduit {
SMCCC_CONDUIT_NONE,
@@ -343,13 +344,25 @@ asmlinkage void __arm_smccc_hvc(unsigned long a0, unsigned long a1,
unsigned long a5, unsigned long a6, unsigned long a7,
struct arm_smccc_res *res, struct arm_smccc_quirk *quirk);
-#define arm_smccc_smc(...) __arm_smccc_smc(__VA_ARGS__, NULL)
+#define __arm_smccc(call, arg0, arg1, arg2, arg3, arg4, arg5, arg6, arg7, res, quirk) \
+ do { \
+ trace_arm_smccc_start(tracepoint_string(#call), \
+ (arg0), (arg1), (arg2), (arg3), (arg4), \
+ (arg5), (arg6), (arg7)); \
+ __arm_smccc_##call((arg0), (arg1), (arg2), (arg3), (arg4), (arg5), (arg6), (arg7),\
+ (res), (quirk)); \
+ trace_arm_smccc_end(tracepoint_string(#call), (arg0), \
+ (res)->a0, (res)->a1, (res)->a2, (res)->a3); \
+ } while (0)
+
-#define arm_smccc_smc_quirk(...) __arm_smccc_smc(__VA_ARGS__)
+#define arm_smccc_smc(...) __arm_smccc(smc, __VA_ARGS__, NULL)
-#define arm_smccc_hvc(...) __arm_smccc_hvc(__VA_ARGS__, NULL)
+#define arm_smccc_smc_quirk(...) __arm_smccc(smc, __VA_ARGS__)
-#define arm_smccc_hvc_quirk(...) __arm_smccc_hvc(__VA_ARGS__)
+#define arm_smccc_hvc(...) __arm_smccc(hvc, __VA_ARGS__, NULL)
+
+#define arm_smccc_hvc_quirk(...) __arm_smccc(hvc, __VA_ARGS__)
/* SMCCC v1.1 implementation madness follows */
#ifdef CONFIG_ARM64
@@ -450,22 +463,35 @@ asmlinkage void __arm_smccc_hvc(unsigned long a0, unsigned long a1,
: smccc_sve_clobbers "memory"
#define __constraints(count) ___constraints(count)
+#define __trace_arg_0 ((unsigned long)arg0)
+#define __trace_arg_1 __trace_arg_0, ((unsigned long)arg1)
+#define __trace_arg_2 __trace_arg_1, ((unsigned long)arg2)
+#define __trace_arg_3 __trace_arg_2, ((unsigned long)arg3)
+#define __trace_arg_4 __trace_arg_3, ((unsigned long)arg4)
+#define __trace_arg_5 __trace_arg_4, ((unsigned long)arg5)
+#define __trace_arg_6 __trace_arg_5, ((unsigned long)arg6)
+#define __trace_arg_7 __trace_arg_6, ((unsigned long)arg7)
+#define ___trace_args(count) __trace_arg_##count
+#define __trace_args(count) ___trace_args(count)
+
/*
* We have an output list that is not necessarily used, and GCC feels
* entitled to optimise the whole sequence away. "volatile" is what
* makes it stick.
*/
-#define __arm_smccc_1_1(inst, ...) \
+#define __arm_smccc_1_1(inst, a0, ...) \
do { \
register unsigned long r0 asm("r0"); \
register unsigned long r1 asm("r1"); \
register unsigned long r2 asm("r2"); \
register unsigned long r3 asm("r3"); \
__declare_args(__count_args(__VA_ARGS__), __VA_ARGS__); \
+ trace_arm_smccc_start_vargs(tracepoint_string(inst), __trace_args(__count_args(a0, __VA_ARGS__))); \
asm volatile(SMCCC_SVE_CHECK \
inst "\n" : \
"=r" (r0), "=r" (r1), "=r" (r2), "=r" (r3) \
- __constraints(__count_args(__VA_ARGS__))); \
+ __constraints(__count_args(a0, __VA_ARGS__))); \
+ trace_arm_smccc_end(tracepoint_string(inst), a0, r0, r1, r2, r3); \
if (___res) \
*___res = (typeof(*___res)){r0, r1, r2, r3}; \
} while (0)
diff --git a/include/trace/events/arm_smccc.h b/include/trace/events/arm_smccc.h
new file mode 100644
index 000000000000..533377d2d066
--- /dev/null
+++ b/include/trace/events/arm_smccc.h
@@ -0,0 +1,99 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright (c) 2021, Qualcomm Innovation Center, Inc. All rights reserved.
+ */
+
+#if IS_REACHABLE(CONFIG_TRACE_ARM_SMCCC) && !defined(__KVM_NVHE_HYPERVISOR__)
+
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM arm_smccc
+
+#if !defined(_TRACE_ARM_SMCCC_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_ARM_SMCCC_H
+
+
+#include <linux/tracepoint.h>
+
+TRACE_EVENT(arm_smccc_start,
+ TP_PROTO(const char *call, unsigned long a0, unsigned long a1, unsigned long a2,
+ unsigned long a3, unsigned long a4, unsigned long a5, unsigned long a6,
+ unsigned long a7),
+
+ TP_ARGS(call, a0, a1, a2, a3, a4, a5, a6, a7),
+
+ TP_STRUCT__entry(
+ __field(unsigned long, a0)
+ __field(unsigned long, a1)
+ __field(unsigned long, a2)
+ __field(unsigned long, a3)
+ __field(unsigned long, a4)
+ __field(unsigned long, a5)
+ __field(unsigned long, a6)
+ __field(unsigned long, a7)
+ __string(call, call)
+ ),
+
+ TP_fast_assign(
+ __entry->a0 = a0;
+ __entry->a1 = a1;
+ __entry->a2 = a2;
+ __entry->a3 = a3;
+ __entry->a4 = a4;
+ __entry->a5 = a5;
+ __entry->a6 = a6;
+ __entry->a7 = a7;
+
+ __assign_str(call, call);
+ ),
+
+ TP_printk("call=%s fn=%04x args={%0lx %0lx %0lx %0lx %0lx %0lx %0lx}",
+ __get_str(call), __entry->a0, __entry->a1, __entry->a2,
+ __entry->a3, __entry->a4, __entry->a5, __entry->a6, __entry->a7)
+);
+
+TRACE_EVENT(arm_smccc_end,
+ TP_PROTO(const char *call, unsigned long a0, unsigned long r0, unsigned long r1,
+ unsigned long r2, unsigned long r3),
+
+ TP_ARGS(call, a0, r0, r1, r2, r3),
+
+ TP_STRUCT__entry(
+ __field(unsigned long, a0)
+ __field(unsigned long, r0)
+ __field(unsigned long, r1)
+ __field(unsigned long, r2)
+ __field(unsigned long, r3)
+ __string(call, call)
+ ),
+
+ TP_fast_assign(
+ __entry->a0 = a0;
+ __entry->r0 = r0;
+ __entry->r1 = r1;
+ __entry->r2 = r2;
+ __entry->r3 = r3;
+
+ __assign_str(call, call);
+ ),
+
+ TP_printk("call=%s fn=%04x res={%0lx %0lx %0lx %0lx}",
+ __get_str(call), __entry->a0, __entry->r0, __entry->r1,
+ __entry->r2, __entry->r3)
+);
+
+#define _trace_arm_smccc_start(call, a0, a1, a2, a3, a4, a5, a6, a7, ...) \
+ trace_arm_smccc_start(call, a0, a1, a2, a3, a4, a5, a6, a7)
+
+#define trace_arm_smccc_start_vargs(call, ...) \
+ _trace_arm_smccc_start(call, __VA_ARGS__, 0, 0, 0, 0, 0, 0, 0, 0)
+
+#endif
+
+/* This part must be outside protection */
+#include <trace/define_trace.h>
+
+#else
+#define trace_arm_smccc_start(...)
+#define trace_arm_smccc_end(...)
+
+#define trace_arm_smccc_start_vargs(...)
+#endif
--
2.33.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] firmware: smccc: Add tracepoints when SMCCC calls are made
2021-09-22 22:37 ` Elliot Berman
@ 2021-09-23 11:21 ` Mark Rutland
-1 siblings, 0 replies; 6+ messages in thread
From: Mark Rutland @ 2021-09-23 11:21 UTC (permalink / raw)
To: Elliot Berman
Cc: Lorenzo Pieralisi, Sudeep Holla, Steven Rostedt, Ingo Molnar,
Prasad Sodagudi, Guru Das Srinagesh, linux-kernel,
linux-arm-kernel, linux-arm-msm
On Wed, Sep 22, 2021 at 03:37:00PM -0700, Elliot Berman wrote:
> Add trace events for SMCCC calls. These traces allow for convenient
> mechanism for kernel to log SMC/HVC instructions without requiring
> extracting traces from firmware. SMCCC spec currently [1] allows for 7
> argument registers and 4 result registers.
I think you've missed additions in recent versions of the spec. Since
SMCCCv1.2 (which is described in version of the document you've linked),
SMC64 calls may pass up to 17 argument registers (x1-x17) and receive up
to 18 result registers (x0-x17). SMC32 calls have up to 7 argument
registers (r1-r7) and up to 8 return registers (r0-r7).
What do you want to use this for? What specifically do you want to
trace.
I'm worried that this is a very low level transport, and hooking this
means tracing a bunch of unrelated users (e.g. PSCI, ARCH_WORKAROUND*
calls, vendor-specific SMC interfaces), and potentially gets in the way
of some of those use-cases (e.g. tracng this means it cannot be used
from noinstr code, which we likely need to be able to do in future).
Generally I'd prefer to have tracepoints in specific drivers rather than
in the SMCCC transport.
> The first argument should
> always be a function identifier, so use that to match the exit trace.
>
> [1]: ARM DEN 0028D (https://developer.arm.com/documentation/den0028/d/)
>
> Signed-off-by: Elliot Berman <quic_eberman@quicinc.com>
> ---
> drivers/firmware/smccc/Kconfig | 8 ++
> drivers/firmware/smccc/Makefile | 1 +
> drivers/firmware/smccc/arm-smccc-traces.c | 14 ++++
> include/linux/arm-smccc.h | 38 +++++++--
> include/trace/events/arm_smccc.h | 99 +++++++++++++++++++++++
> 5 files changed, 154 insertions(+), 6 deletions(-)
> create mode 100644 drivers/firmware/smccc/arm-smccc-traces.c
> create mode 100644 include/trace/events/arm_smccc.h
>
> diff --git a/drivers/firmware/smccc/Kconfig b/drivers/firmware/smccc/Kconfig
> index 15e7466179a6..fb7d2da1558e 100644
> --- a/drivers/firmware/smccc/Kconfig
> +++ b/drivers/firmware/smccc/Kconfig
> @@ -23,3 +23,11 @@ config ARM_SMCCC_SOC_ID
> help
> Include support for the SoC bus on the ARM SMCCC firmware based
> platforms providing some sysfs information about the SoC variant.
> +
> +config TRACE_ARM_SMCCC
> + bool "Trace ARM SMCCC Calls"
> + depends on HAVE_ARM_SMCCC
> + depends on TRACEPOINTS
> + default y
> + help
> + Enable trace events to log SMC and HVC service calls.
> diff --git a/drivers/firmware/smccc/Makefile b/drivers/firmware/smccc/Makefile
> index 40d19144a860..be50ae665040 100644
> --- a/drivers/firmware/smccc/Makefile
> +++ b/drivers/firmware/smccc/Makefile
> @@ -2,3 +2,4 @@
> #
> obj-$(CONFIG_HAVE_ARM_SMCCC_DISCOVERY) += smccc.o kvm_guest.o
> obj-$(CONFIG_ARM_SMCCC_SOC_ID) += soc_id.o
> +obj-$(CONFIG_TRACE_ARM_SMCCC) += arm-smccc-traces.o
> diff --git a/drivers/firmware/smccc/arm-smccc-traces.c b/drivers/firmware/smccc/arm-smccc-traces.c
> new file mode 100644
> index 000000000000..9c6282644c02
> --- /dev/null
> +++ b/drivers/firmware/smccc/arm-smccc-traces.c
> @@ -0,0 +1,14 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * ARM SMCCC Trace points
> + *
> + * Copyright (C) 2021, The Linux Foundation. All rights reserved.
> + */
> +
> +#include <linux/export.h>
> +
> +#define CREATE_TRACE_POINTS
> +#include <trace/events/arm_smccc.h>
> +
> +EXPORT_TRACEPOINT_SYMBOL_GPL(arm_smccc_start);
> +EXPORT_TRACEPOINT_SYMBOL_GPL(arm_smccc_end);
> diff --git a/include/linux/arm-smccc.h b/include/linux/arm-smccc.h
> index 7d1cabe15262..c214cebb1603 100644
> --- a/include/linux/arm-smccc.h
> +++ b/include/linux/arm-smccc.h
> @@ -190,6 +190,7 @@
>
> #include <linux/linkage.h>
> #include <linux/types.h>
> +#include <trace/events/arm_smccc.h>
>
> enum arm_smccc_conduit {
> SMCCC_CONDUIT_NONE,
> @@ -343,13 +344,25 @@ asmlinkage void __arm_smccc_hvc(unsigned long a0, unsigned long a1,
> unsigned long a5, unsigned long a6, unsigned long a7,
> struct arm_smccc_res *res, struct arm_smccc_quirk *quirk);
>
> -#define arm_smccc_smc(...) __arm_smccc_smc(__VA_ARGS__, NULL)
> +#define __arm_smccc(call, arg0, arg1, arg2, arg3, arg4, arg5, arg6, arg7, res, quirk) \
> + do { \
> + trace_arm_smccc_start(tracepoint_string(#call), \
> + (arg0), (arg1), (arg2), (arg3), (arg4), \
> + (arg5), (arg6), (arg7)); \
> + __arm_smccc_##call((arg0), (arg1), (arg2), (arg3), (arg4), (arg5), (arg6), (arg7),\
> + (res), (quirk)); \
> + trace_arm_smccc_end(tracepoint_string(#call), (arg0), \
> + (res)->a0, (res)->a1, (res)->a2, (res)->a3); \
> + } while (0)
> +
>
> -#define arm_smccc_smc_quirk(...) __arm_smccc_smc(__VA_ARGS__)
> +#define arm_smccc_smc(...) __arm_smccc(smc, __VA_ARGS__, NULL)
>
> -#define arm_smccc_hvc(...) __arm_smccc_hvc(__VA_ARGS__, NULL)
> +#define arm_smccc_smc_quirk(...) __arm_smccc(smc, __VA_ARGS__)
>
> -#define arm_smccc_hvc_quirk(...) __arm_smccc_hvc(__VA_ARGS__)
> +#define arm_smccc_hvc(...) __arm_smccc(hvc, __VA_ARGS__, NULL)
> +
> +#define arm_smccc_hvc_quirk(...) __arm_smccc(hvc, __VA_ARGS__)
>
> /* SMCCC v1.1 implementation madness follows */
> #ifdef CONFIG_ARM64
> @@ -450,22 +463,35 @@ asmlinkage void __arm_smccc_hvc(unsigned long a0, unsigned long a1,
> : smccc_sve_clobbers "memory"
> #define __constraints(count) ___constraints(count)
>
> +#define __trace_arg_0 ((unsigned long)arg0)
> +#define __trace_arg_1 __trace_arg_0, ((unsigned long)arg1)
> +#define __trace_arg_2 __trace_arg_1, ((unsigned long)arg2)
> +#define __trace_arg_3 __trace_arg_2, ((unsigned long)arg3)
> +#define __trace_arg_4 __trace_arg_3, ((unsigned long)arg4)
> +#define __trace_arg_5 __trace_arg_4, ((unsigned long)arg5)
> +#define __trace_arg_6 __trace_arg_5, ((unsigned long)arg6)
> +#define __trace_arg_7 __trace_arg_6, ((unsigned long)arg7)
> +#define ___trace_args(count) __trace_arg_##count
> +#define __trace_args(count) ___trace_args(count)
> +
> /*
> * We have an output list that is not necessarily used, and GCC feels
> * entitled to optimise the whole sequence away. "volatile" is what
> * makes it stick.
> */
> -#define __arm_smccc_1_1(inst, ...) \
> +#define __arm_smccc_1_1(inst, a0, ...) \
> do { \
> register unsigned long r0 asm("r0"); \
> register unsigned long r1 asm("r1"); \
> register unsigned long r2 asm("r2"); \
> register unsigned long r3 asm("r3"); \
> __declare_args(__count_args(__VA_ARGS__), __VA_ARGS__); \
> + trace_arm_smccc_start_vargs(tracepoint_string(inst), __trace_args(__count_args(a0, __VA_ARGS__))); \
> asm volatile(SMCCC_SVE_CHECK \
> inst "\n" : \
> "=r" (r0), "=r" (r1), "=r" (r2), "=r" (r3) \
> - __constraints(__count_args(__VA_ARGS__))); \
> + __constraints(__count_args(a0, __VA_ARGS__))); \
> + trace_arm_smccc_end(tracepoint_string(inst), a0, r0, r1, r2, r3); \
> if (___res) \
> *___res = (typeof(*___res)){r0, r1, r2, r3}; \
> } while (0)
We have other functions for incoking SMCCC calls, e.g.
* arm_smccc_smc()
* arm_smccc_hvc()
* arm_smccc_1_2_hvc()
* arm_smccc_1_2_smc()
... so if you really need to hook all SMCCC calls I'd expect you need to
hook those too, somehow
Thanks,
Mark.
> diff --git a/include/trace/events/arm_smccc.h b/include/trace/events/arm_smccc.h
> new file mode 100644
> index 000000000000..533377d2d066
> --- /dev/null
> +++ b/include/trace/events/arm_smccc.h
> @@ -0,0 +1,99 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/* Copyright (c) 2021, Qualcomm Innovation Center, Inc. All rights reserved.
> + */
> +
> +#if IS_REACHABLE(CONFIG_TRACE_ARM_SMCCC) && !defined(__KVM_NVHE_HYPERVISOR__)
> +
> +#undef TRACE_SYSTEM
> +#define TRACE_SYSTEM arm_smccc
> +
> +#if !defined(_TRACE_ARM_SMCCC_H) || defined(TRACE_HEADER_MULTI_READ)
> +#define _TRACE_ARM_SMCCC_H
> +
> +
> +#include <linux/tracepoint.h>
> +
> +TRACE_EVENT(arm_smccc_start,
> + TP_PROTO(const char *call, unsigned long a0, unsigned long a1, unsigned long a2,
> + unsigned long a3, unsigned long a4, unsigned long a5, unsigned long a6,
> + unsigned long a7),
> +
> + TP_ARGS(call, a0, a1, a2, a3, a4, a5, a6, a7),
> +
> + TP_STRUCT__entry(
> + __field(unsigned long, a0)
> + __field(unsigned long, a1)
> + __field(unsigned long, a2)
> + __field(unsigned long, a3)
> + __field(unsigned long, a4)
> + __field(unsigned long, a5)
> + __field(unsigned long, a6)
> + __field(unsigned long, a7)
> + __string(call, call)
> + ),
> +
> + TP_fast_assign(
> + __entry->a0 = a0;
> + __entry->a1 = a1;
> + __entry->a2 = a2;
> + __entry->a3 = a3;
> + __entry->a4 = a4;
> + __entry->a5 = a5;
> + __entry->a6 = a6;
> + __entry->a7 = a7;
> +
> + __assign_str(call, call);
> + ),
> +
> + TP_printk("call=%s fn=%04x args={%0lx %0lx %0lx %0lx %0lx %0lx %0lx}",
> + __get_str(call), __entry->a0, __entry->a1, __entry->a2,
> + __entry->a3, __entry->a4, __entry->a5, __entry->a6, __entry->a7)
> +);
> +
> +TRACE_EVENT(arm_smccc_end,
> + TP_PROTO(const char *call, unsigned long a0, unsigned long r0, unsigned long r1,
> + unsigned long r2, unsigned long r3),
> +
> + TP_ARGS(call, a0, r0, r1, r2, r3),
> +
> + TP_STRUCT__entry(
> + __field(unsigned long, a0)
> + __field(unsigned long, r0)
> + __field(unsigned long, r1)
> + __field(unsigned long, r2)
> + __field(unsigned long, r3)
> + __string(call, call)
> + ),
> +
> + TP_fast_assign(
> + __entry->a0 = a0;
> + __entry->r0 = r0;
> + __entry->r1 = r1;
> + __entry->r2 = r2;
> + __entry->r3 = r3;
> +
> + __assign_str(call, call);
> + ),
> +
> + TP_printk("call=%s fn=%04x res={%0lx %0lx %0lx %0lx}",
> + __get_str(call), __entry->a0, __entry->r0, __entry->r1,
> + __entry->r2, __entry->r3)
> +);
> +
> +#define _trace_arm_smccc_start(call, a0, a1, a2, a3, a4, a5, a6, a7, ...) \
> + trace_arm_smccc_start(call, a0, a1, a2, a3, a4, a5, a6, a7)
> +
> +#define trace_arm_smccc_start_vargs(call, ...) \
> + _trace_arm_smccc_start(call, __VA_ARGS__, 0, 0, 0, 0, 0, 0, 0, 0)
> +
> +#endif
> +
> +/* This part must be outside protection */
> +#include <trace/define_trace.h>
> +
> +#else
> +#define trace_arm_smccc_start(...)
> +#define trace_arm_smccc_end(...)
> +
> +#define trace_arm_smccc_start_vargs(...)
> +#endif
> --
> 2.33.0
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] firmware: smccc: Add tracepoints when SMCCC calls are made
@ 2021-09-23 11:21 ` Mark Rutland
0 siblings, 0 replies; 6+ messages in thread
From: Mark Rutland @ 2021-09-23 11:21 UTC (permalink / raw)
To: Elliot Berman
Cc: Lorenzo Pieralisi, Sudeep Holla, Steven Rostedt, Ingo Molnar,
Prasad Sodagudi, Guru Das Srinagesh, linux-kernel,
linux-arm-kernel, linux-arm-msm
On Wed, Sep 22, 2021 at 03:37:00PM -0700, Elliot Berman wrote:
> Add trace events for SMCCC calls. These traces allow for convenient
> mechanism for kernel to log SMC/HVC instructions without requiring
> extracting traces from firmware. SMCCC spec currently [1] allows for 7
> argument registers and 4 result registers.
I think you've missed additions in recent versions of the spec. Since
SMCCCv1.2 (which is described in version of the document you've linked),
SMC64 calls may pass up to 17 argument registers (x1-x17) and receive up
to 18 result registers (x0-x17). SMC32 calls have up to 7 argument
registers (r1-r7) and up to 8 return registers (r0-r7).
What do you want to use this for? What specifically do you want to
trace.
I'm worried that this is a very low level transport, and hooking this
means tracing a bunch of unrelated users (e.g. PSCI, ARCH_WORKAROUND*
calls, vendor-specific SMC interfaces), and potentially gets in the way
of some of those use-cases (e.g. tracng this means it cannot be used
from noinstr code, which we likely need to be able to do in future).
Generally I'd prefer to have tracepoints in specific drivers rather than
in the SMCCC transport.
> The first argument should
> always be a function identifier, so use that to match the exit trace.
>
> [1]: ARM DEN 0028D (https://developer.arm.com/documentation/den0028/d/)
>
> Signed-off-by: Elliot Berman <quic_eberman@quicinc.com>
> ---
> drivers/firmware/smccc/Kconfig | 8 ++
> drivers/firmware/smccc/Makefile | 1 +
> drivers/firmware/smccc/arm-smccc-traces.c | 14 ++++
> include/linux/arm-smccc.h | 38 +++++++--
> include/trace/events/arm_smccc.h | 99 +++++++++++++++++++++++
> 5 files changed, 154 insertions(+), 6 deletions(-)
> create mode 100644 drivers/firmware/smccc/arm-smccc-traces.c
> create mode 100644 include/trace/events/arm_smccc.h
>
> diff --git a/drivers/firmware/smccc/Kconfig b/drivers/firmware/smccc/Kconfig
> index 15e7466179a6..fb7d2da1558e 100644
> --- a/drivers/firmware/smccc/Kconfig
> +++ b/drivers/firmware/smccc/Kconfig
> @@ -23,3 +23,11 @@ config ARM_SMCCC_SOC_ID
> help
> Include support for the SoC bus on the ARM SMCCC firmware based
> platforms providing some sysfs information about the SoC variant.
> +
> +config TRACE_ARM_SMCCC
> + bool "Trace ARM SMCCC Calls"
> + depends on HAVE_ARM_SMCCC
> + depends on TRACEPOINTS
> + default y
> + help
> + Enable trace events to log SMC and HVC service calls.
> diff --git a/drivers/firmware/smccc/Makefile b/drivers/firmware/smccc/Makefile
> index 40d19144a860..be50ae665040 100644
> --- a/drivers/firmware/smccc/Makefile
> +++ b/drivers/firmware/smccc/Makefile
> @@ -2,3 +2,4 @@
> #
> obj-$(CONFIG_HAVE_ARM_SMCCC_DISCOVERY) += smccc.o kvm_guest.o
> obj-$(CONFIG_ARM_SMCCC_SOC_ID) += soc_id.o
> +obj-$(CONFIG_TRACE_ARM_SMCCC) += arm-smccc-traces.o
> diff --git a/drivers/firmware/smccc/arm-smccc-traces.c b/drivers/firmware/smccc/arm-smccc-traces.c
> new file mode 100644
> index 000000000000..9c6282644c02
> --- /dev/null
> +++ b/drivers/firmware/smccc/arm-smccc-traces.c
> @@ -0,0 +1,14 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * ARM SMCCC Trace points
> + *
> + * Copyright (C) 2021, The Linux Foundation. All rights reserved.
> + */
> +
> +#include <linux/export.h>
> +
> +#define CREATE_TRACE_POINTS
> +#include <trace/events/arm_smccc.h>
> +
> +EXPORT_TRACEPOINT_SYMBOL_GPL(arm_smccc_start);
> +EXPORT_TRACEPOINT_SYMBOL_GPL(arm_smccc_end);
> diff --git a/include/linux/arm-smccc.h b/include/linux/arm-smccc.h
> index 7d1cabe15262..c214cebb1603 100644
> --- a/include/linux/arm-smccc.h
> +++ b/include/linux/arm-smccc.h
> @@ -190,6 +190,7 @@
>
> #include <linux/linkage.h>
> #include <linux/types.h>
> +#include <trace/events/arm_smccc.h>
>
> enum arm_smccc_conduit {
> SMCCC_CONDUIT_NONE,
> @@ -343,13 +344,25 @@ asmlinkage void __arm_smccc_hvc(unsigned long a0, unsigned long a1,
> unsigned long a5, unsigned long a6, unsigned long a7,
> struct arm_smccc_res *res, struct arm_smccc_quirk *quirk);
>
> -#define arm_smccc_smc(...) __arm_smccc_smc(__VA_ARGS__, NULL)
> +#define __arm_smccc(call, arg0, arg1, arg2, arg3, arg4, arg5, arg6, arg7, res, quirk) \
> + do { \
> + trace_arm_smccc_start(tracepoint_string(#call), \
> + (arg0), (arg1), (arg2), (arg3), (arg4), \
> + (arg5), (arg6), (arg7)); \
> + __arm_smccc_##call((arg0), (arg1), (arg2), (arg3), (arg4), (arg5), (arg6), (arg7),\
> + (res), (quirk)); \
> + trace_arm_smccc_end(tracepoint_string(#call), (arg0), \
> + (res)->a0, (res)->a1, (res)->a2, (res)->a3); \
> + } while (0)
> +
>
> -#define arm_smccc_smc_quirk(...) __arm_smccc_smc(__VA_ARGS__)
> +#define arm_smccc_smc(...) __arm_smccc(smc, __VA_ARGS__, NULL)
>
> -#define arm_smccc_hvc(...) __arm_smccc_hvc(__VA_ARGS__, NULL)
> +#define arm_smccc_smc_quirk(...) __arm_smccc(smc, __VA_ARGS__)
>
> -#define arm_smccc_hvc_quirk(...) __arm_smccc_hvc(__VA_ARGS__)
> +#define arm_smccc_hvc(...) __arm_smccc(hvc, __VA_ARGS__, NULL)
> +
> +#define arm_smccc_hvc_quirk(...) __arm_smccc(hvc, __VA_ARGS__)
>
> /* SMCCC v1.1 implementation madness follows */
> #ifdef CONFIG_ARM64
> @@ -450,22 +463,35 @@ asmlinkage void __arm_smccc_hvc(unsigned long a0, unsigned long a1,
> : smccc_sve_clobbers "memory"
> #define __constraints(count) ___constraints(count)
>
> +#define __trace_arg_0 ((unsigned long)arg0)
> +#define __trace_arg_1 __trace_arg_0, ((unsigned long)arg1)
> +#define __trace_arg_2 __trace_arg_1, ((unsigned long)arg2)
> +#define __trace_arg_3 __trace_arg_2, ((unsigned long)arg3)
> +#define __trace_arg_4 __trace_arg_3, ((unsigned long)arg4)
> +#define __trace_arg_5 __trace_arg_4, ((unsigned long)arg5)
> +#define __trace_arg_6 __trace_arg_5, ((unsigned long)arg6)
> +#define __trace_arg_7 __trace_arg_6, ((unsigned long)arg7)
> +#define ___trace_args(count) __trace_arg_##count
> +#define __trace_args(count) ___trace_args(count)
> +
> /*
> * We have an output list that is not necessarily used, and GCC feels
> * entitled to optimise the whole sequence away. "volatile" is what
> * makes it stick.
> */
> -#define __arm_smccc_1_1(inst, ...) \
> +#define __arm_smccc_1_1(inst, a0, ...) \
> do { \
> register unsigned long r0 asm("r0"); \
> register unsigned long r1 asm("r1"); \
> register unsigned long r2 asm("r2"); \
> register unsigned long r3 asm("r3"); \
> __declare_args(__count_args(__VA_ARGS__), __VA_ARGS__); \
> + trace_arm_smccc_start_vargs(tracepoint_string(inst), __trace_args(__count_args(a0, __VA_ARGS__))); \
> asm volatile(SMCCC_SVE_CHECK \
> inst "\n" : \
> "=r" (r0), "=r" (r1), "=r" (r2), "=r" (r3) \
> - __constraints(__count_args(__VA_ARGS__))); \
> + __constraints(__count_args(a0, __VA_ARGS__))); \
> + trace_arm_smccc_end(tracepoint_string(inst), a0, r0, r1, r2, r3); \
> if (___res) \
> *___res = (typeof(*___res)){r0, r1, r2, r3}; \
> } while (0)
We have other functions for incoking SMCCC calls, e.g.
* arm_smccc_smc()
* arm_smccc_hvc()
* arm_smccc_1_2_hvc()
* arm_smccc_1_2_smc()
... so if you really need to hook all SMCCC calls I'd expect you need to
hook those too, somehow
Thanks,
Mark.
> diff --git a/include/trace/events/arm_smccc.h b/include/trace/events/arm_smccc.h
> new file mode 100644
> index 000000000000..533377d2d066
> --- /dev/null
> +++ b/include/trace/events/arm_smccc.h
> @@ -0,0 +1,99 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/* Copyright (c) 2021, Qualcomm Innovation Center, Inc. All rights reserved.
> + */
> +
> +#if IS_REACHABLE(CONFIG_TRACE_ARM_SMCCC) && !defined(__KVM_NVHE_HYPERVISOR__)
> +
> +#undef TRACE_SYSTEM
> +#define TRACE_SYSTEM arm_smccc
> +
> +#if !defined(_TRACE_ARM_SMCCC_H) || defined(TRACE_HEADER_MULTI_READ)
> +#define _TRACE_ARM_SMCCC_H
> +
> +
> +#include <linux/tracepoint.h>
> +
> +TRACE_EVENT(arm_smccc_start,
> + TP_PROTO(const char *call, unsigned long a0, unsigned long a1, unsigned long a2,
> + unsigned long a3, unsigned long a4, unsigned long a5, unsigned long a6,
> + unsigned long a7),
> +
> + TP_ARGS(call, a0, a1, a2, a3, a4, a5, a6, a7),
> +
> + TP_STRUCT__entry(
> + __field(unsigned long, a0)
> + __field(unsigned long, a1)
> + __field(unsigned long, a2)
> + __field(unsigned long, a3)
> + __field(unsigned long, a4)
> + __field(unsigned long, a5)
> + __field(unsigned long, a6)
> + __field(unsigned long, a7)
> + __string(call, call)
> + ),
> +
> + TP_fast_assign(
> + __entry->a0 = a0;
> + __entry->a1 = a1;
> + __entry->a2 = a2;
> + __entry->a3 = a3;
> + __entry->a4 = a4;
> + __entry->a5 = a5;
> + __entry->a6 = a6;
> + __entry->a7 = a7;
> +
> + __assign_str(call, call);
> + ),
> +
> + TP_printk("call=%s fn=%04x args={%0lx %0lx %0lx %0lx %0lx %0lx %0lx}",
> + __get_str(call), __entry->a0, __entry->a1, __entry->a2,
> + __entry->a3, __entry->a4, __entry->a5, __entry->a6, __entry->a7)
> +);
> +
> +TRACE_EVENT(arm_smccc_end,
> + TP_PROTO(const char *call, unsigned long a0, unsigned long r0, unsigned long r1,
> + unsigned long r2, unsigned long r3),
> +
> + TP_ARGS(call, a0, r0, r1, r2, r3),
> +
> + TP_STRUCT__entry(
> + __field(unsigned long, a0)
> + __field(unsigned long, r0)
> + __field(unsigned long, r1)
> + __field(unsigned long, r2)
> + __field(unsigned long, r3)
> + __string(call, call)
> + ),
> +
> + TP_fast_assign(
> + __entry->a0 = a0;
> + __entry->r0 = r0;
> + __entry->r1 = r1;
> + __entry->r2 = r2;
> + __entry->r3 = r3;
> +
> + __assign_str(call, call);
> + ),
> +
> + TP_printk("call=%s fn=%04x res={%0lx %0lx %0lx %0lx}",
> + __get_str(call), __entry->a0, __entry->r0, __entry->r1,
> + __entry->r2, __entry->r3)
> +);
> +
> +#define _trace_arm_smccc_start(call, a0, a1, a2, a3, a4, a5, a6, a7, ...) \
> + trace_arm_smccc_start(call, a0, a1, a2, a3, a4, a5, a6, a7)
> +
> +#define trace_arm_smccc_start_vargs(call, ...) \
> + _trace_arm_smccc_start(call, __VA_ARGS__, 0, 0, 0, 0, 0, 0, 0, 0)
> +
> +#endif
> +
> +/* This part must be outside protection */
> +#include <trace/define_trace.h>
> +
> +#else
> +#define trace_arm_smccc_start(...)
> +#define trace_arm_smccc_end(...)
> +
> +#define trace_arm_smccc_start_vargs(...)
> +#endif
> --
> 2.33.0
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] firmware: smccc: Add tracepoints when SMCCC calls are made
2021-09-23 11:21 ` Mark Rutland
@ 2021-09-24 2:31 ` Bjorn Andersson
-1 siblings, 0 replies; 6+ messages in thread
From: Bjorn Andersson @ 2021-09-24 2:31 UTC (permalink / raw)
To: Mark Rutland
Cc: Elliot Berman, Lorenzo Pieralisi, Sudeep Holla, Steven Rostedt,
Ingo Molnar, Prasad Sodagudi, Guru Das Srinagesh, linux-kernel,
linux-arm-kernel, linux-arm-msm
On Thu 23 Sep 06:21 CDT 2021, Mark Rutland wrote:
> On Wed, Sep 22, 2021 at 03:37:00PM -0700, Elliot Berman wrote:
> > Add trace events for SMCCC calls. These traces allow for convenient
> > mechanism for kernel to log SMC/HVC instructions without requiring
> > extracting traces from firmware. SMCCC spec currently [1] allows for 7
> > argument registers and 4 result registers.
>
> I think you've missed additions in recent versions of the spec. Since
> SMCCCv1.2 (which is described in version of the document you've linked),
> SMC64 calls may pass up to 17 argument registers (x1-x17) and receive up
> to 18 result registers (x0-x17). SMC32 calls have up to 7 argument
> registers (r1-r7) and up to 8 return registers (r0-r7).
>
> What do you want to use this for? What specifically do you want to
> trace.
>
> I'm worried that this is a very low level transport, and hooking this
> means tracing a bunch of unrelated users (e.g. PSCI, ARCH_WORKAROUND*
> calls, vendor-specific SMC interfaces), and potentially gets in the way
> of some of those use-cases (e.g. tracng this means it cannot be used
> from noinstr code, which we likely need to be able to do in future).
>
> Generally I'd prefer to have tracepoints in specific drivers rather than
> in the SMCCC transport.
>
I agree, putting the tracepoint at this low level will essentially
provide us with a hexdump of all SMC operations and one would more or
less need to so some post processing to get something useful out of it.
And in the few cases where the arguments are references to data buffers
there's no way to trace that content (e.g. qcom_scm_assign_mem()).
If we move it one step up we can provide trace data that's directly
useful to a human, provide insights in the data, allow for proper
filtering etc.
Regards,
Bjorn
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] firmware: smccc: Add tracepoints when SMCCC calls are made
@ 2021-09-24 2:31 ` Bjorn Andersson
0 siblings, 0 replies; 6+ messages in thread
From: Bjorn Andersson @ 2021-09-24 2:31 UTC (permalink / raw)
To: Mark Rutland
Cc: Elliot Berman, Lorenzo Pieralisi, Sudeep Holla, Steven Rostedt,
Ingo Molnar, Prasad Sodagudi, Guru Das Srinagesh, linux-kernel,
linux-arm-kernel, linux-arm-msm
On Thu 23 Sep 06:21 CDT 2021, Mark Rutland wrote:
> On Wed, Sep 22, 2021 at 03:37:00PM -0700, Elliot Berman wrote:
> > Add trace events for SMCCC calls. These traces allow for convenient
> > mechanism for kernel to log SMC/HVC instructions without requiring
> > extracting traces from firmware. SMCCC spec currently [1] allows for 7
> > argument registers and 4 result registers.
>
> I think you've missed additions in recent versions of the spec. Since
> SMCCCv1.2 (which is described in version of the document you've linked),
> SMC64 calls may pass up to 17 argument registers (x1-x17) and receive up
> to 18 result registers (x0-x17). SMC32 calls have up to 7 argument
> registers (r1-r7) and up to 8 return registers (r0-r7).
>
> What do you want to use this for? What specifically do you want to
> trace.
>
> I'm worried that this is a very low level transport, and hooking this
> means tracing a bunch of unrelated users (e.g. PSCI, ARCH_WORKAROUND*
> calls, vendor-specific SMC interfaces), and potentially gets in the way
> of some of those use-cases (e.g. tracng this means it cannot be used
> from noinstr code, which we likely need to be able to do in future).
>
> Generally I'd prefer to have tracepoints in specific drivers rather than
> in the SMCCC transport.
>
I agree, putting the tracepoint at this low level will essentially
provide us with a hexdump of all SMC operations and one would more or
less need to so some post processing to get something useful out of it.
And in the few cases where the arguments are references to data buffers
there's no way to trace that content (e.g. qcom_scm_assign_mem()).
If we move it one step up we can provide trace data that's directly
useful to a human, provide insights in the data, allow for proper
filtering etc.
Regards,
Bjorn
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-09-24 2:34 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-22 22:37 [PATCH 1/1] firmware: smccc: Add tracepoints when SMCCC calls are made Elliot Berman
2021-09-22 22:37 ` Elliot Berman
2021-09-23 11:21 ` Mark Rutland
2021-09-23 11:21 ` Mark Rutland
2021-09-24 2:31 ` Bjorn Andersson
2021-09-24 2:31 ` Bjorn Andersson
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.