| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20250619190315.2603194-4-rkrcmar@ventanamicro.com>
Date: Thu, 19 Jun 2025 21:03:15 +0200
From: Radim Krčmář <rkrcmar@...tanamicro.com>
To: linux-riscv@...ts.infradead.org
Cc: linux-kernel@...r.kernel.org,
Paul Walmsley <paul.walmsley@...ive.com>,
Palmer Dabbelt <palmer@...belt.com>,
Albert Ou <aou@...s.berkeley.edu>,
Alexandre Ghiti <alex@...ti.fr>,
Atish Patra <atishp@...osinc.com>,
Andrew Jones <ajones@...tanamicro.com>,
Clément Léger <cleger@...osinc.com>,
Anup Patel <apatel@...tanamicro.com>,
Thomas Weißschuh <thomas.weissschuh@...utronix.de>,
David Laight <david.laight.linux@...il.com>
Subject: [PATCH v2 3/2] RISC-V: sbi: remove sbi_ecall tracepoints
Tracepoits generate bad code in the non-trace path.
The acceptable tracepoint overhead in the non-tracing path is a nop, and
possibly a second 2 byte nop for alignment, but the actual overhead is
way higher. For example, the sbi_fwft_set with tracepoints:
0xffffffff80022ee8 <+0>: auipc a5,0x2cec
0xffffffff80022eec <+4>: lbu a5,1704(a5) # 0xffffffff82d0f590 <sbi_fwft_supported>
0xffffffff80022ef0 <+8>: beqz a5,0xffffffff80022fa0 <sbi_fwft_set+184>
0xffffffff80022ef2 <+10>: addi sp,sp,-48
0xffffffff80022ef4 <+12>: sd s0,32(sp)
0xffffffff80022ef6 <+14>: sd s1,24(sp)
0xffffffff80022ef8 <+16>: sd s2,16(sp)
0xffffffff80022efa <+18>: sd ra,40(sp)
0xffffffff80022efc <+20>: addi s0,sp,48
0xffffffff80022efe <+22>: slli s1,a0,0x20
0xffffffff80022f02 <+26>: mv s2,a1
0xffffffff80022f04 <+28>: srli s1,s1,0x20
0xffffffff80022f06 <+30>: nop
0xffffffff80022f08 <+32>: nop
0xffffffff80022f0c <+36>: lui a7,0x46574
0xffffffff80022f10 <+40>: mv a0,s1
0xffffffff80022f12 <+42>: mv a1,s2
0xffffffff80022f14 <+44>: addi a7,a7,1620 # 0x46574654
0xffffffff80022f18 <+48>: li a6,0
0xffffffff80022f1a <+50>: ecall
0xffffffff80022f1e <+54>: mv s1,a0
0xffffffff80022f20 <+56>: nop
0xffffffff80022f24 <+60>: addiw a0,s1,14
0xffffffff80022f28 <+64>: li a5,14
0xffffffff80022f2a <+66>: bltu a5,a0,0xffffffff80022f9a <sbi_fwft_set+178>
0xffffffff80022f2e <+70>: slli a5,a0,0x20
0xffffffff80022f32 <+74>: srli a0,a5,0x1e
0xffffffff80022f36 <+78>: auipc a5,0x1c75
0xffffffff80022f3a <+82>: addi a5,a5,-886 # 0xffffffff81c97bc0 <CSWTCH.177>
0xffffffff80022f3e <+86>: add a5,a5,a0
0xffffffff80022f40 <+88>: lw a0,0(a5)
0xffffffff80022f42 <+90>: ld ra,40(sp)
0xffffffff80022f44 <+92>: ld s0,32(sp)
0xffffffff80022f46 <+94>: ld s1,24(sp)
0xffffffff80022f48 <+96>: ld s2,16(sp)
0xffffffff80022f4a <+98>: addi sp,sp,48
0xffffffff80022f4c <+100>: ret
[tracepoint slowpaths]
0xffffffff80022f9a <+178>: li a0,-524
0xffffffff80022f9e <+182>: j 0xffffffff80022f42 <sbi_fwft_set+90>
0xffffffff80022fa0 <+184>: li a0,-95
0xffffffff80022fa4 <+188>: ret
Without tracepoints:
0xffffffff80022b40 <+0>: addi sp,sp,-16
0xffffffff80022b42 <+2>: sd s0,0(sp)
0xffffffff80022b44 <+4>: sd ra,8(sp)
0xffffffff80022b46 <+6>: addi s0,sp,16
0xffffffff80022b48 <+8>: auipc a5,0x2ced
0xffffffff80022b4c <+12>: lbu a5,-1464(a5) # 0xffffffff82d0f590 <sbi_fwft_supported>
0xffffffff80022b50 <+16>: beqz a5,0xffffffff80022b8e <sbi_fwft_set+78>
0xffffffff80022b52 <+18>: lui a7,0x46574
0xffffffff80022b56 <+22>: slli a0,a0,0x20
0xffffffff80022b58 <+24>: srli a0,a0,0x20
0xffffffff80022b5a <+26>: addi a7,a7,1620 # 0x46574654
0xffffffff80022b5e <+30>: li a6,0
0xffffffff80022b60 <+32>: ecall
0xffffffff80022b64 <+36>: li a5,14
0xffffffff80022b66 <+38>: addiw a0,a0,14
0xffffffff80022b68 <+40>: bltu a5,a0,0xffffffff80022b88 <sbi_fwft_set+72>
0xffffffff80022b6c <+44>: slli a5,a0,0x20
0xffffffff80022b70 <+48>: srli a0,a5,0x1e
0xffffffff80022b74 <+52>: auipc a5,0x1c75
0xffffffff80022b78 <+56>: addi a5,a5,-300 # 0xffffffff81c97a48 <CSWTCH.176>
0xffffffff80022b7c <+60>: add a5,a5,a0
0xffffffff80022b7e <+62>: lw a0,0(a5)
0xffffffff80022b80 <+64>: ld ra,8(sp)
0xffffffff80022b82 <+66>: ld s0,0(sp)
0xffffffff80022b84 <+68>: addi sp,sp,16
0xffffffff80022b86 <+70>: ret
0xffffffff80022b88 <+72>: li a0,-524
0xffffffff80022b8c <+76>: j 0xffffffff80022b80 <sbi_fwft_set+64>
0xffffffff80022b8e <+78>: li a0,-95
0xffffffff80022b92 <+82>: j 0xffffffff80022b80 <sbi_fwft_set+64>
It would be nice if RISC-V had a way to tell compilers to generate the
desired code, because if this issue isn't limited to ecall tracepoints,
then disabling CONFIG_TRACEPOINTS is starting to look good. :)
Signed-off-by: Radim Krčmář <rkrcmar@...tanamicro.com>
---
arch/riscv/include/asm/sbi.h | 30 ++--------------------------
arch/riscv/include/asm/trace.h | 36 ----------------------------------
arch/riscv/kernel/sbi_ecall.c | 18 -----------------
3 files changed, 2 insertions(+), 82 deletions(-)
diff --git a/arch/riscv/include/asm/sbi.h b/arch/riscv/include/asm/sbi.h
index 7aff31583a3d..ffab0614d24a 100644
--- a/arch/riscv/include/asm/sbi.h
+++ b/arch/riscv/include/asm/sbi.h
@@ -11,7 +11,6 @@
#include <linux/types.h>
#include <linux/cpumask.h>
#include <linux/jump_label.h>
-#include <linux/tracepoint-defs.h>
#ifdef CONFIG_RISCV_SBI
enum sbi_ext_id {
@@ -461,16 +460,6 @@ struct sbiret {
long value;
};
-#ifdef CONFIG_TRACEPOINTS
-DECLARE_TRACEPOINT(sbi_call);
-DECLARE_TRACEPOINT(sbi_return);
-extern void do_trace_sbi_call(int ext, int fid);
-extern void do_trace_sbi_return(int ext, long error, long value);
-#else
-static inline void do_trace_sbi_call(int ext, int fid) {};
-static inline void do_trace_sbi_return(int ext, long error, long value) {};
-#endif
-
void sbi_init(void);
long __sbi_base_ecall(int fid);
@@ -509,32 +498,17 @@ long __sbi_base_ecall(int fid);
#define __sbi_ecall_constraints7 __sbi_ecall_constraints6, "r" (__a4)
#define __sbi_ecall_constraints8 __sbi_ecall_constraints7, "r" (__a5)
-#define __sbi_ecall_trace_call() \
- if (tracepoint_enabled(sbi_call)) \
- do_trace_sbi_call(__ta7, __ta6)
-
-#define __sbi_ecall_trace_return() \
- if (tracepoint_enabled(sbi_return)) \
- do_trace_sbi_return(__ta7, __ret.error, __ret.value)
-
-/*
- * Clear a1 to avoid leaking unrelated kernel state through tracepoints in case
- * the register doesn't get overwritten by the ecall nor the arguments.
- */
#define sbi_ecall(A...) \
({ \
CONCATENATE(__sbi_ecall_args, COUNT_ARGS(A))(A); \
- __sbi_ecall_trace_call(); \
register uintptr_t __r0 asm ("a0"); \
- register uintptr_t __r1 asm ("a1") = 0; \
+ register uintptr_t __r1 asm ("a1"); \
CONCATENATE(__sbi_ecall_regs, COUNT_ARGS(A)); \
asm volatile ("ecall" \
: "=r" (__r0), "=r" (__r1) \
: CONCATENATE(__sbi_ecall_constraints, COUNT_ARGS(A)) \
: "memory"); \
- struct sbiret __ret = {.error = __r0, .value = __r1}; \
- __sbi_ecall_trace_return(); \
- __ret; \
+ (struct sbiret){.error = __r0, .value = __r1}; \
})
#ifdef CONFIG_RISCV_SBI_V01
diff --git a/arch/riscv/include/asm/trace.h b/arch/riscv/include/asm/trace.h
index 6151cee5450c..7c99d91fcce3 100644
--- a/arch/riscv/include/asm/trace.h
+++ b/arch/riscv/include/asm/trace.h
@@ -7,42 +7,6 @@
#include <linux/tracepoint.h>
-TRACE_EVENT_CONDITION(sbi_call,
- TP_PROTO(int ext, int fid),
- TP_ARGS(ext, fid),
- TP_CONDITION(ext != SBI_EXT_HSM),
-
- TP_STRUCT__entry(
- __field(int, ext)
- __field(int, fid)
- ),
-
- TP_fast_assign(
- __entry->ext = ext;
- __entry->fid = fid;
- ),
-
- TP_printk("ext=0x%x fid=%d", __entry->ext, __entry->fid)
-);
-
-TRACE_EVENT_CONDITION(sbi_return,
- TP_PROTO(int ext, long error, long value),
- TP_ARGS(ext, error, value),
- TP_CONDITION(ext != SBI_EXT_HSM),
-
- TP_STRUCT__entry(
- __field(long, error)
- __field(long, value)
- ),
-
- TP_fast_assign(
- __entry->error = error;
- __entry->value = value;
- ),
-
- TP_printk("error=%ld value=0x%lx", __entry->error, __entry->value)
-);
-
#endif /* _TRACE_RISCV_H */
#undef TRACE_INCLUDE_PATH
diff --git a/arch/riscv/kernel/sbi_ecall.c b/arch/riscv/kernel/sbi_ecall.c
index b783a46fff1c..62489ffeae2c 100644
--- a/arch/riscv/kernel/sbi_ecall.c
+++ b/arch/riscv/kernel/sbi_ecall.c
@@ -2,8 +2,6 @@
/* Copyright (c) 2024 Rivos Inc. */
#include <asm/sbi.h>
-#define CREATE_TRACE_POINTS
-#include <asm/trace.h>
long __sbi_base_ecall(int fid)
{
@@ -16,19 +14,3 @@ long __sbi_base_ecall(int fid)
return sbi_err_map_linux_errno(ret.error);
}
EXPORT_SYMBOL(__sbi_base_ecall);
-
-#ifdef CONFIG_TRACEPOINTS
-void do_trace_sbi_call(int ext, int fid)
-{
- trace_sbi_call(ext, fid);
-}
-EXPORT_SYMBOL(do_trace_sbi_call);
-EXPORT_TRACEPOINT_SYMBOL(sbi_call);
-
-void do_trace_sbi_return(int ext, long error, long value)
-{
- trace_sbi_return(ext, error, value);
-}
-EXPORT_SYMBOL(do_trace_sbi_return);
-EXPORT_TRACEPOINT_SYMBOL(sbi_return);
-#endif
--
2.49.0
Powered by blists - more mailing lists