For now, we can use larch_insn_gen_break() to define KPROBE_BP_INSN and
KPROBE_SSTEPBP_INSN. Because larch_insn_gen_break() returns instruction
word, pass around instruction word instead of union for the functions
insns_not_supported(), insns_need_simulation() and arch_simulate_insn(),
no functional change intended.
Signed-off-by: Tiezhu Yang <[email protected]>
---
arch/loongarch/include/asm/inst.h | 18 +++++++++++++++---
arch/loongarch/include/asm/kprobes.h | 2 +-
arch/loongarch/kernel/kprobes.c | 21 +++++----------------
3 files changed, 21 insertions(+), 20 deletions(-)
diff --git a/arch/loongarch/include/asm/inst.h b/arch/loongarch/include/asm/inst.h
index 95b3c20..a0fce06 100644
--- a/arch/loongarch/include/asm/inst.h
+++ b/arch/loongarch/include/asm/inst.h
@@ -409,8 +409,12 @@ static inline bool is_self_loop_ins(union loongarch_instruction *ip, struct pt_r
void simu_pc(struct pt_regs *regs, union loongarch_instruction insn);
void simu_branch(struct pt_regs *regs, union loongarch_instruction insn);
-static inline bool insns_not_supported(union loongarch_instruction insn)
+static inline bool insns_not_supported(u32 code)
{
+ union loongarch_instruction insn;
+
+ insn.word = code;
+
switch (insn.reg2i14_format.opcode) {
case llw_op:
case lld_op:
@@ -429,8 +433,12 @@ static inline bool insns_not_supported(union loongarch_instruction insn)
return false;
}
-static inline bool insns_need_simulation(union loongarch_instruction insn)
+static inline bool insns_need_simulation(u32 code)
{
+ union loongarch_instruction insn;
+
+ insn.word = code;
+
if (is_pc_ins(&insn))
return true;
@@ -440,8 +448,12 @@ static inline bool insns_need_simulation(union loongarch_instruction insn)
return false;
}
-static inline void arch_simulate_insn(union loongarch_instruction insn, struct pt_regs *regs)
+static inline void arch_simulate_insn(u32 code, struct pt_regs *regs)
{
+ union loongarch_instruction insn;
+
+ insn.word = code;
+
if (is_pc_ins(&insn))
simu_pc(regs, insn);
else if (is_branch_ins(&insn))
diff --git a/arch/loongarch/include/asm/kprobes.h b/arch/loongarch/include/asm/kprobes.h
index 798020a..7ef7a0f 100644
--- a/arch/loongarch/include/asm/kprobes.h
+++ b/arch/loongarch/include/asm/kprobes.h
@@ -22,7 +22,7 @@ do { \
#define kretprobe_blacklist_size 0
-typedef union loongarch_instruction kprobe_opcode_t;
+typedef u32 kprobe_opcode_t;
/* Architecture specific copy of original instruction */
struct arch_specific_insn {
diff --git a/arch/loongarch/kernel/kprobes.c b/arch/loongarch/kernel/kprobes.c
index 08c78d2..9f699f0 100644
--- a/arch/loongarch/kernel/kprobes.c
+++ b/arch/loongarch/kernel/kprobes.c
@@ -4,19 +4,8 @@
#include <linux/preempt.h>
#include <asm/break.h>
-static const union loongarch_instruction breakpoint_insn = {
- .reg0i15_format = {
- .opcode = break_op,
- .immediate = BRK_KPROBE_BP,
- }
-};
-
-static const union loongarch_instruction singlestep_insn = {
- .reg0i15_format = {
- .opcode = break_op,
- .immediate = BRK_KPROBE_SSTEPBP,
- }
-};
+#define KPROBE_BP_INSN larch_insn_gen_break(BRK_KPROBE_BP)
+#define KPROBE_SSTEPBP_INSN larch_insn_gen_break(BRK_KPROBE_SSTEPBP)
DEFINE_PER_CPU(struct kprobe *, current_kprobe);
DEFINE_PER_CPU(struct kprobe_ctlblk, kprobe_ctlblk);
@@ -24,7 +13,7 @@ DEFINE_PER_CPU(struct kprobe_ctlblk, kprobe_ctlblk);
static void arch_prepare_ss_slot(struct kprobe *p)
{
p->ainsn.insn[0] = *p->addr;
- p->ainsn.insn[1] = singlestep_insn;
+ p->ainsn.insn[1] = KPROBE_SSTEPBP_INSN;
p->ainsn.restore = (unsigned long)p->addr + LOONGARCH_INSN_SIZE;
}
NOKPROBE_SYMBOL(arch_prepare_ss_slot);
@@ -68,7 +57,7 @@ NOKPROBE_SYMBOL(arch_prepare_kprobe);
/* Install breakpoint in text */
void arch_arm_kprobe(struct kprobe *p)
{
- *p->addr = breakpoint_insn;
+ *p->addr = KPROBE_BP_INSN;
flush_insn_slot(p);
}
NOKPROBE_SYMBOL(arch_arm_kprobe);
@@ -253,7 +242,7 @@ bool kprobe_breakpoint_handler(struct pt_regs *regs)
}
}
- if (addr->word != breakpoint_insn.word) {
+ if (*addr != KPROBE_BP_INSN) {
/*
* The breakpoint instruction was removed right
* after we hit it. Another cpu has removed
--
2.1.0
On Wed, 2023-04-12 at 18:05 +0800, Tiezhu Yang wrote:
> -static inline bool insns_not_supported(union loongarch_instruction insn)
> +static inline bool insns_not_supported(u32 code)
> {
> + union loongarch_instruction insn;
> +
> + insn.word = code;
> +
I remember Xuerui disliked this change. Maybe we can add
__attribute__ (__transparent_union__) [1] for union
loongarch_instruction instead?
[1]: https://gcc.gnu.org/onlinedocs/gcc/Common-Type-Attributes.html#index-transparent_005funion-type-attribute
--
Xi Ruoyao <[email protected]>
School of Aerospace Science and Technology, Xidian University
On 04/12/2023 07:08 PM, Xi Ruoyao wrote:
> On Wed, 2023-04-12 at 18:05 +0800, Tiezhu Yang wrote:
>> -static inline bool insns_not_supported(union loongarch_instruction insn)
>> +static inline bool insns_not_supported(u32 code)
>> {
>> + union loongarch_instruction insn;
>> +
>> + insn.word = code;
>> +
>
> I remember Xuerui disliked this change. Maybe we can add
> __attribute__ (__transparent_union__) [1] for union
> loongarch_instruction instead?
>
> [1]: https://gcc.gnu.org/onlinedocs/gcc/Common-Type-Attributes.html#index-transparent_005funion-type-attribute
>
OK, let me pass union parameter, then define a local variable
to do type conversion for callers, the changes are small and
then the code may be more readable, thank you.
Thanks,
Tiezhu