From: guoren@kernel.org
To: guoren@kernel.org, palmer@dabbelt.com, paul.walmsley@sifive.com,
mhiramat@kernel.org, conor.dooley@microchip.com,
penberg@kernel.org, mark.rutland@arm.com
Cc: linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org,
Guo Ren <guoren@linux.alibaba.com>
Subject: [PATCH] riscv: kprobe: Optimize kprobe with accurate atomicity
Date: Thu, 26 Jan 2023 11:15:59 -0500 [thread overview]
Message-ID: <20230126161559.1467374-1-guoren@kernel.org> (raw)
From: Guo Ren <guoren@linux.alibaba.com>
The previous implementation was based on the stop_matchine mechanism,
which reduced the speed of arm/disarm_kprobe. Using minimum ebreak
instruction would get accurate atomicity.
This patch removes the patch_text of riscv, which is based on
stop_machine. Then riscv only reserved patch_text_nosync, and developers
need to be more careful in dealing with patch_text atomicity.
When CONFIG_RISCV_ISA_C=n, the ebreak could replace the whole
instruction. When CONFIG_RISCV_ISA_C=y, the patch uses 16-bit length
c.ebreak instruction, which may occupy the first part of the 32-bit
instruction and leave half the rest of the broken instruction. Because
ebreak could detour the flow to skip it, leaving it in the kernel text
memory is okay.
Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
Signed-off-by: Guo Ren <guoren@kernel.org>
---
arch/riscv/include/asm/patch.h | 1 -
arch/riscv/kernel/patch.c | 33 ------------------------------
arch/riscv/kernel/probes/kprobes.c | 29 ++++++++++++++++++--------
3 files changed, 21 insertions(+), 42 deletions(-)
diff --git a/arch/riscv/include/asm/patch.h b/arch/riscv/include/asm/patch.h
index 9a7d7346001e..2500782e6f5b 100644
--- a/arch/riscv/include/asm/patch.h
+++ b/arch/riscv/include/asm/patch.h
@@ -7,6 +7,5 @@
#define _ASM_RISCV_PATCH_H
int patch_text_nosync(void *addr, const void *insns, size_t len);
-int patch_text(void *addr, u32 insn);
#endif /* _ASM_RISCV_PATCH_H */
diff --git a/arch/riscv/kernel/patch.c b/arch/riscv/kernel/patch.c
index 765004b60513..8bd51ed8b806 100644
--- a/arch/riscv/kernel/patch.c
+++ b/arch/riscv/kernel/patch.c
@@ -98,36 +98,3 @@ int patch_text_nosync(void *addr, const void *insns, size_t len)
return ret;
}
NOKPROBE_SYMBOL(patch_text_nosync);
-
-static int patch_text_cb(void *data)
-{
- struct patch_insn *patch = data;
- int ret = 0;
-
- if (atomic_inc_return(&patch->cpu_count) == num_online_cpus()) {
- ret =
- patch_text_nosync(patch->addr, &patch->insn,
- GET_INSN_LENGTH(patch->insn));
- atomic_inc(&patch->cpu_count);
- } else {
- while (atomic_read(&patch->cpu_count) <= num_online_cpus())
- cpu_relax();
- smp_mb();
- }
-
- return ret;
-}
-NOKPROBE_SYMBOL(patch_text_cb);
-
-int patch_text(void *addr, u32 insn)
-{
- struct patch_insn patch = {
- .addr = addr,
- .insn = insn,
- .cpu_count = ATOMIC_INIT(0),
- };
-
- return stop_machine_cpuslocked(patch_text_cb,
- &patch, cpu_online_mask);
-}
-NOKPROBE_SYMBOL(patch_text);
diff --git a/arch/riscv/kernel/probes/kprobes.c b/arch/riscv/kernel/probes/kprobes.c
index 475989f06d6d..27f8960c321c 100644
--- a/arch/riscv/kernel/probes/kprobes.c
+++ b/arch/riscv/kernel/probes/kprobes.c
@@ -24,12 +24,18 @@ post_kprobe_handler(struct kprobe *, struct kprobe_ctlblk *, struct pt_regs *);
static void __kprobes arch_prepare_ss_slot(struct kprobe *p)
{
unsigned long offset = GET_INSN_LENGTH(p->opcode);
+#ifdef CONFIG_RISCV_ISA_C
+ u32 opcode = __BUG_INSN_16;
+#else
+ u32 opcode = __BUG_INSN_32;
+#endif
p->ainsn.api.restore = (unsigned long)p->addr + offset;
- patch_text(p->ainsn.api.insn, p->opcode);
- patch_text((void *)((unsigned long)(p->ainsn.api.insn) + offset),
- __BUG_INSN_32);
+ patch_text_nosync(p->ainsn.api.insn, &p->opcode, offset);
+ patch_text_nosync((void *)((unsigned long)(p->ainsn.api.insn) + offset),
+ &opcode, GET_INSN_LENGTH(opcode));
+
}
static void __kprobes arch_prepare_simulate(struct kprobe *p)
@@ -114,16 +120,23 @@ void *alloc_insn_page(void)
/* install breakpoint in text */
void __kprobes arch_arm_kprobe(struct kprobe *p)
{
- if ((p->opcode & __INSN_LENGTH_MASK) == __INSN_LENGTH_32)
- patch_text(p->addr, __BUG_INSN_32);
- else
- patch_text(p->addr, __BUG_INSN_16);
+#ifdef CONFIG_RISCV_ISA_C
+ u32 opcode = __BUG_INSN_16;
+#else
+ u32 opcode = __BUG_INSN_32;
+#endif
+ patch_text_nosync(p->addr, &opcode, GET_INSN_LENGTH(opcode));
}
/* remove breakpoint from text */
void __kprobes arch_disarm_kprobe(struct kprobe *p)
{
- patch_text(p->addr, p->opcode);
+#ifdef CONFIG_RISCV_ISA_C
+ u32 opcode = __BUG_INSN_16;
+#else
+ u32 opcode = __BUG_INSN_32;
+#endif
+ patch_text_nosync(p->addr, &p->opcode, GET_INSN_LENGTH(opcode));
}
void __kprobes arch_remove_kprobe(struct kprobe *p)
--
2.36.1
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
WARNING: multiple messages have this Message-ID (diff)
From: guoren@kernel.org
To: guoren@kernel.org, palmer@dabbelt.com, paul.walmsley@sifive.com,
mhiramat@kernel.org, conor.dooley@microchip.com,
penberg@kernel.org, mark.rutland@arm.com
Cc: linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org,
Guo Ren <guoren@linux.alibaba.com>
Subject: [PATCH] riscv: kprobe: Optimize kprobe with accurate atomicity
Date: Thu, 26 Jan 2023 11:15:59 -0500 [thread overview]
Message-ID: <20230126161559.1467374-1-guoren@kernel.org> (raw)
From: Guo Ren <guoren@linux.alibaba.com>
The previous implementation was based on the stop_matchine mechanism,
which reduced the speed of arm/disarm_kprobe. Using minimum ebreak
instruction would get accurate atomicity.
This patch removes the patch_text of riscv, which is based on
stop_machine. Then riscv only reserved patch_text_nosync, and developers
need to be more careful in dealing with patch_text atomicity.
When CONFIG_RISCV_ISA_C=n, the ebreak could replace the whole
instruction. When CONFIG_RISCV_ISA_C=y, the patch uses 16-bit length
c.ebreak instruction, which may occupy the first part of the 32-bit
instruction and leave half the rest of the broken instruction. Because
ebreak could detour the flow to skip it, leaving it in the kernel text
memory is okay.
Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
Signed-off-by: Guo Ren <guoren@kernel.org>
---
arch/riscv/include/asm/patch.h | 1 -
arch/riscv/kernel/patch.c | 33 ------------------------------
arch/riscv/kernel/probes/kprobes.c | 29 ++++++++++++++++++--------
3 files changed, 21 insertions(+), 42 deletions(-)
diff --git a/arch/riscv/include/asm/patch.h b/arch/riscv/include/asm/patch.h
index 9a7d7346001e..2500782e6f5b 100644
--- a/arch/riscv/include/asm/patch.h
+++ b/arch/riscv/include/asm/patch.h
@@ -7,6 +7,5 @@
#define _ASM_RISCV_PATCH_H
int patch_text_nosync(void *addr, const void *insns, size_t len);
-int patch_text(void *addr, u32 insn);
#endif /* _ASM_RISCV_PATCH_H */
diff --git a/arch/riscv/kernel/patch.c b/arch/riscv/kernel/patch.c
index 765004b60513..8bd51ed8b806 100644
--- a/arch/riscv/kernel/patch.c
+++ b/arch/riscv/kernel/patch.c
@@ -98,36 +98,3 @@ int patch_text_nosync(void *addr, const void *insns, size_t len)
return ret;
}
NOKPROBE_SYMBOL(patch_text_nosync);
-
-static int patch_text_cb(void *data)
-{
- struct patch_insn *patch = data;
- int ret = 0;
-
- if (atomic_inc_return(&patch->cpu_count) == num_online_cpus()) {
- ret =
- patch_text_nosync(patch->addr, &patch->insn,
- GET_INSN_LENGTH(patch->insn));
- atomic_inc(&patch->cpu_count);
- } else {
- while (atomic_read(&patch->cpu_count) <= num_online_cpus())
- cpu_relax();
- smp_mb();
- }
-
- return ret;
-}
-NOKPROBE_SYMBOL(patch_text_cb);
-
-int patch_text(void *addr, u32 insn)
-{
- struct patch_insn patch = {
- .addr = addr,
- .insn = insn,
- .cpu_count = ATOMIC_INIT(0),
- };
-
- return stop_machine_cpuslocked(patch_text_cb,
- &patch, cpu_online_mask);
-}
-NOKPROBE_SYMBOL(patch_text);
diff --git a/arch/riscv/kernel/probes/kprobes.c b/arch/riscv/kernel/probes/kprobes.c
index 475989f06d6d..27f8960c321c 100644
--- a/arch/riscv/kernel/probes/kprobes.c
+++ b/arch/riscv/kernel/probes/kprobes.c
@@ -24,12 +24,18 @@ post_kprobe_handler(struct kprobe *, struct kprobe_ctlblk *, struct pt_regs *);
static void __kprobes arch_prepare_ss_slot(struct kprobe *p)
{
unsigned long offset = GET_INSN_LENGTH(p->opcode);
+#ifdef CONFIG_RISCV_ISA_C
+ u32 opcode = __BUG_INSN_16;
+#else
+ u32 opcode = __BUG_INSN_32;
+#endif
p->ainsn.api.restore = (unsigned long)p->addr + offset;
- patch_text(p->ainsn.api.insn, p->opcode);
- patch_text((void *)((unsigned long)(p->ainsn.api.insn) + offset),
- __BUG_INSN_32);
+ patch_text_nosync(p->ainsn.api.insn, &p->opcode, offset);
+ patch_text_nosync((void *)((unsigned long)(p->ainsn.api.insn) + offset),
+ &opcode, GET_INSN_LENGTH(opcode));
+
}
static void __kprobes arch_prepare_simulate(struct kprobe *p)
@@ -114,16 +120,23 @@ void *alloc_insn_page(void)
/* install breakpoint in text */
void __kprobes arch_arm_kprobe(struct kprobe *p)
{
- if ((p->opcode & __INSN_LENGTH_MASK) == __INSN_LENGTH_32)
- patch_text(p->addr, __BUG_INSN_32);
- else
- patch_text(p->addr, __BUG_INSN_16);
+#ifdef CONFIG_RISCV_ISA_C
+ u32 opcode = __BUG_INSN_16;
+#else
+ u32 opcode = __BUG_INSN_32;
+#endif
+ patch_text_nosync(p->addr, &opcode, GET_INSN_LENGTH(opcode));
}
/* remove breakpoint from text */
void __kprobes arch_disarm_kprobe(struct kprobe *p)
{
- patch_text(p->addr, p->opcode);
+#ifdef CONFIG_RISCV_ISA_C
+ u32 opcode = __BUG_INSN_16;
+#else
+ u32 opcode = __BUG_INSN_32;
+#endif
+ patch_text_nosync(p->addr, &p->opcode, GET_INSN_LENGTH(opcode));
}
void __kprobes arch_remove_kprobe(struct kprobe *p)
--
2.36.1
next reply other threads:[~2023-01-26 16:16 UTC|newest]
Thread overview: 54+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-01-26 16:15 guoren [this message]
2023-01-26 16:15 ` [PATCH] riscv: kprobe: Optimize kprobe with accurate atomicity guoren
2023-01-28 3:52 ` liaochang (A)
2023-01-28 3:52 ` liaochang (A)
2023-01-28 4:45 ` Guo Ren
2023-01-28 4:45 ` Guo Ren
2023-01-30 15:28 ` Björn Töpel
2023-01-30 15:28 ` Björn Töpel
2023-01-30 15:49 ` Mark Rutland
2023-01-30 15:49 ` Mark Rutland
2023-01-30 16:56 ` Björn Töpel
2023-01-30 16:56 ` Björn Töpel
2023-01-31 1:48 ` Guo Ren
2023-01-31 1:48 ` Guo Ren
2023-01-31 7:12 ` Björn Töpel
2023-01-31 7:12 ` Björn Töpel
2023-01-31 8:30 ` Guo Ren
2023-01-31 8:30 ` Guo Ren
2023-01-31 10:33 ` Mark Rutland
2023-01-31 10:33 ` Mark Rutland
2023-02-16 15:23 ` Masami Hiramatsu
2023-02-16 15:23 ` Masami Hiramatsu
2023-02-20 10:35 ` Mark Rutland
2023-02-20 10:35 ` Mark Rutland
2023-02-21 1:30 ` Guo Ren
2023-02-21 1:30 ` Guo Ren
2023-01-31 1:01 ` Guo Ren
2023-01-31 1:01 ` Guo Ren
2023-01-31 1:09 ` Guo Ren
2023-01-31 1:09 ` Guo Ren
2023-01-31 7:03 ` Björn Töpel
2023-01-31 7:03 ` Björn Töpel
2023-01-31 8:27 ` Guo Ren
2023-01-31 8:27 ` Guo Ren
2023-01-31 6:40 ` Björn Töpel
2023-01-31 6:40 ` Björn Töpel
2023-01-31 8:15 ` Guo Ren
2023-01-31 8:15 ` Guo Ren
2023-01-31 10:56 ` Andrea Parri
2023-01-31 10:56 ` Andrea Parri
2023-01-31 13:23 ` Guo Ren
2023-01-31 13:23 ` Guo Ren
2023-02-16 7:54 ` Björn Töpel
2023-02-16 7:54 ` Björn Töpel
2023-02-17 2:28 ` Guo Ren
2023-02-17 2:28 ` Guo Ren
2023-02-17 7:32 ` Björn Töpel
2023-02-17 7:32 ` Björn Töpel
2023-02-21 1:56 ` Guo Ren
2023-02-21 1:56 ` Guo Ren
2023-02-16 15:42 ` Masami Hiramatsu
2023-02-16 15:42 ` Masami Hiramatsu
2023-02-21 0:57 ` Guo Ren
2023-02-21 0:57 ` Guo Ren
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230126161559.1467374-1-guoren@kernel.org \
--to=guoren@kernel.org \
--cc=conor.dooley@microchip.com \
--cc=guoren@linux.alibaba.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-riscv@lists.infradead.org \
--cc=mark.rutland@arm.com \
--cc=mhiramat@kernel.org \
--cc=palmer@dabbelt.com \
--cc=paul.walmsley@sifive.com \
--cc=penberg@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.